[Ffmpeg-devel] [PATCH] Vorbis Encoder

[Oded Shimon]

On Sun, Oct 01, 2006 at 09:19:56PM +0200, Michael Niedermayer wrote:
> On Sun, Oct 01, 2006 at 07:42:39PM +0200, Oded Shimon wrote:
> > 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:
> > > >     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()
> > 
> > fixed
> > 
> > ...
> > 
> > anything else? :)
> 
> regression tests, rate distortion optimal encoding, suport for making coffee

Should I really add regression tests? it's still in heavy development, 
this would be nothing but annoying. (psy needs to be tuned, or done at 
all. which btw, i have no clue about :( any volunteers?).

Hmm, the coffee thing seems fairly trivial. At least easier than getting 
this encoder into ffmpeg :)

> [...]
> > typedef struct {
> >     int channels;
> >     int sample_rate;
> >     int blocksize[2]; // in (1<<n) format
> 
> doxgen incompatible use ///< or another compatible one
> id also change the name to log2_blocksize or change what is stored in it

done

> [...]
> >         for (j = 0; j < cb->nentries; j++) {
> >             float a;
> >             if (!cb->lens[j]) continue;
> >             a = fabs(cb->dimentions[j * cb->ndimentions]);
> >             if (a > rc->maxes[i][0]) rc->maxes[i][0] = a;
> >             a = fabs(cb->dimentions[j * cb->ndimentions + 1]);
> >             if (a > rc->maxes[i][1]) rc->maxes[i][1] = a;
> >         }
> 
> maybe use FFMAX() but it still is unreadable then ...
> 
> 
> [...]
> >         cb->ndimentions = cvectors[book].dim;
> >         cb->nentries = cvectors[book].real_len;
> >         cb->min = cvectors[book].min;
> >         cb->delta = cvectors[book].delta;
> >         cb->lookup = cvectors[book].lookup;
> 
> following looks better IMHO (just a suggestion)
>          cb->ndimentions = cvectors[book].dim;
>          cb->nentries    = cvectors[book].real_len;
>          cb->min         = cvectors[book].min;
>          cb->delta       = cvectors[book].delta;
>          cb->lookup      = cvectors[book].lookup;
> 
> btw, would you mind if i or someone else does such cosmetic changes after
> the code is in ffmpeg-svn?
> what about indenting it sanely like
> for()
>     if()
>         statement;
> 
> instead of
> for() if() statement;
> ?

I don't mind... I won't do it myself though.

> >     // just 1 floor
> >     fc = &venc->floors[0];
> >     fc->partitions = 8;
> >     fc->partition_to_class = av_malloc(sizeof(int) * fc->partitions);
> >     fc->nclasses = 0;
> >     for (i = 0; i < fc->partitions; i++) {
> >         int a[] = {0,1,2,2,3,3,4,4};
> 
> static const
> 
> 
> >         fc->partition_to_class[i] = a[i];
> >         fc->nclasses = FFMAX(fc->nclasses, fc->partition_to_class[i]);
> >     }
> >     fc->nclasses++;
> >     fc->classes = av_malloc(sizeof(floor_class_t) * fc->nclasses);
> >     for (i = 0; i < fc->nclasses; i++) {
> >         floor_class_t * c = &fc->classes[i];
> >         int j, books;
> >         int dim[] = {3,4,3,4,3};
> >         int subclass[] = {0,1,1,2,2};
> >         int masterbook[] = {0/*none*/,0,1,2,3};
> >         int * nbooks[] = {
> >             (int[]){ 4 },
> >             (int[]){ 5, 6 },
> >             (int[]){ 7, 8 },
> >             (int[]){ -1, 9, 10, 11 },
> >             (int[]){ -1, 12, 13, 14 },
> >         };
> 
> static const

fixed

> except these iam ok with seeing this applied to svn, it still needs
> some cleanup work but that can be done in ffmpeg-svn

I'll commit tommorrow morning...

- ods15