From 4e9e424dcff234c51effdbd2d5e960fbce156159 Mon Sep 17 00:00:00 2001 From: audexdev Date: Fri, 5 Jun 2026 02:55:37 +0900 Subject: [PATCH] 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 --- README.md | 2 +- src/codec/lac/decoder.cpp | 3 +-- src/codec/lac/encoder.cpp | 3 +-- src/codec/lac/thread_limit.hpp | 7 +++---- src/main.cpp | 15 ++++++++++++--- 5 files changed, 18 insertions(+), 12 deletions(-) diff --git a/README.md b/README.md index f282092..6310c03 100644 --- a/README.md +++ b/README.md @@ -97,7 +97,7 @@ LAC_THREADS=4 ctest --test-dir build-tests --output-on-failure 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. -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. +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. ## Contributing diff --git a/src/codec/lac/decoder.cpp b/src/codec/lac/decoder.cpp index 7f45726..6b60375 100644 --- a/src/codec/lac/decoder.cpp +++ b/src/codec/lac/decoder.cpp @@ -9,7 +9,6 @@ #include #include "codec/block/decoder.hpp" #include "codec/bitstream/bit_reader.hpp" -#include "codec/lac/thread_limit.hpp" #include "codec/simd/neon.hpp" namespace LAC { @@ -236,7 +235,7 @@ void Decoder::decode(const uint8_t* data, size_t hardware_threads = std::max(1, static_cast(std::thread::hardware_concurrency())); - const size_t thread_limit = LAC::resolve_thread_limit(this->thread_count); + const size_t thread_limit = this->thread_count; // 0 = auto; env is resolved by the CLI if (thread_limit > 0) { hardware_threads = std::min(hardware_threads, thread_limit); } diff --git a/src/codec/lac/encoder.cpp b/src/codec/lac/encoder.cpp index b8f7bcb..0df3bc1 100644 --- a/src/codec/lac/encoder.cpp +++ b/src/codec/lac/encoder.cpp @@ -12,7 +12,6 @@ #include #include #include "codec/simd/neon.hpp" -#include "codec/lac/thread_limit.hpp" #include "utils/logger.hpp" namespace { @@ -384,7 +383,7 @@ namespace LAC { }; size_t hardware_threads = std::max(1, static_cast(std::thread::hardware_concurrency())); - const size_t thread_limit = LAC::resolve_thread_limit(this->thread_count); + const size_t thread_limit = this->thread_count; // 0 = auto; env is resolved by the CLI if (thread_limit > 0) { hardware_threads = std::min(hardware_threads, thread_limit); } diff --git a/src/codec/lac/thread_limit.hpp b/src/codec/lac/thread_limit.hpp index 59fb9d7..4d9140e 100644 --- a/src/codec/lac/thread_limit.hpp +++ b/src/codec/lac/thread_limit.hpp @@ -27,9 +27,8 @@ inline size_t parse_thread_limit(const char* value) { return static_cast(parsed); } -inline size_t resolve_thread_limit(size_t explicit_limit) { - if (explicit_limit > 0) return explicit_limit; - return parse_thread_limit(std::getenv("LAC_THREADS")); -} +// Note: this header intentionally no longer resolves LAC_THREADS. Reading the +// environment is the CLI's job; the library uses the explicit thread count it +// is given (0 = auto). See main.cpp resolve_cli_thread_count. } // namespace LAC diff --git a/src/main.cpp b/src/main.cpp index c35c0b2..34767ff 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -3,6 +3,7 @@ #include #include #include +#include #include #include #include @@ -315,9 +316,8 @@ static FastDecodeStatus decode_lac_v3_to_mapped_wav(const uint8_t* data, size_t hardware_threads = std::max(1, static_cast(std::thread::hardware_concurrency())); - const size_t resolved_limit = LAC::resolve_thread_limit(thread_count); - if (resolved_limit > 0) { - hardware_threads = std::min(hardware_threads, resolved_limit); + if (thread_count > 0) { + hardware_threads = std::min(hardware_threads, thread_count); } const size_t worker_count = std::min(hardware_threads, block_count); @@ -583,6 +583,13 @@ static bool parse_threads_flag(const std::string& flag, size_t& out_threads) { return true; } +static size_t resolve_cli_thread_count(size_t explicit_count) { + // CLI owns environment resolution: --threads (explicit_count) wins, else + // LAC_THREADS. The library never reads the environment itself. + if (explicit_count > 0) return explicit_count; + return LAC::parse_thread_limit(std::getenv("LAC_THREADS")); +} + static void usage() { std::cerr << "Usage:\n"; 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) { return 1; } } + thread_count = resolve_cli_thread_count(thread_count); std::vector left, right; uint16_t channels = 0; uint32_t sample_rate = 0; @@ -725,6 +733,7 @@ int main(int argc, char** argv) { return 1; } } + thread_count = resolve_cli_thread_count(thread_count); std::vector bitstream; if (!load_file(in_path, bitstream)) { std::cerr << "Failed to read LAC file: " << in_path << "\n";