Re: [Qemu-devel] [PATCH v2 4/4] audio: paaudio: ability to specify stream name

["Zoltán Kővágó" <dirty.ice.hu@gmail.com>]

On 2019-08-28 12:39, Maxim Levitsky wrote:
> On Wed, 2019-08-28 at 11:26 +0100, Daniel P. Berrangé wrote:
>> On Wed, Aug 28, 2019 at 01:14:03PM +0300, Maxim Levitsky wrote:
>>> On Wed, 2019-08-28 at 10:53 +0100, Daniel P. Berrangé wrote:
>>>> On Wed, Aug 28, 2019 at 12:43:49AM +0200, Zoltán Kővágó wrote:
>>>>> On 2019-08-27 07:42, Gerd Hoffmann wrote:
>>>>>> On Mon, Aug 26, 2019 at 09:59:04PM +0200, Kővágó, Zoltán wrote:
>>>>>>> This can be used to identify stream in tools like pavucontrol when one
>>>>>>> creates multiple -audiodevs or runs multiple qemu instances.
>>>>>>
>>>>>> Hmm, can we create an useful name automatically, without yet another
>>>>>> config option?
>>>>>>
>>>>>> Useful choices could be the device name (usb-audio, ...) or the device
>>>>>> id (whatever -device id=xxx was specified on the command line).
>>>>>
>>>>> I'm afraid this is not going to work with the current architecture: due
>>>>> to mixeng even if you have multiple devices, they'll be mixed to a
>>>>> single stream and the audio backend will only see this one mixed stream.
>>>>>   As a workaround we could do something like concat all device names or
>>>>> ids, but I don't like that idea.
>>>>>
>>>>> Alternatively we could use the id of the audiodev instead, and no more
>>>>> problems with mixeng.  However, with mixeng off (implemented in my next
>>>>> patch series) suddenly soundcards will have suddenly end up as different
>>>>> streams.  (This can be worked around by creating multiple audiodevs,
>>>>> like what you have to use now to get multiple streams from pa, so this
>>>>> is probably a smaller problem.)
>>>>>
>>>>> Currently I'm leaning for the audiodev's id option, unless someone
>>>>> proposes something better.
>>>>
>>>> Using the audiodev id is not a good idea. If you have multiple QEMU's
>>>> on your host, it is highly likely that libvirt will have assigned
>>>> the same audiodev id to all of them.  Using the vm name would be ok,
>>>> but only if you assume that each gust only has a single audio device.
>>>>
>>>> Using a combination of vm name + audidev id is going to be unique
>>>> per host, but not especially friendly as a user visible name. It
>>>> would be ok as a default, but I'd think we should let the mgmt app
>>>> specify stream name explicitly, so that something user friendly
>>>> can be set.
>>>
>>> No, no!
>>> It seems that pulseaudio has a name for each connection, and a name for each
>>> steam within that connection.
>>>
>>> The suggestion is that we use the VM name for the connection,
>>> (which will be unique per VM usually, at least the user can make it be so)
>>> and then use the audiodev id for each stream. Of course for multiple VMs,
>>> the audiodev ids will be the same, but this is all right since you can
>>> always distinguish them that the streams come from different VMs.
>>
>> Ok, if I'm reading the code correctly, it seems we do take care to
>> re-use a single connection to PA for all audiodevs we create. So a
>> VMname is fine for the connection.
>>
>>> Also note that this thing is cosmetic from the correctness point of view,
>>> that is pulse-audio internally has no problem with duplicate IDs.
>>>
>>> The thing is useful mostly for tweaking the output streams in the pavucontrol,
>>> where the names will allow you to easily know which steam is which.
>>
>> Yep, I wasn't really concerned about internals - from the user POV being
>> able to accurately distinguish streams in pavucontrol is very important
>> though, so we should ensure that's possible. If we use 'id'  for the
>> stream as a default though, we should still allow an override, as 'id'
>> values are not really intended as end user visible data. If a guest
>> has multiple devices I'd expect to be able to give them names that are
>> meaningful to me as a user, not something libvirt auto-generates for
>> its own machine oriented use.
> 
> I have absolutely nothing against user specified override!
> Just that if the idea is shot down, lets at least have device id instead.
> 
> 
> For the reference this is how currently the sound streams are shown,
> without any patches applied
> https://imgur.com/a/I8HZhgx
> 
> Gnome sound panel only shows application names,
> but pavucontrol shows both the application name and stream name.
> 
> Best regards,
> 	Maxim Levitsky
> 

Ping.

If I understand the situation correctly, the current consensus is:

* use VM name for PA server connection
* audiodev id is a good default for PA stream name

What is not clear whether we need a separate qapi option for stream 
name, or just always use the audiodev id.  I don't use pulseaudio or 
libvirt, so I can't really comment about this issue.

Ideally I'd like to create a new patch with this change and a fix for 
the coverity issue reported in [1].

Regards,
Zoltan

[1]: https://lists.gnu.org/archive/html/qemu-devel/2019-09/msg01543.html