[FFmpeg-devel] [PATCH] ffprobe: Print color properties from show_frames

[Vittorio Giovara]

>On 20.07.2017 14:38, Vittorio Giovara wrote:
>> ---
>>  ffprobe.c | 6 ++++++
>>  1 file changed, 6 insertions(+)
>>
>> diff --git a/ffprobe.c b/ffprobe.c
>> index f6d9be0df9..412e2dadab 100644
>> --- a/ffprobe.c
>> +++ b/ffprobe.c
>> @@ -2105,6 +2105,12 @@ static void show_frame(WriterContext *w, AVFrame *frame, AVStream *stream,
>>          print_int("interlaced_frame",       frame->interlaced_frame);
>>          print_int("top_field_first",        frame->top_field_first);
>>          print_int("repeat_pict",            frame->repeat_pict);
>> +
>> +        print_str("color_range",     av_color_range_name(frame->color_range));
>> +        print_str("color_space",     av_color_space_name(frame->colorspace));
>> +        print_str("color_primaries", av_color_primaries_name(frame->color_primaries));
>> +        print_str("color_transfer",  av_color_transfer_name(frame->color_trc));
>> +        print_str("chroma_location", av_chroma_location_name(frame->chroma_location));
>
>I guess this should look like
>
>if (frame->... != ..._UNSPECIFIED)
>     print_str(...);
>else
>     print_str_opt(...);
>
>see the similar code lines handling color properties on stream level (~
>line #2475).

Should it? That approach effectively hides these parameters from the
output if unknown, and I often want to know as much as possible when
hunting down parameters with read_frames (even that the filed is just
"unknown" and not missing). Also if these fields are always output, it
simplify parsing them quite a bit, don't you think so? I'd much rather
change the stream level code to output more information instead.

>>          break;
>>
>>      case AVMEDIA_TYPE_AUDIO:
>>
>
>The schema file at doc/ffprobe.xsd should be updated to reflect the new
>fields.
>
>Also I assume that some FATE references are changed by this patch?

Right, I'll update them in the next iteration, thanks for noticing.
-- 
Vittorio