ai

[taozui666]

1. [High] Incorrect self_delimited calculation in multistream unpadding (UnpadMultistreamPacket) generates invalid packets
2. [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).
3. [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).
4. [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.
5. [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).
6. [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.
7. [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 .

[lostromb]

1, 2, 6, 7: sure, likely legitimate, you are welcome to submit fixes based on the correct code in the Opus reference library
3: Yeah I could look into this
4: The fixme comment was in the original implementation, maybe it's been fixed in the latest speex resampler, I don't know
5: Intended behavior, not a bug