PR #25791 opened by Forgejo_Fairy URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/25791 Patch URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/25791.patch
Fixes #25727. When a coded residual of `0xffff` follows a zero run, the restored residual includes a modifier of one. FFmpeg currently tests that restored value when deciding whether to clamp Rice history, whereas Apple's reference codec tests the coded value. The resulting history mismatch breaks interoperability in both directions. Preserve the coded value for the decoder's clamp test and test `x - sign_modifier` before resetting the encoder's modifier. The codec commit preserves metebalci's authorship of the attached patch. Add generated 16-bit and 24-bit FATE step signals. Each test checks the encoded MOV checksum and decoded PCM, since a round trip alone passes when both codecs share the incorrect behavior. Expected results live in `tests/ref/fate`; no additional external samples are required. Validation: - Apple's reference decoder at c38887c5c5e6 rejects packet 1 of the unpatched 16-bit output. With the fix it reproduces every source sample for both generated bit depths. - Both new FATE tests pass on AArch64 and x86-64. - Reverting either codec fix separately makes both new tests fail. - All 14 selected ALAC tests pass on AArch64: `fate-alac`, `fate-lossless-alac`, `fate-acodec-alac`, `fate-seek-acodec-alac`, and `fate-checkasm-alacdsp` with `/opt/fate-suite`. Compatibility: affected files produced by older FFmpeg encoders use the incorrect interpretation. The corrected decoder drops the affected packet in the issue's old-encoder sample. This patch does not introduce a legacy decoding option; a known-working old decoder can recover that sample to PCM for re-encoding. The separate zero-sample trailing-packet problem discussed in #21111 is outside this change. >From 987d896ca22f661096c714ae11a3515cd4946d92 Mon Sep 17 00:00:00 2001 From: metebalci <[email protected]> Date: Thu, 5 Nov 2026 23:14:38 +0000 Subject: [PATCH 1/2] avcodec/alac: clamp Rice history using the coded residual After a zero run the coded residual excludes the sign modifier. Apple's reference codec uses that coded residual to decide whether to clamp the history, while retaining the modifier in the history update itself. Testing the restored residual instead incorrectly clamps when the coded value is exactly 0xffff and the modifier is one. The resulting Rice parameters diverge and break interoperability in both directions. Keep the coded value in the decoder and test x - sign_modifier before resetting the modifier in the encoder. This changes decoding of affected streams written by older FFmpeg encoders, which use the same incorrect history update. Fixes: #25727 Assisted-by: Fairy --- libavcodec/alac.c | 11 ++++++----- libavcodec/alacenc.c | 5 +++-- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/libavcodec/alac.c b/libavcodec/alac.c index f91288e97c8..ffb40bf121e 100644 --- a/libavcodec/alac.c +++ b/libavcodec/alac.c @@ -120,7 +120,7 @@ static int rice_decompress(ALACContext *alac, int32_t *output_buffer, for (i = 0; i < nb_samples; i++) { int k; - unsigned int x; + unsigned int x, coded; if (get_bits_left(gb) <= 0) return AVERROR_INVALIDDATA; @@ -128,13 +128,14 @@ static int rice_decompress(ALACContext *alac, int32_t *output_buffer, /* calculate rice param and decode next value */ k = av_log2((history >> 9) + 3); k = FFMIN(k, alac->rice_limit); - x = decode_scalar(gb, k, bps); - x += sign_modifier; + coded = decode_scalar(gb, k, bps); + x = coded + sign_modifier; sign_modifier = 0; output_buffer[i] = (x >> 1) ^ -(x & 1); - /* update the history */ - if (x > 0xffff) + /* update the history; the reference decoder tests the coded value, + * before the zero-run modifier is added */ + if (coded > 0xffff) history = 0xffff; else history += x * rice_history_mult - diff --git a/libavcodec/alacenc.c b/libavcodec/alacenc.c index 474200fee58..e1ffb42bb6c 100644 --- a/libavcodec/alacenc.c +++ b/libavcodec/alacenc.c @@ -342,9 +342,10 @@ static void alac_entropy_coder(AlacEncodeContext *s, int ch) history += x * s->rc.history_mult - ((history * s->rc.history_mult) >> 9); - sign_modifier = 0; - if (x > 0xFFFF) + /* the reference encoder tests the coded value, x - sign_modifier */ + if (x - sign_modifier > 0xFFFF) history = 0xFFFF; + sign_modifier = 0; if (history < 128 && i < s->frame_size) { unsigned int block_size = 0; -- 2.54.0 >From c8833cee594630bf5821a2a11c3c084bf7026ff2 Mon Sep 17 00:00:00 2001 From: Michael Niedermayer <[email protected]> Date: Thu, 5 Nov 2026 23:14:38 +0000 Subject: [PATCH 2/2] fate/alac: cover the Rice history clamp after a zero run Generate the step signal from #25727 in 16- and 24-bit variants. Check both the encoded MOV checksum and the decoded PCM: a round trip alone also passes when both sides use the same incorrect clamp. Apple's reference decoder at c38887c5c5e6 decodes the generated streams exactly. Both tests fail when either codec fix is reverted separately, and pass with both fixes on AArch64 and x86-64. The signals are generated locally and require no additional FATE samples. Assisted-by: Fairy --- tests/fate/alac.mak | 20 ++++++++++++++++++++ tests/ref/fate/alac-rice-history-16 | 10 ++++++++++ tests/ref/fate/alac-rice-history-24 | 10 ++++++++++ 3 files changed, 40 insertions(+) create mode 100644 tests/ref/fate/alac-rice-history-16 create mode 100644 tests/ref/fate/alac-rice-history-24 diff --git a/tests/fate/alac.mak b/tests/fate/alac.mak index fd58703b6a8..56b42b5f02a 100644 --- a/tests/fate/alac.mak +++ b/tests/fate/alac.mak @@ -24,3 +24,23 @@ FATE_ALAC-$(call ENCDEC, ALAC, MOV, ARESAMPLE_FILTER PIPE_PROTOCOL) += $(FATE_AL FATE_SAMPLES_FFMPEG += $(FATE_ALAC-yes) fate-alac: $(FATE_ALAC-yes) + +# The step produces a coded residual of 0xffff after a zero run. Check the +# encoded bitstream as well as the decoded PCM: a round trip alone also passes +# when the encoder and decoder both clamp the Rice history incorrectly. +FATE_ALAC_RICE-$(call ALLYES, ALAC_ENCODER ALAC_DECODER MOV_MUXER MOV_DEMUXER \ + LAVFI_INDEV AEVALSRC_FILTER ATRIM_FILTER AFORMAT_FILTER \ + PCM_S16LE_DECODER PCM_S32LE_DECODER \ + ARESAMPLE_FILTER PCM_S16LE_ENCODER PCM_S24LE_ENCODER FRAMECRC_MUXER \ + FILE_PROTOCOL PIPE_PROTOCOL) += fate-alac-rice-history-16 \ + fate-alac-rice-history-24 +fate-alac-rice-history-16: SAMPLE_FMT = s16p +fate-alac-rice-history-24: SAMPLE_FMT = s32p +fate-alac-rice-history-16: PCM_CODEC = pcm_s16le +fate-alac-rice-history-24: PCM_CODEC = pcm_s24le +$(FATE_ALAC_RICE-yes): CMD = transcode lavfi \ + "aevalsrc=if(lt(n\,6080)\,-0.5\,0.5)|0:s=44100,atrim=end_sample=12288,aformat=sample_fmts=$(SAMPLE_FMT)" \ + mov "-af aresample -c:a alac" "-af aresample -c:a $(PCM_CODEC)" +$(FATE_ALAC_RICE-yes): CMP = diff +FATE_FFMPEG += $(FATE_ALAC_RICE-yes) +fate-alac: $(FATE_ALAC_RICE-yes) diff --git a/tests/ref/fate/alac-rice-history-16 b/tests/ref/fate/alac-rice-history-16 new file mode 100644 index 00000000000..c11460b2799 --- /dev/null +++ b/tests/ref/fate/alac-rice-history-16 @@ -0,0 +1,10 @@ +14de29a9d2cbafeb0038101eb6958787 *tests/data/fate/alac-rice-history-16.mov +1156 tests/data/fate/alac-rice-history-16.mov +#tb 0: 1/44100 +#media_type 0: audio +#codec_id 0: pcm_s16le +#sample_rate 0: 44100 +#channel_layout_name 0: stereo +0, 0, 0, 4096, 16384, 0x81fe00b4 +0, 4096, 4096, 4096, 16384, 0x6496e069 +0, 8192, 8192, 4096, 16384, 0x80a5003c diff --git a/tests/ref/fate/alac-rice-history-24 b/tests/ref/fate/alac-rice-history-24 new file mode 100644 index 00000000000..d3917b6929f --- /dev/null +++ b/tests/ref/fate/alac-rice-history-24 @@ -0,0 +1,10 @@ +966074420052e38656fdb0ac8205aec3 *tests/data/fate/alac-rice-history-24.mov +25732 tests/data/fate/alac-rice-history-24.mov +#tb 0: 1/44100 +#media_type 0: audio +#codec_id 0: pcm_s24le +#sample_rate 0: 44100 +#channel_layout_name 0: stereo +0, 0, 0, 4096, 24576, 0xc2a300b4 +0, 4096, 4096, 4096, 24576, 0xa6a5e069 +0, 8192, 8192, 4096, 24576, 0x40e1003c -- 2.54.0 _______________________________________________ ffmpeg-devel mailing list -- [email protected] To unsubscribe send an email to [email protected]
