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

[Zuxy Meng]

Hi,

2007/6/17, Zuxy Meng <zuxy.meng at gmail.com>:
> Hi,
>
> 2007/6/17, Reimar Doeffinger <Reimar.Doeffinger at stud.uni-karlsruhe.de>:
> > 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.
>
> Well, I was just thinking that it makes no sense to tune the balance
> without actual audio output.
>
> > [...]
> > > > > +  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.
>
> It needs to be done only once so may be improper to move it under
> AF_CONTROL_PAN_BALANCE|AF_CONTROL_SET.
>
> > > > 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.

Applied. It works as expected and causes no regression AFAIK. Further
improvements and style issues will be dealt with later.
-- 
Zuxy
Beauty is truth,
While truth is beauty.
PGP KeyID: E8555ED6