diff mbox

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

Message ID CAB0OVGowhU7AHy7NhsiOkP=_fRhOeLFYS9b4j_rq3L9QeQ7XQA@mail.gmail.com
State Accepted
Headers show

Commit Message

Carl Eugen Hoyos April 18, 2019, 12:07 p.m. UTC 
2019-04-18 12:16 GMT+02:00, Michael Niedermayer <michael@niedermayer.cc>:
> On Tue, Apr 16, 2019 at 12:55:03AM +0200, Carl Eugen Hoyos wrote:
>> 2019-04-14 1:02 GMT+02:00, Michael Niedermayer <michael@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@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
>
>>  vf_fspp.h |    2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>> 17e468ee43988ef39d3ce882774dc3e2d625c9e6
>> 0001-lavfi-fspp-Cast-float-values-to-int16_t-instead-of-i.patch
>> From dfda9d8a6714649015f252e0e0bf5e3e52ce1b5c Mon Sep 17 00:00:00 2001
>> From: Carl Eugen Hoyos <ceffmpeg@gmail.com>
>> Date: Tue, 16 Apr 2019 00:50:50 +0200
>> Subject: [PATCH] lavfi/fspp: Cast float values to int16_t instead of int.
>>
>> 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 802db14..bdafe8e 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))
>
> does it need the cast at all to avoid the warning ?

Attached patch also avoids the warning.

Thank you, Carl Eugen

Comments

Carl Eugen Hoyos April 19, 2019, 10:24 p.m. UTC | #1 
2019-04-18 14:07 GMT+02:00, Carl Eugen Hoyos <ceffmpeg@gmail.com>:
> 2019-04-18 12:16 GMT+02:00, Michael Niedermayer <michael@niedermayer.cc>:

>>> 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 802db14..bdafe8e 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))
>>
>> does it need the cast at all to avoid the warning ?
>
> Attached patch also avoids the warning.

Patch applied.

Thank you, Carl Eugen
diff mbox

Patch

From 69c7f0894acccf64438198cd8d01a9f3e9765a95 Mon Sep 17 00:00:00 2001
From: Carl Eugen Hoyos <ceffmpeg@gmail.com>
Date: Thu, 18 Apr 2019 14:05:21 +0200
Subject: [PATCH] lavfi/fspp: Simplify a macro.

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 802db14..73d8c7c 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)  ((x) * (1 << s) + 0.5)
 
 #define MULTIPLY16H(x,k)   (((x) * (k)) >> 16)
 #define THRESHOLD(r,x,t)                         \
-- 
1.7.10.4