[FFmpeg-devel] [PATCH] codec_desc: mark CFHD as intra-only

[James Almer]

On 6/8/2020 10:46 AM, James Almer wrote:
> On 6/8/2020 7:54 AM, Anton Khirnov wrote:
>> Quoting Hendrik Leppkes (2020-06-08 12:42:08)
>>> On Mon, Jun 8, 2020 at 12:22 PM Anton Khirnov <anton at khirnov.net> wrote:
>>>>
>>>> This stops update_thread_context() from being called with frame
>>>> threading, which causes races.
>>>> ---
>>>>  libavcodec/codec_desc.c | 2 +-
>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/libavcodec/codec_desc.c b/libavcodec/codec_desc.c
>>>> index 9f8847544f..5117984c75 100644
>>>> --- a/libavcodec/codec_desc.c
>>>> +++ b/libavcodec/codec_desc.c
>>>> @@ -1520,7 +1520,7 @@ static const AVCodecDescriptor codec_descriptors[] = {
>>>>          .type      = AVMEDIA_TYPE_VIDEO,
>>>>          .name      = "cfhd",
>>>>          .long_name = NULL_IF_CONFIG_SMALL("Cineform HD"),
>>>> -        .props     = AV_CODEC_PROP_LOSSY,
>>>> +        .props     = AV_CODEC_PROP_LOSSY | AV_CODEC_PROP_INTRA_ONLY,
>>>>      },
>>>>      {
>>>>          .id        = AV_CODEC_ID_TRUEMOTION2RT,
>>>> --
>>>> 2.26.2
>>>>
>>>
>>> A codec property influencing a decoder implementation behavior seems
>>> iffy at best, doesn't it?
>>> What if I make an intra-only implementation for a codec that
>>> theoretically supports more? The information no longer matches.
>>>
>>> Flags changing behavior of an implementation should really be on AVCodec.
>>
>> I generally agree, but that condition is already there and changing it
>> to be more robust is not entirely trivial. I am planning to get to that
>> as a part of some other threading work, but I did not want it to delay
>> my other set which is blocked by this.
> 
> Maybe we could partially revert 13b1bbff0b (the intra only part) and
> re-purpose the flag to also apply to decoders? Or only decoders,
> whatever is best.
> 
> We still can seeing 4.3 wasn't tagged.

This is one way it could be implemented (attaching it as a diff since as
patches it would need to be split in at least two).

In short, marking all implementations of intra-only codecs as such with
the relevant codec cap during static init, and then manually do the same
for all intra only implementations of codecs that could support more.
-------------- next part --------------
diff --git a/libavcodec/allcodecs.c b/libavcodec/allcodecs.c
index 5240d0afdf..9a048773e4 100644
--- a/libavcodec/allcodecs.c
+++ b/libavcodec/allcodecs.c
@@ -823,6 +823,13 @@ static AVOnce av_codec_static_init = AV_ONCE_INIT;
 static void av_codec_init_static(void)
 {
     for (int i = 0; codec_list[i]; i++) {
+        AVCodec *codec = (AVCodec*)codec_list[i];
+        const AVCodecDescriptor *desc = avcodec_descriptor_get(codec->id);
+
+        // Mark all implementations of intra-only codecs as such
+        if (desc && (desc->props & AV_CODEC_PROP_INTRA_ONLY))
+            codec->capabilities |= AV_CODEC_CAP_INTRA_ONLY;
+
         if (codec_list[i]->init_static_data)
             codec_list[i]->init_static_data((AVCodec*)codec_list[i]);
     }
diff --git a/libavcodec/cfhd.c b/libavcodec/cfhd.c
index 7956367b49..63d05f9d70 100644
--- a/libavcodec/cfhd.c
+++ b/libavcodec/cfhd.c
@@ -1054,6 +1054,6 @@ AVCodec ff_cfhd_decoder = {
     .init             = cfhd_init,
     .close            = cfhd_close,
     .decode           = cfhd_decode,
-    .capabilities     = AV_CODEC_CAP_DR1 | AV_CODEC_CAP_FRAME_THREADS,
+    .capabilities     = AV_CODEC_CAP_DR1 | AV_CODEC_CAP_FRAME_THREADS | AV_CODEC_CAP_INTRA_ONLY,
     .caps_internal    = FF_CODEC_CAP_INIT_THREADSAFE | FF_CODEC_CAP_INIT_CLEANUP,
 };
diff --git a/libavcodec/codec.h b/libavcodec/codec.h
index 1fda619ee7..eb223ecc7c 100644
--- a/libavcodec/codec.h
+++ b/libavcodec/codec.h
@@ -130,12 +130,12 @@
  * choice for probing.
  */
 #define AV_CODEC_CAP_AVOID_PROBING       (1 << 17)
-
-#if FF_API_UNUSED_CODEC_CAPS
 /**
- * Deprecated and unused. Use AVCodecDescriptor.props instead
+ * Codec is intra only.
  */
 #define AV_CODEC_CAP_INTRA_ONLY       0x40000000
+
+#if FF_API_UNUSED_CODEC_CAPS
 /**
  * Deprecated and unused. Use AVCodecDescriptor.props instead
  */
diff --git a/libavcodec/pthread_frame.c b/libavcodec/pthread_frame.c
index 64121f5a9a..1385b57a24 100644
--- a/libavcodec/pthread_frame.c
+++ b/libavcodec/pthread_frame.c
@@ -246,7 +246,7 @@ static int update_context_from_thread(AVCodecContext *dst, AVCodecContext *src,
 {
     int err = 0;
 
-    if (dst != src && (for_user || !(src->codec_descriptor->props & AV_CODEC_PROP_INTRA_ONLY))) {
+    if (dst != src && (for_user || !(src->codec->capabilities & AV_CODEC_CAP_INTRA_ONLY))) {
         dst->time_base = src->time_base;
         dst->framerate = src->framerate;
         dst->width     = src->width;