Apply degradation preference to the backup codec sender - #2040
Apply degradation preference to the backup codec sender#2040xianshijing-lk wants to merge 1 commit into
Conversation
Degradation preference is a sender-level property, and a backup codec publishes over its own sender. Only the primary sender was configured, so the backup encoder resolved a preference implicitly from the browser and could adapt along a different axis than the primary. Apply the resolved preference when a simulcast (backup codec) sender is registered, and keep every sender in sync when the preference changes after publish. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
🦋 Changeset detectedLatest commit: 91bf9a1 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| private applyDegradationPreference(sender?: RTCRtpSender) { | ||
| if (!sender) { | ||
| return; | ||
| } | ||
| try { | ||
| this.log.debug( | ||
| `setting degradationPreference to ${this.degradationPreference}`, | ||
| this.logContext, | ||
| ); | ||
| const params = sender.getParameters(); | ||
| params.degradationPreference = this.degradationPreference; | ||
| sender.setParameters(params); | ||
| } catch (e: any) { | ||
| this.log.warn(`failed to set degradationPreference`, { error: e, ...this.logContext }); | ||
| } | ||
| } |
There was a problem hiding this comment.
🟡 Failures while applying video degradation settings go unreported and can crash the page with an unhandled error
The video degradation setting is written to each sender without waiting for the write to finish (sender.setParameters(params) at src/room/track/LocalVideoTrack.ts:432), so a failed write escapes the surrounding error handling and surfaces as an unhandled promise rejection instead of a warning.
Impact: On browsers/situations where the write fails (e.g. a backup sender whose connection has already been torn down), the app sees an unhandled rejection and no diagnostic warning is logged.
Why the try/catch never sees setParameters failures, and why the change makes it more likely
RTCRtpSender.setParameters() returns a Promise; it rejects rather than throwing synchronously. The try/catch in applyDegradationPreference (src/room/track/LocalVideoTrack.ts:425-435) therefore only catches synchronous errors from getParameters(), and the rejection from setParameters is never handled.
This was pre-existing for the primary sender, but the PR now fans the same unawaited call out to every backup-codec sender in setDegradationPreference (src/room/track/LocalVideoTrack.ts:410-412) and in setSimulcastTrackSender (src/room/track/LocalVideoTrack.ts:465). Backup senders can be stale/closed — other code in this file explicitly skips them with sc.sender.transport?.state === 'closed' (src/room/track/LocalVideoTrack.ts:333) — so rejections become considerably more likely.
Making applyDegradationPreference async and awaiting setParameters inside the try (and awaiting it from setDegradationPreference) restores the intended warning path.
| private applyDegradationPreference(sender?: RTCRtpSender) { | |
| if (!sender) { | |
| return; | |
| } | |
| try { | |
| this.log.debug( | |
| `setting degradationPreference to ${this.degradationPreference}`, | |
| this.logContext, | |
| ); | |
| const params = sender.getParameters(); | |
| params.degradationPreference = this.degradationPreference; | |
| sender.setParameters(params); | |
| } catch (e: any) { | |
| this.log.warn(`failed to set degradationPreference`, { error: e, ...this.logContext }); | |
| } | |
| } | |
| private async applyDegradationPreference(sender?: RTCRtpSender) { | |
| if (!sender) { | |
| return; | |
| } | |
| try { | |
| this.log.debug( | |
| `setting degradationPreference to ${this.degradationPreference}`, | |
| this.logContext, | |
| ); | |
| const params = sender.getParameters(); | |
| params.degradationPreference = this.degradationPreference; | |
| await sender.setParameters(params); | |
| } catch (e: any) { | |
| this.log.warn(`failed to set degradationPreference`, { error: e, ...this.logContext }); | |
| } | |
| } |
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
This seems like a good suggestion to me, since sender.setParameters is async you probably should await it like the suggested change says, and then also stick an await at the this.applyDegradationPreference(sc.sender); call site above?
Also maybe consider if these can be set in parallel, and if so, then use Promise.all instead of for (const sc of this.simulcastCodecs.values()) {.
size-limit report 📦
|
| private applyDegradationPreference(sender?: RTCRtpSender) { | ||
| if (!sender) { | ||
| return; | ||
| } | ||
| try { | ||
| this.log.debug( | ||
| `setting degradationPreference to ${this.degradationPreference}`, | ||
| this.logContext, | ||
| ); | ||
| const params = sender.getParameters(); | ||
| params.degradationPreference = this.degradationPreference; | ||
| sender.setParameters(params); | ||
| } catch (e: any) { | ||
| this.log.warn(`failed to set degradationPreference`, { error: e, ...this.logContext }); | ||
| } | ||
| } |
There was a problem hiding this comment.
This seems like a good suggestion to me, since sender.setParameters is async you probably should await it like the suggested change says, and then also stick an await at the this.applyDegradationPreference(sc.sender); call site above?
Also maybe consider if these can be set in parallel, and if so, then use Promise.all instead of for (const sc of this.simulcastCodecs.values()) {.
…ackup codec (#1155) Aligns Flutter with the behavior landed in client-sdk-android (livekit/client-sdk-android#991). Two related changes. ## Source-based defaults Previously every video track fell back to `maintainResolution`, and the preference was only applied to camera and screen share tracks at all: ```dart if ([TrackSource.camera, TrackSource.screenShareVideo].contains(track.source)) { final degradationPreference = options.degradationPreference ?? DegradationPreference.maintainResolution; await track.setDegradationPreference(degradationPreference); } ``` Now `getDefaultDegradationPreference(source)` resolves camera → `maintainFramerate` (smoother video for real-time communication), screen share → `maintainResolution` (clarity matters for text/UI), other → `balanced`, and it is applied to every video sender. Custom sources previously got whatever WebRTC derived implicitly from the native source; `balanced` is the preference the WebRTC spec mandates as the default and is the honest choice when the application declined to declare a motion-vs-detail intent. An explicitly set `degradationPreference` still wins in all cases — the default only fills a null. ## Backup codec sender Degradation preference is a property of the **sender**, not of the track — a top-level field on `RtpParameters`, not per-encoding. `publishAdditionalCodecForPublication` adds a second transceiver and therefore a second sender, which was never configured, so the backup encoder resolved a preference implicitly and could adapt along a different axis than the primary. Both senders sink from the same video source, so a diverging backup does not just degrade itself — its restriction is merged onto the shared source and affects the primary too. `setDegradationPreference` now stores the resolved preference and fans out to every sender, and `publishAdditionalCodecForPublication` applies it to the backup sender once created. Using the track's stored resolved value means the two encoders cannot disagree. Note simulcast is unaffected — all simulcast encodings live under one sender and already share its preference. Only the backup codec is a separate sender. ## Tests `test/options/degradation_preference_test.dart` covers the three source mappings. Full suite passes (379 tests), `flutter analyze lib/ test/` clean, `dart format` clean at the repo's 120-column width. ## Cross-SDK client-sdk-js gets the backup-sender half in livekit/client-sdk-js#2040 (its source-based defaults already matched). The Rust SDK already resolves the same defaults and has no backup-codec publish path. Swift follows separately. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-authored-by: Hiroshi Horie <548776+hiroshihorie@users.noreply.github.com>
2 participants
Degradation preference is a property of the sender, not of the track — it is a top-level field on
RtpParameters, not per-encoding. A backup codec publishes over its own transceiver and therefore its own sender, so it needs the preference applied separately.setDegradationPreferenceonly ever configuredthis.sender(the primary). The backup sender, registered viasetSimulcastTrackSenderfromcreateSimulcastTransceiverSender, was never touched, so it fell back to the browser's implicit resolution and could adapt along a different axis than the primary encoder.Concretely, with a VP9/AV1 primary and a VP8 backup:
degradationPreferencereached the primary encoder onlygetDefaultDegradationPreference(camera → maintain-framerate, screen share → maintain-resolution, other → balanced) reached the primary encoder onlyChanges
applyDegradationPreference(sender).setSimulcastTrackSender, so a backup sender gets the preference the primary already resolved to.setDegradationPreferencefan out to every sender, so a change after the backup is published keeps the two in sync.Using the track's stored resolved preference (rather than re-deriving in
publishAdditionalCodecForTrack) means the two encoders can't disagree.Note simulcast itself is unaffected — all simulcast encodings live under one sender and already share its preference. Only the backup codec is a separate sender.
Tests
Three tests in
LocalVideoTrack.test.tscovering the primary sender, the backup sender receiving the resolved preference, and a later preference change reaching both. Verified they fail without the fix.Full suite matches the baseline on
main(same 3 pre-existing data-stream failures, +3 passing).eslint,prettier --checkandtsc --noEmitclean for the touched files.Cross-SDK
This is the JS half of aligning the SDKs on the behavior landed in client-sdk-android (livekit/client-sdk-android#991). The Rust SDK already resolves the same source-based defaults and has no backup-codec publish path. Flutter (livekit/client-sdk-flutter#1155) and Swift (livekit/client-sdk-swift#1083) follow separately.
🤖 Generated with Claude Code