[Ffmpeg-devel] [PATCH] Vorbis Encoder

Michael Niedermayer michaelni
Sun Oct 1 17:43:47 CEST 2006


Hi

On Sun, Oct 01, 2006 at 02:04:59PM +0200, Oded Shimon wrote:
[...]
> Try 2. vorbis_enc.c here..
> 
> - ods15

[...]
> static const uint8_t codebook2[] = {
>    1,  5,  7, 21,  5,  8,  9, 21, 10,  9, 12, 20, 20, 16, 20,
>   20,  4,  8,  9, 20,  6,  8,  9, 20, 11, 11, 13, 20, 20, 15,
>   17, 20,  9, 11, 14, 20,  8, 10, 15, 20, 11, 13, 15, 20, 20,
>   20, 20, 20, 20, 20, 20, 20, 13, 20, 20, 20, 18, 18, 20, 20,
>   20, 20, 20, 20,  3,  6,  8, 20,  6,  7,  9, 20, 10,  9, 12,
>   20, 20, 20, 20, 20,  5,  7,  9, 20,  6,  6,  9, 20, 10,  9,
>   12, 20, 20, 20, 20, 20,  8, 10, 13, 20,  8,  9, 12, 20, 11,
>   10, 12, 20, 20, 20, 20, 20, 18, 20, 20, 20, 15, 17, 18, 20,
>   18, 17, 18, 20, 20, 20, 20, 20,  7, 10, 12, 20,  8,  9, 11,
>   20, 14, 13, 14, 20, 20, 20, 20, 20,  6,  9, 12, 20,  7,  8,
>   11, 20, 12, 11, 13, 20, 20, 20, 20, 20,  9, 11, 15, 20,  8,
>   10, 14, 20, 12, 11, 14, 20, 20, 20, 20, 20, 20, 20, 20, 20,
>   20, 20, 20, 20, 20, 20, 20, 20, 20, 20, 20, 20, 11, 16, 18,
>   20, 15, 15, 17, 20, 20, 17, 20, 20, 20, 20, 20, 20,  9, 14,
>   16, 20, 12, 12, 15, 20, 17, 15, 18, 20, 20, 20, 20, 20, 16,
>   19, 18, 20, 15, 16, 20, 20, 17, 17, 20, 20, 20, 20, 20, 20,
>   20, 20, 20, 20, 20, 20, 20, 20, 20, 20, 20, 20, 20, 20, 20,
>   20,

changing the number of elements per line makes the numbers look
much nicer (yes feel free to ignore me :) )

    1,  5,  7, 21,  5,  8,  9, 21, 10,  9, 12, 20, 20, 16, 20, 20,
    4,  8,  9, 20,  6,  8,  9, 20, 11, 11, 13, 20, 20, 15, 17, 20,  
    9, 11, 14, 20,  8, 10, 15, 20, 11, 13, 15, 20, 20, 20, 20, 20, 
   20, 20, 20, 20, 13, 20, 20, 20, 18, 18, 20, 20, 20, 20, 20, 20,  
    3,  6,  8, 20,  6,  7,  9, 20, 10,  9, 12, 20, 20, 20, 20, 20, 
    5,  7,  9, 20,  6,  6,  9, 20, 10,  9, 12, 20, 20, 20, 20, 20,  
    8, 10, 13, 20,  8,  9, 12, 20, 11, 10, 12, 20, 20, 20, 20, 20,
   18, 20, 20, 20, 15, 17, 18, 20, 18, 17, 18, 20, 20, 20, 20, 20,
    7, 10, 12, 20,  8,  9, 11, 20, 14, 13, 14, 20, 20, 20, 20, 20,
    6,  9, 12, 20,  7,  8, 11, 20, 12, 11, 13, 20, 20, 20, 20, 20,  
    9, 11, 15, 20,  8, 10, 14, 20, 12, 11, 14, 20, 20, 20, 20, 20, 
   20, 20, 20, 20, 20, 20, 20, 20, 20, 20, 20, 20, 20, 20, 20, 20, 
   11, 16, 18, 20, 15, 15, 17, 20, 20, 17, 20, 20, 20, 20, 20, 20,  
    9, 14, 16, 20, 12, 12, 15, 20, 17, 15, 18, 20, 20, 20, 20, 20, 
   16, 19, 18, 20, 15, 16, 20, 20, 17, 17, 20, 20, 20, 20, 20, 20,
   20, 20, 20, 20, 20, 20, 20, 20, 20, 20, 20, 20, 20, 20, 20, 20,



[...]
> static void ready_floor(floor_t * fc) {

duplicate of ff_vorbis_ready_floor1_list() ?

[...]
> static void floor_fit(venc_context_t * venc, floor_t * fc, float * coeffs, int * posts, int samples) {
>     int range = 255 / fc->multiplier + 1;
>     int i;
>     float tot_average = 0.;
>     for (i = 0; i < fc->values; i++) tot_average += get_floor_average(fc, coeffs, i);
>     tot_average /= fc->values;
>     tot_average /= venc->quality;
> 
>     for (i = 0; i < fc->values; i++) {
>         int position = fc->list[fc->list[i].sort].x;
>         float average = get_floor_average(fc, coeffs, i);

doesnt this calculate the average twice?


[...]

>     lx = 0;
>     ly = posts[0] * fc->multiplier; // sorted 0 is still 0
>     for (i = 1; i < fc->values; i++) {
>         int pos = fc->list[i].sort;
>         if (coded[pos]) {
>             render_line(lx, ly, fc->list[pos].x, posts[pos] * fc->multiplier, floor, samples);
>             lx = fc->list[pos].x;
>             ly = posts[pos] * fc->multiplier;
>         }
>         if (lx >= samples) break;
>     }
>     if (lx < samples) render_line(lx, ly, samples, ly, floor, samples);

this looks duplicated from the end of vorbis_floor1_decode()


> }
> 
> static float * put_vector(codebook_t * book, PutBitContext * pb, float * num) {
>     int i;
>     int entry = -1;
>     float distance = 0;
>     assert(book->dimentions);
>     for (i = 0; i < book->nentries; i++) {
>         float d = 0.;
>         int j;
>         if (!book->lens[i]) continue;
>         for (j = 0; j < book->ndimentions; j++) {
>             float a = (book->dimentions[i * book->ndimentions + j] - num[j]);
>             d += a*a;
>         }
>         if (entry == -1 || distance > d) {

set distance to FLOAT_MAX initially and forget the entry check
you can also optimize the loop a little by using:
sum (a-b)^2 = sum a^2 + sum b^2 - 2*sum ab
and precalculating sum a^2 and sum b^2


[...]
>     for (p = 0; p < partitions; p++) {
>         float max1 = 0., max2 = 0.;
>         int s = rc->begin + p * psize;
>         for (k = s; k < s + psize; k += 2) {
>             if (fabs(coeffs[k / real_ch]) > max1) max1 = fabs(coeffs[k / real_ch]);
>             if (fabs(coeffs[samples + k / real_ch]) > max2) max2 = fabs(coeffs[samples + k / real_ch]);

max1 = FFMAX(max1, fabs(coeffs[          k / real_ch]));
max2 = FFMAX(max2, fabs(coeffs[samples + k / real_ch]));


[...]
>                     if (rc->type == 0) {
>                         for (k = 0; k < psize; k += book->ndimentions) {
>                             float * a = put_vector(book, pb, &buf[k]);
>                             int l;
>                             for (l = 0; l < book->ndimentions; l++) buf[k + l] -= a[l];
>                         }
>                     } else {
>                         for (k = 0; k < psize; k += book->ndimentions) {
>                             int dim = book->ndimentions, s = rc->begin + p * psize + k, l;
>                             float vec[dim], * a = vec;
>                             for (l = s; l < s + dim; l++)
>                                 *a++ = coeffs[(l % real_ch) * samples + l / real_ch];
>                             a = put_vector(book, pb, vec);
>                             for (l = s; l < s + dim; l++)
>                                 coeffs[(l % real_ch) * samples + l / real_ch] -= *a++;

the / and % should be avoided, especially considering that real_ch is always 2
in your current code


[...]

> static int window(venc_context_t * venc, signed short * audio, int samples) {

window what? init_window, apply_window, ... please use more descriptive
names, this one is totally unacceptable (this applies the window and runs
the mdct over it) call it apply_window_and_mdct() or whatever ...


[...]
> static int vorbis_encode_init(AVCodecContext * avccontext)
> {
>     venc_context_t * venc = avccontext->priv_data;
> 
>     if (avccontext->channels != 2) return -1;

error message please, or we will have heaps of lemmings spamming the MLs


[...]>     for (i = 0; i < venc->channels; i++) {
>         int j;
>         for (j = 0; j < samples; j++) {
>             venc->coeffs[i * samples + j] /= venc->floor[i * samples + j];
>         }
>     }

for(i=0; i<venc->channels * samples; i++)
    venc->coeffs[i] /= venc->floor[i];


> 
>     for (i = 0; i < mapping->coupling_steps; i++) {
>         float * mag = venc->coeffs + mapping->magnitude[i] * samples;
>         float * ang = venc->coeffs + mapping->angle[i] * samples;
>         int j;
>         for (j = 0; j < samples; j++) {
>             float m = mag[j];
>             float a = ang[j];
>             if (m > 0) {
>                 ang[j] = m - a;
>                 if (a > m) mag[j] = a;
>                 else mag[j] = m;
>             } else {
>                 ang[j] = a - m;
>                 if (a > m) mag[j] = m;
>                 else mag[j] = a;
>             }
>         }
>     }

i think the following is equivalent and simpler, (and its immedeatly
obvious how to reverse it)

a -= m;
if(m>0) a= -a;
if(a<0) m= -m;

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

In the past you could go to a library and read, borrow or copy any book
Today you'd get arrested for mere telling someone where the library is




More information about the ffmpeg-devel mailing list