[FFmpeg-devel] [PATCH v3] avcodec: Add explicit capability flag for encoder flushing

[Anton Khirnov]

Quoting Philip Langdale (2020-04-11 01:47:43)
> We've been in this fuzzy situation where maybe you could call
> avcodec_flush_buffers() on an encoder but there weren't any encoders
> that supported it except maybe audiotoolboxenc. Then we added flush
> support to nvenc very intentionally, and it worked, but that was more a
> coincidence than anything else. And if you call avcodec_flush_buffers()
> on an encoder that doesn't support it, it'll leave the encoder in an
> undefined state, so that's not great.
> 
> As part of cleaning this up, this change introduces a formal capability
> flag for encoders that support flushing and ensures a flush call is a
> no-op for any other encoder. This allows client code to check if it is
> meaningful to call flush on an encoder before actually doing it.
> 
> I have not attempted to separate the steps taken inside
> avcodec_flush_buffers() because it's not doing anything that's wrong
> for an encoder. But I did add a sanity check to reject attempts to
> flush a frame threaded encoder because I couldn't wrap my head around
> whether that code path was actually safe or not. As this combination
> doesn't exist today, we'll deal with it if it ever comes up.
> 
> Signed-off-by: Philip Langdale <philipl at overt.org>
> ---

I'm missing two things here:
- motivation in the commit message (and possibly in the doxy too) - why
  is this needed and how is it better than just closing the encoder and
  creating a new one
- more precise semantics (in the doxy) as to what flush_buffers() will
  do. Are you getting the delayed frames out? Will it reset the encoder
  back from the flushed (with NULL frames) state?

-- 
Anton Khirnov