[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