diff mbox

[15/25] paaudio: do not create multiple connections to the same server

Message ID 48dee3b2d117f59d62cd8e6d4adf735cb2ee2da7.1438884611.git.DirtY.iCE.hu@gmail.com
State New
Headers show

Commit Message

=?UTF-8?B?Wm9sdMOhbiBLxZF2w6Fnw7M=?= Aug. 6, 2015, 6:28 p.m. UTC
Signed-off-by: Kővágó, Zoltán <DirtY.iCE.hu@gmail.com>
---
 audio/paaudio.c | 301 ++++++++++++++++++++++++++++++++------------------------
 1 file changed, 175 insertions(+), 126 deletions(-)

Comments

Marc-André Lureau Aug. 20, 2015, 7:38 p.m. UTC | #1
Hi

On Thu, Aug 6, 2015 at 8:28 PM, Kővágó, Zoltán <dirty.ice.hu@gmail.com> wrote:
> Signed-off-by: Kővágó, Zoltán <DirtY.iCE.hu@gmail.com>
> ---
>  audio/paaudio.c | 301 ++++++++++++++++++++++++++++++++------------------------
>  1 file changed, 175 insertions(+), 126 deletions(-)
>
> diff --git a/audio/paaudio.c b/audio/paaudio.c
> index a53aaf6..e3b8207 100644
> --- a/audio/paaudio.c
> +++ b/audio/paaudio.c
> @@ -9,10 +9,21 @@
>  #include "audio_int.h"
>  #include "audio_pt_int.h"
>
> -typedef struct {
> -    Audiodev *dev;
> +typedef struct PAConnection {
> +    char *server;
> +    int refcount;
> +    QTAILQ_ENTRY(PAConnection) list;
> +
>      pa_threaded_mainloop *mainloop;
>      pa_context *context;
> +} PAConnection;
> +
> +static QTAILQ_HEAD(PAConnectionHead, PAConnection) pa_conns =
> +    QTAILQ_HEAD_INITIALIZER(pa_conns);
> +
> +typedef struct {
> +    Audiodev *dev;
> +    PAConnection *conn;
>  } paaudio;
>
>  typedef struct {
> @@ -43,7 +54,7 @@ typedef struct {
>      int samples;
>  } PAVoiceIn;
>
> -static void qpa_audio_fini(void *opaque);
> +static void qpa_conn_fini(PAConnection *c);
>
>  static void GCC_FMT_ATTR (2, 3) qpa_logerr (int err, const char *fmt, ...)
>  {
> @@ -106,11 +117,11 @@ static inline int PA_STREAM_IS_GOOD(pa_stream_state_t x)
>
>  static int qpa_simple_read (PAVoiceIn *p, void *data, size_t length, int *rerror)
>  {
> -    paaudio *g = p->g;
> +    PAConnection *c = p->g->conn;
>
> -    pa_threaded_mainloop_lock (g->mainloop);
> +    pa_threaded_mainloop_lock(c->mainloop);
>
> -    CHECK_DEAD_GOTO (g, p->stream, rerror, unlock_and_fail);
> +    CHECK_DEAD_GOTO(c, p->stream, rerror, unlock_and_fail);
>
>      while (length > 0) {
>          size_t l;
> @@ -119,11 +130,11 @@ static int qpa_simple_read (PAVoiceIn *p, void *data, size_t length, int *rerror
>              int r;
>
>              r = pa_stream_peek (p->stream, &p->read_data, &p->read_length);
> -            CHECK_SUCCESS_GOTO (g, rerror, r == 0, unlock_and_fail);
> +            CHECK_SUCCESS_GOTO(c, rerror, r == 0, unlock_and_fail);
>
>              if (!p->read_data) {
> -                pa_threaded_mainloop_wait (g->mainloop);
> -                CHECK_DEAD_GOTO (g, p->stream, rerror, unlock_and_fail);
> +                pa_threaded_mainloop_wait(c->mainloop);
> +                CHECK_DEAD_GOTO(c, p->stream, rerror, unlock_and_fail);
>              } else {
>                  p->read_index = 0;
>              }
> @@ -146,53 +157,53 @@ static int qpa_simple_read (PAVoiceIn *p, void *data, size_t length, int *rerror
>              p->read_length = 0;
>              p->read_index = 0;
>
> -            CHECK_SUCCESS_GOTO (g, rerror, r == 0, unlock_and_fail);
> +            CHECK_SUCCESS_GOTO(c, rerror, r == 0, unlock_and_fail);
>          }
>      }
>
> -    pa_threaded_mainloop_unlock (g->mainloop);
> +    pa_threaded_mainloop_unlock(c->mainloop);
>      return 0;
>
>  unlock_and_fail:
> -    pa_threaded_mainloop_unlock (g->mainloop);
> +    pa_threaded_mainloop_unlock(c->mainloop);
>      return -1;
>  }
>
>  static int qpa_simple_write (PAVoiceOut *p, const void *data, size_t length, int *rerror)
>  {
> -    paaudio *g = p->g;
> +    PAConnection *c = p->g->conn;
>
> -    pa_threaded_mainloop_lock (g->mainloop);
> +    pa_threaded_mainloop_lock(c->mainloop);
>
> -    CHECK_DEAD_GOTO (g, p->stream, rerror, unlock_and_fail);
> +    CHECK_DEAD_GOTO(c, p->stream, rerror, unlock_and_fail);
>
>      while (length > 0) {
>          size_t l;
>          int r;
>
>          while (!(l = pa_stream_writable_size (p->stream))) {
> -            pa_threaded_mainloop_wait (g->mainloop);
> -            CHECK_DEAD_GOTO (g, p->stream, rerror, unlock_and_fail);
> +            pa_threaded_mainloop_wait(c->mainloop);
> +            CHECK_DEAD_GOTO(c, p->stream, rerror, unlock_and_fail);
>          }
>
> -        CHECK_SUCCESS_GOTO (g, rerror, l != (size_t) -1, unlock_and_fail);
> +        CHECK_SUCCESS_GOTO(c, rerror, l != (size_t) -1, unlock_and_fail);
>
>          if (l > length) {
>              l = length;
>          }
>
>          r = pa_stream_write (p->stream, data, l, NULL, 0LL, PA_SEEK_RELATIVE);
> -        CHECK_SUCCESS_GOTO (g, rerror, r >= 0, unlock_and_fail);
> +        CHECK_SUCCESS_GOTO(c, rerror, r >= 0, unlock_and_fail);
>
>          data = (const uint8_t *) data + l;
>          length -= l;
>      }
>
> -    pa_threaded_mainloop_unlock (g->mainloop);
> +    pa_threaded_mainloop_unlock(c->mainloop);
>      return 0;
>
>  unlock_and_fail:
> -    pa_threaded_mainloop_unlock (g->mainloop);
> +    pa_threaded_mainloop_unlock(c->mainloop);
>      return -1;
>  }
>
> @@ -430,13 +441,13 @@ static AudioFormat pa_to_audfmt (pa_sample_format_t fmt, int *endianness)
>
>  static void context_state_cb (pa_context *c, void *userdata)
>  {
> -    paaudio *g = userdata;
> +    PAConnection *conn = userdata;
>
>      switch (pa_context_get_state(c)) {
>      case PA_CONTEXT_READY:
>      case PA_CONTEXT_TERMINATED:
>      case PA_CONTEXT_FAILED:
> -        pa_threaded_mainloop_signal (g->mainloop, 0);
> +        pa_threaded_mainloop_signal(conn->mainloop, 0);
>          break;
>
>      case PA_CONTEXT_UNCONNECTED:
> @@ -449,14 +460,14 @@ static void context_state_cb (pa_context *c, void *userdata)
>
>  static void stream_state_cb (pa_stream *s, void * userdata)
>  {
> -    paaudio *g = userdata;
> +    PAConnection *c = userdata;
>
>      switch (pa_stream_get_state (s)) {
>
>      case PA_STREAM_READY:
>      case PA_STREAM_FAILED:
>      case PA_STREAM_TERMINATED:
> -        pa_threaded_mainloop_signal (g->mainloop, 0);
> +        pa_threaded_mainloop_signal(c->mainloop, 0);
>          break;
>
>      case PA_STREAM_UNCONNECTED:
> @@ -467,13 +478,13 @@ static void stream_state_cb (pa_stream *s, void * userdata)
>
>  static void stream_request_cb (pa_stream *s, size_t length, void *userdata)
>  {
> -    paaudio *g = userdata;
> +    PAConnection *c = userdata;
>
> -    pa_threaded_mainloop_signal (g->mainloop, 0);
> +    pa_threaded_mainloop_signal(c->mainloop, 0);
>  }
>
>  static pa_stream *qpa_simple_new (
> -        paaudio *g,
> +        PAConnection *c,
>          const char *name,
>          pa_stream_direction_t dir,
>          const char *dev,
> @@ -484,50 +495,52 @@ static pa_stream *qpa_simple_new (
>  {
>      int r;
>      pa_stream *stream;
> +    pa_stream_flags_t flags;
>
> -    pa_threaded_mainloop_lock (g->mainloop);
> +    pa_threaded_mainloop_lock(c->mainloop);
>
> -    stream = pa_stream_new (g->context, name, ss, map);
> +    stream = pa_stream_new(c->context, name, ss, map);
>      if (!stream) {
>          goto fail;
>      }
>
> -    pa_stream_set_state_callback (stream, stream_state_cb, g);
> -    pa_stream_set_read_callback (stream, stream_request_cb, g);
> -    pa_stream_set_write_callback (stream, stream_request_cb, g);
> +    pa_stream_set_state_callback (stream, stream_state_cb, c);
> +    pa_stream_set_read_callback (stream, stream_request_cb, c);
> +    pa_stream_set_write_callback (stream, stream_request_cb, c);
> +
> +    flags =
> +        PA_STREAM_INTERPOLATE_TIMING
> +#ifdef PA_STREAM_ADJUST_LATENCY
> +        |PA_STREAM_ADJUST_LATENCY
> +#endif
> +        |PA_STREAM_AUTO_TIMING_UPDATE;
> +    if (dev) {
> +        /* don't move the stream if the user specified a sink/source */
> +        flags |= PA_STREAM_DONT_MOVE;

This is unrelated, and I don't think it's justified, imho user should
be allowed to move the stream later if needed.

> +    }
>
>      if (dir == PA_STREAM_PLAYBACK) {
> -        r = pa_stream_connect_playback (stream, dev, attr,
> -                                        PA_STREAM_INTERPOLATE_TIMING
> -#ifdef PA_STREAM_ADJUST_LATENCY
> -                                        |PA_STREAM_ADJUST_LATENCY
> -#endif
> -                                        |PA_STREAM_AUTO_TIMING_UPDATE, NULL, NULL);
> +        r = pa_stream_connect_playback(stream, dev, attr, flags, NULL, NULL);
>      } else {
> -        r = pa_stream_connect_record (stream, dev, attr,
> -                                      PA_STREAM_INTERPOLATE_TIMING
> -#ifdef PA_STREAM_ADJUST_LATENCY
> -                                      |PA_STREAM_ADJUST_LATENCY
> -#endif
> -                                      |PA_STREAM_AUTO_TIMING_UPDATE);
> +        r = pa_stream_connect_record(stream, dev, attr, flags);
>      }
>
>      if (r < 0) {
>        goto fail;
>      }
>
> -    pa_threaded_mainloop_unlock (g->mainloop);
> +    pa_threaded_mainloop_unlock(c->mainloop);
>
>      return stream;
>
>  fail:
> -    pa_threaded_mainloop_unlock (g->mainloop);
> +    pa_threaded_mainloop_unlock(c->mainloop);
>
>      if (stream) {
>          pa_stream_unref (stream);
>      }
>
> -    *rerror = pa_context_errno (g->context);
> +    *rerror = pa_context_errno(c->context);
>
>      return NULL;
>  }
> @@ -543,6 +556,7 @@ static int qpa_init_out(HWVoiceOut *hw, struct audsettings *as,
>      paaudio *g = pa->g = drv_opaque;
>      AudiodevPaOptions *popts = g->dev->pa;
>      AudiodevPaPerDirectionOptions *ppdo = popts->sink;
> +    PAConnection *c = g->conn;
>
>      ss.format = audfmt_to_pa (as->fmt, as->endianness);
>      ss.channels = as->nchannels;
> @@ -560,7 +574,7 @@ static int qpa_init_out(HWVoiceOut *hw, struct audsettings *as,
>      obt_as.fmt = pa_to_audfmt (ss.format, &obt_as.endianness);
>
>      pa->stream = qpa_simple_new (
> -        g,
> +        c,
>          "qemu",
>          PA_STREAM_PLAYBACK,
>          ppdo->has_name ? ppdo->name : NULL,
> @@ -612,6 +626,7 @@ static int qpa_init_in(HWVoiceIn *hw, struct audsettings *as, void *drv_opaque)
>      paaudio *g = pa->g = drv_opaque;
>      AudiodevPaOptions *popts = g->dev->pa;
>      AudiodevPaPerDirectionOptions *ppdo = popts->source;
> +    PAConnection *c = g->conn;
>
>      ss.format = audfmt_to_pa (as->fmt, as->endianness);
>      ss.channels = as->nchannels;
> @@ -620,7 +635,7 @@ static int qpa_init_in(HWVoiceIn *hw, struct audsettings *as, void *drv_opaque)
>      obt_as.fmt = pa_to_audfmt (ss.format, &obt_as.endianness);
>
>      pa->stream = qpa_simple_new (
> -        g,
> +        c,
>          "qemu",
>          PA_STREAM_RECORD,
>          ppdo->has_name ? ppdo->name : NULL,
> @@ -708,7 +723,7 @@ static int qpa_ctl_out (HWVoiceOut *hw, int cmd, ...)
>      PAVoiceOut *pa = (PAVoiceOut *) hw;
>      pa_operation *op;
>      pa_cvolume v;
> -    paaudio *g = pa->g;
> +    PAConnection *c = pa->g->conn;
>
>  #ifdef PA_CHECK_VERSION    /* macro is present in 0.9.16+ */
>      pa_cvolume_init (&v);  /* function is present in 0.9.13+ */
> @@ -728,28 +743,28 @@ static int qpa_ctl_out (HWVoiceOut *hw, int cmd, ...)
>              v.values[0] = ((PA_VOLUME_NORM - PA_VOLUME_MUTED) * sw->vol.l) / UINT32_MAX;
>              v.values[1] = ((PA_VOLUME_NORM - PA_VOLUME_MUTED) * sw->vol.r) / UINT32_MAX;
>
> -            pa_threaded_mainloop_lock (g->mainloop);
> +            pa_threaded_mainloop_lock(c->mainloop);
>
> -            op = pa_context_set_sink_input_volume (g->context,
> +            op = pa_context_set_sink_input_volume(c->context,
>                  pa_stream_get_index (pa->stream),
>                  &v, NULL, NULL);
>              if (!op)
> -                qpa_logerr (pa_context_errno (g->context),
> +                qpa_logerr (pa_context_errno(c->context),
>                              "set_sink_input_volume() failed\n");
>              else
>                  pa_operation_unref (op);
>
> -            op = pa_context_set_sink_input_mute (g->context,
> +            op = pa_context_set_sink_input_mute(c->context,
>                  pa_stream_get_index (pa->stream),
>                 sw->vol.mute, NULL, NULL);
>              if (!op) {
> -                qpa_logerr (pa_context_errno (g->context),
> +                qpa_logerr (pa_context_errno(c->context),
>                              "set_sink_input_mute() failed\n");
>              } else {
>                  pa_operation_unref (op);
>              }
>
> -            pa_threaded_mainloop_unlock (g->mainloop);
> +            pa_threaded_mainloop_unlock(c->mainloop);
>          }
>      }
>      return 0;
> @@ -760,7 +775,7 @@ static int qpa_ctl_in (HWVoiceIn *hw, int cmd, ...)
>      PAVoiceIn *pa = (PAVoiceIn *) hw;
>      pa_operation *op;
>      pa_cvolume v;
> -    paaudio *g = pa->g;
> +    PAConnection *c = pa->g->conn;
>
>  #ifdef PA_CHECK_VERSION
>      pa_cvolume_init (&v);
> @@ -780,123 +795,157 @@ static int qpa_ctl_in (HWVoiceIn *hw, int cmd, ...)
>              v.values[0] = ((PA_VOLUME_NORM - PA_VOLUME_MUTED) * sw->vol.l) / UINT32_MAX;
>              v.values[1] = ((PA_VOLUME_NORM - PA_VOLUME_MUTED) * sw->vol.r) / UINT32_MAX;
>
> -            pa_threaded_mainloop_lock (g->mainloop);
> +            pa_threaded_mainloop_lock(c->mainloop);
>
>              /* FIXME: use the upcoming "set_source_output_{volume,mute}" */
> -            op = pa_context_set_source_volume_by_index (g->context,
> +            op = pa_context_set_source_volume_by_index(c->context,
>                  pa_stream_get_device_index (pa->stream),
>                  &v, NULL, NULL);
>              if (!op) {
> -                qpa_logerr (pa_context_errno (g->context),
> +                qpa_logerr (pa_context_errno(c->context),
>                              "set_source_volume() failed\n");
>              } else {
>                  pa_operation_unref(op);
>              }
>
> -            op = pa_context_set_source_mute_by_index (g->context,
> +            op = pa_context_set_source_mute_by_index(c->context,
>                  pa_stream_get_index (pa->stream),
>                  sw->vol.mute, NULL, NULL);
>              if (!op) {
> -                qpa_logerr (pa_context_errno (g->context),
> +                qpa_logerr (pa_context_errno(c->context),
>                              "set_source_mute() failed\n");
>              } else {
>                  pa_operation_unref (op);
>              }
>
> -            pa_threaded_mainloop_unlock (g->mainloop);
> +            pa_threaded_mainloop_unlock(c->mainloop);
>          }
>      }
>      return 0;
>  }
>
>  /* common */
> +static void *qpa_conn_init(const char *server)
> +{
> +    PAConnection *c = g_malloc0(sizeof(PAConnection));
> +    QTAILQ_INSERT_TAIL(&pa_conns, c, list);
> +
> +    c->mainloop = pa_threaded_mainloop_new();
> +    if (!c->mainloop) {
> +        goto fail;
> +    }
> +
> +    c->context = pa_context_new(pa_threaded_mainloop_get_api(c->mainloop),
> +                                server);
> +    if (!c->context) {
> +        goto fail;
> +    }
> +
> +    pa_context_set_state_callback(c->context, context_state_cb, c);
> +
> +    if (pa_context_connect(c->context, server, 0, NULL) < 0) {
> +        qpa_logerr(pa_context_errno(c->context),
> +                   "pa_context_connect() failed\n");
> +        goto fail;
> +    }
> +
> +    pa_threaded_mainloop_lock(c->mainloop);
> +
> +    if (pa_threaded_mainloop_start(c->mainloop) < 0) {
> +        goto unlock_and_fail;
> +    }
> +
> +    for (;;) {
> +        pa_context_state_t state;
> +
> +        state = pa_context_get_state(c->context);
> +
> +        if (state == PA_CONTEXT_READY) {
> +            break;
> +        }
> +
> +        if (!PA_CONTEXT_IS_GOOD (state)) {
> +            qpa_logerr(pa_context_errno(c->context),
> +                       "Wrong context state\n");
> +            goto unlock_and_fail;
> +        }
> +
> +        /* Wait until the context is ready */
> +        pa_threaded_mainloop_wait(c->mainloop);
> +    }
> +
> +    pa_threaded_mainloop_unlock(c->mainloop);
> +    return c;
> +
> +unlock_and_fail:
> +    pa_threaded_mainloop_unlock(c->mainloop);
> +fail:
> +    AUD_log (AUDIO_CAP, "Failed to initialize PA context");
> +    qpa_conn_fini(c);
> +    return NULL;
> +}
> +
>  static void *qpa_audio_init(Audiodev *dev)
>  {
>      paaudio *g;
>      AudiodevPaOptions *popts;
>      const char *server;
> +    PAConnection *c;
>
>      assert(dev->kind == AUDIODEV_DRIVER_PA);
>
> -    g = g_malloc(sizeof(paaudio));
> +    g = g_malloc0(sizeof(paaudio));
>      popts = dev->pa;
>      server = popts->has_server ? popts->server : NULL;
>
>      g->dev = dev;
> -    g->mainloop = NULL;
> -    g->context = NULL;
>
> -    g->mainloop = pa_threaded_mainloop_new ();
> -    if (!g->mainloop) {
> -        goto fail;
> -    }
> -
> -    g->context = pa_context_new (pa_threaded_mainloop_get_api (g->mainloop),
> -                                 server);
> -    if (!g->context) {
> -        goto fail;
> -    }
> -
> -    pa_context_set_state_callback (g->context, context_state_cb, g);
> -
> -    if (pa_context_connect (g->context, server, 0, NULL) < 0) {
> -        qpa_logerr (pa_context_errno (g->context),
> -                    "pa_context_connect() failed\n");
> -        goto fail;
> -    }
> -
> -    pa_threaded_mainloop_lock (g->mainloop);
> -
> -    if (pa_threaded_mainloop_start (g->mainloop) < 0) {
> -        goto unlock_and_fail;
> -    }
> -
> -    for (;;) {
> -        pa_context_state_t state;
> -
> -        state = pa_context_get_state (g->context);
> -
> -        if (state == PA_CONTEXT_READY) {
> +    QTAILQ_FOREACH(c, &pa_conns, list) {
> +        if (server == NULL || c->server == NULL ?
> +            server == c->server :
> +            strcmp(server, c->server) == 0) {
> +            g->conn = c;
>              break;
>          }
> -
> -        if (!PA_CONTEXT_IS_GOOD (state)) {
> -            qpa_logerr (pa_context_errno (g->context),
> -                        "Wrong context state\n");
> -            goto unlock_and_fail;
> -        }
> -
> -        /* Wait until the context is ready */
> -        pa_threaded_mainloop_wait (g->mainloop);
> +    }
> +    if (!g->conn) {
> +        g->conn = qpa_conn_init(server);
> +    }
> +    if (!g->conn) {
> +        g_free(g);
> +        return NULL;
>      }
>
> -    pa_threaded_mainloop_unlock (g->mainloop);
> -
> +    ++g->conn->refcount;
>      return g;
> +}
>
> -unlock_and_fail:
> -    pa_threaded_mainloop_unlock (g->mainloop);
> -fail:
> -    AUD_log (AUDIO_CAP, "Failed to initialize PA context");
> -    qpa_audio_fini(g);
> -    return NULL;
> +static void qpa_conn_fini(PAConnection *c)
> +{
> +    if (c->mainloop) {
> +        pa_threaded_mainloop_stop(c->mainloop);
> +    }
> +
> +    if (c->context) {
> +        pa_context_disconnect(c->context);
> +        pa_context_unref(c->context);
> +    }
> +
> +    if (c->mainloop) {
> +        pa_threaded_mainloop_free(c->mainloop);
> +    }
> +
> +    QTAILQ_REMOVE(&pa_conns, c, list);
> +    g_free(c);
>  }
>
>  static void qpa_audio_fini (void *opaque)
>  {
>      paaudio *g = opaque;
> +    PAConnection *c = g->conn;
>
> -    if (g->mainloop) {
> -        pa_threaded_mainloop_stop (g->mainloop);
> -    }
> -
> -    if (g->context) {
> -        pa_context_disconnect (g->context);
> -        pa_context_unref (g->context);
> -    }
> -
> -    if (g->mainloop) {
> -        pa_threaded_mainloop_free (g->mainloop);
> +    if (--c->refcount == 0) {
> +        qpa_conn_fini(c);
>      }
>
>      g_free(g);
> --
> 2.4.5
>
>

otherwise

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
=?UTF-8?B?Wm9sdMOhbiBLxZF2w6Fnw7M=?= Aug. 21, 2015, 12:41 p.m. UTC | #2
Hi,

2015-08-20 21:38 keltezéssel, Marc-André Lureau írta:
[snip]
>> +    flags =
>> +        PA_STREAM_INTERPOLATE_TIMING
>> +#ifdef PA_STREAM_ADJUST_LATENCY
>> +        |PA_STREAM_ADJUST_LATENCY
>> +#endif
>> +        |PA_STREAM_AUTO_TIMING_UPDATE;
>> +    if (dev) {
>> +        /* don't move the stream if the user specified a sink/source */
>> +        flags |= PA_STREAM_DONT_MOVE;
>
> This is unrelated, and I don't think it's justified, imho user should
> be allowed to move the stream later if needed.

True, I will remove it from this commit.  But not sure if I want to 
remove it completely, because it looks like pulseaudio has a tendency to 
automatically move the stream on connect, even if you explicitly specify 
a sink/source when connecting.  (And the problem is that all streams 
appear under the name qemu in pulseaudio, so once the user moves one 
stream, pulseaudio will assume all streams subsequently created by qemu 
are the same on, and move them too.  Maybe we shold add some extra 
options to the pa backend?)
Marc-André Lureau Aug. 21, 2015, 2:36 p.m. UTC | #3
On Fri, Aug 21, 2015 at 2:41 PM, Kővágó Zoltán <dirty.ice.hu@gmail.com> wrote:
> True, I will remove it from this commit.  But not sure if I want to remove
> it completely, because it looks like pulseaudio has a tendency to
> automatically move the stream on connect, even if you explicitly specify a
> sink/source when connecting.  (And the problem is that all streams appear
> under the name qemu in pulseaudio, so once the user moves one stream,
> pulseaudio will assume all streams subsequently created by qemu are the same
> on, and move them too.  Maybe we shold add some extra options to the pa
> backend?)


In theory, stream-restore shouldn't move streams if the sink is
already specified, see sink_input_new_hook_callback() in PA
module-stream-restore.c, but there might be something else applying
other policies? Yes, qemu could provide more context to the streams
with additional properties, which could help the policy. I think this
can be discussed seperately.
diff mbox

Patch

diff --git a/audio/paaudio.c b/audio/paaudio.c
index a53aaf6..e3b8207 100644
--- a/audio/paaudio.c
+++ b/audio/paaudio.c
@@ -9,10 +9,21 @@ 
 #include "audio_int.h"
 #include "audio_pt_int.h"
 
-typedef struct {
-    Audiodev *dev;
+typedef struct PAConnection {
+    char *server;
+    int refcount;
+    QTAILQ_ENTRY(PAConnection) list;
+
     pa_threaded_mainloop *mainloop;
     pa_context *context;
+} PAConnection;
+
+static QTAILQ_HEAD(PAConnectionHead, PAConnection) pa_conns =
+    QTAILQ_HEAD_INITIALIZER(pa_conns);
+
+typedef struct {
+    Audiodev *dev;
+    PAConnection *conn;
 } paaudio;
 
 typedef struct {
@@ -43,7 +54,7 @@  typedef struct {
     int samples;
 } PAVoiceIn;
 
-static void qpa_audio_fini(void *opaque);
+static void qpa_conn_fini(PAConnection *c);
 
 static void GCC_FMT_ATTR (2, 3) qpa_logerr (int err, const char *fmt, ...)
 {
@@ -106,11 +117,11 @@  static inline int PA_STREAM_IS_GOOD(pa_stream_state_t x)
 
 static int qpa_simple_read (PAVoiceIn *p, void *data, size_t length, int *rerror)
 {
-    paaudio *g = p->g;
+    PAConnection *c = p->g->conn;
 
-    pa_threaded_mainloop_lock (g->mainloop);
+    pa_threaded_mainloop_lock(c->mainloop);
 
-    CHECK_DEAD_GOTO (g, p->stream, rerror, unlock_and_fail);
+    CHECK_DEAD_GOTO(c, p->stream, rerror, unlock_and_fail);
 
     while (length > 0) {
         size_t l;
@@ -119,11 +130,11 @@  static int qpa_simple_read (PAVoiceIn *p, void *data, size_t length, int *rerror
             int r;
 
             r = pa_stream_peek (p->stream, &p->read_data, &p->read_length);
-            CHECK_SUCCESS_GOTO (g, rerror, r == 0, unlock_and_fail);
+            CHECK_SUCCESS_GOTO(c, rerror, r == 0, unlock_and_fail);
 
             if (!p->read_data) {
-                pa_threaded_mainloop_wait (g->mainloop);
-                CHECK_DEAD_GOTO (g, p->stream, rerror, unlock_and_fail);
+                pa_threaded_mainloop_wait(c->mainloop);
+                CHECK_DEAD_GOTO(c, p->stream, rerror, unlock_and_fail);
             } else {
                 p->read_index = 0;
             }
@@ -146,53 +157,53 @@  static int qpa_simple_read (PAVoiceIn *p, void *data, size_t length, int *rerror
             p->read_length = 0;
             p->read_index = 0;
 
-            CHECK_SUCCESS_GOTO (g, rerror, r == 0, unlock_and_fail);
+            CHECK_SUCCESS_GOTO(c, rerror, r == 0, unlock_and_fail);
         }
     }
 
-    pa_threaded_mainloop_unlock (g->mainloop);
+    pa_threaded_mainloop_unlock(c->mainloop);
     return 0;
 
 unlock_and_fail:
-    pa_threaded_mainloop_unlock (g->mainloop);
+    pa_threaded_mainloop_unlock(c->mainloop);
     return -1;
 }
 
 static int qpa_simple_write (PAVoiceOut *p, const void *data, size_t length, int *rerror)
 {
-    paaudio *g = p->g;
+    PAConnection *c = p->g->conn;
 
-    pa_threaded_mainloop_lock (g->mainloop);
+    pa_threaded_mainloop_lock(c->mainloop);
 
-    CHECK_DEAD_GOTO (g, p->stream, rerror, unlock_and_fail);
+    CHECK_DEAD_GOTO(c, p->stream, rerror, unlock_and_fail);
 
     while (length > 0) {
         size_t l;
         int r;
 
         while (!(l = pa_stream_writable_size (p->stream))) {
-            pa_threaded_mainloop_wait (g->mainloop);
-            CHECK_DEAD_GOTO (g, p->stream, rerror, unlock_and_fail);
+            pa_threaded_mainloop_wait(c->mainloop);
+            CHECK_DEAD_GOTO(c, p->stream, rerror, unlock_and_fail);
         }
 
-        CHECK_SUCCESS_GOTO (g, rerror, l != (size_t) -1, unlock_and_fail);
+        CHECK_SUCCESS_GOTO(c, rerror, l != (size_t) -1, unlock_and_fail);
 
         if (l > length) {
             l = length;
         }
 
         r = pa_stream_write (p->stream, data, l, NULL, 0LL, PA_SEEK_RELATIVE);
-        CHECK_SUCCESS_GOTO (g, rerror, r >= 0, unlock_and_fail);
+        CHECK_SUCCESS_GOTO(c, rerror, r >= 0, unlock_and_fail);
 
         data = (const uint8_t *) data + l;
         length -= l;
     }
 
-    pa_threaded_mainloop_unlock (g->mainloop);
+    pa_threaded_mainloop_unlock(c->mainloop);
     return 0;
 
 unlock_and_fail:
-    pa_threaded_mainloop_unlock (g->mainloop);
+    pa_threaded_mainloop_unlock(c->mainloop);
     return -1;
 }
 
@@ -430,13 +441,13 @@  static AudioFormat pa_to_audfmt (pa_sample_format_t fmt, int *endianness)
 
 static void context_state_cb (pa_context *c, void *userdata)
 {
-    paaudio *g = userdata;
+    PAConnection *conn = userdata;
 
     switch (pa_context_get_state(c)) {
     case PA_CONTEXT_READY:
     case PA_CONTEXT_TERMINATED:
     case PA_CONTEXT_FAILED:
-        pa_threaded_mainloop_signal (g->mainloop, 0);
+        pa_threaded_mainloop_signal(conn->mainloop, 0);
         break;
 
     case PA_CONTEXT_UNCONNECTED:
@@ -449,14 +460,14 @@  static void context_state_cb (pa_context *c, void *userdata)
 
 static void stream_state_cb (pa_stream *s, void * userdata)
 {
-    paaudio *g = userdata;
+    PAConnection *c = userdata;
 
     switch (pa_stream_get_state (s)) {
 
     case PA_STREAM_READY:
     case PA_STREAM_FAILED:
     case PA_STREAM_TERMINATED:
-        pa_threaded_mainloop_signal (g->mainloop, 0);
+        pa_threaded_mainloop_signal(c->mainloop, 0);
         break;
 
     case PA_STREAM_UNCONNECTED:
@@ -467,13 +478,13 @@  static void stream_state_cb (pa_stream *s, void * userdata)
 
 static void stream_request_cb (pa_stream *s, size_t length, void *userdata)
 {
-    paaudio *g = userdata;
+    PAConnection *c = userdata;
 
-    pa_threaded_mainloop_signal (g->mainloop, 0);
+    pa_threaded_mainloop_signal(c->mainloop, 0);
 }
 
 static pa_stream *qpa_simple_new (
-        paaudio *g,
+        PAConnection *c,
         const char *name,
         pa_stream_direction_t dir,
         const char *dev,
@@ -484,50 +495,52 @@  static pa_stream *qpa_simple_new (
 {
     int r;
     pa_stream *stream;
+    pa_stream_flags_t flags;
 
-    pa_threaded_mainloop_lock (g->mainloop);
+    pa_threaded_mainloop_lock(c->mainloop);
 
-    stream = pa_stream_new (g->context, name, ss, map);
+    stream = pa_stream_new(c->context, name, ss, map);
     if (!stream) {
         goto fail;
     }
 
-    pa_stream_set_state_callback (stream, stream_state_cb, g);
-    pa_stream_set_read_callback (stream, stream_request_cb, g);
-    pa_stream_set_write_callback (stream, stream_request_cb, g);
+    pa_stream_set_state_callback (stream, stream_state_cb, c);
+    pa_stream_set_read_callback (stream, stream_request_cb, c);
+    pa_stream_set_write_callback (stream, stream_request_cb, c);
+
+    flags =
+        PA_STREAM_INTERPOLATE_TIMING
+#ifdef PA_STREAM_ADJUST_LATENCY
+        |PA_STREAM_ADJUST_LATENCY
+#endif
+        |PA_STREAM_AUTO_TIMING_UPDATE;
+    if (dev) {
+        /* don't move the stream if the user specified a sink/source */
+        flags |= PA_STREAM_DONT_MOVE;
+    }
 
     if (dir == PA_STREAM_PLAYBACK) {
-        r = pa_stream_connect_playback (stream, dev, attr,
-                                        PA_STREAM_INTERPOLATE_TIMING
-#ifdef PA_STREAM_ADJUST_LATENCY
-                                        |PA_STREAM_ADJUST_LATENCY
-#endif
-                                        |PA_STREAM_AUTO_TIMING_UPDATE, NULL, NULL);
+        r = pa_stream_connect_playback(stream, dev, attr, flags, NULL, NULL);
     } else {
-        r = pa_stream_connect_record (stream, dev, attr,
-                                      PA_STREAM_INTERPOLATE_TIMING
-#ifdef PA_STREAM_ADJUST_LATENCY
-                                      |PA_STREAM_ADJUST_LATENCY
-#endif
-                                      |PA_STREAM_AUTO_TIMING_UPDATE);
+        r = pa_stream_connect_record(stream, dev, attr, flags);
     }
 
     if (r < 0) {
       goto fail;
     }
 
-    pa_threaded_mainloop_unlock (g->mainloop);
+    pa_threaded_mainloop_unlock(c->mainloop);
 
     return stream;
 
 fail:
-    pa_threaded_mainloop_unlock (g->mainloop);
+    pa_threaded_mainloop_unlock(c->mainloop);
 
     if (stream) {
         pa_stream_unref (stream);
     }
 
-    *rerror = pa_context_errno (g->context);
+    *rerror = pa_context_errno(c->context);
 
     return NULL;
 }
@@ -543,6 +556,7 @@  static int qpa_init_out(HWVoiceOut *hw, struct audsettings *as,
     paaudio *g = pa->g = drv_opaque;
     AudiodevPaOptions *popts = g->dev->pa;
     AudiodevPaPerDirectionOptions *ppdo = popts->sink;
+    PAConnection *c = g->conn;
 
     ss.format = audfmt_to_pa (as->fmt, as->endianness);
     ss.channels = as->nchannels;
@@ -560,7 +574,7 @@  static int qpa_init_out(HWVoiceOut *hw, struct audsettings *as,
     obt_as.fmt = pa_to_audfmt (ss.format, &obt_as.endianness);
 
     pa->stream = qpa_simple_new (
-        g,
+        c,
         "qemu",
         PA_STREAM_PLAYBACK,
         ppdo->has_name ? ppdo->name : NULL,
@@ -612,6 +626,7 @@  static int qpa_init_in(HWVoiceIn *hw, struct audsettings *as, void *drv_opaque)
     paaudio *g = pa->g = drv_opaque;
     AudiodevPaOptions *popts = g->dev->pa;
     AudiodevPaPerDirectionOptions *ppdo = popts->source;
+    PAConnection *c = g->conn;
 
     ss.format = audfmt_to_pa (as->fmt, as->endianness);
     ss.channels = as->nchannels;
@@ -620,7 +635,7 @@  static int qpa_init_in(HWVoiceIn *hw, struct audsettings *as, void *drv_opaque)
     obt_as.fmt = pa_to_audfmt (ss.format, &obt_as.endianness);
 
     pa->stream = qpa_simple_new (
-        g,
+        c,
         "qemu",
         PA_STREAM_RECORD,
         ppdo->has_name ? ppdo->name : NULL,
@@ -708,7 +723,7 @@  static int qpa_ctl_out (HWVoiceOut *hw, int cmd, ...)
     PAVoiceOut *pa = (PAVoiceOut *) hw;
     pa_operation *op;
     pa_cvolume v;
-    paaudio *g = pa->g;
+    PAConnection *c = pa->g->conn;
 
 #ifdef PA_CHECK_VERSION    /* macro is present in 0.9.16+ */
     pa_cvolume_init (&v);  /* function is present in 0.9.13+ */
@@ -728,28 +743,28 @@  static int qpa_ctl_out (HWVoiceOut *hw, int cmd, ...)
             v.values[0] = ((PA_VOLUME_NORM - PA_VOLUME_MUTED) * sw->vol.l) / UINT32_MAX;
             v.values[1] = ((PA_VOLUME_NORM - PA_VOLUME_MUTED) * sw->vol.r) / UINT32_MAX;
 
-            pa_threaded_mainloop_lock (g->mainloop);
+            pa_threaded_mainloop_lock(c->mainloop);
 
-            op = pa_context_set_sink_input_volume (g->context,
+            op = pa_context_set_sink_input_volume(c->context,
                 pa_stream_get_index (pa->stream),
                 &v, NULL, NULL);
             if (!op)
-                qpa_logerr (pa_context_errno (g->context),
+                qpa_logerr (pa_context_errno(c->context),
                             "set_sink_input_volume() failed\n");
             else
                 pa_operation_unref (op);
 
-            op = pa_context_set_sink_input_mute (g->context,
+            op = pa_context_set_sink_input_mute(c->context,
                 pa_stream_get_index (pa->stream),
                sw->vol.mute, NULL, NULL);
             if (!op) {
-                qpa_logerr (pa_context_errno (g->context),
+                qpa_logerr (pa_context_errno(c->context),
                             "set_sink_input_mute() failed\n");
             } else {
                 pa_operation_unref (op);
             }
 
-            pa_threaded_mainloop_unlock (g->mainloop);
+            pa_threaded_mainloop_unlock(c->mainloop);
         }
     }
     return 0;
@@ -760,7 +775,7 @@  static int qpa_ctl_in (HWVoiceIn *hw, int cmd, ...)
     PAVoiceIn *pa = (PAVoiceIn *) hw;
     pa_operation *op;
     pa_cvolume v;
-    paaudio *g = pa->g;
+    PAConnection *c = pa->g->conn;
 
 #ifdef PA_CHECK_VERSION
     pa_cvolume_init (&v);
@@ -780,123 +795,157 @@  static int qpa_ctl_in (HWVoiceIn *hw, int cmd, ...)
             v.values[0] = ((PA_VOLUME_NORM - PA_VOLUME_MUTED) * sw->vol.l) / UINT32_MAX;
             v.values[1] = ((PA_VOLUME_NORM - PA_VOLUME_MUTED) * sw->vol.r) / UINT32_MAX;
 
-            pa_threaded_mainloop_lock (g->mainloop);
+            pa_threaded_mainloop_lock(c->mainloop);
 
             /* FIXME: use the upcoming "set_source_output_{volume,mute}" */
-            op = pa_context_set_source_volume_by_index (g->context,
+            op = pa_context_set_source_volume_by_index(c->context,
                 pa_stream_get_device_index (pa->stream),
                 &v, NULL, NULL);
             if (!op) {
-                qpa_logerr (pa_context_errno (g->context),
+                qpa_logerr (pa_context_errno(c->context),
                             "set_source_volume() failed\n");
             } else {
                 pa_operation_unref(op);
             }
 
-            op = pa_context_set_source_mute_by_index (g->context,
+            op = pa_context_set_source_mute_by_index(c->context,
                 pa_stream_get_index (pa->stream),
                 sw->vol.mute, NULL, NULL);
             if (!op) {
-                qpa_logerr (pa_context_errno (g->context),
+                qpa_logerr (pa_context_errno(c->context),
                             "set_source_mute() failed\n");
             } else {
                 pa_operation_unref (op);
             }
 
-            pa_threaded_mainloop_unlock (g->mainloop);
+            pa_threaded_mainloop_unlock(c->mainloop);
         }
     }
     return 0;
 }
 
 /* common */
+static void *qpa_conn_init(const char *server)
+{
+    PAConnection *c = g_malloc0(sizeof(PAConnection));
+    QTAILQ_INSERT_TAIL(&pa_conns, c, list);
+
+    c->mainloop = pa_threaded_mainloop_new();
+    if (!c->mainloop) {
+        goto fail;
+    }
+
+    c->context = pa_context_new(pa_threaded_mainloop_get_api(c->mainloop),
+                                server);
+    if (!c->context) {
+        goto fail;
+    }
+
+    pa_context_set_state_callback(c->context, context_state_cb, c);
+
+    if (pa_context_connect(c->context, server, 0, NULL) < 0) {
+        qpa_logerr(pa_context_errno(c->context),
+                   "pa_context_connect() failed\n");
+        goto fail;
+    }
+
+    pa_threaded_mainloop_lock(c->mainloop);
+
+    if (pa_threaded_mainloop_start(c->mainloop) < 0) {
+        goto unlock_and_fail;
+    }
+
+    for (;;) {
+        pa_context_state_t state;
+
+        state = pa_context_get_state(c->context);
+
+        if (state == PA_CONTEXT_READY) {
+            break;
+        }
+
+        if (!PA_CONTEXT_IS_GOOD (state)) {
+            qpa_logerr(pa_context_errno(c->context),
+                       "Wrong context state\n");
+            goto unlock_and_fail;
+        }
+
+        /* Wait until the context is ready */
+        pa_threaded_mainloop_wait(c->mainloop);
+    }
+
+    pa_threaded_mainloop_unlock(c->mainloop);
+    return c;
+
+unlock_and_fail:
+    pa_threaded_mainloop_unlock(c->mainloop);
+fail:
+    AUD_log (AUDIO_CAP, "Failed to initialize PA context");
+    qpa_conn_fini(c);
+    return NULL;
+}
+
 static void *qpa_audio_init(Audiodev *dev)
 {
     paaudio *g;
     AudiodevPaOptions *popts;
     const char *server;
+    PAConnection *c;
 
     assert(dev->kind == AUDIODEV_DRIVER_PA);
 
-    g = g_malloc(sizeof(paaudio));
+    g = g_malloc0(sizeof(paaudio));
     popts = dev->pa;
     server = popts->has_server ? popts->server : NULL;
 
     g->dev = dev;
-    g->mainloop = NULL;
-    g->context = NULL;
 
-    g->mainloop = pa_threaded_mainloop_new ();
-    if (!g->mainloop) {
-        goto fail;
-    }
-
-    g->context = pa_context_new (pa_threaded_mainloop_get_api (g->mainloop),
-                                 server);
-    if (!g->context) {
-        goto fail;
-    }
-
-    pa_context_set_state_callback (g->context, context_state_cb, g);
-
-    if (pa_context_connect (g->context, server, 0, NULL) < 0) {
-        qpa_logerr (pa_context_errno (g->context),
-                    "pa_context_connect() failed\n");
-        goto fail;
-    }
-
-    pa_threaded_mainloop_lock (g->mainloop);
-
-    if (pa_threaded_mainloop_start (g->mainloop) < 0) {
-        goto unlock_and_fail;
-    }
-
-    for (;;) {
-        pa_context_state_t state;
-
-        state = pa_context_get_state (g->context);
-
-        if (state == PA_CONTEXT_READY) {
+    QTAILQ_FOREACH(c, &pa_conns, list) {
+        if (server == NULL || c->server == NULL ?
+            server == c->server :
+            strcmp(server, c->server) == 0) {
+            g->conn = c;
             break;
         }
-
-        if (!PA_CONTEXT_IS_GOOD (state)) {
-            qpa_logerr (pa_context_errno (g->context),
-                        "Wrong context state\n");
-            goto unlock_and_fail;
-        }
-
-        /* Wait until the context is ready */
-        pa_threaded_mainloop_wait (g->mainloop);
+    }
+    if (!g->conn) {
+        g->conn = qpa_conn_init(server);
+    }
+    if (!g->conn) {
+        g_free(g);
+        return NULL;
     }
 
-    pa_threaded_mainloop_unlock (g->mainloop);
-
+    ++g->conn->refcount;
     return g;
+}
 
-unlock_and_fail:
-    pa_threaded_mainloop_unlock (g->mainloop);
-fail:
-    AUD_log (AUDIO_CAP, "Failed to initialize PA context");
-    qpa_audio_fini(g);
-    return NULL;
+static void qpa_conn_fini(PAConnection *c)
+{
+    if (c->mainloop) {
+        pa_threaded_mainloop_stop(c->mainloop);
+    }
+
+    if (c->context) {
+        pa_context_disconnect(c->context);
+        pa_context_unref(c->context);
+    }
+
+    if (c->mainloop) {
+        pa_threaded_mainloop_free(c->mainloop);
+    }
+
+    QTAILQ_REMOVE(&pa_conns, c, list);
+    g_free(c);
 }
 
 static void qpa_audio_fini (void *opaque)
 {
     paaudio *g = opaque;
+    PAConnection *c = g->conn;
 
-    if (g->mainloop) {
-        pa_threaded_mainloop_stop (g->mainloop);
-    }
-
-    if (g->context) {
-        pa_context_disconnect (g->context);
-        pa_context_unref (g->context);
-    }
-
-    if (g->mainloop) {
-        pa_threaded_mainloop_free (g->mainloop);
+    if (--c->refcount == 0) {
+        qpa_conn_fini(c);
     }
 
     g_free(g);