Better track settings#1047
Draft
hiroshihorie wants to merge 12 commits into
Draft
Conversation
A manual setVideoQuality/Dimensions/FPS/enable/disable sent immediately via _emitTrackUpdate() but did not cancel a pending debounced visibility send. The debounce captured a stale settings snapshot that could fire ~1.5s later and overwrite the manual update; since the visibility send never updated _lastSentTrackSettings, the dedup gate then suppressed any re-correction, leaving the server permanently on stale settings. - _emitTrackUpdate() now cancels any pending debounced send before sending. - the debounced send re-builds from current state at fire time and updates _lastSentTrackSettings, so it can never deliver or wedge on stale settings.
Mirrors the JS SDK tri-state. Previously `disabled` was computed as `!_enabled || !_adaptiveStreamEnabled`, an OR that meant an explicit enable() could never keep an off-screen track streaming when adaptive stream was on. - Replace the plain bool _enabled with a tri-state bool? _requestedDisabled (null = no explicit request); an explicit enable()/disable() now takes precedence over visibility, matching JS isEnabled. - Decouple the misnamed _adaptiveStreamEnabled into _adaptiveStreamActive (feature active for this pub) and _adaptiveStreamVisible (views visible). - Extract the precedence into a pure resolveDisabled() in track_settings.dart.
The merge is performed client-side (resolveVideoSettings), and only the single resolved value is sent; the server does not receive both and pick the smaller.
The existing 'equal areas keep preferred' test passed identical dimensions as both adaptive and preferred, so the assertion held regardless of which branch ran and could not catch a < -> <= regression. Use distinct same-area dimensions (720x320 vs 640x360), and add a one-px-smaller case where adaptive should win.
_buildTrackSettings had no coverage for the disabled computation, fps passthrough, or the proto field mapping. Extract the proto assembly into a pure buildUpdateTrackSettings() (alongside the new resolveDisabled()) so both are unit-testable without a live RemoteTrackPublication, and add tests for the tri-state disabled precedence and the dimensions/quality/fps mapping.
Replace the nullable bool _requestedDisabled (null/false/true) with an @internal TrackEnabledPreference { unset, enabled, disabled } enum. Removes the double-negative (requestedDisabled = false meaning 'enabled') and makes the precedence in resolveDisabled() read as an explicit switch. Behavior is unchanged; the enum is internal-only (non-exported file, @internal).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Attempt to use a "merged" logic with user track preference when adaptiveStream is enabled, instead of ignoring or throwing .