fix(client): don't start audio playback after the element is unbound - #2477
RaphaelFakhri wants to merge 1 commit into
Conversation
|
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: This review used your included allowance. Your plan provides up to 2 included reviews per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe audio binding now skips delayed stream updates after cleanup. A regression test checks that cleanup before deferred work prevents source attachment and playback. ChangesAudio binding cleanup
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix Suggested reviewers: Merge Risk: ⚪ Minimal · up to Deferred audio updates are guarded after unbinding, and the regression test covers that timing. No concrete merge-blocking risk remains. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to The change prevents delayed playback after unbinding without adding a new way to reach the audio-binding API. Some browser-dependent completion paths remain outside this fix. Retained concerns Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
Resilience and Maintainability Implications
Hardening Proposals
🚥 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 |
💡 Overview
DynascaleManager.bindAudioElement()can start audio playback on an element after the function that unbinds it has run. This happens when the element is unbound in the same tick as the stream update, for example when a component mounts and unmounts right away or when a participant leaves just after the bind.📝 Implementation notes
The stream subscription defers the attach with
setTimeout. The callback assignssrcObject, callsplay(), creates aMediaPlaybackWatchdog, and connects a Web Audio source node when Web Audio is in use. The function returned bybindAudioElement()unsubscribes and clearssrcObject, but it doesn't cancel the pending timer. When the timer fires afterwards, the unbound element plays audio, the watchdog is never disposed, and the source node stays connected to the audio context.The unbind function now sets a flag and the deferred callback returns early when the flag is set.
A regression test in
DynascaleManager.test.tsupdates the participant's audio stream, unbinds the element, runs the timers, and checks thatsrcObjectstaysnullandplay()isn't called. The test fails on the previous code and passes with this change.🎫 Ticket: n/a
📑 Docs: n/a
Summary by CodeRabbit
Bug Fixes
Tests