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

[Reimar Doeffinger]

Hello,
On Sun, Jun 17, 2007 at 01:00:21PM +0800, Zuxy Meng wrote:
> +/// Balance (RW)
> +static int mp_property_balance(m_option_t * prop, int action, void *arg,
> +			      MPContext * mpctx)
> +{
> +    float bal;
> +
> +    if (!mpctx->sh_audio || mpctx->sh_audio->channels < 2)
> +	return M_PROPERTY_UNAVAILABLE;
> +
> +    switch (action) {
> +    case M_PROPERTY_GET:
> +	if (!arg)
> +	    return M_PROPERTY_ERROR;
> +	mixer_getbalance(&mpctx->mixer, arg);
> +	return M_PROPERTY_OK;
> +    case M_PROPERTY_PRINT: {
> +	    char** str = arg;
> +	    if (!arg)
> +		return M_PROPERTY_ERROR;

IMO do not add an extra indentation level just because of the { after
the case.

[...]
> +void mixer_getbalance(mixer_t *mixer, float *val)
> +{
> +  *val = 0.f;
> +  if(!mixer->audio_out || !mixer->afilter)
> +    return;

What's the point in checking for audio_out?

> +void mixer_setbalance(mixer_t *mixer, float val)
> +{
> +  float level[AF_NCH];
> +  int i, nout;
> +  af_control_ext_t arg_ext = { .arg = level };
> +  af_instance_t* af_pan_balance;
> +
> +  if(!mixer->audio_out || !mixer->afilter)
> +    return;
> +
> +  if (af_control_any_rev(mixer->afilter,
> +	AF_CONTROL_PAN_BALANCE | AF_CONTROL_SET, &val))
> +    return;
> +
> +  if (!(af_pan_balance = af_add(mixer->afilter, "pan"))) {
> +    mp_msg(MSGT_GLOBAL, MSGL_ERR, MSGTR_NoBalance);
> +    return;
> +  }
> +
> +  af_init(mixer->afilter);
> +  /* make all other channels pass thru since by default pan blocks all */
> +  memset(level, 0, sizeof(level));
> +  for (i = 2; i < AF_NCH; i++) {
> +    arg_ext.ch = i;
> +    level[i] = 1.f;
> +    af_pan_balance->control(af_pan_balance,
> +	AF_CONTROL_PAN_LEVEL | AF_CONTROL_SET, &arg_ext);
> +    level[i] = 0.f;
> +  }

I think this should be done in af_pan as well. I'd strongly suggest
using the same code as for volume control in mixer.c, possibly even
factoring it out into a new function (e.g. af_control_rev_autoadd).
Printing a message when a filter is inserted (as for volume control)
might be nice, too, since it sometimes has weird side effects.

Greetings,
Reimar Doeffinger