[Spice-devel] [PATCH spice-gtk 1/2] session: introduce "video-sync-on-audio-latency" property

[Victor Toso]

From: Victor Toso <me at victortoso.com>

The multi media time (mmtime) is the relative clock used to sync video
and audio decoding. The mmtime is initially set on SPICE_MSG_MAIN_INIT
and readjusted on every SPICE_MSG_MAIN_MULTI_MEDIA_TIME message.

Basically, spice-server as provider of video and audio stream content
is in charge of giving us the clock reference to make audio and video
be played synchronized.

What is currently happening is that, the client is adjusting the
mmtime based on audio backend's latency input. Although this has been
working well,  a more straight forward solution is to inform
spice-server about audio's latency in order to adjust mmtime in the
source.

Considering that the video might not be encoded in the host, having
the client changing the mmtime can increase significantly the video
decoding latency.

This patch does not change how things are working now but introduce a
new property, SpiceSession::video-sync-on-audio-latency that should
help, when disabled, identify synchronization's bug.

This patch also includes a FIXME as we want to inform spice-server
about any latency. This message does not exist yet.

Signed-off-by: Victor Toso <victortoso at redhat.com>
---
 src/channel-playback.c   |  7 ++++++-
 src/spice-session-priv.h |  1 +
 src/spice-session.c      | 33 +++++++++++++++++++++++++++++++++
 3 files changed, 40 insertions(+), 1 deletion(-)

diff --git a/src/channel-playback.c b/src/channel-playback.c
index afc9059..f7f1a5c 100644
--- a/src/channel-playback.c
+++ b/src/channel-playback.c
@@ -471,7 +471,12 @@ void spice_playback_channel_set_delay(SpicePlaybackChannel *channel, guint32 del
 
     session = spice_channel_get_session(SPICE_CHANNEL(channel));
     if (session) {
-        spice_session_set_mm_time(session, c->last_time - delay_ms);
+        if (spice_session_adjust_mm_time_on_audio_latency(session)) {
+            /* FIXME: Instead of setting Session's mm-time, we should send this
+             * latency information back to spice-server which is the only who
+             * handles the mm-time for audio/video synchronization */
+            spice_session_set_mm_time(session, c->last_time - delay_ms);
+        }
     } else {
         CHANNEL_DEBUG(channel, "channel detached from session, mm time skipped");
     }
diff --git a/src/spice-session-priv.h b/src/spice-session-priv.h
index 03005aa..f18c898 100644
--- a/src/spice-session-priv.h
+++ b/src/spice-session-priv.h
@@ -100,6 +100,7 @@ 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);
 const gchar* spice_audio_data_mode_to_string(gint mode);
+gboolean spice_session_adjust_mm_time_on_audio_latency(SpiceSession *session);
 G_END_DECLS
 
 #endif /* __SPICE_CLIENT_SESSION_PRIV_H__ */
diff --git a/src/spice-session.c b/src/spice-session.c
index a729cc3..8ca6019 100644
--- a/src/spice-session.c
+++ b/src/spice-session.c
@@ -64,6 +64,7 @@ struct _SpiceSessionPrivate {
 
     /* whether to enable audio */
     gboolean          audio;
+    gboolean          adjust_mm_time_on_audio;
 
     /* whether to enable smartcard event forwarding to the server */
     gboolean          smartcard;
@@ -204,6 +205,7 @@ enum {
     PROP_USERNAME,
     PROP_UNIX_PATH,
     PROP_PREF_COMPRESSION,
+    PROP_VIDEO_SYNC_ON_AUDIO_LATENCY,
 };
 
 /* signals */
@@ -696,6 +698,9 @@ static void spice_session_get_property(GObject    *gobject,
     case PROP_PREF_COMPRESSION:
         g_value_set_enum(value, s->preferred_compression);
         break;
+    case PROP_VIDEO_SYNC_ON_AUDIO_LATENCY:
+        g_value_set_boolean(value, s->adjust_mm_time_on_audio);
+        break;
     default:
 	G_OBJECT_WARN_INVALID_PROPERTY_ID(gobject, prop_id, pspec);
 	break;
@@ -835,6 +840,9 @@ static void spice_session_set_property(GObject      *gobject,
     case PROP_PREF_COMPRESSION:
         s->preferred_compression = g_value_get_enum(value);
         break;
+    case PROP_VIDEO_SYNC_ON_AUDIO_LATENCY:
+        s->adjust_mm_time_on_audio = g_value_get_boolean(value);
+        break;
     default:
         G_OBJECT_WARN_INVALID_PROPERTY_ID(gobject, prop_id, pspec);
         break;
@@ -1461,6 +1469,24 @@ static void spice_session_class_init(SpiceSessionClass *klass)
                            SPICE_IMAGE_COMPRESSION_INVALID,
                            G_PARAM_READWRITE |
                            G_PARAM_STATIC_STRINGS));
+    /**
+     * SpiceSession:video-sync-on-audio-latency
+     *
+     * Whether to adjust multi media time based on latecy reported by audio
+     * backend.
+     *
+     * Since: 0.35
+     **/
+    g_object_class_install_property
+        (gobject_class, PROP_VIDEO_SYNC_ON_AUDIO_LATENCY,
+         g_param_spec_boolean("video-sync-on-audio-latency",
+                              "Video sync on audio latency",
+                              "Video sync on audio latency",
+                              TRUE,
+                              G_PARAM_READWRITE |
+                              G_PARAM_CONSTRUCT |
+                              G_PARAM_STATIC_STRINGS));
+
 
     g_type_class_add_private(klass, sizeof(SpiceSessionPrivate));
 }
@@ -2805,3 +2831,10 @@ gboolean spice_session_set_migration_session(SpiceSession *session, SpiceSession
 
     return TRUE;
 }
+
+G_GNUC_INTERNAL
+gboolean spice_session_adjust_mm_time_on_audio_latency(SpiceSession *session)
+{
+    g_return_val_if_fail(SPICE_IS_SESSION(session), FALSE);
+    return session->priv->adjust_mm_time_on_audio;
+}
-- 
2.13.0