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

[Reimar Doeffinger]

Hello,
On Fri, Jun 15, 2007 at 05:53:51PM +0800, Zuxy Meng wrote:
[...]
> +    if (af->data->nch >= 2) {
> +      s->level[0][0] = min(1.f, 1.f - val);
> +      s->level[0][1] = max(0.f, val);
> +      s->level[1][0] = max(0.f, -val);
> +      s->level[1][1] = min(1.f, 1.f + val);

Btw. where does this "formula" come from? I think at least my hardware
balance control would only adjust [0][0] and [1][1].

> +    if (af->data->nch < 2)
> +      *(float*)arg = 0.f;
> +    else if (s->level[0][1])
> +      *(float*)arg = s->level[0][1];
> +    else if (s->level[1][0])
> +      *(float*)arg = -s->level[1][0];
> +    else
> +      *(float*)arg = 0.f;

Not very important, but I don't think these many else cases make much
sense, I'd suggest just doing always e.g.
> *(float*)arg = s->level[0][1] - s->level[1][0];
If you want keep the special af->data->nch < 2 case but not the others.
Though I'm not sure if some other AF_ return might make more sense in
that case (but then you have to make sure it doesn't cause the control
code to insert yet another af_pan ;-)

> +// Number of outputs from pan, arg is float*
> +#define AF_CONTROL_PAN_BALANCE	 	0x00002500 | AF_CONTROL_FILTER_SPECIFIC

The description is wrong, and I'd name it just AF_CONTROL_BALANCE, there
is no reason to keep it af_pan specific forever.

Greetings,
Reimar Doeffinger