[FFmpeg-devel] [PATCH]lavf/dump: Fix cpb bitrate types after next major bump

[James Almer]

On 8/11/2019 1:08 PM, Michael Niedermayer wrote:
> On Sat, Aug 10, 2019 at 11:51:15PM +0200, Carl Eugen Hoyos wrote:
>> Am Sa., 10. Aug. 2019 um 17:01 Uhr schrieb James Almer <jamrial at gmail.com>:
>>>
>>> On 8/10/2019 9:47 AM, Carl Eugen Hoyos wrote:
>>>> Hi!
>>>>
>>>> Attached patch fixes a compilation warning when compiling without
>>>> FF_API_UNSANITIZED_BITRATES.
>>>>
>>>> Please comment, Carl Eugen
>>>>
>>>>
>>>> 0001-lavf-dump-Fix-cpb-bitrate-type-after-next-major-bump.patch
>>>>
>>>> From 48f7c57037206fe734bd45dc12c6140220afe34f Mon Sep 17 00:00:00 2001
>>>> From: Carl Eugen Hoyos <ceffmpeg at gmail.com>
>>>> Date: Sat, 10 Aug 2019 14:43:58 +0200
>>>> Subject: [PATCH] lavf/dump: Fix cpb bitrate type after next major bump.
>>>>
>>>> ---
>>>>  libavformat/dump.c | 4 ++++
>>>>  1 file changed, 4 insertions(+)
>>>>
>>>> diff --git a/libavformat/dump.c b/libavformat/dump.c
>>>> index 1c44656071..126641259d 100644
>>>> --- a/libavformat/dump.c
>>>> +++ b/libavformat/dump.c
>>>> @@ -320,7 +320,11 @@ static void dump_cpb(void *ctx, AVPacketSideData *sd)
>>>>      }
>>>>
>>>>      av_log(ctx, AV_LOG_INFO,
>>>> +#if FF_API_UNSANITIZED_BITRATES
>>>>             "bitrate max/min/avg: %d/%d/%d buffer size: %d vbv_delay: %"PRId64,
>>>> +#else
>>>> +           "bitrate max/min/avg: %"PRIu64"/%"PRIu64"/%"PRIu64" buffer size: %d vbv_delay: %"PRId64,
>>>
>>> They are int64_t, so PRId64. vbv_delay is what should be PRIu64.
>>>
>>> LGTM with that fixed.
>>
>> Pushed with that fix.
> 
> This produces:
> cpb: bitrate max/min/avg: 0/0/200000 buffer size: 0 vbv_delay: 18446744073709551615
> prior:
> cpb: bitrate max/min/avg: 0/0/200000 buffer size: 0 vbv_delay: -1
> 
> This doesnt look intended

The field is an uint64_t one, and the doxy states "UINT64_MAX when
unknown or unspecified".

I know -1 looks nicer, but if values > INT64_MAX are valid and expected
for this field, then printing with PRId64 was clearly not the intention.
What i wonder about however is scripts that already take into
consideration the buggy printing behavior, looking for -1 while parsing
the output string from av_dump_format() to report it as undefined. Afaik
it's not something we should consider, since the proper API usage is
looking at the side data proper, but i know some workflows using the cli
exist, so...

> 
> ./ffmpeg -v verbose  -i matrixbench_mpeg2.mpg  -t 1 test.avi
> 
> Thanks
> 
> [...]
> 
> 
> _______________________________________________
> 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".
>