[pulseaudio-discuss] [PATCH] Fix wrong input device in module-waveout

[Tanu Kaskinen]

On Wed, 2017-03-01 at 15:54 +0100, Moritz Bruder wrote:
> Am 28.02.2017 um 14:40 schrieb Tanu Kaskinen:
> > On Thu, 2017-02-23 at 17:38 +0100, Moritz Bruder wrote:
> > > Both input and output device were chosen with the same device number.
> > > This is problematic as those numbers don't have to correspond.
> > > Additionally the input device was named after the output device. This
> > > commit adresses both issues by providing specific parameters for each
> > > type.
> > > ---
> > >   src/modules/module-waveout.c | 89 ++++++++++++++++++++++++++++++--------------
> > >   1 file changed, 62 insertions(+), 27 deletions(-)
> > 
> > Thanks for the patch! One comment below:
> > 
> > >   static const char* const valid_modargs[] = {
> > >       "sink_name",
> > >       "source_name",
> > > -    "device",
> > > -    "device_name",
> > > +    "output_device",
> > > +    "output_device_name",
> > > +    "input_device",
> > > +    "input_device_name",
> > 
> > Could we keep supporting the old "device" and "device_name" options to
> > keep compatibility with old configuration files?
> 
> Those arguments make only sense if you have one device for input and 
> output anyway (because then the device numbers, which are indices, are 
> luckily the same, which is 0). Otherwise input and output device id are 
> paired randomly (in a system-specified order). Also, specifying input 
> device names was via the output device name, so all such used devices 
> are a wrong configuration that luckily works. *Important*: Note that 
> whenever those arguments are not specified, it will have the same 
> behaviour as before (except that the input device name is now correct). 
> Thus most original configurations that work (i.e. without *device* or 
> *device_name*) will work with the patch.
> > Or at least print a helpful error message with instructions for how to update the
> > configuration if those options are passed?
> 
> I think the arguments are pretty self-explanatory. Furthermore I don't 
> know how to do it. Considering there are very few windows users of 
> pulseaudio, I don't think it makes much sense. The best thing to do, 
> would be to split the module up into one for input devices and one for 
> output devices, which should eliminate any ambiguity. Let me know what 
> you think. I'm open for suggestions!

Yes, the arguments are pretty self-explanatory, but when a person
installs a new version of pulseaudio, there's nothing that will notify
them about the changed module interface, so they will not be aware of
the new options, no matter how self-explanatory the option names are.

Printing instructions as I suggested means just adding the old options
back to the valid_modargs table and checking if the options are set.
The checking can be done with pa_modargs_get_value(), which will return
NULL if the option is not set. pa_log_error() can be used for printing
the error message and the instructions to use the new options instead.

As you said, the set of affected users is small, so I can apply the
path as is if you want, but I would prefer you to add a more helpful
error message than the current "failed to parse module arguments".

-- 
Tanu

https://www.patreon.com/tanuk