diff --git a/bad-apple/README.md b/bad-apple/README.md index a3ab315..9add77b 100644 --- a/bad-apple/README.md +++ b/bad-apple/README.md @@ -49,7 +49,8 @@ The 1987 TI TMS34010 SDK runs under DOSBox. The C sources are K&R C: the ```sh # one frame, then dump VRAM at MAME frame 2700 -bad-apple/tools/run_mame.sh one +VT1200_DUMP_VRAM_FRAMES=2700 VT1200_DUMP_DIR=/tmp/ba-one \ + bad-apple/tools/run_mame.sh one # 5 s slice, paced BA_MOVIE=bad-apple/data5/movie.bin BA_START=0 BA_COUNT=300 \ @@ -100,3 +101,9 @@ block with the background colour, then filling the minority-colour rectangles. * **MAME is not the unit's performance verdict.** MAME needed ~2.1 emulated frames per movie record on the full stream; the unit's 50 MHz FILL rate is the number that matters. See `REPORT.md`. +* **The marker/control block lives in `.bss`**, not at bit `0x01810000`: a + 2.4 MB heap allocation can cross that address. `player.env` publishes + `MARK_PTR`; `load_mame.lua` reads it. +* **Pacing reads each record's `ticks`.** 60 source frames took 74 MAME frames + (72 display frames plus startup), so the 60-on-72 cadence is measured, not + assumed. diff --git a/bad-apple/REPORT.md b/bad-apple/REPORT.md index c810bdc..5801e14 100644 --- a/bad-apple/REPORT.md +++ b/bad-apple/REPORT.md @@ -50,9 +50,11 @@ primitive now re-asserts PSIZE=1 on every call. ## Pacing `wait_vsync()` programs `DPYINT=1`, clears `INTPEND`, and polls the DI bit at -`0xC0000120`. With `BA_PACE=1`, ten source frames took roughly fifteen MAME -frames (startup included), i.e. the 60-on-72 cadence is being honoured rather -than free-running. +`0xC0000120`. The loop reads each record's `ticks` from the index and advances +a 1/360 s accumulator by 5 per vsync, so a 1/60 s record needs 6 units and a +held record needs 12. Measured with `BA_COUNT=60`: status at MAME frame 2580, +74 frames after takeover at 2506 — 60 source frames in 72 display frames plus +startup, i.e. the 60-on-72 cadence is real, not free-running. ## The performance gap MAME cannot close @@ -68,12 +70,16 @@ measurement. `player-net.bin` uses the OS UDP socket API (`socket_`, `bind_`, `recvfrom_`, `malloc_`) to receive `movie.bin` on port 7001 into a heap buffer, -then DINTs and plays. The sender is `tools/send_movie.py`. +then DINTs and plays. The sender is `tools/send_movie.py`. Packets are +10-byte headers (offset, length, seq, crc16) plus body; a final +offset==length-of-file / 4-byte-checksum packet is required, and the receiver +refuses to jump unless the whole-file checksum matches. Out-of-range packets +are dropped without reading or writing outside the buffer. -Status: the packet format and sender are unit-tested; the MAME tap was down in -this worktree, so the UDP receive was not exercised end-to-end. The same OS -socket API is already proven on the unit by `harness/udpstream/udpstream.c`. -This is the one delivery-path gap. +Status: the packet format, header offset, checksum path and sender are +unit-tested; the MAME tap was down in this worktree, so the UDP receive was +not exercised end-to-end. The same OS socket API is already proven on the unit +by `harness/udpstream/udpstream.c`. This is the one delivery-path gap. ## Regression diff --git a/bad-apple/pipeline/movie.py b/bad-apple/pipeline/movie.py index 747f7dd..7af49cf 100644 --- a/bad-apple/pipeline/movie.py +++ b/bad-apple/pipeline/movie.py @@ -70,7 +70,8 @@ def _align4(value: int) -> int: def _pack_rect(rect: Rect) -> bytes: if not (0 <= rect.x0 < 32 and 0 <= rect.y0 < 32 - and 1 <= rect.w <= 32 and 1 <= rect.h <= 32): + and 1 <= rect.w <= 32 and 1 <= rect.h <= 32 + and rect.x0 + rect.w <= 32 and rect.y0 + rect.h <= 32): raise MovieFormatError("rect outside a 32x32 tile: %r" % (rect,)) value = (rect.x0 | (rect.y0 << 5) | ((rect.w - 1) << 10) | ((rect.h - 1) << 15)) diff --git a/bad-apple/player/build.sh b/bad-apple/player/build.sh index c26195b..b98b837 100755 --- a/bad-apple/player/build.sh +++ b/bad-apple/player/build.sh @@ -118,11 +118,14 @@ cp "$WORK/PLAYER.OUT" "$OUT/$NAME.coff" ENTRY_PTR=$(sed 's/\r$//' "$WORK/PLAYER.MAP" \ | awk '/ENTRY POINT SYMBOL/ { print $NF; exit }') -if [ -z "$ENTRY_PTR" ]; then - echo "build.sh: no entry point in PLAYER.map" >&2 +MARK_PTR=$(sed 's/\r$//' "$WORK/PLAYER.MAP" \ + | awk '$2 == "_marker_page" { print $1; exit }') +if [ -z "$ENTRY_PTR" ] || [ -z "$MARK_PTR" ]; then + echo "build.sh: no entry point or _marker_page in PLAYER.map" >&2 exit 1 fi -printf 'ENTRY_PTR=0x%s\n' "$ENTRY_PTR" > "$OUT/$NAME.env" +printf 'ENTRY_PTR=0x%s\nMARK_PTR=0x%s\n' "$ENTRY_PTR" "$MARK_PTR" \ + > "$OUT/$NAME.env" echo "build.sh: $OUT/$NAME.bin ($(wc -c < "$OUT/$NAME.bin") bytes)" grep -E "ENTRY POINT|^\.(text|cinit|bss)" "$OUT/player.map" | head -8 diff --git a/bad-apple/player/main.c b/bad-apple/player/main.c index 1b7b920..e0934cf 100644 --- a/bad-apple/player/main.c +++ b/bad-apple/player/main.c @@ -7,15 +7,18 @@ #define NET_DEFAULT 0 #endif -/* The 30 s stream's exact size, from data/manifest.json. The real loader - * allocates this before the first datagram; a different clip needs a rebuild. */ #define NET_LEN 2401942 +#define NET_PORT 7001 #define MOVIE_BASE 0x00080000 #define MOVIE_MAX 0x00308000 #define VRAM_BASE 0x04000000 #define ROW_BITS 2048 -#define MARK 0x01810000 + +/* The marker/control block lives in .bss, not at bit 0x01810000: a 2.4 MB + * heap allocation can cross that address and would corrupt the movie. */ +unsigned int marker_page[16]; +#define MARK ((unsigned int)marker_page) #define CTRL_START (MARK + 0x20 * 8) #define CTRL_COUNT (MARK + 0x24 * 8) #define CTRL_PACE (MARK + 0x28 * 8) @@ -24,8 +27,8 @@ #define CTRL_WAIT (MARK + 0x34 * 8) #define CTRL_READY (MARK + 0x38 * 8) #define CTRL_NET (MARK + 0x3C * 8) - -#define NET_PORT 7001 +#define CTRL_MAGIC (MARK + 0x1C * 8) +#define MAGIC_VALUE 0x42414450 extern void fill_run_(); extern void fill_init_(); @@ -47,7 +50,6 @@ int color; } fill_daddr = VRAM_BASE + (unsigned int)y * ROW_BITS + (unsigned int)x; fill_dydx = ((unsigned int)h << 16) | (unsigned int)w; - /* The decoder's colour is the canvas value: 1 = black, 0 = white. */ fill_color = color ? 0x0000 : 0xFFFF; fill_run_(); } @@ -63,20 +65,31 @@ void *ctx; fill_rect(x, y, w, h, color); } +static void show_fault(code) +unsigned int code; +{ + fill_init_(); + fill_rect(0, 0, 1280, 64, 1); + *(unsigned int *)MARK = code; + for (;;) { + } +} + static unsigned char rxbuf[1600]; static unsigned char sa[16]; static unsigned char from[16]; static int fromlen; -/* Receive movie.bin over the OS's UDP stack into a malloc'd bit buffer. - * Packet: u32 offset, u16 length, u16 seq, then bytes; length 0 is EOF. */ +/* Receive movie.bin over the OS UDP stack into a malloc'd bit buffer. + * Packet: u32 offset, u16 length, u16 seq, u16 crc16, then bytes. + * A final offset==expected/length==4 packet carries the 32-bit file checksum. */ static unsigned int receive_movie(expected) unsigned int expected; { unsigned int ptr; unsigned int offset; unsigned int length; - unsigned int total; + unsigned int sent_sum; unsigned int i; unsigned char *dst; int fd; @@ -95,12 +108,12 @@ unsigned int expected; close_(fd); return 0; } - total = 0; + sent_sum = 0; dst = (unsigned char *)ptr; for (;;) { fromlen = SOCKADDR_LEN; n = recvfrom_(fd, rxbuf, (int)sizeof(rxbuf), 0, from, &fromlen); - if (n < 8) { + if (n < 10) { continue; } offset = (unsigned int)rxbuf[0] @@ -108,21 +121,27 @@ unsigned int expected; | ((unsigned int)rxbuf[2] << 16) | ((unsigned int)rxbuf[3] << 24); length = (unsigned int)rxbuf[4] | ((unsigned int)rxbuf[5] << 8); - if (length == 0) { + if (offset == expected && length == 4 && n >= 14) { + sent_sum = (unsigned int)rxbuf[10] + | ((unsigned int)rxbuf[11] << 8) + | ((unsigned int)rxbuf[12] << 16) + | ((unsigned int)rxbuf[13] << 24); break; } - if (offset + length > expected) { + if (length > (unsigned int)(n - 10)) { + continue; + } + if (offset > expected || length > expected - offset) { continue; } for (i = 0; i < length; i++) { - dst[offset + i] = rxbuf[8 + i]; - } - total += length; - if (total >= expected) { - break; + dst[offset + i] = rxbuf[10 + i]; } } close_(fd); + if (movie_checksum(dst, expected) != sent_sum) { + return 0; + } return ptr; } @@ -133,8 +152,6 @@ static void wait_vsync() unsigned int i; unsigned short pending; - /* Clear a stale DI, then wait for the next one. DPYINT is programmed to - * line 1 once at startup, so DI appears at the top of every frame. */ *(unsigned short *)0xC0000120 = 0; for (i = 0; i < 200000; i++) { pending = *(unsigned short *)0xC0000120; @@ -147,44 +164,54 @@ static void wait_vsync() int main() { struct movie_header header; + unsigned int magic; unsigned int frame; unsigned int last; unsigned int start; unsigned int count; unsigned int pace; + unsigned int net; + unsigned int wait; unsigned int movie_base; unsigned int movie_len; - unsigned int net; + unsigned int ticks; + unsigned int now; + unsigned int next; int rc; - movie_base = *(unsigned int *)CTRL_MOVIE; - movie_len = *(unsigned int *)CTRL_MOVIELEN; - if (movie_base == 0) { + magic = *(unsigned int *)CTRL_MAGIC; + if (magic == MAGIC_VALUE) { + start = *(unsigned int *)CTRL_START; + count = *(unsigned int *)CTRL_COUNT; + pace = *(unsigned int *)CTRL_PACE; + net = *(unsigned int *)CTRL_NET; + wait = *(unsigned int *)CTRL_WAIT; + movie_base = *(unsigned int *)CTRL_MOVIE; + movie_len = *(unsigned int *)CTRL_MOVIELEN; + } else { + start = 0; + count = 0; + pace = 1; + net = NET_DEFAULT; + wait = 0; movie_base = MOVIE_BASE; - } - if (movie_len == 0) { movie_len = MOVIE_MAX; } - - net = *(unsigned int *)CTRL_NET; if (net == 0 && NET_DEFAULT) { net = 1; } + if (net != 0) { - /* Real delivery: the OS stack is still alive for the receive; the - * payload owns the machine only after the last byte arrives. */ movie_len = NET_LEN; *(unsigned int *)(MARK + 0x10 * 8) = 0x20; movie_base = receive_movie(movie_len); if (movie_base == 0) { - *(unsigned int *)MARK = 0xBAD00003; - for (;;) { - } + show_fault(0xBAD00003); } vid_dint_(); } else { vid_dint_(); - if (*(unsigned int *)CTRL_WAIT != 0) { + if (wait != 0) { *(unsigned int *)(MARK + 0x10 * 8) = 0x10; while (*(unsigned int *)CTRL_READY == 0) { } @@ -192,24 +219,14 @@ int main() } fill_init_(); - - /* Display interrupt at line 1: our only timebase. Interrupts stay off; - * wait_vsync() polls the pending bit. */ *(unsigned short *)0xC00000A0 = 1; - - /* Start from white; the OS left its own screen behind. */ fill_rect(0, 0, 1280, 1024, 0); rc = movie_open((unsigned char *)movie_base, movie_len, &header); if (rc != 0) { - *(unsigned int *)MARK = 0xBAD00001; - for (;;) { - } + show_fault(0xBAD00001); } - start = *(unsigned int *)CTRL_START; - count = *(unsigned int *)CTRL_COUNT; - pace = *(unsigned int *)CTRL_PACE; last = header.frame_count; if (last != 0) { last = last - 1; @@ -218,19 +235,33 @@ int main() last = start + count - 1; } - for (frame = 0; frame <= last; frame++) { + now = 0; + next = 0; + frame = 0; + while (frame <= last) { if (pace != 0 && frame >= start) { wait_vsync(); + now += 5; /* 60/72 in 1/360 s units */ + if (now < next) { + continue; + } } rc = movie_decode_frame((unsigned char *)movie_base, movie_len, frame, &header, paint, 0); if (rc != 0) { - *(unsigned int *)MARK = 0xBAD00002; - *(unsigned int *)(MARK + 0x14 * 8) = frame; - for (;;) { - } + show_fault(0xBAD00002); + } + ticks = movie_frame_ticks((unsigned char *)movie_base, movie_len, + frame, &header); + if (ticks < 1) { + ticks = 1; + } + next += 6 * ticks; + if (next + 360 < now) { + next = now; } *(unsigned int *)MARK = frame; + frame++; } *(unsigned int *)(MARK + 0x10 * 8) = (count != 0) ? 2 : 1; diff --git a/bad-apple/player/movie.c b/bad-apple/player/movie.c index c4a67d6..c83a94d 100644 --- a/bad-apple/player/movie.c +++ b/bad-apple/player/movie.c @@ -49,11 +49,17 @@ struct movie_header *header; header->data_offset = (unsigned int)rd32(data + 26); header->source_fps = rd16(data + 30); header->display_hz = rd16(data + 32); - if (header->scale == 0 || header->tile == 0 + if (header->scale == 0 || header->scale > 4 || header->tile == 0 || header->tile > 32 || header->tiles_x == 0 || header->tiles_y == 0) { return -1; } + if (header->width != header->tiles_x * header->tile + || header->height != header->tiles_y * header->tile + || header->width * header->scale > 1280 + || header->height * header->scale > 1024) { + return -1; + } if ((unsigned long)header->index_offset + (unsigned long)header->frame_count * INDEX_ENTRY_SIZE > len) { return -1; @@ -61,6 +67,25 @@ struct movie_header *header; return 0; } +unsigned int movie_frame_ticks(data, len, frame, header) +unsigned char *data; +unsigned long len; +unsigned int frame; +struct movie_header *header; +{ + unsigned long index_off; + + if (data == 0 || header == 0 || frame >= header->frame_count) { + return 1; + } + index_off = (unsigned long)header->index_offset + + (unsigned long)frame * INDEX_ENTRY_SIZE; + if (index_off + INDEX_ENTRY_SIZE > len) { + return 1; + } + return (unsigned int)data[index_off + 4]; +} + int movie_decode_frame(data, len, frame, header, fill, ctx) unsigned char *data; unsigned long len; @@ -140,6 +165,9 @@ void *ctx; y0 = (value >> 5) & 0x1F; w = ((value >> 10) & 0x1F) + 1; h = ((value >> 15) & 0x1F) + 1; + if (x0 + w > header->tile || y0 + h > header->tile) { + return -1; + } sx = (int)((tx * header->tile + x0) * header->scale); sy = (int)((ty * header->tile + y0) * header->scale); fill(sx, sy, (int)(w * header->scale), diff --git a/bad-apple/player/movie.h b/bad-apple/player/movie.h index abf514b..349b77e 100644 --- a/bad-apple/player/movie.h +++ b/bad-apple/player/movie.h @@ -20,6 +20,7 @@ typedef void (*movie_fill_fn)(); int movie_open(); int movie_decode_frame(); +unsigned int movie_frame_ticks(); unsigned int movie_checksum(); #endif diff --git a/bad-apple/tests/test_movie.py b/bad-apple/tests/test_movie.py index 877e7cf..7800a0e 100644 --- a/bad-apple/tests/test_movie.py +++ b/bad-apple/tests/test_movie.py @@ -27,6 +27,15 @@ def test_more_than_127_rects_roundtrip(): assert len(back.tiles[0].rects) == 200 +def test_rect_outside_tile_raises(): + rec = FrameRecord(1, 0, [TileRecord(0, 0, [Rect(31, 0, 2, 1)])]) + try: + encode_frame(rec) + except MovieFormatError: + return + assert False, "rect crossing a tile boundary was accepted" + + def test_truncated_record_raises(): data = encode_frame( FrameRecord(1, 0, [TileRecord(0, 0, [Rect(0, 0, 4, 4)])])) diff --git a/bad-apple/tools/load_mame.lua b/bad-apple/tools/load_mame.lua index 693cbbc..35a3f49 100644 --- a/bad-apple/tools/load_mame.lua +++ b/bad-apple/tools/load_mame.lua @@ -79,8 +79,22 @@ if not payload or not movie then return end local entry = entry_from_env() -print(string.format("BA: payload %d bytes, movie %d bytes, entry %s", - #payload, #movie, hex(entry))) + +do + local f = io.open(ENV_PATH, "r") + if f then + for line in f:lines() do + local value = line:match("MARK_PTR=0x(%x+)") + if value then + MARK = tonumber(value, 16) + end + end + f:close() + end +end + +print(string.format("BA: payload %d bytes, movie %d bytes, entry %s mark %s", + #payload, #movie, hex(entry), hex(MARK))) local function write_bytes(data, base) local i @@ -125,6 +139,7 @@ local function load_and_enter() machine:exit() return end + prog:write_u32(MARK + 0x1C * 8, 0x42414450) prog:write_u32(MARK + 0x20 * 8, START) prog:write_u32(MARK + 0x24 * 8, COUNT) prog:write_u32(MARK + 0x28 * 8, PACE) @@ -185,8 +200,10 @@ emu.register_frame_done(function() end end if frames >= STOP_AT then - print(string.format("BA: end at frame %d (loaded=%s done=%s)", - frames, tostring(loaded), tostring(done))) + print(string.format("BA: end at frame %d (loaded=%s done=%s) status=%04X frame=%d magic=%08X", + frames, tostring(loaded), tostring(done), + prog:read_u16(MARK + 0x10 * 8), prog:read_u16(MARK), + prog:read_u32(MARK + 0x1C * 8))) machine:exit() end end, "vt1200-badapple-loader") diff --git a/bad-apple/tools/send_movie.py b/bad-apple/tools/send_movie.py index ae5f423..2cf35e0 100644 --- a/bad-apple/tools/send_movie.py +++ b/bad-apple/tools/send_movie.py @@ -19,6 +19,10 @@ def crc16(data: bytes) -> int: return sum(data) & 0xFFFF +def checksum32(data: bytes) -> int: + return sum(data) & 0xFFFFFFFF + + def pack_packet(offset: int, seq: int, body: bytes) -> bytes: return struct.pack("