Conversation
Contributor
Author
|
@davidliu @hiroshihorie I'm trying to create a counter-example for that now (something that crashed WebRTC before). |
Unify the two E2E cycle tests into one Scenario axis (audio, video, both) so every case runs the same code path, and always unpublish via unpublishAll(): for one track it is the same _unpublish path as unpublish(publication:), and for .both it races two transceiver stops against one debounced renegotiation — the shape reported in webrtc-sdk/webrtc#194 that actually crashed in the field. Drop the subscriber round-trip. It depended on real playout and on a pre-existing m-line ordering bug when an audio+video pair is re-published, neither related to transceiver release; the suite went 58s -> 9s. Verified as a real detector: with the pre-fix video-only condition restored, exactly the four audio-involving cases fail with 10 leaked transceivers, and against a pre-webrtc-sdk#194 binary (137.7151.04) the deterministic test aborts for both media kinds with "Check failed: !channel_ / Missing call to ClearChannel?". Add MicrophoneTransceiverReleaseTests for the real audio device module, which TestAudioTrack bypasses. Gated on microphone permission so hosts without it skip cleanly instead of failing. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…alone Two defects in the single PC offer path, both surfaced by stopping audio transceivers (which makes rejected m-sections common where they were rare). `SDPMediaSection.isRTP` only checks the protocol field, so a rejected section (`m=audio 0 ... RTP/...`, which is how a stopped transceiver serializes) still matched, and `mungeInactiveToRecvOnlyForMedia` rewrote its `a=inactive` to `a=recvonly` — resurrecting an m-section the client had just killed. Add `SDPMediaSection.isRejected` (port 0, RFC 3264 8.2) and skip those sections. `set(localDescription:munging:)` caught every error and retried with one fewer munge, all the way down to the unmunged original. Any setLocalDescription failure — including ones munging did not cause — could therefore discard the direction rewrite that single PC needs to receive media, leaving a silently deaf connection. Split the parameter into required `munging:` and `droppable:`; once the droppable ones are gone the error propagates. The answer path's stereo and nack preferences are both droppable; single PC keeps the direction rewrite and drops only stereo. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The field repro looped unbounded; the test fixed it at 10 so CI stays fast.
Read LK_SOAK instead, defaulting to 10, so the same test reproduces the
long-running shape on demand:
TEST_RUNNER_LK_SOAK=200 xcodebuild test -scheme LiveKit \
-only-testing LiveKitCoreTests/TransceiverReleaseTests -destination 'platform=macOS'
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Drop the LK_SOAK knob and just do the long run every time: the field repro (webrtc-sdk#194) looped unbounded, and 600 cycles across the six cases cost ~22s, so there is no reason to make it opt-in. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The cycle test created one track per kind and republished those same objects, which is issue #1104's repro shape, not the mic+camera loop from webrtc-sdk#194 that appeared to defeat the ClearChannel fix. Those differ: `unpublishAll()` leaves no publication, so the next setMicrophone/setCamera takes `set(source:enabled:)`'s create-and-publish branch instead of unmuting, churning a new media source and track every iteration rather than only a transceiver. Create both tracks fresh per cycle. A new buffer track has no dimensions and `_publish` waits on them before starting the capturer, so feed it and await the completer first. Same treatment in the ADM test, whose feeder cancel also moves into a `defer` so a throw cannot leak a 30 fps task. Verified 800 publishes per run (400 audio, 400 video) across the six cases. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
Author
|
Stopping audio transceivers is safe because of webrtc-sdk/webrtc#194 ("Ensure ClearChannel is called"), first shipped in |
pblazej
marked this pull request as ready for review
September 14, 2026 08:50
pblazej
requested review from
hiroshihorie and
xianshijing-lk
as code owners
September 14, 2026 08:50
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Note
This report is out of date. Scroll down for Devin Review's latest report on this PR.
🔍 Devin Review: 2 flags
Not posted on this PR by your GitHub settings — view them in Devin Review. (Configure)
The SFU answers a rejected `AddTrack` with a `RequestResponse` that echoes the request rather than carrying a request id, and the handler only looked at the id-keyed data-blob completers. The publish therefore sat on its completer for the full 10s and failed with a bare "Timed out" while the server had already said why. Correlate on the echoed `cid` and fail that add-track completer. This is how the new cycle test presents on a current server: it caps a participant at 20 pending tracks and only clears one when the track's media arrives, so a loop that unpublishes before any RTP flows (and TestAudioTrack never produces any) leaks a slot per publish and is rejected with LIMIT_EXCEEDED. Budget the cycles per scenario to stay under that cap — `.both` publishes two tracks per cycle, so it gets half as many. Only reproducible against livekit-server >= 1.13.6; 1.13.5 has no such limit. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…aze/audio-transceiver
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Completer entries live until disconnect and a resolved AsyncCompleter serves its cached result to every later wait, so reusing a CID replayed the previous attempt's outcome: a rejected publish failed the retry instantly while the request it had just sent still reached the SFU, which could then accept a track the client had already abandoned. The same held for a cached success, which returned a stale TrackInfo for a track republished under the same CID. Rearm the entry as part of getting it, so each attempt waits for its own response. Reported by Devin on #1105. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Pull the fixes that fell out of testing this change back out so the PR carries one thing: stopping audio transceivers on unpublish, and the tests that prove it. Each removed fix is real and gets its own PR from blaze/sdp-munge-and-addtrack-fixes: - SDP: skip rejected (port 0) sections in the single PC direction rewrite, and never drop a required munge on setLocalDescription failure - Signal: fail a server-rejected AddTrack immediately with its reason instead of waiting out the publish timeout - Signal: rearm the add-track completer per attempt so a reused CID does not replay the previous outcome The test cycle budget from those investigations stays: it is what keeps the suite under the SFU's 20 pending-track cap on livekit-server >= 1.13.6. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Resolves #1104