From 8778797c229da984fcc457500002a3ddc60731a1 Mon Sep 17 00:00:00 2001 From: Romain TISSERAND Date: Thu, 10 Sep 2026 08:37:52 +0200 Subject: [PATCH 1/2] Declare the real FLAC block size in the synthesised STREAMINFO The header libchdr builds ahead of a FLAC stream put block_size * channels in the block-size fields, but STREAMINFO counts inter-channel samples, so for stereo it claimed twice the real maximum. dr_flac sizes its decoded-sample buffer from that value, so every FLAC decoder instance carried a buffer twice the size it needs. Over-declaring is otherwise harmless - the value only sizes that buffer and rejects frames larger than it - which is why this survived. The corrected value is exact, not merely safe: chdman picks the encoder's block size with the same function the codecs here call to derive this argument (chd_cd_flac_compressor::blocksize and chd_flac_compressor::blocksize in MAME's chdcodec.cpp), and MAME's own decoder writes block_size unmultiplied. The AVHuff path passes one channel, so it is unaffected either way. Peak heap on rv32imafc under qemu, LOWRAM_TARGET=ON: cd_cdfl 95145 -> 76329 (-18816, -19.8%) hd_flac 32530 -> 24338 ( -8192, -25.2%) Neither codec was in the RAM-budget firmware, which is how an oversized allocation in the largest per-instance buffer of any codec went unmeasured; both are added here, across all three budget targets. Every other codec's peak is unchanged to the byte. Decoded output is byte-identical across the seed corpus, the v3/v4 fixtures, three real CHDv4 images, six real discs and an AVHuff image, in the default build, with CHDR_CD_SCRATCH_BUFFER=OFF and with CHDR_LOWRAM_TARGET=ON - 108 comparisons, no difference. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01846EhHuAFk5qvxwEA5Gq6y --- src/libchdr_flac.c | 13 +++++++++++-- tests/rp2350-arm/check_budget.py | 2 ++ tests/rp2350-arm/fw.c | 4 ++++ tests/rp2350-arm/gen_embed.sh | 6 ++++-- tests/rp2350-riscv/check_budget.py | 2 ++ tests/rp2350-riscv/fw.c | 4 ++++ tests/rp2350-riscv/gen_embed.sh | 6 ++++-- tests/rv32/check_budget.py | 2 ++ tests/rv32/fw.c | 4 ++++ tests/rv32/gen_embed.sh | 6 ++++-- 10 files changed, 41 insertions(+), 8 deletions(-) diff --git a/src/libchdr_flac.c b/src/libchdr_flac.c index 0e393a75..2d1ad10e 100644 --- a/src/libchdr_flac.c +++ b/src/libchdr_flac.c @@ -248,8 +248,17 @@ int flac_decoder_reset(flac_decoder* decoder, uint32_t sample_rate, uint8_t num_ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 /* +2A: start of stream data */ }; memcpy(decoder->custom_header, s_header_template, sizeof(s_header_template)); - decoder->custom_header[0x08] = decoder->custom_header[0x0a] = (block_size*num_channels) >> 8; - decoder->custom_header[0x09] = decoder->custom_header[0x0b] = (block_size*num_channels) & 0xff; + /* STREAMINFO counts inter-channel samples, so the block size goes in as + * given - not multiplied by the channel count, which claimed twice the + * real maximum for stereo and made dr_flac allocate a decoded-sample + * buffer twice the size it needs. Over-declaring is otherwise harmless + * (the value only sizes that buffer and rejects frames larger than it), + * which is why this went unnoticed. The value is exact rather than + * merely safe: the encoder picks its block size with the same function + * the codecs here call to derive this argument. MAME's own decoder + * writes block_size too. */ + decoder->custom_header[0x08] = decoder->custom_header[0x0a] = block_size >> 8; + decoder->custom_header[0x09] = decoder->custom_header[0x0b] = block_size & 0xff; decoder->custom_header[0x12] = sample_rate >> 12; decoder->custom_header[0x13] = sample_rate >> 4; decoder->custom_header[0x14] = (sample_rate << 4) | ((num_channels - 1) << 1); diff --git a/tests/rp2350-arm/check_budget.py b/tests/rp2350-arm/check_budget.py index 1e908253..dee1b6de 100755 --- a/tests/rp2350-arm/check_budget.py +++ b/tests/rp2350-arm/check_budget.py @@ -21,9 +21,11 @@ "hd_zstd": 200_000, "hd_lzma": 80_000, "hd_huff": 60_000, + "hd_flac": 100_000, "cd_cdzl": 250_000, "cd_cdzs": 400_000, "cd_cdlz": 250_000, + "cd_cdfl": 250_000, } FAIL_RE = re.compile(r"^(\S+)\s+(?:OPEN|READ) FAILED: (.*)$") diff --git a/tests/rp2350-arm/fw.c b/tests/rp2350-arm/fw.c index e0185c65..4e995deb 100644 --- a/tests/rp2350-arm/fw.c +++ b/tests/rp2350-arm/fw.c @@ -23,9 +23,11 @@ #include "embed/hd_zstd.h" #include "embed/hd_lzma.h" #include "embed/hd_huff.h" +#include "embed/hd_flac.h" #include "embed/cd_cdzl.h" #include "embed/cd_cdzs.h" #include "embed/cd_cdlz.h" +#include "embed/cd_cdfl.h" /* ---- malloc high-water-mark wrapper (linked via --wrap) ---- */ @@ -168,9 +170,11 @@ int main(void) run_one("hd_zstd", hd_zstd_chd, hd_zstd_chd_len); run_one("hd_lzma", hd_lzma_chd, hd_lzma_chd_len); run_one("hd_huff", hd_huff_chd, hd_huff_chd_len); + run_one("hd_flac", hd_flac_chd, hd_flac_chd_len); run_one("cd_cdzl", cd_cdzl_chd, cd_cdzl_chd_len); run_one("cd_cdzs", cd_cdzs_chd, cd_cdzs_chd_len); run_one("cd_cdlz", cd_cdlz_chd, cd_cdlz_chd_len); + run_one("cd_cdfl", cd_cdfl_chd, cd_cdfl_chd_len); printf("=== done ===\n"); return 0; } diff --git a/tests/rp2350-arm/gen_embed.sh b/tests/rp2350-arm/gen_embed.sh index efad6362..83799f0d 100755 --- a/tests/rp2350-arm/gen_embed.sh +++ b/tests/rp2350-arm/gen_embed.sh @@ -12,8 +12,10 @@ mkdir -p "$OUT_DIR" # One representative CHD per decompressor the RAM-budget check cares about: # every HD/raw codec (small hunks) plus every CD sub-codec (large hunks, the -# ones that pay for a subcode decompressor on top of the base one). -FILES=(hd_zlib hd_zstd hd_lzma hd_huff cd_cdzl cd_cdzs cd_cdlz) +# ones that pay for a subcode decompressor on top of the base one). FLAC is in +# the list because its decoder holds the largest per-instance buffer of any +# codec here - leaving it out is how an oversized allocation went unnoticed. +FILES=(hd_zlib hd_zstd hd_lzma hd_huff hd_flac cd_cdzl cd_cdzs cd_cdlz cd_cdfl) for f in "${FILES[@]}"; do src="$SEEDS_DIR/${f}.chd" diff --git a/tests/rp2350-riscv/check_budget.py b/tests/rp2350-riscv/check_budget.py index f74865f6..7fe0cbd7 100755 --- a/tests/rp2350-riscv/check_budget.py +++ b/tests/rp2350-riscv/check_budget.py @@ -24,9 +24,11 @@ "hd_zstd": 200_000, "hd_lzma": 80_000, "hd_huff": 60_000, + "hd_flac": 100_000, "cd_cdzl": 250_000, "cd_cdzs": 400_000, "cd_cdlz": 250_000, + "cd_cdfl": 250_000, } FAIL_RE = re.compile(r"^(\S+)\s+(?:OPEN|READ) FAILED: (.*)$") diff --git a/tests/rp2350-riscv/fw.c b/tests/rp2350-riscv/fw.c index 53641036..1a43e604 100644 --- a/tests/rp2350-riscv/fw.c +++ b/tests/rp2350-riscv/fw.c @@ -24,9 +24,11 @@ #include "embed/hd_zstd.h" #include "embed/hd_lzma.h" #include "embed/hd_huff.h" +#include "embed/hd_flac.h" #include "embed/cd_cdzl.h" #include "embed/cd_cdzs.h" #include "embed/cd_cdlz.h" +#include "embed/cd_cdfl.h" /* ---- malloc high-water-mark wrapper (linked via --wrap) ---- */ @@ -169,9 +171,11 @@ int main(void) run_one("hd_zstd", hd_zstd_chd, hd_zstd_chd_len); run_one("hd_lzma", hd_lzma_chd, hd_lzma_chd_len); run_one("hd_huff", hd_huff_chd, hd_huff_chd_len); + run_one("hd_flac", hd_flac_chd, hd_flac_chd_len); run_one("cd_cdzl", cd_cdzl_chd, cd_cdzl_chd_len); run_one("cd_cdzs", cd_cdzs_chd, cd_cdzs_chd_len); run_one("cd_cdlz", cd_cdlz_chd, cd_cdlz_chd_len); + run_one("cd_cdfl", cd_cdfl_chd, cd_cdfl_chd_len); printf("=== done ===\n"); return 0; } diff --git a/tests/rp2350-riscv/gen_embed.sh b/tests/rp2350-riscv/gen_embed.sh index 27449b8d..2d6686e6 100755 --- a/tests/rp2350-riscv/gen_embed.sh +++ b/tests/rp2350-riscv/gen_embed.sh @@ -13,8 +13,10 @@ mkdir -p "$OUT_DIR" # One representative CHD per decompressor the RAM-budget check cares about: # every HD/raw codec (small hunks) plus every CD sub-codec (large hunks, the -# ones that pay for a subcode decompressor on top of the base one). -FILES=(hd_zlib hd_zstd hd_lzma hd_huff cd_cdzl cd_cdzs cd_cdlz) +# ones that pay for a subcode decompressor on top of the base one). FLAC is in +# the list because its decoder holds the largest per-instance buffer of any +# codec here - leaving it out is how an oversized allocation went unnoticed. +FILES=(hd_zlib hd_zstd hd_lzma hd_huff hd_flac cd_cdzl cd_cdzs cd_cdlz cd_cdfl) for f in "${FILES[@]}"; do src="$SEEDS_DIR/${f}.chd" diff --git a/tests/rv32/check_budget.py b/tests/rv32/check_budget.py index 2319ae81..c7d5808a 100755 --- a/tests/rv32/check_budget.py +++ b/tests/rv32/check_budget.py @@ -19,9 +19,11 @@ "hd_zstd": 150_000, "hd_lzma": 80_000, "hd_huff": 200_000, + "hd_flac": 100_000, "cd_cdzl": 200_000, "cd_cdzs": 320_000, "cd_cdlz": 200_000, + "cd_cdfl": 250_000, } FAIL_RE = re.compile(r"^(\S+)\s+(?:OPEN|READ) FAILED: (.*)$") diff --git a/tests/rv32/fw.c b/tests/rv32/fw.c index 81f42a0a..f1f58317 100644 --- a/tests/rv32/fw.c +++ b/tests/rv32/fw.c @@ -20,9 +20,11 @@ #include "embed/hd_zstd.h" #include "embed/hd_lzma.h" #include "embed/hd_huff.h" +#include "embed/hd_flac.h" #include "embed/cd_cdzl.h" #include "embed/cd_cdzs.h" #include "embed/cd_cdlz.h" +#include "embed/cd_cdfl.h" /* ---- malloc high-water-mark wrapper (linked via --wrap) ---- */ @@ -165,9 +167,11 @@ int main(void) run_one("hd_zstd", hd_zstd_chd, hd_zstd_chd_len); run_one("hd_lzma", hd_lzma_chd, hd_lzma_chd_len); run_one("hd_huff", hd_huff_chd, hd_huff_chd_len); + run_one("hd_flac", hd_flac_chd, hd_flac_chd_len); run_one("cd_cdzl", cd_cdzl_chd, cd_cdzl_chd_len); run_one("cd_cdzs", cd_cdzs_chd, cd_cdzs_chd_len); run_one("cd_cdlz", cd_cdlz_chd, cd_cdlz_chd_len); + run_one("cd_cdfl", cd_cdfl_chd, cd_cdfl_chd_len); printf("=== done ===\n"); return 0; } diff --git a/tests/rv32/gen_embed.sh b/tests/rv32/gen_embed.sh index d9eda5f7..4a6dd5c0 100755 --- a/tests/rv32/gen_embed.sh +++ b/tests/rv32/gen_embed.sh @@ -12,8 +12,10 @@ mkdir -p "$OUT_DIR" # One representative CHD per decompressor the RAM-budget check cares about: # every HD/raw codec (small hunks) plus every CD sub-codec (large hunks, the -# ones that pay for a subcode decompressor on top of the base one). -FILES=(hd_zlib hd_zstd hd_lzma hd_huff cd_cdzl cd_cdzs cd_cdlz) +# ones that pay for a subcode decompressor on top of the base one). FLAC is in +# the list because its decoder holds the largest per-instance buffer of any +# codec here - leaving it out is how an oversized allocation went unnoticed. +FILES=(hd_zlib hd_zstd hd_lzma hd_huff hd_flac cd_cdzl cd_cdzs cd_cdlz cd_cdfl) for f in "${FILES[@]}"; do src="$SEEDS_DIR/${f}.chd" From 72e597a1846660cbdca889223a78fecbaf016777 Mon Sep 17 00:00:00 2001 From: Romain TISSERAND Date: Thu, 10 Sep 2026 09:11:35 +0200 Subject: [PATCH 2/2] Generate the raw-FLAC corpus seed the budget firmware needs hd_default happens to contain flac hunks because flac is one of chdman's default hard-disk codecs, but relying on a default to cover a codec is how a codec stops being covered. The RAM-budget firmware needs the seed by name. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01846EhHuAFk5qvxwEA5Gq6y --- tests/corpus/generate.sh | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/corpus/generate.sh b/tests/corpus/generate.sh index a872928c..c9985b0f 100755 --- a/tests/corpus/generate.sh +++ b/tests/corpus/generate.sh @@ -76,6 +76,11 @@ create_hd hd_lzma.chd -c lzma create_hd hd_huff.chd -c huff create_hd hd_zstd.chd -c zstd create_hd hd_multi.chd -c zlib,lzma,huff,zstd +# The raw FLAC codec, on its own. hd_default happens to include flac hunks +# because that is one of chdman's default hard-disk codecs, but relying on a +# default to cover a codec is how a codec stops being covered - and the FLAC +# decoder holds the largest per-instance buffer of any codec here. +create_hd hd_flac.chd -c flac # CD-ROM: default + per-codec. create_cd cd_default.chd