Skip to content

3 a bug #65

@taozui666

Description

@taozui666

Issue 1: Missing output offset when writing transition samples during FEC decode

Affected file:

  • Opus/Structs/OpusDecoder.cs

Summary:
In the transition handling path inside opus_decode_frame(), the code writes the first 2.5 ms of transition samples with pcm[i] = pcm_transition[i]; instead of using the output offset (pcm_ptr). This is inconsistent with the surrounding code, which otherwise respects pcm_ptr for non-zero output positions.

Relevant code:

  • Opus/Structs/OpusDecoder.cs:561-564
  • Opus/Structs/OpusDecoder.cs:678-679

Why this is a bug:
Normally this is masked because the first decoded frame in a regular decode call starts at offset 0. However, in the FEC path, opus_decode_native() may first generate PLC for the earlier missing part of the output and then decode FEC for the remaining tail into a non-zero output offset. In that case, the transition branch still writes to the start of the buffer instead of the intended offset.

Trigger conditions:

  • decode_fec == true
  • frame_size > packet_frame_size, so PLC is emitted first and FEC is written later in the output buffer
  • a mode transition occurs (CELT-only <-> non-CELT-only), so transition != 0
  • the transition path enters the audiosize >= F5 branch

Impact:
This can overwrite the beginning of the output buffer with transition samples that should have been written into the later FEC region. The result is misaligned decoded PCM and audible corruption in that specific recovery scenario.

Suggested fix:
Change the write from pcm[i] = pcm_transition[i]; to pcm[pcm_ptr + i] = pcm_transition[i];.

Issue 2: Downmix scaling checks the wrong variable when mixing all channels for analysis

Affected file:

  • Opus/Downmix.cs

Summary:
Both downmix_float() and downmix_int() handle the special case c2 == -2, which means “mix all channels into one analysis signal”. However, the scaling step checks if (C == -2) instead of if (c2 == -2). Since C is the channel count, that condition is never true.

Relevant code:

  • Opus/Downmix.cs:75-92
  • Opus/Downmix.cs:106-121

Why this is a bug:
The code is clearly intended to average by the number of channels when all channels are mixed together, but it currently always falls back to division by 2.

Actual behavior:

  • mono analysis input (C = 1): incorrectly divided by 2 instead of 1
  • stereo analysis input (C = 2): accidentally correct
  • multichannel analysis input (C > 2): incorrectly divided by 2 instead of by C

Trigger conditions:

  • analysis is enabled and the encoder enters a path that calls downmix(..., c1, -2, C)
  • this happens in frame-size analysis and in tonality/bandwidth analysis
  • practical impact differs by channel count:
    • mono: definitely wrong
    • stereo: masked because /2 matches the expected scaling
    • multichannel: definitely wrong

Impact:
This does not directly corrupt PCM samples, but it does feed incorrect energy/amplitude into the analysis stage. That can distort activity, tonality, bandwidth, and variable frame-size decisions, which may in turn lead to suboptimal encoding decisions.

Suggested fix:
Change the scaling condition from if (C == -2) to if (c2 == -2) in both downmix_float() and downmix_int().

Issue 3: Float encoding path hardcodes lsb_depth to 16, making LSBDepth > 16 ineffective

Affected files:

  • Opus/Structs/OpusEncoder.cs
  • Opus/Structs/OpusMSEncoder.cs

Summary:
The public float encoding APIs always pass lsb_depth = 16 into opus_encode_native(), even though the encoder exposes an LSBDepth property that allows values up to 24.

Relevant code:

  • Opus/Structs/OpusEncoder.cs:1577-1578
  • Opus/Structs/OpusMSEncoder.cs:898-899
  • Opus/Structs/OpusEncoder.cs:1921-1935
  • Opus/Structs/OpusEncoder.cs:410

Why this is a bug:
The encoder accepts LSBDepth in the public API, but the float path effectively clamps it to 16 before the internal encoder logic sees it. As a result, setting LSBDepth to 17-24 has no effect when encoding from float input.

Trigger conditions:

  • encoding from float[] / Span
  • user sets LSBDepth to a value greater than 16

Impact:
This does not usually break ordinary 16-bit-equivalent float input, but it makes the public API behavior inconsistent with its own contract. For higher-precision sources (for example 24-bit PCM converted to float, or true high-precision float content), analysis and encoder decisions use a lower effective precision than requested.

Suggested fix:
Pass the configured encoder LSB depth through the float encoding path instead of hardcoding 16.

Additional context:

  • Issue 1 is a real decode correctness bug, but only in a narrow FEC + transition scenario.
  • Issue 2 is a real analysis-path bug; its practical impact depends on channel count and whether analysis is enabled.
  • Issue 3 is an API/behavior mismatch in the float encoding path; it is most relevant for higher-precision input sources.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions