From 386b92e9ab5c92da9ba7d13436b0a73a0f209e17 Mon Sep 17 00:00:00 2001 From: winlin Date: Tue, 27 Sep 2022 14:53:05 +0800 Subject: [PATCH] For #3167: WebRTC: Refine sequence jitter algorithm. v4.0.266 --- trunk/doc/CHANGELOG.md | 1 + trunk/src/app/srs_app_rtc_source.cpp | 67 +++++++++----------- trunk/src/app/srs_app_rtc_source.hpp | 90 +++++++++++++++++++++----- trunk/src/core/srs_core_version4.hpp | 2 +- trunk/src/utest/srs_utest_rtc.cpp | 94 ++++++++++++++++++++++++++++ 5 files changed, 199 insertions(+), 55 deletions(-) diff --git a/trunk/doc/CHANGELOG.md b/trunk/doc/CHANGELOG.md index 87b848930..60a3df0cc 100644 --- a/trunk/doc/CHANGELOG.md +++ b/trunk/doc/CHANGELOG.md @@ -8,6 +8,7 @@ The changelog for SRS. ## SRS 4.0 Changelog +* v4.0, 2022-09-27, For [#3167](https://github.com/ossrs/srs/issues/3167): WebRTC: Refine sequence jitter algorithm. v4.0.266 * v4.0, 2022-09-16, For [#3179](https://github.com/ossrs/srs/issues/3179): WebRTC: Make sure the same m-lines order for offer and answer. v4.0.265 * v4.0, 2022-09-09, For [#3174](https://github.com/ossrs/srs/issues/3174): WebRTC: Support Unity to publish or play stream. v4.0.264 * v4.0, 2022-09-09, Fix [#3093](https://github.com/ossrs/srs/issues/3093): WebRTC: Ignore unknown fmtp for h.264. v4.0.263 diff --git a/trunk/src/app/srs_app_rtc_source.cpp b/trunk/src/app/srs_app_rtc_source.cpp index d654c63d2..aa00a4d39 100644 --- a/trunk/src/app/srs_app_rtc_source.cpp +++ b/trunk/src/app/srs_app_rtc_source.cpp @@ -2546,40 +2546,35 @@ srs_error_t SrsRtcVideoRecvTrack::check_send_nacks() return err; } -SrsRtcJitter::SrsRtcJitter(uint32_t base) +SrsRtcTsJitter::SrsRtcTsJitter(uint32_t base) { - pkt_base_ = pkt_last_ = 0; - correct_last_ = correct_base_ = 0; - base_ = base; - init_ = false; + int32_t threshold = 3 * 90 * 1000; // 3s in TBN=90K. + jitter_ = new SrsRtcJitter(base, threshold, srs_rtp_ts_distance); } -SrsRtcJitter::~SrsRtcJitter() +SrsRtcTsJitter::~SrsRtcTsJitter() { + srs_freep(jitter_); } -uint32_t SrsRtcJitter::correct(uint32_t ts) +uint32_t SrsRtcTsJitter::correct(uint32_t value) { - if (!init_) { - init_ = true; - correct_base_ = base_; - srs_trace("RTC: Jitter init base=%u, ts=%u", base_, ts); - } - if (!pkt_base_) pkt_base_ = ts; + return jitter_->correct(value); +} - if (pkt_last_) { - int32_t distance = srs_rtp_ts_distance(ts, pkt_last_); - static int32_t max_deviation = 90 * 3 * 1000; - if (distance > max_deviation || distance < -1 * max_deviation) { - srs_trace("RTC: Jitter rebase ts=%u, last=%u, distance=%d, pkt-base=%u/%u, correct-base=%u/%u", ts, pkt_last_, distance, pkt_base_, ts, correct_base_, correct_last_); - pkt_base_ = ts; - correct_base_ = correct_last_; - } - } - pkt_last_ = ts; +SrsRtcSeqJitter::SrsRtcSeqJitter(uint16_t base) +{ + jitter_ = new SrsRtcJitter(base, 128, srs_rtp_seq_distance); +} - correct_last_ = correct_base_ + ts - pkt_base_; - return correct_last_; +SrsRtcSeqJitter::~SrsRtcSeqJitter() +{ + srs_freep(jitter_); +} + +uint16_t SrsRtcSeqJitter::correct(uint16_t value) +{ + return jitter_->correct(value); } SrsRtcSendTrack::SrsRtcSendTrack(SrsRtcConnection* session, SrsRtcTrackDescription* track_desc, bool is_audio) @@ -2588,10 +2583,9 @@ SrsRtcSendTrack::SrsRtcSendTrack(SrsRtcConnection* session, SrsRtcTrackDescripti track_desc_ = track_desc->copy(); nack_no_copy_ = false; - seqno_ = 0; - init_ = false; // Make a different start of sequence number, for debugging. - jitter_ = new SrsRtcJitter(track_desc_->type_ == "audio" ? 10000 : 20000); + jitter_ts_ = new SrsRtcTsJitter(track_desc_->type_ == "audio" ? 10000 : 20000); + jitter_seq_ = new SrsRtcSeqJitter(track_desc_->type_ == "audio" ? 100 : 200); if (is_audio) { rtp_queue_ = new SrsRtpRingBuffer(100); @@ -2607,7 +2601,8 @@ SrsRtcSendTrack::~SrsRtcSendTrack() srs_freep(rtp_queue_); srs_freep(track_desc_); srs_freep(nack_epp); - srs_freep(jitter_); + srs_freep(jitter_ts_); + srs_freep(jitter_seq_); } bool SrsRtcSendTrack::has_ssrc(uint32_t ssrc) @@ -2661,18 +2656,14 @@ std::string SrsRtcSendTrack::get_track_id() void SrsRtcSendTrack::rebuild_packet(SrsRtpPacket* pkt) { // Rebuild the sequence number. - if (!init_) { - init_ = true; - // Make a different start of sequence number, for debugging. - seqno_ = track_desc_->type_ == "audio" ? 1000 : 2000; - srs_trace("RTC: Seqno rebuild %s track=%s, ssrc=%d, seqno=%d to %d", track_desc_->type_.c_str(), track_desc_->id_.c_str(), - pkt->header.get_ssrc(), pkt->header.get_sequence(), seqno_); - } - pkt->header.set_sequence(seqno_++); + int16_t seq = pkt->header.get_sequence(); + pkt->header.set_sequence(jitter_seq_->correct(seq)); // Rebuild the timestamp. uint32_t ts = pkt->header.get_timestamp(); - pkt->header.set_timestamp(jitter_->correct(ts)); + pkt->header.set_timestamp(jitter_ts_->correct(ts)); + + srs_warn("RTC: Correct %s seq=%u/%u, ts=%u/%u", track_desc_->type_.c_str(), seq, pkt->header.get_sequence(), ts, pkt->header.get_timestamp()); } srs_error_t SrsRtcSendTrack::on_nack(SrsRtpPacket** ppkt) diff --git a/trunk/src/app/srs_app_rtc_source.hpp b/trunk/src/app/srs_app_rtc_source.hpp index 2f5c4ccc5..90728c8f4 100644 --- a/trunk/src/app/srs_app_rtc_source.hpp +++ b/trunk/src/app/srs_app_rtc_source.hpp @@ -573,24 +573,85 @@ public: virtual srs_error_t check_send_nacks(); }; +// RTC jitter for TS or sequence number, only reset the base, and keep in original order. +template class SrsRtcJitter { private: - // The ts about packet. - uint32_t pkt_base_; - uint32_t pkt_last_; - // The ts after corrected. - uint32_t correct_base_; - uint32_t correct_last_; + ST threshold_; + typedef ST (*PFN) (const T&, const T&); + PFN distance_; +private: + // The value about packet. + T pkt_base_; + T pkt_last_; + // The value after corrected. + T correct_base_; + T correct_last_; // The base timestamp by config, start from it. - uint32_t base_; + T base_; // Whether initialized. Note that we should not use correct_base_(0) as init state, because it might flip back. bool init_; public: - SrsRtcJitter(uint32_t base); - virtual ~SrsRtcJitter(); + SrsRtcJitter(T base, ST threshold, PFN distance) { + threshold_ = threshold; + distance_ = distance; + base_ = base; + + pkt_base_ = pkt_last_ = 0; + correct_last_ = correct_base_ = 0; + init_ = false; + } + virtual ~SrsRtcJitter() { + } public: - uint32_t correct(uint32_t ts); + T correct(T value) { + if (!init_) { + init_ = true; + correct_base_ = base_; + pkt_base_ = value; + srs_trace("RTC: Jitter init base=%u, value=%u", base_, value); + } + + if (true) { + ST distance = distance_(value, pkt_last_); + if (distance > threshold_ || distance < -1 * threshold_) { + srs_trace("RTC: Jitter rebase value=%u, last=%u, distance=%d, pkt-base=%u/%u, correct-base=%u/%u", + value, pkt_last_, distance, pkt_base_, value, correct_base_, correct_last_); + pkt_base_ = value; + correct_base_ = correct_last_; + } + } + + pkt_last_ = value; + correct_last_ = correct_base_ + value - pkt_base_; + + return correct_last_; + } +}; + +// For RTC timestamp jitter. +class SrsRtcTsJitter +{ +private: + SrsRtcJitter* jitter_; +public: + SrsRtcTsJitter(uint32_t base); + virtual ~SrsRtcTsJitter(); +public: + uint32_t correct(uint32_t value); +}; + +// For RTC sequence jitter. +class SrsRtcSeqJitter +{ +private: + SrsRtcJitter* jitter_; +public: + SrsRtcSeqJitter(uint16_t base); + virtual ~SrsRtcSeqJitter(); +public: + uint16_t correct(uint16_t value); }; class SrsRtcSendTrack @@ -604,12 +665,9 @@ protected: // NACK ARQ ring buffer. SrsRtpRingBuffer* rtp_queue_; protected: - // Current sequence number. - uint64_t seqno_; - // Whether initialized. Note that we should not use seqno_(0) as init state, because it might flip back. - bool init_; - // The jitter to correct ts. - SrsRtcJitter* jitter_; + // The jitter to correct ts and sequence number. + SrsRtcTsJitter* jitter_ts_; + SrsRtcSeqJitter* jitter_seq_; private: // By config, whether no copy. bool nack_no_copy_; diff --git a/trunk/src/core/srs_core_version4.hpp b/trunk/src/core/srs_core_version4.hpp index 163b42847..d35bce585 100644 --- a/trunk/src/core/srs_core_version4.hpp +++ b/trunk/src/core/srs_core_version4.hpp @@ -9,6 +9,6 @@ #define VERSION_MAJOR 4 #define VERSION_MINOR 0 -#define VERSION_REVISION 265 +#define VERSION_REVISION 266 #endif diff --git a/trunk/src/utest/srs_utest_rtc.cpp b/trunk/src/utest/srs_utest_rtc.cpp index c243ad695..b4a63614d 100644 --- a/trunk/src/utest/srs_utest_rtc.cpp +++ b/trunk/src/utest/srs_utest_rtc.cpp @@ -1274,3 +1274,97 @@ VOID TEST(KernelRTCTest, SyncTimestampBySenderReportDuplicated) } } +VOID TEST(KernelRTCTest, JitterTimestamp) +{ + SrsRtcTsJitter jitter(1000); + + // Starts from the base. + EXPECT_EQ(1000, jitter.correct(0)); + + // Start from here. + EXPECT_EQ(1010, jitter.correct(10)); + EXPECT_EQ(1010, jitter.correct(10)); + EXPECT_EQ(1020, jitter.correct(20)); + + // Reset the base for jitter detected. + EXPECT_EQ(1020, jitter.correct(20 + 90*3*1000 + 1)); + EXPECT_EQ(1019, jitter.correct(20 + 90*3*1000)); + EXPECT_EQ(1021, jitter.correct(20 + 90*3*1000 + 2)); + EXPECT_EQ(1019, jitter.correct(20 + 90*3*1000)); + EXPECT_EQ(1020, jitter.correct(20 + 90*3*1000 + 1)); + + // Rollback the timestamp. + EXPECT_EQ(1020, jitter.correct(20)); + EXPECT_EQ(1021, jitter.correct(20 + 1)); + EXPECT_EQ(1021, jitter.correct(21)); + + // Reset for jitter again. + EXPECT_EQ(1021, jitter.correct(21 + 90*3*1000 + 1)); + EXPECT_EQ(1021, jitter.correct(21)); + + // No jitter at edge. + EXPECT_EQ(1021 + 90*3*1000, jitter.correct(21 + 90*3*1000)); + EXPECT_EQ(1021 + 90*3*1000 + 1, jitter.correct(21 + 90*3*1000 + 1)); + EXPECT_EQ(1021 + 1, jitter.correct(21 + 1)); + + // Also safety to decrease the value. + EXPECT_EQ(1021, jitter.correct(21)); + EXPECT_EQ(1010, jitter.correct(10)); + + // Try to reset to 0 base. + EXPECT_EQ(1010, jitter.correct(10 + 90*3*1000 + 1010)); + EXPECT_EQ(0, jitter.correct(10 + 90*3*1000)); + EXPECT_EQ(0, jitter.correct(0)); + + // Also safety to start from zero. + EXPECT_EQ(10, jitter.correct(10)); + EXPECT_EQ(11, jitter.correct(11)); +} + +VOID TEST(KernelRTCTest, JitterSequence) +{ + SrsRtcSeqJitter jitter(100); + + // Starts from the base. + EXPECT_EQ(100, jitter.correct(0)); + + // Normal without jitter. + EXPECT_EQ(101, jitter.correct(1)); + EXPECT_EQ(102, jitter.correct(2)); + EXPECT_EQ(101, jitter.correct(1)); + EXPECT_EQ(103, jitter.correct(3)); + EXPECT_EQ(110, jitter.correct(10)); + + // Reset the base for jitter detected. + EXPECT_EQ(110, jitter.correct(10 + 128 + 1)); + EXPECT_EQ(109, jitter.correct(10 + 128)); + EXPECT_EQ(110, jitter.correct(10 + 128 + 1)); + + // Rollback the timestamp. + EXPECT_EQ(110, jitter.correct(10)); + EXPECT_EQ(111, jitter.correct(10 + 1)); + EXPECT_EQ(111, jitter.correct(11)); + + // Reset for jitter again. + EXPECT_EQ(111, jitter.correct(11 + 128 + 1)); + EXPECT_EQ(111, jitter.correct(11)); + + // No jitter at edge. + EXPECT_EQ(111 + 128, jitter.correct(11 + 128)); + EXPECT_EQ(111 + 128 + 1, jitter.correct(11 + 128 + 1)); + EXPECT_EQ(111 + 1, jitter.correct(11 + 1)); + + // Also safety to decrease the value. + EXPECT_EQ(111, jitter.correct(11)); + EXPECT_EQ(110, jitter.correct(10)); + + // Try to reset to 0 base. + EXPECT_EQ(110, jitter.correct(10 + 128 + 110)); + EXPECT_EQ(0, jitter.correct(10 + 128)); + EXPECT_EQ(0, jitter.correct(0)); + + // Also safety to start from zero. + EXPECT_EQ(10, jitter.correct(10)); + EXPECT_EQ(11, jitter.correct(11)); +} +