Skip to content

Reused channel slot records the previous occupant's audio #3901

Closed

Description

@mcfnord

🤖 AI: A channel slot reused by a new client can have the previous occupant's audio attributed to it. On an -R server the stale frame lands in the new client's own recording, at full amplitude.

Root cause. Between disconnect and the new occupant negotiating transport properties a channel sits at CT_NONE, so decoder selection leaves CurOpusDecoder null. In that window nothing writes the channel's decode buffer: the OPUS decode is skipped for want of a decoder, and bIsRawAudio is false because iCeltNumCodedBytes has been reset to CELT_MINIMUM_NUM_BYTES. vecvecsData is indexed by position in the active-channel list rather than by channel ID, so it still holds the previous occupant's decoded audio — which is then passed to the recorder and read by the mix.

Reproduced 3 of 3 on 5146a072. Client A sends a 440 Hz tone and disconnects; after a gap client B takes the freed slot and sends only silence. B's own pre-identification recording held 128 samples of A's tone — one OPUS frame, DOUBLE_SYSTEM_FRAME_SIZE_SAMPLES — at 440 Hz energy fraction 0.999 or above. One frame is the usual length, not a fixed one: later runs on 7ebf8f87 also recorded 256, 768 and 896 samples, each a repeat of the same stale frame. A's own stub is silent in the same runs, so this is not a recorder artifact. Reconnect gaps of 100, 250 and 500 ms all reproduce it.

Scope: recordings. vecvecsData also feeds the mix, so the same stale samples are a candidate for the live path — but they do not survive there audibly. On a server run without -R, what a passive listener receives is statistically unchanged by the fix below: the residual artifact at slot handover measures about 59 dB under the source (peak sample 29 of 32767) both with it and without.

Fix — contribute silence in the no-decoder branch:

else
{
  CurOpusDecoder = nullptr;

  // CT_NONE: nothing else writes this buffer, and it is indexed by position in
  // the active-channel list, so it still holds the previous occupant's audio
  memset ( &vecvecsData[iChanCnt][0], 0, vecvecsData[iChanCnt].Size() * sizeof ( int16_t ) );
}

With that applied, B's stub reads 0.0 in 3 of 3 runs and A's own tone still records normally.

Correction (2026-10-05): this memset is only half the fix. The per-channel Opus decoders also keep the previous client's state, so with only the buffer zeroed, B's identified recording still opens with about 128 samples of A's audio (13 of 13 runs on 7ebf8f87). #3969 zeroes the buffer and resets the decoders when the channel is freed.

One subtlety: zero the whole worst-case buffer rather than iClientFrameSizeSamples worth. That variable is still zero in this branch, since it is only set in the CT_OPUS and CT_OPUS64 arms, so a memset scaled by it writes no bytes at all.


🤖 This message was written by AI and reviewed by @mcfnord.

Activity

  1. pljones commented on Aug 15, 2026

    @pljones
    Collaborator

    I think this is probably worth getting into 4.0.0 and maybe a backport, should we do another 3.12.x.

  2. ann0see commented on Sep 3, 2026

    @ann0see
    Member

    It's probably not too bad of an issue though.

  3. pljones commented on Sep 4, 2026

    @pljones
    Collaborator

    Yeah - it's not something I'd say "we need a new backport release" for.

  4. added this to the Release 4.0.0 milestone on Sep 4, 2026
  5. added
    bugSomething isn't working
    AIAI generated or potentially AI generated
    on Sep 4, 2026
  6. added theissue type on Sep 4, 2026
  7. mcfnord commented on Sep 18, 2026

    @mcfnord
    ContributorAuthor

    🤖 AI: Re-checked against current main (cc40a8a9), where src/server.cpp is byte-identical to the tree cited above — so the analysis and line links in this issue still hold.

    question answer
    Does it still happen? Yes. A client taking a freed slot still starts its recording with the previous occupant's audio.
    How much audio? One frame — 128 samples — in 18 of 18 runs with the slot reused after 50 ms to 1.5 s. Not always: later runs on 7ebf8f87 also recorded 256, 768 and 896 samples (3 of 12), each a repeat of that frame.
    Does the memset fix it? Only half. That first frame becomes silence, and the departing client's own recording is unchanged, also on a second machine running a different Qt against the real client. But the reused channel's Opus decoders still hold the previous client's state, so the new client's identified recording opens with about 128 samples of its audio (13 of 13 runs on 7ebf8f87). #3969 fixes both.
    What does it cost? Nothing during a call. Counting the no-decoder branch on unpatched main: it runs roughly once per client connection — 4 times in 175,000 decode calls with four clients connected — and never while nobody is joining or leaving.
    Does it change the audio? No. Recording a 1 kHz tone through a real client and server, patched and unpatched, gives the same peak sample and the same tone-versus-noise ratio to 0.1 dB.

    The cost figure is the one worth a second look: it was taken on unpatched main, so it describes how often that branch is reached in the existing code, not how often the patch does anything.

  8. pljones commented on Oct 5, 2026

    @pljones
    Collaborator

    Happy to have a patch for this.

  9. added a commit that references this issue on Oct 10, 2026
    090cc44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    AIAI generated or potentially AI generatedbugSomething isn't working

    Type

    Projects

    • Status
      Done

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions