drwily/Dragon-s-Lair-X68k
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Handoff: rate control is next, and it is unsound as written

Browse Source 
Session 5 handoff. The user has chosen rate control as the next session's work,
so this reads ratectl.py properly before that session starts rather than
discovering the problem mid-implementation.

FINDINGS 26: encode_rate_controlled() is not sound. H.encode() is temporally
recursive -- SKIP blocks copy the previous RECONSTRUCTION -- but rate control
builds a ladder of independent whole-sequence encodes and picks each frame from
whichever rung fits the budget. Frames then reference reconstructions the
decoder never saw. Measured on the Singe window: 67 rung switches, 111 of 120
frames drift, worst frame 43.4% of pixels, reported PSNR overstated by 0.36 dB.
It would have wired up cleanly and reported a plausible wrong answer.

Two further defects in the same function: the lam ladder runs to 2e5, 250x past
the FINDINGS 15 cliff, so a frame that only fits up there is destroyed rather
than rate-controlled; and with 5 rungs only two are ever chosen, straddling the
operating point by 7.5x. The docstring describes a per-frame binary search,
which is the right design -- the implementation is a fixed ladder. The leaky
bucket does work and should be kept: 109.1 KB/s against a 110 target.

tools/analysis/09_ratectl_drift.py is the regression test and the acceptance
criterion: it exits non-zero until zero frames drift.

Also corrected the stale 38% blit figure in ratectl.py's profile commentary,
which session 5 measured at 53.6% (FINDINGS 24), and recorded the pgrep -f
self-kill trap again -- four times across three sessions now.

check.sh ALL GREEN.

