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 _______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/spice-devel