[Ffmpeg-devel] [PATCH] Vorbis Encoder

[Michael Niedermayer]

Hi

On Sun, Oct 01, 2006 at 06:19:30PM +0200, Oded Shimon wrote:
[...]
> > > 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
> 
> I fail to see how this would be any faster? Still the same big-O, and is
> d += (a-b)*(a-b)
> in flot significantly slower than
> d += a*b
> ?..

no not significantly, its just 1 subtraction less


[...]
> > >                     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
> 
> Is something like
> if (++a == real_ch) { a=0; b++; }
> acceptable?

yes, better then the /
2 loops might be cleaner though


[...]
> > >     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;
> 
> You got it wrong in the end, it's 'm=old_a;'

iam too tired for these things today :(


> Is this faster or slower? because it is an additional instruction, and 
> branches which depend on each other...

you dont need branches if you are crazy enough, just do one loop with the
a-m
and then play with the signs and exchange the variables with integer
instructions in the second loop

anyway this is getting off topic ...

[...]
-- 
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