Repository navigation
Keep dynacast-paused layers paused across publisher renegotiations - #1148
Conversation
Applying a publisher answer re-enables every RTP encoding, and the server only ever reports subscribed qualities that changed, so layers dynacast had paused were never paused again after a renegotiation. Accumulate the subscribed qualities on the track and re-apply them once the answer is set. Senders are fed the accumulated list on every path, so a re-apply cannot disagree with the live update that preceded it.
|
|
There was a problem hiding this comment.
Devin Review found 1 potential issue.
1 flag not posted on this PR by your GitHub settings — view it in Devin Review. (Configure)
| if !qualities.isEmpty, let sender = track._state.rtpSender { | ||
| sender.raw._set(subscribedQualities: qualities) |
There was a problem hiding this comment.
🔴 Stale generic qualities override primary codec
When updates switch from generic to codec-specific, refreshSubscribedQualities reapplies stale generic qualities after current primary-codec qualities. Both caches target the same sender, so renegotiation can reopen a paused layer or suppress an enabled one.
Learn more
The signal callback applies either a generic quality list or a codec-specific list, depending on which field the server includes (didUpdateSubscribedCodecs). Each path now retains its own cache. A primary codec update does not clear or reconcile the generic cache. On a publisher answer, the refresh applies each codec cache and then writes the generic cache to the primary sender, even if that cache is older. This leaves the sender with the older subscription state until another signal update changes it.
Example: The generic list caches high: true. Later the primary VP8 codec receives high: false. After a new answer, the codec refresh disables high, then the generic refresh re-enables it; the publisher transmits a layer the server paused.
Recommended fix: Track which cache most recently controls the primary sender, or reconcile generic updates into its codec-specific cache. In refreshSubscribedQualities, apply only the authoritative state for each sender, including when signaling changes between generic and codec-specific updates.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
The overwrite would need a track to receive a generic-only update followed by a codec-specific one. Checked the server: SubscribedQualityUpdate has had a single builder since v1.1.0 (May 2022) that always populates both fields (SubscribedQualities = SubscribedCodecs[0].Qualities, kept "for compatible with old client"), and the format never changes for a track within a session. The SDK takes the codec branch whenever subscribed_codecs is non-empty, so the generic cache is never populated against any supported server and this isn't reachable. Theoretical hazard only; removing the deprecated generic path would eliminate it entirely.
There was a problem hiding this comment.
Unrelated to this change, just triggered here.
Applying a publisher answer re-enables every RTP encoding, and the server
only ever reports subscribed qualities that changed, so layers dynacast
had paused were never paused again after a renegotiation. Accumulate the
subscribed qualities on the track and re-apply them once the answer is
set. Senders are fed the accumulated list on every path, so a re-apply
cannot disagree with the live update that preceded it.