[pulseaudio-discuss] [PATCH] Removed the option to specify sample format. (bug 46529)

[Tanu Kaskinen]

On Sat, 2013-01-05 at 08:32 +0530, Arun Raghavan wrote:
> On Fri, 2013-01-04 at 17:27 -0600, Pierre-Louis Bossart wrote:
> > On 01/04/2013 07:59 AM, Tanu Kaskinen wrote:
> > > On Thu, 2013-01-03 at 09:35 -0600, Pierre-Louis Bossart wrote:
> > >> What problem are you trying to fix here? It's not obvious why the sample
> > >> format information should be removed, why not the rate and channels
> > >> while we are at it? And even if this is needed, I guess there's dead
> > >> code to parse the argument that should be removed in the _init routine?
> > >
> > > The patch title refers to bug 46529, I guess you missed that? Here's the
> > > link: https://bugs.freedesktop.org/show_bug.cgi?id=46529
> > >
> > > I hope that makes things clearer. As for removing dead code from
> > > pa__init(), that's not necessary, because the format argument was parsed
> > > by pa_modargs_get_sample_spec_and_channel_map(). There's no dead code to
> > > remove.
> > 
> > My point was that there are multiple modules that were derived from this 
> > initial virtual sink contributed about 3 years ago, and if you want to 
> > be consistent all of them should be cleaned (source, eq, surround, 
> > echo-cancel, etc). Rather than removing the option once you may want to 
> > either remove this option everywhere or fix the bug by introducing the 
> > required conversion.

Good point about checking the derived modules too. It seems that
module-equalizer-sink has the same bug, while module-echo-cancel and
module-virtual-surround-sink support the format option but ignore it.
module-virtual-source supports the option too, and also uses it without
problems.

I originally thought, for some reason, that it would be non-trivial to
fix module-virtual-sink to work with any sample format. That should not
be the case (the module only needs to copy samples without converting
anything), so I think the way forward is to add the format support back
to module-virtual-sink, and remove the support from
module-equalizer-sink, module-echo-cancel and
module-virtual-surround-sink.

> Or, since this is merely meant to be a module that other modules are
> meant to be based on, put that as a big comment for derivative modules
> to deal with, and leave the modarg as is.
> 
> Tangentially, we should also not actually installing
> module-virtual-{sink,source}

I disagree. They are useful modules, I have used them for testing sink
volume code.

-- 
Tanu