Claude-Session: https://claude.ai/code/session_01194oWYW8DQXK1SZ2DnChW6
This commit is contained in:
prosolis
2026-08-23 14:10:07 -07:00
parent e00264a058
commit 145753c0bf
5 changed files with 218 additions and 16 deletions
Download Patch File Download Diff File Expand all files Collapse all files
docs/FINDINGS.md
+70
View File
@@ -1003,3 +1003,73 @@ One 10 s window of one stream, at fixed lam, with `_paint` still a Python loop.
The full-disc survey is still not done, and the numbers above are the *worst*
window rather than a distribution over content. What has changed is that the
worst case is now a measurement rather than a worry.
---
## 26. Rate control is unsound as written — found before wiring it up (session 5)
FINDINGS 25.3 promoted rate control from insurance to a requirement. Reading
`ratectl.py` before wiring it into `encode.py` turned up a correctness bug that
would have produced exactly the kind of plausible-looking wrong result this
project keeps catching (FINDINGS 4, 9, 14, 18).
### 26.1 The lam ladder desynchronises the encoder from the decoder
`H.encode()` is **temporally recursive**: SKIP blocks are copied from the
previous *reconstruction*, and `prev = out` closes the loop
(`vq_hybrid.py:84-109`). A frame's output therefore depends on every frame
before it in that same run.
`encode_rate_controlled()` runs `H.encode()` once per lam over the **whole
sequence**, building a ladder of independent temporal chains, then picks each
frame from whichever rung fits the budget. When frame *f* comes from rung *i*
and frame *f-1* was emitted from rung *j != i*, the SKIP blocks in *f* reference
a reconstruction **the decoder never saw**.
Measured on the Singe window (`tools/analysis/09_ratectl_drift.py`, 120 frames,
5 rungs, target 110 KB/s):
| | |
|---|---|
| rung switches | **67** over 120 frames |
| frames whose emitted output differs from what the encoder recorded | **111 / 120** |
| worst frame | **21,339 px = 43.4% of the frame** |
| encoder-vs-decoder agreement, worst frame | 27.1 dB |
| reported PSNR overstatement | **0.36 dB** |
The 0.36 dB is the least interesting number here. The encoder is reporting
quality for a reconstruction that will never exist, and 43% of a frame differing
is a visible artefact whatever the mean says.
**The fix is structural, not a tuning change:** `H.encode()` must become
frame-drivable — take `prev` and one lam, return one frame — so rate control can
feed back the frame it actually emitted. The current whole-sequence signature is
what makes the ladder tempting in the first place.
### 26.2 The ladder spans 250x past the shippable range
`lam_hi=2e5`, but FINDINGS 15 puts the quality cliff between lam=800 and
lam=2000 and says do not ship past lam~800. Every rung above ~800 is
unshippable, so a frame that only fits at lam=9457 has not been rate-controlled,
it has been destroyed. Cap `lam_hi` at 800 and let a frame that cannot fit
overrun the bucket — a visible overrun is a better failure than silent garbage.
### 26.3 The ladder is far too coarse where it matters
With `steps=5` the geomspace lands on 1 / 21 / 447 / 9457 / 200000, and **only
two rungs were ever chosen**. The budget is 8,721 B/frame; the two straddling
rungs deliver 23,183 B (lam=21) and 3,071 B (lam=447) — a **7.5x** gap across
the operating point. Rate control cannot land near a target it has to jump over.
The module docstring already describes the right approach — *"per frame we
binary-search lam to land inside a byte budget"* — but the implementation is a
fixed precomputed ladder. Doc and code disagree; the doc is correct.
### 26.4 What does work
The leaky bucket lands the mean where it should: **109.1 KB/s against a 110
target**, with 32% of frames over the per-frame budget and banked by the bucket.
That mechanism is sound and worth keeping. It is the per-frame lam *selection*
underneath it that needs rebuilding, not the bucket.
### 26.5 Cost note before starting
Each rung is a full-sequence encode and `_paint` is still a Python per-block
loop, so a 5-rung run over 120 frames takes minutes. **Vectorise `_paint`
first** — it is already on the list for the full-disc survey and it makes the
rate-control work practical rather than merely faster.
docs/STATUS.md
+53 -9
View File
@@ -1,5 +1,44 @@
# Status & next-session handoff — end of session 5 (2026-08-23)
## NEXT SESSION: wire rate control into `encode.py`
Decided with the user at the end of session 5. Everything needed is below; read
**FINDINGS 26** in full before editing `ratectl.py`, because the module does not
work the way its docstring says it does.
**Why it is now top of the list.** FINDINGS 25.3: on the worst sustained window
on the disc, the fixed-`lam` CLI overshoots both shipping targets — `sasi`
110 -> 129.6 KB/s (+18%), `scsi` 280 -> 373.8 KB/s (+34%). This item sat at
priority 4 marked "insurance, not a fix" for three sessions; that was true of the
1.2-1.7 s clips it was judged on and is not true of a sustained action sequence.
**Do NOT just call `encode_rate_controlled()` from `encode.py`.** It is unsound
as written (FINDINGS 26.1), and it fails quietly — it returns a plausible PSNR
for a reconstruction no decoder will ever produce.
Order of work:
1. **Vectorise `_paint`** (`vq_hybrid.py:122`, a Python per-block loop). Not
cosmetic: rate control runs the encoder once per lam rung, so everything
below is minutes-per-experiment until this is done. FINDINGS 26.5.
2. **Make `H.encode()` frame-drivable** — take `prev` and one lam, return one
frame. The current whole-sequence signature is *why* the broken ladder
exists. This is the actual fix for 26.1.
3. **Replace the fixed ladder with the per-frame binary search** the docstring
already promises, feeding back the frame actually emitted. Cap `lam_hi` at
**800**, not 2e5 — past the FINDINGS 15 cliff a frame is not rate-controlled,
it is destroyed (26.2). Keep the leaky bucket; it works (26.4).
4. **Gate on the regression test**: `python3 tools/analysis/09_ratectl_drift.py`
exits non-zero while the desync is present and must report **zero** drifting
frames after the fix. It currently reports 111/120. Needs `tmp/fr_singe`.
5. Then re-measure the Singe window at both profiles and confirm they land on
target rather than 18%/34% over.
Only after that is the full-disc survey worth running — otherwise it measures an
encoder nobody will ship.
---
## Start here: is the tree still green?
```
@@ -112,7 +151,12 @@ rate-distortion curve, not two codecs.
00216 is the feature with a burned-in commentary PiP; 00215 is the commentary
itself. **00223 (9.4 min) is the clean one.** A size-ranked survey would have
encoded live action. FINDINGS 25.1.
7. **On hard content the scene palette, not the display, is the binding
7. **Rate control is unsound as written, caught before wiring it up.** The
lam-ladder in `ratectl.py` picks frames from independent temporal chains,
so SKIP blocks reference reconstructions the decoder never saw: 111 of 120
frames drift, worst frame 43.4%, reported PSNR overstated 0.36 dB. Regression
test `tools/analysis/09_ratectl_drift.py`. FINDINGS 26.
8. **On hard content the scene palette, not the display, is the binding
ceiling** — 31.33 dB on the Singe window against 39.90 dB on 00020 and 40.81
dB for the X68000 display. `scsi` is already within 0.51 dB of it.
FINDINGS 25.4.
@@ -218,6 +262,10 @@ notifier subscription in a global; the stack register is `SP` not `A7`;
- piping MAME (or any long job) through `grep` block-buffers — write to a file.
- `pkill -f <pattern>` matches your own shell and kills it (exit 144).
Use `pkill -x` or kill by PID.
- **`pgrep -f <name> | xargs kill` kills your own shell too — exit 144.** Same
root cause as the `pkill -f` trap above: the shell's own command line contains
the pattern. **Hit again in session 5**, which makes it four times across three
sessions. Kill by PID captured at launch (`$!`), or use `pkill -x`.
- **`until ! pgrep -f foo.py; do sleep; done` watcher loops never exit.** The
watching shell's own command line contains the string `foo.py`, so `pgrep -f`
matches the watcher itself and the loop spins forever. Session 2 left 11 of
@@ -323,14 +371,10 @@ SDL_VIDEODRIVER=dummy mame x68000 -bios ipl10 -video soft -window \
scene-cut frame is ~100% non-SKIP and a held frame near 0%, and the mean of
those two is a number describing no actual frame.
1b. **Wire rate control into `encode.py`. NOW REQUIRED (was priority 4).**
FINDINGS 25.3: on the worst sustained window both profiles overshoot their
targets with the fixed `lam` the CLI uses — `sasi` by 18%, `scsi` by 34%.
The note that used to sit here, "no longer a blocker (FINDINGS 21)", was true
of the 1.2-1.7 s clips measured at the time and is not true of this one.
`ratectl.encode_rate_controlled()` already builds a per-frame lam ladder; it
has never been hooked up. Do this before the full-disc survey or the survey
measures an encoder nobody will ship.
1b. **Wire rate control into `encode.py`. NOW REQUIRED, and it is the agreed
next session's work** — see the **NEXT SESSION** block at the top of this
file for the ordered plan, and FINDINGS 26 for why
`encode_rate_controlled()` cannot simply be called as it stands.
2. **68000 decoder skeleton**, with the inner loop chosen by (1). Parse `DLX1`,
expand codebooks, blit per block mode. The display path is verified *by 68000