On 05/09/2013 11:16 AM, Marc-André Lureau wrote:
On Thu, May 9, 2013 at 5:06 PM, Yonit Halperin <yhalperi@xxxxxxxxxx
<mailto:yhalperi@xxxxxxxxxx>> wrote:
On 05/09/2013 10:46 AM, Marc-André Lureau wrote:
On Wed, May 1, 2013 at 4:19 PM, Yonit Halperin
<yhalperi@xxxxxxxxxx <mailto:yhalperi@xxxxxxxxxx>
<mailto:yhalperi@xxxxxxxxxx <mailto:yhalperi@xxxxxxxxxx>>> wrote:
mm-time can change after migration. This can cause video
synchronization
problems after migration if not handled appropriately (see
rhbz#951664).
---
gtk/spice-session.c | 39
++++++++++++++++++++++++++++++__+++++++++
1 file changed, 39 insertions(+)
diff --git a/gtk/spice-session.c b/gtk/spice-session.c
index f46ac01..1b5a2f2 100644
--- a/gtk/spice-session.c
+++ b/gtk/spice-session.c
@@ -115,11 +115,23 @@ enum {
enum {
SPICE_SESSION_CHANNEL_NEW,
SPICE_SESSION_CHANNEL_DESTROY,
+ SPICE_SESSION_MM_TIME_RESET,
SPICE_SESSION_LAST_SIGNAL,
};
static guint signals[SPICE_SESSION_LAST___SIGNAL];
+/* main context */
+static void do_emit_main_context(GObject *object, int signum,
gpointer params)
+{
+ switch (signum) {
+ case SPICE_SESSION_MM_TIME_RESET:
+ g_signal_emit(object, signals[signum], 0);
+ break;
+ default:
+ g_warn_if_reached();
+ }
+}
static void update_proxy(SpiceSession *self, const gchar
*str)
{
@@ -1071,6 +1083,23 @@ static void
spice_session_class_init(__SpiceSessionClass *klass)
SPICE_TYPE_CHANNEL);
/**
+ * SpiceSession::mm-time-reset:
+ * @session: the session that emitted the signal
+ *
+ * The #SpiceSession::mm-time-reset is emitted when we
identify
discontinuity in mm-time
+ *
+ * Since 0.20
+ **/
+ signals[SPICE_SESSION_MM_TIME___RESET] =
+ g_signal_new("mm-time-reset",
+ G_OBJECT_CLASS_TYPE(gobject___class),
+ G_SIGNAL_RUN_FIRST,
+ 0, NULL, NULL,
+ g_cclosure_marshal_VOID__VOID,
+ G_TYPE_NONE,
+ 0);
+
+ /**
* SpiceSession:read-only:
*
* Whether this connection is read-only mode.
@@ -1927,16 +1956,26 @@ guint32
spice_session_get_mm_time(__SpiceSession *session)
return s->mm_time + (g_get_monotonic_time() -
s->mm_time_at_clock) / 1000;
}
+#define MM_TIME_DIFF_RESET_THRESH 500 // 0.5 sec
+
G_GNUC_INTERNAL
void spice_session_set_mm_time(__SpiceSession *session,
guint32 time)
{
SpiceSessionPrivate *s =
SPICE_SESSION_GET_PRIVATE(__session);
+ guint32 old_time;
g_return_if_fail(s != NULL);
+ old_time = spice_session_get_mm_time(__session);
+
s->mm_time = time;
s->mm_time_at_clock = g_get_monotonic_time();
SPICE_DEBUG("set mm time: %u",
spice_session_get_mm_time(__session));
+ if (time > old_time + MM_TIME_DIFF_RESET_THRESH ||
+ old_time > time + MM_TIME_DIFF_RESET_THRESH) {
If time goes backward, I think we should reset mm-time without
threshold..
I don't see why it should be different.
The threshold is a simple heuristic to detect timer discontinuity, in my
view.
But when time goes backward although it is supposed to be monotonic,
there isn't any need for a threshold.
I understand you point. Practically, for the lipsync, it shouldn't
matter much. But I don't mind changing it to have no threshold.
+ SPICE_DEBUG("%s: mm-time-reset, old %u, new %u",
__FUNCTION__, old_time, s->mm_time);
+ emit_main_context(session,
SPICE_SESSION_MM_TIME_RESET);
This looks problematic, as this function is called from main and
coroutine context. We need to handle that, perhaps like I
proposed in
previous review
mmm...I thought this is what you proposed in the previous review, to
replace g_signal_emit, with emit_main_context. If not, please
explain the problem.
There is no facility today to send signal on main/leader (g_signal_emit)
or coroutine (emit_main_context) depending on the context automatically.
So far, we have avoided sending signaling in both contexts. Using
emit_main_context() from main context will at least produce some
warnings, and the delayed emission might have undesired result depending
on coroutine backend.
One solution would be to check if the current context is main/leader or
if it has a caller coroutine ("coroutine context"). Perhaps the easier
would be to augment emit_main_context() macro to handle this...
I'll do it. Does it matter that with pulse backend, updating the mm-time
is actually done from pulse audio main loop?
--
Marc-André Lureau
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/spice-devel