[FFmpeg-devel] nvenc: implement flush to help allow an encoder to be re-used

It can be useful to re-use an encoder instance when doing segmented
encodings, and this requires flushing the encoder at the start of
each segment.
---
 libavcodec/nvenc.c      | 5 +++++
 libavcodec/nvenc.h      | 2 ++
 libavcodec/nvenc_h264.c | 1 +
 libavcodec/nvenc_hevc.c | 1 +
 4 files changed, 9 insertions(+)

On Fri, 20 Dec 2019 at 15:34, Philip Langdale <philipl@overt.org> wrote:
>
> It can be useful to re-use an encoder instance when doing segmented
> encodings, and this requires flushing the encoder at the start of
> each segment.
> ---
>  libavcodec/nvenc.c      | 5 +++++
>  libavcodec/nvenc.h      | 2 ++
>  libavcodec/nvenc_h264.c | 1 +
>  libavcodec/nvenc_hevc.c | 1 +
>  4 files changed, 9 insertions(+)
>
> diff --git a/libavcodec/nvenc.c b/libavcodec/nvenc.c
> index 310e30805d..9a96bf2bba 100644
> --- a/libavcodec/nvenc.c
> +++ b/libavcodec/nvenc.c
> @@ -2262,3 +2262,8 @@ int ff_nvenc_encode_frame(AVCodecContext *avctx, AVPacket *pkt,
>
>      return 0;
>  }
> +
> +av_cold void ff_nvenc_encode_flush(AVCodecContext *avctx)
> +{
> +    ff_nvenc_send_frame(avctx, NULL);

One concern I had was about the long-term stability of this behavior.
Right now, it works, but perhaps only coincidentally? Being flushable
and resumable like this isn't explicitly part of the "contract" for
nvenc, as far as I can see. Could future changes inadvertently
introduce state that isn't reset in between flushes, breaking the
resumable behavior? If so, is there a way to safeguard against that?

Josh

On Fri, 20 Dec 2019 16:07:18 -0800
Josh Allmann <joshua.allmann@gmail.com> wrote:

> One concern I had was about the long-term stability of this behavior.
> Right now, it works, but perhaps only coincidentally? Being flushable
> and resumable like this isn't explicitly part of the "contract" for
> nvenc, as far as I can see. Could future changes inadvertently
> introduce state that isn't reset in between flushes, breaking the
> resumable behavior? If so, is there a way to safeguard against that?
> 
> Josh

So, the behaviour at the ffmpeg level is something you can view as
stable. If it was to break, I'd expect us to fix it. For nvenc itself,
that's harder to make any statements about. I wouldn't expect the
nvidia folks to change thing casually, but until they document a
specific flush behaviour, there's always going to be a risk -
ultimately we just have to react if they change something.

In an ideal world, you'd have a test running for this, but we're not
set up to exercise any hwaccels in our automated fate executions.

Did this form of the patch work for you?

--phil

On Mon, 30 Dec 2019 at 16:40, Philip Langdale <philipl@overt.org> wrote:
>
> On Sat, 21 Dec 2019 14:54:38 -0800
> Philip Langdale <philipl@overt.org> wrote:
>
> > On Fri, 20 Dec 2019 16:07:18 -0800
> > Josh Allmann <joshua.allmann@gmail.com> wrote:
> >
> > > One concern I had was about the long-term stability of this
> > > behavior. Right now, it works, but perhaps only coincidentally?
> > > Being flushable and resumable like this isn't explicitly part of
> > > the "contract" for nvenc, as far as I can see. Could future changes
> > > inadvertently introduce state that isn't reset in between flushes,
> > > breaking the resumable behavior? If so, is there a way to safeguard
> > > against that?
> > >
> > > Josh
> >
> > So, the behaviour at the ffmpeg level is something you can view as
> > stable. If it was to break, I'd expect us to fix it. For nvenc itself,
> > that's harder to make any statements about. I wouldn't expect the
> > nvidia folks to change thing casually, but until they document a
> > specific flush behaviour, there's always going to be a risk -
> > ultimately we just have to react if they change something.
> >

Hi Phil,

Flushing and resumption is documented/supported in nvenc via
NV_ENC_FLAGS_EOS, but I wasn't sure if this was a feature that
ffmpeg's integration was intentionally designed for. But if you
confirm we can expect this behavior to be supported going forward,
then that's great news.

> > In an ideal world, you'd have a test running for this, but we're not
> > set up to exercise any hwaccels in our automated fate executions.
> >

We do have internal tests [1]  that should catch the issue if anything
changes, so that might be of some help as well, although we currently
only update ffmpeg on an as-needed basis.

[1] https://github.com/livepeer/lpms/blob/master/ffmpeg/nvidia_test.go

> > Did this form of the patch work for you?
> >
>
> Hi Josh,
>
> Did you get a chance to try it?
>
> --phil

Was delayed on testing this due to the holidays, my apologies.

Can confirm that this patch works very nicely in conjunction with
avcodec_flush_buffers . Thanks so much!

Josh

On Wed, 8 Jan 2020 14:31:05 -0800
Josh Allmann <joshua.allmann@gmail.com> wrote:
> 
> Hi Phil,
> 
> Flushing and resumption is documented/supported in nvenc via
> NV_ENC_FLAGS_EOS, but I wasn't sure if this was a feature that
> ffmpeg's integration was intentionally designed for. But if you
> confirm we can expect this behavior to be supported going forward,
> then that's great news.

Yeah. There's nothing weird about this usage - it's just uncommon.
 
> 
> We do have internal tests [1]  that should catch the issue if anything
> changes, so that might be of some help as well, although we currently
> only update ffmpeg on an as-needed basis.
> 
> [1] https://github.com/livepeer/lpms/blob/master/ffmpeg/nvidia_test.go

That's good to know. Hopefully you'll let us know if anything breaks
down the line. :-)
 
> 
> Can confirm that this patch works very nicely in conjunction with
> avcodec_flush_buffers . Thanks so much!

Thanks. I've pushed it.

--phil