[MPlayer-dev-eng] [PATCH] support for external VIDIX
Diego Biurrun
diego at biurrun.de
Sun Mar 26 23:56:11 CEST 2006
On Sun, Mar 26, 2006 at 11:32:45PM +0200, Nico Sabbi wrote:
> Diego Biurrun wrote:
>
> >>+else
> >>+ _external_vidix=no
> >
> >The else clause is superfluous, you've already set _external_vidix=no
> >above.
> >
> no, it handles the case --disable-internal-vidix --disable-external-vidix,
> otherwise disabling vidix entirely would not be possible
Huh? Both variables are 'no' to begin with then, why do you need to set
one to 'no' again?
> >I don't particularly like the _has_vidix variable. Instead you could
> >rename _vidix to _vidix_internal and use _vidix for the common case.
>
> updated
Ideally you would do a commit with the rename first, that will make
reviewing your changes easier. Feel free to do it right away.
> --- configure 21 Mar 2006 05:36:09 -0000 1.1149
> +++ configure 26 Mar 2006 21:22:04 -0000
> @@ -7133,9 +7137,35 @@
> linux && _def_linux='#define TARGET_LINUX 1'
>
> # TODO cleanup the VIDIX stuff here
> -echocheck "VIDIX"
> +echocheck "VIDIX (internal)"
> _def_vidix='#define CONFIG_VIDIX 1'
> -test "$_vidix" = no && _def_vidix='#undef CONFIG_VIDIX'
> +_vidix=yes
> +if test "$_vidix_internal" = no ; then
> + _vidix=no
> + _def_vidix='#undef CONFIG_VIDIX'
> +fi
Hmm, why do you need to set this here at all? Check out the FAAD test,
it should be possible to make two tests, set common variables and derive
the CONFIG_ etc settings from there.
> +echocheck "VIDIX (external)"
> +if test "$_vidix_internal" = no && test "$_vidix_external" != no; then
This test is wrong, the check should only be run if _vidix_external is
set to 'auto'.
> + _vidix_external=no
> + _def_vidix="#undef CONFIG_VIDIX"
> + _vidix=no
Same as above.
Diego
More information about the MPlayer-dev-eng
mailing list