[FFmpeg-devel] [PATCH][RFC] swresample/dither: use integer arithmetic

[Ganesh Ajjanagadde]

On Sun, Aug 23, 2015 at 4:53 PM, Michael Niedermayer
<michael at niedermayer.cc> wrote:
> On Sun, Aug 23, 2015 at 03:10:40PM -0400, Ganesh Ajjanagadde wrote:
>> On Sun, Aug 23, 2015 at 2:45 PM, Michael Niedermayer
>> <michael at 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 <gajjanagadde at 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?
>
> you can use FFABS() with 64bit arguments

Unless I am mistaken, since the rates are ints, FFABS gives an int expression.
The promotion to 64 bits won't take place without an explicit cast,
and overflow is possible.
See the patchv2, I think it is better -> long long is guaranteed to
have 64 bits,
and there is no need of an explicit cast.

>
> [...]
> --
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> Let us carefully observe those good qualities wherein our enemies excel us
> and endeavor to excel them, by avoiding what is faulty, and imitating what
> is excellent in them. -- Plutarch
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>