[FFmpeg-cvslog] r20725 - in trunk/libavcodec: h263.c mpegvideo.h mpegvideo_enc.c

[Diego Biurrun]

On Thu, Dec 24, 2009 at 06:37:57PM +0100, Michael Niedermayer wrote:
> On Thu, Dec 24, 2009 at 05:31:37PM +0100, Diego Biurrun wrote:
> > On Thu, Dec 24, 2009 at 01:49:08PM +0100, Adrian Stutz wrote:
> > > >
> > > > Log:
> > > > Move ff_init_qscale_tab() from h263.c to mpegvideo, the function is not h263 specific.
> > > >
> > > > Modified:
> > > >    trunk/libavcodec/h263.c
> > > >    trunk/libavcodec/mpegvideo.h
> > > >    trunk/libavcodec/mpegvideo_enc.c
> > > 
> > > This broke compiling for me on OSX with the following error:
> > > > Undefined symbols:
> > > >   "_ff_init_qscale_tab", referenced from:
> > > >       _ff_clean_h263_qscales in h263.o
> > > > ld: symbol(s) not found
> > > > collect2: ld returned 1 exit status
> > > > make: *** [libavcodec.52.dylib] Error 1
> > > 
> > > These are my configure options:
> > > ./configure --enable-gpl --enable-shared --disable-static
> > > --disable-ffmpeg --disable-ffserver --enable-pthreads
> > > --disable-devices --disable-encoders --disable-muxers
> > > --disable-network --enable-postproc --enable-encoder=png
> > > 
> > > Reverting the commit made the error go away.
> > 
> > The problem is that h263.c now depends on mpegvideo_enc.c.  This is not good.
> > The code should be moved somewhere else that is not encoder-specific.
> 
> The code is encoder specific, so it belongs in a file that is encoder
> specific

Unfortunately many decoders depend on h263.c and are now pulling in
encoder dependencies...

> > Michael, why did you move ff_init_qscale_tab at all, it is only used in
> > h263.c ...
> 
> see the next commit
> 
> and i suggest we revert the whole mpegvideo split
> As it is currently, functions are placed randomly and iam already failing to
> find them quite often and end up with grep mpegvideo*
> h263 specific code belongs in h263*, encoder specific code belongs in *enc if
> such files exist.
> h263.c does and MUST depend on mpegvideo_enc unless h263enc.c exists
> that is h263.c contains decoder & encoder specific code currently

I suggest we look into splitting h263.c.  At 6368 lines it is the second
largest file in libavcodec.

I'll look into doing that if you can give me directions which files can
or must depend upon each other from an architectural perspective.

Diego