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

Philip Langdale philipl at overt.org
Sat Apr 11 02:47:43 EEST 2020


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..d4bdb9b1c0 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;
+
+        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;
+        }
+
+        // We haven't implemented flushing for frame-threaded encoders.
+        av_assert0(!(caps & AV_CODEC_CAP_FRAME_THREADS));
+    }
+
     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, \
-- 
2.20.1



More information about the ffmpeg-devel mailing list