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

[Reimar Doeffinger]

Hello,
On Sun, Jun 17, 2007 at 08:34:22PM +0800, Zuxy Meng wrote:
> 2007/6/17, Reimar Doeffinger <Reimar.Doeffinger at stud.uni-karlsruhe.de>:
> > [...]
> > > +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?
> 
> Does mixer->afilter != 0 imply mixer->audio_out != 0? If so then this
> would be redundant. I simply did the same check as mixer_getvolume.

Well, does your code failt if mixer->audio_out == 0? The volume
functions only check because of mixer->audio_out->control AFAIK.

[...]
> > > +  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.
> 
> You mean to change to default setting of af_pan?

Or move it as it is into the set balance function. That might be very
slightly better, but I have no real preference.

> > 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).
> 
> Maybe later:-)

Just suggesting it because there are some difference I wanted to avoid
asking about seperately like the extra af_init.

Greetings,
Reimar Doeffinger