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

[Christoph Egger]

> 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"

Thank you for this guidance. I'm only familiar with
autoconf generated configure scripts...

I think you will like this patch better.


> > > > +#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?

umm... yes, sorry.



-- 
Greetings,

Christoph

+++ Neu: Echte DSL-Flatrates von GMX - Surfen ohne Limits +++
Always online ab 4,99 Euro/Monat: http://www.gmx.net/de/go/dsl
-------------- next part --------------
A non-text attachment was scrubbed...
Name: configure.diff
Type: application/octet-stream
Size: 1929 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/mplayer-dev-eng/attachments/20050511/d6df84b4/attachment.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: vo_ggi.diff
Type: application/octet-stream
Size: 5215 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/mplayer-dev-eng/attachments/20050511/d6df84b4/attachment-0001.obj>