Make ReceiveStatisticsImpl::SetMaxReorderingThreshold apply per ssrc
Bug: webrtc:10669
Change-Id: I9fec43fefe301b1e05eaea774a1453c93c4cc106
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/138202
Reviewed-by: Erik Språng <sprang@webrtc.org>
Reviewed-by: Oskar Sundbom <ossu@webrtc.org>
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Commit-Queue: Niels Moller <nisse@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#28069}
diff --git a/audio/channel_receive.cc b/audio/channel_receive.cc
index 5f59275..25f4ac8 100644
--- a/audio/channel_receive.cc
+++ b/audio/channel_receive.cc
@@ -794,11 +794,12 @@
RTC_DCHECK(worker_thread_checker_.IsCurrent());
// None of these functions can fail.
if (enable) {
- rtp_receive_statistics_->SetMaxReorderingThreshold(max_packets);
+ rtp_receive_statistics_->SetMaxReorderingThreshold(remote_ssrc_,
+ max_packets);
audio_coding_->EnableNack(max_packets);
} else {
rtp_receive_statistics_->SetMaxReorderingThreshold(
- kDefaultMaxReorderingThreshold);
+ remote_ssrc_, kDefaultMaxReorderingThreshold);
audio_coding_->DisableNack();
}
}
diff --git a/modules/rtp_rtcp/include/receive_statistics.h b/modules/rtp_rtcp/include/receive_statistics.h
index c299ea6..36e7b24 100644
--- a/modules/rtp_rtcp/include/receive_statistics.h
+++ b/modules/rtp_rtcp/include/receive_statistics.h
@@ -71,8 +71,14 @@
// Returns a pointer to the statistician of an ssrc.
virtual StreamStatistician* GetStatistician(uint32_t ssrc) const = 0;
- // Sets the max reordering threshold in number of packets.
+ // TODO(bugs.webrtc.org/10669): Deprecated, delete as soon as downstream
+ // projects are updated. This method sets the max reordering threshold of all
+ // current and future streams.
virtual void SetMaxReorderingThreshold(int max_reordering_threshold) = 0;
+
+ // Sets the max reordering threshold in number of packets.
+ virtual void SetMaxReorderingThreshold(uint32_t ssrc,
+ int max_reordering_threshold) = 0;
// Detect retransmissions, enabling updates of the retransmitted counters. The
// default is false.
virtual void EnableRetransmitDetection(uint32_t ssrc, bool enable) = 0;
diff --git a/modules/rtp_rtcp/source/receive_statistics_impl.cc b/modules/rtp_rtcp/source/receive_statistics_impl.cc
index a224b1e..0a9bc96 100644
--- a/modules/rtp_rtcp/source/receive_statistics_impl.cc
+++ b/modules/rtp_rtcp/source/receive_statistics_impl.cc
@@ -33,7 +33,6 @@
StreamStatisticianImpl::StreamStatisticianImpl(
uint32_t ssrc,
Clock* clock,
- bool enable_retransmit_detection,
int max_reordering_threshold,
RtcpStatisticsCallback* rtcp_callback,
StreamDataCountersCallback* rtp_callback)
@@ -42,7 +41,7 @@
incoming_bitrate_(kStatisticsProcessIntervalMs,
RateStatistics::kBpsScale),
max_reordering_threshold_(max_reordering_threshold),
- enable_retransmit_detection_(enable_retransmit_detection),
+ enable_retransmit_detection_(false),
jitter_q4_(0),
cumulative_loss_(0),
last_receive_time_ms_(0),
@@ -368,48 +367,42 @@
}
void ReceiveStatisticsImpl::OnRtpPacket(const RtpPacketReceived& packet) {
- StreamStatisticianImpl* impl;
- {
- rtc::CritScope cs(&receive_statistics_lock_);
- auto it = statisticians_.find(packet.Ssrc());
- if (it != statisticians_.end()) {
- impl = it->second;
- } else {
- impl = new StreamStatisticianImpl(
- packet.Ssrc(), clock_, /* enable_retransmit_detection = */ false,
- max_reordering_threshold_, rtcp_stats_callback_, rtp_stats_callback_);
- statisticians_[packet.Ssrc()] = impl;
- }
- }
// StreamStatisticianImpl instance is created once and only destroyed when
// this whole ReceiveStatisticsImpl is destroyed. StreamStatisticianImpl has
// it's own locking so don't hold receive_statistics_lock_ (potential
// deadlock).
- impl->OnRtpPacket(packet);
+ GetOrCreateStatistician(packet.Ssrc())->OnRtpPacket(packet);
}
void ReceiveStatisticsImpl::FecPacketReceived(const RtpPacketReceived& packet) {
- StreamStatisticianImpl* impl;
- {
- rtc::CritScope cs(&receive_statistics_lock_);
- auto it = statisticians_.find(packet.Ssrc());
- // Ignore FEC if it is the first packet.
- if (it == statisticians_.end())
- return;
- impl = it->second;
+ StreamStatisticianImpl* impl = GetStatistician(packet.Ssrc());
+ // Ignore FEC if it is the first packet.
+ if (impl) {
+ impl->FecPacketReceived(packet);
}
- impl->FecPacketReceived(packet);
}
-StreamStatistician* ReceiveStatisticsImpl::GetStatistician(
+StreamStatisticianImpl* ReceiveStatisticsImpl::GetStatistician(
uint32_t ssrc) const {
rtc::CritScope cs(&receive_statistics_lock_);
- auto it = statisticians_.find(ssrc);
+ const auto& it = statisticians_.find(ssrc);
if (it == statisticians_.end())
return NULL;
return it->second;
}
+StreamStatisticianImpl* ReceiveStatisticsImpl::GetOrCreateStatistician(
+ uint32_t ssrc) {
+ rtc::CritScope cs(&receive_statistics_lock_);
+ StreamStatisticianImpl*& impl = statisticians_[ssrc];
+ if (impl == nullptr) { // new element
+ impl =
+ new StreamStatisticianImpl(ssrc, clock_, max_reordering_threshold_,
+ rtcp_stats_callback_, rtp_stats_callback_);
+ }
+ return impl;
+}
+
void ReceiveStatisticsImpl::SetMaxReorderingThreshold(
int max_reordering_threshold) {
std::map<uint32_t, StreamStatisticianImpl*> statisticians;
@@ -423,21 +416,16 @@
}
}
+void ReceiveStatisticsImpl::SetMaxReorderingThreshold(
+ uint32_t ssrc,
+ int max_reordering_threshold) {
+ GetOrCreateStatistician(ssrc)->SetMaxReorderingThreshold(
+ max_reordering_threshold);
+}
+
void ReceiveStatisticsImpl::EnableRetransmitDetection(uint32_t ssrc,
bool enable) {
- StreamStatisticianImpl* impl;
- {
- rtc::CritScope cs(&receive_statistics_lock_);
- StreamStatisticianImpl*& impl_ref = statisticians_[ssrc];
- if (impl_ref == nullptr) { // new element
- impl_ref = new StreamStatisticianImpl(
- ssrc, clock_, enable, max_reordering_threshold_, rtcp_stats_callback_,
- rtp_stats_callback_);
- return;
- }
- impl = impl_ref;
- }
- impl->EnableRetransmitDetection(enable);
+ GetOrCreateStatistician(ssrc)->EnableRetransmitDetection(enable);
}
std::vector<rtcp::ReportBlock> ReceiveStatisticsImpl::RtcpReportBlocks(
diff --git a/modules/rtp_rtcp/source/receive_statistics_impl.h b/modules/rtp_rtcp/source/receive_statistics_impl.h
index c153281..3ab1160 100644
--- a/modules/rtp_rtcp/source/receive_statistics_impl.h
+++ b/modules/rtp_rtcp/source/receive_statistics_impl.h
@@ -30,7 +30,6 @@
public:
StreamStatisticianImpl(uint32_t ssrc,
Clock* clock,
- bool enable_retransmit_detection,
int max_reordering_threshold,
RtcpStatisticsCallback* rtcp_callback,
StreamDataCountersCallback* rtp_callback);
@@ -125,11 +124,16 @@
// Implements ReceiveStatistics.
void FecPacketReceived(const RtpPacketReceived& packet) override;
- StreamStatistician* GetStatistician(uint32_t ssrc) const override;
+ // Note: More specific return type for use in the implementation.
+ StreamStatisticianImpl* GetStatistician(uint32_t ssrc) const override;
void SetMaxReorderingThreshold(int max_reordering_threshold) override;
+ void SetMaxReorderingThreshold(uint32_t ssrc,
+ int max_reordering_threshold) override;
void EnableRetransmitDetection(uint32_t ssrc, bool enable) override;
private:
+ StreamStatisticianImpl* GetOrCreateStatistician(uint32_t ssrc);
+
Clock* const clock_;
rtc::CriticalSection receive_statistics_lock_;
uint32_t last_returned_ssrc_;
diff --git a/modules/rtp_rtcp/source/receive_statistics_unittest.cc b/modules/rtp_rtcp/source/receive_statistics_unittest.cc
index 2c6dc38..7840b97 100644
--- a/modules/rtp_rtcp/source/receive_statistics_unittest.cc
+++ b/modules/rtp_rtcp/source/receive_statistics_unittest.cc
@@ -383,7 +383,7 @@
TEST_F(ReceiveStatisticsTest, StreamRestartDoesntCountAsLoss) {
RtcpStatistics statistics;
- receive_statistics_->SetMaxReorderingThreshold(200);
+ receive_statistics_->SetMaxReorderingThreshold(kSsrc1, 200);
packet1_.SetSequenceNumber(0);
receive_statistics_->OnRtpPacket(packet1_);
@@ -407,7 +407,7 @@
TEST_F(ReceiveStatisticsTest, CountsLossAfterStreamRestart) {
RtcpStatistics statistics;
- receive_statistics_->SetMaxReorderingThreshold(200);
+ receive_statistics_->SetMaxReorderingThreshold(kSsrc1, 200);
packet1_.SetSequenceNumber(0);
receive_statistics_->OnRtpPacket(packet1_);
@@ -428,7 +428,7 @@
TEST_F(ReceiveStatisticsTest, StreamCanRestartAtSequenceNumberWrapAround) {
RtcpStatistics statistics;
- receive_statistics_->SetMaxReorderingThreshold(200);
+ receive_statistics_->SetMaxReorderingThreshold(kSsrc1, 200);
packet1_.SetSequenceNumber(0xffff - 401);
receive_statistics_->OnRtpPacket(packet1_);
@@ -449,7 +449,7 @@
TEST_F(ReceiveStatisticsTest, StreamRestartNeedsTwoConsecutivePackets) {
RtcpStatistics statistics;
- receive_statistics_->SetMaxReorderingThreshold(200);
+ receive_statistics_->SetMaxReorderingThreshold(kSsrc1, 200);
packet1_.SetSequenceNumber(400);
receive_statistics_->OnRtpPacket(packet1_);
@@ -494,7 +494,7 @@
EXPECT_EQ(0x10001u, statistics.extended_highest_sequence_number);
// Receive a couple packets then wrap around again.
- receive_statistics_->SetMaxReorderingThreshold(200);
+ receive_statistics_->SetMaxReorderingThreshold(kSsrc1, 200);
for (int i = 10; i < 0xffff; i += 150) {
packet1_.SetSequenceNumber(i);
receive_statistics_->OnRtpPacket(packet1_);
diff --git a/video/rtp_video_stream_receiver.cc b/video/rtp_video_stream_receiver.cc
index e5e15c3..641af8a 100644
--- a/video/rtp_video_stream_receiver.cc
+++ b/video/rtp_video_stream_receiver.cc
@@ -148,8 +148,16 @@
const int max_reordering_threshold = (config_.rtp.nack.rtp_history_ms > 0)
? kMaxPacketAgeToNack
: kDefaultMaxReorderingThreshold;
- rtp_receive_statistics_->SetMaxReorderingThreshold(max_reordering_threshold);
-
+ rtp_receive_statistics_->SetMaxReorderingThreshold(config_.rtp.remote_ssrc,
+ max_reordering_threshold);
+ // TODO(nisse): For historic reasons, we applied the above
+ // max_reordering_threshold also for RTX stats, which makes little sense since
+ // we don't NACK rtx packets. Consider deleting the below block, and rely on
+ // the default threshold.
+ if (config_.rtp.rtx_ssrc) {
+ rtp_receive_statistics_->SetMaxReorderingThreshold(
+ config_.rtp.rtx_ssrc, max_reordering_threshold);
+ }
if (config_.rtp.rtcp_xr.receiver_reference_time_report)
rtp_rtcp_->SetRtcpXrRrtrStatus(true);
