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>
This commit is contained in:
Camilo Celis Guzman 2023-05-03 20:53:41 +09:00 committed by GStreamer Marge Bot
parent f159fd8568
commit e4d8cda9a1
3 changed files with 71 additions and 7 deletions

View file

@ -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

View file

@ -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

View file

@ -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,