From 73b87f04df9da7f056f15446f60f76ef555c0a72 Mon Sep 17 00:00:00 2001 From: Raja Subramanian Date: Mon, 28 Aug 2023 07:38:44 +0530 Subject: [PATCH] Fix out-of-order check after move to 64-bit extended sequence number. (#2004) Also, clean up the RTCP RR handling a bit by removing redundant check. --- pkg/sfu/buffer/rtpstats.go | 4 +++- pkg/sfu/rtpmunger.go | 19 ++++++++++--------- 2 files changed, 13 insertions(+), 10 deletions(-) diff --git a/pkg/sfu/buffer/rtpstats.go b/pkg/sfu/buffer/rtpstats.go index 3687a0ac9..bcdbbf858 100644 --- a/pkg/sfu/buffer/rtpstats.go +++ b/pkg/sfu/buffer/rtpstats.go @@ -560,7 +560,7 @@ func (r *RTPStats) UpdateFromReceiverReport(rr rtcp.ReceptionReport) (rtt uint32 r.lock.Lock() defer r.lock.Unlock() - if !r.initialized || !r.endTime.IsZero() || !r.params.IsReceiverReportDriven || uint64(rr.LastSequenceNumber) < r.sequenceNumber.GetExtendedHighest() { + if !r.initialized || !r.endTime.IsZero() || !r.params.IsReceiverReportDriven { // it is possible that the `LastSequenceNumber` in the receiver report is before the starting // sequence number when dummy packets are used to trigger Pion's OnTrack path. return @@ -575,6 +575,8 @@ func (r *RTPStats) UpdateFromReceiverReport(rr rtcp.ReceptionReport) (rtt uint32 if extHighestSNOverridden < r.sequenceNumber.GetExtendedHighest() { // it is possible that the `LastSequenceNumber` in the receiver report is before the starting // sequence number when dummy packets are used to trigger Pion's OnTrack path. + r.lastRRTime = time.Now() + r.lastRR = rr return } diff --git a/pkg/sfu/rtpmunger.go b/pkg/sfu/rtpmunger.go index e2c535329..64ac05008 100644 --- a/pkg/sfu/rtpmunger.go +++ b/pkg/sfu/rtpmunger.go @@ -138,8 +138,16 @@ func (r *RTPMunger) PacketDropped(extPkt *buffer.ExtPacket) { } func (r *RTPMunger) UpdateAndGetSnTs(extPkt *buffer.ExtPacket) (*TranslationParamsRTP, error) { - diff := extPkt.ExtSequenceNumber - r.extHighestIncomingSN - if diff > (1 << 31) { + diff := int64(extPkt.ExtSequenceNumber - r.extHighestIncomingSN) + + // can get duplicate packet due to FEC + if diff == 0 { + return &TranslationParamsRTP{ + snOrdering: SequenceNumberOrderingDuplicate, + }, ErrDuplicatePacket + } + + if diff < 0 { // out-of-order, look up sequence number offset cache snOffset, err := r.snRangeMap.GetValue(extPkt.ExtSequenceNumber) if err != nil { @@ -155,13 +163,6 @@ func (r *RTPMunger) UpdateAndGetSnTs(extPkt *buffer.ExtPacket) (*TranslationPara }, nil } - // can get duplicate packet due to FEC - if diff == 0 { - return &TranslationParamsRTP{ - snOrdering: SequenceNumberOrderingDuplicate, - }, ErrDuplicatePacket - } - ordering := SequenceNumberOrderingContiguous if diff > 1 { ordering = SequenceNumberOrderingGap