[MPlayer-dev-eng] [PATCH] Fix dependencies between muxer and demuxer

[Alban Bedel]

On Tue, 8 Apr 2008 18:25:19 +0200
Diego Biurrun <diego at biurrun.de> wrote:

> On Tue, Apr 08, 2008 at 06:14:10PM +0200, Alban Bedel wrote:
> > On Tue, 8 Apr 2008 01:25:33 +0200
> > Diego Biurrun <diego at biurrun.de> wrote:
> > 
> > > On Mon, Apr 07, 2008 at 02:49:56PM +0200, Alban Bedel wrote:
> > > > 
> > > > following my previous post, now are fixes for vivodump. They
> > > > allow to use libmpmux without libmpdemux.
> > > > 
> > > > --- libmpdemux/Makefile	(revision 26343)
> > > > +++ libmpdemux/Makefile	(working copy)
> > > > @@ -36,6 +36,7 @@
> > > >                extension.c \
> > > >                mf.c \
> > > > +              aac_hdr.c \
> > > >                mp3_hdr.c \
> > > 
> > > alphabetical order
> > 
> > Fixed, perhaps you should add comments in the Makefiles. I mean
> > when one is busy with other things than Makefile layout, it's not
> > necessarily obvious.
> 
> Add comments where?  DOCS/tech/svn-howto.txt?  Basically everything is
> in alphabetical order in the Makefiles.  If it is not, it is a bug.

In the Makefile obviously. Sure it's obvious to you the Makefile
maintainer. But for a random dev with his mind busied by other things
(like getting his stuff done) it might easily be overlooked.
It's a suggestion which I thought might lead to you having to reply
"alphabetical order" less often. Now you do what you want with it.

> > > > @@ -68,5 +69,6 @@
> > > >  
> > > >  demux_lavf.o: CFLAGS += -I../libavcodec
> > > > +mp_taglists.o: CFLAGS += -I../libavcodec
> > > 
> > > Merge these two lines.
> > 
> > Done. I used the one-file-per-line trick, I hope it's alright.
> 
> Umm, I find your version very ugly, please use
> 
>   demux_lavf.o mp_taglists.o: CFLAGS += -I../libavcodec
>  
> instead.

As you like.
 
> > > This file could be created with 'svn cp' from demux_lavf.c.
> > 
> > I intend to do that with all derived files, but as you can see that
> > make for ugly hard to review diff. Sorry, I forgot to mention it in
> > my original mails.
> 
> OK.  You are right that it can be easier to review the other way
> around.
> 
> > --- libmpdemux/mp_taglists.c	(revision 26348)
> > +++ libmpdemux/mp_taglists.c	(working copy)
> 
> Is the mp_ prefix for the filename necessary?

It reflect the name of the things in it, which IMHO make sense. But
honestly I couldn't care less, so shout in the next hours or svn mv
later.

> > @@ -1,81 +1,28 @@
> >  /*
> > -    Copyright (C) 2004 Michael Niedermayer <michaelni at gmx.at>
> > + * Copyright (C) 2006 Reimar D??ffinger
> 
> IIRC this file just contains the tables.  In this case you can leave
> out the copyright holder, tables are not copyrightable.

OK.

> Other than that, patch is OK from my side, but you may wish to wait
> for comments from others.

Like the other patch I plan to commit it later tonight.

	Albeu