[FFmpeg-devel] [PATCH] all: use predefined mathematics macros

[Ganesh Ajjanagadde]

On Wed, Nov 11, 2015 at 6:53 AM, Michael Niedermayer
<michael at niedermayer.cc> wrote:
> On Tue, Nov 10, 2015 at 10:35:23PM -0500, Ganesh Ajjanagadde wrote:
>> This uses M_SQRT2, M_PI, and M_E instead of the actual literals.
>> Benefits include:
>> 1. Reduced scope for copy/paste errors and improved readability.
>> 2. Consistency across the codebase.
>> 3. Actually fixes an incorrect sqrt(2) in avcodec/ppc.
>> 4. Greater precision in avcodec/ac3.
>>
>> Patch tested with FATE on x86, ppc untested.
>>
>> Signed-off-by: Ganesh Ajjanagadde <gajjanagadde at gmail.com>
>> ---
>>  libavcodec/ac3.h                   |  2 +-
>>  libavcodec/cos_tablegen.c          |  4 +++-
>>  libavcodec/mpegaudioenc_template.c |  4 ++--
>>  libavcodec/mpegaudiotab.h          |  2 --
>>  libavcodec/mpegvideo.c             |  4 ++--
>>  libavcodec/ppc/fdctdsp.c           | 21 ++++++++++-----------
>>  libavcodec/ratecontrol.c           |  4 ----
>>  libavcodec/simple_idct.c           |  4 ++--
>>  libavfilter/af_dynaudnorm.c        |  4 +---
>>  9 files changed, 21 insertions(+), 28 deletions(-)
>>
> [...]
>
>> diff --git a/libavcodec/mpegaudioenc_template.c b/libavcodec/mpegaudioenc_template.c
>> index ce93cc7..b91d0a8 100644
>> --- a/libavcodec/mpegaudioenc_template.c
>> +++ b/libavcodec/mpegaudioenc_template.c
>> @@ -244,11 +244,11 @@ static void idct32(int *out, int *tab)
>>      do {
>>          int x1, x2, x3, x4;
>>
>> -        x3 = MUL(t[16], FIX(SQRT2*0.5));
>> +        x3 = MUL(t[16], FIX(M_SQRT2*0.5));
>>          x4 = t[0] - x3;
>>          x3 = t[0] + x3;
>>
>> -        x2 = MUL(-(t[24] + t[8]), FIX(SQRT2*0.5));
>> +        x2 = MUL(-(t[24] + t[8]), FIX(M_SQRT2*0.5));
>>          x1 = MUL((t[8] - x2), xp[0]);
>>          x2 = MUL((t[8] + x2), xp[1]);
>
> ok if the FIX() values stay the same

checked, they are identical.

>
>
>>
>> diff --git a/libavcodec/mpegaudiotab.h b/libavcodec/mpegaudiotab.h
>> index 42d42d8..bb2e5de 100644
>> --- a/libavcodec/mpegaudiotab.h
>> +++ b/libavcodec/mpegaudiotab.h
>> @@ -33,8 +33,6 @@
>>  #include <stdint.h>
>>  #include "mpegaudio.h"
>>
>> -#define SQRT2 1.41421356237309514547
>> -
>>  static const int costab32[30] = {
>>      FIX(0.54119610014619701222),
>>      FIX(1.3065629648763763537),
>
> ok if all use of the macro are removed

There is only one (unrelated) usage in avcodec/xvididct.c with a
separate local define. Thus should be ok.

>
>
>> diff --git a/libavcodec/mpegvideo.c b/libavcodec/mpegvideo.c
>> index 96634ec..69e0595 100644
>> --- a/libavcodec/mpegvideo.c
>> +++ b/libavcodec/mpegvideo.c
>> @@ -1870,8 +1870,8 @@ void ff_print_debug_info2(AVCodecContext *avctx, AVFrame *pict, uint8_t *mbskip_
>>                      uint64_t u,v;
>>                      int y;
>>  #define COLOR(theta, r) \
>> -    u = (int)(128 + r * cos(theta * 3.141592 / 180)); \
>> -    v = (int)(128 + r * sin(theta * 3.141592 / 180));
>> +    u = (int)(128 + r * cos(theta * M_PI / 180)); \
>> +    v = (int)(128 + r * sin(theta * M_PI / 180));
>>
>>
>>                      u = v = 128;
>
> ok
>
>
> [...]
>> diff --git a/libavcodec/ratecontrol.c b/libavcodec/ratecontrol.c
>> index 308e34e..e96550c 100644
>> --- a/libavcodec/ratecontrol.c
>> +++ b/libavcodec/ratecontrol.c
>> @@ -35,10 +35,6 @@
>>  #include "mpegvideo.h"
>>  #include "libavutil/eval.h"
>>
>> -#ifndef M_E
>> -#define M_E 2.718281828
>> -#endif
>> -
>>  static int init_pass2(MpegEncContext *s);
>>  static double get_qscale(MpegEncContext *s, RateControlEntry *rce,
>>                           double rate_factor, int frame_num);
>
> ok
>
>
>> diff --git a/libavcodec/simple_idct.c b/libavcodec/simple_idct.c
>> index 4d6d20d..0711e16 100644
>> --- a/libavcodec/simple_idct.c
>> +++ b/libavcodec/simple_idct.c
>> @@ -132,7 +132,7 @@ void ff_simple_idct248_put(uint8_t *dest, int line_size, int16_t *block)
>>  #undef C1
>>  #undef C2
>>  #define CN_SHIFT 12
>> -#define C_FIX(x) ((int)((x) * 1.414213562 * (1 << CN_SHIFT) + 0.5))
>> +#define C_FIX(x) ((int)((x) * M_SQRT2 * (1 << CN_SHIFT) + 0.5))
>>  #define C1 C_FIX(0.6532814824)
>>  #define C2 C_FIX(0.2705980501)
>>  #define C3 C_FIX(0.5)
>> @@ -159,7 +159,7 @@ static inline void idct4col_add(uint8_t *dest, int line_size, const int16_t *col
>>  }
>>
>>  #define RN_SHIFT 15
>> -#define R_FIX(x) ((int)((x) * 1.414213562 * (1 << RN_SHIFT) + 0.5))
>> +#define R_FIX(x) ((int)((x) * M_SQRT2 * (1 << RN_SHIFT) + 0.5))
>>  #define R1 R_FIX(0.6532814824)
>>  #define R2 R_FIX(0.2705980501)
>>  #define R3 R_FIX(0.5)
>
> this is ok if all calculated integer values stay the same

checked, they are the same. Patch series coming in with this and related work.

>
> [...]
> --
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> I am the wisest man alive, for I know one thing, and that is that I know
> nothing. -- Socrates
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>