[FFmpeg-devel] [PATCH v2] avcodec/nvenc: fix flushing for encoder-reuse

[Jai Luthra]

On Fri, Jul 16, 2021, at 10:06 PM, Timo Rothenpieler wrote:

-- snip --

> Can you explain the logic this follows some more? I'm quite confused 
> what exactly changed, and how it broke this.
> How do you trigger a flush, if calling flush does nothing?

Since https://github.com/ffmpeg/ffmpeg/commit/3ea7057677 it was possible to flush nvenc buffers without sending a NULL frame like:

// trigger ff_nvenc_encode_flush which will send a NULL frame internally
// this doesn't set avctx->internal->draining
avcodec_flush_buffers(encoder) 

// get remaining packets out of the buffer
while ... {
    avcodec_receive_packet(encoder, &pkt)
}

// whenever next segment comes in, reuse encoder without closing & re-opening
avcodec_send_frame(encoder, frame)

But after the recent API changes [1] if someone tries to fetch packets after calling avcodec_flush_buffers() they won't receive any and hit EAGAIN.

As the user ran out of frames to encode, no frame is present at avctx->internal->buffer_frame - and avctx->internal->draining is unset because the user didn't want to send a NULL frame. So ff_encode_get_frame returns EAGAIN, which is what the user gets without these changes:

> > @@ -2384,8 +2390,11 @@ int ff_nvenc_receive_packet(AVCodecContext *avctx, AVPacket *pkt)
> >   
> >       if (!frame->buf[0]) {
> >           res = ff_encode_get_frame(avctx, frame);
> > -        if (res < 0 && res != AVERROR_EOF)
> > +        if (res == AVERROR_EOF || (ctx->encoder_flushing && res == AVERROR(EAGAIN))) {
> > +            // flushing mode, continue to send packets
> > +        } else if (res < 0) {
> >               return res;
> > +        }
> >       }

----

> >   
> >   av_cold void ff_nvenc_encode_flush(AVCodecContext *avctx)
> >   {
> > -    NvencContext *ctx = avctx->priv_data;
> > -
> >       nvenc_send_frame(avctx, NULL);
> > -    av_fifo_reset(ctx->timestamp_list);
> >   }
> 
> Is it really correct for this to do absolutely nothing now?

It's still sending the NULL frame internally - which will mark encoder_flushing as 1 here:

> > @@ -2335,6 +2340,7 @@ static int nvenc_send_frame(AVCodecContext *avctx, const AVFrame *frame)
> >           nvenc_codec_specific_pic_params(avctx, &pic_params, ctx->sei_data, sei_count);
> >       } else {
> >           pic_params.encodePicFlags = NV_ENC_PIC_FLAG_EOS;
> > +        ctx->encoder_flushing = 1;

We don't wanna cleanup ctx->timestamp_list until the user has retrieved all packets from the buffer after triggering a flush.

[1]: https://github.com/ffmpeg/ffmpeg/commit/827d6fe73d#diff-ba8b555333cbe9c4d768147d8a7abb152281257770aa314cc057a31f43460fec