On 9/19/2021 1:46 AM, Soft Works wrote:


-----Original Message-----
From: ffmpeg-devel <ffmpeg-devel-boun...@ffmpeg.org> On Behalf Of James Almer
Sent: Sunday, 19 September 2021 05:59
To: ffmpeg-devel@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-boun...@ffmpeg.org> On Behalf Of James
Almer
Sent: Sunday, 19 September 2021 05:05
To: ffmpeg-devel@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 <softwo...@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?).

Yes, something like that could be forced (ReadInterval in ffprobe).


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

We could always not print codec level information, like the presence of closed captions in a bitstream, when container level information is requested, as is the case of show_streams. It is in fact a per-frame property. One could even not show up until halfways into the video and then not even avformat_find_stream_info() would reflect it. And this flag is essentially a "The decoder found a CC at some point" event flag rather than a stream parameter.

The fact ffprobe looks at the decoder context to print stream values to begin with is questionable, and as mentioned, its output is not even guaranteed and depends on external factors, like other user-provided runtime options.


Kind regards,
softworkz
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".


_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Reply via email to