[Spice-devel] [spice-gtk PATCH v9 4/4] agent: sync guest audio with client values
Victor Toso
victortoso at redhat.com
Tue Apr 21 09:04:26 PDT 2015
Hi,
On Tue, Apr 21, 2015 at 05:36:38PM +0200, Christophe Fergeau wrote:
> On Mon, Apr 20, 2015 at 11:56:18AM +0200, Victor Toso wrote:
> > Functions to sync volume and mute value when necessary. In this patch,
> > only one sync is allowed after agent connect.
> >
> > Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1012868
> > ---
> > gtk/channel-main.c | 141 +++++++++++++++++++++++++++++++++++++++++++++++
> > gtk/spice-session-priv.h | 2 +-
> > 2 files changed, 142 insertions(+), 1 deletion(-)
> >
> > diff --git a/gtk/channel-main.c b/gtk/channel-main.c
> > index 82169aa..4d16c07 100644
> > --- a/gtk/channel-main.c
> > +++ b/gtk/channel-main.c
> > @@ -30,6 +30,7 @@
> > #include "spice-util-priv.h"
> > #include "spice-channel-priv.h"
> > #include "spice-session-priv.h"
> > +#include "spice-audio-priv.h"
> >
> > /**
> > * SECTION:channel-main
> > @@ -109,6 +110,10 @@ struct _SpiceMainChannelPrivate {
> > guint migrate_delayed_id;
> > spice_migrate *migrate_data;
> > int max_clipboard;
> > +
> > + bool agent_volume_playback_sync;
> > + bool agent_volume_record_sync;
> > + GCancellable *cancellable_volume_info;
> > };
> >
> > struct spice_migrate {
> > @@ -187,6 +192,7 @@ static const char *agent_msg_types[] = {
> > [ VD_AGENT_CLIPBOARD_GRAB ] = "clipboard grab",
> > [ VD_AGENT_CLIPBOARD_REQUEST ] = "clipboard request",
> > [ VD_AGENT_CLIPBOARD_RELEASE ] = "clipboard release",
> > + [ VD_AGENT_AUDIO_VOLUME_SYNC ] = "volume-sync",
> > };
> >
> > static const char *agent_caps[] = {
> > @@ -201,6 +207,7 @@ static const char *agent_caps[] = {
> > [ VD_AGENT_CAP_GUEST_LINEEND_LF ] = "line-end lf",
> > [ VD_AGENT_CAP_GUEST_LINEEND_CRLF ] = "line-end crlf",
> > [ VD_AGENT_CAP_MAX_CLIPBOARD ] = "max-clipboard",
> > + [ VD_AGENT_CAP_AUDIO_VOLUME_SYNC ] = "volume-sync",
> > };
> > #define NAME(_a, _i) ((_i) < SPICE_N_ELEMENTS(_a) ? (_a[(_i)] ?: "?") : "?")
> >
> > @@ -231,6 +238,7 @@ static void spice_main_channel_init(SpiceMainChannel *channel)
> > c = channel->priv = SPICE_MAIN_CHANNEL_GET_PRIVATE(channel);
> > c->agent_msg_queue = g_queue_new();
> > c->file_xfer_tasks = g_hash_table_new(g_direct_hash, g_direct_equal);
> > + c->cancellable_volume_info = g_cancellable_new();
> >
> > spice_main_channel_reset_capabilties(SPICE_CHANNEL(channel));
> > }
> > @@ -346,6 +354,9 @@ static void spice_main_channel_dispose(GObject *obj)
> > c->migrate_delayed_id = 0;
> > }
> >
> > + g_cancellable_cancel(c->cancellable_volume_info);
> > + g_clear_object(&c->cancellable_volume_info);
> > +
> > if (G_OBJECT_CLASS(spice_main_channel_parent_class)->dispose)
> > G_OBJECT_CLASS(spice_main_channel_parent_class)->dispose(obj);
> > }
> > @@ -413,6 +424,9 @@ static void spice_main_channel_reset(SpiceChannel *channel, gboolean migrating)
> > agent_free_msg_queue(SPICE_MAIN_CHANNEL(channel));
> > c->agent_msg_queue = g_queue_new();
> >
> > + c->agent_volume_playback_sync = false;
> > + c->agent_volume_record_sync = false;
> > +
> > set_agent_connected(SPICE_MAIN_CHANNEL(channel), FALSE);
> >
> > SPICE_CHANNEL_CLASS(spice_main_channel_parent_class)->channel_reset(channel, migrating);
> > @@ -1092,6 +1106,128 @@ gboolean spice_main_send_monitor_config(SpiceMainChannel *channel)
> > return TRUE;
> > }
> >
> > +static void audio_playback_volume_info_cb(GObject *object, GAsyncResult *res, gpointer user_data)
> > +{
> > + SpiceMainChannel *main_channel = user_data;
> > + SpiceSession *session = spice_channel_get_session(SPICE_CHANNEL(main_channel));
> > + SpiceAudio *audio = spice_audio_get(session, NULL);
> > + VDAgentAudioVolumeSync *avs;
> > + guint16 *volume;
> > + guint8 nchannels;
> > + gboolean mute, ret;
> > + gsize array_size;
> > + GError *error = NULL;
> > +
> > + ret = SPICE_AUDIO_GET_CLASS(audio)->get_playback_volume_info_finish(audio,
> > + res,
> > + &mute,
> > + &nchannels,
> > + &volume,
> > + &error);
> > + if (ret == FALSE || volume == NULL || nchannels == 0) {
> > + if (error != NULL) {
> > + spice_warning("Failed to get playback async volume info: %s", error->message);
> > + g_error_free (error);
> > + } else {
> > + SPICE_DEBUG("Failed to get playback async volume info");
> > + }
> > + main_channel->priv->agent_volume_playback_sync = false;
> > + return;
> > + }
> > +
> > + array_size = sizeof(uint16_t) * nchannels;
> > + avs = g_malloc0(sizeof(VDAgentAudioVolumeSync) + array_size);
> > + avs->is_playback = TRUE;
> > + avs->mute = mute;
> > + avs->nchannels = nchannels;
> > + memcpy(avs->volume, volume, array_size);
> > +
> > + SPICE_DEBUG ("%s mute=%s nchannels=%u volume[0]=%u",
> > + __func__, spice_yes_no(mute), nchannels, volume[0]);
> > + g_clear_pointer (&volume, g_free);
>
> g_free(volume); would be enough here.
Sure!
> > + agent_msg_queue(main_channel, VD_AGENT_AUDIO_VOLUME_SYNC,
> > + sizeof(VDAgentAudioVolumeSync) + array_size, avs);
> > +}
> > +
> > +static void agent_sync_audio_playback(SpiceMainChannel *main_channel)
> > +{
> > + SpiceSession *session = spice_channel_get_session(SPICE_CHANNEL(main_channel));
> > + SpiceAudio *audio = spice_audio_get(session, NULL);
> > + SpiceMainChannelPrivate *c = main_channel->priv;
> > +
> > + 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__);
> > + return;
> > + }
> > + /* only one per connection */
> > + g_cancellable_reset(c->cancellable_volume_info);
> > + c->agent_volume_playback_sync = true;
> > + SPICE_AUDIO_GET_CLASS(audio)->get_playback_volume_info_async(audio,
> > + c->cancellable_volume_info, main_channel, audio_playback_volume_info_cb, main_channel);
>
> I tend to wrap such vfunc calls into more readable
> spice_audio_get_playback_volume_info_async() wrappers which just do the
> vfunc call.
>
I agree it is more readable to wrap it. I'll fix this as well.
> > +}
> > +
> > +static void audio_record_volume_info_cb(GObject *object, GAsyncResult *res, gpointer user_data)
> > +{
> > + SpiceMainChannel *main_channel = user_data;
> > + SpiceSession *session = spice_channel_get_session(SPICE_CHANNEL(main_channel));
> > + SpiceAudio *audio = spice_audio_get(session, NULL);
> > + VDAgentAudioVolumeSync *avs;
> > + guint16 *volume;
> > + guint8 nchannels;
> > + gboolean ret, mute;
> > + gsize array_size;
> > + GError *error = NULL;
> > +
> > + ret = SPICE_AUDIO_GET_CLASS(audio)->get_record_volume_info_finish(audio,
> > + res,
> > + &mute,
> > + &nchannels,
> > + &volume,
> > + &error);
> > + if (ret == FALSE || volume == NULL || nchannels == 0) {
> > + if (error != NULL) {
> > + spice_warning ("Failed to get record async volume info: %s", error->message);
> > + g_error_free (error);
> > + } else {
> > + SPICE_DEBUG("Failed to get record async volume info");
> > + }
> > + main_channel->priv->agent_volume_record_sync = false;
>
> Any reason to use false rather than FALSE? There are a few other
> true/false in this patch.
Not really, I tend to use false for bool and FALSE for gboolean :-)
It is better to keep this as gboolean and use FALSE as well.
Thanks,
>
> Christophe
>
> > + return;
> > + }
> > +
> > + array_size = sizeof(uint16_t) * nchannels;
> > + avs = g_malloc0(sizeof(VDAgentAudioVolumeSync) + array_size);
> > + avs->is_playback = FALSE;
> > + avs->mute = mute;
> > + avs->nchannels = nchannels;
> > + memcpy(avs->volume, volume, array_size);
> > +
> > + SPICE_DEBUG ("%s mute=%s nchannels=%u volume[0]=%u",
> > + __func__, spice_yes_no(mute), nchannels, volume[0]);
> > + g_clear_pointer (&volume, g_free);
> > + agent_msg_queue(main_channel, VD_AGENT_AUDIO_VOLUME_SYNC,
> > + sizeof(VDAgentAudioVolumeSync) + array_size, avs);
> > +}
> > +
> > +static void agent_sync_audio_record(SpiceMainChannel *main_channel)
> > +{
> > + SpiceSession *session = spice_channel_get_session(SPICE_CHANNEL(main_channel));
> > + SpiceAudio *audio = spice_audio_get(session, NULL);
> > + SpiceMainChannelPrivate *c = main_channel->priv;
> > +
> > + 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__);
> > + return;
> > + }
> > + /* only one per connection */
> > + g_cancellable_reset(c->cancellable_volume_info);
> > + c->agent_volume_record_sync = true;
> > + SPICE_AUDIO_GET_CLASS(audio)->get_record_volume_info_async(audio,
> > + c->cancellable_volume_info, main_channel, audio_record_volume_info_cb, main_channel);
> > +}
> > +
> > /* any context: the message is not flushed immediately,
> > you can wakeup() the channel coroutine or send_msg_queue() */
> > static void agent_display_config(SpiceMainChannel *channel)
> > @@ -1345,6 +1481,8 @@ static void agent_start(SpiceMainChannel *channel)
> > };
> > SpiceMsgOut *out;
> >
> > + c->agent_volume_playback_sync = false;
> > + c->agent_volume_record_sync = false;
> > c->agent_caps_received = false;
> > set_agent_connected(channel, TRUE);
> >
> > @@ -1801,6 +1939,9 @@ static void main_agent_handle_msg(SpiceChannel *channel,
> > c->agent_display_config_sent = true;
> > }
> >
> > + agent_sync_audio_playback(self);
> > + agent_sync_audio_record(self);
> > +
> > agent_max_clipboard(self);
> >
> > agent_send_msg_queue(self);
> > diff --git a/gtk/spice-session-priv.h b/gtk/spice-session-priv.h
> > index 46938ff..049973a 100644
> > --- a/gtk/spice-session-priv.h
> > +++ b/gtk/spice-session-priv.h
> > @@ -98,7 +98,7 @@ PhodavServer* channel_webdav_server_new(SpiceSession *session);
> > guint spice_session_get_n_display_channels(SpiceSession *session);
> > void spice_session_set_main_channel(SpiceSession *session, SpiceChannel *channel);
> > gboolean spice_session_set_migration_session(SpiceSession *session, SpiceSession *mig_session);
> > -
> > +SpiceAudio *spice_audio_get(SpiceSession *session, GMainContext *context);
> > G_END_DECLS
> >
> > #endif /* __SPICE_CLIENT_SESSION_PRIV_H__ */
> > --
> > 2.1.0
> >
> > _______________________________________________
> > Spice-devel mailing list
> > Spice-devel at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/spice-devel
More information about the Spice-devel
mailing list