[MPlayer-dev-eng] [PATCH] GGI driver update

[Diego Biurrun]

On Sat, May 07, 2005 at 06:38:00PM +0200, Christoph Egger wrote:
> 
> I've tried to implement the --enable and --disable options.
> New patches attached.

Here comes the review..

> +  --enable-ggiwmh        build with GGI libggiwmh extension (implies --enable-ggi) [autodetect]

This line in the output is far too long, please don't exceed 80
characters.  Just remove the (...), we never mention implicit
dependencies in the --help output anyway.

> +echocheck "GGI extension: libggiwmh"
> +_ggiwmh=no
> +_def_ggiwmh='#undef HAVE_GGIWMH'
> +if test "$_ggi" = yes ; then

You're not implementing correct semantics there.  You are
unconditionally doing autodetection and overriding the
--(dis|en)able-ggiwmh options you introduced above.  Check for the state
of _ggiwmh.


> +#ifdef HAVE_GGIWMH
> +    if (ggiWmhInit() < 0) {
> +	mp_msg(MSGT_VO, MSGL_FATAL, "[ggi] unable to initialize libggiwmh\n");
> +	return(-1);

You're messing up the indentation here by mixing spaces and tabs, please
don't.

Diego