diff mbox series

[FFmpeg-devel,38/42] avcodec/codec_internal: Remove FF_CODEC_CAP_ALLOCATE_PROGRESS

Message ID AS8P250MB0744902A9A0035FDDF5EA9108FFAA@AS8P250MB0744.EURP250.PROD.OUTLOOK.COM
State New
Headers show
Series New API for reference counting and ThreadFrames | expand

Commit Message

Andreas Rheinhardt Sept. 19, 2023, 7:57 p.m. UTC
Before commit f025b8e110b36c1cdb4fb56c4cd57aeca1767b5b,
every frame-threaded decoder used ThreadFrames, even when
they did not have any inter-frame dependencies at all.
In order to distinguish those decoders that need the AVBuffer
for progress communication from those that do not (to avoid
the allocation for the latter), the former decoders were marked
with the FF_CODEC_CAP_ALLOCATE_PROGRESS internal codec cap.

Yet distinguishing these two can be done in a more natural way:
Don't use ThreadFrames when not needed and split ff_thread_get_buffer()
into a core function that calls the user's get_buffer2 callback
and a wrapper around it that also allocates the progress AVBuffer.
This has been done in 02220b88fc38ef9dd4f2d519f5d3e4151258b60c
and since that commit the ALLOCATE_PROGRESS cap was nearly redundant.

The only exception was WebP and VP8. WebP can contain VP8
and uses the VP8 decoder directly (i.e. they share the same
AVCodecContext). Both decoders are frame-threaded and VP8
has inter-frame dependencies (in general, not in valid WebP)
and therefore the ALLOCATE_PROGRESS cap. In order to avoid
allocating progress in case of a frame-threaded WebP decoder
the cap and the check for the cap has been kept in place.

Yet now the VP8 decoder has been switched to use ProgressFrames
and therefore there is just no reason any more for this check
and the cap. This commit therefore removes both.

Also change the value of FF_CODEC_CAP_USES_PROGRESSFRAMES
to leave no gaps.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
 doc/multithreading.txt      |  8 ++++----
 libavcodec/codec_internal.h |  7 +------
 libavcodec/ffv1dec.c        |  3 +--
 libavcodec/h264dec.c        |  2 +-
 libavcodec/hevcdec.c        |  2 +-
 libavcodec/mpeg4videodec.c  |  3 +--
 libavcodec/pngdec.c         |  3 +--
 libavcodec/pthread_frame.c  | 12 +++++-------
 libavcodec/rv30.c           |  1 -
 libavcodec/rv40.c           |  1 -
 libavcodec/tests/avcodec.c  |  7 +------
 11 files changed, 16 insertions(+), 33 deletions(-)

Comments

Anton Khirnov Oct. 25, 2023, 1:38 p.m. UTC | #1
Quoting Andreas Rheinhardt (2023-09-19 21:57:30)
> Before commit f025b8e110b36c1cdb4fb56c4cd57aeca1767b5b,
> every frame-threaded decoder used ThreadFrames, even when
> they did not have any inter-frame dependencies at all.
> In order to distinguish those decoders that need the AVBuffer
> for progress communication from those that do not (to avoid
> the allocation for the latter), the former decoders were marked
> with the FF_CODEC_CAP_ALLOCATE_PROGRESS internal codec cap.
> 
> Yet distinguishing these two can be done in a more natural way:
> Don't use ThreadFrames when not needed and split ff_thread_get_buffer()
> into a core function that calls the user's get_buffer2 callback
> and a wrapper around it that also allocates the progress AVBuffer.
> This has been done in 02220b88fc38ef9dd4f2d519f5d3e4151258b60c
> and since that commit the ALLOCATE_PROGRESS cap was nearly redundant.
> 
> The only exception was WebP and VP8. WebP can contain VP8
> and uses the VP8 decoder directly (i.e. they share the same
> AVCodecContext). Both decoders are frame-threaded and VP8
> has inter-frame dependencies (in general, not in valid WebP)
> and therefore the ALLOCATE_PROGRESS cap. In order to avoid
> allocating progress in case of a frame-threaded WebP decoder
> the cap and the check for the cap has been kept in place.
> 
> Yet now the VP8 decoder has been switched to use ProgressFrames
> and therefore there is just no reason any more for this check
> and the cap. This commit therefore removes both.
> 
> Also change the value of FF_CODEC_CAP_USES_PROGRESSFRAMES
> to leave no gaps.
> 
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
> ---
>  doc/multithreading.txt      |  8 ++++----
>  libavcodec/codec_internal.h |  7 +------
>  libavcodec/ffv1dec.c        |  3 +--
>  libavcodec/h264dec.c        |  2 +-
>  libavcodec/hevcdec.c        |  2 +-
>  libavcodec/mpeg4videodec.c  |  3 +--
>  libavcodec/pngdec.c         |  3 +--
>  libavcodec/pthread_frame.c  | 12 +++++-------
>  libavcodec/rv30.c           |  1 -
>  libavcodec/rv40.c           |  1 -
>  libavcodec/tests/avcodec.c  |  7 +------
>  11 files changed, 16 insertions(+), 33 deletions(-)

