[FFmpeg-devel] [PATCH] avcodec/ac3enc: use long long after switch to 64 bit bitrate

[Ganesh Ajjanagadde]

On Fri, Sep 18, 2015 at 5:52 PM, James Almer <jamrial at gmail.com> wrote:
> On 9/18/2015 6:35 PM, Ganesh Ajjanagadde wrote:
>> On Fri, Sep 18, 2015 at 5:30 PM, James Almer <jamrial at gmail.com> wrote:
>>> On 9/17/2015 9:46 AM, Ganesh Ajjanagadde wrote:
>>>> Commit 7404f3bdb switched bitrate to 64 bits.
>>>> This triggers -Wabsolute-value on clang, e.g
>>>> http://fate.ffmpeg.org/log.cgi?time=20150917122742&log=compile&slot=x86_64-darwin-clang-3.7-O3.
>>>> Therefore, usage of abs is changed to llabs, which is available on all of the platforms.
>>>> Have not tested whether LLONG_MAX (C99 feature) is available on Microsoft or not.
>>>
>>> Use int64_t and INT64_MAX instead.
>>
>> I can definitely do that, but am curious about this. The codebase
>> already uses long long (see e.g ffprobe), and morally llabs returns
>> long long; hence I use it. As for the INT64_MAX, I do not have a
>> current example to know whether it is safe on all platforms, but
>> again, the "clean" solution is LLONG_MAX due to the type of llabs.
>
> grepping INT64_MAX shows a lot of hits in the codebase, so it's safe to
> use. Also, int64_t is used in hundreds of files whereas long long is
> used in about seven.

So Microsoft, even in 2015 after countless opportunities to do c99
stuff correctly, and thousands of engineers on its payroll, can't add
the few lines of #defines etc to define LLONG_MAX:
https://msdn.microsoft.com/en-us/library/296az74e.aspx

I checked out msinttypes; it seems like it does not handle LLONG_MAX
either. I guess if and when someone patches msinttypes we can finally
have our desired "clean" code.

As for long long, it has been in use for over a month (at least) in
the codebase and has caused no breakage on fate. Since it is the
correct/clean way of doing thing, I will update patch replacing the
LLONG_MAX by INT64_MAX but keeping the long long unless there are
objections.

>
> C guarantees that long long is at least 64bits so int64_t should be ok
> for this patch, but in any case wait for someone else to comment before
> sending a new patch.
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel