Re: [PATCH] Vorbis Encoder

On Sat, Sep 30, 2006 at 11:33:49PM +0300, Oded Shimon wrote:
> Quality is comparable to official vorbis encoder (to my ears), at similar 
> bitrate. bitrate and quality can be manipulated with -aq, 10 to 30 are 
> sane values, the higher the number, the higher the bitrate/quality.

This is highly misleading. Quality may be comparable at very high
bitrates, but for most applications moderate to low bitrates are more
interesting.

BTW how does speed compare?

Rich

[patch] idct_mmx_xvid - LGPL license

enclosed is an ammendment to the license description of idct_mmx_xvid.c
to grant usage of this file under the LGPL.

-- pete

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel <at> mplayerhq.hu
http://lists.mplayerhq.hu/mailman/listinfo/ffmpeg-devel

[PATCH] fix build with --disable-encoders

dv.c fails to compile because one function isn't hidden when encoders  
are disabled. The attached patch corrects it.

PS that -Wdisabled-optimization sure makes a lot of warnings on Darwin.

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel <at> mplayerhq.hu
http://lists.mplayerhq.hu/mailman/listinfo/ffmpeg-devel

Re: [PATCH] Vorbis Encoder

On Sat, Sep 30, 2006 at 09:07:12PM -0400, Rich Felker wrote:
> On Sat, Sep 30, 2006 at 11:33:49PM +0300, Oded Shimon wrote:
> > Quality is comparable to official vorbis encoder (to my ears), at similar 
> > bitrate. bitrate and quality can be manipulated with -aq, 10 to 30 are 
> > sane values, the higher the number, the higher the bitrate/quality.
> 
> This is highly misleading. Quality may be comparable at very high
> bitrates, but for most applications moderate to low bitrates are more
> interesting.

~110kbps doesn't sound like "very high bitrate" .. at 90kbps my encoder 
stll works well, below that quality is noticably lower.

> BTW how does speed compare?

No effort was put into optimization, but not much to optimize either. 
Seems same speed/slightly faster than oggenc.

- ods15

Re: [PATCH] Vorbis Encoder

On Sat, Sep 30, 2006 at 11:33:49PM +0300, Oded Shimon wrote:
> $subj
> 
> Some cleanups still left - move the codebook stuff to a seperate file, 
> move the bitstream stuff to bitstream.h . Will move the codebook stuff 
> only after moving to ffmpeg repo...
> 
> This will be commited using ~90 commits for full history, with an 
> additional commit at the end for makefile/avocdec.h/allcodecs.c
> 
> Quality is comparable to official vorbis encoder (to my ears), at similar 
> bitrate. bitrate and quality can be manipulated with -aq, 10 to 30 are 
> sane values, the higher the number, the higher the bitrate/quality.
> 
> Only 2 channel is supported, and, in a psy sense, 44100/48000 is best 
> supported...
> 
> Open for review... :)

I'll double check my commit script now, if no objections till later today, 
I'll commit...

- ods15

Re: [PATCH] Vorbis Encoder

Hi

On Sat, Sep 30, 2006 at 11:33:49PM +0300, Oded Shimon wrote:
> $subj
> 
> Some cleanups still left - move the codebook stuff to a seperate file, 
> move the bitstream stuff to bitstream.h . Will move the codebook stuff 
> only after moving to ffmpeg repo...
> 
> This will be commited using ~90 commits for full history, with an 
> additional commit at the end for makefile/avocdec.h/allcodecs.c
> 
> Quality is comparable to official vorbis encoder (to my ears), at similar 
> bitrate. bitrate and quality can be manipulated with -aq, 10 to 30 are 
> sane values, the higher the number, the higher the bitrate/quality.
> 
> Only 2 channel is supported, and, in a psy sense, 44100/48000 is best 
> supported...
> 
> Open for review... :)
> 
> - ods15

