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

[Victor Toso]

Hi,

On Mon, Sep 07, 2015 at 11:30:05AM +0200, Victor Toso wrote:
> > 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"
> >

I only thought about first part of your comment but I forgot the second
part, sorry.

I agree, the check could go in the following if. I'm sending a v3

Cheers,
  toso

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