LGTM
diff mbox series

Patch

diff --git a/doc/multithreading.txt b/doc/multithreading.txt
index 470194ff85..0fb467392d 100644
--- a/doc/multithreading.txt
+++ b/doc/multithreading.txt
@@ -53,10 +53,10 @@  thread.
 Add AV_CODEC_CAP_FRAME_THREADS to the codec capabilities. There will be very little
 speed gain at this point but it should work.
 
-If there are inter-frame dependencies, so the codec calls
-ff_thread_report/await_progress(), set FF_CODEC_CAP_ALLOCATE_PROGRESS in
-AVCodec.caps_internal and use ff_thread_get_buffer() to allocate frames. The
-frames must then be freed with ff_thread_release_buffer().
+Use ff_thread_get_buffer() (or ff_thread_get_ext_buffer() or
+ff_thread_progress_get_buffer() in case you have inter-frame dependencies)
+to allocate frames. The frames must then be freed with ff_thread_release_buffer()
+(or ff_thread_release_ext_buffer() or ff_thread_progress_unref()).
 Otherwise decode directly into the user-supplied frames.
 
 Call ff_thread_report_progress() after some part of the current picture has decoded.
diff --git a/libavcodec/codec_internal.h b/libavcodec/codec_internal.h
index c03b64ceba..4ddf394c0b 100644
--- a/libavcodec/codec_internal.h
+++ b/libavcodec/codec_internal.h
@@ -65,12 +65,7 @@ 
 /**
  * The decoder might make use of the ProgressFrame API.
  */
-#define FF_CODEC_CAP_USES_PROGRESSFRAMES    (1 << 11)
-/*
- * The codec supports frame threading and has inter-frame dependencies, so it
- * uses ff_thread_report/await_progress().
- */
-#define FF_CODEC_CAP_ALLOCATE_PROGRESS      (1 << 6)
+#define FF_CODEC_CAP_USES_PROGRESSFRAMES    (1 << 6)
 /**
  * Codec handles avctx->thread_count == 0 (auto) internally.
  */
diff --git a/libavcodec/ffv1dec.c b/libavcodec/ffv1dec.c
index 54cf075b8f..20cc345780 100644
--- a/libavcodec/ffv1dec.c
+++ b/libavcodec/ffv1dec.c
@@ -1125,6 +1125,5 @@  const FFCodec ff_ffv1_decoder = {
     UPDATE_THREAD_CONTEXT(update_thread_context),
     .p.capabilities = AV_CODEC_CAP_DR1 |
                       AV_CODEC_CAP_FRAME_THREADS | AV_CODEC_CAP_SLICE_THREADS,
-    .caps_internal  = FF_CODEC_CAP_INIT_CLEANUP |
-                      FF_CODEC_CAP_ALLOCATE_PROGRESS,
+    .caps_internal  = FF_CODEC_CAP_INIT_CLEANUP,
 };
diff --git a/libavcodec/h264dec.c b/libavcodec/h264dec.c
index 8578d3b346..39666faace 100644
--- a/libavcodec/h264dec.c
+++ b/libavcodec/h264dec.c
@@ -1140,7 +1140,7 @@  const FFCodec ff_h264_decoder = {
                                NULL
                            },
     .caps_internal         = FF_CODEC_CAP_EXPORTS_CROPPING |
