From e1ad840494eeb6bfc06ebf84dc95d3f98fde4f64 Mon Sep 17 00:00:00 2001
From: Thibault Saunier <tsaunier@igalia.com>
Date: Thu, 14 May 2020 18:45:11 -0400
Subject: [PATCH] scenario: Mark seek as done only when reaching next state

There is a race where following actions could generate a
flush-start/flush-stop dance but the state change resulting from the
seek hasn't been committed yet, leading to the ASYNC_START being
ignored by GstBin since its pending_state is not VOID when receiving
the ASYNC_START message.

Conceptually it is totally correct to consider an action done when
the state change of the pipeline is stabilized..

Part-of: <https://gitlab.freedesktop.org/gstreamer/gst-devtools/-/merge_requests/198>
---
 validate/gst/validate/gst-validate-scenario.c | 35 ++++++++++++++-----
 1 file changed, 26 insertions(+), 9 deletions(-)

diff --git a/validate/gst/validate/gst-validate-scenario.c b/validate/gst/validate/gst-validate-scenario.c
index 80f63c34dd..9ce6ae2799 100644
--- a/validate/gst/validate/gst-validate-scenario.c
+++ b/validate/gst/validate/gst-validate-scenario.c
@@ -3349,6 +3349,20 @@ gst_validate_scenario_check_latency (GstValidateScenario * scenario,
   }
 }
 
+static gboolean
+gst_validate_scenario_is_flush_seeking (GstValidateScenario * scenario)
+{
+  GstValidateSeekInformation *seekinfo = scenario->priv->current_seek;
+
+  if (!seekinfo)
+    return FALSE;
+
+  if (!(seekinfo->flags & GST_SEEK_FLAG_FLUSH))
+    return FALSE;
+
+  return seekinfo->action->priv->state == GST_VALIDATE_EXECUTE_ACTION_ASYNC;
+}
+
 static gboolean
 message_cb (GstBus * bus, GstMessage * message, GstValidateScenario * scenario)
 {
@@ -3366,12 +3380,8 @@ message_cb (GstBus * bus, GstMessage * message, GstValidateScenario * scenario)
 
   switch (GST_MESSAGE_TYPE (message)) {
     case GST_MESSAGE_ASYNC_DONE:
-      if (priv->current_seek
-          && ((priv->current_seek->flags & GST_SEEK_FLAG_FLUSH) &&
-              (priv->current_seek->action->priv->state ==
-                  GST_VALIDATE_EXECUTE_ACTION_ASYNC))) {
-        gst_validate_action_set_done (priv->current_seek->action);
-      } else if (scenario->priv->needs_async_done) {
+      if (!gst_validate_scenario_is_flush_seeking (scenario) &&
+          scenario->priv->needs_async_done) {
         scenario->priv->needs_async_done = FALSE;
         if (priv->actions && _action_sets_state (priv->actions->data)
             && !priv->changing_state)
@@ -3387,10 +3397,12 @@ message_cb (GstBus * bus, GstMessage * message, GstValidateScenario * scenario)
       break;
     case GST_MESSAGE_STATE_CHANGED:
     {
-      if (pipeline && GST_MESSAGE_SRC (message) == GST_OBJECT (pipeline)) {
-        GstState nstate, pstate;
 
-        gst_message_parse_state_changed (message, &pstate, &nstate, NULL);
+      if (pipeline && GST_MESSAGE_SRC (message) == GST_OBJECT (pipeline)) {
+        GstState pstate, nstate, pending_state;
+
+        gst_message_parse_state_changed (message, &pstate, &nstate,
+            &pending_state);
 
         if (pstate == GST_STATE_PAUSED && nstate == GST_STATE_READY) {
           /* Reset sink information */
@@ -3403,6 +3415,11 @@ message_cb (GstBus * bus, GstMessage * message, GstValidateScenario * scenario)
           SCENARIO_UNLOCK (scenario);
         }
 
+        if (gst_validate_scenario_is_flush_seeking (scenario)
+            && pending_state == GST_STATE_VOID_PENDING) {
+          gst_validate_action_set_done (priv->current_seek->action);
+        }
+
         if (scenario->priv->changing_state &&
             scenario->priv->target_state == nstate) {
           scenario->priv->changing_state = FALSE;