On Sun, Aug 11, 2019 at 01:38:38PM -0300, James Almer wrote: > 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 <jamr...@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 <ceffm...@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...
i think if we change what is printed from the original "-1" then it should be something like "N/A", "unspecified" or the "vbv_delay" should be ommited altogether from the line. in the same sense the max bitrate and buffer size in this case maybe should be changed too Is this line used more by developers/user reading it or scripts? if its humans then i think its better to ommit unknown fields instead of using special nummeric value with specific meaning defined in some docs. thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Those who would give up essential Liberty, to purchase a little temporary Safety, deserve neither Liberty nor Safety -- Benjamin Franklin
signature.asc
Description: PGP signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
