Repository navigation
Refresh subscribed codecs after publisher answer - #1037
Conversation
Android publishers keep dynacast-paused simulcast layers paused across publisher renegotiations (publish/unpublish, codec change, ICE restart), instead of silently uploading them until the next subscribed-quality change.
🦋 Changeset detectedLatest commit: fd7ba91 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 |
lukasIO
changed the title
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| internal fun refreshSubscribedCodecs() { | ||
| val codecs = subscribedCodecs ?: return | ||
| setPublishingCodecs(codecs) | ||
| } |
There was a problem hiding this comment.
🔴 Quality-only updates lose paused layers
After a quality-only update, refreshSubscribedCodecs replays old codec qualities or does nothing. handleSubscribedQualityUpdate applies those qualities without caching them, so renegotiation restores the wrong layers.
Learn more
A subscribed-quality update can contain subscribedQualitiesList instead of subscribedCodecsList. handleSubscribedQualityUpdate applies that list directly to the sender and does not update the track's cached subscribedCodecs. After an answer, replaying the cache either does nothing or overwrites that newer quality state. A server that sends no further changes leaves the wrong layers active indefinitely.
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.
There was a problem hiding this comment.
This is deliberately scoped to the subscribedCodecs path, matching the merged JS fix in client-sdk-js#2121, which has the identical gap — keeping the two SDKs behaviourally the same was the point of this port. The analysis is right that a quality-only update stores nothing and so cannot be replayed: subscribedCodecs is only ever written inside setPublishingCodecs. Against a server that sends per-codec updates the path is unreachable, so closing it is follow-up work rather than something this PR should widen into.
| when (val outcome = publisher?.setRemoteDescription(sessionDescription, offerId).nullSafe()) { | ||
| is Either.Left -> { | ||
| // do nothing. | ||
| listener?.onRequestSubscribedCodecRefresh() |
There was a problem hiding this comment.
🟡 Sender refresh bypasses the RTC thread
On a publisher answer, onRequestSubscribedCodecRefresh accesses 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 launchBlockingOnRTCThread or executeBlockingOnRTCThread with 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.
|
Diffuse output: Base AAR cache miss. Please run the build job on main to generate the base AAR. |
setPublishingLayersForSender reads and writes RtpSender.parameters, and is now reached from two separate coroutine scopes over the same multi-threaded dispatcher, so the read-modify-write can interleave. Wrapping the shared call rather than either call site keeps the dynacast path and the publisher-answer refresh on one thread.
livekit/livekit#4907 (server v1.13.8 and later) restates the subscribed qualities a few seconds after every publisher answer, so the old claim that the server only sends an update when the set changes no longer holds for newer servers. The client-side refresh still closes that window and covers older servers. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Servers from v1.13.8 restate the dynacast pause a few seconds after a publisher answer (livekit/livekit#4907), so the layers only stayed on indefinitely with older servers. Scope the release note accordingly. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
The PR fixes client side what was introduced in v.1.9.8 through https://github.com/livekit/livekit/pull/4129/changes and fixed through a server-side resend in v1.13.7 through livekit/livekit#4907 (comment). Any server version number in between has the issue and newer servers still have a window of roughly 2s until the resend is applied. So "instead of silently uploading them until the next subscribed-quality change" is only correct for servers with v.1.9.8 < server version < v1.13.7 |
|
I tested both main and the branch with the Android Studio simulator and my phone and can confirm the fix, though as my previous comment, livekit/livekit#4907 (comment) already fixes the issue quickly through the resend. |
2 participants
keep dynacast-paused simulcast layers paused across publisher renegotiations (publish/unpublish, codec change, ICE restart), instead of silently uploading them until the next subscribed-quality change.