[FFmpeg-devel] [PATCH] ffprobe: add decode_error_flags

[Marton Balint]

On Thu, 27 May 2021, Tobias Rapp wrote:

> On 25.05.2021 13:27, Nicolas George wrote:
>> Michael Niedermayer (12021-05-25):
>>> Signed-off-by: Michael Niedermayer <michael at niedermayer.cc>
>>> ---
>>>   doc/ffprobe.xsd                    |   1 +
>>>   fftools/ffprobe.c                  |   2 +
>>>   tests/ref/fate/exif-image-embedded |  22 ++
>>>   tests/ref/fate/exif-image-jpg      |   1 +
>>>   tests/ref/fate/exif-image-tiff     |   1 +
>>>   tests/ref/fate/exif-image-webp     |   1 +
>>>   tests/ref/fate/ffprobe_compact     |  28 +--
>>>   tests/ref/fate/ffprobe_csv         |  28 +--
>>>   tests/ref/fate/ffprobe_default     |  14 ++
>>>   tests/ref/fate/ffprobe_flat        |  14 ++
>>>   tests/ref/fate/ffprobe_ini         |  14 ++
>>>   tests/ref/fate/flcl1905            | 352 ++++++++++++++---------------
>>>   tests/ref/fate/h264-dts_5frames    |   5 +
>>>   tests/ref/fate/mov-zombie          | 130 +++++------
>>>   tests/ref/fate/png-side-data       |   1 +
>>>   15 files changed, 345 insertions(+), 269 deletions(-)
>>> 
>>> diff --git a/doc/ffprobe.xsd b/doc/ffprobe.xsd
>>> index 250de8cd5c..370d68374c 100644
>>> --- a/doc/ffprobe.xsd
>>> +++ b/doc/ffprobe.xsd
>>> @@ -96,6 +96,7 @@
>>>         <xsd:attribute name="pkt_duration_time" type="xsd:float"/>
>>>         <xsd:attribute name="pkt_pos"       type="xsd:long" />
>>>         <xsd:attribute name="pkt_size"      type="xsd:int" />
>> 
>>> +      <xsd:attribute name="decode_error_flags" type="xsd:string" />
>>
>>>           <!-- audio attributes -->
>>>         <xsd:attribute name="sample_fmt"             type="xsd:string"/>
>>> diff --git a/fftools/ffprobe.c b/fftools/ffprobe.c
>>> index 88e82a4195..37aec0564a 100644
>>> --- a/fftools/ffprobe.c
>>> +++ b/fftools/ffprobe.c
>>> @@ -2259,6 +2259,8 @@ static void show_frame(WriterContext *w, AVFrame 
>>> *frame, AVStream *stream,
>>>       else                      print_str_opt("pkt_pos", "N/A");
>>>       if (frame->pkt_size != -1) print_val    ("pkt_size", 
>>> frame->pkt_size, unit_byte_str);
>>>       else                       print_str_opt("pkt_size", "N/A");
>> 
>>> +    if (frame->decode_error_flags) print_fmt    ("decode_error_flags", 
>>> "0x%x", frame->decode_error_flags);
>>> +    else                           print_str_opt("decode_error_flags", 
>>> "0");
>> 
>> Since the value is an int, I think it would be better to have it
>> declared as a number, even if it cannot be in hex.
>
> IMHO for bit-fields an hexadecimal number may be more readable. My 
> improvement suggestion would be to use some zero-padded formatting like 
> "0x%02x" and also use a hexadecimal value "0x00" for the default case for 
> consistency.

If xsd:int could be hex, then sure. But since xsd:int is regular integer 
only, then I'd prefer the number. After all, not many decode error flags 
exists and reading 4 bits in decimal is easy.

An alternative approach is to print the meaning of the actually used
flags:

<frame ...>
  <decode_error_flags invalid_bitstream="0" missing_reference="0" concealment_active="0" decode_slices="0" />
</frame>

This is the most readable, but maybe too verbose for the default use?

Regards,
Marton