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
