Add bitrate priority control #969
Conversation
📝 WalkthroughWalkthroughA bitrate priority control API was added: new Priority enum and AudioEncoding type; VideoEncoding and audio publish options now support bitrate/network priorities; local publishing and RTCRtpEncoding construction use these encodings; RTCConfiguration gains enableDscp; exports and WebRTC dependency versions updated. Changes
Sequence Diagram(s)sequenceDiagram
participant App as App
participant Local as LocalParticipant
participant Enc as AudioEncoding
participant RTC as WebRTC (RTCRtpSender)
App->>Local: publishAudioTrack(publishOptions)
Local->>Enc: select publishOptions.encoding or presetMusic
Local->>Local: enc.toRTCRtpEncoding() -> sendEncodings
Local->>RTC: addTransceiver / setSenderEncodings (maxBitrate, priority, networkPriority)
RTC-->>Local: transceiver negotiated / sender returned
Local-->>App: publish completed
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
✏️ Tip: You can disable this entire section by setting Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
lib/src/types/other.dart (1)
144-187: Remove or gateenableDscpto Android only—it will silently fail on all other platforms.Lines 144–187:
enableDscpis not a standard WebRTC RTCConfiguration option and will be ignored on Flutter Web (browser) and typically on iOS and desktop. It is supported only on Android (via native WebRTC'sPeerConnection.RTCConfiguration.enableDscp). The code currently passes it unconditionally to all platforms, causing silent failures. Either:
- Gate it to Android only before adding to the configuration map, or
- Document explicitly that it is Android-only and will be ignored elsewhere.
Standard WebRTC DSCP/QoS control is per-RTP encoding via
RTCRtpSender.setParameters()withnetworkPriority, not a globalRTCConfigurationproperty.lib/src/types/video_encoding.dart (1)
77-86: Comparable contract drift: compareTo ignores priority fields.The
compareTo()method only compares bitrate and framerate, but the==operator andhashCodeinclude all four fields (maxBitrate, maxFramerate, bitratePriority, networkPriority). Two instances with identical bitrate/framerate but different priorities will return 0 fromcompareTo()while==returns false, violating the Comparable contract. This can cause unexpected behavior in TreeSet/TreeMap where objects with compareTo() == 0 won't insert as distinct elements, and breaks the consistency guarantee between equality and ordering.Consider adding priority tie-breakers to
compareTo()to maintain consistency:Suggested fix
`@override` int compareTo(VideoEncoding other) { // compare bitrates - final result = maxBitrate.compareTo(other.maxBitrate); - // if bitrates are the same, compare by fps - if (result == 0) { - return maxFramerate.compareTo(other.maxFramerate); - } - - return result; + var result = maxBitrate.compareTo(other.maxBitrate); + if (result != 0) return result; + + result = maxFramerate.compareTo(other.maxFramerate); + if (result != 0) return result; + + result = (bitratePriority?.index ?? -1).compareTo(other.bitratePriority?.index ?? -1); + if (result != 0) return result; + + return (networkPriority?.index ?? -1).compareTo(other.networkPriority?.index ?? -1); }
🧹 Nitpick comments (1)
lib/src/participant/local.dart (1)
128-134: Consider centralizing audio RTCRtpEncoding construction to avoid drift.The two RTCRtpEncoding blocks are nearly identical; a small helper (or AudioEncodingExt) will reduce duplication and keep the maxBitrate guard consistent across both code paths.
♻️ Suggested refactor
- final List<rtc.RTCRtpEncoding> encodings = [ - rtc.RTCRtpEncoding( - maxBitrate: audioEncoding.maxBitrate, - priority: (audioEncoding.bitratePriority ?? Priority.low).toRtcpPriorityType(), - networkPriority: audioEncoding.networkPriority?.toRtcpPriorityType(), - ) - ]; + final encodings = [ + if (audioEncoding.maxBitrate > 0) audioEncoding.toRTCRtpEncoding(), + ]; ... - sendEncodings: [ - if (audioEncoding.maxBitrate > 0) - rtc.RTCRtpEncoding( - maxBitrate: audioEncoding.maxBitrate, - priority: (audioEncoding.bitratePriority ?? Priority.low).toRtcpPriorityType(), - networkPriority: audioEncoding.networkPriority?.toRtcpPriorityType(), - ), - ], + sendEncodings: [ + if (audioEncoding.maxBitrate > 0) audioEncoding.toRTCRtpEncoding(), + ],Also applies to: 171-176
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pubspec.lockis excluded by!**/*.lock
📒 Files selected for processing (11)
.changes/priority-controllib/livekit_client.dartlib/src/options.dartlib/src/participant/local.dartlib/src/track/web/_audio_analyser.dartlib/src/types/audio_encoding.dartlib/src/types/other.dartlib/src/types/priority.dartlib/src/types/video_encoding.dartlib/src/utils.dartpubspec.yaml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: Build for Flutter Web
- GitHub Check: Build for Flutter Web WASM
- GitHub Check: Dart Analyze Check
- GitHub Check: Build for Flutter iOS
- GitHub Check: Build for Flutter macOS
- GitHub Check: Build for Flutter Android
- GitHub Check: Build for Flutter Windows
- GitHub Check: Build for Flutter Linux
🔇 Additional comments (9)
lib/src/track/web/_audio_analyser.dart (1)
5-5: Import consolidation looks good.Line 5: clean and consistent with current usage.
.changes/priority-control (1)
1-1: Changelog entry looks good.Clear and concise.
lib/livekit_client.dart (1)
62-66: Exports look good.Line 62–Line 66: exposing the new types is appropriate for the public API.
lib/src/utils.dart (1)
432-434: No changes needed. Both the legacy SVC path (lines 432–434) and the non-legacytoRTCRtpEncoding()method already use identical logic for priority:(bitratePriority ?? Priority.low).toRtcpPriorityType(). The priority handling is already consistent between the two paths. The suggested diff would introduce inconsistency rather than alignment.Likely an incorrect or invalid review comment.
pubspec.yaml (1)
49-50: flutter_webrtc 1.3.0 and dart_webrtc 1.7.0 support priority/networkPriority APIs; enableDscp is not exposed by dart_webrtc.Both libraries support the priority/networkPriority APIs you're using (flutter_webrtc 1.3.0 added
priorityandnetworkPrioritytoRTCRtpEncoding; dart_webrtc 1.7.0 added Priority control APIs). No breaking changes affect existing platforms.However,
enableDscpis a native WebRTC configuration field (part ofRTCConfigurationon Android/iOS), not a surface exposed by dart_webrtc's API wrapper. If you're relying on DSCP tagging, ensure your native WebRTC implementation (via platform channels or native code) handles it directly.Likely an incorrect or invalid review comment.
lib/src/types/priority.dart (1)
17-39: LGTM: clear Priority enum and mapping.Nice, minimal enum plus conversion helper; the switch is exhaustive and readable.
lib/src/options.dart (1)
309-353: LGTM: AudioPublishOptions now exposes AudioEncoding.Constructor, copyWith, and toString are updated consistently.
lib/src/types/audio_encoding.dart (1)
20-88: LGTM: AudioEncoding presets + RTC conversion are cohesive.The type is immutable, presets are sensible, and the conversion helper is clean.
lib/src/types/video_encoding.dart (1)
18-73: LGTM: priority fields are wired through copyWith/equality and RTC conversion.Nice, consistent propagation of the new priority fields.
Also applies to: 92-105
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.