[FFmpeg-devel] [PATCH 1/9] lavc: add a header for internal generic-layer APIs

[FFmpeg-devel] [PATCH 1/9] lavc: add a header for internal generic-layer APIs

[Anton Khirnov]

The goal is to distinguish between APIs provided by the generic layer to
individual codecs and APIs internal to the generic layer.

Start by moving ff_{decode,encode}_receive_frame() and
ff_{decode,encode}_preinit() into this new header, as those functions
are called from generic code and should not be visible to individual
codecs.
---
 libavcodec/avcodec.c          |  1 +
 libavcodec/avcodec_internal.h | 53 +++++++++++++++++++++++++++++++++++
 libavcodec/decode.c           |  1 +
 libavcodec/decode.h           | 11 --------
 libavcodec/encode.c           |  1 +
 libavcodec/encode.h           | 11 --------
 6 files changed, 56 insertions(+), 22 deletions(-)
 create mode 100644 libavcodec/avcodec_internal.h

diff --git a/libavcodec/avcodec.c b/libavcodec/avcodec.c
index db8226f9b3..638cb55146 100644
--- a/libavcodec/avcodec.c
+++ b/libavcodec/avcodec.c
@@ -34,6 +34,7 @@
 #include "libavutil/opt.h"
 #include "libavutil/thread.h"
 #include "avcodec.h"
+#include "avcodec_internal.h"
 #include "bsf.h"
 #include "codec_internal.h"
 #include "decode.h"
diff --git a/libavcodec/avcodec_internal.h b/libavcodec/avcodec_internal.h
new file mode 100644
index 0000000000..be60a36644
--- /dev/null
+++ b/libavcodec/avcodec_internal.h
@@ -0,0 +1,53 @@
+/*
+ * This file is part of FFmpeg.
+ *
+ * FFmpeg is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * FFmpeg is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with FFmpeg; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+/*
+ * APIs internal to the generic codec layer.
+ *
+ * MUST NOT be included by individual encoders or decoders.
+ */
+
+#ifndef AVCODEC_AVCODEC_INTERNAL_H
+#define AVCODEC_AVCODEC_INTERNAL_H
+
+struct AVCodecContext;
+struct AVFrame;
+
+/**
+ * avcodec_receive_frame() implementation for decoders.
+ */
+int ff_decode_receive_frame(struct AVCodecContext *avctx, struct AVFrame *frame);
+
+/**
+ * avcodec_receive_frame() implementation for encoders.
+ */
+int ff_encode_receive_frame(struct AVCodecContext *avctx, struct AVFrame *frame);
+
+/*
+ * Perform encoder initialization and validation.
+ * Called when opening the encoder, before the FFCodec.init() call.
+ */
+int ff_encode_preinit(struct AVCodecContext *avctx);
+
+/**
+ * Perform decoder initialization and validation.
+ * Called when opening the decoder, before the FFCodec.init() call.
+ */
+int ff_decode_preinit(struct AVCodecContext *avctx);
+
+#endif // AVCODEC_AVCODEC_INTERNAL_H
diff --git a/libavcodec/decode.c b/libavcodec/decode.c
index a7c130207c..8adb532616 100644
--- a/libavcodec/decode.c
+++ b/libavcodec/decode.c
@@ -41,6 +41,7 @@
 #include "libavutil/opt.h"
 
 #include "avcodec.h"
+#include "avcodec_internal.h"
 #include "bytestream.h"
 #include "bsf.h"
 #include "codec_internal.h"
diff --git a/libavcodec/decode.h b/libavcodec/decode.h
index aaa29bc7f5..2b9fe59907 100644
--- a/libavcodec/decode.h
+++ b/libavcodec/decode.h
@@ -53,11 +53,6 @@ typedef struct FrameDecodeData {
     void (*hwaccel_priv_free)(void *priv);
 } FrameDecodeData;
 
-/**
- * avcodec_receive_frame() implementation for decoders.
- */
-int ff_decode_receive_frame(AVCodecContext *avctx, AVFrame *frame);
-
 /**
  * Called by decoders to get the next packet for decoding.
  *
@@ -99,12 +94,6 @@ int ff_attach_decode_data(AVFrame *frame);
  */
 int ff_copy_palette(void *dst, const AVPacket *src, void *logctx);
 
-/**
- * Perform decoder initialization and validation.
- * Called when opening the decoder, before the FFCodec.init() call.
- */
-int ff_decode_preinit(AVCodecContext *avctx);
-
 /**
  * Check that the provided frame dimensions are valid and set them on the codec
  * context.
diff --git a/libavcodec/encode.c b/libavcodec/encode.c
index ab5f889615..3a016b14c1 100644
--- a/libavcodec/encode.c
+++ b/libavcodec/encode.c
@@ -27,6 +27,7 @@
 #include "libavutil/samplefmt.h"
 
 #include "avcodec.h"
+#include "avcodec_internal.h"
 #include "codec_internal.h"
 #include "encode.h"
 #include "frame_thread_encoder.h"
diff --git a/libavcodec/encode.h b/libavcodec/encode.h
index 26a3304045..dfaab7c976 100644
--- a/libavcodec/encode.h
+++ b/libavcodec/encode.h
@@ -26,11 +26,6 @@
 #include "avcodec.h"
 #include "packet.h"
 
-/**
- * avcodec_receive_frame() implementation for encoders.
- */
-int ff_encode_receive_frame(AVCodecContext *avctx, AVFrame *frame);
-
 /**
  * Called by encoders to get the next frame for encoding.
  *
@@ -75,12 +70,6 @@ int ff_alloc_packet(AVCodecContext *avctx, AVPacket *avpkt, int64_t size);
 int ff_encode_reordered_opaque(AVCodecContext *avctx,
                                AVPacket *pkt, const AVFrame *frame);
 
-/*
- * Perform encoder initialization and validation.
- * Called when opening the encoder, before the FFCodec.init() call.
- */
-int ff_encode_preinit(AVCodecContext *avctx);
-
 int ff_encode_encode_cb(AVCodecContext *avctx, AVPacket *avpkt,
                         AVFrame *frame, int *got_packet);
 
