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

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(-)

On Mon, Jun 8, 2020 at 12:22 PM Anton Khirnov <anton@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.

- Hendrik

Quoting Hendrik Leppkes (2020-06-08 12:42:08)
> On Mon, Jun 8, 2020 at 12:22 PM Anton Khirnov <anton@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.

On 6/8/20, Anton Khirnov <anton@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
>


I think codec have features that are not intra only.
Just out implementation currently is.

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@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.

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@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.
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;

Quoting James Almer (2020-06-08 18:30:16)
> 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@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.

I don't think this needs to be visible externally, since it's only
meaningful for internal use. I'm wondering if the presence of
update_thread_context() callback won't be sufficient for this.

On 6/9/2020 6:06 AM, Anton Khirnov wrote:
> Quoting James Almer (2020-06-08 18:30:16)
>> 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@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.
> 
> I don't think this needs to be visible externally, since it's only
> meaningful for internal use. I'm wondering if the presence of
> update_thread_context() callback won't be sufficient for this.

True, it could be a caps_internal. But take for example the possibility
of an external library with a fully featured decoder getting a wrapper,
this being a public capability would let the user choose it over the
internal one, same as how it can choose a stable decoder over an
experimental one, or a software one over an hybrid/hw one.

The cap is there and has been for some time, so silently undoing the
deprecation and finally give it some good use is a good approach to
solve this.

Quoting James Almer (2020-06-09 14:45:33)
> On 6/9/2020 6:06 AM, Anton Khirnov wrote:
> > I don't think this needs to be visible externally, since it's only
> > meaningful for internal use. I'm wondering if the presence of
> > update_thread_context() callback won't be sufficient for this.
> 
> True, it could be a caps_internal. But take for example the possibility
> of an external library with a fully featured decoder getting a wrapper,
> this being a public capability would let the user choose it over the
> internal one, same as how it can choose a stable decoder over an
> experimental one, or a software one over an hybrid/hw one.

I don't see why this should be a criterium for the user to base any
decisions on. It's purely an internal implementation detail that's tied
to the way frame threading is currently implemented and can potentially
change later. So I don't see why it should be accessible through the
API.

Also, not sure if you saw but I have new patch fixing this problem in
another manner.

On 6/10/2020 6:19 AM, Anton Khirnov wrote:
> Quoting James Almer (2020-06-09 14:45:33)
>> On 6/9/2020 6:06 AM, Anton Khirnov wrote:
>>> I don't think this needs to be visible externally, since it's only
>>> meaningful for internal use. I'm wondering if the presence of
>>> update_thread_context() callback won't be sufficient for this.
>>
>> True, it could be a caps_internal. But take for example the possibility
>> of an external library with a fully featured decoder getting a wrapper,
>> this being a public capability would let the user choose it over the
>> internal one, same as how it can choose a stable decoder over an
>> experimental one, or a software one over an hybrid/hw one.
> 
> I don't see why this should be a criterium for the user to base any
> decisions on. It's purely an internal implementation detail that's tied
> to the way frame threading is currently implemented and can potentially
> change later. So I don't see why it should be accessible through the
> API.

An intra only decoder returns I frames only. A fully feature decoder
would return I and P (and actually get you something other than a key
frame slideshow). Or at least that's how i understood it.
Giving the user a way to choose the best decoder for their needs is not
too crazy. We already do with other implementation specific
capabilities, as i mentioned above.

> 
> Also, not sure if you saw but I have new patch fixing this problem in
> another manner.

Yes, i saw. I'm arguing about this other approach because if it's done
now, it can be done seamlessly (Just revert the change and pretend the
flag was never deprecated). After 4.3 is effectively tagged, we would
require to undo the deprecation with another API change, and it will
look messy.

The flag was meaningless and a duplicate of the codec prop, but now we
found a way to use it to actually convey information about a decoder, so
why not do it?

Quoting James Almer (2020-06-10 14:46:21)
> On 6/10/2020 6:19 AM, Anton Khirnov wrote:
> > Quoting James Almer (2020-06-09 14:45:33)
> >> On 6/9/2020 6:06 AM, Anton Khirnov wrote:
> >>> I don't think this needs to be visible externally, since it's only
> >>> meaningful for internal use. I'm wondering if the presence of
> >>> update_thread_context() callback won't be sufficient for this.
> >>
> >> True, it could be a caps_internal. But take for example the possibility
> >> of an external library with a fully featured decoder getting a wrapper,
> >> this being a public capability would let the user choose it over the
> >> internal one, same as how it can choose a stable decoder over an
> >> experimental one, or a software one over an hybrid/hw one.
> > 
> > I don't see why this should be a criterium for the user to base any
> > decisions on. It's purely an internal implementation detail that's tied
> > to the way frame threading is currently implemented and can potentially
> > change later. So I don't see why it should be accessible through the
> > API.
> 
> An intra only decoder returns I frames only. A fully feature decoder
> would return I and P (and actually get you something other than a key
> frame slideshow). Or at least that's how i understood it.
> Giving the user a way to choose the best decoder for their needs is not
> too crazy. We already do with other implementation specific
> capabilities, as i mentioned above.
> 
> > 
> > Also, not sure if you saw but I have new patch fixing this problem in
> > another manner.
> 
> Yes, i saw. I'm arguing about this other approach because if it's done
> now, it can be done seamlessly (Just revert the change and pretend the
> flag was never deprecated). After 4.3 is effectively tagged, we would
> require to undo the deprecation with another API change, and it will
> look messy.
> 
> The flag was meaningless and a duplicate of the codec prop, but now we
> found a way to use it to actually convey information about a decoder, so
> why not do it?

It still seems to me that this is a solution in search of a problem. If
I understand you correctly, the case you have in mind is a codec where
there is an internal implementation that only supports intra frames
and an external implementation that also supports inter frames.
But that seems incredibly contrived to me. For one thing, I don't know
of any actual situations like this, now or in the past. For another, why
pick out specifically the ability to decode intra vs inter frames?
There is a huge number of codec features that may be potentially
unsupported by various implementations (see e.g. AAC) and there's no
sane way we can have capability flags for them all.

>
> It still seems to me that this is a solution in search of a problem. If
> I understand you correctly, the case you have in mind is a codec where
> there is an internal implementation that only supports intra frames
> and an external implementation that also supports inter frames.
> But that seems incredibly contrived to me. For one thing, I don't know
> of any actual situations like this, now or in the past. For another, why
> pick out specifically the ability to decode intra vs inter frames?
> There is a huge number of codec features that may be potentially
> unsupported by various implementations (see e.g. AAC) and there's no
> sane way we can have capability flags for them all.
>

Hi,

I have no opinion on the issue at hand but this is currently the case with
Cineform.
The internal decoder supports the intra features but the open sourced one
from them supports intra/inter features.

Kieran

On 6/12/2020 7:05 AM, Anton Khirnov wrote:
> Quoting James Almer (2020-06-10 14:46:21)
>> On 6/10/2020 6:19 AM, Anton Khirnov wrote:
>>> Quoting James Almer (2020-06-09 14:45:33)
>>>> On 6/9/2020 6:06 AM, Anton Khirnov wrote:
>>>>> I don't think this needs to be visible externally, since it's only
>>>>> meaningful for internal use. I'm wondering if the presence of
>>>>> update_thread_context() callback won't be sufficient for this.
>>>>
>>>> True, it could be a caps_internal. But take for example the possibility
>>>> of an external library with a fully featured decoder getting a wrapper,
>>>> this being a public capability would let the user choose it over the
>>>> internal one, same as how it can choose a stable decoder over an
>>>> experimental one, or a software one over an hybrid/hw one.
>>>
>>> I don't see why this should be a criterium for the user to base any
>>> decisions on. It's purely an internal implementation detail that's tied
>>> to the way frame threading is currently implemented and can potentially
>>> change later. So I don't see why it should be accessible through the
>>> API.
>>
>> An intra only decoder returns I frames only. A fully feature decoder
>> would return I and P (and actually get you something other than a key
>> frame slideshow). Or at least that's how i understood it.
>> Giving the user a way to choose the best decoder for their needs is not
>> too crazy. We already do with other implementation specific
>> capabilities, as i mentioned above.
>>
>>>
>>> Also, not sure if you saw but I have new patch fixing this problem in
>>> another manner.
>>
>> Yes, i saw. I'm arguing about this other approach because if it's done
>> now, it can be done seamlessly (Just revert the change and pretend the
>> flag was never deprecated). After 4.3 is effectively tagged, we would
>> require to undo the deprecation with another API change, and it will
>> look messy.
>>
>> The flag was meaningless and a duplicate of the codec prop, but now we
>> found a way to use it to actually convey information about a decoder, so
>> why not do it?
> 
> It still seems to me that this is a solution in search of a problem. If
> I understand you correctly, the case you have in mind is a codec where
> there is an internal implementation that only supports intra frames
> and an external implementation that also supports inter frames.
> But that seems incredibly contrived to me. For one thing, I don't know
> of any actual situations like this, now or in the past.

What do you think prompted the whole issue with the intra-only flag used
for cfhd if not the fact the internal decoder is intra only when the
codec itself isn't?

> For another, why
> pick out specifically the ability to decode intra vs inter frames?

Because the flag already exists. I wouldn't suggest to introduce it if
it didn't. I'm saying it could be easily and seamlessly repurposed.

Anyway, no point arguing about something as obscure as this scenario (Of
which only one case exists, as stated by Kieran, and it's a matter of
merging an old gsoc patch to render it moot), so i'm not going to insist
on it.
I already approved your alternate solution for the threading issue, for
that matter.

> There is a huge number of codec features that may be potentially
> unsupported by various implementations (see e.g. AAC) and there's no
> sane way we can have capability flags for them all.
>