diff mbox series

[FFmpeg-devel,2/2] avcodec/decode: Don't allocate FrameDecodeData if it is unneeded

Message ID DB6PR0101MB2214CE24CD07EE826635011D8FC89@DB6PR0101MB2214.eurprd01.prod.exchangelabs.com
State New
Headers show
Series [FFmpeg-devel,1/2] avcodec/wrapped_avframe: Don't attach FrameDecodeData unnecessarily | expand

Checks

Context Check Description
yinshiyou/make_loongarch64 success Make finished
yinshiyou/make_fate_loongarch64 success Make fate finished
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished

Commit Message

Andreas Rheinhardt May 11, 2022, 4:17 p.m. UTC 
Currently, every call to ff_get_buffer() allocates FrameDecodeData,
although only very few decoders (those that might use nvdec or
videotoolbox hardware acceleration) actually need it.

This commit addresses this by adding an internal codec cap
to ensure that FrameDecodeData is only allocated for codecs
that might need it (i.e. for which there is a hardware acceleration
that might actually need it).

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
1. It seems that this has even been allocated for some encoders
before c954cf1e1b766a0d1992d5be0a8be0055a8e1a6a.
2. Given that the videotoolbox hwaccels call ff_attach_decode_data()
themselves, they actually don't need the generic part of
ff_get_buffer() to allocate it for them; in particular, one could
avoid adding the flag to the prores decoder. I can modify the patch
in this way if desired (it would also necessitate modifying the assert
in decode_receive_frame_internal()).
3. Unfortunately, I don't have hardware to actually test nvdec or
videotoolbox, so only the "it works if allocating it is certainly
unnecessary" has been tested. It would be nice if someone could 
also test the other part.
4. One could of course set these flags only if the relevant hwaccels
are enabled at compile-time.
5. Unfortunately, it seems to be too late to call
ff_attach_decode_data() from ff_nvdec_start_frame(), as it is called
after ff_thread_finish_setup() might have been called, so modifying
the frame would be a race.
6. Maybe the following would also work: Add a caps_internal to AVHWAccel
and only allocate FrameDecodeData if the hardware acceleration currently
in use needs it (as given by this new cap). It should work (hopefully)
and would be cleaner.

 libavcodec/av1dec.c         |  3 ++-
 libavcodec/codec_internal.h |  5 +++++
 libavcodec/decode.c         | 16 +++++++++-------
 libavcodec/h263dec.c        |  6 ++++--
 libavcodec/h264dec.c        |  3 ++-
 libavcodec/hevcdec.c        |  3 ++-
 libavcodec/mjpegdec.c       |  3 ++-
 libavcodec/mpeg12dec.c      |  6 ++++--
 libavcodec/mpeg4videodec.c  |  3 ++-
 libavcodec/proresdec2.c     |  3 ++-
 libavcodec/tests/avcodec.c  | 15 ++++++++++++++-
 libavcodec/vc1dec.c         |  6 ++++--
 libavcodec/vp8.c            |  3 ++-
 libavcodec/vp9.c            |  3 ++-
 14 files changed, 56 insertions(+), 22 deletions(-)
diff mbox series

Patch

