From 2312ae4d10e10e315d12797b78155dbc0bc56cea Mon Sep 17 00:00:00 2001 From: Ayoub Nabil <237098474+ayoubnabil@users.noreply.github.com> 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 | 53 +++++++++++++++++++++++++++++---------------- libavcodec/ac3dec.h | 4 +--- tests/fate/ac3.mak | 40 ++++++++++++++++++++++++++++++++-- 4 files changed, 74 insertions(+), 25 deletions(-) diff --git a/configure b/configure index f2896b81c7..e2be2ee397 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 9f85446bf4..79a8751154 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]; } } } @@ -390,13 +405,13 @@ typedef struct mant_groups { int b4; } mant_groups; -static av_always_inline int dequantize_coeff(int mantissa, int exponent, - int coeff_bits) +static av_always_inline INTFLOAT dequantize_coeff(int mantissa, int exponent, + int coeff_bits) { #if USE_FIXED return (mantissa * (1 << coeff_bits)) >> exponent; #else - return mantissa >> exponent; + return mantissa * scale_factors[exponent]; #endif } @@ -420,7 +435,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]; #if USE_FIXED int coeff_bits = fixed_coeff_bits(s); @@ -516,7 +531,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; } } } @@ -534,7 +549,7 @@ 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] = dequantize_coeff( + s->coeffs[ch][bin] = dequantize_coeff( s->pre_mantissa[ch][bin][blk], s->dexps[ch][bin], 0); } } @@ -567,7 +582,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); } @@ -590,9 +605,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]; } } } @@ -1326,16 +1341,16 @@ static int decode_audio_block(AC3DecodeContext *s, int blk, int offset) #if USE_FIXED if (fixed_coeff_bits(s)) - scale_coefs_q2(s->transform_coeffs[ch], s->fixed_coeffs[ch], gain, + scale_coefs_q2(s->transform_coeffs[ch], s->coeffs[ch], gain, 256); else - 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 c0e6f683ef..25ce1b1c90 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 #if USE_FIXED diff --git a/tests/fate/ac3.mak b/tests/fate/ac3.mak index 70aa4f4568..c579b65e69 100644 --- a/tests/fate/ac3.mak +++ b/tests/fate/ac3.mak @@ -71,6 +71,14 @@ fate-eac3-fixed-dependent-substream: REF = $(SAMPLES)/eac3/the_great_wall_7.1.pc $(FATE_AC3) $(FATE_EAC3) $(FATE_EAC3_FIXED): 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 and fate-ac3-5.1. +fate-ac3-2.0: FUZZ = 2 +fate-ac3-5.1: 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) FATE_EAC3-$(call PCM, EAC3, EAC3 AC3_FIXED, PCM_S16LE_MUXER ARESAMPLE_FILTER) += $(FATE_EAC3_FIXED) @@ -131,12 +139,40 @@ fate-ac3-fixed-dexp24: CMD = framecrc -auto_conversion_filters -c ac3_fixed \ -i $(TARGET_PATH)/tests/data/fate/ac3-fixed-dexp24.ac3 \ -af atrim=start_sample=264192 +# 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_FIXED_DEXP24-yes) +FATE_FFMPEG += $(FATE_AC3_DITHER-yes) $(FATE_AC3_FIXED_DEXP24-yes) -fate-ac3: $(FATE_AC3-yes) $(FATE_EAC3-yes) $(FATE_AC3_FIXED_DEXP24-yes) +fate-ac3: $(FATE_AC3-yes) $(FATE_EAC3-yes) $(FATE_AC3_DITHER-yes) $(FATE_AC3_FIXED_DEXP24-yes)