diff --git a/runtime/src/mdec.c b/runtime/src/mdec.c index faa150f6..b9446483 100644 --- a/runtime/src/mdec.c +++ b/runtime/src/mdec.c @@ -291,11 +291,13 @@ static uint8_t to_output_u8(int value) { return (uint8_t)(value + 128); } -/* 8-bit unsigned channel → 5-bit, Beetle RGB_to_RGB555 rounding (mdec.cpp:306). */ -static int rgb_to_555_chan(int c) { - c = (c + 4) >> 3; - if (c > 0x1F) c = 0x1F; - return c; +/* 8-bit unsigned channel → 5-bit, Beetle RGB_to_RGB555 rounding (mdec.cpp:306). + * Beetle's RGB_to_RGB555 takes uint8 params, so the ^0x80 result is truncated to + * 0..255 BEFORE the round/shift — `c` here is already that uint8. */ +static int rgb_to_555_chan(uint8_t c) { + int v = (c + 4) >> 3; + if (v > 0x1F) v = 0x1F; + return v; } static void append_rgb_pixel(int y, int cr, int cb) { diff --git a/tools/mdec_pin/FINDING.md b/tools/mdec_pin/FINDING.md index 2f5e02cf..19da4a25 100644 --- a/tools/mdec_pin/FINDING.md +++ b/tools/mdec_pin/FINDING.md @@ -1,36 +1,30 @@ -# MDEC numeric pin — finding (2026-06-27) +# MDEC pin — finding + RESOLUTION (2026-06-27) -`mdec_pin.c` runs OUR mdec.c numeric pipeline (dequant/IDCT/YUV, verbatim from the -WIP transcription) vs BEETLE's verbatim pipeline (mdec.cpp) on identical synthetic -input, diffing each stage. +## The bug (FOUND + FIXED) +The faithful dequant/IDCT/YUV transcription was correct, but the **15bpp pack lost the +uint8 truncation** Beetle's `RGB_to_RGB555(uint8 r,g,b)` performs implicitly: -## Result: numeric core is BYTE-EXACT vs Beetle - [scale matrix] diffs: 0 - [dequant Coeff] diffs: 0 - [idct block] diffs: 0 - [yuv->rgb] diffs: 0 -So the D1/D2/D4/D5 transcription (dequant <<4 + bias, two-pass IDCT (sum+0x4000)>>15 -+ Mask9ClampS8 + transposed >>3 matrix + Beetle ZigZag, YUV /256 + green mask) is a -faithful match to the oracle IN ISOLATION. + int ru = r ^ 0x80; // r = Mask9ClampS8 ∈ [-128,127], a SIGNED int + ... rgb_to_555_chan(ru) ... // (c+4)>>3 on a possibly-NEGATIVE int → garbage -## BUT the integrated change REGRESSES the live FMV -Live Tomba 2 with the WIP change: the Whoopee Camp logo shows RAINBOW colour fringing. -Baseline (old numeric core) renders it CLEAN PINK (matching Beetle). So the numeric-only -change makes it worse on-screen even though the math is byte-exact. +For negative `r`, `r ^ 0x80` is a negative int (e.g. r=-1 → -129), and the old +`rgb_to_555_chan(int)` did `(c+4)>>3` on it without truncating to 0..255. Beetle passes +the value through `uint8` params, truncating first. Result on real FMV: B saturated to +31, R/G ~doubled → the rainbow fringing. The 24bpp path was unaffected (it already casts +to `uint8_t`). FIX: `rgb_to_555_chan(uint8_t c)` (caller's `int` truncates on the call), +matching Beetle. mdec.c:rgb_to_555_chan. -## Root cause: layout pairing, not the math -Adopting Beetle's ZigZag + transposed IDCT changes the block's internal spatial layout. -The old numeric core and the old block-assembly (`append_color_macroblock` + the -multi-block Cr/Cb/Y flow) were a MATCHED PAIR producing correct output. Swapping only the -numeric core to Beetle's convention without transcribing the assembly breaks the pairing -→ chroma misaligns vs luma → rainbow fringing. +## Why both pins initially MISSED it (important) +`mdec_pin.c` and `mdec_e2e.c`'s reference both computed the 555 channel on the same +un-truncated `int` — i.e. the reference REPLICATED mdec.c's bug, so they agreed (0 diffs) +while BOTH diverged from real Beetle. A standalone pin whose oracle is your own +transcription cannot catch an error shared by both. The truth came from the +**real Beetle process**: a `vram_peek` added to beetle_debug_server.c, diffing our VRAM +vs Beetle's VRAM at the held FMV (the gold-standard oracle). That collapsed the channel +deltas from mean (dR 12, dG 16, dB 27) to (0.7, 0.9, 1.3) and removed the rainbow. -## Proper completion (do NOT merge until this passes) -1. Transcribe Beetle's EncodeImage assembly (mdec.cpp:324-419) too — block→output pixel - mapping — so the assembly matches the new (Beetle) block layout. -2. Build a FULL-PATH pin: drive the REAL mdec.c end-to-end via its DMA API (mdec_write / - mdec_dma_write_word / mdec_dma_read_word) on a synthetic full 6-block colour macroblock, - diff the packed output against a Beetle-faithful full pipeline (decode + EncodeImage). - The isolated pin here misses the integration; the full-path pin would have caught it. -3. Only when the full-path packed output is byte-exact AND the live FMV renders clean → - merge. +## Validation now +- `mdec_e2e.c` full-path pin (with the corrected uint8 oracle): byte-exact. +- Live VRAM vs real Beetle: rainbow gone, FMV renders natural colours; residual <1.3 + mean (sub-LSB / Beetle's upscaled-VRAM representation + free-running frame mismatch). +LESSON: validate against the REAL oracle process, not a self-transcribed reference. diff --git a/tools/mdec_pin/mdec_e2e.c b/tools/mdec_pin/mdec_e2e.c new file mode 100644 index 00000000..912a114a --- /dev/null +++ b/tools/mdec_pin/mdec_e2e.c @@ -0,0 +1,131 @@ +/* mdec_e2e.c — FULL-PATH MDEC pin. Drives the REAL runtime/src/mdec.c end-to-end + * through its public DMA/MMIO API (SET_QUANT, SET_SCALE, DECODE, read output) on a + * synthetic 6-block colour macroblock, and diffs the packed 16bpp output against a + * Beetle-faithful full pipeline (dequant + IDCT + EncodeImage assembly) at every + * spatial pixel. Localizes integration bugs the isolated mdec_pin.c misses. + * + * Build (from runtime root): gcc -O2 -I runtime/include -o mdec_e2e tools/mdec_pin/mdec_e2e.c runtime/src/mdec.c + */ +#include +#include +#include +#include "mdec.h" + +uint64_t s_frame_count = 0; /* mdec.c extern */ + +/* ---------------- synthetic input ---------------- */ +static uint8_t QY[64], QC[64]; +static uint16_t SCALEW[64]; +static uint16_t BLK[6][24]; static int BLKN[6]; + +static void make_input(void){ + for(int i=0;i<64;i++) SCALEW[i]=(uint16_t)(0x1000 + (i*53%777) - 388); + for(int i=0;i<64;i++){ QY[i]=(uint8_t)(2+(i%17)); QC[i]=(uint8_t)(3+(i%13)); } + /* 6 distinct blocks, AGGRESSIVE: many ACs, large/saturating levels, a qscale=0 + * block, runs that reach high zig indices, EOB at varied points. */ + for(int b=0;b<6;b++){ + int n=0; + int qs = (b==3) ? 0 : (8+b); /* block 3: qscale=0 path */ + BLK[b][n++]=(uint16_t)((qs<<10)|((400-37*b)&0x3FF)); /* DC, larger level */ + BLK[b][n++]=(uint16_t)((0u<<10)|((uint16_t)(-300+11*b)&0x3FF)); /* big neg */ + BLK[b][n++]=(uint16_t)((1u<<10)|((250-9*b)&0x3FF)); + BLK[b][n++]=(uint16_t)((0u<<10)|((uint16_t)(-120)&0x3FF)); + BLK[b][n++]=(uint16_t)((4u<<10)|((333+b)&0x3FF)); /* level >0x100 (10-bit) */ + BLK[b][n++]=(uint16_t)((2u<<10)|((uint16_t)(-77)&0x3FF)); + BLK[b][n++]=(uint16_t)((7u<<10)|((90)&0x3FF)); + BLK[b][n++]=(uint16_t)((11u<<10)|((uint16_t)(-200)&0x3FF)); /* high zig index */ + BLK[b][n++]=(uint16_t)((1u<<10)|((45)&0x3FF)); + BLK[b][n++]=0xFE00u; /* end of block */ + BLKN[b]=n; + } +} + +/* ---------------- drive the REAL mdec.c ---------------- */ +static uint8_t OUT[1024]; static int OUTN; +static void run_real(void){ + mdec_init(); + /* control: enable nothing special; we feed via port 0 directly */ + /* SET_QUANT (both tables): cmd (2<<29)|1, 64 halfwords = 128 bytes = 32 words */ + mdec_write(0, (2u<<29)|1u); + uint8_t qbytes[128]; memcpy(qbytes,QY,64); memcpy(qbytes+64,QC,64); + for(int i=0;i<128;i+=4){ uint32_t w=qbytes[i]|(qbytes[i+1]<<8)|(qbytes[i+2]<<16)|(qbytes[i+3]<<24); mdec_write(0,w); } + /* SET_SCALE: cmd (3<<29), 64 halfwords = 32 words */ + mdec_write(0, (3u<<29)); + for(int i=0;i<64;i+=2){ uint32_t w=SCALEW[i]|((uint32_t)SCALEW[i+1]<<16); mdec_write(0,w); } + /* DECODE 16bpp (depth=3): assemble all 6 blocks' halfwords, pad to even, frame as words */ + uint16_t hw[256]; int hn=0; + for(int b=0;b<6;b++) for(int i=0;i1024)OUTN=1024; + for(int i=0;i>8)&0xFF; if(i+2>16)&0xFF; if(i+3>24)&0xFF; } +} + +/* ---------------- BEETLE-faithful reference (verbatim from mdec.cpp) ---------------- */ +static const uint8_t bz[64]={ + 0x00,0x08,0x01,0x02,0x09,0x10,0x18,0x11,0x0a,0x03,0x04,0x0b,0x12,0x19,0x20,0x28, + 0x21,0x1a,0x13,0x0c,0x05,0x06,0x0d,0x14,0x1b,0x22,0x29,0x30,0x38,0x31,0x2a,0x23, + 0x1c,0x15,0x0e,0x07,0x0f,0x16,0x1d,0x24,0x2b,0x32,0x39,0x3a,0x33,0x2c,0x25,0x1e, + 0x17,0x1f,0x26,0x2d,0x34,0x3b,0x3c,0x35,0x2e,0x27,0x2f,0x36,0x3d,0x3e,0x37,0x3f}; +static int16_t bmat[64]; +static int bsx(int bits,int v){int s=32-bits;return (int)(((int32_t)((uint32_t)v<>s);} +static int b10(int v){return (int16_t)bsx(10,v);} +static int bm9(int v){v=bsx(9,v);if(v<-128)v=-128;if(v>127)v=127;return v;} +static int bmin(int a,int b){return ab?a:b;} +static void bset(void){for(int i=0;i<64;i++)bmat[((i&7)<<3)|((i>>3)&7)]=(int16_t)((int16_t)SCALEW[i]>>3);} +static void bdq(int16_t*C,const uint16_t*w,int n,const uint8_t*QM){ + memset(C,0,64*sizeof(int16_t)); int p=0,ci=0,qs=0; + while(p>10; int q=QM[0],c=b10(V&0x3FF),t; + if(q)t=(int32_t)((uint32_t)(c*q)<<4)+(c?((c<0)?8:-8):0); else t=(uint32_t)(c*2)<<4; + C[bz[0]]=bmin(0x3FFF,bmax(-0x4000,t)); ci++; + } else { if(V==0xFE00){ while(ci<64)C[bz[ci++]]=0; } + else { uint32_t rl=V>>10; for(uint32_t i=0;i>3)<<4)+(c?((c<0)?8:-8):0); else t=(uint32_t)(c*2)<<4; + C[bz[ci]]=bmin(0x3FFF,bmax(-0x4000,t)); ci++; } } } } +} +static void bidct(int16_t*in,int16_t*out){ int16_t t[64]; + for(int c=0;c<8;c++)for(int x=0;x<8;x++){int s=0;for(int u=0;u<8;u++)s+=in[c*8+u]*bmat[x*8+u];t[x*8+c]=(int16_t)((s+0x4000)>>15);} + for(int c=0;c<8;c++)for(int x=0;x<8;x++){int s=0;for(int u=0;u<8;u++)s+=t[c*8+u]*bmat[x*8+u];out[c*8+x]=(int16_t)bm9((s+0x4000)>>15);} } +static void byuv(int8_t y,int8_t cb,int8_t cr,int*r,int*g,int*b){ + *r=bm9(y+(((359*cr)+0x80)>>8)); *g=bm9(y+((((-88*cb)&~0x1F)+((-183*cr)&~0x07)+0x80)>>8)); *b=bm9(y+(((454*cb)+0x80)>>8)); + *r^=0x80;*g^=0x80;*b^=0x80; } +/* Beetle RGB_to_RGB555 takes uint8 params -> the ^0x80 result truncates to 0..255 + * before the round/shift. (The earlier int-param version replicated mdec.c's bug, + * so the pin falsely passed; uint8 makes it a true oracle.) */ +static int b555(uint8_t c){int v=(c+4)>>3;if(v>0x1F)v=0x1F;return v;} + +static uint16_t ref_px[256]; +static void run_ref(void){ + bset(); + int16_t Cr[64],Cb[64],Y[4][64], cr8[64],cb8[64],y8[4][64]; + bdq(Cr,BLK[0],BLKN[0],QC); bidct(Cr,cr8); + bdq(Cb,BLK[1],BLKN[1],QC); bidct(Cb,cb8); + for(int k=0;k<4;k++){ bdq(Y[k],BLK[2+k],BLKN[2+k],QY); bidct(Y[k],y8[k]); } + for(int py=0;py<16;py++)for(int px=0;px<16;px++){ + int ybn=(px>=8?1:0)+(py>=8?2:0); int x=px&7,y=py&7; + int chroma=(py>>1)*8+(px>>1); + int r,g,b; byuv((int8_t)y8[ybn][y*8+x],(int8_t)cb8[chroma],(int8_t)cr8[chroma],&r,&g,&b); + ref_px[py*16+px]=(uint16_t)(b555(r)|(b555(g)<<5)|(b555(b)<<10)); /* signed=0,bit15=0 -> xor 0 */ + } +} + +int main(void){ + make_input(); run_real(); run_ref(); + printf("real output bytes: %d (expect 512 for one 16x16 16bpp macroblock)\n", OUTN); + int diffs=0, shown=0; + for(int py=0;py<16;py++)for(int px=0;px<16;px++){ + int n=py*16+px; if(2*n+1>=OUTN) continue; + uint16_t got = OUT[2*n] | (OUT[2*n+1]<<8); + uint16_t exp = ref_px[n]; + if(got!=exp){ diffs++; if(shown<12){ printf(" px(%2d,%2d) real=0x%04X beetle=0x%04X\n",px,py,got,exp); shown++; } } + } + printf("[full-path 16bpp] pixel diffs: %d / 256\n", diffs); + printf("\n%s\n", diffs?"*** INTEGRATION DIVERGENCE ***":"FULL PATH BYTE-EXACT vs BEETLE"); + return 0; +}