rtpvp8pay, rtpvp9pay: increment PictureID on FLUSH_START

In recent versions of Chrome (M106) a change on their jitter buffer means that
they are very susceptible to PictureID discontinuities.

Then avoid at all cost resetting the PictureID. Moreover, according to
the RFCs for VP8 and VP9 payloads; the PictureID can start off at any
random value. So there is no logical problem of incrementing it here
rather than resetting it, as long as it is a different PictureID.

WebRTC's recent corruption issue:
https://bugs.chromium.org/p/webrtc/issues/detail?id=15101

Part-of: <https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/4530>

3 files changed, 71 insertions, 7 deletions

diff --git a/subprojects/gst-plugins-good/gst/rtp/gstrtpvp8pay.c b/subprojects/gst-plugins-good/gst/rtp/gstrtpvp8pay.c
index ede2deb464..8d3221911d 100644
--- a/subprojects/gst-plugins-good/gst/rtp/gstrtpvp8pay.c
+++ b/subprojects/gst-plugins-good/gst/rtp/gstrtpvp8pay.c
@@ -708,9 +708,13 @@ static gboolean
 gst_rtp_vp8_pay_sink_event (GstRTPBasePayload * payload, GstEvent * event)
 {
   GstRtpVP8Pay *self = GST_RTP_VP8_PAY (payload);
+  GstEventType event_type = GST_EVENT_TYPE (event);
 
-  if (GST_EVENT_TYPE (event) == GST_EVENT_FLUSH_START) {
-    gst_rtp_vp8_pay_reset (self);
+  if (event_type == GST_EVENT_GAP || event_type == GST_EVENT_FLUSH_START) {
+    guint picture_id = self->picture_id;
+    gst_rtp_vp8_pay_picture_id_increment (self);
+    GST_DEBUG_OBJECT (payload, "Incrementing picture ID on %s event %u->%u",
+        GST_EVENT_TYPE_NAME (event), picture_id, self->picture_id);
   }
 
   return GST_RTP_BASE_PAYLOAD_CLASS (gst_rtp_vp8_pay_parent_class)->sink_event
diff --git a/subprojects/gst-plugins-good/gst/rtp/gstrtpvp9pay.c b/subprojects/gst-plugins-good/gst/rtp/gstrtpvp9pay.c
index f981f1ee01..42b96b198e 100644
--- a/subprojects/gst-plugins-good/gst/rtp/gstrtpvp9pay.c
+++ b/subprojects/gst-plugins-good/gst/rtp/gstrtpvp9pay.c
@@ -598,14 +598,13 @@ static gboolean
 gst_rtp_vp9_pay_sink_event (GstRTPBasePayload * payload, GstEvent * event)
 {
   GstRtpVP9Pay *self = GST_RTP_VP9_PAY (payload);
+  GstEventType event_type = GST_EVENT_TYPE (event);
 
-  if (GST_EVENT_TYPE (event) == GST_EVENT_FLUSH_START) {
-    gst_rtp_vp9_pay_picture_id_reset (self);
-  } else if (GST_EVENT_TYPE (event) == GST_EVENT_GAP) {
+  if (event_type == GST_EVENT_GAP || event_type == GST_EVENT_FLUSH_START) {
     guint picture_id = self->picture_id;
     gst_rtp_vp9_pay_picture_id_increment (self);
-    GST_DEBUG_OBJECT (payload, "Incrementing picture ID on GAP event %u->%u",
-        picture_id, self->picture_id);
+    GST_DEBUG_OBJECT (payload, "Incrementing picture ID on %s event %u->%u",
+        GST_EVENT_TYPE_NAME (event), picture_id, self->picture_id);
   }
 
   return GST_RTP_BASE_PAYLOAD_CLASS (gst_rtp_vp9_pay_parent_class)->sink_event
diff --git a/subprojects/gst-plugins-good/tests/check/elements/rtpvp8.c b/subprojects/gst-plugins-good/tests/check/elements/rtpvp8.c
index d658a3a702..d07def1f34 100644
--- a/subprojects/gst-plugins-good/tests/check/elements/rtpvp8.c
+++ b/subprojects/gst-plugins-good/tests/check/elements/rtpvp8.c
@@ -516,6 +516,64 @@ GST_START_TEST (test_pay_tl0picidx_split_buffer)
 
 GST_END_TEST;
 
+GST_START_TEST (test_pay_continuous_picture_id_on_flush)
+{
+  guint8 vp8_bitstream_payload[] = {
+    0x30, 0x00, 0x00, 0x9d, 0x01, 0x2a, 0xb0, 0x00, 0x90, 0x00, 0x06, 0x47,
+    0x08, 0x85, 0x85, 0x88, 0x99, 0x84, 0x88, 0x21, 0x00
+  };
+  GstHarness *h = gst_harness_new ("rtpvp8pay");
+  const gint header_len = 3;
+  const gint packet_len = 12 + header_len + sizeof (vp8_bitstream_payload);
+  const gint picid_offset = 14;
+  GstBuffer *buffer;
+  GstMapInfo map;
+
+  g_object_set (h->element,
+      "picture-id-mode", VP8_PAY_PICTURE_ID_7BITS,
+      "picture-id-offset", 0, NULL);
+
+  gst_harness_set_src_caps_str (h, "video/x-vp8");
+
+  /* First, push a frame */
+  buffer = gst_buffer_new_from_array (vp8_bitstream_payload);
+  buffer = gst_harness_push_and_pull (h, buffer);
+  fail_unless (gst_buffer_map (buffer, &map, GST_MAP_READ));
+  fail_unless_equals_uint64 (map.size, packet_len);
+  fail_unless_equals_int (map.data[picid_offset], 0x00);
+  gst_buffer_unmap (buffer, &map);
+  gst_buffer_unref (buffer);
+
+  /* Push another one and expect the PictureID to increment */
+  buffer = gst_buffer_new_from_array (vp8_bitstream_payload);
+  buffer = gst_harness_push_and_pull (h, buffer);
+  fail_unless (gst_buffer_map (buffer, &map, GST_MAP_READ));
+  fail_unless_equals_uint64 (map.size, packet_len);
+  fail_unless_equals_int (map.data[picid_offset], 0x01);
+  gst_buffer_unmap (buffer, &map);
+  gst_buffer_unref (buffer);
+
+  /* Yet another frame followed by a FLUSH of the pipeline should result
+   * on an increase rather than a reset to maximize interop. */
+  fail_unless (gst_harness_push_event (h, gst_event_new_flush_start ()));
+  fail_unless (gst_harness_push_event (h, gst_event_new_flush_stop (FALSE)));
+  gst_harness_set_src_caps_str (h, "video/x-vp8");
+
+  buffer = gst_buffer_new_from_array (vp8_bitstream_payload);
+  buffer = gst_harness_push_and_pull (h, buffer);
+  fail_unless (gst_buffer_map (buffer, &map, GST_MAP_READ));
+  fail_unless_equals_uint64 (map.size, packet_len);
+  /* PictureID should increment by 2
+   * One due to the FLUSH_START, and another one due to the new frame */
+  fail_unless_equals_int (map.data[picid_offset], 0x03);
+  gst_buffer_unmap (buffer, &map);
+  gst_buffer_unref (buffer);
+
+  gst_harness_teardown (h);
+}
+
+GST_END_TEST;
+
 typedef struct _DepayGapEventTestData
 {
   gint seq_num;
@@ -790,6 +848,9 @@ rtpvp8_suite (void)
       G_N_ELEMENTS (with_meta_test_data));
   tcase_add_test (tc_chain, test_pay_continuous_picture_id_and_tl0picidx);
   tcase_add_test (tc_chain, test_pay_tl0picidx_split_buffer);
+  tcase_add_test (tc_chain, test_pay_continuous_picture_id_on_flush);
+
+  suite_add_tcase (s, (tc_chain = tcase_create ("vp8depay")));
   tcase_add_loop_test (tc_chain, test_depay_stop_gap_events, 0,
       G_N_ELEMENTS (stop_gap_events_test_data));
   tcase_add_loop_test (tc_chain, test_depay_resend_gap_event, 0,