[MPlayer-dev-eng] BSD BT848 Radio interface (patch)

[Diego Biurrun]

On Thu, Nov 16, 2006 at 01:42:53AM +0600, Vladimir Voroshilov wrote:
> 2006/11/16, voroshil at gmail.com <voroshil at gmail.com>:
> >> > 2. Does freq_min and freq_max parameters need for bt848?
> >>
> >> See my question above. I would like it that way.
> >Ok. I'll add freq_min and freq_max.
> Done. Patch applyed.
> Please, anybody test it.

...

> --- stream/stream_radio.c	(revision 20942)
> +++ stream/stream_radio.c	(working copy)
> @@ -33,6 +33,13 @@
> +
> +#ifdef RADIO_BSDBT848_HDR
> +#include <sys/param.h>
> +#include RADIO_BSDBT848_HDR
> +
> +#else // !BSDBT848
> +
> +#endif //!BSDBT848

The ifdef, the else and the endif don't have the same text, please fix
this.

> --- configure	(revision 20942)
> +++ configure	(working copy)
> @@ -1648,6 +1649,7 @@
>  _radio_v4l2=auto
> +_radio_bsdbt848=auto 

You have trailing whitespace here.  This will not work.

> @@ -1906,6 +1908,7 @@
>    --disable-radio-v4l)	_radio_v4l=no	;;
>    --enable-radio-v4l2)	_radio_v4l2=yes	;;
>    --disable-radio-v4l2)	_radio_v4l2=no	;;
> +  --disable-radio-bsdbt848)	_radio_bsdbt848=no	;;

The enable option is missing.

> @@ -6749,10 +6752,32 @@
>  
> +if test "$_radio_bsdbt848" = auto -a "$_radio" = yes && bsd; then

Use && instead of -a.  Also, I'd put the bsd test first as it's the most
likely to fail.

> + echocheck "*BSD BrookTree 848 Radio interface header"

One-space indentation sucks.  Feel free not to indent the check after
the if that checks whether to run it or not.

> + _radio_bsdbt848_hdr=

Why this empty variable declaration?

> + for file in "dev/ic/bt8xx.h" "machine/ioctl_bt848.h" "dev/bktr/ioctl_bt848.h" "dev/video/bktr/ioctl_bt848.h" ; do

Please break this long line.

> +    cat > $TMPC <<EOF
> +#include <sys/types.h>
> +#include <$file>
> +int main(void) { return 0; }
> +EOF
> +    cc_check && _radio_bsdbt848_hdr=$file
> + done
> + echores "$_radio_bsdbt848_hdr"
> +else
> +  _radio_bsdbt848_hdr=

Why this empty variable declaration?

> +fi #if bsd && radio && radio_bsdbt848

Nit: The check above is in a different order.

> +if test -n "$_radio_bsdbt848_hdr"  ; then
> +  _def_radio_bsdbt848="#define RADIO_BSDBT848_HDR <$_radio_bsdbt848_hdr>"
> +else
> +  _def_radio_bsdbt848='#undef RADIO_BSDBT848_HDR '
>  fi
>  
> +if test "$_radio_v4l" = no && test "$_radio_v4l2" = no && test -z "$_radio_bsdbt848_hdr" && test "$_radio" = yes ; then

Please break this long line.

> --- DOCS/man/en/mplayer.1	(revision 20942)
> +++ DOCS/man/en/mplayer.1	(working copy)
> @@ -1614,12 +1614,16 @@
> +.IPs freq_min=<value> (*BSD BT848 only)
> +minimal allowed frequency (default: 87.50)

minimum

> +.IPs freq_max=<value> (*BSD BT848 only)
> +maximal allowed frequency (default: 108.00)

maximum

I could also complain about some trailing whitespace in the rest of your
patch, but I won't ;)

Diego