#10732(undetermined:new): avcodec_flush_buffers() not resetting E-AC-3 decoder
#10732: avcodec_flush_buffers() not resetting E-AC-3 decoder ----------------------------------------+---------------------------------- Reporter: Peter Krefting | Type: defect Status: new | Priority: minor Component: undetermined | Version: 5.0.3 Keywords: | Blocked By: Blocking: | Reproduced by developer: 0 Analyzed by developer: 0 | ----------------------------------------+---------------------------------- Summary of the bug: I am reusing the same AVCodecContext to decode several parallel E-AC-3 audio streams. As per documentation I am calling [https://ffmpeg.org/doxygen/trunk/group__lavc__misc.html#gaf60b0e076f822abcb2... avcodec_flush_buffers()] between each decoding run. This seems to work for most codecs, but when used with the E-AC-3 decoder the output is not identical between runs How to reproduce: {{{#!c #include <libavcodec/avcodec.h> #include <libavformat/avformat.h> #include <stdio.h> #include <assert.h> int main(int argc, char *argv[]) { const AVCodec *codec = avcodec_find_decoder(AV_CODEC_ID_EAC3); AVCodecContext *decoder = avcodec_alloc_context3(codec); avcodec_open2(decoder, codec, NULL); assert(decoder->sample_fmt == AV_SAMPLE_FMT_FLTP); for (int l = 0; l < 2; ++ l) { printf("[%d] Reading file\n", l); avcodec_flush_buffers(decoder); AVFormatContext *avf_ctx = NULL; avformat_open_input(&avf_ctx, "file:i28553p303t610156395819694.ac3", NULL, NULL); avformat_find_stream_info(avf_ctx, NULL); AVPacket avpkt = { 0 }; while (av_read_frame(avf_ctx, &avpkt) == 0) { avcodec_send_packet(decoder, &avpkt); AVFrame *frame = av_frame_alloc(); avcodec_receive_frame(decoder, frame); printf("Got %d samples in %d channels\n", frame->nb_samples, frame->channels); for (int i = 0; i < frame->nb_samples; ++ i) { printf("%4d:", i); for (int j = 0; j < frame->channels; ++ j) { printf(" [%d]%f", j, ((float *) frame->data[j])[i]); } printf("\n"); } av_frame_free(&frame); } avformat_close_input(&avf_ctx); } avcodec_free_context(&decoder); return 0; } }}} The test file is available here: https://e.pcloud.link/publink/show?code=XZS8y1ZlEryJIDYU0yIzDTUhw1sDfHDRtUV and contains a ten-second extract of an audio track from an MPEGTS container, captured off-air while looking at a problem where the encoder suddenly only outputs silence. I expect the output to be identical on the first and second run, but that is not what I am seeing: {{{ $ ./decode |grep -A5 'Reading file' [eac3 @ 0x55fc4b0a5ec0] Estimating duration from bitrate, this may be inaccurate [0] Reading file Got 1536 samples in 6 channels 0: [0]0.000000 [1]0.000000 [2]0.000000 [3]0.000000 [4]0.000000 [5]0.000000 1: [0]0.000000 [1]0.000000 [2]0.000000 [3]0.000000 [4]0.000000 [5]0.000000 2: [0]0.000000 [1]0.000000 [2]0.000000 [3]0.000000 [4]0.000000 [5]0.000000 3: [0]0.000000 [1]0.000000 [2]0.000000 [3]0.000000 [4]0.000000 [5]0.000000 [eac3 @ 0x55fc4b11a840] Estimating duration from bitrate, this may be inaccurate -- [1] Reading file Got 1536 samples in 6 channels 0: [0]0.000000 [1]0.000000 [2]0.000000 [3]0.000000 [4]0.000000 [5]0.000000 1: [0]-0.000000 [1]0.000000 [2]-0.000000 [3]-0.000000 [4]-0.000000 [5]-0.000000 2: [0]0.000000 [1]0.000000 [2]0.000000 [3]0.000000 [4]0.000000 [5]0.000000 3: [0]-0.000000 [1]-0.000000 [2]-0.000000 [3]-0.000000 [4]-0.000000 [5]-0.000000 }}} In real life we are also interleaving other E-AC-3 audio streams from the same MPEGTS between decodes, and see that the initial decoded samples differ even more than when just redecoding the same sample over and over. -- Ticket URL: <https://trac.ffmpeg.org/ticket/10732> FFmpeg <https://ffmpeg.org> FFmpeg issue tracker
#10732: avcodec_flush_buffers() not resetting E-AC-3 decoder -------------------------------------+------------------------------------- Reporter: Peter | Owner: (none) Krefting | Type: defect | Status: new Priority: minor | Component: | undetermined Version: 5.0.3 | Resolution: Keywords: | Blocked By: Blocking: | Reproduced by developer: 0 Analyzed by developer: 0 | -------------------------------------+------------------------------------- Comment (by Peter Krefting): We are also seeing a similar issue in an unrelated case using the HEVC decoder (AV_CODEC_ID_H265). When multiplexing several streams into the same AVCodecContext object, calling avcodec_flush_buffers() between each call, we sometimes fail to get frames decoded, but if we change to having a dedicated AVCodecContext for each of the streams it decodes fine. -- Ticket URL: <https://trac.ffmpeg.org/ticket/10732#comment:1> FFmpeg <https://ffmpeg.org> FFmpeg issue tracker
#10732: avcodec_flush_buffers() not resetting E-AC-3 decoder -------------------------------------+------------------------------------- Reporter: Peter | Owner: (none) Krefting | Type: defect | Status: new Priority: minor | Component: | undetermined Version: 5.0.3 | Resolution: Keywords: | Blocked By: Blocking: | Reproduced by developer: 0 Analyzed by developer: 0 | -------------------------------------+------------------------------------- Comment (by Peter Krefting): Also verified that the I see the same issue in the 6.1 release version. Updating the build framework is fairly heavy process, and I have not tested the latest git master. -- Ticket URL: <https://trac.ffmpeg.org/ticket/10732#comment:2> FFmpeg <https://ffmpeg.org> FFmpeg issue tracker
#10732: avcodec_flush_buffers() not resetting E-AC-3 decoder -------------------------------------+------------------------------------- Reporter: Peter | Owner: (none) Krefting | Type: defect | Status: new Priority: minor | Component: | undetermined Version: 6.1 | Resolution: Keywords: | Blocked By: Blocking: | Reproduced by developer: 0 Analyzed by developer: 0 | -------------------------------------+------------------------------------- Changes (by Peter Krefting): * version: 5.0.3 => 6.1 -- Ticket URL: <https://trac.ffmpeg.org/ticket/10732#comment:3> FFmpeg <https://ffmpeg.org> FFmpeg issue tracker
#10732: avcodec_flush_buffers() not resetting E-AC-3 decoder -------------------------------------+------------------------------------- Reporter: Peter | Owner: (none) Krefting | Type: defect | Status: new Priority: minor | Component: | undetermined Version: 6.1 | Resolution: Keywords: eac3 | Blocked By: Blocking: | Reproduced by developer: 0 Analyzed by developer: 0 | -------------------------------------+------------------------------------- Changes (by Balling): * keywords: => eac3 Comment: Is this still a problem? -- Ticket URL: <https://trac.ffmpeg.org/ticket/10732#comment:4> FFmpeg <https://ffmpeg.org> FFmpeg issue tracker
#10732: avcodec_flush_buffers() not resetting E-AC-3 decoder -------------------------------------+------------------------------------- Reporter: Peter | Owner: (none) Krefting | Type: defect | Status: new Priority: minor | Component: | undetermined Version: 6.1 | Resolution: Keywords: eac3 | Blocked By: Blocking: | Reproduced by developer: 0 Analyzed by developer: 0 | -------------------------------------+------------------------------------- Comment (by James): Does the following work for you? There may have been a reason the ac3 decoder did not include a flush() method, but perhaps it was simply forgotten. {{{ diff --git a/libavcodec/ac3dec.c b/libavcodec/ac3dec.c index 3cc20f32a9..3490e02c89 100644 --- a/libavcodec/ac3dec.c +++ b/libavcodec/ac3dec.c @@ -252,6 +252,17 @@ static av_cold int ac3_decode_init(AVCodecContext *avctx) return 0; } +static av_cold void ac3_decode_flush(AVCodecContext *avctx) +{ + AC3DecodeContext *s = avctx->priv_data; + + memset(s + offsetof(AC3DecodeContext, frame_type), 0, + sizeof(*s) - offsetof(AC3DecodeContext, frame_type)); + + AC3_RENAME(ff_kbd_window_init)(s->window, 5.0, 256); + av_lfg_init(&s->dith_state, 0); +} + /** * Parse the 'sync info' and 'bit stream info' from the AC-3 bitstream. * GetBitContext within AC3DecodeContext must point to diff --git a/libavcodec/ac3dec.h b/libavcodec/ac3dec.h index 98de7b5abf..cfaad446ab 100644 --- a/libavcodec/ac3dec.h +++ b/libavcodec/ac3dec.h @@ -75,8 +75,29 @@ typedef struct AC3DecodeContext { AVCodecContext *avctx; ///< parent context GetBitContext gbc; ///< bitstream reader + AVTXContext *tx_128, *tx_256; + av_tx_fn tx_fn_128, tx_fn_256; + +///@name Optimization + BswapDSPContext bdsp; +#if USE_FIXED + AVFixedDSPContext *fdsp; +#else + AVFloatDSPContext *fdsp; +#endif + AC3DSPContext ac3dsp; + FmtConvertContext fmt_conv; ///< optimized conversion functions +///@} + + INTFLOAT *xcfptr[AC3_MAX_CHANNELS]; + INTFLOAT *dlyptr[AC3_MAX_CHANNELS]; + + AVChannelLayout downmix_layout; + +// Start of flushable fields ///@name Bit stream information ///@{ + // frame_type must be first int frame_type; ///< frame type (strmtyp) int substreamid; ///< substream identification int superframe_size; ///< current superframe size, in bytes @@ -222,24 +243,9 @@ typedef struct AC3DecodeContext { ///@name IMDCT int block_switch[AC3_MAX_CHANNELS]; ///< block switch flags (blksw) - AVTXContext *tx_128, *tx_256; - av_tx_fn tx_fn_128, tx_fn_256; -///@} - -///@name Optimization - BswapDSPContext bdsp; -#if USE_FIXED - AVFixedDSPContext *fdsp; -#else - AVFloatDSPContext *fdsp; -#endif - AC3DSPContext ac3dsp; - FmtConvertContext fmt_conv; ///< optimized conversion functions ///@} SHORTFLOAT *outptr[AC3_MAX_CHANNELS]; - INTFLOAT *xcfptr[AC3_MAX_CHANNELS]; - INTFLOAT *dlyptr[AC3_MAX_CHANNELS]; ///@name Aligned arrays DECLARE_ALIGNED(16, int, fixed_coeffs)[AC3_MAX_CHANNELS][AC3_MAX_COEFS]; ///< fixed-point transform coefficients @@ -252,7 +258,7 @@ typedef struct AC3DecodeContext { DECLARE_ALIGNED(32, SHORTFLOAT, output_buffer)[EAC3_MAX_CHANNELS][AC3_BLOCK_SIZE * 6]; ///< final output buffer ///@} - AVChannelLayout downmix_layout; +// End of flushable fields } AC3DecodeContext; /** diff --git a/libavcodec/ac3dec_fixed.c b/libavcodec/ac3dec_fixed.c index e0db9b2260..e284140e74 100644 --- a/libavcodec/ac3dec_fixed.c +++ b/libavcodec/ac3dec_fixed.c @@ -181,6 +181,7 @@ const FFCodec ff_ac3_fixed_decoder = { .p.priv_class = &ac3_decoder_class, .priv_data_size = sizeof (AC3DecodeContext), .init = ac3_decode_init, + .flush = ac3_decode_flush, .close = ac3_decode_end, FF_CODEC_DECODE_CB(ac3_decode_frame), .p.capabilities = AV_CODEC_CAP_CHANNEL_CONF | diff --git a/libavcodec/ac3dec_float.c b/libavcodec/ac3dec_float.c index d94070bc0c..60d3472ca6 100644 --- a/libavcodec/ac3dec_float.c +++ b/libavcodec/ac3dec_float.c @@ -87,6 +87,7 @@ const FFCodec ff_eac3_decoder = { .p.id = AV_CODEC_ID_EAC3, .priv_data_size = sizeof (AC3DecodeContext), .init = ac3_decode_init, + .flush = ac3_decode_flush, .close = ac3_decode_end, FF_CODEC_DECODE_CB(ac3_decode_frame), .p.capabilities = AV_CODEC_CAP_CHANNEL_CONF | }}} -- Ticket URL: <https://trac.ffmpeg.org/ticket/10732#comment:5> FFmpeg <https://ffmpeg.org> FFmpeg issue tracker
#10732: avcodec_flush_buffers() not resetting E-AC-3 decoder -------------------------------------+------------------------------------- Reporter: Peter | Owner: (none) Krefting | Type: defect | Status: new Priority: minor | Component: | undetermined Version: 6.1 | Resolution: Keywords: eac3 | Blocked By: Blocking: | Reproduced by developer: 0 Analyzed by developer: 0 | -------------------------------------+------------------------------------- Comment (by Peter Krefting):
Can you add that sample?
The test file is linked in the bug description.
What is this, -0 and +0 issue... It is not even a real issue technically.
I should have included more of the output, when running the binary with the original library we see that there are different outputs each iteration, whereas I expect getting the same output for the same input each iteration: {{{ $ ./ffmpeg-10732 |grep ' 18: '|head -n5 [eac3 @ 0x59650c9c5bc0] Estimating duration from bitrate, this may be inaccurate 18: [0]0.000000 [1]0.000000 [2]0.000000 [3]0.000000 [4]0.000000 [5]0.000000 18: [0]0.000000 [1]-0.000002 [2]0.000000 [3]0.000000 [4]0.000000 [5]-0.000002 18: [0]0.000000 [1]-0.000002 [2]0.000000 [3]0.000000 [4]0.000000 [5]-0.000001 18: [0]0.000000 [1]-0.000002 [2]0.000000 [3]0.000000 [4]0.000000 [5]0.000000 18: [0]0.000000 [1]-0.000002 [2]0.000000 [3]0.000000 [4]0.000000 [5]0.000000 }}} I will need some more time to investigate your patch, as it doesn't apply cleanly to the version we are still on, and the API changes in the current version will require some work to integrate (including the test program from the description). -- Ticket URL: <https://trac.ffmpeg.org/ticket/10732#comment:6> FFmpeg <https://ffmpeg.org> FFmpeg issue tracker
#10732: avcodec_flush_buffers() not resetting E-AC-3 decoder -------------------------------------+------------------------------------- Reporter: Peter | Owner: (none) Krefting | Type: defect | Status: new Priority: minor | Component: | undetermined Version: 6.1 | Resolution: Keywords: eac3 | Blocked By: Blocking: | Reproduced by developer: 0 Analyzed by developer: 0 | -------------------------------------+------------------------------------- Comment (by Balling):
The test file is linked in the bug description.
You also mentioned you have a better sample. I think this should be applied. -- Ticket URL: <https://trac.ffmpeg.org/ticket/10732#comment:7> FFmpeg <https://ffmpeg.org> FFmpeg issue tracker
#10732: avcodec_flush_buffers() not resetting E-AC-3 decoder -------------------------------------+------------------------------------- Reporter: Peter | Owner: (none) Krefting | Type: defect | Status: new Priority: minor | Component: | undetermined Version: 6.1 | Resolution: Keywords: eac3 | Blocked By: Blocking: | Reproduced by developer: 0 Analyzed by developer: 0 | -------------------------------------+------------------------------------- Comment (by Peter Krefting):
You also mentioned you have a better sample. I think this should be applied.
Unfortunately not. The code that is affected and that my sample code is set to imitate runs on off-air samples and produce loudness information only, it does not store the incoming streams. The other case I mentioned was in similar code where we process multiple HEVC streams using the same decoder. In that case we detect the keyframe and pass that to the HEVC decoder to extract an image, reset the decoder, then pass another keyframe (possibly from a completely unrelated stream), extract an image, etc. I have not yet had the time to test the provided bugfix, we are currently preparing a new release, so upgrading is impractical, but it is on my to- do list. -- Ticket URL: <https://trac.ffmpeg.org/ticket/10732#comment:8> FFmpeg <https://ffmpeg.org> FFmpeg issue tracker
#10732: avcodec_flush_buffers() not resetting E-AC-3 decoder -------------------------------------+------------------------------------- Reporter: Peter | Owner: (none) Krefting | Type: defect | Status: new Priority: minor | Component: | undetermined Version: 6.1 | Resolution: Keywords: eac3 | Blocked By: Blocking: | Reproduced by developer: 0 Analyzed by developer: 0 | -------------------------------------+------------------------------------- Comment (by Peter Krefting): I am now in the process of updating the source tree (to ffmpeg 7.1). I applied the patch from comment:5. With the patch, the tester program now crashes with a segmentation fault inside av3_decode_flush(). Adding an `if (s)` around the memset() invocation is not enough to rectify this, unfortunately. To compile the tester with 7.1, I replaced `frame->channels` with `frame->ch_layout.nb_channels` (two places), otherwise the code is as in the description above. -- Ticket URL: <https://trac.ffmpeg.org/ticket/10732#comment:9> FFmpeg <https://ffmpeg.org> FFmpeg issue tracker
I am now in the process of updating the source tree (to ffmpeg 7.1). I applied the patch from comment:5. With the patch, the tester program now crashes with a segmentation fault inside av3_decode_flush(). Adding an `if (s)` around the memset() invocation is not enough to rectify this, unfortunately.
To compile the tester with 7.1, I replaced `frame->channels` with `frame->ch_layout.nb_channels` (two places), otherwise the code is as in
#10732: avcodec_flush_buffers() not resetting E-AC-3 decoder -------------------------------------+------------------------------------- Reporter: Peter | Owner: (none) Krefting | Type: defect | Status: open Priority: minor | Component: avcodec Version: git-master | Resolution: Keywords: eac3 | Blocked By: Blocking: | Reproduced by developer: 1 Analyzed by developer: 1 | -------------------------------------+------------------------------------- Changes (by James): * analyzed: 0 => 1 * component: undetermined => avcodec * reproduced: 0 => 1 * status: new => open * version: 6.1 => git-master Comment: Replying to [comment:9 Peter Krefting]: the description above. Oh, i see where i messed up. I sent a fixed version to the mailing list. Can you test it? It's in https://patchwork.ffmpeg.org/project/ffmpeg/list/?series=13875 -- Ticket URL: <https://trac.ffmpeg.org/ticket/10732#comment:10> FFmpeg <https://ffmpeg.org> FFmpeg issue tracker
#10732: avcodec_flush_buffers() not resetting E-AC-3 decoder -------------------------------------+------------------------------------- Reporter: Peter | Owner: (none) Krefting | Type: defect | Status: open Priority: minor | Component: avcodec Version: git-master | Resolution: Keywords: eac3 | Blocked By: Blocking: | Reproduced by developer: 1 Analyzed by developer: 1 | -------------------------------------+------------------------------------- Comment (by Peter Krefting): Thank you, the updated patch from patchwork does indeed fix the issue I am seeing. Is it possible to identify which codecs do not have an implementation for avcodec_flush_buffers(), we would like to re-use the objects as much as possible, for performance reasons. Clearing an object is usually cheaper than deallocation and allocating a new one. -- Ticket URL: <https://trac.ffmpeg.org/ticket/10732#comment:11> FFmpeg <https://ffmpeg.org> FFmpeg issue tracker
#10732: avcodec_flush_buffers() not resetting E-AC-3 decoder -------------------------------------+------------------------------------- Reporter: Peter | Owner: (none) Krefting | Type: defect | Status: closed Priority: normal | Component: avcodec Version: git-master | Resolution: fixed Keywords: eac3 | Blocked By: Blocking: | Reproduced by developer: 1 Analyzed by developer: 1 | -------------------------------------+------------------------------------- Changes (by James): * priority: minor => normal * resolution: => fixed * status: open => closed Comment: Replying to [comment:11 Peter Krefting]:
Thank you, the updated patch from patchwork does indeed fix the issue I am seeing.
Is it possible to identify which codecs do not have an implementation for avcodec_flush_buffers(), we would like to re-use the objects as much as possible, for performance reasons. Clearing an object is usually cheaper than deallocation and allocating a new one. All decoders by default support being flushed with that function. Those
Fixed in 045a8b15b19ec7f872fb01cfb986faeaa26cb8bb. that need it provide an internal flush() callback to clear some decoder private state. In the case of ac3/eac3 as you found out, it was most likely forgotten. If you find another decoder where flushing doesn't behave as it should, create another ticket. -- Ticket URL: <https://trac.ffmpeg.org/ticket/10732#comment:12> FFmpeg <https://ffmpeg.org> FFmpeg issue tracker
#10732: avcodec_flush_buffers() not resetting E-AC-3 decoder -------------------------------------+------------------------------------- Reporter: Peter | Owner: (none) Krefting | Type: defect | Status: closed Priority: normal | Component: avcodec Version: git-master | Resolution: fixed Keywords: eac3 | Blocked By: Blocking: | Reproduced by developer: 1 Analyzed by developer: 1 | -------------------------------------+------------------------------------- Comment (by Balling): For your information TrueHD and MLP recently added flush that fixed a terrible bug where at every seek it was warning about lossless absence. 5673a4842556b79a92a1ede6e9696506fd4161ad 034133a0df5f327aba36ee25db9452cda9e1a62b -- Ticket URL: <https://trac.ffmpeg.org/ticket/10732#comment:13> FFmpeg <https://ffmpeg.org> FFmpeg issue tracker
#10732: avcodec_flush_buffers() not resetting E-AC-3 decoder -------------------------------------+------------------------------------- Reporter: Peter | Owner: (none) Krefting | Type: defect | Status: closed Priority: normal | Component: avcodec Version: git-master | Resolution: fixed Keywords: eac3 | Blocked By: Blocking: | Reproduced by developer: 1 Analyzed by developer: 1 | -------------------------------------+------------------------------------- Comment (by Balling): Hi, can you check whether this bug is the same bug you had with your code https://trac.ffmpeg.org/ticket/11578#comment:14 -- Ticket URL: <https://trac.ffmpeg.org/ticket/10732#comment:14> FFmpeg <https://ffmpeg.org> FFmpeg issue tracker
#10732: avcodec_flush_buffers() not resetting E-AC-3 decoder -------------------------------------+------------------------------------- Reporter: Peter | Owner: (none) Krefting | Type: defect | Status: closed Priority: normal | Component: avcodec Version: git-master | Resolution: fixed Keywords: eac3 | Blocked By: Blocking: | Reproduced by developer: 1 Analyzed by developer: 1 | -------------------------------------+------------------------------------- Comment (by Peter Krefting): Replying to [comment:14 Balling]:
can you check whether this bug is the same bug you had with your code https://trac.ffmpeg.org/ticket/11578#comment:14
It does indeed look similar to what we saw. We are not playing back the decoded audio, only graphing the loudness, but the graphs showed short peaks immediately after calling the flush function and switching buffers, consistent with a "click" sound. We are only using the ffmpeg functionality for reading data, we feed the data directly to the decoder. -- Ticket URL: <https://trac.ffmpeg.org/ticket/10732#comment:15> FFmpeg <https://ffmpeg.org> FFmpeg issue tracker
#10732: avcodec_flush_buffers() not resetting E-AC-3 decoder -------------------------------------+------------------------------------- Reporter: Peter | Owner: (none) Krefting | Type: defect | Status: closed Priority: normal | Component: avcodec Version: git-master | Resolution: fixed Keywords: eac3 | Blocked By: Blocking: | Reproduced by developer: 1 Analyzed by developer: 1 | -------------------------------------+------------------------------------- Comment (by Balling): No, there if you do not seek there is a click. If you do to the start after click has played there is no click. -- Ticket URL: <https://trac.ffmpeg.org/ticket/10732#comment:16> FFmpeg <https://ffmpeg.org> FFmpeg issue tracker
participants (1)
-
FFmpeg