[Ffmpeg-devel] [PATCH] Vorbis Encoder
Oded Shimon
ods15
Sun Oct 1 18:19:30 CEST 2006
On Sun, Oct 01, 2006 at 05:43:47PM +0200, Michael Niedermayer wrote:
> On Sun, Oct 01, 2006 at 02:04:59PM +0200, Oded Shimon wrote:
> > static void ready_floor(floor_t * fc) {
>
> duplicate of ff_vorbis_ready_floor1_list() ?
oops :)
> [...]
> > 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?
doesn't really matter, i guess affects speed slightly, fixed.
> > 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()
Hmm, I'll try to fix... I'll also move the static function from vorbis.h .
> > 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
?..
> > 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]));
Fixed...
> > 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?
> > 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 ...
fixed
> > 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
fixed
> > 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];
evantually this encoder is supposed to support non-constant window sizes,
though maybe I'll drop that idea. If it does, then not all values are gone
through.
> > 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;'
Is this faster or slower? because it is an additional instruction, and
branches which depend on each other...
- ods15
More information about the ffmpeg-devel
mailing list