[MPlayer-dev-eng] [PATCH] Mac OS X Finder support

Reimar Döffinger Reimar.Doeffinger at stud.uni-karlsruhe.de
Sat Nov 6 18:23:13 CET 2004


Hi,

> >I will look into that, Maybe the patch author can come up with 
> >something.
> >Anyway this patch can make thing easier for me with vo_quartz.
> 
> Here I am, with a new and revised patch.

Looks quite nice to me, but I still have a few comments of course ;-)

> if [ "$_macosx" = yes ] && [ "$_macosx_finder_support" != no ]; then

Two things here:
Use test instead of [], that's how it's done everywhere else, lets
better not risk portability issues.
If --enable or --disable are given always behave like
that even if it seems sense less (there are enough configure parts that
violate that rule).

I would suggest something like:
if test "$_macosx_finder_support" = auto ; then
  _macosx_finder_support=$macosx
fi
if test "$_macosx_finder_support" = yes; then
...


The fprintfs in macosx_main seem to be for debugging. They should be
commented-out, removed or whatever.

The patch to version.h needs explanation. But as it is unrelated 
it must be in an independent patch anyway...

That little define in mplayer.c seems acceptable to me, I just didn't
want another 100 lines in that 4000+ lines beast.
In addition having it in a separate file will allow Nicolas to fiddle
with it without risking flames so much ;-).

Greetings,
Reimar Döffinger




More information about the MPlayer-dev-eng mailing list