All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Daniel P. Berrangé" <berrange@redhat.com>
To: Maxim Levitsky <mlevitsk@redhat.com>
Cc: qemu-devel@nongnu.org, "Markus Armbruster" <armbru@redhat.com>,
	"Gerd Hoffmann" <kraxel@redhat.com>,
	"Zoltán Kővágó" <dirty.ice.hu@gmail.com>
Subject: Re: [Qemu-devel] [PATCH v2 4/4] audio: paaudio: ability to specify stream name
Date: Wed, 28 Aug 2019 11:26:03 +0100	[thread overview]
Message-ID: <20190828102603.GG2991@redhat.com> (raw)
In-Reply-To: <6a1f7b1dc372abfbbf8efaa7d0ce9c7db1881849.camel@redhat.com>

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.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|


  reply	other threads:[~2019-08-28 10:27 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-08-26 19:59 [Qemu-devel] [PATCH v2 0/4] Audio: misc fixes for "Audio 20190821 patches" Kővágó, Zoltán
2019-08-26 19:59 ` [Qemu-devel] [PATCH v2 1/4] audio: fix invalid malloc size in audio_create_pdos Kővágó, Zoltán
2019-08-26 19:59 ` [Qemu-devel] [PATCH v2 2/4] audio: omitting audiodev= parameter is only deprecated Kővágó, Zoltán
2019-08-28 11:53   ` Gerd Hoffmann
2019-08-26 19:59 ` [Qemu-devel] [PATCH v2 3/4] audio: paaudio: fix client name Kővágó, Zoltán
2019-08-27  5:37   ` Gerd Hoffmann
2019-08-27 11:26     ` Maxim Levitsky
2019-08-26 19:59 ` [Qemu-devel] [PATCH v2 4/4] audio: paaudio: ability to specify stream name Kővágó, Zoltán
2019-08-27  5:42   ` Gerd Hoffmann
2019-08-27 22:43     ` Zoltán Kővágó
2019-08-28  7:33       ` Gerd Hoffmann
2019-08-28  9:12         ` Maxim Levitsky
2019-08-28  9:53       ` Daniel P. Berrangé
2019-08-28 10:14         ` Maxim Levitsky
2019-08-28 10:26           ` Daniel P. Berrangé [this message]
2019-08-28 10:39             ` Maxim Levitsky
2019-09-09 23:42               ` Zoltán Kővágó
2019-09-10  7:00                 ` Gerd Hoffmann
2019-08-26 21:43 ` [Qemu-devel] [PATCH v2 0/4] Audio: misc fixes for "Audio 20190821 patches" Maxim Levitsky

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190828102603.GG2991@redhat.com \
    --to=berrange@redhat.com \
    --cc=armbru@redhat.com \
    --cc=dirty.ice.hu@gmail.com \
    --cc=kraxel@redhat.com \
    --cc=mlevitsk@redhat.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.