[Ffmpeg-devel] [PATCH] Vorbis Encoder

[Oded Shimon]

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

I checked better now, most functions can be shared easily, but only with 
some changes to both vorbis.c and vorbis_enc.c . Shared functions and 
structs will be vorbis.h, everything else in their respective C file ..

- ods15