rtpbasedepayload: look at ssrc before sequence numbers

Doing so prevents us dropping buffers in the rare, but possible, situations,
when the stream changes SSRC and new sequence numbers does not differ
much from the last sequence number from previous SSRC. For example:
ssrc - 0xaaaa 101,102,103,104 ssrc - 0xbbbb 102, 103, 104, 105...
In the scenario above we don't want to drop the first 3 packets of
0xbbbb stream.

https://bugzilla.gnome.org/show_bug.cgi?id=764459
This commit is contained in:
Mikhail Fludkov 2016-04-02 10:37:55 +02:00 committed by Sebastian Dröge
parent 6788003912
commit 7a206336dd
2 changed files with 76 additions and 20 deletions

View file

@ -46,6 +46,7 @@ struct _GstRTPBaseDepayloadPrivate
GstClockTime dts;
GstClockTime duration;
guint32 last_ssrc;
guint32 last_seqnum;
guint32 last_rtptime;
guint32 next_seqnum;
@ -356,6 +357,7 @@ gst_rtp_base_depayload_handle_buffer (GstRTPBaseDepayload * filter,
GstRTPBaseDepayloadPrivate *priv;
GstFlowReturn ret = GST_FLOW_OK;
GstBuffer *out_buf;
guint32 ssrc;
guint16 seqnum;
guint32 rtptime;
gboolean discont, buf_discont;
@ -380,6 +382,7 @@ gst_rtp_base_depayload_handle_buffer (GstRTPBaseDepayload * filter,
priv->dts = GST_BUFFER_DTS (in);
priv->duration = GST_BUFFER_DURATION (in);
ssrc = gst_rtp_buffer_get_ssrc (&rtp);
seqnum = gst_rtp_buffer_get_seq (&rtp);
rtptime = gst_rtp_buffer_get_timestamp (&rtp);
@ -396,32 +399,40 @@ gst_rtp_base_depayload_handle_buffer (GstRTPBaseDepayload * filter,
* are strictly increasing, dropping anything that is out of the ordinary. We
* can only do this when the next_seqnum is known. */
if (G_LIKELY (priv->next_seqnum != -1)) {
gap = gst_rtp_buffer_compare_seqnum (seqnum, priv->next_seqnum);
if (ssrc != priv->last_ssrc) {
GST_LOG_OBJECT (filter,
"New ssrc %u (current ssrc %u), sender restarted",
ssrc, priv->last_ssrc);
discont = TRUE;
} else {
gap = gst_rtp_buffer_compare_seqnum (seqnum, priv->next_seqnum);
/* if we have no gap, all is fine */
if (G_UNLIKELY (gap != 0)) {
GST_LOG_OBJECT (filter, "got packet %u, expected %u, gap %d", seqnum,
priv->next_seqnum, gap);
if (gap < 0) {
/* seqnum > next_seqnum, we are missing some packets, this is always a
* DISCONT. */
GST_LOG_OBJECT (filter, "%d missing packets", gap);
discont = TRUE;
} else {
/* seqnum < next_seqnum, we have seen this packet before or the sender
* could be restarted. If the packet is not too old, we throw it away as
* a duplicate, otherwise we mark discont and continue. 100 misordered
* packets is a good threshold. See also RFC 4737. */
if (gap < 100)
goto dropping;
/* if we have no gap, all is fine */
if (G_UNLIKELY (gap != 0)) {
GST_LOG_OBJECT (filter, "got packet %u, expected %u, gap %d", seqnum,
priv->next_seqnum, gap);
if (gap < 0) {
/* seqnum > next_seqnum, we are missing some packets, this is always a
* DISCONT. */
GST_LOG_OBJECT (filter, "%d missing packets", gap);
discont = TRUE;
} else {
/* seqnum < next_seqnum, we have seen this packet before or the sender
* could be restarted. If the packet is not too old, we throw it away as
* a duplicate, otherwise we mark discont and continue. 100 misordered
* packets is a good threshold. See also RFC 4737. */
if (gap < 100)
goto dropping;
GST_LOG_OBJECT (filter,
"%d > 100, packet too old, sender likely restarted", gap);
discont = TRUE;
GST_LOG_OBJECT (filter,
"%d > 100, packet too old, sender likely restarted", gap);
discont = TRUE;
}
}
}
}
priv->next_seqnum = (seqnum + 1) & 0xffff;
priv->last_ssrc = ssrc;
if (G_UNLIKELY (discont)) {
priv->discont = TRUE;

View file

@ -729,6 +729,50 @@ GST_START_TEST (rtp_base_depayload_reversed_test)
destroy_depayloader (state);
}
GST_END_TEST
/* The same scenario as in rtp_base_depayload_reversed_test
* except that SSRC is changed for the 2nd packet that is why
* it should not be discarded.
*/
GST_START_TEST (rtp_base_depayload_ssrc_changed_test)
{
State *state;
state = create_depayloader ("application/x-rtp", NULL);
set_state (state, GST_STATE_PLAYING);
push_rtp_buffer (state,
"pts", 0 * GST_SECOND,
"rtptime", G_GUINT64_CONSTANT (0x43214321),
"seq", 0x4242, "ssrc", 0xabe2b0b, NULL);
push_rtp_buffer (state,
"pts", 1 * GST_SECOND,
"rtptime", G_GUINT64_CONSTANT (0x43214321) + 1 * DEFAULT_CLOCK_RATE,
"seq", 0x4242 - 1, "ssrc", 0xcafebabe, NULL);
set_state (state, GST_STATE_NULL);
validate_buffers_received (2);
validate_buffer (0, "pts", 0 * GST_SECOND, "discont", FALSE, NULL);
validate_buffer (1, "pts", 1 * GST_SECOND, "discont", TRUE, NULL);
validate_events_received (3);
validate_event (0, "stream-start", NULL);
validate_event (1, "caps", "media-type", "application/x-rtp", NULL);
validate_event (2, "segment",
"time", G_GUINT64_CONSTANT (0),
"start", G_GUINT64_CONSTANT (0), "stop", G_MAXUINT64, NULL);
destroy_depayloader (state);
}
GST_END_TEST
/* the intent of this test is to push two RTP packets that have reverse sequence
* numbers that differ significantly. the depayloader will consider RTP packets
@ -1198,6 +1242,7 @@ rtp_basepayloading_suite (void)
tcase_add_test (tc_chain, rtp_base_depayload_invalid_rtp_packet_test);
tcase_add_test (tc_chain, rtp_base_depayload_with_gap_test);
tcase_add_test (tc_chain, rtp_base_depayload_reversed_test);
tcase_add_test (tc_chain, rtp_base_depayload_ssrc_changed_test);
tcase_add_test (tc_chain, rtp_base_depayload_old_reversed_test);
tcase_add_test (tc_chain, rtp_base_depayload_without_negotiation_test);