webui: play a finished song against its review #10

2026-09-25T23:33:31-07:00

dejima-agent commented

2026-09-25 23:33:31 -07:00

Summary generated by LLM agent

The review panel now plays the finished song folder, so checking whether the notes line up no longer means packaging the song and finding it in the game. A transport sits above the lyrics; as it plays the line being sung is highlighted, words light up as they are reached, and a playhead sweeps each line against its notes. Clicking a line or a word plays from there. The player sticks to the top of the panel and the view follows the active line, so the picture stays in frame while the lyrics scroll under it.

This is the first step towards editing in place, so it is deliberately only playback: nothing here writes.

Modelled on the game, not on convenience

The #MP3 is the audio and the clock; the #VIDEO is muted background slaved to it, re-seeked whenever it drifts more than 0.25s. That is what the game does, and it matters for the purpose: the times driving the highlight are the same seconds the notes were generated from, so a word that lights up late in the browser is late in the game too. Measured drift across a playthrough stayed at or below 0.04s.

The echo

While this was in progress you hit a song playing multiple copies of its audio at once. I think this is the cause, and it is fixed here.

package wrote the video with its audio track intact, next to an #MP3 holding the same recording. Any player that does not mute the video plays both, a few frames apart, which is heard as an echo. Both branches now drop it: the compatible-video path remuxes with -c copy -an instead of copying the file, so it still does not re-encode, and the transcode path passes -an.

I ruled out the other candidate first. bestvideo*+bestaudio looks like it could merge two audio streams, but yt-dlp ignores every format with an audio stream after the first unless --audio-multistreams is set, and that selector is yt-dlp's own default. It is not the source.

Worth confirming on your end, since I could not reproduce your song: ffprobe '<song>.mp4' on an affected folder should show an audio stream. If it does, this is it and a rebuild clears it — the fix only applies to songs packaged from now on. If it does not, the echo is something else and I would want the folder listing and which player you were in.

The browser player mutes the video element regardless, so it stays correct for folders built before this.

The endpoint

GET /api/jobs/{id}/media/{audio,video,cover} serves whatever that song's own #MP3/#VIDEO/#COVER headers name, read through parse_ultrastar rather than guessed from extensions — package rewrites #VIDEO when it transcodes, so the extension is not fixed. A resolved path whose parent is not the song folder is refused, which covers both ../ and absolute headers. Range requests come free from FileResponse; without 206 the browser cannot seek, which would defeat the point.

Also in here, from your later notes

CREATE SONG moved out of the form to the top of the queue, always available, replacing NEW SONG INSTEAD. It clears the form and brings it back into view from wherever playback has scrolled to, asking first if you have typed anything.

A successful submission now keeps the fields it sent rather than emptying them. The song just submitted is the obvious starting point for the next one, and nothing has to be retyped to correct it. The form drops back to NEW SONG mode so a second submit creates a new song rather than re-rebuilding the old parent, and the queue's CREATE SONG button is now the way to get an empty form.

One related fix: setFormMode opened the Advanced section based on whether a job was loaded, which would have hidden retained genre/year/language values after a submit. It now keys off whether the fields actually hold anything.

Testing

17 new endpoint tests: each media kind served, the header-rewritten extension followed, range requests, unknown kinds, missing files, a dropped #COVER, a missing song txt, failed and running jobs, and the two traversal cases. I checked the containment guard is load-bearing rather than incidentally passing — with path.parent != out removed, both traversal tests fail and the secret file is served.

A new _playback_flaws frontend checker with six mutations: sync falling back to timeupdate's ~4Hz, scrolling while paused, a left line keeping its highlight, a word staying lit, an unrenderable review leaving the old song loaded, the player not pointed at the packaged audio, the background video unmuted, and the picture never re-synced. Plus mutations for the two form changes. package gains a parametrized test that both branches pass -an, and the copy-branch test now asserts -c copy so the remux cannot quietly become a re-encode.

Full suite: 312 passed, 1 skipped. tests/test_audio_io.py and tests/test_separate.py were not run — they import torch, which is not installable in the environment I had; neither touches this change.

