[MPlayer-cvslog] r21265 - in trunk: configure loader/config.h

Uoti Urpala uoti.urpala at pp1.inet.fi
Sun Nov 26 17:33:11 CET 2006


On Sun, 2006-11-26 at 16:56 +0100, Diego Biurrun wrote:
> On Sun, Nov 26, 2006 at 02:17:50PM +0100, uau wrote:
> > Log:
> > Revert r21251 (moving loader/config.h contents to main config.h).
> 
> Please don't just revert commits from other devs (in code not maintained
> by you) without discussion.  Yes, we got a report that this broke
> compilation, but only one one system, not in the general case.  If I was

The compilation problems were not why I reverted it (and I didn't even
mention those in the commit message). The people who got random crashes
most likely didn't realize what was causing them; I had to search for a
while myself to figure out why h264 decoding suddenly caused sig11.

> unavailable or not reacting (quickly) to bug reports, it would be a
> different thing.  The issue is not so urgent that it cannot wait a few
> hours.

IMO it was bad enough not to leave for a few hours, and it's not like
reverting the commit would make implementing whatever alternative
solution harder. What would you gain by keeping the broken version
there?

> > There are #ifdef TRACE tests in libav* and defining TRACE in toplevel
> > config.h breaks things.
> 
> I want to see loader/config.h gone.  IMO there should never be multiple
> header files with the same name, period.
> 
> I see two possible solutions apart from renaming TRACE in loader/ or
> libav*/:
> 
> - Move those definitions to the CFLAGS of loader/Makefile.
> - Move those definitions to config.h and surround them by '#ifdef LOADER'
>   or something similar that gets defined only during the compilation of
>   loader/.

I think renaming the header from config.h to something else is a better
alternative than those 2.




More information about the MPlayer-cvslog mailing list