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

[Zuxy Meng]

Hi,

2007/6/7, Reimar Döffinger <Reimar.Doeffinger at stud.uni-karlsruhe.de>:
> 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...

OK.

>
> > 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?).

(W/ my patch) If it's been REINITed before then previous settings will
be retained. If not then pan won't alter the number of audio channels,
similar behavior as most other audio filters.

> > > > 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).

Would u come up with a patch for this?

-- 
Zuxy
Beauty is truth,
While truth is beauty.
PGP KeyID: E8555ED6