Beyond the tests I drove the real page with Playwright against real ffmpeg-generated media and checked the states rather than assuming them: the endpoint's content types and 206 responses, the audio element as the clock with the video muted and its drift, seeking mid-line, playing across a line boundary, click-to-seek on a word, rewinding to silence, the player unloading for a song with no media, and the sticky player still pinned after scrolling to the bottom. Separately for the form changes: the queue button visible in both modes, clearing and refocusing the form from the bottom of the page, and the discard prompt both dismissed and accepted.

docs/images/webui.png is regenerated and now shows the panel in use. The screenshot script gained an ffmpeg step to give the selected song a playable folder; without ffmpeg on PATH it still runs, just without the player.

Follow-up: clearing the page, and redoing a stage

CREATE SONG now clears the whole page. It emptied the form but left the previous song's review, player, output path and queue selection sitting under it. The cause was worse than it looked: clearDetail reached for an element id that no longer exists, so it threw partway through and everything after the first two lines never ran. It now targets the review box, takes the reason to show ("no songs queued yet" is wrong when you have simply deselected), and a composing flag stops the next poll selecting the newest song back into the blank page.

Rebuild can now restart from a chosen stage. Rebuilding a song packaged before the de-echoing found every stage cached and kept the echo, because the planner works off which input fields changed and a change in songtrace itself changes none of them. The rebuild form gains a Restart from select — the CLI's --force-from, populated from /api/stages. It combines with the planner rather than replacing it: an explicit stage may widen what is rerun, never narrow it below what the edit requires. Picking package repackages without re-downloading, which is the fix for an affected song.

Two things found while verifying this:

A browser submits a textarea with CRLF while the stored lyrics use LF, so every rebuild from the page looked like a lyrics edit and redid the alignment. Lyrics are normalised to LF on the way in. This showed up as a rebuild logging restarting from align (lyrics_text changed) when nothing had been touched.
[hidden] is only display: none, which .field's display: flex was winning, so the new control rendered in new-song mode even though the property was set. Caught by looking at the screenshot rather than trusting the DOM property — the browser check has been changed to assert real visibility via offsetParent.

Testing: 9 new endpoint tests for the restart stage (asked-for stage used when nothing changed, planner used when nothing is asked, each one winning when it is the earlier, unknown stage rejected, blank treated as unset, rebuild-from-scratch unchanged), 2 for CRLF, 2 unit tests for earliest, and 2 more frontend mutations (the review left behind under a blank form, the poll reselecting into it). Full suite 325 passed, 1 skipped.

