Repository navigation
Refresh subscribed codecs after publisher answer #1037
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| --- | ||
| "client-sdk-android": patch | ||
| --- | ||
|
|
||
| Keep dynacast-paused simulcast layers paused across publisher renegotiations. Previously a paused layer started to upload again after any renegotiation of the publisher connection. It stayed on until the next subscribed-quality change. Publishing a track, unpublishing one, a codec change, and an ICE restart all caused this. | ||
|
|
||
| The server's answer to a publisher offer no longer carries the RFC 8853 pause markers. Applying that answer re-enables every layer the client paused. If the subscribed set does not change, the server sends no subscribed-codec update, so the pause was never restated. Directly after it applies a publisher answer, the SDK now re-applies the last known subscribed codecs. | ||
|
|
||
| One frame per layer can still escape, because the encoders restart during the apply. This matches the JavaScript and Rust SDKs. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -39,6 +39,7 @@ import io.livekit.android.util.FlowObservable | |
| import io.livekit.android.util.LKLog | ||
| import io.livekit.android.util.flowDelegate | ||
| import io.livekit.android.webrtc.peerconnection.RTCThreadToken | ||
| import io.livekit.android.webrtc.peerconnection.executeBlockingOnRTCThread | ||
| import livekit.LivekitRtc | ||
| import livekit.LivekitRtc.SubscribedCodec | ||
| import livekit.org.webrtc.CameraVideoCapturer | ||
|
|
@@ -334,54 +335,58 @@ constructor( | |
| LKLog.i { "attempted to set publishing layer for disposed video track." } | ||
| return | ||
| } | ||
| try { | ||
| val parameters = sender.parameters ?: return | ||
| val encodings = parameters.encodings ?: return | ||
| var hasChanged = false | ||
|
|
||
| if (encodings.firstOrNull()?.scalabilityMode != null) { | ||
| val encoding = encodings.first() | ||
| var maxQuality = ProtoVideoQuality.OFF | ||
| for (quality in qualities) { | ||
| if (quality.enabled && (maxQuality == ProtoVideoQuality.OFF || quality.quality.number > maxQuality.number)) { | ||
| maxQuality = quality.quality | ||
| // Both the dynacast path and the publisher-answer refresh reach this from separate | ||
| // coroutine scopes on the same multi-threaded dispatcher, so the read-modify-write of | ||
| // sender.parameters is serialized here rather than at either call site. | ||
| executeBlockingOnRTCThread(rtcThreadToken) { | ||
| try { | ||
| val parameters = sender.parameters ?: return@executeBlockingOnRTCThread | ||
| val encodings = parameters.encodings ?: return@executeBlockingOnRTCThread | ||
| var hasChanged = false | ||
|
|
||
| if (encodings.firstOrNull()?.scalabilityMode != null) { | ||
| val encoding = encodings.first() | ||
| var maxQuality = ProtoVideoQuality.OFF | ||
| for (quality in qualities) { | ||
| if (quality.enabled && (maxQuality == ProtoVideoQuality.OFF || quality.quality.number > maxQuality.number)) { | ||
| maxQuality = quality.quality | ||
| } | ||
| } | ||
| } | ||
|
|
||
| if (maxQuality == ProtoVideoQuality.OFF) { | ||
| if (encoding.active) { | ||
| LKLog.v { "setting svc track to disabled" } | ||
| encoding.active = false | ||
| if (maxQuality == ProtoVideoQuality.OFF) { | ||
| if (encoding.active) { | ||
| LKLog.v { "setting svc track to disabled" } | ||
| encoding.active = false | ||
| hasChanged = true | ||
| } | ||
| } else if (!encoding.active) { | ||
| LKLog.v { "setting svc track to enabled" } | ||
| encoding.active = true | ||
| hasChanged = true | ||
| } | ||
| } else if (!encoding.active) { | ||
| LKLog.v { "setting svc track to enabled" } | ||
| encoding.active = true | ||
| hasChanged = true | ||
| } | ||
| } else { | ||
| // simulcast dynacast encodings | ||
| for (quality in qualities) { | ||
| val rid = EncodingUtils.ridForVideoQuality(quality.quality) ?: continue | ||
| val encoding = encodings.firstOrNull { it.rid == rid } | ||
| // use low quality layer settings for non-simulcasted streams | ||
| ?: encodings.takeIf { it.size == 1 && quality.quality == ProtoVideoQuality.LOW }?.first() | ||
| ?: continue | ||
| if (encoding.active != quality.enabled) { | ||
| hasChanged = true | ||
| encoding.active = quality.enabled | ||
| LKLog.v { "setting layer ${quality.quality} to ${quality.enabled}" } | ||
| } else { | ||
| // simulcast dynacast encodings | ||
| for (quality in qualities) { | ||
| val rid = EncodingUtils.ridForVideoQuality(quality.quality) ?: continue | ||
| val encoding = encodings.firstOrNull { it.rid == rid } | ||
| // use low quality layer settings for non-simulcasted streams | ||
| ?: encodings.takeIf { it.size == 1 && quality.quality == ProtoVideoQuality.LOW }?.first() | ||
| ?: continue | ||
| if (encoding.active != quality.enabled) { | ||
| hasChanged = true | ||
| encoding.active = quality.enabled | ||
| LKLog.v { "setting layer ${quality.quality} to ${quality.enabled}" } | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| if (hasChanged) { | ||
| // This refreshes the native code with the new information | ||
| sender.parameters = parameters | ||
| if (hasChanged) { | ||
| // This refreshes the native code with the new information | ||
| sender.parameters = parameters | ||
| } | ||
| } catch (e: Exception) { | ||
| LKLog.w(e) { "Exception caught while setting publishing layers." } | ||
| } | ||
| } catch (e: Exception) { | ||
| LKLog.w(e) { "Exception caught while setting publishing layers." } | ||
| return | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -430,6 +435,18 @@ constructor( | |
| return newCodecs | ||
| } | ||
|
|
||
| /** | ||
| * Re-applies the most recently received subscribed codecs. | ||
| * | ||
| * Applying a publisher answer re-enables simulcast layers that dynacast had paused, and the | ||
| * server only sends a subscribed codec update when the set changes, so the pause is restated | ||
| * here instead. | ||
| */ | ||
| internal fun refreshSubscribedCodecs() { | ||
| val codecs = subscribedCodecs ?: return | ||
| setPublishingCodecs(codecs) | ||
| } | ||
|
Comment on lines
+445
to
+448
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔴 Quality-only updates lose paused layers After a quality-only update, Learn moreA subscribed-quality update can contain Example: A VP8 track receives a codec update enabling HIGH and LOW. A later quality-only update disables HIGH. The publisher renegotiates; the refresh replays the older codec list and enables HIGH again, although the last quality-only update disabled it. Recommended fix: Store the last effective quality-only update alongside codec updates and replay the latest state for each sender after an answer. Preserve the ordering between the two update formats so older cached codec qualities cannot override newer quality-only values. Was this helpful? React with 👍 or 👎 to provide feedback.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is deliberately scoped to the |
||
|
|
||
| internal fun addSimulcastTrack(codec: VideoCodec, encodings: List<RtpParameters.Encoding>): SimulcastTrackInfo? { | ||
| if (this.simulcastCodecs.containsKey(codec)) { | ||
| LKLog.w { "$codec already added, skipping." } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🟡 Sender refresh bypasses the RTC thread
On a publisher answer,
onRequestSubscribedCodecRefreshaccesses WebRTC sender parameters from the engine's I/O coroutine. This can race with RTC-thread negotiation and leave paused layers active.Learn more
The publisher answer is applied by setRemoteDescription on the RTC thread, but the suspended caller resumes on the engine's I/O dispatcher. The new synchronous callback reaches setPublishingLayersForSender, which reads and writes
RtpSender.parameters. Sender operations belong on the RTC thread according to the repository's WebRTC threading rule; otherwise they can overlap further negotiation work.Example: An answer for an ICE restart arrives while another publisher negotiation starts. The refresh accesses the sender on the I/O worker while the other negotiation uses that sender on the RTC executor.
Recommended fix: Schedule the refresh's WebRTC sender operations through
launchBlockingOnRTCThreadorexecuteBlockingOnRTCThreadwith the room's RTC token. Keep the cached codec state synchronized with signaling updates if the callback is dispatched asynchronously.Was this helpful? React with 👍 or 👎 to provide feedback.