[FFmpeg-devel] [PATCH] Some ra144.c simplifications

[Michael Niedermayer]

On Sun, May 11, 2008 at 05:45:29PM +0200, Vitor Sessak wrote:
> Michael Niedermayer wrote:
>> On Sat, May 10, 2008 at 04:50:03PM +0200, Vitor Sessak wrote:
>>> Hi,
>>>
>>> libavcodec/ra144.c really needs some cleanup. I'll start with the 
>>> following:
>>>
>>> ra144_indent.diff: Reindent the whole file. Since there was no
>>> substantial change in this file since it inclusion in 2003, I don't
>>> think there is any problem in making its indentation inline with the
>>> rest of FFmpeg.
>>>
>>> ra144_unpack_rewrite.diff: Rewrite completely unpack_input()
>>>
>>> ra144_simp{1,2}.diff: minor simplifications
>> all ok if you have tested your code
>
> Next round:
>
> ra144_simp{3,4}.diff: minor simplifications (FFSWAP, unused stuff removal)

ok


>
> ra144_sqrt.diff: remove ff_sqrt reimplementation and its corresponding
> big table (output is not anymore binary-identical but this function is
> more accurate, not less). Nice thing we have now a fast ff_sqrt :-)

by how much does the output differ?


>
> ra144_do_voice.diff: simplify do_voice()
>

> ra144_split_val.diff: glob->val is unrelated with the rest of the
> unpacked buffer. This patch split its table out of the others

ok


> and move
> it parsing to a more reasonable place.

not ok, the place does not look reasonable to me

and i assume of course that this has been tested ...


[...]

> diff -ur ../old/libavcodec/ra144.c ./libavcodec/ra144.c
> --- ../old/libavcodec/ra144.c	2008-05-11 11:40:01.000000000 +0200
> +++ ./libavcodec/ra144.c	2008-05-11 11:39:48.000000000 +0200
> @@ -90,16 +90,16 @@
>  static int irms(short *data, int factor);
>  static void rotate_block(short *source, short *target, int offset);
>  
> -/* lookup square roots in table */
> -static int t_sqrt(unsigned int x)
> -{
> -    int s = 0;
> -    while (x > 0xfff) {
> -        s++;
> -        x = x >> 2;
> -    }
> +/**
> + * Evaluate sqrt(x << 24). x must fit in 20 bits.
> + */
> +static inline int t_sqrt(unsigned int x)
> +{
> +    /* To have a good precision, sqrt(x << 24) = sqrt(x << 2*s) << (12-s)
> +       where s is chosen to be as big as possible without overflow */
> +    int s = (25-av_log2(x)) >> 1;
>  
> -    return (sqrt_table[x] << s) << 2;
> +    return ff_sqrt(x << (s << 1)) << (12 - s);
>  }

the addition of inline is unrelated and should be in a seperate commit.


[...]
> --- ../ffmpeg.old2/libavcodec/ra144.c	2008-05-11 11:41:01.000000000 +0200
> +++ libavcodec/ra144.c	2008-05-11 11:43:27.000000000 +0200
> @@ -106,28 +106,21 @@
>  static void do_voice(int *a1, int *a2)
>  {
>      int buffer[10];
> +    int *b1 = buffer;
> +    int *b2 = a2;
>      int x, y;
>  
>      for (x=0; x < 10; x++) {
> +        b1[x] = a1[x] << 4;
> +
> +        for (y=0; y < x; y++)

> +            b1[y] = ((a1[x] * (b2[x-y-1])) >> 12) + b2[y];
                                 ^         ^
superflous ()

[...]

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

No human being will ever know the Truth, for even if they happen to say it
by chance, they would not even known they had done so. -- Xenophanes
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080511/c1e7335e/attachment.pgp>