[FFmpeg-devel] [PATCH v2 00/11] fix broken CC detection and ffprobe fields (cover letter)

[Marton Balint]

On Sun, 1 Dec 2024, James Almer wrote:

> On 11/28/2024 5:02 PM, Marton Balint wrote:
>>
>>
>>  On Wed, 27 Nov 2024, Marth64 wrote:
>>
>>>  As it stands today, ffprobe has two stream-level fields (closed_captions
>>>  and film_grain)
>>>  that do not work. Their value is always 0 because ffprobe cannot access
>>>  the internal
>>>  codec properties in the stream context when it is setting up its internal
>>>  streams.
>>>
>>>  Additionally, avformat/dump used to successfully print about the presence
>>>  of
>>>  Closed Captions (EIA-608/CEA-708) and the presence of film grain. This
>>>  does
>>>  not work either anymore, because avformat_find_stream_info() does not
>>>  copy
>>>  them in the skeleton context it uses after calling codec_close().
>>>
>>>  To clarify: the aforementioned features/code are broken.
>>>
>>>  So, to the user, Closed Caption detection as a feature is broken, and
>>>  ffprobe
>>>  and the dump is essentially having two fields that do not work.
>>>
>>>  This patchset aims to fix the issue by:
>>>  (1) Adding a video stream disposition, AV_DISPOSITION_CAPTIONS_EIA608
>>>  that
>>>     can be set in avformat_find_stream_info(), and using it to express
>>>     the presence of Closed Captions (EIA-608/CEA-708)
>>
>>  Manually mapping between different codec properties and stream
>>  dispositions seems ugly, and I am not sure if we should even use the
>>  disposition field for something like this.
>>
>>  I suggest you try adding the "properties" attribute to AVCodecParameters,
>>  because as far as I see it is very similar to profile or level. Can you
>>  see if that works? If it does, we might even promote the FF_CODEC_PROPERTY
>>  flags to AV_CODEC_PROPERTY so they will become properly public.
>
> I don't agree it should be in codecpar. It's rarely (if ever) filled during 
> header parsing (codec or container), and instead set when parsing a frame 
> that contains the relevant characteristic that's exported here as a 
> "property", like captions or film grain parameters. Therefore it's not a 
> field used for initialization.
>

Okay, but if this is a frame property, we should not clobber neither 
AVStream nor AVCodecParameters with it.

Probably we need a more generic solution for the problem that some 
information is only available in the first read packet or in the first 
decoded frame.

I see two possibilites:

1) Make ffprobe read the first packet or decode a first frame from every
    stream and print information based on that.

2) Extend avstream_find_stream_info() so
  - the API user can force it to read a packet or decode a frame from every
    stream (this is not always the case right now)
  - it has a way to return the first frame or first packet from every
    stream when probing

Regards,
Marton