[FFmpeg-devel] [PATCH V3 1/1] ensure closed caption info which is visible in default stream dump is also available in results when -show_streams is used

[Marton Balint]

On Sat, 25 Apr 2020, vectronic wrote:

>
>
>> On 25 Apr 2020, at 09:08, Marton Balint <cus at passwd.hu> wrote:
>> 
>> 
>> 
>> On Fri, 24 Apr 2020, vectronic wrote:
>> 
>>> Signed-off-by: vectronic <hello.vectronic at gmail.com>
>>> ---
>>> doc/ffprobe.xsd                                           | 1 +
>>> fftools/ffprobe.c                                         | 2 ++
>>> tests/ref/fate/concat-demuxer-extended-lavf-mxf           | 2 +-
>>> tests/ref/fate/concat-demuxer-extended-lavf-mxf_d10       | 2 +-
>>> tests/ref/fate/concat-demuxer-simple1-lavf-mxf            | 2 +-
>>> tests/ref/fate/concat-demuxer-simple1-lavf-mxf_d10        | 2 +-
>>> tests/ref/fate/concat-demuxer-simple2-lavf-ts             | 2 +-
>>> tests/ref/fate/ffprobe_compact                            | 4 ++--
>>> tests/ref/fate/ffprobe_csv                                | 4 ++--
>>> tests/ref/fate/ffprobe_default                            | 2 ++
>>> tests/ref/fate/ffprobe_flat                               | 2 ++
>>> tests/ref/fate/ffprobe_ini                                | 2 ++
>>> tests/ref/fate/ffprobe_json                               | 2 ++
>>> tests/ref/fate/ffprobe_xml                                | 4 ++--
>>> tests/ref/fate/hapqa-extract-nosnappy-to-hapalphaonly-mov | 1 +
>>> tests/ref/fate/hapqa-extract-nosnappy-to-hapq-mov         | 1 +
>>> tests/ref/fate/mov-zombie                                 | 2 +-
>>> tests/ref/fate/mxf-probe-d10                              | 1 +
>>> tests/ref/fate/mxf-probe-dnxhd                            | 1 +
>>> tests/ref/fate/mxf-probe-dv25                             | 1 +
>>> 20 files changed, 28 insertions(+), 12 deletions(-)
>>> 
>>> diff --git a/doc/ffprobe.xsd b/doc/ffprobe.xsd
>>> index 97dc67def6..f88045232f 100644
>>> --- a/doc/ffprobe.xsd
>>> +++ b/doc/ffprobe.xsd
>>> @@ -227,6 +227,7 @@
>>>      <xsd:attribute name="coded_width"          type="xsd:int"/>
>>>      <xsd:attribute name="coded_height"         type="xsd:int"/>
>>>      <xsd:attribute name="has_b_frames"         type="xsd:int"/>
>>> +      <xsd:attribute name="closed_captions"      type="xsd:int"/>
>> 
>> xsd:boolean seems more appropriate, no?
>
> I was copying off has_b_frames above which I believed was also a 0/1. The output in the stream info is 0/1, not true/false.

xsd:boolean can be 0/1 according ot this:
https://www.w3schools.com/xml/schema_dtypes_misc.asp
And I think boolean is more appropriate.

>
>>>      <xsd:attribute name="sample_aspect_ratio"  type="xsd:string"/>
>>>      <xsd:attribute name="display_aspect_ratio" type="xsd:string"/>
>>>      <xsd:attribute name="pix_fmt"              type="xsd:string"/>
>>> diff --git a/fftools/ffprobe.c b/fftools/ffprobe.c
>>> index 840fcb71e2..f0916cbd70 100644
>>> --- a/fftools/ffprobe.c
>>> +++ b/fftools/ffprobe.c
>>> @@ -2550,6 +2550,7 @@ static int show_stream(WriterContext *w, AVFormatContext *fmt_ctx, int stream_id
>>>        }
>>> #endif
>>>        print_int("has_b_frames", par->video_delay);
>>> +        print_int("closed_captions", !!(dec_ctx->properties & FF_CODEC_PROPERTY_CLOSED_CAPTIONS));
>> 
>> Not strictly related to this patch, but maybe these FF_CODEC_PROPERTY_* constants should be promoted to AV_*?
>
> I’m not sure what is meant by this, but happy to do so if you can elaborate a bit more…?

We typically use AV_ prefix for part-of-the-API constants and FF_ prefix 
for internal or not-part-of-the-API constants. Admittedly it is not very 
consistent, but I think that is the idea. Anyway, this can be changed 
later and in a separate patch.

>
>
>>>        sar = av_guess_sample_aspect_ratio(fmt_ctx, stream, NULL);
>>>        if (sar.num) {
>>>            print_q("sample_aspect_ratio", sar, ':');
>>> @@ -2950,6 +2951,7 @@ static int open_input_file(InputFile *ifile, const char *filename,
>>>
>>>            ist->dec_ctx->pkt_timebase = stream->time_base;
>>>            ist->dec_ctx->framerate = stream->avg_frame_rate;
>>> +            ist->dec_ctx->properties = stream->codec->properties;
>> 
>> This is using the depreacted stream->codec, so I think this should go under the #IF below.
>> 
>>> #if FF_API_LAVF_AVCTX
>>>            ist->dec_ctx->coded_width = stream->codec->coded_width;
>>>            ist->dec_ctx->coded_height = stream->codec->coded_height;
>
> OK, if this is the case, is there anything to worry to worry about if FF_API_LAVF_AVCTX is not defined? Should I be outputting closed_captions value in this case? Or am I thinking too much?

You are right, printing it should probably go to the block which prints 
the similar coded_height/coded_width values.

Regards,
Marton