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

[Zuxy Meng]

Hi,

2007/6/8, Reimar Döffinger <Reimar.Doeffinger at stud.uni-karlsruhe.de>:
> Hello,
> On Fri, Jun 08, 2007 at 12:29:18PM +0800, Zuxy Meng wrote:
> [...]
> > > 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.
>
> That will become a huge pain for supporting AC3 that changes the number
> of channels. Luckily enough it is not true, at least the resample
> filters update nch on reinit to the new input value.

How does such AC3 work? Will it change the # of channels within one
audio stream from time to time or will it contain multiple streams
that have different # of channels?

I have to admit that if not intended to implement audio balance, I
wouldn't bother look into the pan filter. I simply wants this feature
to be least intrusive: it touches only the two front channels; (1) if
the original content has surround channels, they will be played as is;
or (2) if the user has some previous settings of pan in command line
(e.g. mix existing channels to produce a new one), they will be
reserved as much as possible.

If by default pan sets the output number to 1, and doesn't do anything
about output channel on REINIT, how can we get the input # of channels
of pan (the # of input just for this filter, not the # of channels
that enters the filter chain; therefore it depends on the position of
pan within the chain as you've said)? Without known # of input
channels, (1) would be impossible.

IMHO it's natural to use pan to implement balance. Although the term
(pan and balance) themselves do not have quite identical meanings,
they are often used interchangeably and I see no reason to implement a
new "balance" audio filter. But one difference is that balance
shouldn't alter the # of channels. Maybe we can differentiate the use
of pan as a balancer with a static variable? See attached 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?
>
> Not really. Half a fix is attached though.

Pretty nice. :-)

-- 
Zuxy
Beauty is truth,
While truth is beauty.
PGP KeyID: E8555ED6
-------------- next part --------------
Index: libaf/af_pan.c
===================================================================
--- libaf/af_pan.c	?????? 23505??
+++ libaf/af_pan.c	????????????
@@ -25,6 +25,8 @@
   float level[AF_NCH][AF_NCH];	// Gain level for each channel
 }af_pan_t;
 
+static int used_as_balance;
+
 // Initialization and runtime control
 static int control(struct af_instance_s* af, int cmd, void* arg)
 {
@@ -35,6 +37,8 @@
     // Sanity check
     if(!arg) return AF_ERROR;
 
+    if (used_as_balance)
+      af->data->nch = ((af_data_t*)arg)->nch;
     af->data->rate   = ((af_data_t*)arg)->rate;
     af->data->format = AF_FORMAT_FLOAT_NE;
     af->data->bps    = 4;
@@ -48,7 +52,11 @@
       ((af_data_t*)arg)->bps = af->data->bps;
       return AF_FALSE;
     }
-    return control(af,AF_CONTROL_PAN_NOUT | AF_CONTROL_SET, &af->data->nch);
+
+    if (used_as_balance)
+      return AF_OK;
+    else
+      return control(af,AF_CONTROL_PAN_NOUT | AF_CONTROL_SET, &af->data->nch);
   case AF_CONTROL_COMMAND_LINE:{
     int   nch = 0;
     int   n = 0;
@@ -104,6 +112,7 @@
 	     " between 1 and %i. Current value is %i\n",AF_NCH,((int*)arg)[0]);
       return AF_ERROR;
     }
+    used_as_balance = 0;
     af->data->nch=((int*)arg)[0];
     return AF_OK;
   case AF_CONTROL_PAN_NOUT | AF_CONTROL_GET:
@@ -169,6 +178,7 @@
   af->play=play;
   af->mul.n=1;
   af->mul.d=1;
+  used_as_balance=1;
   af->data=calloc(1,sizeof(af_data_t));
   af->setup=calloc(1,sizeof(af_pan_t));
   if(af->data == NULL || af->setup == NULL)