Skip to content

Commit 4e9e424

Browse files
audexdevclaude
andcommitted
Resolve LAC_THREADS in the CLI, not inside the library
The encoder and decoder previously called resolve_thread_limit() deep inside encode()/decode(), which read getenv("LAC_THREADS"). That made a library call depend on ambient process state and was not safe against concurrent setenv. Move environment resolution to the CLI: main.cpp now resolves --threads (which takes precedence) and otherwise LAC_THREADS via resolve_cli_thread_count(), and passes an explicit thread count to the library. The encoder/decoder use that count directly (0 = auto) and no longer include thread_limit.hpp. CLI behavior is unchanged. Direct library use no longer reads the environment; README is updated to state that LAC_THREADS applies to lac_cli (the unit tests set their own thread counts via set_thread_count). Refs #27 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 1c21b6f commit 4e9e424

5 files changed

Lines changed: 18 additions & 12 deletions

File tree

README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ LAC_THREADS=4 ctest --test-dir build-tests --output-on-failure
9797

9898
The default CTest configuration uses lightweight generated WAV fixtures and exercises both internal codec paths and `lac_cli` subprocess roundtrips. To opt into larger local E2E fixtures, configure with `-DLAC_TEST_ASSETS_DIR="$PWD/assets"`. The generated fixtures keep clean checkouts and routine development self-contained.
9999

100-
Set `LAC_THREADS=N` to cap encode and decode worker threads during tests. The heavier `test_all.sh` asset roundtrip script defaults to `LAC_THREADS=12` unless the environment already sets a different value.
100+
Set `LAC_THREADS=N` to cap encode and decode worker threads in the `lac_cli` binary; `--threads=N` takes precedence over it. This is resolved by the CLI, so it applies to `lac_cli` usage and the CLI subprocess tests (the internal codec unit tests set their own thread counts). The heavier `test_all.sh` asset roundtrip script defaults to `LAC_THREADS=12` unless the environment already sets a different value.
101101

102102
## Contributing
103103

src/codec/lac/decoder.cpp

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@
99
#include <thread>
1010
#include "codec/block/decoder.hpp"
1111
#include "codec/bitstream/bit_reader.hpp"
12-
#include "codec/lac/thread_limit.hpp"
1312
#include "codec/simd/neon.hpp"
1413

1514
namespace LAC {
@@ -236,7 +235,7 @@ void Decoder::decode(const uint8_t* data,
236235

237236
size_t hardware_threads =
238237
std::max<size_t>(1, static_cast<size_t>(std::thread::hardware_concurrency()));
239-
const size_t thread_limit = LAC::resolve_thread_limit(this->thread_count);
238+
const size_t thread_limit = this->thread_count; // 0 = auto; env is resolved by the CLI
240239
if (thread_limit > 0) {
241240
hardware_threads = std::min(hardware_threads, thread_limit);
242241
}

src/codec/lac/encoder.cpp

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@
1212
#include <utility>
1313
#include <iostream>
1414
#include "codec/simd/neon.hpp"
15-
#include "codec/lac/thread_limit.hpp"
1615
#include "utils/logger.hpp"
1716

1817
namespace {
@@ -384,7 +383,7 @@ namespace LAC {
384383
};
385384

386385
size_t hardware_threads = std::max<size_t>(1, static_cast<size_t>(std::thread::hardware_concurrency()));
387-
const size_t thread_limit = LAC::resolve_thread_limit(this->thread_count);
386+
const size_t thread_limit = this->thread_count; // 0 = auto; env is resolved by the CLI
388387
if (thread_limit > 0) {
389388
hardware_threads = std::min(hardware_threads, thread_limit);
390389
}

src/codec/lac/thread_limit.hpp

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -27,9 +27,8 @@ inline size_t parse_thread_limit(const char* value) {
2727
return static_cast<size_t>(parsed);
2828
}
2929

30-
inline size_t resolve_thread_limit(size_t explicit_limit) {
31-
if (explicit_limit > 0) return explicit_limit;
32-
return parse_thread_limit(std::getenv("LAC_THREADS"));
33-
}
30+
// Note: this header intentionally no longer resolves LAC_THREADS. Reading the
31+
// environment is the CLI's job; the library uses the explicit thread count it
32+
// is given (0 = auto). See main.cpp resolve_cli_thread_count.
3433

3534
} // namespace LAC

src/main.cpp

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
#include <atomic>
44
#include <cerrno>
55
#include <cstdint>
6+
#include <cstdlib>
67
#include <vector>
78
#include <string>
89
#include <cmath>
@@ -315,9 +316,8 @@ static FastDecodeStatus decode_lac_v3_to_mapped_wav(const uint8_t* data,
315316

316317
size_t hardware_threads =
317318
std::max<size_t>(1, static_cast<size_t>(std::thread::hardware_concurrency()));
318-
const size_t resolved_limit = LAC::resolve_thread_limit(thread_count);
319-
if (resolved_limit > 0) {
320-
hardware_threads = std::min(hardware_threads, resolved_limit);
319+
if (thread_count > 0) {
320+
hardware_threads = std::min(hardware_threads, thread_count);
321321
}
322322
const size_t worker_count = std::min<size_t>(hardware_threads, block_count);
323323

@@ -583,6 +583,13 @@ static bool parse_threads_flag(const std::string& flag, size_t& out_threads) {
583583
return true;
584584
}
585585

586+
static size_t resolve_cli_thread_count(size_t explicit_count) {
587+
// CLI owns environment resolution: --threads (explicit_count) wins, else
588+
// LAC_THREADS. The library never reads the environment itself.
589+
if (explicit_count > 0) return explicit_count;
590+
return LAC::parse_thread_limit(std::getenv("LAC_THREADS"));
591+
}
592+
586593
static void usage() {
587594
std::cerr << "Usage:\n";
588595
std::cerr << " lac_cli encode input.wav output.lac [--stereo-mode=lr|ms] [--threads=N] [--debug-threads] [--debug-lpc] [--debug-stereo-est] [--debug-zr] [--debug-partitions] [--no-partitioning]\n";
@@ -643,6 +650,7 @@ int main(int argc, char** argv) {
643650
return 1;
644651
}
645652
}
653+
thread_count = resolve_cli_thread_count(thread_count);
646654
std::vector<int32_t> left, right;
647655
uint16_t channels = 0;
648656
uint32_t sample_rate = 0;
@@ -725,6 +733,7 @@ int main(int argc, char** argv) {
725733
return 1;
726734
}
727735
}
736+
thread_count = resolve_cli_thread_count(thread_count);
728737
std::vector<uint8_t> bitstream;
729738
if (!load_file(in_path, bitstream)) {
730739
std::cerr << "Failed to read LAC file: " << in_path << "\n";

0 commit comments

Comments
 (0)