[FFmpeg-devel] [PATCH] ffprobe: add -show_pixel_formats option

[Tobias Rapp]

On 11.09.2014 12:12, Stefano Sabatini wrote:
> On date Thursday 2014-09-11 09:54:42 +0200, Tobias Rapp encoded:
>> Have now added documentation and cleaned up some unused variables
>> with the new patch. Also on a second thought the option names
>> "show_pixel_format_descriptions" or "show_pixel_formats" seem less
>> confusing to me than "show_pixel_descriptions" so I did choose the
>> shorter "show_pixel_formats".
>>
>> What I would like to add in a follow-up patch is a way to output the
>> flags. I see different options:
>>
>> a) Output a character per flag (R=RGB, A=ALPHA, B=BITSTREAM, ...)
>> but this needs extra documentation and might be confusing due to
>> collisions (P=PALETTE or P=PLANAR? B=BIG-ENDIAN or B=BITSTREAM?)
>>
>> Example: flag="RAP"
>>
>> b) Output a list of flag names separated by comma or space. This
>> would fit mostly with JSON (comma) or XML (space) output format
>> style, I guess.
>>
>> Example 1: flag="rgb,alpha,palette"
>> Example 2: flag="rgb alpha palette"
>>
>> c) Output each flag as a boolean value. This increases amount of
>> output but fits better to output formats like CSV or INI.
>>
>> Example: is_rgb="1" has_alpha="1" is_bitstream="0" has_palette="1"
>>
>> What do you think?
>
> Flags are a collection, so probably it makes sense to print it as an
> array. See how it is done with PRINT_DISPOSITION. That's not very
> elegant though, but IMO the best solution for easing the following
> parsing (no need to further serialize the flags value).

OK, this is like (c) with an own sub-section. Sounds fine.

>>  From 5938c171472469e91bb2b7445100ab6509929ba7 Mon Sep 17 00:00:00 2001
>> From: Tobias Rapp <t.rapp at noa-audio.com>
>> Date: Thu, 11 Sep 2014 09:16:52 +0200
>> Subject: [PATCH] ffprobe: add -show_pixel_formats option
>>
>> Adds option -show_pixel_formats to ffprobe which lists all
>> available pixel formats with some details.
>> ---
>>   doc/ffprobe.texi |  6 +++++
>>   doc/ffprobe.xsd  | 15 +++++++++++
>>   ffprobe.c        | 78 ++++++++++++++++++++++++++++++++++++++++++++++++++++++--
>>   3 files changed, 97 insertions(+), 2 deletions(-)

[...]

>> +static int get_bits_per_component(const AVPixFmtDescriptor *pixdesc)
>> +{
>> +    int c, bits, first_bits = 0;
>> +
>> +    for (c = 0; c < pixdesc->nb_components; c++) {
>> +        bits = pixdesc->comp[c].depth_minus1 + 1;
>> +        if (c == 0)
>> +            first_bits = bits;
>> +        else if (bits != first_bits)
>> +            return 0;
>> +    }
>> +
>> +    return first_bits;
>> +}
>
> What if you compute this in the script? Since the bits_per_component
> semantics is ambiguous it is probably better let the user compute it
> herself.

So you suggest to output a component array like (in XML):

<pixel_format>
   ...
   <component name="R" bit_depth="5"/>
   <component name="G" bit_depth="6"/>
   <component name="B" bit_depth="5"/>
</pixel_format>

instead?

>> +
>> +static const char *get_chroma_sub_sample_string(const AVPixFmtDescriptor *pixdesc)
>> +{
>> +    if (!pixdesc)
>> +        return NULL;
>> +    if ((pixdesc->flags & AV_PIX_FMT_FLAG_RGB) || pixdesc->nb_components < 3)
>> +        return NULL;
>> +
>> +    if ((pixdesc->log2_chroma_w == 0) && (pixdesc->log2_chroma_h == 0))
>> +        return "4:4:4";
>> +    else if ((pixdesc->log2_chroma_w == 0) && (pixdesc->log2_chroma_h == 1))
>> +        return "4:4:0";
>> +    else if ((pixdesc->log2_chroma_w == 1) && (pixdesc->log2_chroma_h == 0))
>> +        return "4:2:2";
>> +    else if ((pixdesc->log2_chroma_w == 1) && (pixdesc->log2_chroma_h == 1))
>> +        return "4:2:0";
>> +    else if ((pixdesc->log2_chroma_w == 2) && (pixdesc->log2_chroma_h == 0))
>> +        return "4:1:1";
>> +    else if ((pixdesc->log2_chroma_w == 2) && (pixdesc->log2_chroma_h == 2))
>> +        return "4:1:0";
>> +    else
>> +        return "unknown";
>> +}
>
> Why don't you just write the log2_chroma_w/h values?

That looks a bit low-level for me - like printing the audio channel 
layout as "0x063f" instead of "7.1". From my POV I see ffprobe as a file 
format reporting tool. For actually accessing pixel data I would use the 
av* libraries.

Tobias