PR #25443 opened by Jun Zhao (mypopydev) URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/25443 Patch URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/25443.patch
This series fixes issue #25430: 1. avcodec/ac3dec: preserve AVOptions across flush: Move all AVOption fields in AC3DecodeContext above frame_type so ac3_decode_flush() does not zero user-set options (such as drc_scale) when flushing. 2. Add a fate-api-ac3-flush regression test to verify that ac3, eac3, and ac3_fixed preserve their options across avcodec_flush_buffers(). >From e0a9318c3aaf93144876bd3531fc35b90b6d9b47 Mon Sep 17 00:00:00 2001 From: Jun Zhao <[email protected]> Date: Sun, 25 Oct 2026 08:27:39 +0800 Subject: [PATCH 1/2] avcodec/ac3dec: preserve AVOptions across flush ac3_decode_flush() clears every field from frame_type to the end of AC3DecodeContext with a single memset. Several fields backing AVOptions (drc_scale, heavy_compression, target_level and consistent_noise_generation, plus the exported downmix fields) were located in that range, so avcodec_flush_buffers() silently zeroed them. For drc_scale that does not even restore the default of 1.0, effectively disabling dynamic range compression after any flush, e.g. on seek. Move all AVOption fields above the start of the flushable fields, next to downmix_layout which was already kept there, so that flush only resets actual decoding state. The exported downmix fields are written from the frame header for every frame, so preserving them across flush does not change decoding. Fixes: https://code.ffmpeg.org/FFmpeg/FFmpeg/issues/25430 Signed-off-by: Jun Zhao <[email protected]> --- libavcodec/ac3dec.h | 32 +++++++++++++++++++------------- 1 file changed, 19 insertions(+), 13 deletions(-) diff --git a/libavcodec/ac3dec.h b/libavcodec/ac3dec.h index 25ce1b1c90e..3c98393c6cf 100644 --- a/libavcodec/ac3dec.h +++ b/libavcodec/ac3dec.h @@ -93,8 +93,26 @@ typedef struct AC3DecodeContext { AVChannelLayout downmix_layout; SHORTFLOAT *downmix_coeffs[2]; ///< stereo downmix coefficients +///@name AVOptions +/// Fields backing AVOptions. They hold user configuration (or, for the +/// exported downmix fields, values derived from the last decoded frame) and +/// must survive avcodec_flush_buffers(), so they must stay above the start +/// of the flushable fields below. +///@{ + int preferred_stereo_downmix; + float ltrt_center_mix_level; + float ltrt_surround_mix_level; + float loro_center_mix_level; + float loro_surround_mix_level; + int target_level; ///< target level in dBFS + INTFLOAT drc_scale; ///< percentage of dynamic range compression to be applied + int heavy_compression; ///< apply heavy compression + int consistent_noise_generation; ///< seed noise generation with AC-3 frame on decode +///@} + // Start of flushable fields. -// frame_type must be the flushable field, or the offset changed in ac3_decode_flush(). +// frame_type must be the first flushable field, or the offset changed in +// ac3_decode_flush(). No AVOption field may be placed at or below this line. ///@name Bit stream information ///@{ @@ -127,12 +145,6 @@ typedef struct AC3DecodeContext { int dolby_headphone_mode; ///< dolby headphone mode (dheadphonmod) ///@} - int preferred_stereo_downmix; - float ltrt_center_mix_level; - float ltrt_surround_mix_level; - float loro_center_mix_level; - float loro_surround_mix_level; - int target_level; ///< target level in dBFS float level_gain[2]; ///@name Frame syntax parameters @@ -194,8 +206,6 @@ typedef struct AC3DecodeContext { ///@name Dynamic range INTFLOAT dynamic_range[2]; ///< dynamic range - INTFLOAT drc_scale; ///< percentage of dynamic range compression to be applied - int heavy_compression; ///< apply heavy compression INTFLOAT heavy_dynamic_range[2]; ///< heavy dynamic range compression ///@} @@ -204,10 +214,6 @@ typedef struct AC3DecodeContext { int end_freq[AC3_MAX_CHANNELS]; ///< end frequency bin (endmant) ///@} -///@name Consistent noise generation - int consistent_noise_generation; ///< seed noise generation with AC-3 frame on decode -///@} - ///@name Rematrixing int num_rematrixing_bands; ///< number of rematrixing bands (nrematbnd) int rematrixing_flags[4]; ///< rematrixing flags (rematflg) -- 2.54.0 >From 63834db561c059ac3d644c494ddf5277a8fb9194 Mon Sep 17 00:00:00 2001 From: Jun Zhao <[email protected]> Date: Sun, 25 Oct 2026 08:46:34 +0800 Subject: [PATCH 2/2] tests/api: add a test for AC-3 flush preserving options Set non-default values for the AC-3, E-AC-3 and fixed-point AC-3 decoder options, decode a frame so that the exported downmix fields hold values set from the frame header, flush with avcodec_flush_buffers(), and check that all values are still readable afterwards. This is a regression test for the decoders forgetting options such as drc_scale on flush. Signed-off-by: Jun Zhao <[email protected]> --- tests/api/Makefile | 1 + tests/api/api-ac3-flush-test.c | 343 +++++++++++++++++++++++++++++++++ tests/fate/api.mak | 5 + 3 files changed, 349 insertions(+) create mode 100644 tests/api/api-ac3-flush-test.c diff --git a/tests/api/Makefile b/tests/api/Makefile index 7da5f6da32b..c411e82e5f1 100644 --- a/tests/api/Makefile +++ b/tests/api/Makefile @@ -1,3 +1,4 @@ +APITESTPROGS-$(call ALLYES, AC3_ENCODER AC3_DECODER EAC3_DECODER AC3_FIXED_DECODER) += api-ac3-flush APITESTPROGS-$(call ALLYES, LIBASTCENC_ENCODER LIBASTCENC_DECODER) += api-astc-profile APITESTPROGS-$(call ENCDEC, FLAC, FLAC) += api-flac APITESTPROGS-$(call DEMDEC, H264, H264) += api-h264 diff --git a/tests/api/api-ac3-flush-test.c b/tests/api/api-ac3-flush-test.c new file mode 100644 index 00000000000..6b72adc94b3 --- /dev/null +++ b/tests/api/api-ac3-flush-test.c @@ -0,0 +1,343 @@ +/* + * Copyright (c) 2026 Jun Zhao <[email protected]> + * + * This file is part of FFmpeg. + * + * FFmpeg is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * FFmpeg is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with FFmpeg; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + */ + +/* + * Check that avcodec_flush_buffers() preserves the AVOptions of the + * AC-3, E-AC-3 and fixed-point AC-3 decoders. + */ + +#include <inttypes.h> +#include <math.h> +#include <stdio.h> +#include <string.h> + +#include "libavcodec/avcodec.h" +#include "libavutil/channel_layout.h" +#include "libavutil/error.h" +#include "libavutil/frame.h" +#include "libavutil/opt.h" +#include "libavutil/samplefmt.h" + +static int check_double(AVCodecContext *ctx, const char *name, double expected) +{ + double value = -12345.0; + int ret = av_opt_get_double(ctx->priv_data, name, 0, &value); + + if (ret < 0) { + fprintf(stderr, "%s: av_opt_get_double(\"%s\") failed: %s\n", + ctx->codec->name, name, av_err2str(ret)); + return 1; + } + if (fabs(value - expected) > 1e-6) { + fprintf(stderr, "%s: %s = %g, expected %g\n", + ctx->codec->name, name, value, expected); + return 1; + } + return 0; +} + +static int check_int(AVCodecContext *ctx, const char *name, int64_t expected) +{ + int64_t value = -12345; + int ret = av_opt_get_int(ctx->priv_data, name, 0, &value); + + if (ret < 0) { + fprintf(stderr, "%s: av_opt_get_int(\"%s\") failed: %s\n", + ctx->codec->name, name, av_err2str(ret)); + return 1; + } + if (value != expected) { + fprintf(stderr, "%s: %s = %"PRId64", expected %"PRId64"\n", + ctx->codec->name, name, value, expected); + return 1; + } + return 0; +} + +static int check_downmix(AVCodecContext *ctx) +{ + AVChannelLayout layout = { 0 }; + char buf[64]; + int ret = av_opt_get_chlayout(ctx->priv_data, "downmix", 0, &layout); + + if (ret < 0) { + fprintf(stderr, "%s: av_opt_get_chlayout(\"downmix\") failed: %s\n", + ctx->codec->name, av_err2str(ret)); + return 1; + } + av_channel_layout_describe(&layout, buf, sizeof(buf)); + av_channel_layout_uninit(&layout); + if (strcmp(buf, "stereo") != 0) { + fprintf(stderr, "%s: downmix = %s, expected stereo\n", + ctx->codec->name, buf); + return 1; + } + return 0; +} + +static int check_options(AVCodecContext *ctx, int has_target_level) +{ + int ret = 0; + + ret |= check_double(ctx, "drc_scale", 0.5); + ret |= check_int(ctx, "cons_noisegen", 1); + ret |= check_int(ctx, "heavy_compr", 1); + if (has_target_level) + ret |= check_int(ctx, "target_level", -20); + ret |= check_downmix(ctx); + return ret; +} + +struct ExportValues { + int64_t dmix_mode; + double ltrt_cmixlev; + double ltrt_surmixlev; + double loro_cmixlev; + double loro_surmixlev; +}; + +static int get_export_values(AVCodecContext *ctx, struct ExportValues *values) +{ + if (!av_opt_find(ctx->priv_data, "dmix_mode", NULL, 0, 0)) + return 0; + if (av_opt_get_int(ctx->priv_data, "dmix_mode", 0, + &values->dmix_mode) < 0 || + av_opt_get_double(ctx->priv_data, "ltrt_cmixlev", 0, + &values->ltrt_cmixlev) < 0 || + av_opt_get_double(ctx->priv_data, "ltrt_surmixlev", 0, + &values->ltrt_surmixlev) < 0 || + av_opt_get_double(ctx->priv_data, "loro_cmixlev", 0, + &values->loro_cmixlev) < 0 || + av_opt_get_double(ctx->priv_data, "loro_surmixlev", 0, + &values->loro_surmixlev) < 0) + return 1; + return 0; +} + +static int generate_ac3_packet(AVPacket *pkt) +{ + const AVCodec *codec = avcodec_find_encoder(AV_CODEC_ID_AC3); + AVCodecContext *ctx = NULL; + AVFrame *frame = NULL; + int ret = AVERROR_ENCODER_NOT_FOUND; + + if (!codec) + return ret; + + ctx = avcodec_alloc_context3(codec); + frame = av_frame_alloc(); + if (!ctx || !frame) { + ret = AVERROR(ENOMEM); + goto end; + } + + ctx->sample_rate = 48000; + ctx->bit_rate = 128000; + ctx->sample_fmt = AV_SAMPLE_FMT_FLTP; + ctx->time_base = (AVRational){ 1, ctx->sample_rate }; + if ((ret = av_channel_layout_copy(&ctx->ch_layout, + &(AVChannelLayout)AV_CHANNEL_LAYOUT_STEREO)) < 0) + goto end; + if ((ret = avcodec_open2(ctx, codec, NULL)) < 0) + goto end; + + frame->nb_samples = ctx->frame_size; + frame->format = ctx->sample_fmt; + frame->pts = 0; + if ((ret = av_channel_layout_copy(&frame->ch_layout, &ctx->ch_layout)) < 0) + goto end; + if ((ret = av_frame_get_buffer(frame, 0)) < 0 || + (ret = av_frame_make_writable(frame)) < 0) + goto end; + for (int ch = 0; ch < ctx->ch_layout.nb_channels; ch++) { + float *samples = (float *)frame->extended_data[ch]; + for (int i = 0; i < frame->nb_samples; i++) + samples[i] = 0.25f * ((i % 64) / 32.0f - 1.0f) * (ch ? -1.0f : 1.0f); + } + + if ((ret = avcodec_send_frame(ctx, frame)) < 0) + goto end; + ret = avcodec_receive_packet(ctx, pkt); + if (ret < 0) + goto end; + + /* Drain the encoder so that closing it stays silent. */ + avcodec_send_frame(ctx, NULL); + { + AVPacket *tmp = av_packet_alloc(); + + if (!tmp) { + ret = AVERROR(ENOMEM); + goto end; + } + while (avcodec_receive_packet(ctx, tmp) == 0) + av_packet_unref(tmp); + av_packet_free(&tmp); + } + ret = 0; +end: + av_frame_free(&frame); + avcodec_free_context(&ctx); + return ret; +} + +static int decode_one_frame(AVCodecContext *ctx, const AVPacket *src_pkt) +{ + AVPacket *pkt = av_packet_alloc(); + AVFrame *frame = av_frame_alloc(); + int ret; + + if (!pkt || !frame) { + ret = AVERROR(ENOMEM); + goto end; + } + if ((ret = av_packet_ref(pkt, src_pkt)) < 0) + goto end; + ret = avcodec_send_packet(ctx, pkt); + av_packet_unref(pkt); + if (ret < 0) + goto end; + ret = avcodec_receive_frame(ctx, frame); +end: + av_packet_free(&pkt); + av_frame_free(&frame); + return ret; +} + +static int check_export_values(AVCodecContext *ctx, + const struct ExportValues *expected) +{ + int ret = 0; + + if (!av_opt_find(ctx->priv_data, "dmix_mode", NULL, 0, 0)) + return 0; + ret |= check_int(ctx, "dmix_mode", expected->dmix_mode); + ret |= check_double(ctx, "ltrt_cmixlev", expected->ltrt_cmixlev); + ret |= check_double(ctx, "ltrt_surmixlev", expected->ltrt_surmixlev); + ret |= check_double(ctx, "loro_cmixlev", expected->loro_cmixlev); + ret |= check_double(ctx, "loro_surmixlev", expected->loro_surmixlev); + return ret; +} + +static int run_test(const char *name, const AVPacket *pkt) +{ + const AVCodec *codec = avcodec_find_decoder_by_name(name); + AVCodecContext *ctx; + struct ExportValues export_values = { 0 }; + int has_target_level; + int ret = 1; + + if (!codec) + return 0; + + ctx = avcodec_alloc_context3(codec); + if (!ctx) + return 1; + ctx->pkt_timebase = (AVRational){ 1, 48000 }; + + has_target_level = !!av_opt_find(ctx->priv_data, "target_level", NULL, 0, 0); + + if (av_opt_set_double(ctx->priv_data, "drc_scale", 0.5, 0) < 0 || + av_opt_set_int(ctx->priv_data, "cons_noisegen", 1, 0) < 0 || + av_opt_set_int(ctx->priv_data, "heavy_compr", 1, 0) < 0 || + (has_target_level && + av_opt_set_int(ctx->priv_data, "target_level", -20, 0) < 0) || + av_opt_set(ctx->priv_data, "downmix", "stereo", 0) < 0) { + fprintf(stderr, "%s: failed to set decoder options\n", name); + goto end; + } + + if (avcodec_open2(ctx, codec, NULL) < 0) { + fprintf(stderr, "%s: avcodec_open2() failed\n", name); + goto end; + } + + if (check_options(ctx, has_target_level)) + goto end; + + /* Decode a frame first so that the exported downmix fields hold + * values set from the frame header. Checking them across a flush + * is only meaningful once they differ from their initial values. */ + if (decode_one_frame(ctx, pkt) < 0) { + fprintf(stderr, "%s: failed to decode a frame\n", name); + goto end; + } + if (get_export_values(ctx, &export_values)) + goto end; + if (av_opt_find(ctx->priv_data, "dmix_mode", NULL, 0, 0) && + export_values.ltrt_cmixlev == 0.0 && + export_values.ltrt_surmixlev == 0.0 && + export_values.loro_cmixlev == 0.0 && + export_values.loro_surmixlev == 0.0) { + fprintf(stderr, "%s: exported downmix fields were not set by decoding\n", + name); + goto end; + } + + avcodec_flush_buffers(ctx); + if (check_options(ctx, has_target_level) || + check_export_values(ctx, &export_values)) + goto end; + + /* Flushing twice must not change anything either. */ + avcodec_flush_buffers(ctx); + if (check_options(ctx, has_target_level) || + check_export_values(ctx, &export_values)) + goto end; + + printf("%s: options preserved across avcodec_flush_buffers()\n", name); + ret = 0; +end: + avcodec_free_context(&ctx); + return ret; +} + +int main(void) +{ + static const char *const decoder_names[] = { "ac3", "eac3", "ac3_fixed" }; + AVPacket *pkt = av_packet_alloc(); + int tested = 0; + int ret; + + if (!pkt) + return 1; + ret = generate_ac3_packet(pkt); + if (ret < 0) { + fprintf(stderr, "failed to generate an AC-3 packet: %s\n", + av_err2str(ret)); + av_packet_free(&pkt); + return 1; + } + + ret = 0; + for (int i = 0; i < (int)(sizeof(decoder_names) / sizeof(decoder_names[0])); i++) { + if (!avcodec_find_decoder_by_name(decoder_names[i])) + continue; + tested++; + ret |= run_test(decoder_names[i], pkt); + } + av_packet_free(&pkt); + + if (!tested) { + fprintf(stderr, "no (E-)AC-3 decoder available\n"); + return 1; + } + return ret; +} diff --git a/tests/fate/api.mak b/tests/fate/api.mak index 7b90e3dd3b8..1659c7979c0 100644 --- a/tests/fate/api.mak +++ b/tests/fate/api.mak @@ -1,3 +1,8 @@ +FATE_API_LIBAVCODEC-$(call ALLYES, AC3_ENCODER AC3_DECODER EAC3_DECODER AC3_FIXED_DECODER) += fate-api-ac3-flush +fate-api-ac3-flush: $(APITESTSDIR)/api-ac3-flush-test$(EXESUF) +fate-api-ac3-flush: CMD = run $(APITESTSDIR)/api-ac3-flush-test$(EXESUF) +fate-api-ac3-flush: CMP = null + FATE_API_LIBAVCODEC-$(call ALLYES, LIBASTCENC_ENCODER LIBASTCENC_DECODER) += fate-api-astc-profile fate-api-astc-profile: $(APITESTSDIR)/api-astc-profile-test$(EXESUF) fate-api-astc-profile: CMD = run $(APITESTSDIR)/api-astc-profile-test$(EXESUF) -- 2.54.0 _______________________________________________ ffmpeg-devel mailing list -- [email protected] To unsubscribe send an email to [email protected]
