Re: [PATCH 2/4] conf: allow to map sound device to host device

2020-07-22 Thread Daniel P . Berrangé
On Wed, Jul 22, 2020 at 08:55:02AM +0200, Gerd Hoffmann wrote:
>   Hi,
> 
> > IIUC, QEMU can expose multiple sound devices to the guest too.
> > 
> > I think this means that we can have a M:N relationship between
> > a sound device, and an audio backend, not just 1:1.
> 
> It's 1:N.  Sound devices have a single backend, but a backends can
> service multiple sound devices.

Sigh yes, of course, wasn't thinking right.

> > Assuming I'm right about the M:N relationship, I assume that
> > of multiple cards all do playback concurrently, something
> > will have todo mixing of the streams ?
> 
> In general it is a good idea to go with 1:1 if possible.  With
> pulseaudio this works fine.  You'll have two streams to pulseaudio
> then, pulseaudio does the mixing, and you'll see both streams in
> mixer apps.
> 
> OSS devices tend to not like being opened multiple times, so you
> have to go with 1:N if you want multiple sound devices.  qemu mixes
> the playback streams then.  Not fully sure what happens with
> recording, probably all sound devices see the very same stream.

Ok, so it sounds like we need the XML schema I illustrated. Even though
we'll recommend 1:1 for general use, we'll want the option to supoort
1:N setups.

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 :|



Re: [PATCH 2/4] conf: allow to map sound device to host device

2020-07-22 Thread Roman Bogorodskiy
  Daniel P. Berrangé wrote:

> On Sat, Jul 18, 2020 at 04:31:16PM +0400, Roman Bogorodskiy wrote:
> > Extend  device element to accept "soundDevice"
> > sub-element which allows to map guest sound device to host
> > sound device.
> > 
> > Example
> > 
> >   
> > 
> >   
> 
> IIUC, FreeBSD's audio subsystem is the classic OSS API ?

The sound(4) manpage[1] claims it supports most of the OSS ioctls().

> 
> > 
> > The "playback" attribute points to the playback device,
> > and "recording" attribute points to the recording device.
> 
> I'm thinking about how we'll have to deal with QEMU's sound backend
> options, and alignment with BHyve / FreeBSD.
> 
> In QEMU there are multiple backends, OSS, ALSA, PulseAudio, SPICE,
> VNC, and many more. The backends have many properties, and many
> of the properties can be set separately for input and output.
> 
> IIUC, QEMU can expose multiple sound devices to the guest too.
> 
> I think this means that we can have a M:N relationship between
> a sound device, and an audio backend, not just 1:1.
> 
> Assuming I'm right about the M:N relationship, I assume that
> of multiple cards all do playback concurrently, something
> will have todo mixing of the streams ?  How will that work
> with audio capture, is only one card allowed to capture at
> any time ?

In case of FreeBSD, the sound driver does mixing on its own, i.e.
you can boot multiple guests pointed to /dev/dsp0 and audio streams from
these guests will be played properly.

I didn't test capturing though.

