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

[Alban Bedel]

On Wed, 30 May 2007 22:53:19 +0800
"Zuxy Meng" <zuxy.meng at gmail.com> 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