Fix pixel density in AdaptiveStream#1098
Merged
Merged
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).
hiroshihorie
force-pushed
the
hiroshi/adaptive-stream-pixel-density
branch
from
1fdd82b to
86a4ee3
Compare
The adaptive-stream visibility observer fed each view's logical (DIP) size straight to the server, so on retina/HiDPI displays it under-requested by the device pixel ratio and the server returned an upscaled/soft layer. Server layers are sized in physical pixels. - Add AdaptiveStreamPixelDensity (auto | fixed(double)) capped at 3x, mirroring the JS pixelDensity option. - Add a per-view VideoTrackRenderer.adaptiveStreamPixelDensity (default auto); the density is stored per view-key on the track. - _computeVideoViewVisibility scales each view's logical size by its own resolved density (auto = that view's MediaQuery devicePixelRatio) before taking the largest size across the track's views. Because density is resolved per view, a track shown on screens with different scales (e.g. built-in retina + external 1x) requests correctly for each.
hiroshihorie
force-pushed
the
hiroshi/adaptive-stream-pixel-density
branch
from
86a4ee3 to
a4c0dbe
Compare
xianshijing-lk
requested changes
May 29, 2026
|
|
||
| @internal | ||
| void updateViewKeyPixelDensity(GlobalKey key, AdaptiveStreamPixelDensity pixelDensity) { | ||
| if (viewPixelDensities.containsKey(key)) viewPixelDensities[key] = pixelDensity; |
Contributor
There was a problem hiding this comment.
should we log or handle the case where if (!viewPixelDensities.containsKey(key)) ?
Member
Author
There was a problem hiding this comment.
Good point 🙂 I changed this to keep the key and pixel density together in VideoTrackViewRegistration, so there is no separate map lookup anymore. Missing context/size is still skipped as part of the normal widget lifecycle.
xianshijing-lk
approved these changes
May 29, 2026
Contributor
xianshijing-lk
left a comment
xianshijing-lk
left a comment
There was a problem hiding this comment.
one question, lgtm in general
…tion The map().nonNulls iterable was lazy and consumed three times per 300ms visibility tick (length, isNotEmpty, reduce), re-running findRenderObject and MediaQuery lookups on each pass. Materialize once with toList().
Use MediaQuery.maybeDevicePixelRatioOf instead of maybeOf(context)?.devicePixelRatio so the view depends only on the pixel ratio rather than every MediaQuery field (padding, text scale, etc.).
hiroshihorie
changed the title
hiroshihorie
changed the title
cloudwebrtc
approved these changes
Jun 1, 2026
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
3 participants
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.
The adaptive-stream visibility observer fed each view's logical (DIP) size straight to the server, so on retina/HiDPI displays it under-requested by the device pixel ratio and the server returned an upscaled/soft layer. Server layers are sized in physical pixels.
AdaptiveStreamPixelDensity(auto|fixed(double)) mirroring the JS SDK'spixelDensityoption, plusVideoTrackRenderer.adaptiveStreamPixelDensity(default:auto), set per view rather than room-wide. Fractional densities are supported and the resolved value is capped at 3x to bound bandwidth.VideoTrackViewRegistrationthat owns both theGlobalKeyand pixel density, so density updates stay tied to the renderer lifecycle.devicePixelRatioviaMediaQuery.maybeDevicePixelRatioOf; fixed modes use a constant multiplier._computeVideoViewVisibilityscales each view's logical size by the resolved density, materializes the visible sizes once per tick, and requests the largest scaled dimensions across all views.