Simplify ReceiveStatistics: merge GetActiveStatisticians into RtcpReportBlocks
BUG=webrtc:8016
Change-Id: Ie38a86b730298039915baaac12b7fd97a5440345
Reviewed-on: https://webrtc-review.googlesource.com/1842
Commit-Queue: Danil Chapovalov <danilchap@webrtc.org>
Reviewed-by: Niels Moller <nisse@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#19891}diff --git a/modules/rtp_rtcp/include/receive_statistics.h b/modules/rtp_rtcp/include/receive_statistics.h
index f707e56..ee7dbbb 100644
--- a/modules/rtp_rtcp/include/receive_statistics.h
+++ b/modules/rtp_rtcp/include/receive_statistics.h
@@ -55,8 +55,6 @@
virtual bool IsPacketInOrder(uint16_t sequence_number) const = 0;
};
-typedef std::map<uint32_t, StreamStatistician*> StatisticianMap;
-
class ReceiveStatistics : public ReceiveStatisticsProvider {
public:
~ReceiveStatistics() override = default;
@@ -72,10 +70,6 @@
virtual void FecPacketReceived(const RTPHeader& header,
size_t packet_length) = 0;
- // Returns a map of all statisticians which have seen an incoming packet
- // during the last two seconds.
- virtual StatisticianMap GetActiveStatisticians() const = 0;
-
// Returns a pointer to the statistician of an ssrc.
virtual StreamStatistician* GetStatistician(uint32_t ssrc) const = 0;
diff --git a/modules/rtp_rtcp/source/receive_statistics_impl.cc b/modules/rtp_rtcp/source/receive_statistics_impl.cc
index f1db547..7f3d5bb 100644
--- a/modules/rtp_rtcp/source/receive_statistics_impl.cc
+++ b/modules/rtp_rtcp/source/receive_statistics_impl.cc
@@ -197,6 +197,28 @@
return true;
}
+bool StreamStatisticianImpl::GetActiveStatisticsAndReset(
+ RtcpStatistics* statistics) {
+ {
+ rtc::CritScope cs(&stream_lock_);
+ if (clock_->CurrentNtpInMilliseconds() - last_receive_time_ntp_.ToMs() >=
+ kStatisticsTimeoutMs) {
+ // Not active.
+ return false;
+ }
+ if (received_seq_first_ == 0 &&
+ receive_counters_.transmitted.payload_bytes == 0) {
+ // We have not received anything.
+ return false;
+ }
+
+ *statistics = CalculateRtcpStatistics();
+ }
+
+ rtcp_callback_->StatisticsUpdated(*statistics, ssrc_);
+ return true;
+}
+
RtcpStatistics StreamStatisticianImpl::CalculateRtcpStatistics() {
RtcpStatistics stats;
@@ -296,13 +318,6 @@
return incoming_bitrate_.Rate(clock_->TimeInMilliseconds()).value_or(0);
}
-void StreamStatisticianImpl::LastReceiveTimeNtp(uint32_t* secs,
- uint32_t* frac) const {
- rtc::CritScope cs(&stream_lock_);
- *secs = last_receive_time_ntp_.seconds();
- *frac = last_receive_time_ntp_.fractions();
-}
-
bool StreamStatisticianImpl::IsRetransmitOfOldPacket(
const RTPHeader& header, int64_t min_rtt) const {
rtc::CritScope cs(&stream_lock_);
@@ -380,7 +395,7 @@
StreamStatisticianImpl* impl;
{
rtc::CritScope cs(&receive_statistics_lock_);
- StatisticianImplMap::iterator it = statisticians_.find(header.ssrc);
+ auto it = statisticians_.find(header.ssrc);
if (it != statisticians_.end()) {
impl = it->second;
} else {
@@ -400,7 +415,7 @@
StreamStatisticianImpl* impl;
{
rtc::CritScope cs(&receive_statistics_lock_);
- StatisticianImplMap::iterator it = statisticians_.find(header.ssrc);
+ auto it = statisticians_.find(header.ssrc);
// Ignore FEC if it is the first packet.
if (it == statisticians_.end())
return;
@@ -409,26 +424,10 @@
impl->FecPacketReceived(header, packet_length);
}
-StatisticianMap ReceiveStatisticsImpl::GetActiveStatisticians() const {
- StatisticianMap active_statisticians;
- rtc::CritScope cs(&receive_statistics_lock_);
- for (StatisticianImplMap::const_iterator it = statisticians_.begin();
- it != statisticians_.end(); ++it) {
- uint32_t secs;
- uint32_t frac;
- it->second->LastReceiveTimeNtp(&secs, &frac);
- if (clock_->CurrentNtpInMilliseconds() -
- Clock::NtpToMs(secs, frac) < kStatisticsTimeoutMs) {
- active_statisticians[it->first] = it->second;
- }
- }
- return active_statisticians;
-}
-
StreamStatistician* ReceiveStatisticsImpl::GetStatistician(
uint32_t ssrc) const {
rtc::CritScope cs(&receive_statistics_lock_);
- StatisticianImplMap::const_iterator it = statisticians_.find(ssrc);
+ auto it = statisticians_.find(ssrc);
if (it == statisticians_.end())
return NULL;
return it->second;
@@ -437,9 +436,8 @@
void ReceiveStatisticsImpl::SetMaxReorderingThreshold(
int max_reordering_threshold) {
rtc::CritScope cs(&receive_statistics_lock_);
- for (StatisticianImplMap::iterator it = statisticians_.begin();
- it != statisticians_.end(); ++it) {
- it->second->SetMaxReorderingThreshold(max_reordering_threshold);
+ for (auto& statistician : statisticians_) {
+ statistician.second->SetMaxReorderingThreshold(max_reordering_threshold);
}
}
@@ -482,7 +480,11 @@
std::vector<rtcp::ReportBlock> ReceiveStatisticsImpl::RtcpReportBlocks(
size_t max_blocks) {
- StatisticianMap statisticians = GetActiveStatisticians();
+ std::map<uint32_t, StreamStatisticianImpl*> statisticians;
+ {
+ rtc::CritScope cs(&receive_statistics_lock_);
+ statisticians = statisticians_;
+ }
std::vector<rtcp::ReportBlock> result;
result.reserve(std::min(max_blocks, statisticians.size()));
for (auto& statistician : statisticians) {
@@ -494,7 +496,7 @@
// Do we have receive statistics to send?
RtcpStatistics stats;
- if (!statistician.second->GetStatistics(&stats, true))
+ if (!statistician.second->GetActiveStatisticsAndReset(&stats))
continue;
result.emplace_back();
rtcp::ReportBlock& block = result.back();
diff --git a/modules/rtp_rtcp/source/receive_statistics_impl.h b/modules/rtp_rtcp/source/receive_statistics_impl.h
index 55110ea..dd17d77 100644
--- a/modules/rtp_rtcp/source/receive_statistics_impl.h
+++ b/modules/rtp_rtcp/source/receive_statistics_impl.h
@@ -31,7 +31,9 @@
StreamDataCountersCallback* rtp_callback);
virtual ~StreamStatisticianImpl() {}
+ // |reset| here and in next method restarts calculation of fraction_lost stat.
bool GetStatistics(RtcpStatistics* statistics, bool reset) override;
+ bool GetActiveStatisticsAndReset(RtcpStatistics* statistics);
void GetDataCounters(size_t* bytes_received,
uint32_t* packets_received) const override;
void GetReceiveStreamDataCounters(
@@ -46,7 +48,6 @@
bool retransmitted);
void FecPacketReceived(const RTPHeader& header, size_t packet_length);
void SetMaxReorderingThreshold(int max_reordering_threshold);
- virtual void LastReceiveTimeNtp(uint32_t* secs, uint32_t* frac) const;
private:
bool InOrderPacketInternal(uint16_t sequence_number) const;
@@ -108,7 +109,6 @@
bool retransmitted) override;
void FecPacketReceived(const RTPHeader& header,
size_t packet_length) override;
- StatisticianMap GetActiveStatisticians() const override;
StreamStatistician* GetStatistician(uint32_t ssrc) const override;
void SetMaxReorderingThreshold(int max_reordering_threshold) override;
@@ -125,11 +125,9 @@
void DataCountersUpdated(const StreamDataCounters& counters,
uint32_t ssrc) override;
- typedef std::map<uint32_t, StreamStatisticianImpl*> StatisticianImplMap;
-
Clock* const clock_;
rtc::CriticalSection receive_statistics_lock_;
- StatisticianImplMap statisticians_;
+ std::map<uint32_t, StreamStatisticianImpl*> statisticians_;
RtcpStatisticsCallback* rtcp_stats_callback_;
StreamDataCountersCallback* rtp_stats_callback_;
diff --git a/modules/rtp_rtcp/source/receive_statistics_unittest.cc b/modules/rtp_rtcp/source/receive_statistics_unittest.cc
index 283f51b..a832383 100644
--- a/modules/rtp_rtcp/source/receive_statistics_unittest.cc
+++ b/modules/rtp_rtcp/source/receive_statistics_unittest.cc
@@ -71,8 +71,7 @@
EXPECT_EQ(600u, bytes_received);
EXPECT_EQ(2u, packets_received);
- StatisticianMap statisticians = receive_statistics_->GetActiveStatisticians();
- EXPECT_EQ(2u, statisticians.size());
+ EXPECT_EQ(2u, receive_statistics_->RtcpReportBlocks(3).size());
// Add more incoming packets and verify that they are registered in both
// access methods.
receive_statistics_->IncomingPacket(header1_, kPacketSize1, false);
@@ -80,13 +79,6 @@
receive_statistics_->IncomingPacket(header2_, kPacketSize2, false);
++header2_.sequenceNumber;
- statisticians[kSsrc1]->GetDataCounters(&bytes_received, &packets_received);
- EXPECT_EQ(300u, bytes_received);
- EXPECT_EQ(3u, packets_received);
- statisticians[kSsrc2]->GetDataCounters(&bytes_received, &packets_received);
- EXPECT_EQ(900u, bytes_received);
- EXPECT_EQ(3u, packets_received);
-
receive_statistics_->GetStatistician(kSsrc1)->GetDataCounters(
&bytes_received, &packets_received);
EXPECT_EQ(300u, bytes_received);
@@ -103,26 +95,22 @@
clock_.AdvanceTimeMilliseconds(1000);
receive_statistics_->IncomingPacket(header2_, kPacketSize2, false);
++header2_.sequenceNumber;
- StatisticianMap statisticians = receive_statistics_->GetActiveStatisticians();
// Nothing should time out since only 1000 ms has passed since the first
// packet came in.
- EXPECT_EQ(2u, statisticians.size());
+ EXPECT_EQ(2u, receive_statistics_->RtcpReportBlocks(3).size());
clock_.AdvanceTimeMilliseconds(7000);
// kSsrc1 should have timed out.
- statisticians = receive_statistics_->GetActiveStatisticians();
- EXPECT_EQ(1u, statisticians.size());
+ EXPECT_EQ(1u, receive_statistics_->RtcpReportBlocks(3).size());
clock_.AdvanceTimeMilliseconds(1000);
// kSsrc2 should have timed out.
- statisticians = receive_statistics_->GetActiveStatisticians();
- EXPECT_EQ(0u, statisticians.size());
+ EXPECT_EQ(0u, receive_statistics_->RtcpReportBlocks(3).size());
receive_statistics_->IncomingPacket(header1_, kPacketSize1, false);
++header1_.sequenceNumber;
// kSsrc1 should be active again and the data counters should have survived.
- statisticians = receive_statistics_->GetActiveStatisticians();
- EXPECT_EQ(1u, statisticians.size());
+ EXPECT_EQ(1u, receive_statistics_->RtcpReportBlocks(3).size());
StreamStatistician* statistician =
receive_statistics_->GetStatistician(kSsrc1);
ASSERT_TRUE(statistician != NULL);
