validate: Properly check that the seqnum of the EOS is always properly set

In the pipeline, an EOS should always have the same seqnum of the
previous SEGMENT event that was received. If the segment is the result
of a seek, it should always be the same as the seek seqnum too.

+ (Mathieu Duponchelle): fix reporting and concatenation tests.

diff --git a/validate/gst/validate/gst-validate-pad-monitor.c b/validate/gst/validate/gst-validate-pad-monitor.c
index a5f1557..396e2ff 100644 (file)
--- a/validate/gst/validate/gst-validate-pad-monitor.c
+++ b/validate/gst/validate/gst-validate-pad-monitor.c
@@ -1435,6 +1435,9 @@ gst_validate_pad_monitor_common_event_check (GstValidatePadMonitor *
         }
       }
 
+      pad_monitor->pending_newsegment_seqnum = seqnum;
+      pad_monitor->pending_eos_seqnum = seqnum;
+
       if (!pad_monitor->pending_flush_stop) {
         gchar *event_str = _get_event_string (event);
 
@@ -1584,13 +1587,14 @@ gst_validate_pad_monitor_downstream_event_check (GstValidatePadMonitor *
         if (pad_monitor->pending_newsegment_seqnum == seqnum) {
           pad_monitor->pending_newsegment_seqnum = 0;
         } else {
-          /* TODO is this an error? could be a segment from the start
-           * received just before the seek segment */
+          GST_VALIDATE_REPORT (pad_monitor, EVENT_HAS_WRONG_SEQNUM,
+              "The expected EOS seqnum should be the same as the "
+              "one from the seek that caused it. Got: %u."
+              " Expected: %u", seqnum, pad_monitor->pending_eos_seqnum);
         }
       }
 
-      /* got a segment, no need for EOS now */
-      pad_monitor->pending_eos_seqnum = 0;
+      pad_monitor->pending_eos_seqnum = seqnum;
 
       if (GST_PAD_DIRECTION (pad) == GST_PAD_SINK) {
         gst_validate_pad_monitor_add_expected_newsegment (pad_monitor, event);
@@ -1626,13 +1630,13 @@ gst_validate_pad_monitor_downstream_event_check (GstValidatePadMonitor *
     }
     case GST_EVENT_EOS:
       pad_monitor->is_eos = TRUE;
-      if (pad_monitor->pending_eos_seqnum &&
-          pad_monitor->pending_eos_seqnum != seqnum) {
+      if (pad_monitor->pending_eos_seqnum != seqnum) {
         GST_VALIDATE_REPORT (pad_monitor, EVENT_HAS_WRONG_SEQNUM,
             "The expected EOS seqnum should be the same as the "
             "one from the seek that caused it. Got: %u."
             " Expected: %u", seqnum, pad_monitor->pending_eos_seqnum);
       }
+
       /*
        * TODO add end of stream checks for
        *  - events not pushed
@@ -1724,8 +1728,6 @@ gst_validate_pad_monitor_src_event_check (GstValidatePadMonitor * pad_monitor,
         pad_monitor->pending_flush_start_seqnum = seqnum;
         pad_monitor->pending_flush_stop_seqnum = seqnum;
       }
-      pad_monitor->pending_newsegment_seqnum = seqnum;
-      pad_monitor->pending_eos_seqnum = seqnum;
     }
       break;
       /* both flushes are handled by the common event handling function */

diff --git a/validate/gst/validate/gst-validate-report.c b/validate/gst/validate/gst-validate-report.c
index ab26ea0..3145e9d 100644 (file)
--- a/validate/gst/validate/gst-validate-report.c
+++ b/validate/gst/validate/gst-validate-report.c
@@ -225,6 +225,12 @@ gst_validate_report_load_issues (void)
   REGISTER_VALIDATE_ISSUE (CRITICAL, EVENT_SEEK_RESULT_POSITION_WRONG,
       _("position after a seek is wrong"), NULL);
 
+  REGISTER_VALIDATE_ISSUE (WARNING, EVENT_EOS_WITHOUT_SEGMENT,
+      _("EOS received without segment event before"),
+      _("A segment event should always be sent before data flow"
+          " EOS being some kind of data flow, there is no exception"
+          " in that regard"));
+
   REGISTER_VALIDATE_ISSUE (CRITICAL, STATE_CHANGE_FAILURE,
       _("state change failed"), NULL);
 

diff --git a/validate/gst/validate/gst-validate-report.h b/validate/gst/validate/gst-validate-report.h
index 37144d5..335e10f 100644 (file)
--- a/validate/gst/validate/gst-validate-report.h
+++ b/validate/gst/validate/gst-validate-report.h
@@ -80,6 +80,7 @@ typedef enum {
 #define EVENT_CAPS_DUPLICATE                     _QUARK("event::caps-duplicate")
 #define EVENT_SEEK_NOT_HANDLED                   _QUARK("event::seek-not-handled")
 #define EVENT_SEEK_RESULT_POSITION_WRONG         _QUARK("event::seek-result-position-wrong")
+#define EVENT_EOS_WITHOUT_SEGMENT                _QUARK("event::eos-without-segment")
 
 #define STATE_CHANGE_FAILURE                     _QUARK("state::change-failure")
 

diff --git a/validate/tests/check/validate/padmonitor.c b/validate/tests/check/validate/padmonitor.c
index 7242728..657ce2f 100644 (file)
--- a/validate/tests/check/validate/padmonitor.c
+++ b/validate/tests/check/validate/padmonitor.c
@@ -440,10 +440,10 @@ GST_START_TEST (issue_concatenation)
 
   /* There's gonna be some clunkiness in here because of funnel*/
   probe_id1 = gst_pad_add_probe (srcpad1,
-        GST_PAD_PROBE_TYPE_BUFFER | GST_PAD_PROBE_TYPE_BUFFER_LIST,
+        GST_PAD_PROBE_TYPE_BUFFER | GST_PAD_PROBE_TYPE_BUFFER_LIST | GST_PAD_PROBE_TYPE_EVENT_DOWNSTREAM,
         (GstPadProbeCallback) drop_buffers, NULL, NULL);
   probe_id2 = gst_pad_add_probe (srcpad2,
-        GST_PAD_PROBE_TYPE_BUFFER | GST_PAD_PROBE_TYPE_BUFFER_LIST,
+        GST_PAD_PROBE_TYPE_BUFFER | GST_PAD_PROBE_TYPE_BUFFER_LIST | GST_PAD_PROBE_TYPE_EVENT_DOWNSTREAM,
         (GstPadProbeCallback) drop_buffers, NULL, NULL);
 
   /* We want to handle the src behaviour ourselves */

diff --git a/validate/tests/check/validate/reporting.c b/validate/tests/check/validate/reporting.c
index aba5329..4aa09dd 100644 (file)
--- a/validate/tests/check/validate/reporting.c
+++ b/validate/tests/check/validate/reporting.c
@@ -151,10 +151,10 @@ _create_issues (GstValidateRunner * runner)
 
   /* There's gonna be some clunkiness in here because of funnel */
   probe_id1 = gst_pad_add_probe (srcpad1,
-      GST_PAD_PROBE_TYPE_BUFFER | GST_PAD_PROBE_TYPE_BUFFER_LIST,
+      GST_PAD_PROBE_TYPE_BUFFER | GST_PAD_PROBE_TYPE_BUFFER_LIST | GST_PAD_PROBE_TYPE_EVENT_DOWNSTREAM,
       (GstPadProbeCallback) drop_buffers, NULL, NULL);
   probe_id2 = gst_pad_add_probe (srcpad2,
-      GST_PAD_PROBE_TYPE_BUFFER | GST_PAD_PROBE_TYPE_BUFFER_LIST,
+      GST_PAD_PROBE_TYPE_BUFFER | GST_PAD_PROBE_TYPE_BUFFER_LIST | GST_PAD_PROBE_TYPE_EVENT_DOWNSTREAM,
       (GstPadProbeCallback) drop_buffers, NULL, NULL);
 
   /* We want to handle the src behaviour ourselves */