You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
[High] Incorrect self_delimited
calculation in multistream unpadding ( UnpadMultistreamPacket
) generates invalid packets Location: Opus/Structs/OpusRepacketizer.cs:540 Current behavior: int self_delimited = ((s != nb_streams) ? 1 : 0) - 1;
Because the loop condition is s < nb_streams, s != nb_streams is always true , so this expression always evaluates to 0. Impact:
When removing padding from multistream packets, non-final substreams are not parsed according to the self-delimited rule, which can easily produce malformed output packets.
Downstream effects may include decode failures, audible pops/clicks, dropped frames, or obvious audio quality degradation. Evidence: The same file already contains the comment FIXME THIS METHOD FAILS IN TEST_OPUS_ENCODE
( Opus/Structs/OpusRepacketizer.cs:507). Recommendation:
Change to int self_delimited = (s != nb_streams - 1) ? 1 : 0;
Add multistream round-trip tests (Pad → Unpad → Decode verification).
[High] Native decoder SafeHandle
destructor calls the wrong destroy API Location: Native/NativeOpusDecoder.cs:18 Current behavior: ReleaseHandle() calls NativeOpus.opus_encoder_destroy(handle) instead of opus_decoder_destroy. Impact:
This is an incorrect resource release path and may lead to native resource leaks or undefined behavior.
In long-running scenarios, decoder stability may degrade, indirectly triggering audio issues. Recommendation:
Fix to NativeOpus.opus_decoder_destroy(handle).
Add regression tests for the native decoder lifecycle (repeated create/destroy + stress decoding).
[Medium] Known “incorrect implementation” in the resampler dynamic update path may introduce transition distortion Location: Common/SpeexResampler.cs:535–541 Current behavior: The code comment explicitly states /* FIXME: This is wrong ... */. Trigger conditions:
Calling SetRateFraction(...) or changing Quality during streaming, which triggers update_filter() (see Common/SpeexResampler.cs:979–980).
This branch is taken when the new filter length is greater than the old one ( Common/SpeexResampler.cs:516+). Impact:
Historical sample handling is incorrect and may cause transient clicks, phase jumps, or short-term distortion. Recommendation:
Prefer aligning with the fixes from upstream speexdsp.
If a proper fix is not feasible short-term, at least call ResetMem() when dynamically changing sample rate/quality and document the brief transition artifact.
[Medium] Managed float
encode/decode paths are actually quantized to int16
, causing precision loss Locations:
Decode: Opus/Structs/OpusDecoder.cs:863–882
Multistream decode output: Opus/Structs/OpusMSDecoder.cs:304–309
Multistream encode input: Opus/Structs/OpusMSEncoder.cs:824–827
Single-stream encode input: Opus/Structs/OpusEncoder.cs:1570–1574 Current behavior: The float API does not maintain a fully float pipeline; it goes through a short
intermediate ( FLOAT2INT16 / short → float). Impact:
Dynamic range is limited to 16-bit, introducing additional quantization noise.
For high-dynamic-range material or post-processing chains, perceived quality is rougher than a true float pipeline. Recommendation:
Clearly document this behavior to avoid users assuming a high-precision float path.
For high-fidelity use cases, prefer the native libopus float API (via OpusCodecFactory when native support is available).
[Medium] Encoder input length validation does not account for channel count (may allow invalid input) Locations: Opus/Structs/OpusEncoder.cs:1476 Opus/Structs/OpusEncoder.cs:1563 Current behavior: Only checks internal_frame_size > in_pcm.Length without multiplying by channels. Impact:
For stereo or multichannel input, length validation may incorrectly pass; later stages are more likely to throw exceptions or exhibit undefined behavior. Recommendation:
Change the check to internal_frame_size * this.channels > in_pcm.Length.
[Low] OpusMSDecoder
remaining-length checks use the wrong variable Locations: Opus/Structs/OpusMSDecoder.cs:162 Opus/Structs/OpusMSDecoder.cs:234 Current behavior: The loop checks data.Length instead of the decremented len. Impact:
Early detection of truncated or malformed packets is weakened; errors surface deeper in the pipeline.Recommendation: Change the condition to check len <= 0 .
calculation in multistream unpadding ( UnpadMultistreamPacket
) generates invalid packets Location: Opus/Structs/OpusRepacketizer.cs:540 Current behavior: int self_delimited = ((s != nb_streams) ? 1 : 0) - 1;
Because the loop condition is s < nb_streams, s != nb_streams is always true , so this expression always evaluates to 0. Impact:
When removing padding from multistream packets, non-final substreams are not parsed according to the self-delimited rule, which can easily produce malformed output packets.
Downstream effects may include decode failures, audible pops/clicks, dropped frames, or obvious audio quality degradation. Evidence: The same file already contains the comment FIXME THIS METHOD FAILS IN TEST_OPUS_ENCODE
( Opus/Structs/OpusRepacketizer.cs:507). Recommendation:
Change to int self_delimited = (s != nb_streams - 1) ? 1 : 0;
Add multistream round-trip tests (Pad → Unpad → Decode verification).
destructor calls the wrong destroy API Location: Native/NativeOpusDecoder.cs:18 Current behavior: ReleaseHandle() calls NativeOpus.opus_encoder_destroy(handle) instead of opus_decoder_destroy. Impact:
This is an incorrect resource release path and may lead to native resource leaks or undefined behavior.
In long-running scenarios, decoder stability may degrade, indirectly triggering audio issues. Recommendation:
Fix to NativeOpus.opus_decoder_destroy(handle).
Add regression tests for the native decoder lifecycle (repeated create/destroy + stress decoding).
Calling SetRateFraction(...) or changing Quality during streaming, which triggers update_filter() (see Common/SpeexResampler.cs:979–980).
This branch is taken when the new filter length is greater than the old one ( Common/SpeexResampler.cs:516+). Impact:
Historical sample handling is incorrect and may cause transient clicks, phase jumps, or short-term distortion. Recommendation:
Prefer aligning with the fixes from upstream speexdsp.
If a proper fix is not feasible short-term, at least call ResetMem() when dynamically changing sample rate/quality and document the brief transition artifact.
encode/decode paths are actually quantized to int16
, causing precision loss Locations:
Decode: Opus/Structs/OpusDecoder.cs:863–882
Multistream decode output: Opus/Structs/OpusMSDecoder.cs:304–309
Multistream encode input: Opus/Structs/OpusMSEncoder.cs:824–827
Single-stream encode input: Opus/Structs/OpusEncoder.cs:1570–1574 Current behavior: The float API does not maintain a fully float pipeline; it goes through a short
intermediate ( FLOAT2INT16 / short → float). Impact:
Dynamic range is limited to 16-bit, introducing additional quantization noise.
For high-dynamic-range material or post-processing chains, perceived quality is rougher than a true float pipeline. Recommendation:
Clearly document this behavior to avoid users assuming a high-precision float path.
For high-fidelity use cases, prefer the native libopus float API (via OpusCodecFactory when native support is available).
For stereo or multichannel input, length validation may incorrectly pass; later stages are more likely to throw exceptions or exhibit undefined behavior. Recommendation:
Change the check to internal_frame_size * this.channels > in_pcm.Length.
remaining-length checks use the wrong variable Locations: Opus/Structs/OpusMSDecoder.cs:162 Opus/Structs/OpusMSDecoder.cs:234 Current behavior: The loop checks data.Length instead of the decremented len. Impact:
Early detection of truncated or malformed packets is weakened; errors surface deeper in the pipeline.Recommendation: Change the condition to check len <= 0 .