Re: [FFmpeg-devel] [PATCH 2/2] avcodec/sbrdsp_fixed: Fix integer overflow

2017-11-24 Thread Michael Niedermayer
On Thu, Nov 23, 2017 at 11:00:21PM +0100, Michael Niedermayer wrote:
> On Wed, Nov 22, 2017 at 11:59:30PM +0100, Hendrik Leppkes wrote:
> > On Wed, Nov 22, 2017 at 11:38 PM, Carl Eugen Hoyos  
> > wrote:
> > > 2017-11-22 21:00 GMT+01:00 Michael Niedermayer :
> > >
> > >> diff --git a/libavcodec/sbrdsp_fixed.c b/libavcodec/sbrdsp_fixed.c
> > >> index a0ef6859f1..0db932a105 100644
> > >> --- a/libavcodec/sbrdsp_fixed.c
> > >> +++ b/libavcodec/sbrdsp_fixed.c
> > >> @@ -133,7 +133,7 @@ static av_always_inline SoftFloat 
> > >> autocorr_calc(int64_t accu)
> > >>
> > >>  round = 1U << (nz-1);
> > >>  mant = (int)((accu + round) >> nz);
> > >> -mant = (mant + 0x40)>>7;
> > >> +mant = (mant + 0x40ll)>>7;
> > >
> > > LL?
> 
> I can change it to LL if preferred, i guess ill do that, LL seems more
> common in our source

and applied with LL


[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Concerning the gods, I have no means of knowing whether they exist or not
or of what sort they may be, because of the obscurity of the subject, and
the brevity of human life -- Protagoras


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH 2/2] avcodec/sbrdsp_fixed: Fix integer overflow

2017-11-23 Thread James Almer
On 11/22/2017 7:59 PM, Hendrik Leppkes wrote:
> On Wed, Nov 22, 2017 at 11:38 PM, Carl Eugen Hoyos  wrote:
>> 2017-11-22 21:00 GMT+01:00 Michael Niedermayer :
>>
>>> diff --git a/libavcodec/sbrdsp_fixed.c b/libavcodec/sbrdsp_fixed.c
>>> index a0ef6859f1..0db932a105 100644
>>> --- a/libavcodec/sbrdsp_fixed.c
>>> +++ b/libavcodec/sbrdsp_fixed.c
>>> @@ -133,7 +133,7 @@ static av_always_inline SoftFloat autocorr_calc(int64_t 
>>> accu)
>>>
>>>  round = 1U << (nz-1);
>>>  mant = (int)((accu + round) >> nz);
>>> -mant = (mant + 0x40)>>7;
>>> +mant = (mant + 0x40ll)>>7;
>>
>> LL?
>>
> 
> More correctly, shouldnt this use one of those fancy integer constant
> macros, like INT64_C(0x40)? (I don't actually know if those are
> supposed to work with hex constants, but the fact that they exist
> seems to indicate that LL is not entirely portable)
> 
> - Hendrik

LL/ULL are used plenty of times in the tree, so it's portable enough.
What i remember not working at least on old msvc was LLU.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH 2/2] avcodec/sbrdsp_fixed: Fix integer overflow

2017-11-23 Thread Michael Niedermayer
On Wed, Nov 22, 2017 at 11:59:30PM +0100, Hendrik Leppkes wrote:
> On Wed, Nov 22, 2017 at 11:38 PM, Carl Eugen Hoyos  wrote:
> > 2017-11-22 21:00 GMT+01:00 Michael Niedermayer :
> >
> >> diff --git a/libavcodec/sbrdsp_fixed.c b/libavcodec/sbrdsp_fixed.c
> >> index a0ef6859f1..0db932a105 100644
> >> --- a/libavcodec/sbrdsp_fixed.c
> >> +++ b/libavcodec/sbrdsp_fixed.c
> >> @@ -133,7 +133,7 @@ static av_always_inline SoftFloat 
> >> autocorr_calc(int64_t accu)
> >>
> >>  round = 1U << (nz-1);
> >>  mant = (int)((accu + round) >> nz);
> >> -mant = (mant + 0x40)>>7;
> >> +mant = (mant + 0x40ll)>>7;
> >
> > LL?

I can change it to LL if preferred, i guess ill do that, LL seems more
common in our source


> >
> 
> More correctly, shouldnt this use one of those fancy integer constant
> macros, like INT64_C(0x40)? (I don't actually know if those are
> supposed to work with hex constants, but the fact that they exist
> seems to indicate that LL is not entirely portable)

we use both LL and ll already, so there should be no issue that we
arent already affected by since "forever"

[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Democracy is the form of government in which you can choose your dictator


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH 2/2] avcodec/sbrdsp_fixed: Fix integer overflow

2017-11-22 Thread Hendrik Leppkes
On Wed, Nov 22, 2017 at 11:38 PM, Carl Eugen Hoyos  wrote:
> 2017-11-22 21:00 GMT+01:00 Michael Niedermayer :
>
>> diff --git a/libavcodec/sbrdsp_fixed.c b/libavcodec/sbrdsp_fixed.c
>> index a0ef6859f1..0db932a105 100644
>> --- a/libavcodec/sbrdsp_fixed.c
>> +++ b/libavcodec/sbrdsp_fixed.c
>> @@ -133,7 +133,7 @@ static av_always_inline SoftFloat autocorr_calc(int64_t 
>> accu)
>>
>>  round = 1U << (nz-1);
>>  mant = (int)((accu + round) >> nz);
>> -mant = (mant + 0x40)>>7;
>> +mant = (mant + 0x40ll)>>7;
>
> LL?
>

More correctly, shouldnt this use one of those fancy integer constant
macros, like INT64_C(0x40)? (I don't actually know if those are
supposed to work with hex constants, but the fact that they exist
seems to indicate that LL is not entirely portable)

- Hendrik
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH 2/2] avcodec/sbrdsp_fixed: Fix integer overflow

2017-11-22 Thread Carl Eugen Hoyos
2017-11-22 21:00 GMT+01:00 Michael Niedermayer :

> diff --git a/libavcodec/sbrdsp_fixed.c b/libavcodec/sbrdsp_fixed.c
> index a0ef6859f1..0db932a105 100644
> --- a/libavcodec/sbrdsp_fixed.c
> +++ b/libavcodec/sbrdsp_fixed.c
> @@ -133,7 +133,7 @@ static av_always_inline SoftFloat autocorr_calc(int64_t 
> accu)
>
>  round = 1U << (nz-1);
>  mant = (int)((accu + round) >> nz);
> -mant = (mant + 0x40)>>7;
> +mant = (mant + 0x40ll)>>7;

LL?

Carl Eugen
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel