[MPlayer-dev-eng] [PATCH] amr support

[Reimar Döffinger]

Hi,
> +echocheck "amr narrowband"
> +if test "$_amr_nb" = auto ; then
> +  if test -d libavcodec/amr_float && test -f libavcodec/amr_float/sp_dec.c ; then

What is that test -d good for? If the test -f succeeds it obviously is
there... Or is that to avoid that it's a link? I wouldn't care about
that case.
See below for my suggestion for it.

> +    _amr_nb=yes
> +  else
> +    _amr_nb=no
> +  fi
> +fi
> +if test "$_amr_nb" = yes ; then
> +  if test "$_libavcodec" = yes ; then
> +    _def_amr_nb='#define AMR_NB 1'
> +    _def_amr_nb_fixed='#undef AMR_NB_FIXED'
> +    echores "$_amr_nb"
> +  else
> +    _def_amr_nb='#undef AMR_NB'
> +    _def_amr_nb_fixed='#undef AMR_NB_FIXED'
> +    amr_nb = no
> +    echores "libavcodec (static) is required by amr_nb, sorry"

Please don't do that. If the user says --enable, do it and don't assume
that a script could know better (also makes your patch simpler ;-) ).
Instead put that test above in the auto section.

> +  fi
> +else
> +  _def_amr_nb='#undef AMR_NB'
> +  _def_amr_nb_fixed='#undef AMR_NB_FIXED'
> +  echores "$_amr_nb"
> +fi
> +
> +echocheck "amr narrowband, fixed point"

you can't include both fixed point and floating point, can you? So test
for fixed point first and change its autodetection code to
  if test "$_libavcodec" = yes && test -f libavcodec/amr_float/sp_dec.c
&& test "$_amr_nb_fixed" = no ; then
And as I said, always respect the users wishes, however stupid/broken you might
think they are.

Greetings,
Reimar Döffinger