[Ffmpeg-devel] [PATCH] Vorbis Encoder

[Michael Niedermayer]

Hi

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


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


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


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


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

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