On Fri, Nov 13, 2015 at 11:43 AM, Ganesh Ajjanagadde <gajja...@mit.edu> wrote:
> On Wed, Nov 11, 2015 at 6:53 AM, Michael Niedermayer
> <mich...@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 <gajjanaga...@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.
pushed all of the above that Michael has reviewed. Remainder sent in a
separate patch.
>
>>
>> [...]
>> --
>> 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@ffmpeg.org
>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
