[Spice-devel] [spice-gtk PATCH v1] audio: Do not volume-sync without audio

[Victor Toso]

Thanks for the review!

On Mon, Sep 07, 2015 at 11:17:41AM +0200, Pavel Grunt wrote:
> Hi Victor,
>
> On Mon, 2015-09-07 at 10:41 +0200, Victor Toso wrote:
> > In case audio backend is not initialized correctly or there isn't audio
> > in the VM, we should not try to volume-sync.
> >
> > Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1257210
> > ---
> >  src/channel-main.c | 8 ++++++++
> >  src/spice-audio.c  | 4 ++++
> >  2 files changed, 12 insertions(+)
> >
> > diff --git a/src/channel-main.c b/src/channel-main.c
> > index f5115aa..b169b21 100644
> > --- a/src/channel-main.c
> > +++ b/src/channel-main.c
> > @@ -1159,6 +1159,10 @@ static void agent_sync_audio_playback(SpiceMainChannel
> > *main_channel)
> >      SpiceAudio *audio = spice_audio_get(session, NULL);
> >      SpiceMainChannelPrivate *c = main_channel->priv;
> >
> > +    if (audio == NULL) {
> > +        return;
> > +    }
> > +
> So this is correct behaviour - returning because audio is not available ?
> Maybe it can be in the following 'if', because it "is not going to sync audio
> with guest"
>

Well, I think so. At this point (channel-main), we receive the
capabilites from the agent and try to handle them. In case of
volume-sync we try to sync the volume. If we don't have audio, returning
here should be okay.

The other part of the patch is to improve the internal API.

> >      if (!test_agent_cap(main_channel, VD_AGENT_CAP_AUDIO_VOLUME_SYNC) ||
> >          c->agent_volume_playback_sync == TRUE) {
> >          SPICE_DEBUG("%s - is not going to sync audio with guest", __func__);
> > @@ -1215,6 +1219,10 @@ static void agent_sync_audio_record(SpiceMainChannel
> > *main_channel)
> >      SpiceAudio *audio = spice_audio_get(session, NULL);
> >      SpiceMainChannelPrivate *c = main_channel->priv;
> >
> > +    if (audio == NULL) {
> > +        return;
> > +    }
> > +
> Same here
>
> >      if (!test_agent_cap(main_channel, VD_AGENT_CAP_AUDIO_VOLUME_SYNC) ||
> >          c->agent_volume_record_sync == TRUE) {
> >          SPICE_DEBUG("%s - is not going to sync audio with guest", __func__);
> > diff --git a/src/spice-audio.c b/src/spice-audio.c
> > index ce191e1..7d27b5b 100644
> > --- a/src/spice-audio.c
> > +++ b/src/spice-audio.c
> > @@ -197,6 +197,7 @@ void
> > spice_audio_get_playback_volume_info_async(SpiceAudio *audio,
> >                                                  GAsyncReadyCallback callback,
> >                                                  gpointer user_data)
> >  {
> > +    g_return_val_if_fail (audio != NULL, FALSE);
> just g_return_if_fail()

True, fixed. Thanks!

> >      SPICE_AUDIO_GET_CLASS(audio)->get_playback_volume_info_async(audio,
> >              cancellable, main_channel, callback, user_data);
> >  }
> > @@ -208,6 +209,7 @@ gboolean
> > spice_audio_get_playback_volume_info_finish(SpiceAudio *audio,
> >                                                       guint16 **volume,
> >                                                       GError **error)
> >  {
> > +    g_return_val_if_fail (audio != NULL, FALSE);
> >      return SPICE_AUDIO_GET_CLASS(audio)-
> > >get_playback_volume_info_finish(audio,
> >              res, mute, nchannels, volume, error);
> >  }
> > @@ -218,6 +220,7 @@ void spice_audio_get_record_volume_info_async(SpiceAudio
> > *audio,
> >                                                GAsyncReadyCallback callback,
> >                                                gpointer user_data)
> >  {
> > +    g_return_val_if_fail (audio != NULL, FALSE);
> same here
> >      SPICE_AUDIO_GET_CLASS(audio)->get_record_volume_info_async(audio,
> >              cancellable, main_channel, callback, user_data);
> >  }
> > @@ -229,6 +232,7 @@ gboolean
> > spice_audio_get_record_volume_info_finish(SpiceAudio *audio,
> >                                                     guint16 **volume,
> >                                                     GError **error)
> >  {
> > +    g_return_val_if_fail (audio != NULL, FALSE);
> >      return SPICE_AUDIO_GET_CLASS(audio)->get_record_volume_info_finish(audio,
> >              res, mute, nchannels, volume, error);
> >  }
>
> Thanks,
> Pavel

Sending a v2 in a soon
Cheers,
  toso