Re: [spice-gtk PATCH v2 1/7] audio: spice-pulse aware of app changes

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hey,

On Mon, Mar 23, 2015 at 09:31:36AM -0400, Marc-André Lureau wrote:
> 
> 
> ----- Original Message -----
> > If there are changes in the audio stream like mute or volume,
> > spice-pulse should be aware of this changes in case it is
> > necessary to sync this values with guest.
> > 
> > This patch subscribe a callback for changes in sink-input and
> > source-output of pulse and keep track of volume and mute changes.
> 
> Why subscribe for events if the value is required just once?

Mainly because the calls to get volume/mute are async so I thought
filtering the events would be okay.

Also, I'm still not sure if volume-sync only once would be enough to fix
volume-changes issues. The example that David pointed here [0] could
also be done without disconnecting the client. I can see this problem
happening with Boxes users for instance.

[0] https://bugzilla.redhat.com/show_bug.cgi?id=1012868#c10
> 
> > ---
> >  gtk/spice-pulse.c | 136
> >  ++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 136 insertions(+)
> > 
> > diff --git a/gtk/spice-pulse.c b/gtk/spice-pulse.c
> > index dd7f309..6b27c97 100644
> > --- a/gtk/spice-pulse.c
> > +++ b/gtk/spice-pulse.c
> > @@ -37,6 +37,9 @@ struct stream {
> >      pa_operation            *cork_op;
> >      gboolean                started;
> >      guint                   num_underflow;
> > +    gboolean                mute;
> > +    guint8                  nchannels;
> > +    guint16                 *volume;
> >  };
> >  
> >  struct _SpicePulsePrivate {
> > @@ -50,6 +53,7 @@ struct _SpicePulsePrivate {
> >      struct stream           record;
> >      guint                   last_delay;
> >      guint                   target_delay;
> > +    gboolean                context_subscribed;
> >  };
> >  
> >  G_DEFINE_TYPE(SpicePulse, spice_pulse, SPICE_TYPE_AUDIO)
> > @@ -110,6 +114,8 @@ static void spice_pulse_dispose(GObject *obj)
> >          pa_operation_unref(p->playback.cork_op);
> >      p->playback.cork_op = NULL;
> >  
> > +    g_clear_pointer(&p->playback.volume, g_free);
> > +
> >      if (p->record.uncork_op)
> >          pa_operation_unref(p->record.uncork_op);
> >      p->record.uncork_op = NULL;
> > @@ -118,6 +124,8 @@ static void spice_pulse_dispose(GObject *obj)
> >          pa_operation_unref(p->record.cork_op);
> >      p->record.cork_op = NULL;
> >  
> > +    g_clear_pointer(&p->record.volume, g_free);
> > +
> >      if (p->pchannel)
> >          g_object_weak_unref(G_OBJECT(p->pchannel), channel_weak_notified,
> >          pulse);
> >      p->pchannel = NULL;
> > @@ -780,6 +788,116 @@ static gboolean connect_channel(SpiceAudio *audio,
> > SpiceChannel *channel)
> >      return FALSE;
> >  }
> >  
> > +static void sink_input_info_cb(pa_context *context,
> > +                               const pa_sink_input_info *info,
> > +                               int eol,
> > +                               void *userdata)
> > +{
> > +    SpicePulsePrivate *p = SPICE_PULSE(userdata)->priv;
> > +    gint i;
> > +
> > +    if (eol)
> > +        return;
> > +
> > +    if (p->playback.nchannels != info->volume.channels) {
> > +        spice_warning ("Number of channels changed from %d to %d",
> > +                       p->playback.nchannels, info->volume.channels);
> > +        g_clear_pointer (&p->playback.volume, g_free);
> > +        p->playback.nchannels = info->volume.channels;
> > +        p->playback.volume = g_new(guint16, p->playback.nchannels);
> > +    }
> > +
> > +    for (i = 0; i < p->playback.nchannels; i++) {
> > +        p->playback.volume[i] = (guint16) info->volume.values[i];
> > +        SPICE_DEBUG("playback volume changed (client-side) %u",
> > p->playback.volume[i]);
> > +    }
> > +
> > +    p->playback.mute = (info->mute) ? TRUE : FALSE;
> > +}
> > +
> > +static void source_output_info_cb(pa_context *context,
> > +                                  const pa_source_output_info *info,
> > +                                  int eol,
> > +                                  void *userdata)
> > +{
> > +    SpicePulsePrivate *p = SPICE_PULSE(userdata)->priv;
> > +    gint i;
> > +
> > +    if (eol)
> > +        return;
> > +
> > +    if (p->record.nchannels != info->volume.channels) {
> > +        spice_warning ("Number of channels changed from %d to %d",
> > +                       p->record.nchannels, info->volume.channels);
> > +        g_clear_pointer (&p->record.volume, g_free);
> > +        p->record.nchannels = info->volume.channels;
> > +        p->record.volume = g_new(guint16, p->record.nchannels);
> > +    }
> > +
> > +    for (i = 0; i < p->record.nchannels; i++) {
> > +        p->record.volume[i] = (guint16) info->volume.values[i];
> > +        SPICE_DEBUG("record volume changed (client-side) %u",
> > p->record.volume[i]);
> > +    }
> > +
> > +    p->record.mute = (info->mute) ? TRUE : FALSE;
> > +}
> > +
> > +static void context_subscribe_callback(pa_context *c,
> > +                                       pa_subscription_event_type_t event,
> > +                                       uint32_t index,
> > +                                       void *userdata)
> > +{
> > +    SpicePulse *pulse = userdata;
> > +    SpicePulsePrivate *p = pulse->priv;
> > +    pa_subscription_event_type_t type, facility;
> > +
> > +    type = event & PA_SUBSCRIPTION_EVENT_TYPE_MASK;
> > +    if (type != PA_SUBSCRIPTION_EVENT_CHANGE &&
> > +        type != PA_SUBSCRIPTION_EVENT_NEW)
> > +        return;
> > +
> > +    facility = event & PA_SUBSCRIPTION_EVENT_FACILITY_MASK;
> > +    if (p->playback.stream != NULL &&
> > +        facility == PA_SUBSCRIPTION_EVENT_SINK_INPUT) {
> > +        pa_operation *op;
> > +        guint32 stream_index;
> > +
> > +        stream_index = pa_stream_get_index(p->playback.stream);
> > +        if (stream_index != index) {
> > +            SPICE_DEBUG ("Playback stream %d differs from sink-input %d",
> > +                         stream_index, index);
> > +            return;
> > +        }
> > +        op = pa_context_get_sink_input_info(p->context, stream_index,
> > +                                            sink_input_info_cb, pulse);
> > +        if (!op)
> > +            spice_warning("get_sink_input_info failed: %s",
> > +                          pa_strerror(pa_context_errno(p->context)));
> > +        else
> > +            pa_operation_unref(op);
> > +    }
> > +
> > +    if (p->record.stream != NULL &&
> > +        facility == PA_SUBSCRIPTION_EVENT_SOURCE_OUTPUT) {
> > +        pa_operation *op;
> > +        guint32 stream_index;
> > +
> > +        stream_index = pa_stream_get_index(p->record.stream);
> > +        if (stream_index != index) {
> > +            SPICE_DEBUG ("Record stream %d differs from sink-input %d",
> > +                         stream_index, index);
> > +            return;
> > +        }
> > +        op = pa_context_get_source_output_info(p->context, stream_index,
> > +                                               source_output_info_cb,
> > pulse);
> > +        if (!op)
> > +            spice_warning("get_source_output_info failed: %s",
> > +                          pa_strerror(pa_context_errno(p->context)));
> > +        else
> > +            pa_operation_unref(op);
> > +    }
> > +}
> > +
> >  static void context_state_callback(pa_context *c, void *userdata)
> >  {
> >      SpicePulse *pulse = userdata;
> > @@ -802,6 +920,24 @@ static void context_state_callback(pa_context *c, void
> > *userdata)
> >  
> >          if (!p->playback.stream && p->playback.started)
> >              create_playback(SPICE_PULSE(userdata));
> > +
> > +        if (p->context_subscribed == FALSE) {
> > +            pa_operation *op;
> > +            pa_context_set_subscribe_callback(p->context,
> > +                                              context_subscribe_callback,
> > +                                              pulse);
> > +            op = pa_context_subscribe(p->context,
> > +                                      PA_SUBSCRIPTION_MASK_SINK_INPUT|
> > +                                        PA_SUBSCRIPTION_MASK_SOURCE_OUTPUT,
> > +                                      NULL, NULL);
> > +            if (op) {
> > +                pa_operation_unref(op);
> > +                p->context_subscribed = TRUE;
> > +            } else {
> > +                spice_warning("context_subscribe failed: %s",
> > +                              pa_strerror(pa_context_errno(p->context)));
> > +            }
> > +        }
> >          break;
> >      }
> >  
> > --
> > 2.1.0
> > 
> > _______________________________________________
> > Spice-devel mailing list
> > Spice-devel@xxxxxxxxxxxxxxxxxxxxx
> > http://lists.freedesktop.org/mailman/listinfo/spice-devel
> > 
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/spice-devel





[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]