Skip to content

[players] Guard the review room against event feedback storms#2114

Open
frankrousseau wants to merge 1 commit into
cgwire:mainfrom
frankrousseau:fix-1574-review-room-event-storm
Open

[players] Guard the review room against event feedback storms#2114
frankrousseau wants to merge 1 commit into
cgwire:mainfrom
frankrousseau:fix-1574-review-room-event-storm

Conversation

@frankrousseau

Copy link
Copy Markdown
Contributor

Problems

  • In a playlist review room, changing shots 10-12 times quickly during playback freezes Kitsu: clients keep oscillating between the last two videos (fixes Loss of control in the playlist review room #1574). Applying a remote room update writes refs (speed, laser mode, waveform, annotations display, comparison state...) whose component watchers call updateRoomStatus(), so every applied update was re-broadcast and stale in-flight events kept re-applying old states.

Solutions

  • Mute outgoing room-updated broadcasts while a remote state is applied (and for a 500ms grace window so the deferred watcher/nextTick/time-update echo paths are covered) — an applied network state is never re-broadcast.
  • Add a hybrid-logical-clock sequence to outgoing updates and drop incoming events whose sequence is behind the last applied update or the user's own last local action. The Zou relay rebuilds the payload from a field whitelist, so the sequence rides inside local_id ("<uuid>#<seq>", relayed verbatim) in addition to a first-class room_seq field for a future Zou pass-through; the existing local_id self-echo guard now compares the uuid prefix.
  • Throttle outgoing status updates (200ms, leading + trailing) so a burst of quick shot changes emits the first and final state instead of one event per change.
  • Backward compatible: events without a sequence (older clients, or the server-built join payload) are always applied as before; old clients ignore the suffix since they only compare local_id against their own plain uuid. Residual risk: an old client in a mixed-version room still lacks the re-broadcast mute, so a storm remains possible between two old clients; full arbitration would need a Zou-side sequence check.
  • Guard logic extracted as pure helpers in src/lib/players/roomSync.js with unit tests (stale, out-of-order, missing-sequence, self-echo) plus composable-level tests for the mute, the stale drop and the burst throttle.

Rapidly changing shots in a review room made clients re-apply stale
in-flight room updates and re-broadcast each other's states in a loop
(cgwire#1574). Broadcasts are now muted while a remote state is applied,
room-updated events carry a sequence number (riding inside local_id
because the Zou relay only passes whitelisted fields through) so stale
and out-of-order events are dropped, and outgoing bursts are throttled
to a leading and a trailing emit. Payloads without a sequence keep the
previous behavior for mixed-version rooms.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Loss of control in the playlist review room

1 participant