[FFmpeg-devel] [PATCH] all: silence clang -Wabsolute-value for unsigned subtractions
gajjanag at mit.edu
Sun Aug 23 16:47:06 CEST 2015
On Sun, Aug 23, 2015 at 9:57 AM, Ganesh Ajjanagadde <gajjanag at mit.edu> wrote:
> 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.
Actually, regardless this needs to be done, since abs is for ints.
Nevertheless, I think the second macro is better (without the cast to ints),
since it correctly preserves the ranges for which operation is allowed.
> 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
> Such a macro would be duplicated across files.
> I just wanted to place it near the FFABS definition for removing such
> unless people object to this.
>> Nicolas George
>> ffmpeg-devel mailing list
>> ffmpeg-devel at ffmpeg.org
More information about the ffmpeg-devel