fix(badapple): checksum, packet bounds, ticks and marker placement

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.

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
 

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))

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

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;

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),

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

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)])]))

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")

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("<IHHH", offset, len(body), seq, crc16(body)) + body
 
@@ -43,7 +47,9 @@ def send_movie(host: str, port: int, path: str, chunk: int = 1450,
             seq += 1
             if delay:
                 time.sleep(delay)
-        sock.sendto(pack_packet(len(data), seq, b""), (host, port))
+        sock.sendto(pack_packet(len(data), seq,
+                                checksum32(data).to_bytes(4, "little")),
+                    (host, port))
     finally:
         sock.close()
     print("sent %d bytes in %d packets" % (len(data), seq + 1))