fix(player): read the whole WAV header, and say so when a track cannot load - #362
Conversation
The chunked engine parses WAV containers itself and asked only for bytes 0-1023 when looking for the `data` chunk. RIFF is a linked list, so anything the writer puts in front of `data` -- a LIST/INFO block, a JUNK chunk padded for sector alignment -- pushes it out of that window. The parser returned null, the engine reported a duration of 0, and playback was disabled. The track rendered normally and the header showed its real length, so it looked like a GPU or renderer fault rather than a parse failure (#343). Walk the chunk table properly and widen the request when it runs past what was fetched, capped at 1 MB and 5 attempts. The parser reports "need more bytes" separately from "not a WAV", since only the caller knows whether more bytes can be had. Two further container cases fixed along the way: - WAVE_FORMAT_EXTENSIBLE carries the real format code in its SubFormat GUID. Without reading it, a float32 file parsed cleanly and then decoded to silence. - A `data` size of 0 or 0xffffffff, written by encoders that stream to a non-seekable target and never patch the length, gave a duration of 0 or 24347 seconds respectively. Clamp to the real length reported in Content-Range. Adds tests/js/wav-header.test.mjs, which drives the real engine against synthetic layouts through a Range-honouring fetch stub. Against the pre-fix engine it reports 25/38, with JUNK 4096, LIST 2 KB, JUNK 300 KB and both unpatched data sizes failing. CI runs it alongside node --check. Closes #358
A track whose stems could not be loaded left the studio looking normal and said nothing. The only trace was a console warning, which in a release desktop build has no reachable devtools, so the failure was invisible to the user and undiagnosable from a bug report. Working out why #343 could not play took a screenshot and a round trip for a hexdump. Both engines now record why ready() resolved false and expose it via getLoadError(), separating a stem that could not be fetched from one that could not be parsed or decoded -- those send the user somewhere completely different. The player puts that message in the error box above the track header. Playback errors reuse the import error box, so they are tagged: the player retracts its own message when another track loads, without wiping an import failure the user has not read yet. Nothing cleared that box on track switch before. The player also now retries with the full-decode engine when the chunked one cannot read a container, under the same RAM ceiling the missing-peaks swap uses. The browser's own decoder handles layouts the hand-rolled parser may not, so this turns "playback disabled" into "playback works" for the whole class of container problems behind #343. Closes #359
_pcmToAudioBuffer only handles 16-bit PCM and 32-bit float, but the header parser accepted any depth. A 24-bit or 32-bit-integer file therefore measured correctly, reported ready, and then decoded to nothing on every chunk. That is worse than failing outright. An all-empty chunk result is treated as a transient network failure and evicted from the cache, so the scheduler retries it on the next animation frame, forever, with the playhead pinned at zero and no message on screen. Measured against a synthetic 24-bit file with playback running: 82 range requests in 700 ms (~117/sec) versus 3 for a healthy file. Reject those formats at parse time instead. The engine then reports a readable reason and the player hands the file to the full-decode engine, whose decoder handles 24-bit and integer PCM -- so these files now play instead of hanging. Verified end to end with a real ffmpeg-produced 24-bit stem: the chunked engine declines it, the fallback picks it up, the transport advances, and playback issues no further range requests. Also covers WAVE_FORMAT_EXTENSIBLE float32, which is only accepted because the real format code is read out of the SubFormat GUID; without that it reads as 0xfffe and is now correctly rejected rather than silently decoding to nothing.
|
Third commit added after a self-review pass turned up a related defect in the same code path.
That is worse than failing outright. An all-empty chunk result is treated as a transient network failure and evicted from the cache, so the scheduler retries on the next animation frame indefinitely, with the playhead pinned at zero and nothing on screen. Measured with playback running:
Rejecting those formats at parse time hands the file to the full-decode engine, whose decoder handles them, so they now play instead of hanging. Verified end to end with a real ffmpeg-produced 24-bit stem: the chunked engine declines it, the fallback picks it up, the transport advances to 00:05, and playback issues no further range requests. Healthy files are unaffected at 3 requests either way. Worth noting this is a candidate cause for #343 in its own right: a transport stuck at 0:00 against a correct header duration is exactly the reported symptom, and it needs no unusual chunk layout at all, just a torchaudio backend that writes 24-bit. JS regression test now at 48 checks. |
Closes #358. Closes #359. Very likely the cause of #343, pending the reporter's hexdump.
The bug
The chunked engine parses WAV containers itself rather than using the browser decoder, and it asked only for
bytes=0-1023when looking for thedatachunk. RIFF is a linked list, so anything a writer puts in front ofdatapushes it out of that window: a LIST/INFO block, or a JUNK chunk padded for sector alignment.The parser returned null, the engine reported a duration of 0, and
wireUpAudiodisabled playback with aconsole.warn. Everything else rendered correctly, including the real track length in the header, which is why #343 looked like a GPU or renderer fault.torchaudio.savedispatches to whichever backend is installed, so two machines running the same StemDeck build can emit different container layouts. That is how a file-format bug ends up looking hardware dependent.Changes
Header parsing walks the chunk table and widens the request when it runs past what was fetched, capped at 1 MB and 5 attempts. The parser distinguishes "need more bytes" from "not a WAV", since only the caller knows whether more bytes can be had. A corrupt table that fails to advance, or claims to run past EOF, is rejected instead of driving the widening loop forever.
Two further container cases surfaced while testing:
datasize of 0 or0xffffffff, written by encoders that stream to a non-seekable target and never patch the length, produced a duration of 0 or 24347 seconds. Now clamped to the real length fromContent-Range.Failure reporting. Both engines record why
ready()resolved false and expose it viagetLoadError(), separating a stem that could not be fetched from one that could not be read. The player shows that message in the error box above the track header.Playback errors reuse the import error box, so they are tagged: the player retracts its own message when another track loads, without wiping an import failure the user has not read. Nothing cleared that box on track switch before, so this would otherwise have left stale errors over healthy tracks.
Full-decode fallback. When the chunked engine cannot read a container, the player now retries with the browser's own decoder under the same RAM ceiling the missing-peaks swap already uses. This turns "playback disabled" into "playback works" for the whole class of container problems, independently of whether the parser handles a given layout.
Verification
tests/js/wav-header.test.mjsdrives the real engine through a Range-honouring fetch stub, so it covers the widening loop, the parser and the reporting together via the module's public API. CI runs it next tonode --check.Against the pre-fix engine it reports 25/38:
Against this branch, 38/38.
Browser checks against a real track on a local server, 8/8: boots with no page errors (the job.js <-> player.js cycle is pre-existing, catalog.js does the same), a healthy track still reports
00:00 / 03:04, corrupt stems now surface an on-screen message naming the cause, and the stale error is retracted when a healthy track loads. Separately confirmed a healthy track still loads on the chunked engine and is not quietly rescued by the new fallback, which would have regressed RAM use for everyone.Backend suite: 526 passed.
tests/test_stems_api.py::test_all_stems_zip_oggfails, but it fails identically on cleanmain: the local ffmpeg has nolibvorbisencoder and the test's skip guard only checks that ffmpeg exists. Unrelated to this branch, not fixed here.Not included