Re: [MPlayer-dev-eng] [PATCH] Audio balance feature

On Wed, 30 May 2007 22:53:19 +0800
"Zuxy Meng" <[EMAIL PROTECTED]> wrote:

> +/// Balance (RW)
> +static int mp_property_balance(m_option_t * prop, int action, void *arg,
> +                           MPContext * mpctx)
> +{
> +    float bal;
> +
> +    if (!arg)
> +     return M_PROPERTY_ERROR;

This is bad STEP_UP and STEP_DOWN can get called without argument.

> +    if (!mpctx->sh_audio || mpctx->sh_audio->channels < 2)
> +     return M_PROPERTY_UNAVAILABLE;
> +
> +    switch (action) {
> +    case M_PROPERTY_GET:
> +     mixer_getbalance(&mpctx->mixer, arg);
> +     return M_PROPERTY_OK;
> +    case M_PROPERTY_PRINT:{
> +         mixer_getbalance(&mpctx->mixer, &bal);
> +         return m_property_float_range(prop, action, arg, &bal);
> +     }

Perhaps something like "X% left", "center" and "X% right" would be nicer.
Otherwise you should better call m_property_float_ro(), that's where
PRINT is implemented.

> +    case M_PROPERTY_STEP_UP:
> +        *(float*)arg = -*(float*)arg;

You shouldn't modifiy the caller arguments.

> +     // falling thru
> +    case M_PROPERTY_STEP_DOWN:
> +     mixer_getbalance(&mpctx->mixer, &bal);
> +     bal -= *(float*)arg;

STEP_UP and STEP_DOWN should use some default step size if no argument
is provided.

        Albeu

_______________________________________________
MPlayer-dev-eng mailing list
[EMAIL PROTECTED]
http://lists.mplayerhq.hu/mailman/listinfo/mplayer-dev-eng