**Summary generated by LLM agent** The review panel now plays the finished song folder, so checking whether the notes line up no longer means packaging the song and finding it in the game. A transport sits above the lyrics; as it plays the line being sung is highlighted, words light up as they are reached, and a playhead sweeps each line against its notes. Clicking a line or a word plays from there. The player sticks to the top of the panel and the view follows the active line, so the picture stays in frame while the lyrics scroll under it. This is the first step towards editing in place, so it is deliberately only playback: nothing here writes. ## Modelled on the game, not on convenience The `#MP3` is the audio and the clock; the `#VIDEO` is muted background slaved to it, re-seeked whenever it drifts more than 0.25s. That is what the game does, and it matters for the purpose: the times driving the highlight are the same seconds the notes were generated from, so a word that lights up late in the browser is late in the game too. Measured drift across a playthrough stayed at or below 0.04s. ## The echo While this was in progress you hit a song playing multiple copies of its audio at once. I think this is the cause, and it is fixed here. `package` wrote the video with its audio track intact, next to an `#MP3` holding the same recording. Any player that does not mute the video plays both, a few frames apart, which is heard as an echo. Both branches now drop it: the compatible-video path remuxes with `-c copy -an` instead of copying the file, so it still does not re-encode, and the transcode path passes `-an`. I ruled out the other candidate first. `bestvideo*+bestaudio` looks like it could merge two audio streams, but yt-dlp ignores every format with an audio stream after the first unless `--audio-multistreams` is set, and that selector is yt-dlp's own default. It is not the source. Worth confirming on your end, since I could not reproduce your song: `ffprobe '<song>.mp4'` on an affected folder should show an audio stream. If it does, this is it and a rebuild clears it — the fix only applies to songs packaged from now on. If it does not, the echo is something else and I would want the folder listing and which player you were in. The browser player mutes the video element regardless, so it stays correct for folders built before this. ## The endpoint `GET /api/jobs/{id}/media/{audio,video,cover}` serves whatever that song's own `#MP3`/`#VIDEO`/`#COVER` headers name, read through `parse_ultrastar` rather than guessed from extensions — `package` rewrites `#VIDEO` when it transcodes, so the extension is not fixed. A resolved path whose parent is not the song folder is refused, which covers both `../` and absolute headers. Range requests come free from `FileResponse`; without 206 the browser cannot seek, which would defeat the point. ## Also in here, from your later notes CREATE SONG moved out of the form to the top of the queue, always available, replacing NEW SONG INSTEAD. It clears the form and brings it back into view from wherever playback has scrolled to, asking first if you have typed anything. A successful submission now keeps the fields it sent rather than emptying them. The song just submitted is the obvious starting point for the next one, and nothing has to be retyped to correct it. The form drops back to NEW SONG mode so a second submit creates a new song rather than re-rebuilding the old parent, and the queue's CREATE SONG button is now the way to get an empty form. One related fix: `setFormMode` opened the Advanced section based on whether a job was loaded, which would have hidden retained genre/year/language values after a submit. It now keys off whether the fields actually hold anything. ## Testing 17 new endpoint tests: each media kind served, the header-rewritten extension followed, range requests, unknown kinds, missing files, a dropped `#COVER`, a missing song txt, failed and running jobs, and the two traversal cases. I checked the containment guard is load-bearing rather than incidentally passing — with `path.parent != out` removed, both traversal tests fail and the secret file is served. A new `_playback_flaws` frontend checker with six mutations: sync falling back to `timeupdate`'s ~4Hz, scrolling while paused, a left line keeping its highlight, a word staying lit, an unrenderable review leaving the old song loaded, the player not pointed at the packaged audio, the background video unmuted, and the picture never re-synced. Plus mutations for the two form changes. `package` gains a parametrized test that both branches pass `-an`, and the copy-branch test now asserts `-c copy` so the remux cannot quietly become a re-encode. Full suite: 312 passed, 1 skipped. `tests/test_audio_io.py` and `tests/test_separate.py` were not run — they import torch, which is not installable in the environment I had; neither touches this change. Beyond the tests I drove the real page with Playwright against real ffmpeg-generated media and checked the states rather than assuming them: the endpoint's content types and 206 responses, the audio element as the clock with the video muted and its drift, seeking mid-line, playing across a line boundary, click-to-seek on a word, rewinding to silence, the player unloading for a song with no media, and the sticky player still pinned after scrolling to the bottom. Separately for the form changes: the queue button visible in both modes, clearing and refocusing the form from the bottom of the page, and the discard prompt both dismissed and accepted. `docs/images/webui.png` is regenerated and now shows the panel in use. The screenshot script gained an ffmpeg step to give the selected song a playable folder; without ffmpeg on `PATH` it still runs, just without the player. --- ## Follow-up: clearing the page, and redoing a stage **CREATE SONG now clears the whole page.** It emptied the form but left the previous song's review, player, output path and queue selection sitting under it. The cause was worse than it looked: `clearDetail` reached for an element id that no longer exists, so it threw partway through and everything after the first two lines never ran. It now targets the review box, takes the reason to show ("no songs queued yet" is wrong when you have simply deselected), and a `composing` flag stops the next poll selecting the newest song back into the blank page. **Rebuild can now restart from a chosen stage.** Rebuilding a song packaged before the de-echoing found every stage cached and kept the echo, because the planner works off which input fields changed and a change in songtrace itself changes none of them. The rebuild form gains a **Restart from** select — the CLI's `--force-from`, populated from `/api/stages`. It combines with the planner rather than replacing it: an explicit stage may widen what is rerun, never narrow it below what the edit requires. Picking `package` repackages without re-downloading, which is the fix for an affected song. Two things found while verifying this: - A browser submits a textarea with CRLF while the stored lyrics use LF, so **every** rebuild from the page looked like a lyrics edit and redid the alignment. Lyrics are normalised to LF on the way in. This showed up as a rebuild logging `restarting from align (lyrics_text changed)` when nothing had been touched. - `[hidden]` is only `display: none`, which `.field`'s `display: flex` was winning, so the new control rendered in new-song mode even though the property was set. Caught by looking at the screenshot rather than trusting the DOM property — the browser check has been changed to assert real visibility via `offsetParent`. Testing: 9 new endpoint tests for the restart stage (asked-for stage used when nothing changed, planner used when nothing is asked, each one winning when it is the earlier, unknown stage rejected, blank treated as unset, rebuild-from-scratch unchanged), 2 for CRLF, 2 unit tests for `earliest`, and 2 more frontend mutations (the review left behind under a blank form, the poll reselecting into it). Full suite 325 passed, 1 skipped.

