[FFmpeg-devel] [PATCH] all: silence clang -Wabsolute-value for unsigned subtractions

Ganesh Ajjanagadde gajjanag at mit.edu
Sun Aug 23 15:57:55 CEST 2015


On Sun, Aug 23, 2015 at 4:20 AM, Nicolas George <george at nsup.org> wrote:
> Le quintidi 5 fructidor, an CCXXIII, Ganesh Ajjanagadde a écrit :
>> >> +    return abs((int) (t1 - t2)) + abs((int) ((c1 & 0x000000ff) - (c2 & 0x000000ff))) +
>> >> +        abs((int) (((c1 & 0x0000ff00) >> 8) - ((c2 & 0x0000ff00) >> 8))) +
>> >> +        abs((int) (((c1 & 0x00ff0000) >> 16) - ((c2 & 0x00ff0000) >> 16)));
>> The cast idea is incorrect as stated, though your idea is sound:
>> int is not guaranteed to be of 32 bits on all platforms.
>> Casting a uint32_t to an int will be bad.
>
> Look at the code itself: the numbers are all smaller than 768. The cast in
> that case is completely reliable.

You are right, in current use cases your solution would work.
However, it lacks generality and is prone to mistakes:
the code writer must reason with the bit widths as you did.
Do you also agree that a macro is a better idea?

>
>> I assume this is what you had in mind with your macro idea.
>
> I had in mind just a local macro to make that precise part of the code more
> readable.

Such a macro would be duplicated across files.
I just wanted to place it near the FFABS definition for removing such
duplication,
unless people object to this.

>
> Regards,
>
> --
>   Nicolas George
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>


More information about the ffmpeg-devel mailing list