diff --git a/HANDOFF.md b/HANDOFF.md index 154a66e..8ec77b5 100644 --- a/HANDOFF.md +++ b/HANDOFF.md @@ -1,291 +1,130 @@ -# HANDOFF, 2026-08-12 +# HANDOFF, 2026-08-12 (second session of the day) -Live state is in `NEXT.md`. This file is only what this session did. +Live state is in `NEXT.md`. This file is only what this session did. The previous handoff's content is +now in `JOURNAL.md` and in `decisions/`. ## Asked -Get up to speed from the previous handoff. Then: the user watched `chapter.mp4` for the first time and -read out 19 timestamped defects. Mid-session, what about characters. Then, write the handoff. +Get up to speed from the previous handoff. Then: rebuild the chapter, and what about characters. ## Result -**Somebody finally watched the video.** That single act found more than four sessions of measuring did. -The recorded metrics said `script` 116/116 and "9 named speech lines". Both were true. Both measured the -wrong thing, because the 9 names are the *wrong* name. +Two things landed. The A/V gap is fixed and the rebuilt `chapter.mp4` is in the bucket. The `bbox` +coordinate space is settled with measurements instead of comments, and converted at the boundary. -Two hard numbers came out of it. +One correction to carry forward. **The first fix of the day named the wrong cause.** Commit `1457556` +claimed the xfade offset drift was the shipped 72.7s gap. The rebuild came out byte-identical to the +broken file, which disproved it. Both are real defects. Only the second one shipped. -1. **The chapter is 20% out of sync.** `chapter.mp4` is video 436.39s over audio 363.67s. The narration - ends 72.7 seconds before the picture. The gap accumulates, which is why everything after 2:54 goes - sideways. The 49 clips are clean: video and audio agree to 0.03s and sum to 363.6s. Assembly adds - 72.7s of video and no audio. -2. **Identity binds names to the wrong people, and to people who have no name.** Walking panel 7 against - the art found zero correct bindings out of two, plus the one character who matters left unbound. See - `Panel 7, walked against the art`, which supersedes the earlier reading of this. +## The chapter, rebuilt -Nothing is committed. `worker_render.py` is edited in the working tree. The fix is **not** verified. - -## The user's 19 notes, grouped by cause - -| cause | timestamps | what is wrong | -| --- | --- | --- | -| one row absorbed every identity | 0:20, 1:07, 1:51, 1:59, end | anyone identified comes out `Choi Haeseon` | -| the MC has no name | 0:44, 0:50, 1:02, 1:23, 1:45 | falls back to "the worker", "someone", "she" | -| gender read off the wrongly bound row | 1:45 "she admits", end "as he waves" | `Choi` is `f`, so "he" means a nameless `m` row got the line | -| narration invents facts | 0:43 "results", 2:03, 2:05, 2:15 "long shift" | the verifier checks quotes and names, not invented claims | -| vision reads art-within-art as scene | 1:35 chibi on a monitor as "a man holding a drink", 1:59 "pointing towards the screen" | panel-in-panel and screen content taken as reality | -| a beat carries nothing | 0:35-0:37 | no content worth narrating | -| no parallax, so a still holds | 2:24-2:52, 28s static | `layers` wrote nothing (`caveats/audit-open.md#layers-writes-nothing`) | -| transition quality | 2:52-2:54 slide "too sharp and laggy" | `push` is `slideleft` at 0.4s. Retest after the sync fix | -| A/V drift | everything after 2:54 | the 72.7s gap above | - -## Panel 7, walked against the art - -This is the load-bearing finding of the session. The user pulled up the panel and checked every -detection by eye. **Read this before touching identity.** It contradicts what the earlier sessions -recorded, and it contradicts two theories I floated today before the user corrected them. - -Panel `7c944dd4-e972-42c7-ba60-9f6939548e80_p007`, crop `s3://panels/.../panels/p006.png`, 900x1650. -A wide establishing shot of an office seen through a window. Vision emitted 6 characters. - -| detection | vision said | the art shows | identity assigned | -| --- | --- | --- | --- | -| `person_5` | m, short black, **yellow sweater**, sitting | **Seonho**, foreground, yellow plaid, headphones, back to camera. The character who matters | **nothing** | -| `person_6` | f, short brown, **white shirt**, sitting | the **colleague**, green dress, ponytail. She has **no name** in the story | `Choi Haeseon` at **0.9** | -| `person_2` | m, short brown, green sweater, sitting | a background extra, seated beyond the next window pane | `Lim Seonho` at **0.9** | -| `person_1` | m, short black, suit, standing | **nobody. A window frame** | nothing | -| `person_3` | m, short black, blue sweater, standing | background extra | nothing | -| `person_4` | m, short black, grey sweater, standing | background extra | nothing | - -**Zero of the two bindings are right, and the one character who matters got nothing.** Both wrong binds -carry confidence 0.9. - -Three separate defects stack here. - -**1. The stored bbox coordinate space is wrong.** Consumed as absolute pixels, all six boxes land in the -top third of a 1650px-tall panel, two of them inside the "YEAH!" speech balloon. Divided by 1000 against -the panel's own dimensions, `person_2`, `person_3`, `person_4` and `person_5` fit their subjects tightly. -So the numbers are not pixels. Two places assert that they are: - -- `worker_vision.py:271` prompt text: `pixel bounding box [x1,y1,x2,y2] (top-left, bottom-right corners)` -- `worker_identity.py:91` comment: `vision emits [x1, y1, x2, y2] pixel corners (gemma4's native bbox convention)` - -Everything reading `bbox` is therefore cropping the wrong region. `worker_identity.py:200` embeds -`_crop_bbox(img, ch["bbox"])`, so `Choi Haeseon` at 0.9 was matched on a crop of the speech balloon's -edge and `Lim Seonho` at 0.9 on a crop of empty window frame. Wrong crops are mostly blank white, which -embed alike, which is a plausible mechanism for one row absorbing 25 assignments. - -**Do not treat rescaling as the fix.** I claimed that and the user disproved it in one screenshot. -After scaling, `person_1` still sits on a window frame with nobody in it, and `person_6` is offset, -clipping the woman and running onto the dark frame. `worker_vision.py:38` already calls the box -"coarse, imprecise". Scaling buys roughly-right boxes, not right ones. - -**2. Vision has no concept of extra versus cast.** Four of the six detections are background extras or -nothing at all. They are handed to identity as candidates on equal footing with the two people who -carry the scene. That also means the "113 detected people" figure that framed the whole roadmap was -never the right denominator, so "26 of 113 carry an identity" measured nothing useful. - -**3. Identity mints a name onto a person who has none.** The colleague has no name in the story. She was -labelled `Choi Haeseon` at 0.9. Across the chapter `Choi Haeseon` holds 25 of 26 assignments, so in -practice that row is the label this pipeline stamps on any unnamed woman. This is the direct cause of -the user's 0:20 note, "Choi Haeseon when there's no Choi in the frame, it's the colleague", and of the -gender flips, since gender is read off whichever row got bound. - -This is a cousin of invariant 6 in `CLAUDE.md`, which forbids minting a character from an unparseable -model answer. The missing rule is the same shape: **never attach a name to a detection that carries no -name evidence.** An unnamed recurring person needs a stable anonymous identity so the narration can -call her "the colleague" every time, rather than being forced onto a named row. - -Partly checked, not finished: `match()` in `worker_identity.py:69` does abstain, returning `None` below -threshold, so the 0.9 came from cosine clearing the threshold on a garbage crop. Whether the Tier-2 -gemma resolver can answer "none of these" was not verified. Check that first. - -### Descriptions are not trustworthy either - -`person_6` is "white shirt" for a woman in a green dress. `person_5` is "yellow sweater" for yellow -plaid, which is close enough. Any downstream rule keyed on appearance text inherits this. - -## Registry, measured - -`characters` is keyed by `manga_id`, not chapter. Two manga share the table. `d7104032` has 34 rows with -9 named. This chapter's `ef105a86` has 19 rows with 3 named. The `Kei`, `Zen`, `Kanade`, `Rico` and `K3` -rows belong to the other manga, so they are not polluting this chapter. - -This manga's 19 rows, all `status=confirmed`, all `first_seen_panel=NULL`: - -- named: `Choi Haeseon` (f), `Seonho` (m, aliases `["Lim Seonho","Seonho"]`), `Lim Seonho` (m) -- 16 rows with `name=NULL` and an empty alias list - -`Seonho` and `Lim Seonho` are the same person in two rows. `Seonho`'s alias list contains the other -row's name. That is why either spelling matches two rows and binds nothing. - -Assignments across the whole table, for scale: - -| character | name | assignments | -| --- | --- | --- | -| `character_afa7623b` | Choi Haeseon | 25 | -| `character_bb79cfb4` | Kanade | 25 | -| `character_cfd34340` | Rico | 19 | -| `character_6f491712` | Lim Seonho | 1 | - -In this chapter only `Choi Haeseon` (25) and `Lim Seonho` (1) appear. - -Read those 25 together with panel 7. `Choi Haeseon` is not a character who appears 25 times. It is the -row that absorbs any unnamed woman. The 16 nameless rows are not a backlog of people waiting for names. -Some of them are background extras that should never have become rows, and at least one of them, the -colleague, is a real recurring person who correctly has no name and needs to keep it. - -An earlier draft of this file said "naming is the ceiling, 16 of 19 rows need names". **That was wrong** -and it is corrected here. The ceiling is that identity cannot say "person, no name" and cannot tell an -extra from cast. - -## The A/V bug, located but not fixed - -Reproduced on the 49 real clips with `repro.py` and `probe.py` (see Open). Per-round probe with -`ASSEMBLE_BATCH=8` and 6 `fade_black` boundaries spread across batches: +`s3://video/ef105a86-4b7e-4ac4-b45c-b7d83b8f5b5e/7c944dd4-e972-42c7-ba60-9f6939548e80/chapter.mp4` ``` -r0 g0 n=8 XFADE in v= 49.44 a= 49.44 -> out v= 48.56 a= 48.64 lost_v=+0.88 lost_a=+0.80 -r0 g1 n=8 XFADE in v= 47.04 a= 47.04 -> out v= 46.16 a= 46.23 lost_v=+0.88 lost_a=+0.81 -r0 g2 n=8 XFADE in v= 48.00 a= 48.00 -> out v= 47.12 a= 47.19 lost_v=+0.88 lost_a=+0.81 -r0 g3 n=8 XFADE in v=100.80 a=100.80 -> out v= 99.92 a= 99.96 lost_v=+0.88 lost_a=+0.84 -r0 g4 n=8 concat in v= 64.18 a= 64.16 -> out v= 64.24 a= 64.26 lost_v=-0.06 lost_a=-0.10 -r0 g5 n=8 XFADE in v= 44.00 a= 44.00 -> out v= 43.12 a= 43.17 lost_v=+0.88 lost_a=+0.83 -r1 g0 n=7 XFADE in v=359.29 a=359.60 -> out v= 99.96 a=358.79 lost_v=+259.33 lost_a=+0.81 +before v=436.392 a=363.675 nb_frames=9902 avg_frame_rate=22.69 gap +72.72 +after v=364.120 a=364.122 nb_frames=9101 r_frame_rate=25/1 gap -0.002 ``` -Round 0 is correct. Each group loses only the xfade overlap. **Round 1 loses 259s of video against 0.8s -of audio.** Its output video is 99.96s at 2499 frames, almost exactly the frame count of input `n3` on -its own, the 99.92s intermediate. The final video appears to carry the frames of one input. +Cause: `assemble` sends an all-`cut` chapter down a `concat` demuxer with `-c copy`. That path writes +the output in the **first** input's `time_base` and reinterprets every later packet in it. 14 of the 49 clips +are `30/1` at `1/15360`, the other 35 are `25/1` at `1/12800`, so those 14 play `15360/12800 = 1.2` too +long with their audio untouched. `collage_cmd` hardcoded `-r 30`. +`decisions/chapter-assembly.md#mixed-rate-stream-copy`. -The round-1 filtergraph is arithmetically correct, so this is ffmpeg behaviour, not offset math: +Fixes: `collage_cmd` emits `-r FPS`. `assemble` probes `_fps_of` across the clips and routes mixed rates +through `_assemble_batched`, whose branches both normalize with `fps={FPS}`. + +A second, latent defect on the transition path was fixed and committed separately. `_xfade_chain` took +offsets from `format=duration`, which is `max(video, audio)`. The accumulator crept past the end of its +input, and ffmpeg discarded whole clips at `rc 0` with nothing on stderr. +`decisions/chapter-assembly.md#offsets-from-min-stream`. + +The single-item passthrough theory from the previous handoff is dead, recorded void at +`decisions/chapter-assembly.md#passthrough-innocent`. The one-path rewrite it recommended is not needed. + +**Still open here.** The 14 clips in the bucket are still 30fps. Assembly normalizes them, so the chapter +is correct, but the fast stream-copy path stays off for this chapter until `render` re-runs. Nobody has +watched the rebuilt video yet. The 2:52 slide transition and the 28s static hold from 2:24 were both +supposed to be re-judged after the sync fix. + +## Characters: the `bbox` space, settled + +All 113 detections, straight from `/review/identity`: + +| test | result | +| --- | --- | +| boxes with `x2` past the 900px panel width | **47 of 113** | +| boxes with `y2` past 1000, on panels 1257 to 2307px tall | **0 of 113** | +| boxes clamped at exactly 1000 | 21 in x, 5 in y | +| coordinate range over every box | `[0, 1000]` | + +Gemma's native 0-1000 grid. Not pixels. `worker_vision.py` prompt text and the old +`worker_identity.py:91` comment both claimed pixels and both were wrong. + +`/vision` now calls `_bbox_to_pixels(characters, w, h)` before returning. Four consumers are fixed at +once: `_crop_bbox` in identity, `_pair_faces_to_present`, the set-of-mark boxes, and the review UI's +client-side crop. The pairing one was comparing real pixel face boxes against 0-1000 character boxes, +which is the likely mechanism behind 7 `unknown` out of 7 `som_face` lines. +`decisions/identity-bbox.md#bbox-is-normalized`. + +Checked by eye on panel 7, not just asserted. Five of six converted boxes land on their subject. That +includes `person_5`, who is Seonho in the foreground with headphones and carried no identity. `person_1` +still frames an empty window mullion, which is `caveats/speaker-attribution.md#extras-as-cast`. + +Converted boxes for panel 7, for whoever redraws the overlay: ``` -[n0][n1]xfade=transition=fade:duration=0.050:offset=48.510[v1] -[v1][n2]xfade=transition=fade:duration=0.050:offset=94.620[v2] -[v2][n3]xfade=transition=fadeblack:duration=0.600:offset=141.140[v3] -[v3][n4]xfade=transition=fade:duration=0.050:offset=241.010[v4] -[v4][n5]xfade=transition=fade:duration=0.050:offset=305.200[v5] -[v5][n6]xfade=transition=fade:duration=0.050:offset=348.270[v6] +person_1 [226, 414, 286, 553] window mullion, nobody +person_2 [ 34, 558, 106, 749] background extra, was assigned Lim Seonho +person_3 [428, 384, 494, 533] background extra +person_4 [498, 389, 561, 549] background extra +person_5 [460, 657, 631, 939] Seonho, foreground. was assigned nothing +person_6 [646, 591, 767, 794] the colleague, no name in the story. was assigned Choi Haeseon at 0.9 ``` -### The strongest clue, found last +**The registry is unchanged and still wrong.** Every stored box, embedding and `ref_image_uris` was +enrolled from the wrong space. `vision` and `identity` have to re-run before any of it means anything, +and that is GPU work nobody authorized. `caveats/speaker-attribution.md#bbox-wrong-space` is marked +resolved with the rerun pending. -Re-running that chain by hand over only the **6** round-0 intermediates gives a correct 348.24s at 8708 -frames, `rc 0`, no warnings. Round 1 collapses with **7** inputs, not 6. +## Checks -The 7th input is the leftover 49th clip. With 49 clips and batch 8, round 0 makes 6 groups of 8 and one -group of 1, and `_assemble_batched` passes a lone group through un-encoded: +Every self-check runs from the repo root and passes: -```python -if len(group) == 1 and not final_round: - next_items.append(group[0]) +```bash +.venv/bin/python worker_render.py # about 4 minutes, real ffmpeg +.venv/bin/python worker_vision.py +.venv/bin/python worker_identity.py ``` -So the final xfade mixes 6 encoded intermediates with 1 raw clip. That passthrough is a **third** path -next to `concat` and `xfade`, and it is the prime suspect. Start here tomorrow. Confirm it by running -`probe.py` with 48 clips instead of 49, which removes the leftover entirely. +Three checks were added, because the existing ones passed all the way through both shipped defects: -### What was tried and what it cost +- `_fps_of(collage clip) == "25/1"`, on a real collage encode. This is the one that would have caught the + mixed-rate bug at the source. +- three clips whose audio outlasts their video by 0.4s, assembled through the xfade branch. Mutation + tested by restoring `_audio_dur`: fires with `video=1.80 audio=3.56 expected=3.56`. +- `_bbox_to_pixels` against panel 7's real `person_5` box, asserting the result covers the lower half of a + 1650px panel, which a raw grid value cannot. -Two paths exist in `_assemble_once`. A `concat` branch handles cut-only batches. An `xfade` branch -handles batches with a real transition. They disagreed on frame rate. The concat branch forced `fps=30` -while the xfade branch normalized nothing, and clips are 25fps. `436.39 / 363.63 = 1.2001`, exactly -`30/25`, which is what sent me down this path. +`_check_assembled` now runs after every encode on both paths, because ffmpeg returns 0 while dropping +whole inputs. -Working-tree changes to `worker_render.py`, all uncommitted: +## Next command -- new `FPS = 25` constant. The three hardcoded `25`s and the one `30` now reference it -- the xfade branch normalizes every input with `setsar=1,fps={FPS}` into `[n{i}]` labels, matching what - the concat branch already did -- new `_stream_dur(path, kind)`. `_audio_dur` probes `format=duration`, which is `max(video, audio)`, so - it hides A/V drift by construction -- the `__main__` xfade self-check now assembles 4 clips through `_assemble_batched` with - `ASSEMBLE_BATCH=2` and asserts `abs(video - audio) < 0.25`. It previously asserted only - `getsize(out) > 0`, which is why this shipped +Watch the rebuilt chapter before anything else. That is what found every real defect so far. -**A dead end worth not repeating.** I also pinned `-r FPS` on both output encodes. That made it worse. -The chapter collapsed to exactly 100.00s at 2500 frames, because forcing CFR on irregular input -timestamps drops frames. The comment already sitting at the concat branch warns about this. Both `-r` -flags were removed again. The in-graph `fps=` filter is the right normalization. The output `-r` is not. +```bash +/usr/bin/ssh kami@192.168.1.104 'mc cat homesrv/video/ef105a86-4b7e-4ac4-b45c-b7d83b8f5b5e/7c944dd4-e972-42c7-ba60-9f6939548e80/chapter.mp4' > chapter.mp4 +``` -The fps inconsistency is real and worth keeping fixed. It is **not** proven to be the cause of the -shipped 72.7s gap. The scene graphs hold 356 `cut` against 6 `fade_black`, so round 2 of the real run -most likely stayed on the concat branch, where no mixing occurs. Treat the fps work as necessary and -insufficient. +Then, with a go-ahead, the vision and identity rerun in `NEXT.md` item 1. -### The user's own hypothesis, which is the recommended direction +## Traps confirmed again this session -Two guesses, both worth following: - -1. the assembly is wrong in an ffmpeg sense -2. there are two different paths, and they get mixed when there should only be one - -Guess 2 matches the code. `concat`, `xfade` and the single-item passthrough are three paths, and -`_assemble_batched` feeds the output of one into the input of another. **Collapse it to one path.** -Normalize every input, then xfade every boundary, with `cut` as a 0.05s fade. `acrossfade` shortens -audio by the same amount that xfade shortens video, so A/V stays locked. The chapter then comes out -about 2.4s shorter than the sum of the clips, with both streams agreeing. That removes the branch -interaction instead of tuning it. - -## Measured - -Job `778297bc-e7ce-439d-91b5-8a027060d17f`, chapter `7c944dd4-e972-42c7-ba60-9f6939548e80`. - -- job `status=completed`, every stage at its unit count, finished `2026-08-11T20:08:16Z` -- it still carries `error: "partial: 112/116 completed"` (`caveats/audit-open.md#stale-job-error`) -- `chapter.mp4` 50MiB, video 436.392s, audio 363.675s, `r_frame_rate=25/1`, - `avg_frame_rate=63372800/2792909` which is 22.69, `nb_frames=9902` -- 49 clips, every one `25/1` exactly, sum video 363.63s, sum audio 363.60s. No clip has the two - differing by more than 0.05s -- scene-graph transitions: `cut` 356, `fade_black` 6 -- `worker_render.py` `__main__` self-check passes on the current working tree -- workers up in tmux `manga-workers`, 9 windows - -## Open - -- **Finish the round-1 diagnosis.** The `_assemble_batched` tree turns 359s of video into 100s. This is - the worst defect found and it reproduces offline in about two minutes with no GPU. Suspect the 7th - passthrough input first. - - ```bash - .venv/bin/python /probe.py # the per-round loss table above - .venv/bin/python /repro.py # end-to-end verdict - ``` - - The scratchpad is session-scoped and will be gone. **Re-download the clips first:** - - ```bash - /usr/bin/ssh kami@192.168.1.104 'P=homesrv/video/ef105a86-4b7e-4ac4-b45c-b7d83b8f5b5e/7c944dd4-e972-42c7-ba60-9f6939548e80; mc cp -q -r $P/clips/ /tmp/rclips/; cd /tmp/rclips && tar cf - .' | tar xf - -C clips/ - ``` - - Both scripts are worth committing next session. They are the only check that has ever caught this. - -- **Do not trust `format=duration`.** It returns `max(video, audio)`, so every existing duration assert - in `worker_render.py` is blind to drift. `_stream_dur` exists now. The other asserts still use - `_audio_dur`. -- **Identity, in the order the panel 7 evidence implies.** First, settle the `bbox` coordinate space and - fix every consumer, since nothing else can be judged while crops are wrong. Second, let identity - abstain and hold a stable anonymous identity, so the colleague stays "the colleague". Third, separate - extra from cast so extras never reach identity. Only then merge `Seonho` into `Lim Seonho` and split - `character_afa7623b`, which still needs the reversible-merge design - (`caveats/audit-open.md#destructive-reconcile`). -- **Verify the bbox space before changing anything.** Two independent claims in the code say pixels, and - the art says otherwise. Confirm what the model was told and what it returns, rather than trusting - either comment. Then check whether the crop is the only consumer, or whether SoM marker placement and - the face-pairing in `worker_vision.py:57` read the same numbers. -- Re-derive the panel 7 overlay when needed. It took one `mc cat` of the crop plus a Pillow script, and - it found more than any query did: - - ```bash - /usr/bin/ssh kami@192.168.1.104 'mc cat homesrv/panels/ef105a86-4b7e-4ac4-b45c-b7d83b8f5b5e/7c944dd4-e972-42c7-ba60-9f6939548e80/panels/p006.png' > p007.png - ``` - - Draw each `bbox` twice, once as pixels and once divided by 1000, then look at it. -- `mc` aliases on homesrv: use `homesrv` or `mio`, not `local`, which returns Access Denied. `rfs` is - the empty rustfs. +- `tmux` session `manga-workers` was gone and both systemd units were inactive. `./start_workers.sh` + starts 9 windows. The render worker is window 9 and must be restarted by hand to pick up an edit. - Plain `ssh` is the kitty ssh kitten and refuses non-interactive stdin. Use `/usr/bin/ssh`. -- `cp` is aliased to `cp -i` in this shell and hangs on overwrite. Use `/usr/bin/cp -f`. -- The Bash tool's default timeout is 120s no matter what `timeout` the command itself carries. Pass the - tool's own timeout or background the run. Otherwise a restore step after a mutation test never runs, - which left a deliberately broken `worker_render.py` on disk once this session. +- `cd $dir && .venv/bin/python` fails, because the venv path is relative to the repo root. Use the + absolute interpreter path when the working directory is elsewhere. +- The whole assembly investigation ran offline on 49 downloaded clips with no GPU and no orchestrator. + Re-download with the command in `NEXT.md`. diff --git "a/II, d[i+4:i+12]))\n\"; done" "b/II, d[i+4:i+12]))\n\"; done" new file mode 100644 index 0000000..e69de29 diff --git a/JOURNAL.md b/JOURNAL.md index 286cbff..c197905 100644 --- a/JOURNAL.md +++ b/JOURNAL.md @@ -269,3 +269,40 @@ back: the new check fires with `video=1.80 audio=3.56 expected=3.56`. Not done: `s3://video/.../chapter.mp4` is still the broken 436s file. Rebuilding it means clearing the `assemble` stage and resuming, which is CPU-only and was not run. + +## 2026-08-12, the chapter rebuilt, and the bbox space settled + +**The rebuild came out byte-identical to the broken file.** Clearing `assemble` and resuming produced +video 436.392031s over audio 363.674667s and `nb_frames` 9902 again, which proved the xfade fix committed +earlier today never runs for this chapter. With all-`cut` transitions `assemble` takes the `else` branch, +a `concat` demuxer with `-c copy`. + +Reproduced that path offline in seconds and got the shipped numbers exactly. The cause is mixed frame +rates: 14 of the 49 clips are `r_frame_rate=30/1` at `time_base=1/15360`, the other 35 are `25/1` at +`1/12800`. `-c copy` writes the output in the first input's timebase, so those 14 play `15360/12800 = 1.2` +too long with their audio untouched. `collage_cmd` hardcoded `-r 30`, which yesterday's `FPS` sweep +missed. `decisions/chapter-assembly.md#mixed-rate-stream-copy`. + +Fixed `collage_cmd` to emit `-r FPS`, and made `assemble` probe `r_frame_rate` across the clips and route +mixed rates through the re-encoding tree. Rebuilt: + +``` +before v=436.392 a=363.675 nb_frames=9902 avg_frame_rate=22.69 +after v=364.120 a=364.122 nb_frames=9101 r=25/1 +``` + +The 14 clips in the bucket are still 30fps. Assembly normalizes them, so the chapter is correct without +re-rendering, but the fast stream-copy path stays disabled for this chapter until `render` re-runs. + +**The `bbox` space is 0-1000, not pixels.** Pulled all 113 detections from `/review/identity` and +measured: 47 boxes have `x2` past the 900px panel width, none has `y2` past 1000 on panels 1257 to 2307px +tall, 21 clamp at exactly 1000 in x, and the whole range is `[0, 1000]`. `/vision` now converts to pixels +before returning, so identity crops, gated face pairing, the set-of-mark boxes and the review UI all read +pixels (`decisions/identity-bbox.md#bbox-is-normalized`). + +Checked by eye the way the user did. Drew the converted boxes on panel 7: five of six land on their +subject, including `person_5`, who is Seonho in the foreground with headphones and carried no identity. +`person_1` still frames an empty window mullion, which is the extra-versus-cast caveat, not this one. + +Not done: `vision` and `identity` have not re-run, so every box, embedding and `ref_image_uris` in the +registry is still from the wrong space. That rerun is GPU work and was not started. diff --git a/NEXT.md b/NEXT.md index f9ae0d6..7ff5af1 100644 --- a/NEXT.md +++ b/NEXT.md @@ -4,44 +4,41 @@ Updated 2026-08-12. What this session did is in `HANDOFF.md`. ## State -The chapter runs end to end and the output is **not watchable**. That is now measured, not guessed. -Assembly is fixed and verified offline. The shipped `chapter.mp4` has not been rebuilt yet. +The chapter runs end to end. The A/V sync defect is fixed and `chapter.mp4` is rebuilt: video 364.120s +against audio 364.122s at `25/1`. The identity defects are still in the output. Job `778297bc-e7ce-439d-91b5-8a027060d17f`, chapter `7c944dd4-e972-42c7-ba60-9f6939548e80`, 116 panels, `status=completed`, finished 2026-08-11T20:08:16Z. `s3://video/` holds 49 clips and a 50MiB `chapter.mp4`. The user watched it and read out 19 defects. They are grouped by cause in `HANDOFF.md`. -Two numbers set the agenda: +One number sets the agenda: -- The shipped `chapter.mp4` is video 436.39s over audio 363.67s. Cause found and fixed, see below. - Panel 7 checked against the art has **zero correct identity bindings** out of two, and the one - character who matters is unbound. `HANDOFF.md#panel-7-walked-against-the-art` has the table. + character who matters is unbound. `HANDOFF.md#panel-7-walked-against-the-art` has the table. The + coordinate cause is fixed. The registry built on it is not. ## Next -1. **Rebuild `chapter.mp4`.** Assembly is fixed in `worker_render.py`. Verified over the 49 real clips of - this chapter: video 358.76s against audio 358.76s, agreeing to the frame. The cause was `_xfade_chain` - taking offsets from `format=duration`, which is `max(video, audio)`. The accumulator drifted past the - end of its input, and ffmpeg silently discarded whole clips at `rc 0` - (`decisions/chapter-assembly.md#offsets-from-min-stream`). The single-item passthrough was innocent - and the one-path rewrite is not needed (`decisions/chapter-assembly.md#passthrough-innocent`). +1. **Re-run vision and identity.** The `bbox` space is settled and converted at `/vision` + (`decisions/identity-bbox.md#bbox-is-normalized`). Every stored box, embedding and `ref_image_uris` in + the registry came from the wrong space. The fix changes nothing until those stages run again. + This is GPU work and needs the user's go-ahead. Clear `vision` and everything downstream of it, or + accept that the boxes in the database stay normalized while new ones are pixels. - What is left is to clear the `assemble` stage and resume, then watch the result. That is CPU-only - ffmpeg, no GPU, but it needs the user's go-ahead. + Watch two things on the rerun. Whether `som_face` still returns `unknown` on every face, since gated + pairing was comparing pixel face boxes against 0-1000 character boxes. And whether `Choi Haeseon` still + absorbs every unnamed woman, which is item (b) below and independent of the crops. - Smaller follow-on: nine other `_audio_dur` calls in `worker_render.py` measure finished clips with + Smaller follow-on: nine `_audio_dur` calls in `worker_render.py` measure finished clips with `format=duration`. So the durations reported to the orchestrator are blind to per-clip drift. They position no filter, so invariant 9 does not cover them. Worth converting to `_stream_dur`. 2. **Fix identity, in this order.** Panel 7 is the worked example and `HANDOFF.md#panel-7-walked-against-the-art` carries the evidence. Do not start at the registry. - a. **Settle the `bbox` coordinate space.** Consumed as pixels, all six boxes on panel 7 land in the - top third of the panel, two inside a speech balloon. Divided by 1000 they mostly land on their - subjects. `worker_vision.py:271` and `worker_identity.py:91` both assert pixels, and the art says - otherwise. Identity embeds `_crop_bbox(img, ch["bbox"])` at `worker_identity.py:200`, so today it - matches faces against crops of balloons and window frames. Nothing downstream can be judged until - this is right. Rescaling alone is **not** the fix: after scaling, one box still sits on an empty - window frame and another clips its subject. + a. ~~Settle the `bbox` coordinate space.~~ **Done 2026-08-12**, proven over all 113 detections and + checked by eye on panel 7, where five of six converted boxes land on their subject + (`decisions/identity-bbox.md#bbox-is-normalized`). `person_1` still frames an empty window mullion, + which is (c). b. **Let identity abstain and stay abstained.** The colleague has no name in the story and was labelled `Choi Haeseon` at 0.9. An unnamed recurring person needs a stable anonymous identity so narration says "the colleague" every time. `match()` already returns `None` below threshold. Check diff --git a/caveats/CLAUDE.md b/caveats/CLAUDE.md index 6836615..16d6e6d 100644 --- a/caveats/CLAUDE.md +++ b/caveats/CLAUDE.md @@ -38,7 +38,7 @@ a complaint, so give it one or drop it. | [One invented word still halts the chapter](speaker-attribution.md#one-word-halts-chapter) | 2026-08-11 run | | [A completed job keeps the error from an earlier failure](audit-open.md#stale-job-error) | 2026-08-11 run | | [`layers` reports success on an empty bucket](audit-open.md#layers-writes-nothing) | 2026-08-11 run | -| [Every `bbox` is read in the wrong coordinate space](speaker-attribution.md#bbox-wrong-space) | 2026-08-12 panel 7 | +| [Every `bbox` is read in the wrong coordinate space](speaker-attribution.md#bbox-wrong-space) | resolved, rerun pending | | [Identity cannot say "a person with no name"](speaker-attribution.md#no-anonymous-identity) | 2026-08-12 panel 7 | | [Vision does not separate a background extra from cast](speaker-attribution.md#extras-as-cast) | 2026-08-12 panel 7 | | [Cast reference profiles are enrolled from wrong crops](speaker-attribution.md#poisoned-reference-set) | 2026-08-12 panel 7 | diff --git a/caveats/speaker-attribution.md b/caveats/speaker-attribution.md index 8709925..ec6d824 100644 --- a/caveats/speaker-attribution.md +++ b/caveats/speaker-attribution.md @@ -72,6 +72,13 @@ is to flag the beat for review and continue, which is `#136` gate work, not a ve ## Every `bbox` is read in the wrong coordinate space {#bbox-wrong-space} +**Resolved 2026-08-12, `decisions/identity-bbox.md#bbox-is-normalized`.** The space is gemma's 0-1000 +grid, proven over all 113 detections, and `/vision` now converts to pixels before returning. The face +pairing at `worker_vision.py:57` was reading the same numbers against real pixel face boxes, so it is +fixed by the same change. What is left of this entry is the consequence. Every stored assignment, +embedding and `ref_image_uris` came from a wrong crop. Identity has to re-run before any of it means +anything. The rest below is kept as the record of how it read before. + Vision's `bbox` values are stored and consumed as absolute pixels. On panel `7c944dd4-e972-42c7-ba60-9f6939548e80_p007` (crop 900x1650) all six boxes then land in the top third of the panel, two of them inside the "YEAH!" speech balloon. Divided by 1000 against the panel's own diff --git a/decisions/CLAUDE.md b/decisions/CLAUDE.md index 021bc8a..4542463 100644 --- a/decisions/CLAUDE.md +++ b/decisions/CLAUDE.md @@ -36,6 +36,8 @@ still live belongs in `caveats/`. | [An interjection is not a name and not a misquote](speaker-attribution.md#interjection-false-positive) | closed | | [Cast names enter the verifier tokenized](speaker-attribution.md#multiword-cast-names) | closed | | [Clearing a stage strips the vision blob it wrote](storage-layout.md#clear-vision-blob) | closed | +| [The shipped 72.7s gap was a stream copy across mixed frame rates](chapter-assembly.md#mixed-rate-stream-copy) | closed | | [xfade offsets come from `min(video, audio)`, never `format=duration`](chapter-assembly.md#offsets-from-min-stream) | closed | +| [A vision `bbox` is gemma's 0-1000 grid, converted to pixels at `/vision`](identity-bbox.md#bbox-is-normalized) | closed | | [Assembly verifies its own output instead of trusting ffmpeg's exit code](chapter-assembly.md#check-assembled) | closed | | [The single-item passthrough is not the assembly bug](chapter-assembly.md#passthrough-innocent) | void | diff --git a/decisions/chapter-assembly.md b/decisions/chapter-assembly.md index d0d81a9..aa9d64f 100644 --- a/decisions/chapter-assembly.md +++ b/decisions/chapter-assembly.md @@ -2,10 +2,40 @@ Settled questions about `_assemble_batched` / `_assemble_once` / `_xfade_chain` in `worker_render.py`. -## xfade offsets are computed from `min(video, audio)`, never `format=duration` {#offsets-from-min-stream} +## The shipped 72.7s gap was a stream copy across mixed frame rates {#mixed-rate-stream-copy} **Closed, 2026-08-12.** +`assemble` routes an all-`cut` chapter to a `concat` demuxer with `-c copy`. That path writes the output +with the **first** input's `time_base` and reinterprets every later packet in it. + +14 of this chapter's 49 clips came off `collage_cmd`, which hardcoded `-r 30`. They carry +`r_frame_rate=30/1` and `time_base=1/15360`. The other 35 are `25/1` at `1/12800`. Copied into the first +clip's timebase, those 14 play `15360/12800 = 1.2` times too long while their audio is untouched. That is +the 1.2001 ratio, and the whole of video 436.39s over audio 363.67s. + +Reproduced offline by running the same `-c copy` concat over the 49 real clips. Duration 436.392031 and +`nb_frames` 9902, identical to the shipped file. Seconds to run, no GPU. + +Two changes hold it closed: + +* `collage_cmd` emits `-r FPS` like every other clip path, and the `__main__` self-check asserts + `_fps_of(clip) == "25/1"` on a real collage encode. +* `assemble` probes `r_frame_rate` across the clips and sends mixed rates through `_assemble_batched`, + whose branches both normalize with `fps={FPS}`. Only a single shared rate keeps the stream copy. + +Verified end to end. The rebuilt `chapter.mp4` is video 364.120s against audio 364.122s at `25/1`. + +**An earlier version of this file, and commit `1457556`, blamed `#offsets-from-min-stream` below for the +shipped gap. That was wrong.** The rebuild came out byte-identical to the broken file, which proved the +xfade tree never ran for this chapter. The entry below is a real defect and stays closed on its own +evidence. It was not this one. + +## xfade offsets are computed from `min(video, audio)`, never `format=duration` {#offsets-from-min-stream} + +**Closed, 2026-08-12.** A real latent defect on the transition path. Not the cause of the shipped gap, +see `#mixed-rate-stream-copy` above. + `_xfade_chain` accumulates `cum += dur[i] - td` and hands each boundary `offset=cum-td`. That offset is an assertion about where input `i-1` still has frames. It fed on `_audio_dur`, which probes `format=duration`, which is `max(video, audio)`. A rendered clip's audio outlasts its video by about a diff --git a/decisions/identity-bbox.md b/decisions/identity-bbox.md new file mode 100644 index 0000000..0c637e1 --- /dev/null +++ b/decisions/identity-bbox.md @@ -0,0 +1,45 @@ +# identity-bbox + +The coordinate space of a vision character box, and what reads it. + +## A vision `bbox` arrives on gemma's 0-1000 grid, and `/vision` converts it to pixels {#bbox-is-normalized} + +**Closed, 2026-08-12.** + +`build_detect_prompt` asks for a "pixel bounding box". The model answers on its own normalized grid +regardless. Measured over all 113 detections of job `778297bc`, read straight from `/review/identity`: + +| test | result | +| --- | --- | +| boxes with `x2` past the 900px panel width | **47 of 113** | +| boxes with `y2` past 1000, on panels 1257 to 2307px tall | **0 of 113** | +| boxes clamped at exactly 1000 | 21 in x, 5 in y | +| coordinate range over every box | `[0, 1000]` | + +Pixels cannot behave that way. A person standing in the lower half of a 2307px panel needs `y2` near +2000, and it never once exceeds 1000. + +Consumed as pixels the boxes collapse into the top-left corner of the panel. Four consumers were reading +them: + +* `worker_identity._crop_bbox` at `worker_identity.py:200`, which embeds the crop. This is why a crop of + a speech balloon's edge matched `Choi Haeseon` at 0.9. +* `_pair_faces_to_present` in `worker_vision.py`, which compares real detector face boxes, in pixels, + against these. The gate could almost never pass, which is the mechanism behind the 7 `unknown` results + out of 7 `som_face` lines already recorded at `worker_vision.py:169`. +* the set-of-mark boxes drawn for attribution. +* the review UI, which crops client-side off the panel PNG. + +`/vision` now calls `_bbox_to_pixels(characters, w, h)` before returning, so all four see pixels and no +consumer needs to know the grid existed. Verified by drawing the converted boxes on panel 7. Five of six +land on their subject, including `person_5`, who is Seonho in the foreground and had no identity. +`person_1` still frames a window mullion with nobody in it, which is `#extras-as-cast`, not this. + +The prompt text still says "pixel bounding box". Rewording it changes what the model emits and needs a +GPU run to re-verify, so the boundary converts instead. The `ponytail:` note on `_bbox_to_pixels` records +that. It also records the trap: a model that really answered in pixels would be scaled down here. + +**Consequence: every assignment in the registry came from a wrong crop.** The existing embeddings and +`ref_image_uris` are enrolled on balloons and window frames. Re-running identity +is what makes the registry mean anything. The anonymous-identity and extra-versus-cast work cannot be +judged until that rerun happens. diff --git a/worker_identity.py b/worker_identity.py index ad4038c..18677bb 100644 --- a/worker_identity.py +++ b/worker_identity.py @@ -88,7 +88,10 @@ def shortlist(emb: np.ndarray, known: list, k: int = 5, gender: str = None) -> l def _crop_bbox(img, bbox): - # vision emits [x1, y1, x2, y2] pixel corners (gemma4's native bbox convention). + # [x1, y1, x2, y2] pixel corners. gemma answers on a 0-1000 normalized grid and `/vision` converts + # to pixels before returning (`worker_vision._bbox_to_pixels`), so this reads real pixels. It did + # not before 2026-08-12, which is why crops landed on balloons + # (`decisions/identity-bbox.md#bbox-is-normalized`). x1, y1, x2, y2 = bbox return img[y1:y2, x1:x2] diff --git a/worker_render.py b/worker_render.py index 5875124..5a0f89c 100644 --- a/worker_render.py +++ b/worker_render.py @@ -127,6 +127,15 @@ def _stream_dur(path: str, kind: str) -> float: return 0.0 +def _fps_of(path: str) -> str: + """`r_frame_rate` as ffprobe reports it. Compared as a string on purpose: two clips agree only when + their rate AND therefore their time_base agree, and the stream-copy concat path cares about that.""" + r = subprocess.run(["ffprobe", "-v", "error", "-select_streams", "v:0", + "-show_entries", "stream=r_frame_rate", "-of", "default=nk=1:nw=1", path], + capture_output=True, text=True) + return r.stdout.strip() + + ZMAX, ZPAN = 1.15, 1.18 # ken-burns zoom ceiling; constant zoom that gives pans room to travel # Every clip and every assembly stage MUST agree on this. xfade does not resample: it reinterprets the @@ -614,7 +623,7 @@ def collage_cmd(imgs, plate_i, rects, entrances, z_order, audio, ass, out, D, tr cur = f"o{k}" fc = ";".join(parts) + f";[{cur}]ass={ass}[v];[{n}:a]apad=pad_dur={PAD_S:.3f}[a]" cmd += ["-filter_complex", fc, "-map", "[v]", "-map", "[a]", "-t", f"{T:.3f}", - "-r", "30", "-c:v", "libx264", "-pix_fmt", "yuv420p", "-c:a", "aac", "-b:a", "192k", out] + "-r", str(FPS), "-c:v", "libx264", "-pix_fmt", "yuv420p", "-c:a", "aac", "-b:a", "192k", out] return cmd @@ -863,15 +872,26 @@ async def assemble(data: AssembleInput): cleanup = list(locals_) + [out] fancy = len(locals_) >= 2 and any(t not in ("", "cut") for t in data.transitions) - if fancy: + # The stream-copy path writes the output with the FIRST input's time_base and reinterprets every + # later packet in it. A clip encoded at 30fps (time_base 1/15360) copied into a 25fps container + # (1/12800) therefore plays 15360/12800 = 1.2x too long with its audio untouched. That is the whole + # of the shipped chapter's 436.39s of video over 363.67s of narration: 14 of 49 clips came off the + # collage path, which hardcoded `-r 30`. Mixed rates must re-encode, so they go through the tree, + # whose branches both normalize with `fps={FPS}`. + rates = {_fps_of(p) for p in locals_} + if fancy or len(rates) > 1: + if len(rates) > 1: + print(f"[render] mixed clip rates {sorted(rates)}, re-encoding instead of stream copy", + flush=True) _assemble_batched(locals_, data.transitions, out, tag, cleanup) else: - # all hard cuts: stream-copy concat (no re-encode) -- unchanged fast path. + # all hard cuts at one shared rate: stream-copy concat, no re-encode. listfile = f"{SHM}/asm_{tag}.txt"; cleanup.append(listfile) with open(listfile, "w") as f: f.write("".join(f"file '{p}'\n" for p in locals_)) subprocess.run(["ffmpeg", "-y", "-f", "concat", "-safe", "0", "-i", listfile, "-c", "copy", out], check=True, capture_output=True) + _check_assembled(out, sum(min(_stream_dur(p, "v"), _stream_dur(p, "a")) for p in locals_)) out = _add_music_bed(out, tag, cleanup) @@ -1087,6 +1107,11 @@ if __name__ == "__main__": check=True, capture_output=True) assert abs(_audio_dur(out) - (1.5 + PAD_S)) < 0.2, _audio_dur(out) # collage clip = beat length assert os.path.getsize(out) > 0 + # ...and it must come out at FPS like every other clip path. This one hardcoded `-r 30`, so 14 of + # 49 clips in the shipped chapter were 30fps. `concat -c copy` writes the output with the FIRST + # clip's time_base and reinterprets later packets in it, so those 14 played 1.2x too long with + # their narration untouched. That, not the xfade tree, is where the 72.7s gap came from. + assert _fps_of(out) == f"{FPS}/1", f"collage clip is {_fps_of(out)}, not {FPS}/1" for p in (b0, b1, b2, bnar): os.remove(p) os.remove(aud); os.remove(out) diff --git a/worker_vision.py b/worker_vision.py index b1b1133..5e076c6 100644 --- a/worker_vision.py +++ b/worker_vision.py @@ -300,6 +300,46 @@ class VisionInput(BaseModel): session_id: str = "" +BBOX_GRID = 1000 # gemma's native normalized box grid + + +def _bbox_to_pixels(chars: list, w: int, h: int) -> list: + """Rewrite every character box from gemma's 0-1000 grid to pixels on this panel. + + The prompt asks for pixels. The model answers on its own normalized grid regardless. Measured over + the 113 detections of job 778297bc: 47 boxes had x2 beyond the 900px panel width, and not one had y2 + beyond 1000 on panels 1257 to 2307px tall. Consumed as pixels the boxes collapse into the top-left + corner of the panel, which is how identity came to embed crops of speech balloons and window frames + and match them at 0.9, and why gated face pairing returned 7 unknowns out of 7 real faces. + + Convert once here so every consumer sees pixels: `_crop_bbox` in identity, the face pairing below, + the set-of-mark boxes, and the review UI's client-side crop. + + ponytail: the prompt still says "pixel bounding box". Rewording it would change what the model + emits and needs a GPU run to re-verify, so the boundary converts instead. If a future model really + does answer in pixels, this scales them down -- check the box range before swapping models. + """ + for c in chars: + b = c.get("bbox") + if not (isinstance(b, list) and len(b) == 4 and all(isinstance(v, (int, float)) for v in b)): + continue + c["bbox"] = [min(w, max(0, round(b[0] * w / BBOX_GRID))), + min(h, max(0, round(b[1] * h / BBOX_GRID))), + min(w, max(0, round(b[2] * w / BBOX_GRID))), + min(h, max(0, round(b[3] * h / BBOX_GRID)))] + return chars + + +def _panel_size(path: str) -> tuple: + """(width, height) of a panel image, or (0, 0) when it cannot be read.""" + import cv2 + img = cv2.imread(path) + if img is None: + return (0, 0) + h, w = img.shape[:2] + return (w, h) + + @app.post("/vision") async def vision(data: VisionInput): local = transport.get(data.panel_uri, f"{SHM}/vision_{uuid.uuid4().hex[:8]}.png") @@ -315,8 +355,14 @@ async def vision(data: VisionInput): print(f"[vision/detect] parse failed for {data.panel_id} after repair retry: {e}", flush=True) result = {"skip": False, "parse_failed": True, "characters": [], "scene": {}} finally: + pw, ph = _panel_size(local) os.remove(local) result.setdefault("characters", []) + if pw and ph: + _bbox_to_pixels(result["characters"], pw, ph) + else: + print(f"[vision/detect] panel size unreadable for {data.panel_id}, boxes left normalized", + flush=True) result["panel_id"] = data.panel_id return result @@ -1128,6 +1174,19 @@ if __name__ == "__main__": # trailing braced prose after a complete object parses (used to burn a repair call) assert _extract_json('{"skip":false}\nnote {see above}')["skip"] is False + # gemma's boxes arrive on a 0-1000 grid and leave /vision as pixels. The 900x1650 panel below is + # panel 7 of job 778297bc: person_5 is Seonho in the foreground, and read as pixels his box lands in + # the top sixth of the panel, inside a speech balloon, which is what identity embedded. + _ch = [{"local_id": "person_5", "bbox": [222, 405, 654, 1000]}, + {"local_id": "edge", "bbox": [0, 0, 1000, 1000]}, + {"local_id": "junk", "bbox": "nope"}] + _bbox_to_pixels(_ch, 900, 1650) + assert _ch[0]["bbox"] == [200, 668, 589, 1650], _ch[0]["bbox"] + assert _ch[1]["bbox"] == [0, 0, 900, 1650], _ch[1]["bbox"] # a clamped box spans the whole panel + assert _ch[2]["bbox"] == "nope", _ch[2]["bbox"] # unparseable is left alone, not crashed + # the box must now cover the lower half of a tall panel, which the raw grid value never can + assert _ch[0]["bbox"][3] > 1000 > _ch[0]["bbox"][1] + # face->identity pairing is GATED on containment: a face outside every gemma bbox stays unknown. faces = [{"bbox": [10, 10, 30, 30]}, {"bbox": [900, 900, 920, 920]}] present = [{"local_id": "person_1", "name": "Teto", "bbox": [0, 0, 100, 200]}]