Change default degradation preference by video source - #991
Conversation
🦋 Changeset detectedLatest commit: 85a76ee 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}
xianshijing-lk
left a comment
xianshijing-lk
left a comment
There was a problem hiding this comment.
@adrian-niculescu @davidliu , I broke down default degradation preference changes from #973 to keep the discussion more focused.
Please review this PR.
|
Diffuse output: AARJAR |
adrian-niculescu
left a comment
adrian-niculescu
left a comment
There was a problem hiding this comment.
The backup codec sender never gets the preference.
publishAdditionalCodecForTrack creates a second transceiver over the same rtcTrack and never touches sender.parameters, so that sender keeps resolving implicitly from the native source's is_screencast. Consequences:
- Camera and screen share stay media-equivalent across the two encoders as long as
options.sourceagrees withtrack.options.isScreencast. A screencast-backed track published withsource = CAMERAnow diverges: primary MAINTAIN_FRAMERATE, backup MAINTAIN_RESOLUTION. - An application-supplied
degradationPreferencereaches the primary encoder only. - The new BALANCED fallback reaches the primary encoder only.
The explicit-override half predates this PR and JS has the same gap, but this PR is what makes a resolved preference a value worth carrying, so applying it to the backup sender belongs here.
| return when (source) { | ||
| Track.Source.CAMERA -> RtpParameters.DegradationPreference.MAINTAIN_FRAMERATE | ||
| Track.Source.SCREEN_SHARE -> RtpParameters.DegradationPreference.MAINTAIN_RESOLUTION | ||
| else -> RtpParameters.DegradationPreference.BALANCED |
There was a problem hiding this comment.
A custom feed published with VideoTrackPublishOptions(source = Track.Source.UNKNOWN) lands here, and this overwrites what libwebrtc would have derived from the native source: a screencast-backed track goes MAINTAIN_RESOLUTION to BALANCED, a camera-like one MAINTAIN_FRAMERATE to BALANCED. track.options.isScreencast already carries that information. BALANCED is also the mode libwebrtc keeps behind the WebRTC-Video-BalancedDegradation field trial, with the in-tree note that it "needs to be tuned first".
Proposal:
private fun getDefaultDegradationPreference(source: Track.Source): RtpParameters.DegradationPreference? {
return when (source) {
Track.Source.CAMERA -> RtpParameters.DegradationPreference.MAINTAIN_FRAMERATE
Track.Source.SCREEN_SHARE -> RtpParameters.DegradationPreference.MAINTAIN_RESOLUTION
else -> null
}
}The KDoc bullet above changes with it. Not a blocker: JS made the same BALANCED choice deliberately, so if this is cross-SDK alignment it stands, but the argument applies there too.
There was a problem hiding this comment.
I think BALANCED is fine for the unknown source here.
And balanced mode is set when the source is neither camera nor screen share, which requires an app to explicitly declare something else, source defaults to null and resolves to Camera/ScreenShare from isScreencast. And it only takes effect when the app hasn't set degradationPreference itself, so anyone who wants a specific behavior (including libwebrtc's implicit derivation) can name it directly.
I ran this locally in both good and constrained network conditions and personally found BALANCED to work pretty well, even without further tuning on the WebRTC side. And for a feed the app has declined to label as motion or detail, a balanced tradeoff seems like the more honest default than committing hard to either axis.
Once this PR is landed, I am going to make follow-up on other SDKs to follow what this PR is doing.
Good catch. Confirmed: degradation preference is a sender level property (a top level field on RtpParameters, not per encoding), and publishAdditionalCodecForTrack adds a second transceiver over the same rtc track, so the backup codec has its own sender with its own parameters, which we were never setting. Worth noting both senders sink from the same VideoSource, and VideoBroadcaster::UpdateWants takes the MIN of max_pixel_count and max_framerate_fps across sinks, so a diverging backup drags its restriction onto the primary too. Applied the resolved preference to the backup sender, resolving the source the same way the primary publish does rather than reading it back off the publication, so the two can't disagree. Added tests asserting both senders match for the default camera, default screen share, explicit-override, and screencast-backed-published-as-camera cases. I will port the same fix to other sdks once landing this PR. |
|
Hi @adrian-niculescu , thanks for the comments, can you take another look ? |
adrian-niculescu
left a comment
adrian-niculescu
left a comment
There was a problem hiding this comment.
Read the changes and your comments. LGTM, thanks!
|
Thanks @adrian-niculescu and @davidliu |
…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>
3 participants
Summary
degradationPreferenceis not explicitly set.MAINTAIN_FRAMERATE.MAINTAIN_RESOLUTION.BALANCED.degradationPreferenceoverrides.Testing
git diff --checkAGENT_ERROR,PUBLISH_DATA_TRACK_RESPONSE,clientProtocol, etc.).