On 23/08/15 4:10 PM, Ganesh Ajjanagadde wrote:
> On Sun, Aug 23, 2015 at 2:45 PM, Michael Niedermayer
> <mich...@niedermayer.cc> wrote:
>> On Sun, Aug 23, 2015 at 02:28:07PM -0400, Ganesh Ajjanagadde wrote:
>>> This fixes a -Wabsolute-value reported by clang 3.5+ complaining about
>>> misuse of fabs() for integer absolute value.
>>> An additional benefit is the removal of floating point calculations.
>>> Note that the behaviors are not exactly identical, but should be ok in most
>>> situations.
>>>
>>> Signed-off-by: Ganesh Ajjanagadde <gajjanaga...@gmail.com>
>>> ---
>>> libswresample/dither.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/libswresample/dither.c b/libswresample/dither.c
>>> index 248062a..fc08932 100644
>>> --- a/libswresample/dither.c
>>> +++ b/libswresample/dither.c
>>> @@ -109,7 +109,7 @@ av_cold int swri_dither_init(SwrContext *s, enum
>>> AVSampleFormat out_fmt, enum AV
>>> memset(s->dither.ns_errors, 0, sizeof(s->dither.ns_errors));
>>> for (i=0; filters[i].coefs; i++) {
>>> const filter_t *f = &filters[i];
>>> - if (fabs(s->out_sample_rate - f->rate) / f->rate <= .05 && f->name
>>> == s->dither.method) {
>>> + if ((abs(s->out_sample_rate - f->rate) <= f->rate / 20) && f->name
>>> == s->dither.method) {
>>
>> unneeded () and the identical condition should be used if possible
>
> Thanks for pointing this out, will change.
>
>>
>> something like
>> llabs(s->out_sample_rate - f->rate) * 20 <= f->rate
>
> I initially thought of this approach, but went with my current patch
> because I was afraid of overflow with abs().
> llabs() is a good clean solution, but is it available on all platforms?
> As per the documentation, it is C99 which is in general not safe to use.
>
> I notice one use in entire codebase (line 515 of avformat/dump.c), so
> it should be ok in my view.
> I will resend the patch only after someone confirms the safety of
> llabs on all platforms.
labs seems to be on every msvc release, but llabs only on msvc 2013 and newer.
https://msdn.microsoft.com/en-us/library/a206stx2.aspx
Maybe a replacement could be added to libavutil/libm.h.
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
