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

[Anton Khirnov]

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

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.

-- 
Anton Khirnov