[Qemu-devel] [PATCH 0/3] Audio: misc fixes for "Audio 20190821 patches", part two

[Qemu-devel] [PATCH 0/3] Audio: misc fixes for "Audio 20190821 patches", part two

[Kővágó, Zoltán]

Hi,

This series contains some random fixes for the "Audio 20190821 patches":
a coverity bugfix and pulseaudio connection/stream names fix.

Since there wasn't a clear consensus about naming the pa streams, I've
split it into two commits: the first time just uses the audiodev id
unconditionally, while the last commit adds a new option to qapi to
override it.  This way we can easily drop the last commit if it turns
out to be unnecessary.

Regards,
Zoltan


Kővágó, Zoltán (3):
  audio: fix parameter dereference before NULL check
  audio: paaudio: fix connection and stream name
  audio: paaudio: ability to specify stream name

 audio/audio_template.h | 7 +++++--
 audio/paaudio.c        | 9 ++++++---
 qapi/audio.json        | 6 ++++++
 3 files changed, 17 insertions(+), 5 deletions(-)

-- 
2.23.0

[Qemu-devel] [PATCH 1/3] audio: fix parameter dereference before NULL check

[Kővágó, Zoltán]

This should fix Coverity issues CID 1405305 and 1405301.

Signed-off-by: Kővágó, Zoltán <DirtY.iCE.hu@gmail.com>
---
 audio/audio_template.h | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/audio/audio_template.h b/audio/audio_template.h
