fix(upnp): stop the volume overlay jumping on hardware key presses - #256
tbrackbill wants to merge 1 commit into
Conversation
Android answers a hardware volume key by setting MediaSessionRecord's mOptimisticVolume to currentVolume ± 1 and displaying that for one second before the real value arrives. The VolumeProvider was published with maxVolume 100 while each press adjusted by 5, so the optimistic value moved by 1 and ours by 5. A second after every press the system volume overlay visibly jumped from one to the other. Publish the provider on a scale of one unit per step instead, so Android's ±1 is exactly the adjustment we make and there is nothing to correct. Everything the handler hands to callers stays in UPnP percent; only the scale published to the media session changes. Verified against a Pixel 7 Pro and upmpdcli: dumpsys media_session reported volumeType=REMOTE, max=100 before this change. Note that the overlay itself is not observable through dumpsys — mOptimisticVolume lives inside MediaSessionRecord and is never published to the provider value — so the scale relationship is pinned by unit tests rather than by instrumentation. Also documents why direction 0 is ignored: Android sends ADJUST_SAME on key-up, and acting on it issued a redundant SOAP SetVolume for no change. The same applies to the percentage the renderer last reported. The handler used to add 5 to it, so an off-step value (upmpdcli reported 33%) stayed off-step on every press: 28, 23, 28, 33, while the system slider showed round(x/5), i.e. 35%. Presses now step in provider units, so the first one lands on the slider's scale and the renderer and slider agree from then on. Trade-off: dragging the system volume slider now moves in 5% steps (21 stops) rather than 1%. This applies to Chromecast as well, which shares setRemotePlayback. It is inherent to the fix: Android's optimistic ±1 only matches our adjustment if one provider unit is one step. The unit tests cover the percent/unit conversions; the instance methods are gated on Platform.isAndroid, so the wiring is covered by the device checks above. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 4 included reviews per hour; 2 remain after this review. 📝 WalkthroughWalkthroughRemote Android volume now uses provider units on a 0–20 scale. Conversion helpers map between those units and percentages. Playback updates, callbacks, and hardware adjustments use the scaled values. ChangesRemote volume scaling
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix Suggested reviewers: Merge Risk: ⚪ Minimal · up to No concrete issue remains that requires a fix before merging. The handler-level test coverage question remains open. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to The new scale aligns hardware-key presses with five-percent volume steps. Callers still receive percentages, and the reviewed paths show no new access route. Android callback behavior and rapid or failed remote updates remain less fully verified. Retained concerns Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Problem
With a DLNA renderer connected, the Android volume overlay jumped a second after every hardware
volume key press.
Android answers a volume key by showing an optimistic
currentVolume ± 1for about a secondbefore the real value arrives. The VolumeProvider was published with
maxVolume100 while eachpress adjusted by 5, so the optimistic value moved by 1 and the real one by 5.
A second problem: the handler added 5 to whatever percentage the renderer last reported, so an
off-step value stayed off-step. upmpdcli reported 33%, and presses went 28, 23, 28, 33 while the
system slider showed 35%.
Fix
change we make. Everything the handler passes to callers stays in UPnP percent; only the scale
published to the media session changes.
slider agree from then on.
a redundant SetVolume.
Trade-off: dragging the system volume slider now moves in 5% steps (21 stops) instead of 1%.
This applies to Chromecast too, since it shares
setRemotePlayback. It is inherent to the fix:Android's ±1 only matches our adjustment if one provider unit is one step.
Testing
gated on
Platform.isAndroid, so the wiring was checked on device instead.flutter test: master's 118 passing tests plus the 8 new ones pass. The 13 tests that fail onmaster fail the same way here, and there are no new analyzer issues.
on every key press the renderer's
GetVolumeequals the provider value × 5, and an external33% snaps to 30/35 on the next press.
dumpsys(the optimistic value lives insideMediaSessionRecord), so the scale relationship is pinned by the unit tests.
Independent of the other UPnP PRs; they merge cleanly in any order.
🤖 Generated with Claude Code
Summary by CodeRabbit