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.

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
+          GST_VALIDATE_REPORT (pad_monitor, EVENT_HAS_WRONG_SEQNUM,
-           * received just before the seek segment */
+              "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 = seqnum;
-      pad_monitor->pending_eos_seqnum = 0;
 
       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 &&
+      if (pad_monitor->pending_eos_seqnum != seqnum) {
-          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 */

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);
 

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")
 

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 */

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 */