Fix audio sync + a batch of correctness/robustness bugs#166
Open
xn101de wants to merge 9 commits into
Open
Conversation
TimeProvider.setDiff computes the median client/server clock offset to drive audio/video synchronization, but sorted the offset buffer with a bare Array.prototype.sort(). With no comparator, sort() coerces elements to strings and orders them lexicographically, so the "median" was taken from a wrongly ordered array (e.g. [2, 10, -5, 100] -> [-5, 10, 100, 2]). The resulting offset is unstable and frequently wrong, corrupting every serverTime() calculation and causing playback drift and "Chunk too old, dropping" stalls. Sort numerically with an explicit comparator. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed PCM samples peak at 2^(bits-1) (e.g. 32767 for 16-bit), so to map them into the Web Audio [-1, 1) float range they must be divided by 2^(bits-1). Dividing by 2^bits halved every sample, playing all audio roughly 6 dB quieter than intended. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
JsonMessage.serialize() wrote the UTF-16 string length into the size field while allocating the buffer from getSize(), which uses the UTF-8 byte length. For any payload containing non-ASCII characters (e.g. a client name like "Küche") the declared size was smaller than the actual encoded bytes, truncating the message on the wire. Use the encoded byte length for the size field. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Prefer the native crypto.randomUUID() for generating the persistent client id, falling back to the existing Math.random implementation when it is unavailable (non-secure contexts) or throws. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The seekbackward/seekforward/seekto handlers computed Math.max/Math.min clamps whose results were never assigned or used. They had no effect; remove them to clarify that the relative offset / absolute position is sent to the server as-is. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The 'change' listener for the system color scheme was added in the component render body, leaking a new listener on every render. Move it into a useEffect that registers once and removes the listener on cleanup. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Drop the multi-kilobyte commented-out example Server JSON (and the unused `server.fromJson(json)` line) that was left in the component for local testing. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The dummy re-render counters were updated with setUpdate(update + 1), which reads a value that may be stale under React batching. Switch to the functional form setUpdate(u => u + 1) and drop the now-unused state value binding. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
updateVolume() divided the summed client volume by clients.length without checking for an empty group, producing a NaN slider value when a group had no visible clients. Return a volume of 0 in that case. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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
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.
A batch of independent, self-contained fixes (one commit each, so they can be
reviewed/cherry-picked individually).
Audio correctness
TimeProvider.setDiffsorted theclock-offset buffer with a bare
Array.sort(), ordering numberslexicographically → wrong median, unstable
serverTime(), playback drift andChunk too old, droppingstalls. Now sorts numerically. (likely Snapweb slightly out of sync with snapdroid clients #39;contributes to Safari not working #49, Change snapweb client to stream in playback, does not work #108)
2^bitsinstead of2^(bits-1), so all audio played ~6 dB too quiet.JsonMessage.serialize()wrote thestring length while the buffer was sized from UTF-8 bytes, truncating payloads
with non-ASCII characters (e.g. "Küche").
Robustness / React
uuidv4()preferscrypto.randomUUID()with a safe fallback.prefers-color-schemelistener moved from render body into auseEffectwithcleanup (was leaking a listener per render).
setState.Cleanup
Verified:
tscclean,vite buildsucceeds.🤖 Generated with Claude Code