From 219749c40c37f66b1130cbb8eedf7730f5b83753 Mon Sep 17 00:00:00 2001 From: Havard Graff Date: Fri, 18 Dec 2020 14:01:23 +0100 Subject: rtptwcc: fix seqnum-wrap Using the proper API to do this is obviously an improvement, and adding a test for the case of a packet-loss when the seqnum wrap is also a good idea. Co-authored-by: Tulio Beloqui Part-of: --- gst/rtpmanager/rtptwcc.c | 6 ++--- tests/check/elements/rtpsession.c | 47 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 50 insertions(+), 3 deletions(-) diff --git a/gst/rtpmanager/rtptwcc.c b/gst/rtpmanager/rtptwcc.c index 515c69b23..ad0628abc 100644 --- a/gst/rtpmanager/rtptwcc.c +++ b/gst/rtpmanager/rtptwcc.c @@ -596,7 +596,7 @@ rtp_twcc_manager_recv_packet (RTPTWCCManager * twcc, { gboolean send_feedback = FALSE; RecvPacket packet; - gint32 diff; + gint diff; /* if this packet would exceed the capacity of our MTU, we create a feedback with the current packets, and start over with this one */ @@ -614,8 +614,8 @@ rtp_twcc_manager_recv_packet (RTPTWCCManager * twcc, /* check if we are reordered, and treat it as lost if we already sent a feedback msg with a higher seqnum. If the diff is huge, treat it as a restart of a stream */ - diff = (gint32) seqnum - (gint32) twcc->expected_recv_seqnum; - if (twcc->fb_pkt_count > 0 && diff < 0 && diff > -1000) { + diff = gst_rtp_buffer_compare_seqnum (twcc->expected_recv_seqnum, seqnum); + if (twcc->fb_pkt_count > 0 && diff < 0) { GST_INFO ("Received out of order packet (%u after %u), treating as lost", seqnum, twcc->expected_recv_seqnum); return FALSE; diff --git a/tests/check/elements/rtpsession.c b/tests/check/elements/rtpsession.c index c8eb5bdbc..d18b55bf0 100644 --- a/tests/check/elements/rtpsession.c +++ b/tests/check/elements/rtpsession.c @@ -3027,6 +3027,52 @@ GST_START_TEST (test_twcc_seqnum_wrap) GST_END_TEST; +GST_START_TEST (test_twcc_seqnum_wrap_with_loss) +{ + SessionHarness *h = session_harness_new (); + GstBuffer *buf; + + TWCCPacket packets[] = { + {65534, 0 * 250 * GST_USECOND, TRUE}, + {1, 3 * 250 * GST_USECOND, TRUE}, + }; + + guint8 exp_fci0[] = { + 0xff, 0xfe, /* base sequence number: 65534 */ + 0x00, 0x01, /* packet status count: 2 */ + 0x00, 0x00, 0x00, /* reference time: 0 */ + 0x00, /* feedback packet count: 0 */ + 0x20, 0x01, /* packet chunk */ + 0x00, /* recv delta: +0:00:00.000000000 */ + 0x00, /* padding */ + }; + + guint8 exp_fci1[] = { + 0x00, 0x01, /* base sequence number: 1 */ + 0x00, 0x01, /* packet status count: 1 */ + 0x00, 0x00, 0x00, /* reference time: 0 */ + 0x01, /* feedback packet count: 1 */ + 0x20, 0x01, /* packet chunk */ + 0x03, /* recv delta: +0:00:00.000750000 */ + 0x00, /* padding */ + }; + + twcc_push_packets (h, packets); + + buf = session_harness_produce_twcc (h); + twcc_verify_fci (buf, exp_fci0); + gst_buffer_unref (buf); + + buf = session_harness_produce_twcc (h); + twcc_verify_fci (buf, exp_fci1); + gst_buffer_unref (buf); + + session_harness_free (h); +} + +GST_END_TEST; + + GST_START_TEST (test_twcc_double_packets) { SessionHarness *h = session_harness_new (); @@ -3956,6 +4002,7 @@ rtpsession_suite (void) tcase_add_loop_test (tc_chain, test_twcc_various_gaps, 0, 50); tcase_add_test (tc_chain, test_twcc_negative_delta); tcase_add_test (tc_chain, test_twcc_seqnum_wrap); + tcase_add_test (tc_chain, test_twcc_seqnum_wrap_with_loss); tcase_add_test (tc_chain, test_twcc_huge_seqnum_gap); tcase_add_test (tc_chain, test_twcc_double_packets); tcase_add_test (tc_chain, test_twcc_duplicate_seqnums); -- cgit v1.2.1