-- 
2.40.1

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 2/9] lavc/avcodec: split flushing into decode- and encode-specific functions

[Anton Khirnov]

Will allow making some state private to encoding/decoding in the future.
---
 libavcodec/avcodec.c          | 26 ++------------------------
 libavcodec/avcodec_internal.h |  3 +++
 libavcodec/decode.c           | 15 +++++++++++++++
 libavcodec/encode.c           | 18 ++++++++++++++++++
 4 files changed, 38 insertions(+), 24 deletions(-)

diff --git a/libavcodec/avcodec.c b/libavcodec/avcodec.c
index 638cb55146..a5cb6035b6 100644
--- a/libavcodec/avcodec.c
+++ b/libavcodec/avcodec.c
@@ -373,33 +373,11 @@ 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;
-        }
-        if (avci->in_frame)
-            av_frame_unref(avci->in_frame);
-        if (avci->recon_frame)
-            av_frame_unref(avci->recon_frame);
-    } else {
-        av_packet_unref(avci->last_pkt_props);
-        av_packet_unref(avci->in_pkt);
-
-        avctx->pts_correction_last_pts =
-        avctx->pts_correction_last_dts = INT64_MIN;
-
-        av_bsf_flush(avci->bsf);
-    }
+    av_codec_is_encoder(avctx->codec) ?
+        ff_encode_flush_buffers(avctx) : ff_decode_flush_buffers(avctx);
 
     avci->draining      = 0;
     avci->draining_done = 0;
-    avci->nb_draining_errors = 0;
     av_frame_unref(avci->buffer_frame);
     av_packet_unref(avci->buffer_pkt);
 
diff --git a/libavcodec/avcodec_internal.h b/libavcodec/avcodec_internal.h
index be60a36644..6ffe575c3e 100644
--- a/libavcodec/avcodec_internal.h
+++ b/libavcodec/avcodec_internal.h
@@ -50,4 +50,7 @@ int ff_encode_preinit(struct AVCodecContext *avctx);
  */
 int ff_decode_preinit(struct AVCodecContext *avctx);
 
+void ff_decode_flush_buffers(struct AVCodecContext *avctx);
+void ff_encode_flush_buffers(struct AVCodecContext *avctx);
+
 #endif // AVCODEC_AVCODEC_INTERNAL_H
diff --git a/libavcodec/decode.c b/libavcodec/decode.c
index 8adb532616..7d000fec32 100644
--- a/libavcodec/decode.c
+++ b/libavcodec/decode.c
@@ -1739,3 +1739,18 @@ AVBufferRef *ff_hwaccel_frame_priv_alloc(AVCodecContext *avctx,
 
     return ref;
 }
+
+void ff_decode_flush_buffers(AVCodecContext *avctx)
+{
+    AVCodecInternal *avci = avctx->internal;
+
+        av_packet_unref(avci->last_pkt_props);
+        av_packet_unref(avci->in_pkt);
+
+        avctx->pts_correction_last_pts =
+        avctx->pts_correction_last_dts = INT64_MIN;
+
+        av_bsf_flush(avci->bsf);
+
+    avci->nb_draining_errors = 0;
+}
diff --git a/libavcodec/encode.c b/libavcodec/encode.c
index 3a016b14c1..3341a79c9b 100644
--- a/libavcodec/encode.c
+++ b/libavcodec/encode.c
@@ -785,3 +785,21 @@ int ff_encode_receive_frame(AVCodecContext *avctx, AVFrame *frame)
     av_frame_move_ref(frame, avci->recon_frame);
     return 0;
 }
+
+void ff_encode_flush_buffers(AVCodecContext *avctx)
+{
+    AVCodecInternal *avci = avctx->internal;
+        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;
+        }
+        if (avci->in_frame)
+            av_frame_unref(avci->in_frame);
+        if (avci->recon_frame)
+            av_frame_unref(avci->recon_frame);
+}
-- 
2.40.1

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 2/9] lavc/avcodec: split flushing into decode- and encode-specific functions

[Andreas Rheinhardt]

Anton Khirnov:
> Will allow making some state private to encoding/decoding in the future.
> ---
>  libavcodec/avcodec.c          | 26 ++------------------------
>  libavcodec/avcodec_internal.h |  3 +++
>  libavcodec/decode.c           | 15 +++++++++++++++
>  libavcodec/encode.c           | 18 ++++++++++++++++++
>  4 files changed, 38 insertions(+), 24 deletions(-)
> 
> diff --git a/libavcodec/avcodec.c b/libavcodec/avcodec.c
> index 638cb55146..a5cb6035b6 100644
> --- a/libavcodec/avcodec.c
> +++ b/libavcodec/avcodec.c
> @@ -373,33 +373,11 @@ 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;

This return makes flushing an encoder that does not support it a no-op;
this will no longer be true after this patch.