-                             FF_CODEC_CAP_ALLOCATE_PROGRESS | FF_CODEC_CAP_INIT_CLEANUP,
+                             FF_CODEC_CAP_INIT_CLEANUP,
     .flush                 = h264_decode_flush,
     UPDATE_THREAD_CONTEXT(ff_h264_update_thread_context),
     UPDATE_THREAD_CONTEXT_FOR_USER(ff_h264_update_thread_context_for_user),
diff --git a/libavcodec/hevcdec.c b/libavcodec/hevcdec.c
index 23cc543f82..c8067a736e 100644
--- a/libavcodec/hevcdec.c
+++ b/libavcodec/hevcdec.c
@@ -3671,7 +3671,7 @@  const FFCodec ff_hevc_decoder = {
     .p.capabilities        = AV_CODEC_CAP_DR1 | AV_CODEC_CAP_DELAY |
                              AV_CODEC_CAP_SLICE_THREADS | AV_CODEC_CAP_FRAME_THREADS,
     .caps_internal         = FF_CODEC_CAP_EXPORTS_CROPPING |
-                             FF_CODEC_CAP_ALLOCATE_PROGRESS | FF_CODEC_CAP_INIT_CLEANUP,
+                             FF_CODEC_CAP_INIT_CLEANUP,
     .p.profiles            = NULL_IF_CONFIG_SMALL(ff_hevc_profiles),
     .hw_configs            = (const AVCodecHWConfigInternal *const []) {
 #if CONFIG_HEVC_DXVA2_HWACCEL
diff --git a/libavcodec/mpeg4videodec.c b/libavcodec/mpeg4videodec.c
index a8dd57bf6b..e0b0e1e08c 100644
--- a/libavcodec/mpeg4videodec.c
+++ b/libavcodec/mpeg4videodec.c
@@ -3860,8 +3860,7 @@  const FFCodec ff_mpeg4_decoder = {
     FF_CODEC_DECODE_CB(ff_h263_decode_frame),
     .p.capabilities        = AV_CODEC_CAP_DRAW_HORIZ_BAND | AV_CODEC_CAP_DR1 |
                              AV_CODEC_CAP_DELAY | AV_CODEC_CAP_FRAME_THREADS,
-    .caps_internal         = FF_CODEC_CAP_SKIP_FRAME_FILL_PARAM |
-                             FF_CODEC_CAP_ALLOCATE_PROGRESS,
+    .caps_internal         = FF_CODEC_CAP_SKIP_FRAME_FILL_PARAM,
     .flush                 = ff_mpeg_flush,
     .p.max_lowres          = 3,
     .p.pix_fmts            = ff_h263_hwaccel_pixfmt_list_420,
diff --git a/libavcodec/pngdec.c b/libavcodec/pngdec.c
index 0369d1c449..c011ca386e 100644
--- a/libavcodec/pngdec.c
+++ b/libavcodec/pngdec.c
@@ -1862,7 +1862,6 @@  const FFCodec ff_apng_decoder = {
     UPDATE_THREAD_CONTEXT(update_thread_context),
     .p.capabilities = AV_CODEC_CAP_DR1 | AV_CODEC_CAP_FRAME_THREADS,
     .caps_internal  = FF_CODEC_CAP_INIT_CLEANUP |
-                      FF_CODEC_CAP_ALLOCATE_PROGRESS |
                       FF_CODEC_CAP_ICC_PROFILES,
 };
 #endif
@@ -1880,7 +1879,7 @@  const FFCodec ff_png_decoder = {
     UPDATE_THREAD_CONTEXT(update_thread_context),
     .p.capabilities = AV_CODEC_CAP_DR1 | AV_CODEC_CAP_FRAME_THREADS,
     .caps_internal  = FF_CODEC_CAP_SKIP_FRAME_FILL_PARAM |
-                      FF_CODEC_CAP_ALLOCATE_PROGRESS | FF_CODEC_CAP_INIT_CLEANUP |
+                      FF_CODEC_CAP_INIT_CLEANUP |
                       FF_CODEC_CAP_ICC_PROFILES,
 };
 #endif
diff --git a/libavcodec/pthread_frame.c b/libavcodec/pthread_frame.c
index 219ab16ccd..28b727dec9 100644
--- a/libavcodec/pthread_frame.c
+++ b/libavcodec/pthread_frame.c
@@ -998,14 +998,12 @@  int ff_thread_get_ext_buffer(AVCodecContext *avctx, ThreadFrame *f, int flags)
     if (!(avctx->active_thread_type & FF_THREAD_FRAME))
         return ff_get_buffer(avctx, f->f, flags);
 
-    if (ffcodec(avctx->codec)->caps_internal & FF_CODEC_CAP_ALLOCATE_PROGRESS) {
-        f->progress = ff_refstruct_allocz(sizeof(*f->progress));
-        if (!f->progress)
-            return AVERROR(ENOMEM);
+    f->progress = ff_refstruct_allocz(sizeof(*f->progress));
+    if (!f->progress)
+        return AVERROR(ENOMEM);
 
-        atomic_init(&f->progress->progress[0], -1);
-        atomic_init(&f->progress->progress[1], -1);
-    }
+    atomic_init(&f->progress->progress[0], -1);
+    atomic_init(&f->progress->progress[1], -1);
 
     ret = ff_thread_get_buffer(avctx, f->f, flags);
     if (ret)
diff --git a/libavcodec/rv30.c b/libavcodec/rv30.c
index be62577f99..d0e10722cd 100644
--- a/libavcodec/rv30.c
+++ b/libavcodec/rv30.c
@@ -308,5 +308,4 @@  const FFCodec ff_rv30_decoder = {
         AV_PIX_FMT_NONE
     },
     UPDATE_THREAD_CONTEXT(ff_rv34_decode_update_thread_context),
-    .caps_internal         = FF_CODEC_CAP_ALLOCATE_PROGRESS,
 };
diff --git a/libavcodec/rv40.c b/libavcodec/rv40.c
index d2f8ef9f5a..689432b4bb 100644
--- a/libavcodec/rv40.c
+++ b/libavcodec/rv40.c
@@ -591,5 +591,4 @@  const FFCodec ff_rv40_decoder = {
         AV_PIX_FMT_NONE
     },
     UPDATE_THREAD_CONTEXT(ff_rv34_decode_update_thread_context),
-    .caps_internal         = FF_CODEC_CAP_ALLOCATE_PROGRESS,
 };
diff --git a/libavcodec/tests/avcodec.c b/libavcodec/tests/avcodec.c
index f6e394c78d..d65f682c76 100644
--- a/libavcodec/tests/avcodec.c
+++ b/libavcodec/tests/avcodec.c
@@ -141,8 +141,7 @@  int main(void){
                     ret = 1;
                 }
             }
