Gate emitUIInteraction on data-channel readiness instead of video#911
Conversation
|
Looks good. My one concern is consistency: emitCommand, emitConsoleCommand, and the other methods using the same isVideoReady guard send over the same data channel, so they'd presumably benefit from the same change. Was leaving them out intentional, or worth doing here too? |
|
Good catch, thanks, not a deliberate distinction. You're right that these are the same class as emitUIInteraction: they send application input/commands over the reliable data channel and don't actually depend on video being decoded. I've extended the gate to emitCommand, emitConsoleCommand, sendTextboxEntry, requestShowFps, and requestDataChannelLatencyTest (that last one only ever needed the data channel). I left requestLatencyTest and requestIframe on isVideoReady on purpose — those genuinely concern the video pipeline (one measures video round-trip latency, the other asks the encoder for a keyframe), so gating them on the data channel would let them fire before there's anything to measure or refresh. I did notice the request* functions are marked for an upcoming refactor, so I kept the change there minimal — this is just a correctness fix for the current behavior. Happy to drop the two request* ones if you'd rather not touch them ahead of that work. |
Relevant components:
Problem statement:
PixelStreaming.emitUIInteraction()only sends the message ifisVideoReady()returns true, i.e. once the video element has started decoding frames. But a UIInteraction travels over the reliable, ordered data channel, which opens several seconds before the first video frame arrives. So on a slow or high-latency connection, any UIInteraction sent in that early window is silently dropped (the method just returnsfalse).This bit us when the application uses an early UIInteraction to receive setup parameters from the page before it starts streaming video. The interaction was sent while the data channel was already open but the video wasn't ready yet, so it never reached the streamer, and the app fell back to its defaults. It's intermittent because it only happens when video init loses the race.
The same wrong guard applies to the other methods that send over the data channel, not the video track, so they have the same latent problem.
Solution
Gate the data-channel methods on the data channel being open rather than on the video being ready, since the data channel is what these messages actually use. I added a small public helper
WebRtcPlayerController.isDataChannelOpen()(returns true when the to-streamer send channel'sreadyState === 'open') and switched the guard to use it in the methods that transmit over the data channel:emitUIInteractionemitCommandemitConsoleCommand(still also gated onallowConsoleCommands)sendTextboxEntryrequestShowFpsrequestDataChannelLatencyTestrequestLatencyTestandrequestIframekeep theisVideoReady()guard, because they genuinely depend on the video stream.Documentation
The new
isDataChannelOpen()method has a doc comment, and the reasoning is in a comment on the changedemitUIInteractionguard. No public API was removed and the normal case is unchanged — this only stops early data-channel messages from being dropped.Test Plan and Compatibility
npm run buildandnpm run lintpass forFrontend/library. I've been running this change in our own deployment for a while: opening with per-session parameters and reconnecting repeatedly, the parameters now arrive on every connect, including on a slow headless instance where it previously failed roughly one connect in five. Behaviour is identical once video is ready, so existing callers are unaffected.