> -        }
> -        if (avci->in_frame)
> -            av_frame_unref(avci->in_frame);
> -        if (avci->recon_frame)
> -            av_frame_unref(avci->recon_frame);
> -    } else {
> -        av_packet_unref(avci->last_pkt_props);
> -        av_packet_unref(avci->in_pkt);
> -
> -        avctx->pts_correction_last_pts =
> -        avctx->pts_correction_last_dts = INT64_MIN;
> -
> -        av_bsf_flush(avci->bsf);
> -    }
> +    av_codec_is_encoder(avctx->codec) ?
> +        ff_encode_flush_buffers(avctx) : ff_decode_flush_buffers(avctx);
>  
>      avci->draining      = 0;
>      avci->draining_done = 0;
> -    avci->nb_draining_errors = 0;
>      av_frame_unref(avci->buffer_frame);
>      av_packet_unref(avci->buffer_pkt);
>  
> diff --git a/libavcodec/avcodec_internal.h b/libavcodec/avcodec_internal.h
> index be60a36644..6ffe575c3e 100644
> --- a/libavcodec/avcodec_internal.h
> +++ b/libavcodec/avcodec_internal.h
> @@ -50,4 +50,7 @@ int ff_encode_preinit(struct AVCodecContext *avctx);
>   */
>  int ff_decode_preinit(struct AVCodecContext *avctx);
>  
> +void ff_decode_flush_buffers(struct AVCodecContext *avctx);
> +void ff_encode_flush_buffers(struct AVCodecContext *avctx);
> +
>  #endif // AVCODEC_AVCODEC_INTERNAL_H
> diff --git a/libavcodec/decode.c b/libavcodec/decode.c
> index 8adb532616..7d000fec32 100644
> --- a/libavcodec/decode.c
> +++ b/libavcodec/decode.c
> @@ -1739,3 +1739,18 @@ AVBufferRef *ff_hwaccel_frame_priv_alloc(AVCodecContext *avctx,
>  
>      return ref;
>  }
> +
> +void ff_decode_flush_buffers(AVCodecContext *avctx)
> +{
> +    AVCodecInternal *avci = avctx->internal;
> +
> +        av_packet_unref(avci->last_pkt_props);
> +        av_packet_unref(avci->in_pkt);
> +
> +        avctx->pts_correction_last_pts =
> +        avctx->pts_correction_last_dts = INT64_MIN;
> +
> +        av_bsf_flush(avci->bsf);
> +
> +    avci->nb_draining_errors = 0;
> +}
> diff --git a/libavcodec/encode.c b/libavcodec/encode.c
> index 3a016b14c1..3341a79c9b 100644
> --- a/libavcodec/encode.c
> +++ b/libavcodec/encode.c
> @@ -785,3 +785,21 @@ int ff_encode_receive_frame(AVCodecContext *avctx, AVFrame *frame)
>      av_frame_move_ref(frame, avci->recon_frame);
>      return 0;
>  }
> +
> +void ff_encode_flush_buffers(AVCodecContext *avctx)
> +{
> +    AVCodecInternal *avci = avctx->internal;
> +        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;
> +        }
> +        if (avci->in_frame)
> +            av_frame_unref(avci->in_frame);
> +        if (avci->recon_frame)
> +            av_frame_unref(avci->recon_frame);
> +}

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 3/9] lavc: reindent after previous commit

[Anton Khirnov]

---
 libavcodec/decode.c | 10 +++++-----
 libavcodec/encode.c | 24 ++++++++++++------------
 2 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/libavcodec/decode.c b/libavcodec/decode.c
index 7d000fec32..b5e5b4a2db 100644
--- a/libavcodec/decode.c
+++ b/libavcodec/decode.c
@@ -1744,13 +1744,13 @@ void ff_decode_flush_buffers(AVCodecContext *avctx)
 {
     AVCodecInternal *avci = avctx->internal;
 
-        av_packet_unref(avci->last_pkt_props);
-        av_packet_unref(avci->in_pkt);
+    av_packet_unref(avci->last_pkt_props);
+    av_packet_unref(avci->in_pkt);
 
-        avctx->pts_correction_last_pts =
-        avctx->pts_correction_last_dts = INT64_MIN;
+    avctx->pts_correction_last_pts =
+    avctx->pts_correction_last_dts = INT64_MIN;
 
-        av_bsf_flush(avci->bsf);
+    av_bsf_flush(avci->bsf);
 
     avci->nb_draining_errors = 0;
 }
diff --git a/libavcodec/encode.c b/libavcodec/encode.c
index 3341a79c9b..6d0ef2be10 100644
--- a/libavcodec/encode.c
+++ b/libavcodec/encode.c
@@ -789,17 +789,17 @@ int ff_encode_receive_frame(AVCodecContext *avctx, AVFrame *frame)
 void ff_encode_flush_buffers(AVCodecContext *avctx)
 {
     AVCodecInternal *avci = avctx->internal;
-        int caps = avctx->codec->capabilities;
+    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;
-        }
-        if (avci->in_frame)
-            av_frame_unref(avci->in_frame);
-        if (avci->recon_frame)
-            av_frame_unref(avci->recon_frame);
+    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;
+    }
+    if (avci->in_frame)
+        av_frame_unref(avci->in_frame);
+    if (avci->recon_frame)
+        av_frame_unref(avci->recon_frame);
 }
-- 
2.40.1

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 4/9] lavc: add generic-decode-layer private data

[Anton Khirnov]

Move AVCodecInternal.nb_draining_errors to it, should should not be
visible outside of decode.c.
---
 libavcodec/avcodec.c          |  4 +++-
 libavcodec/avcodec_internal.h |  2 ++
 libavcodec/decode.c           | 22 ++++++++++++++++++++--
 libavcodec/internal.h         | 10 +++++++---
 libavcodec/pthread_frame.c    |  3 ++-
 5 files changed, 34 insertions(+), 7 deletions(-)

