[FFmpeg-devel] [PATCH]lavfi/fspp: Add a cast to silence a clang warning

[Carl Eugen Hoyos]

2019-04-14 1:02 GMT+02:00, Michael Niedermayer <michael at niedermayer.cc>:
> On Sat, Apr 13, 2019 at 07:25:54PM -0300, James Almer wrote:
>> On 4/13/2019 7:04 PM, Carl Eugen Hoyos wrote:
>> > Hi!
>> >
>> > Attached patch silences two warnings shown when compiling with clang:
>> > libavfilter/vf_fspp.h:51:42: warning: implicit conversion from 'int'
>> > to 'int16_t' (aka 'short') changes value from 44130 to -21406
>> >
>> > Please comment, Carl Eugen
>> >
>> >
>> > 0001-lavfi-vf_fspp-Use-the-intended-cast-to-int16_t.patch
>> >
>> > From d731e523d9c5854183d20d37fe921f49fb048498 Mon Sep 17 00:00:00 2001
>> > From: Carl Eugen Hoyos <ceffmpeg at gmail.com>
>> > Date: Sat, 13 Apr 2019 23:05:44 +0200
>> > Subject: [PATCH] lavfi/fspp: Add two casts to int16_t.
>> >
>> > Silences a warning with clang:
>> > warning: implicit conversion from 'int' to 'int16_t' (aka 'short')
>> > changes value from 44130 to -21406
>> > ---
>> >  libavfilter/vf_fspp.h |    2 +-
>> >  1 file changed, 1 insertion(+), 1 deletion(-)
>> >
>> > diff --git a/libavfilter/vf_fspp.h b/libavfilter/vf_fspp.h
>> > index 74a3447..e346439 100644
>> > --- a/libavfilter/vf_fspp.h
>> > +++ b/libavfilter/vf_fspp.h
>> > @@ -31,7 +31,7 @@
>> >  #define DCTSIZE 8
>> >  #define DCTSIZE_S "8"
>> >
>> > -#define FIX(x,s)  ((int) ((x) * (1 << s) + 0.5) & 0xffff)
>> > +#define FIX(x,s)  ((int16_t)((x) * (1 << s) + 0.5) & (int16_t)0xffff)
>>
>> The "& 0xffff" is turning -21406 into 44130 since it zeroes all the high
>> bits, and then that value gets converted back to -21406 when stored into
>> an int16_t variable.
>>
>> Change the int cast into int16_t if you want since it doesn't hurt (all
>> the values are safely within the int16_t range anyway), but remove the
>> "& 0xffff" altogether instead of casting it, to get rid of this warning.
>>
>> >  #define C64(x)    ((uint64_t)((x) | (x) << 16)) <<32 | (uint64_t)(x) |
>> > (uint64_t)(x) << 16
>> >  #define FIX64(x,s)  C64(FIX(x,s))
>
> i think the C64 / FIX64 macros depend on the & 0xFFFF. They are maybe
> unused
> and just referenced in comments but having them not give correct values
> could still be confusing. The 0xFFFF if its removed should be added to the
> code / comments that refer to it in a way that needs it

New patch attached.
Not sure if / where to add comments.

Please comment, Carl Eugen
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-lavfi-fspp-Cast-float-values-to-int16_t-instead-of-i.patch
Type: text/x-patch
Size: 881 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20190416/00084557/attachment.bin>