[MPlayer-dev-eng] [PATCH] af_pan.c: Avoid zero output channels when reinit af pan

[Reimar Döffinger]

Hello,
On Thu, Jun 07, 2007 at 08:42:36PM +0800, Zuxy Meng wrote:
> 2007/6/7, Reimar Döffinger <Reimar.Doeffinger at stud.uni-karlsruhe.de>:
> > Hello,
> > On Thu, Jun 07, 2007 at 07:00:51PM +0800, Zuxy Meng wrote:
> > > 2007/6/7, Reimar Döffinger <Reimar.Doeffinger at stud.uni-karlsruhe.de>:
> > [...]
> > > > I don't think it's really the right solution, but a proper one might not
> > > > be worth the effort. Though you might consider if initializing the nch
> > > > to 1 during filter creation isn't the better (and simpler) solution.
> > >
> > > I don't think it simpler; they're both conditional assignments. And at
> > > least it makes life harder for the audio balance feature :-)
> >
> > Huh? Why would you have to make setting
> > af->data->nch = 1;
> > conditional in af_open? And why would it make audio balance harder?
> 
> As I've explained before, AF_REINIT is actually called *after*
> AF_COMMAND_LINE. So unconditionally setting the output number to 1
> will render the command line useless.

So? I was talking about adding it to af_open...

> As for audio balance, all it does is to blend the two front channels.
> It doesn't touch other channels (if there are). It'll save me two or
> three lines if pan doesn't change the output number by default.

Which is not quite what your patch does. It always uses the number of
channels supplied on first reinit, which in the future may not be the
final number of input channels (and maybe even isn't today?).

> > > I can't judge if it's better, but IMHO not to change the number of
> > > output is simply no worse.
> >
> > It makes the behaviour somewhat non-consistent, e.g. assume MPlayer
> > probing support of the filter chain for number of channels.
> > With your proposed patch af_pan output gets "stuck" to a number of
> > output channels depending on the order in which you probe.
> 
> I can't understand this. What's better in this case if we initialize it to one?

The result does not depend on in which order you called REINIT with
which parameters, IOW REINIT does not have such a very weird
side-effect.

> > One possibility would be to store the parsed nch value some place else
> > so it won't get overwritten on reinit.
> 
> Not only nch but all previous settings or we have no sound thanks to
> pan's default behavior. Maybe in a different patch.

Huh? The only other thing parsed on the command line is s->level, which
is not overwritten on init, so no need to save and restore it.
If you mean the bugzilla bug, that bug is somewhere else (filter
suboption parser does not treat command line as const string, which is
simply wrong).

Greetings,
Reimar Döffinger