From 7de8b9c16c7ec72fbd913cfea32b5b80ecb7383d Mon Sep 17 00:00:00 2001
From: Thibault Saunier <tsaunier@gnome.org>
Date: Mon, 7 Jul 2014 22:25:51 +0200
Subject: [PATCH] composition: Rework the seqnum logic to avoid races when
 setting the new stack seqnum

When we were seeking the same stack without a logic that gurantees that we actually
saw the seek with the new seqnum set, we could have ended up with an EOS set with
the right seqnum even if it was actually not the case.

Co-Authored by: Mathieu Duponchelle <mathieu.duponchelle@opencreed.com>
---
 gnl/gnlcomposition.c |  3 ++-
 gnl/gnlghostpad.c    | 26 ++++++++++++++++++++++----
 gnl/gnlobject.c      |  8 +++++---
 gnl/gnlobject.h      |  4 ++++
 4 files changed, 33 insertions(+), 8 deletions(-)

diff --git a/gnl/gnlcomposition.c b/gnl/gnlcomposition.c
index 5873f5e239..b5252c9da2 100644
--- a/gnl/gnlcomposition.c
+++ b/gnl/gnlcomposition.c
@@ -460,11 +460,11 @@ _seek_pipeline_func (SeekData * seekd)
   seek_handling (seekd->comp, TRUE, FALSE);
 
   priv->reset_time = TRUE;
+  GNL_OBJECT (seekd->comp)->wanted_seqnum = gst_event_get_seqnum (seekd->event);
   priv->gnl_event_pad_func (GNL_OBJECT (seekd->comp)->srcpad,
       GST_OBJECT (seekd->comp), get_new_seek_event (seekd->comp, FALSE, FALSE));
   priv->reset_time = FALSE;
 
-  GNL_OBJECT (seekd->comp)->seqnum = gst_event_get_seqnum (seekd->event);
 
 beach:
   gst_event_unref (seekd->event);
@@ -1519,6 +1519,7 @@ gnl_composition_event_handler (GstPad * ghostpad, GstObject * parent,
         return TRUE;
       }
 
+      GNL_OBJECT (comp)->wanted_seqnum = gst_event_get_seqnum (event);
       break;
     }
     case GST_EVENT_QOS:
diff --git a/gnl/gnlghostpad.c b/gnl/gnlghostpad.c
index a1ca71cd66..0b9c03b9b4 100644
--- a/gnl/gnlghostpad.c
+++ b/gnl/gnlghostpad.c
@@ -323,15 +323,31 @@ internalpad_event_function (GstPad * internal, GstObject * parent,
     case GST_PAD_SRC:{
       switch (GST_EVENT_TYPE (event)) {
         case GST_EVENT_SEEK:
-          object->seqnum = gst_event_get_seqnum (event);
-          GST_INFO_OBJECT (object, "Setting seqnum to %i", object->seqnum);
+          object->wanted_seqnum = gst_event_get_seqnum (event);
+          object->seqnum = 0;
+          GST_DEBUG_OBJECT (object, "Setting wanted_seqnum to %i",
+              object->wanted_seqnum);
           break;
         case GST_EVENT_SEGMENT:
+          if (object->wanted_seqnum == 0) {
+            g_assert ("All gnlobject should be seeked at one point or another"
+                " and thus we should always have a wanted_seqnum when getting"
+                " a new segment" == NULL);
+          }
+
+          GST_DEBUG_OBJECT (object, "Got segment, seqnum-> %i (wanted %i)",
+              gst_event_get_seqnum (event), object->wanted_seqnum);
+
+          object->seqnum = object->wanted_seqnum;
+          object->wanted_seqnum = 0;
+
           event = translate_outgoing_segment (object, event);
+          gst_event_set_seqnum (event, object->seqnum);
           break;
         case GST_EVENT_EOS:
-          GST_INFO_OBJECT (object, "Tweaking seqnum to %i", object->seqnum);
+          if (object->seqnum);
           gst_event_set_seqnum (event, object->seqnum);
+          GST_INFO_OBJECT (object, "Tweaking seqnum to %i", object->seqnum);
           break;
         default:
           break;
@@ -500,8 +516,9 @@ ghostpad_event_function (GstPad * ghostpad, GstObject * parent,
         {
           GstPad *target;
 
-          GST_ERROR_OBJECT (object, "GOT SEEKD: %" GST_PTR_FORMAT, event);
           event = gnl_object_translate_incoming_seek (object, event);
+          object->wanted_seqnum = gst_event_get_seqnum (event);
+          object->seqnum = 0;
           if (!(target = gst_ghost_pad_get_target (GST_GHOST_PAD (ghostpad)))) {
             priv->pending_seek = event;
             GST_INFO_OBJECT (ghostpad, "No target set yet, "
@@ -520,6 +537,7 @@ ghostpad_event_function (GstPad * ghostpad, GstObject * parent,
       switch (GST_EVENT_TYPE (event)) {
         case GST_EVENT_SEGMENT:
           event = translate_incoming_segment (object, event);
+          gst_event_set_seqnum (event, object->seqnum);
           break;
         default:
           break;
diff --git a/gnl/gnlobject.c b/gnl/gnlobject.c
index 0d1ede9b16..a6e3b6948e 100644
--- a/gnl/gnlobject.c
+++ b/gnl/gnlobject.c
@@ -103,9 +103,9 @@ static gboolean
 gnl_object_send_event (GstElement * element, GstEvent * event)
 {
   if (GST_EVENT_TYPE (event) == GST_EVENT_SEEK) {
-    GNL_OBJECT (element)->seqnum = gst_event_get_seqnum (event);
-    GST_INFO_OBJECT (element, "Remember seqnum! %i",
-        GNL_OBJECT (element)->seqnum);
+    GNL_OBJECT (element)->wanted_seqnum = gst_event_get_seqnum (event);
+    GST_DEBUG_OBJECT (element, "Remember seqnum! %i",
+        GNL_OBJECT (element)->wanted_seqnum);
   }
 
 
@@ -426,6 +426,7 @@ gnl_object_cleanup (GnlObject * object)
   GST_DEBUG_OBJECT (object, "cleaning-up");
 
   object->seqnum = 0;
+  object->wanted_seqnum = 0;
   if (!(GNL_OBJECT_GET_CLASS (object)->cleanup (object)))
     ret = GST_STATE_CHANGE_FAILURE;
 
@@ -663,6 +664,7 @@ gnl_object_reset (GnlObject * object)
   GST_INFO_OBJECT (object, "Resetting child timing values to default");
 
   object->seqnum = 0;
+  object->wanted_seqnum = 0;
   object->start = 0;
   object->duration = 0;
   object->stop = 0;
diff --git a/gnl/gnlobject.h b/gnl/gnlobject.h
index 979a35c9cf..450f5d9664 100644
--- a/gnl/gnlobject.h
+++ b/gnl/gnlobject.h
@@ -125,6 +125,10 @@ struct _GnlObject
   gint64 segment_start;
   gint64 segment_stop;
 
+  /* the seqnum we want, will be conciderd as real one right after
+   * a new segment */
+  gint wanted_seqnum; 
+  /*  The seqnum that we are working with, 0 if not set */
   gint seqnum;
 };