diff --git a/libavcodec/av1dec.c b/libavcodec/av1dec.c
index 1c09b1d6d6..291ba94a36 100644
--- a/libavcodec/av1dec.c
+++ b/libavcodec/av1dec.c
@@ -1250,7 +1250,8 @@  const FFCodec ff_av1_decoder = {
     .p.capabilities        = AV_CODEC_CAP_DR1 | AV_CODEC_CAP_AVOID_PROBING,
     .caps_internal         = FF_CODEC_CAP_INIT_THREADSAFE |
                              FF_CODEC_CAP_INIT_CLEANUP |
-                             FF_CODEC_CAP_SETS_PKT_DTS,
+                             FF_CODEC_CAP_SETS_PKT_DTS |
+                             FF_CODEC_CAP_FRAME_DECODE_DATA,
     .flush                 = av1_decode_flush,
     .p.profiles            = NULL_IF_CONFIG_SMALL(ff_av1_profiles),
     .p.priv_class          = &av1_class,
diff --git a/libavcodec/codec_internal.h b/libavcodec/codec_internal.h
index 5df286ce52..cefe3fdd1a 100644
--- a/libavcodec/codec_internal.h
+++ b/libavcodec/codec_internal.h
@@ -73,6 +73,11 @@ 
  * internal logic derive them from AVCodecInternal.last_pkt_props.
  */
 #define FF_CODEC_CAP_SETS_FRAME_PROPS       (1 << 8)
+/**
+ * This decoder might use FrameDecodeData. In particular, ff_get_buffer()
+ * shall allocate it.
+ */
+#define FF_CODEC_CAP_FRAME_DECODE_DATA      (1 << 9)
 
 /**
  * FFCodec.codec_tags termination value
diff --git a/libavcodec/decode.c b/libavcodec/decode.c
index 264fc66a81..33c97c41a6 100644
--- a/libavcodec/decode.c
+++ b/libavcodec/decode.c
@@ -548,10 +548,10 @@  static int decode_receive_frame_internal(AVCodecContext *avctx, AVFrame *frame)
                                                          frame->pts,
                                                          frame->pkt_dts);
 
-        /* the only case where decode data is not set should be decoders
-         * that do not call ff_get_buffer() */
-        av_assert0((frame->private_ref && frame->private_ref->size == sizeof(FrameDecodeData)) ||
-                   !(avctx->codec->capabilities & AV_CODEC_CAP_DR1));
+        /* the only case where decode data may be set should be decoders
+         * with the FF_CODEC_CAP_FRAME_DECODE_DATA cap */
+        av_assert0(!frame->private_ref ||
+                   codec->caps_internal & FF_CODEC_CAP_FRAME_DECODE_DATA);
 
         if (frame->private_ref) {
             FrameDecodeData *fdd = (FrameDecodeData*)frame->private_ref->data;
@@ -1460,9 +1460,11 @@  FF_ENABLE_DEPRECATION_WARNINGS
 
     validate_avframe_allocation(avctx, frame);
 
-    ret = ff_attach_decode_data(frame);
-    if (ret < 0)
-        goto fail;
+    if (ffcodec(avctx->codec)->caps_internal & FF_CODEC_CAP_FRAME_DECODE_DATA) {
+        ret = ff_attach_decode_data(frame);
+        if (ret < 0)
+            goto fail;
+    }
 
 end:
     if (avctx->codec_type == AVMEDIA_TYPE_VIDEO && !override_dimensions &&
diff --git a/libavcodec/h263dec.c b/libavcodec/h263dec.c
index 1f9f3e5e95..8160b394e2 100644
--- a/libavcodec/h263dec.c
+++ b/libavcodec/h263dec.c
@@ -769,7 +769,8 @@  const FFCodec ff_h263_decoder = {
 #endif
                       AV_CODEC_CAP_DELAY,
     .caps_internal  = FF_CODEC_CAP_INIT_THREADSAFE |
-                      FF_CODEC_CAP_SKIP_FRAME_FILL_PARAM,
+                      FF_CODEC_CAP_SKIP_FRAME_FILL_PARAM |
+                      FF_CODEC_CAP_FRAME_DECODE_DATA,
     .flush          = ff_mpeg_flush,
     .p.max_lowres   = 3,
     .p.pix_fmts     = ff_h263_hwaccel_pixfmt_list_420,
@@ -791,7 +792,8 @@  const FFCodec ff_h263p_decoder = {
 #endif
                       AV_CODEC_CAP_DELAY,
     .caps_internal  = FF_CODEC_CAP_INIT_THREADSAFE |
-                      FF_CODEC_CAP_SKIP_FRAME_FILL_PARAM,
+                      FF_CODEC_CAP_SKIP_FRAME_FILL_PARAM |
+                      FF_CODEC_CAP_FRAME_DECODE_DATA,
     .flush          = ff_mpeg_flush,
     .p.max_lowres   = 3,
     .p.pix_fmts     = ff_h263_hwaccel_pixfmt_list_420,
diff --git a/libavcodec/h264dec.c b/libavcodec/h264dec.c
index d80bc6b17f..a7190dbfc5 100644
--- a/libavcodec/h264dec.c
+++ b/libavcodec/h264dec.c
@@ -1099,7 +1099,8 @@  const FFCodec ff_h264_decoder = {
                                NULL
                            },
     .caps_internal         = FF_CODEC_CAP_INIT_THREADSAFE | FF_CODEC_CAP_EXPORTS_CROPPING |
-                             FF_CODEC_CAP_ALLOCATE_PROGRESS | FF_CODEC_CAP_INIT_CLEANUP,
+                             FF_CODEC_CAP_ALLOCATE_PROGRESS | FF_CODEC_CAP_INIT_CLEANUP |
+                             FF_CODEC_CAP_FRAME_DECODE_DATA,
     .flush                 = h264_decode_flush,
     .update_thread_context = ONLY_IF_THREADS_ENABLED(ff_h264_update_thread_context),
     .update_thread_context_for_user = ONLY_IF_THREADS_ENABLED(ff_h264_update_thread_context_for_user),
diff --git a/libavcodec/hevcdec.c b/libavcodec/hevcdec.c
index f782ea6394..df5822f365 100644
--- a/libavcodec/hevcdec.c
+++ b/libavcodec/hevcdec.c
@@ -3890,7 +3890,8 @@  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_INIT_THREADSAFE | FF_CODEC_CAP_EXPORTS_CROPPING |
-                             FF_CODEC_CAP_ALLOCATE_PROGRESS | FF_CODEC_CAP_INIT_CLEANUP,
+                             FF_CODEC_CAP_ALLOCATE_PROGRESS | FF_CODEC_CAP_INIT_CLEANUP |
+                             FF_CODEC_CAP_FRAME_DECODE_DATA,
     .p.profiles            = NULL_IF_CONFIG_SMALL(ff_hevc_profiles),
     .hw_configs            = (const AVCodecHWConfigInternal *const []) {
 #if CONFIG_HEVC_DXVA2_HWACCEL
diff --git a/libavcodec/mjpegdec.c b/libavcodec/mjpegdec.c
index 32874a5a19..aee4871346 100644
--- a/libavcodec/mjpegdec.c
+++ b/libavcodec/mjpegdec.c
@@ -3027,7 +3027,8 @@  const FFCodec ff_mjpeg_decoder = {
     .p.priv_class   = &mjpegdec_class,
     .p.profiles     = NULL_IF_CONFIG_SMALL(ff_mjpeg_profiles),
     .caps_internal  = FF_CODEC_CAP_INIT_THREADSAFE | FF_CODEC_CAP_INIT_CLEANUP |
-                      FF_CODEC_CAP_SKIP_FRAME_FILL_PARAM | FF_CODEC_CAP_SETS_PKT_DTS,
+                      FF_CODEC_CAP_SKIP_FRAME_FILL_PARAM | FF_CODEC_CAP_SETS_PKT_DTS |
+                      FF_CODEC_CAP_FRAME_DECODE_DATA,
     .hw_configs     = (const AVCodecHWConfigInternal *const []) {
 #if CONFIG_MJPEG_NVDEC_HWACCEL
                         HWACCEL_NVDEC(mjpeg),
diff --git a/libavcodec/mpeg12dec.c b/libavcodec/mpeg12dec.c
index e9bde48f7a..53c14a7a04 100644
--- a/libavcodec/mpeg12dec.c
+++ b/libavcodec/mpeg12dec.c
@@ -2877,7 +2877,8 @@  const FFCodec ff_mpeg1video_decoder = {
 #endif
                              AV_CODEC_CAP_DELAY | AV_CODEC_CAP_SLICE_THREADS,
     .caps_internal         = FF_CODEC_CAP_INIT_THREADSAFE |
-                             FF_CODEC_CAP_SKIP_FRAME_FILL_PARAM,
+                             FF_CODEC_CAP_SKIP_FRAME_FILL_PARAM |
+                             FF_CODEC_CAP_FRAME_DECODE_DATA,
     .flush                 = flush,
     .p.max_lowres          = 3,
     .update_thread_context = ONLY_IF_THREADS_ENABLED(mpeg_decode_update_thread_context),
@@ -2910,7 +2911,8 @@  const FFCodec ff_mpeg2video_decoder = {
 #endif
                       AV_CODEC_CAP_DELAY | AV_CODEC_CAP_SLICE_THREADS,
     .caps_internal  = FF_CODEC_CAP_INIT_THREADSAFE |
-                      FF_CODEC_CAP_SKIP_FRAME_FILL_PARAM,
+                      FF_CODEC_CAP_SKIP_FRAME_FILL_PARAM |
+                      FF_CODEC_CAP_FRAME_DECODE_DATA,
     .flush          = flush,
     .p.max_lowres   = 3,
     .p.profiles     = NULL_IF_CONFIG_SMALL(ff_mpeg2_video_profiles),
diff --git a/libavcodec/mpeg4videodec.c b/libavcodec/mpeg4videodec.c
index e2bde73639..99d85495ea 100644
--- a/libavcodec/mpeg4videodec.c
+++ b/libavcodec/mpeg4videodec.c
@@ -3678,7 +3678,8 @@  const FFCodec ff_mpeg4_decoder = {
                              AV_CODEC_CAP_DELAY | AV_CODEC_CAP_FRAME_THREADS,
     .caps_internal         = FF_CODEC_CAP_INIT_THREADSAFE |
                              FF_CODEC_CAP_SKIP_FRAME_FILL_PARAM |
-                             FF_CODEC_CAP_ALLOCATE_PROGRESS,
+                             FF_CODEC_CAP_ALLOCATE_PROGRESS |
+                             FF_CODEC_CAP_FRAME_DECODE_DATA,
     .flush                 = ff_mpeg_flush,
     .p.max_lowres          = 3,
     .p.pix_fmts            = ff_h263_hwaccel_pixfmt_list_420,
diff --git a/libavcodec/proresdec2.c b/libavcodec/proresdec2.c
index dba64a2489..64a38a2a14 100644
--- a/libavcodec/proresdec2.c
+++ b/libavcodec/proresdec2.c
@@ -882,7 +882,8 @@  const FFCodec ff_prores_decoder = {
     .update_thread_context = ONLY_IF_THREADS_ENABLED(update_thread_context),
     .p.capabilities = AV_CODEC_CAP_DR1 | AV_CODEC_CAP_SLICE_THREADS | AV_CODEC_CAP_FRAME_THREADS,
     .p.profiles     = NULL_IF_CONFIG_SMALL(ff_prores_profiles),
-    .caps_internal  = FF_CODEC_CAP_INIT_THREADSAFE,
+    .caps_internal  = FF_CODEC_CAP_INIT_THREADSAFE |
+                      FF_CODEC_CAP_FRAME_DECODE_DATA,
     .hw_configs     = (const AVCodecHWConfigInternal *const []) {
 #if CONFIG_PRORES_VIDEOTOOLBOX_HWACCEL
         HWACCEL_VIDEOTOOLBOX(prores),
diff --git a/libavcodec/tests/avcodec.c b/libavcodec/tests/avcodec.c
index 08b5fbede1..1ed7eb0a90 100644
--- a/libavcodec/tests/avcodec.c
+++ b/libavcodec/tests/avcodec.c
@@ -20,6 +20,7 @@ 
 #include "libavcodec/codec.h"
 #include "libavcodec/codec_desc.h"
 #include "libavcodec/codec_internal.h"
+#include "libavcodec/hwconfig.h"
 #include "libavcodec/internal.h"
 
 static const char *get_type_string(enum AVMediaType type)
@@ -146,7 +147,8 @@  int main(void){
                                         FF_CODEC_CAP_SETS_PKT_DTS |
                                         FF_CODEC_CAP_SKIP_FRAME_FILL_PARAM |
                                         FF_CODEC_CAP_EXPORTS_CROPPING |
-                                        FF_CODEC_CAP_SETS_FRAME_PROPS) ||
+                                        FF_CODEC_CAP_SETS_FRAME_PROPS |
+                                        FF_CODEC_CAP_FRAME_DECODE_DATA) ||
                 codec->capabilities  & (AV_CODEC_CAP_AVOID_PROBING |
                                         AV_CODEC_CAP_CHANNEL_CONF  |
                                         AV_CODEC_CAP_DRAW_HORIZ_BAND |
@@ -170,6 +172,17 @@  int main(void){
                 !(codec->capabilities & AV_CODEC_CAP_FRAME_THREADS))
                 ERR("Decoder %s wants allocated progress without supporting"
                     "frame threads\n");
+            if (!(codec2->caps_internal & FF_CODEC_CAP_FRAME_DECODE_DATA) &&
+                codec2->hw_configs) {
+                const AVCodecHWConfigInternal *const *hw_configs = codec2->hw_configs;
+                for (; *hw_configs; hw_configs++) {
+                    if ((*hw_configs)->hwaccel &&
+                        (strstr((*hw_configs)->hwaccel->name, "nvdec") ||
+                         strstr((*hw_configs)->hwaccel->name, "videotoolbox")))
+                        ERR("Decoder %s has videotoolbox or nvdec HWAccel, "
+                            "yet these need FrameDecodeData.\n");
+                }
+            }
         }
         if (priv_data_size_wrong(codec2))
             ERR_EXT("Private context of codec %s is impossibly-sized (size %d).",
diff --git a/libavcodec/vc1dec.c b/libavcodec/vc1dec.c
index a7d556c378..8ea9903350 100644
--- a/libavcodec/vc1dec.c
+++ b/libavcodec/vc1dec.c
@@ -1212,7 +1212,8 @@  const FFCodec ff_vc1_decoder = {
     FF_CODEC_DECODE_CB(vc1_decode_frame),
     .flush          = ff_mpeg_flush,
     .p.capabilities = AV_CODEC_CAP_DR1 | AV_CODEC_CAP_DELAY,
-    .caps_internal  = FF_CODEC_CAP_INIT_THREADSAFE,
+    .caps_internal  = FF_CODEC_CAP_INIT_THREADSAFE |
+                      FF_CODEC_CAP_FRAME_DECODE_DATA,
     .p.pix_fmts     = vc1_hwaccel_pixfmt_list_420,
     .hw_configs     = (const AVCodecHWConfigInternal *const []) {
 #if CONFIG_VC1_DXVA2_HWACCEL
@@ -1250,7 +1251,8 @@  const FFCodec ff_wmv3_decoder = {
     FF_CODEC_DECODE_CB(vc1_decode_frame),
     .flush          = ff_mpeg_flush,
     .p.capabilities = AV_CODEC_CAP_DR1 | AV_CODEC_CAP_DELAY,
-    .caps_internal  = FF_CODEC_CAP_INIT_THREADSAFE,
+    .caps_internal  = FF_CODEC_CAP_INIT_THREADSAFE |
+                      FF_CODEC_CAP_FRAME_DECODE_DATA,
     .p.pix_fmts     = vc1_hwaccel_pixfmt_list_420,
     .hw_configs     = (const AVCodecHWConfigInternal *const []) {
 #if CONFIG_WMV3_DXVA2_HWACCEL
diff --git a/libavcodec/vp8.c b/libavcodec/vp8.c
index f521f2c9de..37fe4cb611 100644
--- a/libavcodec/vp8.c
+++ b/libavcodec/vp8.c
@@ -2967,7 +2967,8 @@  const FFCodec ff_vp8_decoder = {
     .p.capabilities        = AV_CODEC_CAP_DR1 | AV_CODEC_CAP_FRAME_THREADS |
                              AV_CODEC_CAP_SLICE_THREADS,
     .caps_internal         = FF_CODEC_CAP_INIT_THREADSAFE |
-                             FF_CODEC_CAP_ALLOCATE_PROGRESS,
+                             FF_CODEC_CAP_ALLOCATE_PROGRESS |
+                             FF_CODEC_CAP_FRAME_DECODE_DATA,
     .flush                 = vp8_decode_flush,
     .update_thread_context = ONLY_IF_THREADS_ENABLED(vp8_decode_update_thread_context),
     .hw_configs            = (const AVCodecHWConfigInternal *const []) {
diff --git a/libavcodec/vp9.c b/libavcodec/vp9.c
index fee79fb45b..244dbe8aa6 100644
--- a/libavcodec/vp9.c
+++ b/libavcodec/vp9.c
@@ -1878,7 +1878,8 @@  const FFCodec ff_vp9_decoder = {
     .p.capabilities        = AV_CODEC_CAP_DR1 | AV_CODEC_CAP_FRAME_THREADS | AV_CODEC_CAP_SLICE_THREADS,
     .caps_internal         = FF_CODEC_CAP_INIT_THREADSAFE | FF_CODEC_CAP_INIT_CLEANUP |
                              FF_CODEC_CAP_SLICE_THREAD_HAS_MF |
-                             FF_CODEC_CAP_ALLOCATE_PROGRESS,
+                             FF_CODEC_CAP_ALLOCATE_PROGRESS |
+                             FF_CODEC_CAP_FRAME_DECODE_DATA,
     .flush                 = vp9_decode_flush,
     .update_thread_context = ONLY_IF_THREADS_ENABLED(vp9_decode_update_thread_context),
     .p.profiles            = NULL_IF_CONFIG_SMALL(ff_vp9_profiles),