[MPlayer-dev-eng] TOOLS/binary_codecs.sh update

[Diego Biurrun]

On Wed, Oct 13, 2010 at 05:53:58PM +0200, Reinhard Tartler wrote:
> 
> The original author of TOOLS/binary_codecs.sh proposed three changes,
> two of which I found clear and easy to understand and committed it. The
> last one made me think. I'm talking about this change:
> 
> --- binary_codecs.sh	(Revision 32485)
> +++ binary_codecs.sh	(Arbeitskopie)
> @@ -20,6 +20,36 @@
>  
> +mywget ()
> +{
> +    # note the the URL must be the first option
> +    t=`tempfile`

horrible variable name

$() is preferable to ``.

> +    #check that we are not redirected
> +    if LANG=C LC_ALL=C LC_MESSAGES=C wget --spider -S -o $t "$1" ; then

These variables should be set globally I think.

> +       if egrep -qx ' *HTTP.* 302 Found' $t ; then
> +         echo Error, there is an unexpected redirection, for the
> +         echo URL "$1" 
> +         echo that is redirected to
> +	 grep Location $t | head -n 1 
> +         echo See more info in the log in $t
> +         echo 
> +         return 1
> +       else
> +         rm $t
> +       fi
> +    else
> +       echo Errors while checking URL "$1" 
> +       echo See the log in  $t
> +       return 1
> +    fi

Indentation looks pretty random.

> +    url="$1"
> +    shift
> +    if wget "$@" "$url" ; then 
> +     return 0
> +    else
> +     return 1
> +    fi

random indeed

> My thoughts:
> 
>  - I haven't tested it myself yet, but if it works (I suppose it does),
>    I'm inclined to include it
>  - is that mywget approach somewhat sane?

I haven't given it enough thought to form an opinion yet.

>  - should we host the files at
>    http://people.debian.org/~mennucc1/mplayer/ somewhere on mplayerhq.hu
>    instead?

Yes.

Diego