[PR] avcodec/ac3dec: avoid coefficient truncation in float decoder (PR #24009)
ayoubnabil via ffmpeg-devel <[email protected]> Tue, 04 Aug 2026 17:19:35 -0000
| Newsgroups | gmane.comp.video.ffmpeg.devel |
|---|---|
| Message-ID | <178586397600.59.4076054476249170842@29965ddac10e> |
PR #24009 opened by ayoubnabil URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/24009 Patch URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/24009.patch Fixes #23684. The float decoder dequantized spectral coefficients into an int32_t buffer using mantissa >> exponent. This discarded the fractional part before conversion to float and substantially raised the dither noise floor for low-level coefficients. Store decoded coefficients as INTFLOAT and apply 2^-exponent directly in the floating-point path. Keep the fixed-point decoder unchanged, and perform coupling, rematrixing and final scaling in the native type. This also removes the now-unused fmtconvert dependency. Add a synthetic FATE test that measures the corrected dither level. Existing PCM references remain within one sample except fate-ac3-2.0, whose measured maximum difference is two samples. >From 7e452ec5bab83e6e1a88bc554f829fe38953e720 Mon Sep 17 00:00:00 2001 From: Ayoub Nabil <[email protected]> Date: Tue, 4 Aug 2026 19:13:32 +0200 Subject: [PATCH] avcodec/ac3dec: avoid coefficient truncation in float decoder The float decoder dequantized spectral coefficients into an int32_t buffer using mantissa >> exponent. This discarded the fractional part before conversion to float and substantially raised the dither noise floor for low-level coefficients. Store decoded coefficients as INTFLOAT and apply 2^-exponent directly in the floating-point path. Keep the fixed-point decoder unchanged, and perform coupling, rematrixing and final scaling in the native type. This also removes the now-unused fmtconvert dependency. Add a synthetic FATE test that measures the corrected dither level. Existing PCM references remain within one sample except fate-ac3-2.0, whose measured maximum difference is two samples. Fixes #23684. --- configure | 2 +- libavcodec/ac3dec.c | 54 ++++++++++++++++++++++++++++++++------------- libavcodec/ac3dec.h | 4 +--- tests/fate/ac3.mak | 38 ++++++++++++++++++++++++++++++- 4 files changed, 78 insertions(+), 20 deletions(-) diff --git a/configure b/configure index 8ac272a155..9c91e2000b 100755 --- a/configure +++ b/configure @@ -3094,7 +3094,7 @@ aac_decoder_select="adts_header mpeg4audio sinewin" aac_fixed_decoder_select="adts_header mpeg4audio" aac_encoder_select="audio_frame_queue lpc sinewin" aac_latm_decoder_select="aac_decoder aac_latm_parser" -ac3_decoder_select="ac3_parser ac3dsp bswapdsp fmtconvert" +ac3_decoder_select="ac3_parser ac3dsp bswapdsp" ac3_fixed_decoder_select="ac3_parser ac3dsp bswapdsp" ac3_encoder_select="ac3dsp audiodsp me_cmp" ac3_fixed_encoder_select="ac3dsp audiodsp me_cmp" diff --git a/libavcodec/ac3dec.c b/libavcodec/ac3dec.c index 6f7306ea59..de9c80b6d2 100644 --- a/libavcodec/ac3dec.c +++ b/libavcodec/ac3dec.c @@ -51,6 +51,14 @@ /** dynamic range table. converts codes to scale factors. */ static float dynamic_range_tab[256]; float ff_ac3_heavy_dynamic_range_tab[256]; +/** scale factor for each decoded exponent: 2^-exp */ +static const float scale_factors[25] = { + 0x1p-0f, 0x1p-1f, 0x1p-2f, 0x1p-3f, 0x1p-4f, + 0x1p-5f, 0x1p-6f, 0x1p-7f, 0x1p-8f, 0x1p-9f, + 0x1p-10f, 0x1p-11f, 0x1p-12f, 0x1p-13f, 0x1p-14f, + 0x1p-15f, 0x1p-16f, 0x1p-17f, 0x1p-18f, 0x1p-19f, + 0x1p-20f, 0x1p-21f, 0x1p-22f, 0x1p-23f, 0x1p-24f, +}; /* * Initialize tables at runtime. @@ -115,7 +123,6 @@ static av_cold int ac3_decode_init(AVCodecContext *avctx) #if (USE_FIXED) s->fdsp = avpriv_alloc_fixed_dsp(avctx->flags & AV_CODEC_FLAG_BITEXACT); #else - ff_fmt_convert_init(&s->fmt_conv); s->fdsp = avpriv_float_dsp_alloc(avctx->flags & AV_CODEC_FLAG_BITEXACT); #endif if (!s->fdsp) @@ -363,14 +370,22 @@ static void calc_transform_coeffs_cpl(AC3DecodeContext *s) int band_end = bin + s->cpl_band_sizes[band]; for (ch = 1; ch <= s->fbw_channels; ch++) { if (s->channel_in_cpl[ch]) { +#if USE_FIXED int cpl_coord = s->cpl_coords[ch][band] << 5; +#else + float cpl_coord = s->cpl_coords[ch][band] * (1.0f / (1 << 23)); +#endif for (bin = band_start; bin < band_end; bin++) { - s->fixed_coeffs[ch][bin] = - MULH(s->fixed_coeffs[CPL_CH][bin] * (1 << 4), cpl_coord); +#if USE_FIXED + s->coeffs[ch][bin] = + MULH(s->coeffs[CPL_CH][bin] * (1 << 4), cpl_coord); +#else + s->coeffs[ch][bin] = s->coeffs[CPL_CH][bin] * cpl_coord; +#endif } if (ch == 2 && s->phase_flags[band]) { for (bin = band_start; bin < band_end; bin++) - s->fixed_coeffs[2][bin] = -s->fixed_coeffs[2][bin]; + s->coeffs[2][bin] = -s->coeffs[2][bin]; } } } @@ -400,7 +415,7 @@ static void ac3_decode_transform_coeffs_ch(AC3DecodeContext *s, int ch_index, ma int end_freq = s->end_freq[ch_index]; uint8_t *baps = s->bap[ch_index]; int8_t *exps = s->dexps[ch_index]; - int32_t *coeffs = s->fixed_coeffs[ch_index]; + INTFLOAT *coeffs = s->coeffs[ch_index]; int dither = (ch_index == CPL_CH) || s->dither_flag[ch_index]; GetBitContext *gbc = &s->gbc; int freq; @@ -466,7 +481,11 @@ static void ac3_decode_transform_coeffs_ch(AC3DecodeContext *s, int ch_index, ma mantissa = (unsigned)get_sbits(gbc, ff_ac3_quantization_tab[bap]) << (24 - ff_ac3_quantization_tab[bap]); break; } +#if USE_FIXED coeffs[freq] = mantissa >> exps[freq]; +#else + coeffs[freq] = mantissa * scale_factors[exps[freq]]; +#endif } } @@ -482,7 +501,7 @@ static void remove_dithering(AC3DecodeContext *s) { if (!s->dither_flag[ch] && s->channel_in_cpl[ch]) { for (i = s->start_freq[CPL_CH]; i < s->end_freq[CPL_CH]; i++) { if (!s->bap[CPL_CH][i]) - s->fixed_coeffs[ch][i] = 0; + s->coeffs[ch][i] = 0; } } } @@ -500,7 +519,12 @@ static inline void decode_transform_coeffs_ch(AC3DecodeContext *s, int blk, if (CONFIG_EAC3_DECODER && !blk) ff_eac3_decode_transform_coeffs_aht_ch(s, ch); for (bin = s->start_freq[ch]; bin < s->end_freq[ch]; bin++) { - s->fixed_coeffs[ch][bin] = s->pre_mantissa[ch][bin][blk] >> s->dexps[ch][bin]; +#if USE_FIXED + s->coeffs[ch][bin] = s->pre_mantissa[ch][bin][blk] >> s->dexps[ch][bin]; +#else + s->coeffs[ch][bin] = s->pre_mantissa[ch][bin][blk] * + scale_factors[s->dexps[ch][bin]]; +#endif } } } @@ -532,7 +556,7 @@ static inline void decode_transform_coeffs(AC3DecodeContext *s, int blk) end = s->end_freq[ch]; } do - s->fixed_coeffs[ch][end] = 0; + s->coeffs[ch][end] = 0; while (++end < 256); } @@ -555,9 +579,9 @@ static void do_rematrixing(AC3DecodeContext *s) if (s->rematrixing_flags[bnd]) { bndend = FFMIN(end, ff_ac3_rematrix_band_tab[bnd + 1]); for (i = ff_ac3_rematrix_band_tab[bnd]; i < bndend; i++) { - int tmp0 = s->fixed_coeffs[1][i]; - s->fixed_coeffs[1][i] += s->fixed_coeffs[2][i]; - s->fixed_coeffs[2][i] = tmp0 - s->fixed_coeffs[2][i]; + INTFLOAT tmp0 = s->coeffs[1][i]; + s->coeffs[1][i] += s->coeffs[2][i]; + s->coeffs[2][i] = tmp0 - s->coeffs[2][i]; } } } @@ -1287,13 +1311,13 @@ static int decode_audio_block(AC3DecodeContext *s, int blk, int offset) gain = s->dynamic_range[audio_channel]; #if USE_FIXED - scale_coefs(s->transform_coeffs[ch], s->fixed_coeffs[ch], gain, 256); + scale_coefs(s->transform_coeffs[ch], s->coeffs[ch], gain, 256); #else if (s->target_level != 0) gain = gain * s->level_gain[audio_channel]; - gain *= 1.0 / 4194304.0f; - s->fmt_conv.int32_to_float_fmul_scalar(s->transform_coeffs[ch], - s->fixed_coeffs[ch], gain, 256); + gain *= 1.0f / 4194304.0f; + s->fdsp->vector_fmul_scalar(s->transform_coeffs[ch], s->coeffs[ch], + gain, 256); #endif } diff --git a/libavcodec/ac3dec.h b/libavcodec/ac3dec.h index a099264475..609acf320d 100644 --- a/libavcodec/ac3dec.h +++ b/libavcodec/ac3dec.h @@ -61,7 +61,6 @@ #include "avcodec.h" #include "bswapdsp.h" #include "get_bits.h" -#include "fmtconvert.h" #define AC3_OUTPUT_LFEON 8 @@ -83,7 +82,6 @@ typedef struct AC3DecodeContext { AVFloatDSPContext *fdsp; #endif AC3DSPContext ac3dsp; - FmtConvertContext fmt_conv; ///< optimized conversion functions ///@} AVTXContext *tx_128, *tx_256; @@ -249,7 +247,7 @@ typedef struct AC3DecodeContext { SHORTFLOAT *outptr[AC3_MAX_CHANNELS]; ///@name Aligned arrays - DECLARE_ALIGNED(16, int, fixed_coeffs)[AC3_MAX_CHANNELS][AC3_MAX_COEFS]; ///< fixed-point transform coefficients + DECLARE_ALIGNED(32, INTFLOAT, coeffs)[AC3_MAX_CHANNELS][AC3_MAX_COEFS]; ///< decoded transform coefficients DECLARE_ALIGNED(32, INTFLOAT, transform_coeffs)[AC3_MAX_CHANNELS][AC3_MAX_COEFS]; ///< transform coefficients DECLARE_ALIGNED(32, INTFLOAT, delay)[EAC3_MAX_CHANNELS][AC3_BLOCK_SIZE]; ///< delay - added to the next block DECLARE_ALIGNED(32, INTFLOAT, window)[AC3_BLOCK_SIZE]; ///< window coefficients diff --git a/tests/fate/ac3.mak b/tests/fate/ac3.mak index 875fa12418..d6f6a761fb 100644 --- a/tests/fate/ac3.mak +++ b/tests/fate/ac3.mak @@ -65,6 +65,13 @@ fate-eac3-5: REF = $(SAMPLES)/eac3/the_great_wall_7.1.pcm $(FATE_AC3) $(FATE_EAC3): CMP = oneoff +# the references were generated with the truncating dequantization +# (mantissa >> exp) that the float decoder used to share with ac3_fixed. +# the float decoder now dequantizes exactly (mantissa * 2^-exp), which +# moves its output by up to 1 s16 unit on these streams, except a measured +# MAXDIFF of 2 on fate-ac3-2.0. +fate-ac3-2.0: FUZZ = 2 + FATE_AC3-$(call PCM, AC3, AC3 AC3_FIXED, PCM_S16LE_MUXER ARESAMPLE_FILTER) += $(FATE_AC3) FATE_EAC3-$(call PCM, EAC3, EAC3, PCM_S16LE_MUXER ARESAMPLE_FILTER) += $(FATE_EAC3) @@ -102,11 +109,40 @@ fate-ac3-fixed-encode-3: tests/data/asynth-44100-6.wav fate-ac3-fixed-encode-3: SRC = $(TARGET_PATH)/tests/data/asynth-44100-6.wav fate-ac3-fixed-encode-3: CMD = framecrc -i $(SRC) -c:a ac3_fixed -flags2 +fixed_frame_size -ab 256k -af aresample,atrim=start_sample=0:end_sample=12096 +# digital silence encoded with the bitexact fixed-point encoder must decode +# to pure dither noise at the level mandated by the spec (mantissa * 2^-exp). +# The former truncating dequantization (mantissa >> exp) collapsed the dither +# mantissas to coarse {-1, 0} steps, raising the decoded noise floor by a +# factor of ~7 (stddev 18.22 instead of 2.69, tiny_psnr f32 units). +tests/data/fate/ac3-silence.ac3: TAG = GEN +tests/data/fate/ac3-silence.ac3: ffmpeg$(PROGSSUF)$(EXESUF) | tests/data/fate + $(M)$(TARGET_EXEC) $(TARGET_PATH)/$< -nostdin \ + -f lavfi -i anullsrc=r=44100:cl=stereo -t 1 \ + -c:a ac3_fixed -b:a 192k -flags +bitexact -f ac3 -y $(TARGET_PATH)/$@ 2>/dev/null + +tests/data/fate/ac3-silence.f32: TAG = GEN +tests/data/fate/ac3-silence.f32: ffmpeg$(PROGSSUF)$(EXESUF) | tests/data/fate + $(M)$(TARGET_EXEC) $(TARGET_PATH)/$< -nostdin \ + -f lavfi -i anullsrc=r=44100:cl=stereo -af atrim=end_sample=44100 \ + -f f32le -y $(TARGET_PATH)/$@ 2>/dev/null + +FATE_AC3_DITHER-$(call ALLYES, FFMPEG LAVFI_INDEV ANULLSRC_FILTER ATRIM_FILTER \ + ARESAMPLE_FILTER AC3_FIXED_ENCODER AC3_MUXER \ + AC3_DEMUXER AC3_DECODER PCM_F32LE_ENCODER \ + PCM_F32LE_MUXER FILE_PROTOCOL PIPE_PROTOCOL) += fate-ac3-float-dither +fate-ac3-float-dither: tests/data/fate/ac3-silence.ac3 tests/data/fate/ac3-silence.f32 +fate-ac3-float-dither: CMD = ffmpeg -auto_conversion_filters -cons_noisegen 1 -i $(TARGET_PATH)/tests/data/fate/ac3-silence.ac3 -af atrim=end_sample=44100 -f f32le - +fate-ac3-float-dither: CMP = stddev +fate-ac3-float-dither: CMP_UNIT = f32 +fate-ac3-float-dither: REF = tests/data/fate/ac3-silence.f32 +fate-ac3-float-dither: CMP_TARGET = 2.69 + FATE_EAC3-$(call ALLYES, EAC3_DEMUXER EAC3_MUXER EAC3_CORE_BSF) += fate-eac3-core-bsf fate-eac3-core-bsf: CMD = md5pipe -i $(TARGET_SAMPLES)/eac3/the_great_wall_7.1.eac3 -c:a copy -bsf:a eac3_core -fflags +bitexact -f eac3 fate-eac3-core-bsf: CMP = oneline fate-eac3-core-bsf: REF = b704bf851e99b7442e9bed368b60e6ca FATE_SAMPLES_AVCONV += $(FATE_AC3-yes) $(FATE_EAC3-yes) +FATE_FFMPEG += $(FATE_AC3_DITHER-yes) -fate-ac3: $(FATE_AC3-yes) $(FATE_EAC3-yes) +fate-ac3: $(FATE_AC3-yes) $(FATE_EAC3-yes) $(FATE_AC3_DITHER-yes) -- 2.52.0 _______________________________________________ ffmpeg-devel mailing list -- [email protected] To unsubscribe send an email to [email protected]