> I'm copying Gerd to confirm the above...
> 
> Anyway, if we have M:N relation, then we'll need separate
> configuration elements.
> 
> So I think we'd need to allow something like this:
> 
>
>  
>
> 
>
>  
>
> 
>
>  
>
> 
>
>  
>  
>
> 
>
> 
> 
> Here we have one sound device connected to OSS, and two sound
> devices connected to SPICE. 
> 
> > 
> > Signed-off-by: Roman Bogorodskiy 
> > ---
> >  docs/schemas/domaincommon.rng | 15 +++
> >  src/conf/domain_conf.c| 24 
> >  src/conf/domain_conf.h|  3 +++
> >  3 files changed, 42 insertions(+)
> > 
> > diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
> > index a810f569c6..b11b3f2af6 100644
> > --- a/docs/schemas/domaincommon.rng
> > +++ b/docs/schemas/domaincommon.rng
> > @@ -4346,6 +4346,18 @@
> >
> >  
> >
> > +  
> > +
> > +   
> > + 
> > +   
> > +   
> > + 
> > +   
> > + 
> > +   
> > +
> > +  
> >
> >  
> >
> > @@ -4366,6 +4378,9 @@
> >  
> >
> >  
> > +   
> > + 
> > +   
> >  
> >
> >  
> > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> > index 7ecd2818b9..b678a2319d 100644
> > --- a/src/conf/domain_conf.c
> > +++ b/src/conf/domain_conf.c
> > @@ -2850,6 +2850,9 @@ void virDomainSoundDefFree(virDomainSoundDefPtr def)
> >  virDomainSoundCodecDefFree(def->codecs[i]);
> >  VIR_FREE(def->codecs);
> >  
> > +VIR_FREE(def->playback);
> > +VIR_FREE(def->recording);
> > +
> >  VIR_FREE(def);
> >  }
> >  
> > @@ -14984,6 +14987,8 @@ virDomainSoundDefParseXML(virDomainXMLOptionPtr 
> > xmlopt,
> >  virDomainSoundDefPtr def;
> >  VIR_XPATH_NODE_AUTORESTORE(ctxt);
> >  g_autofree char *model = NULL;
> > +char *recording = NULL;
> > +xmlNodePtr soundDeviceNode;
> >  
> >  if (VIR_ALLOC(def) < 0)
> >  return NULL;
> > @@ -15024,6 +15029,14 @@ virDomainSoundDefParseXML(virDomainXMLOptionPtr 
> > xmlopt,
> >  }
> >  }
> >  
> > +soundDeviceNode = virXPathNode("./soundDevice", ctxt);
> > +if (soundDeviceNode) {
> > +def->playback = virXMLPropString(soundDeviceNode, "playback");
> > +recording = virXMLPropString(soundDeviceNode, "recording");
> > +if (recording)
> > +def->recording = recording;
> > +}
> > +
> >  if (virDomainDeviceInfoParseXML(xmlopt, node, &def->info, flags) < 0)
> >  goto error;
> >  
> > @@ -15056,6 +15069,9 @@ virDomainSoundDefEquals(const virDomainSoundDef *a,
> >  !virDomainDeviceInfoAddressIsEqual(&a->info, &b->info))
> >  return false;
> >  
> > +if ((a->playback != b->playback) || (a->recording != b->recording))
> > +return false;
> > +
> >  return true;
> >  }
> >  
> > @@ -27284,6 +27300,14 @@ virDomainSoundDefFormat(virBufferPtr buf,
> >  for (i = 0; i < def->ncodecs; i++)
> >  virDomainSoundCodecDefFormat(&childBuf, def->codecs[i]);
> >  
> > +if (def->playback) {
> > +virBufferAsprintf(&childBuf, " > def->playback);
> > +if (def->recording)
> > +virBufferAsprintf(&childBuf, " recording='%s'/>\n", 
> > def->recording);
> > +else
> > +virBufferAddLit(&childBuf, "/>\n");
> > +}
> > +
> >  if (virDomainDeviceInfoFormat(&childBuf

Re: [PATCH 2/4] conf: allow to map sound device to host device

2020-07-21 Thread Gerd Hoffmann
  Hi,

> IIUC, QEMU can expose multiple sound devices to the guest too.
> 
> I think this means that we can have a M:N relationship between
> a sound device, and an audio backend, not just 1:1.

It's 1:N.  Sound devices have a single backend, but a backends can
service multiple sound devices.

> Assuming I'm right about the M:N relationship, I assume that
> of multiple cards all do playback concurrently, something
> will have todo mixing of the streams ?

In general it is a good idea to go with 1:1 if possible.  With
pulseaudio this works fine.  You'll have two streams to pulseaudio
then, pulseaudio does the mixing, and you'll see both streams in
mixer apps.

OSS devices tend to not like being opened multiple times, so you
have to go with 1:N if you want multiple sound devices.  qemu mixes
the playback streams then.  Not fully sure what happens with
recording, probably all sound devices see the very same stream.

take care,
  Gerd



Re: [PATCH 2/4] conf: allow to map sound device to host device

2020-07-21 Thread Daniel P . Berrangé
On Sat, Jul 18, 2020 at 04:31:16PM +0400, Roman Bogorodskiy wrote:
> Extend  device element to accept "soundDevice"
> sub-element which allows to map guest sound device to host
> sound device.
> 
> Example
> 
>   
> 
>   

IIUC, FreeBSD's audio subsystem is the classic OSS API ?

> 
> The "playback" attribute points to the playback device,
> and "recording" attribute points to the recording device.

I'm thinking about how we'll have to deal with QEMU's sound backend
options, and alignment with BHyve / FreeBSD.

In QEMU there are multiple backends, OSS, ALSA, PulseAudio, SPICE,
VNC, and many more. The backends have many properties, and many
of the properties can be set separately for input and output.

IIUC, QEMU can expose multiple sound devices to the guest too.

I think this means that we can have a M:N relationship between
a sound device, and an audio backend, not just 1:1.

Assuming I'm right about the M:N relationship, I assume that
of multiple cards all do playback concurrently, something
will have todo mixing of the streams ?  How will that work
with audio capture, is only one card allowed to capture at
any time ?

I'm copying Gerd to confirm the above...

Anyway, if we have M:N relation, then we'll need separate
configuration elements.

So I think we'd need to allow something like this:

   
 
   

   
 
   

   
 
   

   
 
 
   

   


Here we have one sound device connected to OSS, and two sound
devices connected to SPICE. 

> 
> Signed-off-by: Roman Bogorodskiy 
> ---
>  docs/schemas/domaincommon.rng | 15 +++
>  src/conf/domain_conf.c| 24 
>  src/conf/domain_conf.h|  3 +++
>  3 files changed, 42 insertions(+)
> 
> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
> index a810f569c6..b11b3f2af6 100644
> --- a/docs/schemas/domaincommon.rng
> +++ b/docs/schemas/domaincommon.rng
> @@ -4346,6 +4346,18 @@
>
>  
>
> +  
> +
> +   
> + 
> +   
> +   
> + 
> +   
> + 
> +   
> +
> +  
>
>  
>
> @@ -4366,6 +4378,9 @@
>  
>
>  
> + 
> +   
> + 
>  
>
>  
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 7ecd2818b9..b678a2319d 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -2850,6 +2850,9 @@ void virDomainSoundDefFree(virDomainSoundDefPtr def)
>  virDomainSoundCodecDefFree(def->codecs[i]);
>  VIR_FREE(def->codecs);
>  
> +VIR_FREE(def->playback);
> +VIR_FREE(def->recording);
> +
>  VIR_FREE(def);
>  }
>  
> @@ -14984,6 +14987,8 @@ virDomainSoundDefParseXML(virDomainXMLOptionPtr 
> xmlopt,
>  virDomainSoundDefPtr def;
>  VIR_XPATH_NODE_AUTORESTORE(ctxt);
>  g_autofree char *model = NULL;
> +char *recording = NULL;
> +xmlNodePtr soundDeviceNode;
>  
>  if (VIR_ALLOC(def) < 0)
>  return NULL;
> @@ -15024,6 +15029,14 @@ virDomainSoundDefParseXML(virDomainXMLOptionPtr 
> xmlopt,
>  }
>  }
>  
> +soundDeviceNode = virXPathNode("./soundDevice", ctxt);
> +if (soundDeviceNode) {
> +def->playback = virXMLPropString(soundDeviceNode, "playback");
> +recording = virXMLPropString(soundDeviceNode, "recording");
> +if (recording)
> +def->recording = recording;
> +}
> +
>  if (virDomainDeviceInfoParseXML(xmlopt, node, &def->info, flags) < 0)
>  goto error;
>  
> @@ -15056,6 +15069,9 @@ virDomainSoundDefEquals(const virDomainSoundDef *a,
>  !virDomainDeviceInfoAddressIsEqual(&a->info, &b->info))
>  return false;
>  
> +if ((a->playback != b->playback) || (a->recording != b->recording))
> +return false;
> +
>  return true;
>  }
>  
> @@ -27284,6 +27300,14 @@ virDomainSoundDefFormat(virBufferPtr buf,
>  for (i = 0; i < def->ncodecs; i++)
>  virDomainSoundCodecDefFormat(&childBuf, def->codecs[i]);
>  
> +if (def->playback) {
> +virBufferAsprintf(&childBuf, " def->playback);
> +if (def->recording)
> +virBufferAsprintf(&childBuf, " recording='%s'/>\n", 
> def->recording);
> +else
> +virBufferAddLit(&childBuf, "/>\n");
> +}
> +
>  if (virDomainDeviceInfoFormat(&childBuf, &def->info, flags) < 0)
>  return -1;
>  
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index 241149af24..b501f48442 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -1415,6 +1415,9 @@ struct _virDomainSoundDef {
>  
>  size_t ncodecs;
>  virDomainSoundCodecDefPtr *codecs;
> +
> +char *playback;
> +char *recording;
>  };
>  
>  typedef enum {
> -- 
> 2.27.0
> 

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

[PATCH 2/4] conf: allow to map sound device to host device

2020-07-18 Thread Roman Bogorodskiy
Extend  device element to accept "soundDevice"
sub-element which allows to map guest sound device to host
sound device.

Example

  

  

The "playback" attribute points to the playback device,
and "recording" attribute points to the recording device.

Signed-off-by: Roman Bogorodskiy 
---
 docs/schemas/domaincommon.rng | 15 +++
 src/conf/domain_conf.c| 24 
 src/conf/domain_conf.h|  3 +++
 3 files changed, 42 insertions(+)

diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index a810f569c6..b11b3f2af6 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -4346,6 +4346,18 @@
   
 
   
+  
+
+   
+ 
+   
+   
+ 
+   
+ 
+   
+
+  
   
 
   
@@ -4366,6 +4378,9 @@
 
   
 
+   
+ 
+   
 
   
 
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 7ecd2818b9..b678a2319d 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -2850,6 +2850,9 @@ void virDomainSoundDefFree(virDomainSoundDefPtr def)
 virDomainSoundCodecDefFree(def->codecs[i]);
 VIR_FREE(def->codecs);
 
+VIR_FREE(def->playback);
+VIR_FREE(def->recording);
+
 VIR_FREE(def);
 }
 
@@ -14984,6 +14987,8 @@ virDomainSoundDefParseXML(virDomainXMLOptionPtr xmlopt,
 virDomainSoundDefPtr def;
 VIR_XPATH_NODE_AUTORESTORE(ctxt);
 g_autofree char *model = NULL;
+char *recording = NULL;
+xmlNodePtr soundDeviceNode;
 
 if (VIR_ALLOC(def) < 0)
 return NULL;
@@ -15024,6 +15029,14 @@ virDomainSoundDefParseXML(virDomainXMLOptionPtr xmlopt,
 }
 }
 
+soundDeviceNode = virXPathNode("./soundDevice", ctxt);
+if (soundDeviceNode) {
+def->playback = virXMLPropString(soundDeviceNode, "playback");
+recording = virXMLPropString(soundDeviceNode, "recording");
+if (recording)
+def->recording = recording;
+}
+
 if (virDomainDeviceInfoParseXML(xmlopt, node, &def->info, flags) < 0)
 goto error;
 
@@ -15056,6 +15069,9 @@ virDomainSoundDefEquals(const virDomainSoundDef *a,
 !virDomainDeviceInfoAddressIsEqual(&a->info, &b->info))
 return false;
 
+if ((a->playback != b->playback) || (a->recording != b->recording))
+return false;
+
 return true;
 }
 
@@ -27284,6 +27300,14 @@ virDomainSoundDefFormat(virBufferPtr buf,
 for (i = 0; i < def->ncodecs; i++)
 virDomainSoundCodecDefFormat(&childBuf, def->codecs[i]);
 
+if (def->playback) {
+virBufferAsprintf(&childBuf, "playback);
+if (def->recording)
+virBufferAsprintf(&childBuf, " recording='%s'/>\n", 
def->recording);
+else
+virBufferAddLit(&childBuf, "/>\n");
+}
+
 if (virDomainDeviceInfoFormat(&childBuf, &def->info, flags) < 0)
 return -1;
 
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 241149af24..b501f48442 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1415,6 +1415,9 @@ struct _virDomainSoundDef {
 
 size_t ncodecs;
 virDomainSoundCodecDefPtr *codecs;
+
+char *playback;
+char *recording;
 };
 
 typedef enum {
-- 
2.27.0