index 2562bf5f00..cc4b53b5d7 100644
--- a/audio/audio_template.h
+++ b/audio/audio_template.h
@@ -428,8 +428,8 @@ SW *glue (AUD_open_, TYPE) (
     struct audsettings *as
     )
 {
-    AudioState *s = card->state;
-    AudiodevPerDirectionOptions *pdo = glue(audio_get_pdo_, TYPE)(s->dev);
+    AudioState *s;
+    AudiodevPerDirectionOptions *pdo;
 
     if (audio_bug(__func__, !card || !name || !callback_fn || !as)) {
         dolog ("card=%p name=%p callback_fn=%p as=%p\n",
@@ -437,6 +437,9 @@ SW *glue (AUD_open_, TYPE) (
         goto fail;
     }
 
+    s = card->state;
+    pdo = glue(audio_get_pdo_, TYPE)(s->dev);
+
     ldebug ("open %s, freq %d, nchannels %d, fmt %d\n",
             name, as->freq, as->nchannels, as->fmt);
 
-- 
2.23.0

[Qemu-devel] [PATCH 2/3] audio: paaudio: fix connection and stream name

[Kővágó, Zoltán]

Connection name was previously erroneously set to the server socket
path, while connection names were simply "qemu".  After this patch, the
connection name will be the vm name (falling back to "qemu" if not
specified), while stream names will be the audiodev's id.

Signed-off-by: Kővágó, Zoltán <DirtY.iCE.hu@gmail.com>
---
 audio/paaudio.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/audio/paaudio.c b/audio/paaudio.c
index bfef9acaad..19b7e39092 100644
--- a/audio/paaudio.c
+++ b/audio/paaudio.c
@@ -2,6 +2,7 @@
 
 #include "qemu/osdep.h"
 #include "qemu/module.h"
+#include "qemu-common.h"
 #include "audio.h"
 #include "qapi/opts-visitor.h"
 
@@ -562,7 +563,7 @@ static int qpa_init_out(HWVoiceOut *hw, struct audsettings *as,
 
     pa->stream = qpa_simple_new (
         c,
-        "qemu",
+        g->dev->id,
         PA_STREAM_PLAYBACK,
         ppdo->has_name ? ppdo->name : NULL,
         &ss,
@@ -630,7 +631,7 @@ static int qpa_init_in(HWVoiceIn *hw, struct audsettings *as, void *drv_opaque)
 
     pa->stream = qpa_simple_new (
         c,
-        "qemu",
+        g->dev->id,
         PA_STREAM_RECORD,
         ppdo->has_name ? ppdo->name : NULL,
         &ss,
@@ -857,6 +858,7 @@ static int qpa_validate_per_direction_opts(Audiodev *dev,
 /* common */
 static void *qpa_conn_init(const char *server)
 {
+    const char *vm_name;
     PAConnection *c = g_malloc0(sizeof(PAConnection));
     QTAILQ_INSERT_TAIL(&pa_conns, c, list);
 
@@ -865,8 +867,9 @@ static void *qpa_conn_init(const char *server)
         goto fail;
     }
 
+    vm_name = qemu_get_vm_name();
     c->context = pa_context_new(pa_threaded_mainloop_get_api(c->mainloop),
-                                server);
+                                vm_name ? vm_name : "qemu");
     if (!c->context) {
         goto fail;
     }
-- 
2.23.0

[Qemu-devel] [PATCH 3/3] audio: paaudio: ability to specify stream name

[Kővágó, Zoltán]

This can be used to identify stream in tools like pavucontrol when one
creates multiple -audiodevs or runs multiple qemu instances.

Signed-off-by: Kővágó, Zoltán <DirtY.iCE.hu@gmail.com>
---
 audio/paaudio.c | 4 ++--
 qapi/audio.json | 6 ++++++
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/audio/paaudio.c b/audio/paaudio.c
index 19b7e39092..d9cff9a2be 100644
--- a/audio/paaudio.c
+++ b/audio/paaudio.c
@@ -563,7 +563,7 @@ static int qpa_init_out(HWVoiceOut *hw, struct audsettings *as,
 
     pa->stream = qpa_simple_new (
         c,
-        g->dev->id,
+        ppdo->has_stream_name ? ppdo->stream_name : g->dev->id,
         PA_STREAM_PLAYBACK,
         ppdo->has_name ? ppdo->name : NULL,
         &ss,
@@ -631,7 +631,7 @@ static int qpa_init_in(HWVoiceIn *hw, struct audsettings *as, void *drv_opaque)
 
     pa->stream = qpa_simple_new (
         c,
-        g->dev->id,
+        ppdo->has_stream_name ? ppdo->stream_name : g->dev->id,
         PA_STREAM_RECORD,
         ppdo->has_name ? ppdo->name : NULL,
         &ss,
diff --git a/qapi/audio.json b/qapi/audio.json
index 9fefdf5186..e45218f081 100644
--- a/qapi/audio.json
+++ b/qapi/audio.json
@@ -206,6 +206,11 @@
 #
 # @name: name of the sink/source to use
 #
+# @stream-name: name of the PulseAudio stream created by qemu.  Can be
+#               used to identify the stream in PulseAudio when you
+#               create multiple PulseAudio devices or run multiple qemu
+#               instances (default: audiodev's id, since 4.2)
+#
 # @latency: latency you want PulseAudio to achieve in microseconds
 #           (default 15000)
 #
@@ -215,6 +220,7 @@
   'base': 'AudiodevPerDirectionOptions',
   'data': {
     '*name': 'str',
+    '*stream-name': 'str',
     '*latency': 'uint32' } }
 
 ##
-- 
2.23.0

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

[Markus Armbruster]

"Kővágó, Zoltán" <dirty.ice.hu@gmail.com> writes:

> This can be used to identify stream in tools like pavucontrol when one
> creates multiple -audiodevs or runs multiple qemu instances.
>
> Signed-off-by: Kővágó, Zoltán <DirtY.iCE.hu@gmail.com>
> ---
>  audio/paaudio.c | 4 ++--
>  qapi/audio.json | 6 ++++++
>  2 files changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/audio/paaudio.c b/audio/paaudio.c
> index 19b7e39092..d9cff9a2be 100644
> --- a/audio/paaudio.c
> +++ b/audio/paaudio.c
> @@ -563,7 +563,7 @@ static int qpa_init_out(HWVoiceOut *hw, struct audsettings *as,
>  
>      pa->stream = qpa_simple_new (
>          c,
> -        g->dev->id,
> +        ppdo->has_stream_name ? ppdo->stream_name : g->dev->id,
>          PA_STREAM_PLAYBACK,
>          ppdo->has_name ? ppdo->name : NULL,
>          &ss,
> @@ -631,7 +631,7 @@ static int qpa_init_in(HWVoiceIn *hw, struct audsettings *as, void *drv_opaque)
>  
>      pa->stream = qpa_simple_new (
>          c,
> -        g->dev->id,
> +        ppdo->has_stream_name ? ppdo->stream_name : g->dev->id,
>          PA_STREAM_RECORD,
>          ppdo->has_name ? ppdo->name : NULL,
>          &ss,
> diff --git a/qapi/audio.json b/qapi/audio.json
> index 9fefdf5186..e45218f081 100644
> --- a/qapi/audio.json
> +++ b/qapi/audio.json
> @@ -206,6 +206,11 @@
>  #
>  # @name: name of the sink/source to use
>  #
> +# @stream-name: name of the PulseAudio stream created by qemu.  Can be
> +#               used to identify the stream in PulseAudio when you
> +#               create multiple PulseAudio devices or run multiple qemu
> +#               instances (default: audiodev's id, since 4.2)
> +#
>  # @latency: latency you want PulseAudio to achieve in microseconds
>  #           (default 15000)
>  #
> @@ -215,6 +220,7 @@
>    'base': 'AudiodevPerDirectionOptions',
>    'data': {
>      '*name': 'str',
> +    '*stream-name': 'str',
>      '*latency': 'uint32' } }
>  
>  ##

QAPI schema
Acked-by: Markus Armbruster <armbru@redhat.com>

Re: [Qemu-devel] [PATCH 0/3] Audio: misc fixes for "Audio 20190821 patches", part two

[Peter Maydell]

Hi -- not sure what's happened with this patchset, but
can we at least get the first patch (which fixes some
coverity errors) in, even if the other two need more
consideration?

(cc'ing Gerd as audio maintainer.)

thanks
-- PMM


On Wed, 11 Sep 2019 at 00:27, Kővágó, Zoltán <dirty.ice.hu@gmail.com> wrote:
>
> Hi,
>
> This series contains some random fixes for the "Audio 20190821 patches":
> a coverity bugfix and pulseaudio connection/stream names fix.
>
> Since there wasn't a clear consensus about naming the pa streams, I've
> split it into two commits: the first time just uses the audiodev id
> unconditionally, while the last commit adds a new option to qapi to
> override it.  This way we can easily drop the last commit if it turns
> out to be unnecessary.
>
> Regards,
> Zoltan
>
>
> Kővágó, Zoltán (3):
>   audio: fix parameter dereference before NULL check
>   audio: paaudio: fix connection and stream name
>   audio: paaudio: ability to specify stream name
>
>  audio/audio_template.h | 7 +++++--
>  audio/paaudio.c        | 9 ++++++---
>  qapi/audio.json        | 6 ++++++
>  3 files changed, 17 insertions(+), 5 deletions(-)