[FFmpeg-devel] [PATCH v2 2/2] ffprobe: Fix incorrect display of closed_captions property

[Soft Works]

> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of James Almer
> Sent: Sunday, 19 September 2021 05:59
> To: ffmpeg-devel at ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH v2 2/2] ffprobe: Fix incorrect display of
> closed_captions property
> 
> On 9/19/2021 12:28 AM, Soft Works wrote:
> >
> >
> >> -----Original Message-----
> >> From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of James
> Almer
> >> Sent: Sunday, 19 September 2021 05:05
> >> To: ffmpeg-devel at ffmpeg.org
> >> Subject: Re: [FFmpeg-devel] [PATCH v2 2/2] ffprobe: Fix incorrect display
> of
> >> closed_captions property
> >>
> >> On 9/18/2021 7:10 PM, Soft Works wrote:
> >>> Repro Example:
> >>> ffprobe -show_entries stream=closed_captions:disposition=:side_data=
> >> "http://streams.videolan.org/streams/ts/CC/NewsStream-608-ac3.ts"
> >>>
> >>> While the codec string includes "Closed Captions",
> >>> the stream data is showing: closed_captions=0
> >>>
> >>> The test ref was incorrect as the test media file
> >>> actually does have cc.
> >>>
> >>> Signed-off-by: softworkz <softworkz at hotmail.com>
> >>> ---
> >>> v2: Fix test result. It was undiscovered locally as make fate
> >>>       does not seem to properly rebuild ffprobe.
> >>>
> >>>    fftools/ffprobe.c       | 5 +++--
> >>>    tests/ref/fate/ts-demux | 2 +-
> >>>    2 files changed, 4 insertions(+), 3 deletions(-)
> >>>
> >>> diff --git a/fftools/ffprobe.c b/fftools/ffprobe.c
> >>> index 880f05a6c2..9425c1a2e3 100644
> >>> --- a/fftools/ffprobe.c
> >>> +++ b/fftools/ffprobe.c
> >>> @@ -2678,10 +2678,11 @@ static int show_stream(WriterContext *w,
> >> AVFormatContext *fmt_ctx, int stream_id
> >>>            print_int("width",        par->width);
> >>>            print_int("height",       par->height);
> >>>            if (dec_ctx) {
> >>> +            unsigned codec_properties =
> >> av_stream_get_codec_properties(ist->st);
> >>
> >> Library users are meant to use their own decoder contexts if they want
> >> this kind of information. AVStream->internal->avctx is internal to lavf.
> >> Accessors like this should be avoided.
> >
> > If you look further up in util
> >>
> >> ffprobe is already decoding frames for the purpose of show_frames, and
> >> in fact if you add -show_frames to the above command line example you
> >> gave, the output of show_streams will report closed_captions=1.
> >
> > Yes, I know that, but if you don't - the output is wrong.
> 
> Yeah, and fixing that would be ideal. The output of show_streams should
> (if possible) not depend on other options also being passed.
> Your patch will only do it for properties, and it requires new public
> API, whereas my suggestion will do it for all other fields, like
> coded_{width,height}, and requires changes contained entirely within
> ffprobe with proper usage of existing API.
> 
> >
> >> So the correct approach here is to make ffprobe decode a few frames when
> >> you request show_streams, even if you don't request show_frames.
> >
> > This is something that I would want to avoid. It has already happened
> > and the result is available already. The "Closed Captions" part in
> > the video codec string is coming from that result.
> >
> > It doesn't make sense IMO, to perform another decoding run to replicate
> > that result, because:
> >
> > - It can't be taken for granted that this will lead to the same result
> >    that already exists
> >    How many frames do you think would be the right amount to process?
> >    What if the file start with a bunch of audio frames?
> 
> You decode until you get the info you need, which is what
> avformat_find_stream_info() does to fill its own internal
> AVCodecContext. The simplest way would be to ensure one frame per stream
> is decoded.

Sometimes, you could process hours of data without
seeing a frame for certain streams, so you would need a timeout
and maybe a limit for the amount of decoded data (maybe 
naming them analyyzeduration and probesize?).

What you're essentially suggesting is more or less a duplication
of avformat_find_stream_info() and to run it right after 
avformat_find_stream_info() has just been run.

I'm sure we can find a better solution :-)

Kind regards,
softworkz