fix(watch): survive an audio decoder error, resample, and re-anchor on resubscribe#2189
Closed
fperex wants to merge 2 commits into
Closed
fix(watch): survive an audio decoder error, resample, and re-anchor on resubscribe#2189fperex wants to merge 2 commits into
fperex wants to merge 2 commits into
Conversation
Every run loop logged any error from a stream as a warning, so a normal unsubscribe, a publisher handover or a peer closing the session produced scary warnings on every viewer that stopped watching. isStreamAbort() decides on the application code the relay encodes (WebTransportError.streamErrorCode natively, or the trailing number in qmux's RESET_STREAM/STOP_SENDING and Connection closed messages on the WebSocket fallback). Coded faults are a denylist, so an unknown code is treated as routine teardown while a client-actionable fault (auth, not-found, protocol, unroutable) still surfaces at warn. Call sites in the lite/ietf publishers, the subscriber and the connection run loop now pick their log level from it. The bandwidth probe streams no longer swallow genuine faults at debug.
…n resubscribe Audio had the same fatal-decoder problem as video: an AudioDecoder that errored never decoded again, so the stream went silent until a reload. Restart it in place on a budget, with the same config-supersession check. Also fix three sources of crackle and drift: - Resample when the catalog rate and the AudioContext rate differ, instead of writing samples into a ring built at the wrong rate. - Snap timestamps onto the frame cadence so a jittery per-frame timestamp does not zero-fill or overwrite a sample every frame. - Re-anchor the discontinuity counter on resubscribe. Audio.Decoder is built once and its #discontinuity outlives the subscription, but the container Consumer's rewind counter is per-consumer and restarts at zero, so after any rewind the next resubscribe delivered 0, looked like a rewind, and reset the Sync reference shared with video. Resume the AudioContext on a user gesture rather than assuming it starts running.
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Collaborator
How did this happen? Blindly retrying is the wrong fix, please try to reproduce the original bug instead. |
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
2 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.
Split out of #2163.
Audio had the same fatal-decoder problem as video (#2187): an
AudioDecoderthat errored never decoded again, so the stream went silent until a reload. It restarts in place on a budget now, with the same config-supersession check.Plus three fixes for crackle and drift, all of which I hit on Safari but which are not Safari-specific:
AudioContextrate disagree, samples were written into a ring built at the wrong rate, which garbles playback outright.Audio.Decoderis built once and its#discontinuityoutlives the subscription, but the containerConsumer's rewind counter is per-consumer and restarts at zero. So after any rewind had bumped the decoder's count, the next resubscribe delivered 0, which looked like a rewind and reset theSyncreference shared with video, desyncing audio from video for no reason.Also resumes the
AudioContexton a user gesture rather than assuming it starts inrunning.Public API changes:
@moq/watchgainsAudioBuffer.wait(timestamp, signal?)(additive).Test plan:
bun test js/watch/src/audio js/hang/src/container: includes a test pinning that a fresh/resubscribedConsumerstarts its rewind counter at zero (the bug above), plus the timestamp-snapping math.just js checkgreen.