[MPlayer-dev-eng] [PATCH] Fix dependencies between muxer and demuxer
Alban Bedel
albeu at free.fr
Tue Apr 8 19:53:09 CEST 2008
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
More information about the MPlayer-dev-eng
mailing list