[MPlayer-dev-eng] [PATCH] simplify tremor local diff
Diego Biurrun
diego at biurrun.de
Mon Nov 20 13:59:52 CET 2006
On Mon, Nov 20, 2006 at 01:33:43PM +0100, Reimar Doeffinger wrote:
> Hello,
> On Mon, Nov 20, 2006 at 12:06:35PM +0100, Diego Biurrun wrote:
> > On Sun, Nov 19, 2006 at 11:22:10PM +0100, Reimar Döffinger wrote:
> > > On Sun, Nov 19, 2006 at 11:05:03PM +0100, Diego Biurrun wrote:
> > > > The attached patch reverts most of our local modifications to tremor and
> > > > adds the missing header file from upstream. Some testing on platforms
> > > > other than Linux is welcome, especially Windows.
> > > >
> > > > I intend to apply this soon if there are no objections.
> > >
> > > I am against this, the upstream version is extremely complicated and
> > > bloated
> >
> > Nah, you're exaggerating now. It's a bunch of nested #ifdefs, but this
> > is to be expected in a header that is designed to smooth over
> > os-dependent differences.
>
> It is 71 no only completely useless but probable to break something
> lines of code. And this change was made because it broke something.
This case may (or may not) be different, but most such problems with
included libs were solved by quick and ugly hacks, see libdvd* for
example.
> > > I don't find it justified to add this back in just to reduce the diff.
> >
> > I disagree. I think the policy should be not to patch external
> > libraries for purely cosmetic reasons. On the contrary, an attempt
> > should be made to minimize local changes.
>
> Yes, of course, but there should be limit, and IMHO 50 lines of code for
> something that can basically be done with one include seems justified,
> esp. since the code was broken somewhen. And I did not check that the
> upstream one is correct now.
I'll investigate if this breaks on Windows.
Diego
More information about the MPlayer-dev-eng
mailing list