[MPlayer-dev-eng] [PATCH] support for external VIDIX

[Diego Biurrun]

On Sun, Mar 26, 2006 at 03:44:22PM +0200, Nico Sabbi wrote:
> Nico Sabbi wrote:
> 
> >$Subject.
> >
> >As usual the internal vidix is given preference over (and excludes) 
> >the external one.
> >For consistency with other configure options I also renamed 
> >--disable-vidix and
> >--enable-vidix to --disable-internal-vidix and 
> >--enable-internal-vidix, respectively.
> >Notice that I don't have a way to test the build on windows, but it 
> >should be safe.

Don't worry about the Windows build, we'll fix afterwards if problems
creep up.

> >If no one objects I'll commit this patch tomorrow.

Umm, I have some remarks :)

> --- configure	21 Mar 2006 05:36:09 -0000	1.1149
> +++ configure	26 Mar 2006 13:40:29 -0000
> @@ -1562,6 +1563,7 @@
>  _winsock2=auto
>  _smbsupport=auto
>  _vidix=auto
> +_external_vidix=auto

Please call the variable _vidix_external for consistency.

> @@ -1798,8 +1800,12 @@
>    --disable-winsock2)	_winsock2=no	;;
>    --enable-smb)		_smbsupport=yes	;;
>    --disable-smb)	_smbsupport=no	;;
> -  --enable-vidix)	_vidix=yes	;;
> -  --disable-vidix)	_vidix=no	;;
> +  --enable-vidix)       _vidix=yes      ;;
> +  --disable-vidix)      _vidix=no; _external_vidix=no ;;
> +  --enable-internal-vidix)	_vidix=yes	;;
> +  --disable-internal-vidix)	_vidix=no	;;
> +  --enable-external-vidix)	_external_vidix=yes	;;
> +  --disable-external-vidix)	_external_vidix=no	;;

Why do you keep --disable-vidix and --enable-vidix?  I say remove them,
there is enough cruft as it is...

> @@ -7133,29 +7139,54 @@
>  
>  # TODO cleanup the VIDIX stuff here
> -echocheck "VIDIX"
> +echocheck "VIDIX(internal)"

Nit: "VIDIX (internal)"

>  _def_vidix='#define CONFIG_VIDIX 1'
> -test "$_vidix" = no && _def_vidix='#undef CONFIG_VIDIX'
> -if test "$_vidix" = yes; then
> +_has_vidix=yes
> +if test "$_vidix" = no ; then
> +  _has_vidix=no
> +  _def_vidix='#undef CONFIG_VIDIX'
> +fi
> +echores "$_vidix"
> +
> +echocheck "VIDIX(external)"

Nit: "VIDIX (external)"

> +if test "$_vidix" = no && test "$_external_vidix" != no; then
> + _external_vidix=no
> + _def_vidix="#undef CONFIG_VIDIX"
> + _has_vidix=no
> + cat > $TMPC <<EOF
> +#include <vidix/vidix.h>
> +int main(void) { return 0; }
> +EOF
> +  cc_check -lvidix && _external_vidix=yes
> +else
> +  _external_vidix=no

The else clause is superfluous, you've already set _external_vidix=no
above.

I don't particularly like the _has_vidix variable.  Instead you could
rename _vidix to _vidix_internal and use _vidix for the common case.

Diego