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

[Diego Biurrun]

On Tue, May 10, 2005 at 07:38:29PM +0200, Christoph Egger wrote:
> > On Sat, May 07, 2005 at 06:38:00PM +0200, Christoph Egger wrote:
> > > 
> > > +  --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.
> 
> done.

OK

> > > +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.
> 
> done.

Let me elaborate..

> +if test "$_ggiwmh" = auto -o "$_ggiwmh" = yes ; then
> +  echocheck "GGI extension: libggiwmh"

Swap these two lines, the text should always get printed.

> +  _ggiwmh=no
> +  _def_ggiwmh='#undef HAVE_GGIWMH'
> +  if test "$_ggi" = yes ; then
> +    cat > $TMPC << EOF

The test for $_ggi should go along with the test for the extension,
something like this:

echocheck "GGI extension: libggiwmh"
if test "$_ggi" = yes && test "$_ggiwmh" = auto ; then
  _ggiwmh=no
  cat > $TMPC << EOF
#include <ggi/ggi.h>
#include <ggi/wmh.h>
int main(void) { return 0; }
EOF
  cc_check -lggi -lggiwmh && _ggiwmh=yes

  if test "$_ggiwmh" = yes ; then
    _def_ggiwmh='#define HAVE_GGIWMH 1'
    _ld_ggi="$_ld_ggi -lggiwmh"
  else
    _def_ggiwmh='#undef HAVE_GGIWMH'
  fi
fi
echores "$_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.
> 
> fixed.

Not in the version you sent.  Maybe you mixed up patches?

Diego