[FFmpeg-devel] [PATCH] TwinVQ decoder

[Diego Biurrun]

On Wed, Jul 22, 2009 at 09:16:50AM +0200, Vitor Sessak wrote:
> 
> New version attached. Changes:
> 
> - Use DSP functions
> - Do not alloc big buffers on the stack
> - Some random optimizations
> - Diego's cosmetics suggestions

I have some more :)

> #define PPC_SHAPE_CB_SIZE 64
> #define SUB_AMP_MAX       4500.
> #define MULAW_MU          100.
> #define GAIN_BITS         8
> #define AMP_MAX           13000.
> #define SUB_GAIN_BITS     5
> #define WINDOW_TYPE_BITS  4
> #define PGAIN_MU          200

Maybe you could use 'XXX.0' for float constants?  The final dot without
zero keeps confusing me.

> static float eval_lpc_spectrum(const float *lsp, float cos_val, int order)
> {
>     int j;
>     float p=0.5f;
>     float q=0.5f;
>     float two_cos_w=2.0f*cos_val;
> 
>     for (j=0; j+1 < order; j += 2*2) {

Sometimes you place spaces around '=', sometimes you don't.  I think it
would be more readable with spaces, same below..

>         // Unroll once the loop since order is a multiple of four

Unroll the loop once ..

>         q *= lsp[j  ]-two_cos_w;
>         p *= lsp[j+1]-two_cos_w;
> 
>         q *= lsp[j+2]-two_cos_w;
>         p *= lsp[j+3]-two_cos_w;
>     }
> 
>     p *= p*(2.0f-two_cos_w);
>     q *= q*(2.0f+two_cos_w);
> 
>     return 0.5/(p+q);

Here (and in other places) the code could be more readable with a bit of
whitespace around operators..

>     int size_s = mtab->size / mtab->fmode[FT_SHORT].sub;
>     int mag_95 = 2 * mtab->fmode[FT_SHORT].sub;

align

> /**
>  * Evaluates the LPC amplitude spectrum envelope from the line spectrum pairs.
>  * Probably for speed reasons, the coefficients are evaluated like

evaluated as

>  * siiiibiiiiisiiiibiiiiisiiiibiiiiisiiiibiiiiis ...
>  * where s is an evaluated value, i is a value interpolated from the others
>  * and b might be either calculated or interpolated, dependent on a
>  * unexplained condition.

dependING on aN

> /**
>  * Evaluate a*b/400 rounded to the nearest integer. When, for example,
>  * a*b == 200 and the nearest integer is ill-defined, use a table to emulate
>  * the following broken float-based implementation used by the binary decoder:
>  *
>  * \code
>  * static int very_broken_op(int a, int b)
>  * {
>  *    static float test; // Ugh, force gcc to do the division first...
>  *
>  *    test = a/400.;
>  *    return b * test +  .5;

0.5?

>  * @note if this function is replaced by just ROUNDED_DIV(a*b,400.), the stddev
>  * between the original file (before encoding with Yamaha encoder) and the
>  * decoded output increases, which leads one to believe that the encoder expect
>  * exactly this broken calculation.

expectS

> /**
>  * Sum to data a periodic peak of period period/400.0 and width width and shape
>  * shape.

Ummm, what?

>     // First peak centered in zero

centered around?

>     // This is actually the period multiplied by 400. It is just linearly coded
>     //  between its maximum and minimum value

End sentences in a period; double space.

>  * Rearrange the LSP coefficients so that they have a minimum distance of
>  * min_dist. This function does it exactly as described in section of 3.2.4
>  * of the G729 specification (but interestingly is different from what the
>  * reference decoder actually does).

G.729

>     // Copy input's forth block to history

foUrth input block

>     // Update the rest of the history for using in the next frame

use

> /**
>  * Init IMDCT and windowing tables

Initialize IMDCT and windowing tables.

> /**
>  * Interpret the data as inside a num_blocks x line_len[0] matrix and for each

Interpret the data as if it were inside..



> /*
>  * The bark_tab_* tables are constructed so that
>  *
>  *       /i-1              \
>  *       |--               |
>  *  bark |\   bark_tab[j]  | == i
>  *       |/                |
>  *       |--               |
>  *       \j=0              /
>  *
>  *
>  * for some slightly non conventional bark-scale function

nonconventional or non-conventional

Diego