dejima-agent added 1 commit 2026-09-25 23:33:32 -07:00

webui: play a finished song against its review b75b313a83

Checking whether the notes line up meant packaging the song, finding it in
the game and starting it there. The review panel now plays the song folder
itself: a transport above the lyrics, the line being sung highlighted, words
lighting up as they are reached, and a playhead sweeping each line against
its notes. Clicking a line or a word plays from there.

Playback is modelled on the game rather than on whatever is convenient: #MP3
is the audio and the clock, #VIDEO is muted background slaved to it, and the
times are the same seconds the notes were generated from. A word that lights
up late here is late in the game.

package now strips the audio track from the video it writes. It is the same
recording as the #MP3, so a player that does not mute the video plays both a
few frames apart, which is heard as an echo. The compatible-video branch
remuxes with -c copy -an rather than copying the file, so it still does not
re-encode. The browser player mutes the video as well, which covers song
folders built before this.

A new /api/jobs/{id}/media/{audio,video,cover} serves what the song's own
#MP3/#VIDEO/#COVER headers name, refusing anything resolving outside the song
folder. Range requests come from FileResponse, so seeking works.

Also, per review: CREATE SONG moves out of the form and into the top of the
queue, where it is always available, and a successful submission keeps the
fields it sent instead of emptying them.

dejima-agent added 1 commit 2026-09-25 23:52:08 -07:00

webui: clear the page for a new song, and let a rebuild redo a stage 38f8d9ff18

Two problems found in review.

CREATE SONG emptied the form but left the previous song's review, player,
output path and queue selection in place, which reads as though the blank
form and that song belong together. It now clears the page down to a blank
sheet, and a `composing` flag stops the next poll selecting the newest song
back into it.

The reason it cleared so little is that clearDetail reached for an element id
that no longer exists, so it threw partway through and left everything after
the first two lines untouched. It now targets the review box and takes the
reason to show, since "no songs queued yet" is wrong when you have simply
deselected.

Separately, rebuild plans from the fields that changed, which cannot see a
change in songtrace itself: rebuilding a song packaged before the video's
audio was stripped found every stage cached and kept the echo. The rebuild
form gains "restart from", the CLI's --force-from, which combines with the
planner rather than replacing it — an explicit stage may widen what is rerun
but never narrow it below what the edit requires.

Also: a browser submits a textarea with CRLF, so every rebuild from the page
looked like a lyrics edit and redid the alignment. Lyrics are normalised to LF
on the way in. And [hidden] is only display:none, which .field's display:flex
was winning, so the new control showed in new-song mode despite the property
being set.

branderson approved these changes 2026-10-06 13:44:19 -07:00

branderson merged commit 613591edb2 into main

2026-10-06 13:44:24 -07:00

branderson referenced this issue from a commit

2026-10-15 14:17:07 -07:00

Merge pull request 'webui: play a finished song against its review' (#10) from review-playback into main

dejima-agent referenced this issue from a commit

2026-10-15 17:44:36 -07:00

Merge pull request 'webui: play a finished song against its review' (#10) from review-playback into main

Sign in to join this conversation.