From 86a28d2a3bf56fca420e57dd3576134eae4875a1 Mon Sep 17 00:00:00 2001 From: claude-noether Date: Tue, 26 May 2026 07:02:29 +0200 Subject: [PATCH] Stage 2 PR-A2: per-MB inspection callback wiring + invariant checks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Validates marfrit-packages patch 0016 (PR #106) end-to-end against the daedalus_decode_h264 CLI. Callback fires once per macroblock in coded order; this PR checks the count + uniqueness invariants WITHOUT yet driving daedalus-decoder differently — that's PR-A3. Infrastructure landed --------------------- CMake gains DAEDALUS_FFMPEG_PREFIX option pointing at a private FFmpeg install carrying patch 0016. When set, the CLI links against it (static .a's from $prefix/lib) and the inspection codepath is compiled in (DAEDALUS_HAVE_H264_MB_INSPECT_CB). When unset, the CLI falls back to the pkg-config-discovered system FFmpeg and behaves as PR-A1b did (identity-passthrough only, no callback). The H264Context struct stays opaque (forward-decl only — its real definition lives in libavcodec's internal h264dec.h which isn't installed). Real per-MB state extraction (sl->mb coeffs, mb_type, intra modes, deblock params) will land in PR-A3 alongside an internal-header include path. The callback's only job in this PR: assert (mb_x, mb_y) lies in the coded grid, mark "seen" in a per-frame bitmap, count invocations. At end-of-frame: assert seen-count == mb_w*mb_h, 0 duplicates, 0 out-of-bounds. Per-frame mb-grid init goes BEFORE first avcodec_send_packet (callbacks fire from inside send_packet, before the first receive_frame ever returns — lazy init from AVFrame would miss all of frame 0). Dims come from codecpar->width/height rounded up to 16-mod (H.264 codes 1080 display as 1088 coded). Raster-order check considered but dropped: libavcodec uses MB-level threading in some configs so callbacks fire out of raster order. The contract is "each MB exactly once", not "in raster order"; the bitmap check captures that. Result on hertz (Pi 5, patched FFmpeg at /tmp/ffmpeg-inspect-prefix) ------------------------------------------------------------------- 320x240 I-only, 3 frames: mb-grid 20x15 callback invocations: 900 (= 3 * 300) missing/duplicates/oob: 0/0/0 identity-passthrough Y diff 0/230400, UV diff 0/115200 PASS 1920x1088 I-only, 3 frames: mb-grid 120x68 callback invocations: 24480 (= 3 * 8160) missing/duplicates/oob: 0/0/0 identity-passthrough Y diff 0/6266880, UV diff 0/3133440 PASS Followups --------- - PR-A3: include libavcodec/h264dec.h via -I to access H264Context internals; extract sl->mb coefficients in the callback, compute P = pre-deblock pixels - IDCT(C) using a transcribed C reference; feed daedalus_decoder with REAL (P, C, edges) instead of identity. Use avctx->skip_loop_filter = AVDISCARD_ALL to make libavcodec output pre-deblock so the subtraction is exact. - PR-A4 onwards: extend to P/B frames + chroma DC + intra prediction coverage. --- CMakeLists.txt | 26 ++++++- tools/daedalus_decode_h264.c | 131 +++++++++++++++++++++++++++++++++-- 2 files changed, 150 insertions(+), 7 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 41e57f3..387772e 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -175,7 +175,31 @@ target_compile_options(bench_flush_frame PRIVATE -O2) # so the standard ctest build doesn't pull in FFmpeg as a hard dep. option(DAEDALUS_BUILD_TOOLS "Build daedalus-decoder CLI tools (requires libavcodec)" OFF) if(DAEDALUS_BUILD_TOOLS) - pkg_check_modules(FFMPEG REQUIRED libavcodec libavformat libavutil) + # Optional path to a private FFmpeg install carrying the per-MB + # inspection callback (marfrit-packages patch 0016). When set, + # the CLI links against it instead of the system FFmpeg and the + # inspection-callback code path is compiled in. + set(DAEDALUS_FFMPEG_PREFIX "" CACHE PATH + "Path to a patched FFmpeg install (with 0016 mb-inspect-callback) for daedalus_decode_h264. Empty = use system pkg-config FFmpeg.") + + if(DAEDALUS_FFMPEG_PREFIX) + message(STATUS "daedalus_decode_h264: patched FFmpeg at ${DAEDALUS_FFMPEG_PREFIX}") + set(FFMPEG_INCLUDE_DIRS ${DAEDALUS_FFMPEG_PREFIX}/include) + set(FFMPEG_LIBRARY_DIRS ${DAEDALUS_FFMPEG_PREFIX}/lib) + # Patched libavcodec is built static (no shared libs in the private prefix). + # System pull-ins are still needed for libav* dependencies. + set(FFMPEG_LIBRARIES + ${DAEDALUS_FFMPEG_PREFIX}/lib/libavformat.a + ${DAEDALUS_FFMPEG_PREFIX}/lib/libavcodec.a + ${DAEDALUS_FFMPEG_PREFIX}/lib/libavutil.a + ${DAEDALUS_FFMPEG_PREFIX}/lib/libswresample.a + m z pthread) + set(FFMPEG_CFLAGS_OTHER "-DDAEDALUS_HAVE_H264_MB_INSPECT_CB=1") + else() + pkg_check_modules(FFMPEG REQUIRED libavcodec libavformat libavutil) + message(STATUS "daedalus_decode_h264: system FFmpeg (no inspection callback)") + endif() + add_executable(daedalus_decode_h264 tools/daedalus_decode_h264.c) target_link_libraries(daedalus_decode_h264 PRIVATE daedalus_decoder ${FFMPEG_LIBRARIES}) diff --git a/tools/daedalus_decode_h264.c b/tools/daedalus_decode_h264.c index cfccca9..9c115b6 100644 --- a/tools/daedalus_decode_h264.c +++ b/tools/daedalus_decode_h264.c @@ -50,6 +50,22 @@ #include #include +/* Per-MB inspection callback API — provided by the patched FFmpeg + * fork via marfrit-packages 0016. The H264Context struct itself + * remains internal (declared in libavcodec/h264dec.h which isn't + * installed), so we only forward-declare it here and use it + * opaquely through the callback signature. Real per-MB state + * extraction (sl->mb coefficients, mb_type, etc.) will land in + * PR-A3 alongside an internal-header include path. */ +#ifdef DAEDALUS_HAVE_H264_MB_INSPECT_CB +struct H264Context; +typedef void (*ff_h264_mb_inspect_cb)(void *opaque, + const struct H264Context *h, + int mb_x, int mb_y); +void ff_h264_set_mb_inspect_cb(AVCodecContext *avctx, + ff_h264_mb_inspect_cb cb, void *opaque); +#endif + #include #include #include @@ -59,6 +75,39 @@ static const char *substrate_str = "auto"; static int max_frames = -1; +/* Inspection-callback state: per-frame counter + "each MB seen exactly + * once" check. We use a bitmap rather than a raster-order assertion + * because libavcodec's MB-level threading + multi-slice frames mean + * MBs reach the callback in non-strictly-raster order; the contract + * is "every MB fires the callback exactly once per frame", not "in + * raster order". Reset at end of each frame. */ +#ifdef DAEDALUS_HAVE_H264_MB_INSPECT_CB +struct inspect_state { + int n_cbs_this_frame; + int mb_w, mb_h; + uint8_t *seen; /* mb_w * mb_h bitmap */ + int duplicate_mbs; /* same (mb_x, mb_y) seen twice this frame */ + int out_of_bounds; /* (mb_x, mb_y) outside the coded grid */ +}; + +static void inspect_cb(void *opaque, + const struct H264Context *h, + int mb_x, int mb_y) +{ + (void) h; + struct inspect_state *st = opaque; + + if (mb_x < 0 || mb_x >= st->mb_w || mb_y < 0 || mb_y >= st->mb_h) { + st->out_of_bounds++; + } else { + const size_t idx = (size_t) mb_y * st->mb_w + (size_t) mb_x; + if (st->seen[idx]) st->duplicate_mbs++; + st->seen[idx] = 1; + } + st->n_cbs_this_frame++; +} +#endif + /* Extract one MB's predicted-samples block from a YUV420P AVFrame * (stock libavcodec) and pack it into the 384-byte mb_input.predicted * layout: 16x16 luma raster, then 8x8 Cb raster, then 8x8 Cr raster. @@ -206,19 +255,43 @@ int main(int argc, char **argv) AVPacket *pkt = av_packet_alloc(); AVFrame *fr = av_frame_alloc(); - /* ---- Create daedalus_decoder. Coded width/height come from - * the bitstream's SPS via libavcodec (after the first packet - * is decoded — defer creation until then). ---- */ + /* ---- Allocate output buffers + state needed before first decode ---- */ daedalus_decoder *dec = NULL; uint8_t *out_y_dadec = NULL, *out_uv_dadec = NULL; uint8_t *out_y_ref = NULL, *out_uv_ref = NULL; size_t y_size = 0, uv_size = 0; FILE *out_dadec_f = NULL, *out_ref_f = NULL; - int rc = 0; int n_frames = 0; size_t total_y_diffs = 0, total_uv_diffs = 0; +#ifdef DAEDALUS_HAVE_H264_MB_INSPECT_CB + /* Init inspect state BEFORE the first avcodec_send_packet — the + * callback fires from inside send_packet (i.e. before the first + * receive_frame ever returns), so lazy-init after-the-fact + * would miss the entire first frame. Use codecpar dims; round + * up to MB granularity (H.264 codes 1080 height as 1088). */ + struct inspect_state inspect_st = {0}; + { + const AVCodecParameters *cp = fmt->streams[vstream]->codecpar; + const int W_round = (cp->width + 15) & ~15; + const int H_round = (cp->height + 15) & ~15; + inspect_st.mb_w = W_round / 16; + inspect_st.mb_h = H_round / 16; + inspect_st.seen = calloc(1, (size_t) inspect_st.mb_w * inspect_st.mb_h); + if (!inspect_st.seen) { rc = 1; goto cleanup; } + } + ff_h264_set_mb_inspect_cb(avctx, inspect_cb, &inspect_st); + int inspect_total_cbs = 0; + int inspect_total_duplicate = 0; + int inspect_total_oob = 0; + int inspect_total_missing = 0; +#endif + + /* ---- daedalus_decoder is lazy-created on the first AVFrame + * (coded width/height come from the bitstream's SPS via + * libavcodec). ---- */ + while (av_read_frame(fmt, pkt) >= 0) { if (pkt->stream_index != vstream) { av_packet_unref(pkt); continue; } @@ -269,6 +342,12 @@ int main(int argc, char **argv) } printf("daedalus_decode_h264: %dx%d, substrate=%s\n", W, H, substrate_str); +#ifdef DAEDALUS_HAVE_H264_MB_INSPECT_CB + printf(" inspection callback: ACTIVE (patched libavcodec); " + "mb-grid %dx%d\n", inspect_st.mb_w, inspect_st.mb_h); +#else + printf(" inspection callback: not built in (stock libavcodec)\n"); +#endif } /* Pack each MB's predicted samples from the AVFrame. @@ -320,6 +399,33 @@ int main(int argc, char **argv) if (out_uv_dadec[i] != out_uv_ref[i]) uv_diffs++; total_y_diffs += y_diffs; total_uv_diffs += uv_diffs; +#ifdef DAEDALUS_HAVE_H264_MB_INSPECT_CB + { + const int expected = mb_w * mb_h; + /* Count MBs that fired the callback. */ + int seen_count = 0; + for (int i = 0; i < expected; i++) + if (inspect_st.seen[i]) seen_count++; + int missing = expected - seen_count; + if (missing || inspect_st.duplicate_mbs || inspect_st.out_of_bounds) { + fprintf(stderr, + " frame %d: callback invariants: fired=%d expected=%d " + "missing=%d duplicates=%d oob=%d\n", + n_frames, inspect_st.n_cbs_this_frame, expected, + missing, inspect_st.duplicate_mbs, inspect_st.out_of_bounds); + rc = 4; + } + inspect_total_cbs += inspect_st.n_cbs_this_frame; + inspect_total_duplicate += inspect_st.duplicate_mbs; + inspect_total_oob += inspect_st.out_of_bounds; + inspect_total_missing += missing; + /* Reset for next frame. */ + inspect_st.n_cbs_this_frame = 0; + inspect_st.duplicate_mbs = 0; + inspect_st.out_of_bounds = 0; + memset(inspect_st.seen, 0, (size_t) expected); + } +#endif printf(" frame %d: Y diff %zu/%zu UV diff %zu/%zu%s\n", n_frames, y_diffs, y_size, uv_diffs, uv_size, (y_diffs || uv_diffs) ? " ***" : ""); @@ -348,11 +454,21 @@ int main(int argc, char **argv) drained: printf("\n%d frames decoded; total Y diff %zu, UV diff %zu\n", n_frames, total_y_diffs, total_uv_diffs); - if (total_y_diffs || total_uv_diffs) { +#ifdef DAEDALUS_HAVE_H264_MB_INSPECT_CB + printf("inspection callback: %d total invocations, %d missing, %d duplicates, %d oob\n", + inspect_total_cbs, inspect_total_missing, inspect_total_duplicate, inspect_total_oob); + if (inspect_total_missing || inspect_total_duplicate || inspect_total_oob) + rc = 4; +#endif + if (rc == 0 && (total_y_diffs || total_uv_diffs)) { printf("FAIL: daedalus-decoder output does NOT match libavcodec reference byte-for-byte\n"); rc = 4; - } else { + } else if (rc == 0) { printf("PASS: byte-exact identity-passthrough across %d frames\n", n_frames); + } else { + printf("FAIL: %s\n", + (total_y_diffs || total_uv_diffs) ? "byte-exact comparison failed" + : "inspection callback invariants violated"); } cleanup: @@ -360,6 +476,9 @@ cleanup: if (out_ref_f) fclose(out_ref_f); free(out_uv_ref); free(out_y_ref); free(out_uv_dadec);free(out_y_dadec); +#ifdef DAEDALUS_HAVE_H264_MB_INSPECT_CB + free(inspect_st.seen); +#endif if (dec) daedalus_decoder_destroy(dec); av_frame_free(&fr); av_packet_free(&pkt); -- 2.47.3