[FFmpeg-devel] [PATCH] fftools/ffprobe: fix printing side data components and pieces

[James Almer]

On 3/5/2024 11:43 AM, Stefano Sabatini wrote:
> On date Tuesday 2024-03-05 11:38:20 -0300, James Almer wrote:
>> On 3/5/2024 11:34 AM, Stefano Sabatini wrote:
>>> On date Monday 2024-03-04 15:56:46 -0300, James Almer wrote:
>>>> Components and pieces are side data specific fields and there's a variable
>>>> amount of them.
>>>> They also need to be identified in some form, so print a type too.
>>>>
>>>> Signed-off-by: James Almer <jamrial at gmail.com>
>>>> ---
>>>> You can see the difference with
>>>>
>>>> ffprobe -show_frames -of compact $fate_samples/mov/dovi-p7.mp4
>>>> ffprobe -show_frames -of xml $fate_samples/mov/dovi-p7.mp4
>>>>
>>>>    fftools/ffprobe.c | 33 +++++++++++++++++++++++++++------
>>>>    1 file changed, 27 insertions(+), 6 deletions(-)
>>>>
> [...]
>>> LGTM. Not clear if this is fixing a regression or if the format (at
>>> least for XML/compact) was always wrong.
>>
> 
>> I'm on the fence on this. Try removing the SECTION_FLAG_HAS_TYPE from
>> SECTION_ID_FRAME_SIDE_DATA_COMPONENT and SECTION_ID_FRAME_SIDE_DATA_PIECE
>> and the output is broken again for the compact output. So at least
>> compact_print_section_header() may be faulty. I did not investigate much
>> because I'm not familiar with the code.
> 
> That is, if you define the type (as in the current implementation) it
> looks correct?

Yes. Without the type, the unexpected line breaks return. So the path in 
compact_print_section_header() taken when SECTION_FLAG_HAS_TYPE is not 
set may be wrong.

> 
>> In any case, this change is needed because of the
>> SECTION_FLAG_HAS_VARIABLE_FIELDS addition for side data where the printed
>> fields are not a fixed amount.
> 
> Agree.
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".