[FFmpeg-devel] [PATCH v2] avcodec: Add explicit capability flag for encoder flushing
James Almer
jamrial at gmail.com
Sat Apr 11 02:01:29 EEST 2020
On 4/10/2020 7:38 PM, Philip Langdale wrote:
> 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>
> ---
> doc/APIchanges | 6 ++++++
> libavcodec/audiotoolboxenc.c | 3 ++-
> libavcodec/avcodec.h | 21 ++++++++++++++++-----
> libavcodec/decode.c | 15 +++++++++++++++
> libavcodec/nvenc_h264.c | 6 ++++--
> libavcodec/nvenc_hevc.c | 6 ++++--
> libavcodec/version.h | 4 ++--
> 7 files changed, 49 insertions(+), 12 deletions(-)
>
> diff --git a/doc/APIchanges b/doc/APIchanges
> index 4cc2367e69..938ccf3aaa 100644
> --- a/doc/APIchanges
> +++ b/doc/APIchanges
> @@ -15,6 +15,12 @@ libavutil: 2017-10-21
>
> API changes, most recent first:
>
> +2020-04-10 - xxxxxxxxxx - lavc 58.79.100 - avcodec.h
> + Add formal support for calling avcodec_flush_buffers() on encoders.
> + Encoders that set the cap AV_CODEC_CAP_ENCODER_FLUSH will be flushed.
> + For all other encoders, the call is now a no-op rather than undefined
> + behaviour.
> +
> 2020-xx-xx - xxxxxxxxxx - lavc 58.78.100 - avcodec.h codec_desc.h codec_id.h packet.h
> Move AVCodecDesc-related public API to new header codec_desc.h.
> Move AVCodecID enum to new header codec_id.h.
> diff --git a/libavcodec/audiotoolboxenc.c b/libavcodec/audiotoolboxenc.c
> index 2c1891693e..27632decf5 100644
> --- a/libavcodec/audiotoolboxenc.c
> +++ b/libavcodec/audiotoolboxenc.c
> @@ -627,7 +627,8 @@ static const AVOption options[] = {
> .encode2 = ffat_encode, \
> .flush = ffat_encode_flush, \
> .priv_class = &ffat_##NAME##_enc_class, \
> - .capabilities = AV_CODEC_CAP_DR1 | AV_CODEC_CAP_DELAY __VA_ARGS__, \
> + .capabilities = AV_CODEC_CAP_DR1 | AV_CODEC_CAP_DELAY | \
> + AV_CODEC_CAP_ENCODER_FLUSH __VA_ARGS__, \
> .sample_fmts = (const enum AVSampleFormat[]) { \
> AV_SAMPLE_FMT_S16, \
> AV_SAMPLE_FMT_U8, AV_SAMPLE_FMT_NONE \
> diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
> index 55151a0b71..5efe5583a1 100644
> --- a/libavcodec/avcodec.h
> +++ b/libavcodec/avcodec.h
> @@ -513,6 +513,13 @@ typedef struct RcOverride{
> */
> #define AV_CODEC_CAP_ENCODER_REORDERED_OPAQUE (1 << 20)
>
> +/**
> + * This encoder can be flushed using avcodec_flush_buffers(). If this flag is
> + * not set, the encoder must be closed and reopened to ensure that no frames
> + * remain pending.
> + */
> +#define AV_CODEC_CAP_ENCODER_FLUSH (1 << 21)
> +
> /* Exported side data.
> These flags can be passed in AVCodecContext.export_side_data before initialization.
> */
> @@ -4473,13 +4480,17 @@ int avcodec_fill_audio_frame(AVFrame *frame, int nb_channels,
> int buf_size, int align);
>
> /**
> - * Reset the internal decoder state / flush internal buffers. Should be called
> + * Reset the internal codec state / flush internal buffers. Should be called
> * e.g. when seeking or when switching to a different stream.
> *
> - * @note when refcounted frames are not used (i.e. avctx->refcounted_frames is 0),
> - * this invalidates the frames previously returned from the decoder. When
> - * refcounted frames are used, the decoder just releases any references it might
> - * keep internally, but the caller's reference remains valid.
> + * @note for decoders, when refcounted frames are not used
> + * (i.e. avctx->refcounted_frames is 0), this invalidates the frames previously
> + * returned from the decoder. When refcounted frames are used, the decoder just
> + * releases any references it might keep internally, but the caller's reference
> + * remains valid.
> + *
> + * @note for encoders, this function will only do something if the encoder
> + * declares support for AV_CODEC_CAP_ENCODER_FLUSH.
> */
> void avcodec_flush_buffers(AVCodecContext *avctx);
>
> diff --git a/libavcodec/decode.c b/libavcodec/decode.c
> index b7ae1fbb84..1602efdf03 100644
> --- a/libavcodec/decode.c
> +++ b/libavcodec/decode.c
> @@ -2084,6 +2084,21 @@ void avcodec_flush_buffers(AVCodecContext *avctx)
> {
> AVCodecInternal *avci = avctx->internal;
>
> + if (av_codec_is_encoder(avctx->codec)) {
> + int caps = avctx->codec->capabilities;
> +
> + // We haven't implemented flushing for frame-threaded encoders.
> + av_assert0(!(caps & AV_CODEC_CAP_FRAME_THREADS));
The assert needs to be under the following chunk, like it was in the
first version, after we have already established that we're dealing with
a flush enabled encoder to ensure that it's not also wrongly marked as
supporting frame threading.
Otherwise you'll be crashing the user if they call
avcodec_flush_buffers() on an encoder that supports frame threading but
never claimed to support flushing, when we were only supposed to print
the warning and return.
> +
> + if (!(caps & AV_CODEC_CAP_ENCODER_FLUSH)) {
> + // Only encoders that explicitly declare support for it can be
> + // flushed. Otherwise, this is a no-op.
> + av_log(avctx, AV_LOG_WARNING, "Ignoring attempt to flush encoder "
> + "that doesn't support it\n");
> + return;
> + }
> + }
> +
> avci->draining = 0;
> avci->draining_done = 0;
> avci->nb_draining_errors = 0;
> diff --git a/libavcodec/nvenc_h264.c b/libavcodec/nvenc_h264.c
> index 479155fe15..02392db46a 100644
> --- a/libavcodec/nvenc_h264.c
> +++ b/libavcodec/nvenc_h264.c
> @@ -214,7 +214,8 @@ AVCodec ff_nvenc_h264_encoder = {
> .priv_data_size = sizeof(NvencContext),
> .priv_class = &nvenc_h264_class,
> .defaults = defaults,
> - .capabilities = AV_CODEC_CAP_DELAY | AV_CODEC_CAP_HARDWARE,
> + .capabilities = AV_CODEC_CAP_DELAY | AV_CODEC_CAP_HARDWARE |
> + AV_CODEC_CAP_ENCODER_FLUSH,
> .caps_internal = FF_CODEC_CAP_INIT_CLEANUP,
> .pix_fmts = ff_nvenc_pix_fmts,
> .wrapper_name = "nvenc",
> @@ -244,7 +245,8 @@ AVCodec ff_h264_nvenc_encoder = {
> .priv_data_size = sizeof(NvencContext),
> .priv_class = &h264_nvenc_class,
> .defaults = defaults,
> - .capabilities = AV_CODEC_CAP_DELAY | AV_CODEC_CAP_HARDWARE,
> + .capabilities = AV_CODEC_CAP_DELAY | AV_CODEC_CAP_HARDWARE |
> + AV_CODEC_CAP_ENCODER_FLUSH,
> .caps_internal = FF_CODEC_CAP_INIT_CLEANUP,
> .pix_fmts = ff_nvenc_pix_fmts,
> .wrapper_name = "nvenc",
> diff --git a/libavcodec/nvenc_hevc.c b/libavcodec/nvenc_hevc.c
> index 7c9b3848f1..ea337a514f 100644
> --- a/libavcodec/nvenc_hevc.c
> +++ b/libavcodec/nvenc_hevc.c
> @@ -174,7 +174,8 @@ AVCodec ff_nvenc_hevc_encoder = {
> .priv_class = &nvenc_hevc_class,
> .defaults = defaults,
> .pix_fmts = ff_nvenc_pix_fmts,
> - .capabilities = AV_CODEC_CAP_DELAY | AV_CODEC_CAP_HARDWARE,
> + .capabilities = AV_CODEC_CAP_DELAY | AV_CODEC_CAP_HARDWARE |
> + AV_CODEC_CAP_ENCODER_FLUSH,
> .caps_internal = FF_CODEC_CAP_INIT_CLEANUP,
> .wrapper_name = "nvenc",
> };
> @@ -203,7 +204,8 @@ AVCodec ff_hevc_nvenc_encoder = {
> .priv_class = &hevc_nvenc_class,
> .defaults = defaults,
> .pix_fmts = ff_nvenc_pix_fmts,
> - .capabilities = AV_CODEC_CAP_DELAY | AV_CODEC_CAP_HARDWARE,
> + .capabilities = AV_CODEC_CAP_DELAY | AV_CODEC_CAP_HARDWARE |
> + AV_CODEC_CAP_ENCODER_FLUSH,
> .caps_internal = FF_CODEC_CAP_INIT_CLEANUP,
> .wrapper_name = "nvenc",
> };
> diff --git a/libavcodec/version.h b/libavcodec/version.h
> index 278f6be0cf..a4eb83124c 100644
> --- a/libavcodec/version.h
> +++ b/libavcodec/version.h
> @@ -28,8 +28,8 @@
> #include "libavutil/version.h"
>
> #define LIBAVCODEC_VERSION_MAJOR 58
> -#define LIBAVCODEC_VERSION_MINOR 78
> -#define LIBAVCODEC_VERSION_MICRO 101
> +#define LIBAVCODEC_VERSION_MINOR 79
> +#define LIBAVCODEC_VERSION_MICRO 100
>
> #define LIBAVCODEC_VERSION_INT AV_VERSION_INT(LIBAVCODEC_VERSION_MAJOR, \
> LIBAVCODEC_VERSION_MINOR, \
>
More information about the ffmpeg-devel
mailing list