[...]
> static int cb_lookup_vals(int lookup, int dimentions, int entries) {
>     if (lookup == 1) {
>         int tmp, i;
>         for (tmp = 0; ; tmp++) {
>                 int n = 1;
>                 for (i = 0; i < dimentions; i++) n *= tmp;

Re: [PATCH] Vorbis Encoder

On Sun, Oct 01, 2006 at 10:04:43AM +0200, Michael Niedermayer wrote:
> On Sat, Sep 30, 2006 at 11:33:49PM +0300, Oded Shimon wrote:
> [..]
> duplicate of nth_root() of vorbis.c
[..]
> this looks like a duplicate of vorbis_len2vlc() in vorbis.c
[..]
> that one looks like its part of vorbis_parse_setup_hdr_floors() from vorbis.c
> please remove all code duplications between vorbis.c & vorbis_enc.c
> ill review the rest after you removed all duplicate code

It's quite non trivial to remove all duplicate code between vorbis.c and 
vorbis_enc.c, because even though the algos are the same, the structs are 
completely different. The only way I can think of doing is by using common 
struct names with similar members, and have these helper functions in 
vorbis.h, and include vorbis.h AFTER the struct definitions in vorbis.c 
and vorbis_enc.c (same code, but different compilation for different 
files). This seems quite dirty, is this the way you intended?

> [...]
> >     int codebook_sizes[] = { 16, 8, 256, 64, 128, 32, 96, 32, 96, 17, 32, 78, 17, 32, 78, 100, 1641, 443, 105, 68,
81, 289, 81, 121, 169, 25, 169, 225, 289, };
> >     int * codebook_lens[] = { codebook0,  codebook1,  codebook2,  codebook3,  codebook4,  codebook5, 
codebook6,  codebook7,
> >                               codebook8,  codebook9,  codebook10, codebook11, codebook12, codebook13, codebook14, codebook15,
> >                               codebook16, codebook17, codebook18, codebook19, codebook20, codebook21, codebook22, codebook23,
> >                               codebook24, codebook25, codebook26, codebook27, codebook28, };
> > 
> 
> why is this int and not (u)int8_t ?

Re: [PATCH] Vorbis Encoder

Hi

On Sun, Oct 01, 2006 at 10:28:29AM +0200, Oded Shimon wrote:
> On Sun, Oct 01, 2006 at 10:04:43AM +0200, Michael Niedermayer wrote:
> > On Sat, Sep 30, 2006 at 11:33:49PM +0300, Oded Shimon wrote:
> > [..]
> > duplicate of nth_root() of vorbis.c
> [..]
> > this looks like a duplicate of vorbis_len2vlc() in vorbis.c
> [..]
> > that one looks like its part of vorbis_parse_setup_hdr_floors() from vorbis.c
> > please remove all code duplications between vorbis.c & vorbis_enc.c
> > ill review the rest after you removed all duplicate code
> 
> It's quite non trivial to remove all duplicate code between vorbis.c and 
> vorbis_enc.c, because even though the algos are the same, the structs are 

iam not saying its easy, but pretty much all other codecs in lavc also 
share common code

> completely different. The only way I can think of doing is by using common 
> struct names with similar members, and have these helper functions in 
> vorbis.h, and include vorbis.h AFTER the struct definitions in vorbis.c 
> and vorbis_enc.c (same code, but different compilation for different 
> files). This seems quite dirty, is this the way you intended?

no, the structures used by the common functions should be the same
not just have the same name and similarely named members

if this really isnt possible without having huge amounts of unused

[PATCH] move ready_floor1 to vorbis.h , change some sruct internals in vorbis.c

first step to make functions common between vorbis.c and vorbis_enc.c .

OK to commit?

BTW, the floor calculation in vorbis.c is inlined (render_line) right in 
the floor decode function, so it would be somewhat harder for me to split 
that out...

All of the common functions I am putting in vorbis.h . They are mostly 
small, biggest of them is this one, maybe the codebook len2vlc . So I 
don't think they deserve a seperate file...

- ods15

Index: vorbis.c
===================================================================
--- vorbis.c	(revision 6398)
+++ vorbis.c	(working copy)
 <at>  <at>  -88,10 +88,7  <at>  <at> 
             int_fast16_t subclass_books[16][8];
             uint_fast8_t multiplier;
             uint_fast16_t x_list_dim;
-            uint_fast16_t *x_list;
-            uint_fast16_t *x_list_order;
-            uint_fast16_t *low_neighbour;
-            uint_fast16_t *high_neighbour;
+            floor1_entry_t * list;
         } t1;
     } data;

Re: [PATCH] Vorbis Encoder

On Sun, Oct 01, 2006 at 11:01:39AM +0200, Michael Niedermayer wrote:
> Hi
> 
> On Sun, Oct 01, 2006 at 10:28:29AM +0200, Oded Shimon wrote:
> > On Sun, Oct 01, 2006 at 10:04:43AM +0200, Michael Niedermayer wrote:
> > > On Sat, Sep 30, 2006 at 11:33:49PM +0300, Oded Shimon wrote:
> > > [..]
> > > duplicate of nth_root() of vorbis.c
> > [..]
> > > this looks like a duplicate of vorbis_len2vlc() in vorbis.c
> > [..]
> > > that one looks like its part of vorbis_parse_setup_hdr_floors() from vorbis.c
> > > please remove all code duplications between vorbis.c & vorbis_enc.c
> > > ill review the rest after you removed all duplicate code
> > 
> > It's quite non trivial to remove all duplicate code between vorbis.c and 
> > vorbis_enc.c, because even though the algos are the same, the structs are 
> 
> iam not saying its easy, but pretty much all other codecs in lavc also 
> share common code
> 
> 
> > completely different. The only way I can think of doing is by using common 
> > struct names with similar members, and have these helper functions in 
> > vorbis.h, and include vorbis.h AFTER the struct definitions in vorbis.c 
> > and vorbis_enc.c (same code, but different compilation for different 
> > files). This seems quite dirty, is this the way you intended?
> 
> no, the structures used by the common functions should be the same
> not just have the same name and similarely named members