diff --git a/libavcodec/avcodec.c b/libavcodec/avcodec.c
index a5cb6035b6..ff251d2dae 100644
--- a/libavcodec/avcodec.c
+++ b/libavcodec/avcodec.c
@@ -150,7 +150,9 @@ int attribute_align_arg avcodec_open2(AVCodecContext *avctx, const AVCodec *code
     if (avctx->extradata_size < 0 || avctx->extradata_size >= FF_MAX_EXTRADATA_SIZE)
         return AVERROR(EINVAL);
 
-    avci = av_mallocz(sizeof(*avci));
+    avci = av_codec_is_decoder(codec) ?
+        ff_decode_internal_alloc()    :
+        av_mallocz(sizeof(AVCodecInternal));
     if (!avci) {
         ret = AVERROR(ENOMEM);
         goto end;
diff --git a/libavcodec/avcodec_internal.h b/libavcodec/avcodec_internal.h
index 6ffe575c3e..f52f91e07c 100644
--- a/libavcodec/avcodec_internal.h
+++ b/libavcodec/avcodec_internal.h
@@ -53,4 +53,6 @@ int ff_decode_preinit(struct AVCodecContext *avctx);
 void ff_decode_flush_buffers(struct AVCodecContext *avctx);
 void ff_encode_flush_buffers(struct AVCodecContext *avctx);
 
+struct AVCodecInternal *ff_decode_internal_alloc(void);
+
 #endif // AVCODEC_AVCODEC_INTERNAL_H
diff --git a/libavcodec/decode.c b/libavcodec/decode.c
index b5e5b4a2db..c070148b58 100644
--- a/libavcodec/decode.c
+++ b/libavcodec/decode.c
@@ -50,6 +50,11 @@
 #include "internal.h"
 #include "thread.h"
 
+struct DecodeContext {
+    /* to prevent infinite loop on errors when draining */
+    int nb_draining_errors;
+};
+
 static int apply_param_change(AVCodecContext *avctx, const AVPacket *avpkt)
 {
     int ret;
@@ -439,7 +444,7 @@ FF_ENABLE_DEPRECATION_WARNINGS
             int nb_errors_max = 20 + (HAVE_THREADS && avctx->active_thread_type & FF_THREAD_FRAME ?
                                 avctx->thread_count : 1);
 
-            if (avci->nb_draining_errors++ >= nb_errors_max) {
+            if (avci->d->nb_draining_errors++ >= nb_errors_max) {
                 av_log(avctx, AV_LOG_ERROR, "Too many errors when draining, this is a bug. "
                        "Stop draining and force EOF.\n");
                 avci->draining_done = 1;
@@ -1752,5 +1757,18 @@ void ff_decode_flush_buffers(AVCodecContext *avctx)
 
     av_bsf_flush(avci->bsf);
 
-    avci->nb_draining_errors = 0;
+    avci->d->nb_draining_errors = 0;
+}
+
+AVCodecInternal *ff_decode_internal_alloc(void)
+{
+    struct Dummy {
+        AVCodecInternal   i;
+        DecodeContext     d;
+    } *dummy = av_mallocz(sizeof(*dummy));
+    if (!dummy)
+        return NULL;
+    dummy->i.d = &dummy->d;
+
+    return &dummy->i;
 }
diff --git a/libavcodec/internal.h b/libavcodec/internal.h
index dceae182c0..b672092ac4 100644
--- a/libavcodec/internal.h
+++ b/libavcodec/internal.h
@@ -49,7 +49,14 @@
 #   define STRIDE_ALIGN 8
 #endif
 
+typedef struct DecodeContext DecodeContext;
+
 typedef struct AVCodecInternal {
+    /**
+     * Generic decoding private data.
+     */
+    DecodeContext *d;
+
     /**
      * When using frame-threaded decoding, this field is set for the first
      * worker thread (e.g. to decode extradata just once).
@@ -148,9 +155,6 @@ typedef struct AVCodecInternal {
     AVFrame *buffer_frame;
     int draining_done;
 
-    /* to prevent infinite loop on errors when draining */
-    int nb_draining_errors;
-
     /* used when avctx flag AV_CODEC_FLAG_DROPCHANGED is set */
     int changed_frames_dropped;
     int initial_format;
diff --git a/libavcodec/pthread_frame.c b/libavcodec/pthread_frame.c
index 008f3da43b..bc305f561f 100644
--- a/libavcodec/pthread_frame.c
+++ b/libavcodec/pthread_frame.c
@@ -28,6 +28,7 @@
 #include <stdint.h>
 
 #include "avcodec.h"
+#include "avcodec_internal.h"
 #include "codec_internal.h"
 #include "decode.h"
 #include "hwconfig.h"
@@ -815,7 +816,7 @@ static av_cold int init_thread(PerThreadContext *p, int *threads_to_free,
     p->parent = fctx;
     p->avctx  = copy;
 
-    copy->internal = av_mallocz(sizeof(*copy->internal));
+    copy->internal = ff_decode_internal_alloc();
     if (!copy->internal)
         return AVERROR(ENOMEM);
     copy->internal->thread_ctx = p;
-- 
2.40.1

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 4/9] lavc: add generic-decode-layer private data

[Andreas Rheinhardt]

Anton Khirnov:
> Move AVCodecInternal.nb_draining_errors to it, should should not be
> visible outside of decode.c.
> ---
>  libavcodec/avcodec.c          |  4 +++-
>  libavcodec/avcodec_internal.h |  2 ++
>  libavcodec/decode.c           | 22 ++++++++++++++++++++--
>  libavcodec/internal.h         | 10 +++++++---
>  libavcodec/pthread_frame.c    |  3 ++-
>  5 files changed, 34 insertions(+), 7 deletions(-)
> 
> diff --git a/libavcodec/avcodec.c b/libavcodec/avcodec.c
> index a5cb6035b6..ff251d2dae 100644
> --- a/libavcodec/avcodec.c
> +++ b/libavcodec/avcodec.c
> @@ -150,7 +150,9 @@ int attribute_align_arg avcodec_open2(AVCodecContext *avctx, const AVCodec *code
>      if (avctx->extradata_size < 0 || avctx->extradata_size >= FF_MAX_EXTRADATA_SIZE)
>          return AVERROR(EINVAL);
>  
> -    avci = av_mallocz(sizeof(*avci));
> +    avci = av_codec_is_decoder(codec) ?
> +        ff_decode_internal_alloc()    :
> +        av_mallocz(sizeof(AVCodecInternal));
>      if (!avci) {
>          ret = AVERROR(ENOMEM);
>          goto end;
> diff --git a/libavcodec/avcodec_internal.h b/libavcodec/avcodec_internal.h
> index 6ffe575c3e..f52f91e07c 100644
> --- a/libavcodec/avcodec_internal.h
> +++ b/libavcodec/avcodec_internal.h
> @@ -53,4 +53,6 @@ int ff_decode_preinit(struct AVCodecContext *avctx);
>  void ff_decode_flush_buffers(struct AVCodecContext *avctx);
>  void ff_encode_flush_buffers(struct AVCodecContext *avctx);
>  
> +struct AVCodecInternal *ff_decode_internal_alloc(void);
> +
>  #endif // AVCODEC_AVCODEC_INTERNAL_H
> diff --git a/libavcodec/decode.c b/libavcodec/decode.c
> index b5e5b4a2db..c070148b58 100644
> --- a/libavcodec/decode.c
> +++ b/libavcodec/decode.c
> @@ -50,6 +50,11 @@
>  #include "internal.h"
>  #include "thread.h"
>  
> +struct DecodeContext {
> +    /* to prevent infinite loop on errors when draining */
> +    int nb_draining_errors;
> +};
> +
>  static int apply_param_change(AVCodecContext *avctx, const AVPacket *avpkt)
>  {
>      int ret;
> @@ -439,7 +444,7 @@ FF_ENABLE_DEPRECATION_WARNINGS
>              int nb_errors_max = 20 + (HAVE_THREADS && avctx->active_thread_type & FF_THREAD_FRAME ?
>                                  avctx->thread_count : 1);
>  
> -            if (avci->nb_draining_errors++ >= nb_errors_max) {
> +            if (avci->d->nb_draining_errors++ >= nb_errors_max) {
>                  av_log(avctx, AV_LOG_ERROR, "Too many errors when draining, this is a bug. "
>                         "Stop draining and force EOF.\n");
>                  avci->draining_done = 1;
> @@ -1752,5 +1757,18 @@ void ff_decode_flush_buffers(AVCodecContext *avctx)
>  
>      av_bsf_flush(avci->bsf);
>  
> -    avci->nb_draining_errors = 0;
> +    avci->d->nb_draining_errors = 0;
> +}
> +
> +AVCodecInternal *ff_decode_internal_alloc(void)
> +{
> +    struct Dummy {
> +        AVCodecInternal   i;
> +        DecodeContext     d;
> +    } *dummy = av_mallocz(sizeof(*dummy));
> +    if (!dummy)
> +        return NULL;
> +    dummy->i.d = &dummy->d;
> +
> +    return &dummy->i;
>  }
> diff --git a/libavcodec/internal.h b/libavcodec/internal.h
> index dceae182c0..b672092ac4 100644
> --- a/libavcodec/internal.h
> +++ b/libavcodec/internal.h
> @@ -49,7 +49,14 @@
>  #   define STRIDE_ALIGN 8
>  #endif
>  
> +typedef struct DecodeContext DecodeContext;
> +
>  typedef struct AVCodecInternal {
> +    /**
> +     * Generic decoding private data.
> +     */
> +    DecodeContext *d;

This approach has the downside of adding unnecessary indirecations; it
furthermore adds pointers to DecodeContext and EncodeContext to
AVCodecInternal, as if both could be valid at the same time.

The above could be overcome by using the typical approach to access
these extra fields, as is used for FFStream etc.

Furthermore, I do not really think that is worth it: The number of
fields affected by it are just so small. Has encoder code ever tried to
set nb_draining_errors?

> +
>      /**
>       * When using frame-threaded decoding, this field is set for the first
>       * worker thread (e.g. to decode extradata just once).
> @@ -148,9 +155,6 @@ typedef struct AVCodecInternal {
>      AVFrame *buffer_frame;
>      int draining_done;
>  
> -    /* to prevent infinite loop on errors when draining */
> -    int nb_draining_errors;
> -
>      /* used when avctx flag AV_CODEC_FLAG_DROPCHANGED is set */
>      int changed_frames_dropped;
>      int initial_format;
> diff --git a/libavcodec/pthread_frame.c b/libavcodec/pthread_frame.c
> index 008f3da43b..bc305f561f 100644
> --- a/libavcodec/pthread_frame.c
> +++ b/libavcodec/pthread_frame.c
> @@ -28,6 +28,7 @@
>  #include <stdint.h>
>  
>  #include "avcodec.h"
> +#include "avcodec_internal.h"
>  #include "codec_internal.h"
>  #include "decode.h"
>  #include "hwconfig.h"
> @@ -815,7 +816,7 @@ static av_cold int init_thread(PerThreadContext *p, int *threads_to_free,
>      p->parent = fctx;
>      p->avctx  = copy;
>  
> -    copy->internal = av_mallocz(sizeof(*copy->internal));
> +    copy->internal = ff_decode_internal_alloc();
>      if (!copy->internal)
>          return AVERROR(ENOMEM);
>      copy->internal->thread_ctx = p;

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 4/9] lavc: add generic-decode-layer private data

[Anton Khirnov]

Quoting Andreas Rheinhardt (2023-06-24 21:34:58)
> > diff --git a/libavcodec/internal.h b/libavcodec/internal.h
> > index dceae182c0..b672092ac4 100644
> > --- a/libavcodec/internal.h
> > +++ b/libavcodec/internal.h
> > @@ -49,7 +49,14 @@
> >  #   define STRIDE_ALIGN 8
> >  #endif
> >  
> > +typedef struct DecodeContext DecodeContext;
> > +
> >  typedef struct AVCodecInternal {
> > +    /**
> > +     * Generic decoding private data.
> > +     */
> > +    DecodeContext *d;
> 
> This approach has the downside of adding unnecessary indirecations; it
> furthermore adds pointers to DecodeContext and EncodeContext to
> AVCodecInternal, as if both could be valid at the same time.
> 
> The above could be overcome by using the typical approach to access
> these extra fields, as is used for FFStream etc.
> 
> Furthermore, I do not really think that is worth it: The number of
> fields affected by it are just so small. Has encoder code ever tried to
> set nb_draining_errors?

True, for decoding there are currently very few fields. But there are
many more that are encoding-only - I am not handling more of them in
this set because some of them are not trivial and I did not want to make
this patchset overly large, as its point is fixing the FATE test in 9/9.

And I belive there is a strong case for hiding generic-layer private
state from individual decoders/encoders, as it is prone to being
misunderstood and misused (e.g. AVCodecInternal.draining is currently
used in ways it most likely should not be).

Since the overhead of having a private decode context in addition to a
private encode context is very small, I prefer adding it now, at least
for consistency. Otherwise I'd have to add a shared encode-decode
context, which would be about the same amount of code.

-- 
Anton Khirnov
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 5/9] lavc: add generic-encode-layer private data

[Anton Khirnov]

Move AVCodecInternal.intra_only_flag to it, should should not be visible
outside of encode.c.
---
 libavcodec/avcodec.c          |  2 +-
 libavcodec/avcodec_internal.h |  1 +
 libavcodec/encode.c           | 26 ++++++++++++++++++++++++--
 libavcodec/internal.h         | 13 ++++++-------
 4 files changed, 32 insertions(+), 10 deletions(-)

diff --git a/libavcodec/avcodec.c b/libavcodec/avcodec.c
index ff251d2dae..21d1e42c10 100644
--- a/libavcodec/avcodec.c
+++ b/libavcodec/avcodec.c
@@ -152,7 +152,7 @@ int attribute_align_arg avcodec_open2(AVCodecContext *avctx, const AVCodec *code
 
     avci = av_codec_is_decoder(codec) ?
         ff_decode_internal_alloc()    :
-        av_mallocz(sizeof(AVCodecInternal));
+        ff_encode_internal_alloc();
     if (!avci) {
         ret = AVERROR(ENOMEM);
         goto end;
diff --git a/libavcodec/avcodec_internal.h b/libavcodec/avcodec_internal.h
index f52f91e07c..9b93ff3d81 100644
--- a/libavcodec/avcodec_internal.h
+++ b/libavcodec/avcodec_internal.h
@@ -54,5 +54,6 @@ void ff_decode_flush_buffers(struct AVCodecContext *avctx);
 void ff_encode_flush_buffers(struct AVCodecContext *avctx);
 
 struct AVCodecInternal *ff_decode_internal_alloc(void);
+struct AVCodecInternal *ff_encode_internal_alloc(void);
 
 #endif // AVCODEC_AVCODEC_INTERNAL_H
diff --git a/libavcodec/encode.c b/libavcodec/encode.c
index 6d0ef2be10..4273dceb31 100644
--- a/libavcodec/encode.c
+++ b/libavcodec/encode.c
@@ -33,6 +33,15 @@
 #include "frame_thread_encoder.h"
 #include "internal.h"
 
+struct EncodeContext {
+    /**
+     * This is set to AV_PKT_FLAG_KEY for encoders that encode intra-only
+     * formats (i.e. whose codec descriptor has AV_CODEC_PROP_INTRA_ONLY set).
+     * This is used to set said flag generically for said encoders.
+     */
+    int intra_only_flag;
+};
+
 int ff_alloc_packet(AVCodecContext *avctx, AVPacket *avpkt, int64_t size)
 {
     if (size < 0 || size > INT_MAX - AV_INPUT_BUFFER_PADDING_SIZE) {
@@ -372,7 +381,7 @@ static int encode_receive_packet_internal(AVCodecContext *avctx, AVPacket *avpkt
     } else
         ret = encode_simple_receive_packet(avctx, avpkt);
     if (ret >= 0)
-        avpkt->flags |= avci->intra_only_flag;
+        avpkt->flags |= avci->e->intra_only_flag;
 
     if (ret == AVERROR_EOF)
         avci->draining_done = 1;
@@ -710,7 +719,7 @@ int ff_encode_preinit(AVCodecContext *avctx)
         avctx->rc_initial_buffer_occupancy = avctx->rc_buffer_size * 3LL / 4;
 
     if (avctx->codec_descriptor->props & AV_CODEC_PROP_INTRA_ONLY)
-        avctx->internal->intra_only_flag = AV_PKT_FLAG_KEY;
+        avctx->internal->e->intra_only_flag = AV_PKT_FLAG_KEY;
 
     if (ffcodec(avctx->codec)->cb_type == FF_CODEC_CB_TYPE_ENCODE) {
         avci->in_frame = av_frame_alloc();
@@ -803,3 +812,16 @@ void ff_encode_flush_buffers(AVCodecContext *avctx)
     if (avci->recon_frame)
         av_frame_unref(avci->recon_frame);
 }
+
+AVCodecInternal *ff_encode_internal_alloc(void)
+{
+    struct Dummy {
+        AVCodecInternal   i;
+        EncodeContext     e;
+    } *dummy = av_mallocz(sizeof(*dummy));
+    if (!dummy)
+        return NULL;
+    dummy->i.e = &dummy->e;
+
+    return &dummy->i;
+}
diff --git a/libavcodec/internal.h b/libavcodec/internal.h
index b672092ac4..2f4aee8ecc 100644
--- a/libavcodec/internal.h
+++ b/libavcodec/internal.h
@@ -50,6 +50,7 @@
 #endif
 
 typedef struct DecodeContext DecodeContext;
+typedef struct EncodeContext EncodeContext;
 
 typedef struct AVCodecInternal {
     /**
@@ -57,6 +58,11 @@ typedef struct AVCodecInternal {
      */
     DecodeContext *d;
 
+    /**
+     * Generic decoding private data.
+     */
+    EncodeContext *e;
+
     /**
      * When using frame-threaded decoding, this field is set for the first
      * worker thread (e.g. to decode extradata just once).
@@ -102,13 +108,6 @@ typedef struct AVCodecInternal {
     uint8_t *byte_buffer;
     unsigned int byte_buffer_size;
 
-    /**
-     * This is set to AV_PKT_FLAG_KEY for encoders that encode intra-only
-     * formats (i.e. whose codec descriptor has AV_CODEC_PROP_INTRA_ONLY set).
-     * This is used to set said flag generically for said encoders.
-     */
-    int intra_only_flag;
-
     void *frame_thread_encoder;
 
     /**
-- 
2.40.1

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 6/9] lavc: move AVCodecInternal.last_audio_frame to EncodeContext

[Anton Khirnov]

It does not need to be visible outside of encode.c.
---
 libavcodec/encode.c   | 12 +++++++++---
 libavcodec/internal.h |  6 ------
 2 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/libavcodec/encode.c b/libavcodec/encode.c
index 4273dceb31..2b304d0771 100644
--- a/libavcodec/encode.c
+++ b/libavcodec/encode.c
@@ -40,6 +40,12 @@ struct EncodeContext {
      * This is used to set said flag generically for said encoders.
      */
     int intra_only_flag;
+
+    /**
+     * An audio frame with less than required samples has been submitted (and
+     * potentially padded with silence). Reject all subsequent frames.
+     */
+    int last_audio_frame;
 };
 
 int ff_alloc_packet(AVCodecContext *avctx, AVPacket *avpkt, int64_t size)
@@ -167,7 +173,7 @@ static int pad_last_frame(AVCodecContext *s, AVFrame *frame, const AVFrame *src,
 
 fail:
     av_frame_unref(frame);
-    s->internal->last_audio_frame = 0;
+    s->internal->e->last_audio_frame = 0;
     return ret;
 }
 
@@ -451,7 +457,7 @@ static int encode_send_frame_internal(AVCodecContext *avctx, const AVFrame *src)
         /* check for valid frame size */
         if (!(avctx->codec->capabilities & AV_CODEC_CAP_VARIABLE_FRAME_SIZE)) {
             /* if we already got an undersized frame, that must have been the last */
-            if (avctx->internal->last_audio_frame) {
+            if (avctx->internal->e->last_audio_frame) {
                 av_log(avctx, AV_LOG_ERROR, "frame_size (%d) was not respected for a non-last frame\n", avctx->frame_size);
                 return AVERROR(EINVAL);
             }
@@ -460,7 +466,7 @@ static int encode_send_frame_internal(AVCodecContext *avctx, const AVFrame *src)
                 return AVERROR(EINVAL);
             }
             if (src->nb_samples < avctx->frame_size) {
-                avctx->internal->last_audio_frame = 1;
+                avctx->internal->e->last_audio_frame = 1;
                 if (!(avctx->codec->capabilities & AV_CODEC_CAP_SMALL_LAST_FRAME)) {
                     int pad_samples = avci->pad_samples ? avci->pad_samples : avctx->frame_size;
                     int out_samples = (src->nb_samples + pad_samples - 1) / pad_samples * pad_samples;
diff --git a/libavcodec/internal.h b/libavcodec/internal.h
index 2f4aee8ecc..2673c7d494 100644
--- a/libavcodec/internal.h
+++ b/libavcodec/internal.h
@@ -69,12 +69,6 @@ typedef struct AVCodecInternal {
      */
     int is_copy;
 
-    /**
-     * An audio frame with less than required samples has been submitted (and
-     * potentially padded with silence). Reject all subsequent frames.
-     */
-    int last_audio_frame;
-
     /**
      * Audio encoders can set this flag during init to indicate that they
      * want the small last frame to be padded to a multiple of pad_samples.
-- 
2.40.1

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 7/9] lavc/decode: track whether the caller started draining with a separate flag

[Anton Khirnov]

Decoding pipeline has multiple stages, some of which may have their own
delay (e.g. bitstream filters). The code currently uses
AVCodecInternal.draining to track all of them, but they do not have to
all be in sync.
---
 libavcodec/decode.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/libavcodec/decode.c b/libavcodec/decode.c
index c070148b58..c61ce74fb8 100644
--- a/libavcodec/decode.c
+++ b/libavcodec/decode.c
@@ -53,6 +53,11 @@
 struct DecodeContext {
     /* to prevent infinite loop on errors when draining */
     int nb_draining_errors;
+
+    /**
+     * The caller has submitted a NULL packet on input.
+     */
+    int draining_started;
 };
 
 static int apply_param_change(AVCodecContext *avctx, const AVPacket *avpkt)
@@ -624,7 +629,7 @@ int attribute_align_arg avcodec_send_packet(AVCodecContext *avctx, const AVPacke
     if (!avcodec_is_open(avctx) || !av_codec_is_decoder(avctx->codec))
         return AVERROR(EINVAL);
 
-    if (avctx->internal->draining)
+    if (avci->d->draining_started)
         return AVERROR_EOF;
 
     if (avpkt && !avpkt->size && avpkt->data)
@@ -635,7 +640,8 @@ int attribute_align_arg avcodec_send_packet(AVCodecContext *avctx, const AVPacke
         ret = av_packet_ref(avci->buffer_pkt, avpkt);
         if (ret < 0)
             return ret;
-    }
+    } else
+        avci->d->draining_started = 1;
 
     ret = av_bsf_send_packet(avci->bsf, avci->buffer_pkt);
     if (ret < 0) {
@@ -1758,6 +1764,7 @@ void ff_decode_flush_buffers(AVCodecContext *avctx)
     av_bsf_flush(avci->bsf);
 
     avci->d->nb_draining_errors = 0;
+    avci->d->draining_started   = 0;
 }
 
 AVCodecInternal *ff_decode_internal_alloc(void)
-- 
2.40.1

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 8/9] lavc/decode: move submitting input packets to bitstream filters

[Anton Khirnov]

Do it from ff_decode_get_packet() rather than from
avcodec_send_packet(). This way all nontrivial stages of the decoding
pipeline (i.e. other than just placing a packet at its entrance) are
pull-based rather than a mix of push an pull.
---
 libavcodec/decode.c | 36 ++++++++++++++++++++++++++----------
 1 file changed, 26 insertions(+), 10 deletions(-)

diff --git a/libavcodec/decode.c b/libavcodec/decode.c
index c61ce74fb8..8d892432be 100644
--- a/libavcodec/decode.c
+++ b/libavcodec/decode.c
@@ -193,14 +193,11 @@ fail:
     return ret;
 }
 
-int ff_decode_get_packet(AVCodecContext *avctx, AVPacket *pkt)
+static int decode_get_packet(AVCodecContext *avctx, AVPacket *pkt)
 {
     AVCodecInternal *avci = avctx->internal;
     int ret;
 
-    if (avci->draining)
-        return AVERROR_EOF;
-
     ret = av_bsf_receive_packet(avci->bsf, pkt);
     if (ret == AVERROR_EOF)
         avci->draining = 1;
@@ -223,6 +220,31 @@ finish:
     return ret;
 }
 
+int ff_decode_get_packet(AVCodecContext *avctx, AVPacket *pkt)
+{
+    AVCodecInternal *avci = avctx->internal;
+
+    if (avci->draining)
+        return AVERROR_EOF;
+
+    while (1) {
+        int ret = decode_get_packet(avctx, pkt);
+        if (ret == AVERROR(EAGAIN) &&
+            (avci->buffer_pkt->data || avci->buffer_pkt->side_data_elems ||
+             avci->d->draining_started)) {
+            ret = av_bsf_send_packet(avci->bsf, avci->buffer_pkt);
+            if (ret < 0) {
+                av_packet_unref(avci->buffer_pkt);
+                return ret;
+            }
+
+            continue;
+        }
+
+        return ret;
+    }
+}
+
 /**
  * Attempt to guess proper monotonic timestamps for decoded video frames
  * which might have incorrect times. Input timestamps may wrap around, in
@@ -643,12 +665,6 @@ int attribute_align_arg avcodec_send_packet(AVCodecContext *avctx, const AVPacke
     } else
         avci->d->draining_started = 1;
 
-    ret = av_bsf_send_packet(avci->bsf, avci->buffer_pkt);
-    if (ret < 0) {
-        av_packet_unref(avci->buffer_pkt);
-        return ret;
-    }
-
     if (!avci->buffer_frame->buf[0]) {
         ret = decode_receive_frame_internal(avctx, avci->buffer_frame);
         if (ret < 0 && ret != AVERROR(EAGAIN) && ret != AVERROR_EOF)
-- 
2.40.1

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 8/9] lavc/decode: move submitting input packets to bitstream filters

[James Almer]

On 6/20/2023 11:16 AM, Anton Khirnov wrote:
> Do it from ff_decode_get_packet() rather than from
> avcodec_send_packet(). This way all nontrivial stages of the decoding
> pipeline (i.e. other than just placing a packet at its entrance) are
> pull-based rather than a mix of push an pull.
> ---
>   libavcodec/decode.c | 36 ++++++++++++++++++++++++++----------
>   1 file changed, 26 insertions(+), 10 deletions(-)
> 
> diff --git a/libavcodec/decode.c b/libavcodec/decode.c
> index c61ce74fb8..8d892432be 100644
> --- a/libavcodec/decode.c
> +++ b/libavcodec/decode.c
> @@ -193,14 +193,11 @@ fail:
>       return ret;
>   }
>   
> -int ff_decode_get_packet(AVCodecContext *avctx, AVPacket *pkt)
> +static int decode_get_packet(AVCodecContext *avctx, AVPacket *pkt)
>   {
>       AVCodecInternal *avci = avctx->internal;
>       int ret;
>   
> -    if (avci->draining)
> -        return AVERROR_EOF;
> -
>       ret = av_bsf_receive_packet(avci->bsf, pkt);
>       if (ret == AVERROR_EOF)
>           avci->draining = 1;
> @@ -223,6 +220,31 @@ finish:
>       return ret;
>   }
>   
> +int ff_decode_get_packet(AVCodecContext *avctx, AVPacket *pkt)
> +{
> +    AVCodecInternal *avci = avctx->internal;
> +
> +    if (avci->draining)
> +        return AVERROR_EOF;
> +
> +    while (1) {
> +        int ret = decode_get_packet(avctx, pkt);
> +        if (ret == AVERROR(EAGAIN) &&
> +            (avci->buffer_pkt->data || avci->buffer_pkt->side_data_elems ||

nit: Since i removed the IS_EMPTY() macro recently from this file, you 
could move the one in bsf.c into packet_internal.h, rename it to 
something like AVPACKET_IS_EMPTY(), and use it here.


> +             avci->d->draining_started)) {
> +            ret = av_bsf_send_packet(avci->bsf, avci->buffer_pkt);
> +            if (ret < 0) {
> +                av_packet_unref(avci->buffer_pkt);
> +                return ret;
> +            }
> +
> +            continue;
> +        }
> +
> +        return ret;
> +    }
> +}
> +
>   /**
>    * Attempt to guess proper monotonic timestamps for decoded video frames
>    * which might have incorrect times. Input timestamps may wrap around, in
> @@ -643,12 +665,6 @@ int attribute_align_arg avcodec_send_packet(AVCodecContext *avctx, const AVPacke
>       } else
>           avci->d->draining_started = 1;
>   
> -    ret = av_bsf_send_packet(avci->bsf, avci->buffer_pkt);
> -    if (ret < 0) {
> -        av_packet_unref(avci->buffer_pkt);
> -        return ret;
> -    }
> -
>       if (!avci->buffer_frame->buf[0]) {
>           ret = decode_receive_frame_internal(avctx, avci->buffer_frame);
>           if (ret < 0 && ret != AVERROR(EAGAIN) && ret != AVERROR_EOF)
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 9/9] lavc/decode: do not perform decoding when sending draining packets

[Anton Khirnov]

This way decoding error will not be returned when the user starts
draining the decoder, avoiding confusion over whether draining did or
did not start.

Fixes failures of fate-h264-attachment-631 for certain numbers of frame
threads (e.g. 5).
---
 libavcodec/decode.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavcodec/decode.c b/libavcodec/decode.c
index 8d892432be..67f2f052e5 100644
--- a/libavcodec/decode.c
+++ b/libavcodec/decode.c
@@ -665,7 +665,7 @@ int attribute_align_arg avcodec_send_packet(AVCodecContext *avctx, const AVPacke
     } else
         avci->d->draining_started = 1;
 
-    if (!avci->buffer_frame->buf[0]) {
+    if (!avci->buffer_frame->buf[0] && !avci->d->draining_started) {
         ret = decode_receive_frame_internal(avctx, avci->buffer_frame);
         if (ret < 0 && ret != AVERROR(EAGAIN) && ret != AVERROR_EOF)
             return ret;
-- 
2.40.1

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".