-            if (codec2->caps_internal & (FF_CODEC_CAP_ALLOCATE_PROGRESS |
-                                        FF_CODEC_CAP_SETS_PKT_DTS |
+            if (codec2->caps_internal & (FF_CODEC_CAP_SETS_PKT_DTS |
                                         FF_CODEC_CAP_SKIP_FRAME_FILL_PARAM |
                                         FF_CODEC_CAP_EXPORTS_CROPPING |
                                         FF_CODEC_CAP_SETS_FRAME_PROPS |
@@ -172,10 +171,6 @@  int main(void){
                                        AV_CODEC_CAP_ENCODER_REORDERED_OPAQUE |
                                        AV_CODEC_CAP_ENCODER_FLUSH))
                 ERR("Decoder %s has encoder-only capabilities\n");
-            if (codec2->caps_internal & FF_CODEC_CAP_ALLOCATE_PROGRESS &&
-                !(codec->capabilities & AV_CODEC_CAP_FRAME_THREADS))
-                ERR("Decoder %s wants allocated progress without supporting"
-                    "frame threads\n");
             if (codec2->cb_type != FF_CODEC_CB_TYPE_DECODE &&
                 codec2->caps_internal & FF_CODEC_CAP_SETS_PKT_DTS)
                 ERR("Decoder %s is marked as setting pkt_dts when it doesn't have"