[FFmpeg-devel] [PATCH] Add a codebook generator

[Vitor]

Hi

Michael Niedermayer wrote:
> Hi
>
>
> [...]
>   
>> +
>> +/**
>> + * Implementation of the simple LBG algorithm for just two codebooks
>> + */
>> +static int simple_lbg(int *centroid0, int *centroid1,
>> +                      int *newutility0, int *newutility1,
>> +                      int **points,
>> +                      cell *cells, int dim)
>> +{
>> +    int i, idx;
>> +    int numpoints[2];
>> +    int newcentroid[2][dim];
>> +    cell *tempcell;
>> +
>> +    memset(newcentroid[0], 0, dim*sizeof(int));
>> +    memset(newcentroid[1], 0, dim*sizeof(int));
>> +
>> +    numpoints[0]=0;
>> +    numpoints[1]=0;
>>     
>
> int numpoints[2]={0,0};
> int newcentroid[2][dim]= {0};
>
>
>   

My compiler (gcc 4.1.2) don't like very much "int newcentroid[2][dim]= {0};"

> [...]
>   
>> +/**
>> + * Evaluate if a shift lower the error. If it does, call shift_codebooks
>> + * and update elbg->error, elbg->utility and elbg->nearest_neigh.
>> + *
>> + * @param i The low utility cell
>> + * @param p The high utility cell
>> + * @param l The closest cell to the i'th cell
>>     
>
> may i protest against the i/p/l naming
> a more logic naming would be
> luc  low  utility cell
> huc  high utility cell
> cluc The closest cell to the low untilit cell
>
> there are of course millions of other namings but the i/p/l looks completely
> arbitrary to me, also i is generally an integer used in for loops and not
> some argument ...
>
> and these parameters could also be passed as an array like
> {cluc, luc, huc} or another order
> this would simplify some code, for example it would avoid the
> int idx[2] = {i, l};
>
>
>   

I decided to use these names to mimic the notation of the paper that 
describes the algorithm. But I guess that if someone wants to understand 
the code will just read it, and not the paper, so I agree.

> [...]
>   
> one trick which might speed this up is to apply a orthogonal decorrelation
> transform on the points (for example the DCT or hadamard transform) the
> reason why this might be faster is that it concentrates the energy into
> the first few coefficients and so the distance_limited() will on average
> stop sooner
> note that the euclidian distance does not change between orthogonally
> transformed (=rotated) points
> though this maybe should be left for a future patch ...
>
>
> [...]
>   

I indeed prefer to leave to after the RoQ encoder is in the tree. The 
other points I didn't mentioned where changed as suggested.

-Vitor
-------------- next part --------------
A non-text attachment was scrubbed...
Name: elbg4.diff
Type: text/x-patch
Size: 16002 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20070601/4d6973b3/attachment.bin>