avcodec_flush_buffers() not resetting E-AC-3 decoder

[Peter Krefting]

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 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:

#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.

[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.

[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.

[Balling]

Is this still a problem?

and see that the initial decoded samples

Can you add that sample? What is this, -0 and +0 issue... It is not even a real issue technically.

[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 |

[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).

[Balling]

The test file is linked in the bug description.

You also mentioned you have a better sample. I think this should be applied.

[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.