[Ffmpeg-devel] dependency tracking improvements

[Michael Niedermayer]

Hi

On Wed, Jan 24, 2007 at 11:25:53PM +0100, Aurelien Jacobs wrote:
> On Tue, 23 Jan 2007 00:50:19 +0100
> Aurelien Jacobs <aurel at gnuage.org> wrote:
> 
> > On Mon, 22 Jan 2007 09:08:00 +0000
> > M?ns Rullg?rd <mru at inprovide.com> wrote:
> > 
> > > Diego Biurrun <diego at biurrun.de> writes:
> > > 
> > > > On Mon, Jan 22, 2007 at 01:21:21AM +0100, Aurelien Jacobs wrote:
> > > >> On Mon, 22 Jan 2007 01:06:18 +0100
> > > >> Diego Biurrun <diego at biurrun.de> wrote:
> > > >> 
> > > >> > On Mon, Jan 22, 2007 at 12:52:15AM +0100, Aurelien Jacobs wrote:
> > > >> > > 
> > > >> > > Here is a patch which further improve new Mans dependency tracking system.
> > > >> > > It moves dependency information from configure to allcodecs.c/allformats.c.
> > > >> > > Advantages are that it simplifies configure and that when you add a new
> > > >> > > codec, you don't need to mess with configure. Just add a new line in
> > > >> > > allcodecs.c and you're done.
> > > >> > > Opinions ?
> > > >> > > Can I commit this ?
> > > >> > 
> > > >> > Putting that info in a comment sounds very fragile to me.  This way
> > > >> > compilation might fail if somebody changes a comment ...
> > > >> 
> > > >> Anyway, we already heavily rely on the structure of this file to parse
> > > >> it with sed. It's already very easy to break.
> > > >> But if you think using comments is bad, maybe we could do something
> > > >> like this:
> > > >> 
> > > >> -  REGISTER_DECODER(AAC, aac);
> > > >> +  REGISTER_DECODER(AAC, aac, libfaad);
> > > >> 
> > > >> Do you prefer it ?
> > > >> IMO, it's as much fragile, and uglier than a comment.
> > > >
> > > > That's better and less fragile.
> > > 
> > > But it doesn't allow the current dependencies to be correctly expressed.
> > 
> > It does !
> > Ok I admit that I didn't explained all the details of my idea.
> > To express deps_any, just use:
> > 
> > -    REGISTER_ENCDEC (AMR_NB, amr_nb);
> > +    REGISTER_ENCDEC (AMR_NB, amr_nb, , amr_nb amr_nb_fixed);
> > 
> > Let's see attached patch. Does it better suits your mind ?
> 
> Any thought about this patch ?

yes i dont like it

if it where something like

REGISTER_ENCDEC (AMR_NB, amr_nb);  //depends: amr_nb OR amr_nb_fixed

or

REGISTER_ENCDEC (AMR_NB, amr_nb);  //depends: amr_nb || amr_nb_fixed

or

REGISTER_ENCDEC (AMR_NB, amr_nb);  //requires: amr_nb || amr_nb_fixed

the reason for my dislike of 
REGISTER_ENCDEC (AMR_NB, amr_nb, , amr_nb amr_nb_fixed);

is that a normal developer will not be able to make sense of it, she would
have to look at REGISTER_ENCDEC() and then find in a comment there that the
parameter passed to it actually has nothing to do with registering a codec
but rather is somehow extracted by configure for checking the availability
of dependancies IMHO code should be understandable without having to read
too much other distant stuff

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Democracy is the form of government in which you can choose your dictator
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20070125/ba10271e/attachment.pgp>