Delete RtcpStatisticsCallback from ReceiveStatistics

Update VideoReceiveStream::GetStats to use
StreamStatistician::GetStatistics instead, similarly to the audio
receiver.

Bug: webrtc:10679
Change-Id: I8a701e8a8e921c87895424362dc83500737c916d
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/142233
Reviewed-by: Erik Språng <sprang@webrtc.org>
Reviewed-by: Åsa Persson <asapersson@webrtc.org>
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Commit-Queue: Niels Moller <nisse@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#28793}
This commit is contained in:
Niels Möller
2019-08-06 16:04:12 +02:00
committed by Commit Bot
parent b6685420c4
commit 12ebfa69ba
8 changed files with 52 additions and 153 deletions

View File

@ -61,9 +61,14 @@ class ReceiveStatistics : public ReceiveStatisticsProvider,
~ReceiveStatistics() override = default;
static ReceiveStatistics* Create(Clock* clock) {
return Create(clock, nullptr, nullptr).release();
return Create(clock, nullptr).release();
}
static std::unique_ptr<ReceiveStatistics> Create(
Clock* clock,
StreamDataCountersCallback* rtp_callback);
RTC_DEPRECATED
static std::unique_ptr<ReceiveStatistics> Create(
Clock* clock,
RtcpStatisticsCallback* rtcp_callback,

View File

@ -34,7 +34,6 @@ StreamStatisticianImpl::StreamStatisticianImpl(
uint32_t ssrc,
Clock* clock,
int max_reordering_threshold,
RtcpStatisticsCallback* rtcp_callback,
StreamDataCountersCallback* rtp_callback)
: ssrc_(ssrc),
clock_(clock),
@ -51,7 +50,6 @@ StreamStatisticianImpl::StreamStatisticianImpl(
last_report_inorder_packets_(0),
last_report_old_packets_(0),
last_report_seq_max_(-1),
rtcp_callback_(rtcp_callback),
rtp_callback_(rtp_callback) {}
StreamStatisticianImpl::~StreamStatisticianImpl() = default;
@ -181,48 +179,40 @@ void StreamStatisticianImpl::EnableRetransmitDetection(bool enable) {
bool StreamStatisticianImpl::GetStatistics(RtcpStatistics* statistics,
bool reset) {
{
rtc::CritScope cs(&stream_lock_);
if (!ReceivedRtpPacket()) {
return false;
}
if (!reset) {
if (last_report_inorder_packets_ == 0) {
// No report.
return false;
}
// Just get last report.
*statistics = last_reported_statistics_;
return true;
}
*statistics = CalculateRtcpStatistics();
rtc::CritScope cs(&stream_lock_);
if (!ReceivedRtpPacket()) {
return false;
}
if (rtcp_callback_)
rtcp_callback_->StatisticsUpdated(*statistics, ssrc_);
if (!reset) {
if (last_report_inorder_packets_ == 0) {
// No report.
return false;
}
// Just get last report.
*statistics = last_reported_statistics_;
return true;
}
*statistics = CalculateRtcpStatistics();
return true;
}
bool StreamStatisticianImpl::GetActiveStatisticsAndReset(
RtcpStatistics* statistics) {
{
rtc::CritScope cs(&stream_lock_);
if (clock_->TimeInMilliseconds() - last_receive_time_ms_ >=
kStatisticsTimeoutMs) {
// Not active.
return false;
}
if (!ReceivedRtpPacket()) {
return false;
}
*statistics = CalculateRtcpStatistics();
rtc::CritScope cs(&stream_lock_);
if (clock_->TimeInMilliseconds() - last_receive_time_ms_ >=
kStatisticsTimeoutMs) {
// Not active.
return false;
}
if (!ReceivedRtpPacket()) {
return false;
}
if (rtcp_callback_)
rtcp_callback_->StatisticsUpdated(*statistics, ssrc_);
*statistics = CalculateRtcpStatistics();
return true;
}
@ -355,22 +345,26 @@ bool StreamStatisticianImpl::IsRetransmitOfOldPacket(
return time_diff_ms > rtp_time_stamp_diff_ms + max_delay_ms;
}
std::unique_ptr<ReceiveStatistics> ReceiveStatistics::Create(
Clock* clock,
StreamDataCountersCallback* rtp_callback) {
return absl::make_unique<ReceiveStatisticsImpl>(clock, rtp_callback);
}
std::unique_ptr<ReceiveStatistics> ReceiveStatistics::Create(
Clock* clock,
RtcpStatisticsCallback* rtcp_callback,
StreamDataCountersCallback* rtp_callback) {
return absl::make_unique<ReceiveStatisticsImpl>(clock, rtcp_callback,
rtp_callback);
RTC_CHECK(rtcp_callback == nullptr);
return Create(clock, rtp_callback);
}
ReceiveStatisticsImpl::ReceiveStatisticsImpl(
Clock* clock,
RtcpStatisticsCallback* rtcp_callback,
StreamDataCountersCallback* rtp_callback)
: clock_(clock),
last_returned_ssrc_(0),
max_reordering_threshold_(kDefaultMaxReorderingThreshold),
rtcp_stats_callback_(rtcp_callback),
rtp_stats_callback_(rtp_callback) {}
ReceiveStatisticsImpl::~ReceiveStatisticsImpl() {
@ -410,9 +404,8 @@ StreamStatisticianImpl* ReceiveStatisticsImpl::GetOrCreateStatistician(
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_);
impl = new StreamStatisticianImpl(ssrc, clock_, max_reordering_threshold_,
rtp_stats_callback_);
}
return impl;
}

View File

@ -30,7 +30,6 @@ class StreamStatisticianImpl : public StreamStatistician,
StreamStatisticianImpl(uint32_t ssrc,
Clock* clock,
int max_reordering_threshold,
RtcpStatisticsCallback* rtcp_callback,
StreamDataCountersCallback* rtp_callback);
~StreamStatisticianImpl() override;
@ -104,14 +103,12 @@ class StreamStatisticianImpl : public StreamStatistician,
RtcpStatistics last_reported_statistics_ RTC_GUARDED_BY(&stream_lock_);
// stream_lock_ shouldn't be held when calling callbacks.
RtcpStatisticsCallback* const rtcp_callback_;
StreamDataCountersCallback* const rtp_callback_;
};
class ReceiveStatisticsImpl : public ReceiveStatistics {
public:
ReceiveStatisticsImpl(Clock* clock,
RtcpStatisticsCallback* rtcp_callback,
StreamDataCountersCallback* rtp_callback);
~ReceiveStatisticsImpl() override;
@ -141,7 +138,6 @@ class ReceiveStatisticsImpl : public ReceiveStatistics {
std::map<uint32_t, StreamStatisticianImpl*> statisticians_
RTC_GUARDED_BY(receive_statistics_lock_);
RtcpStatisticsCallback* const rtcp_stats_callback_;
StreamDataCountersCallback* const rtp_stats_callback_;
};
} // namespace webrtc

View File

@ -65,16 +65,11 @@ void IncrementSequenceNumber(RtpPacketReceived* packet) {
IncrementSequenceNumber(packet, 1);
}
void IncrementTimestamp(RtpPacketReceived* packet, uint32_t incr) {
packet->SetTimestamp(packet->Timestamp() + incr);
}
class ReceiveStatisticsTest : public ::testing::Test {
public:
ReceiveStatisticsTest()
: clock_(0),
receive_statistics_(
ReceiveStatistics::Create(&clock_, nullptr, nullptr)) {
receive_statistics_(ReceiveStatistics::Create(&clock_, nullptr)) {
packet1_ = CreateRtpPacket(kSsrc1, kPacketSize1);
packet2_ = CreateRtpPacket(kSsrc2, kPacketSize2);
}
@ -233,67 +228,6 @@ TEST_F(ReceiveStatisticsTest, GetReceiveStreamDataCounters) {
EXPECT_EQ(2u, counters.transmitted.packets);
}
TEST_F(ReceiveStatisticsTest, RtcpCallbacks) {
class TestCallback : public RtcpStatisticsCallback {
public:
TestCallback()
: RtcpStatisticsCallback(), num_calls_(0), ssrc_(0), stats_() {}
~TestCallback() override {}
void StatisticsUpdated(const RtcpStatistics& statistics,
uint32_t ssrc) override {
ssrc_ = ssrc;
stats_ = statistics;
++num_calls_;
}
uint32_t num_calls_;
uint32_t ssrc_;
RtcpStatistics stats_;
} callback;
receive_statistics_ = ReceiveStatistics::Create(&clock_, &callback, nullptr);
receive_statistics_->EnableRetransmitDetection(kSsrc1, true);
// Add some arbitrary data, with loss and jitter.
packet1_.SetSequenceNumber(1);
clock_.AdvanceTimeMilliseconds(7);
IncrementTimestamp(&packet1_, 3);
receive_statistics_->OnRtpPacket(packet1_);
IncrementSequenceNumber(&packet1_, 2);
clock_.AdvanceTimeMilliseconds(9);
IncrementTimestamp(&packet1_, 9);
receive_statistics_->OnRtpPacket(packet1_);
IncrementSequenceNumber(&packet1_, -1);
clock_.AdvanceTimeMilliseconds(13);
IncrementTimestamp(&packet1_, 47);
receive_statistics_->OnRtpPacket(packet1_);
IncrementSequenceNumber(&packet1_, 3);
clock_.AdvanceTimeMilliseconds(11);
IncrementTimestamp(&packet1_, 17);
receive_statistics_->OnRtpPacket(packet1_);
IncrementSequenceNumber(&packet1_);
EXPECT_EQ(0u, callback.num_calls_);
// Call GetStatistics, simulating a timed rtcp sender thread.
RtcpStatistics statistics;
receive_statistics_->GetStatistician(kSsrc1)->GetStatistics(&statistics,
true);
EXPECT_EQ(1u, callback.num_calls_);
EXPECT_EQ(callback.ssrc_, kSsrc1);
EXPECT_EQ(statistics.packets_lost, callback.stats_.packets_lost);
EXPECT_EQ(statistics.extended_highest_sequence_number,
callback.stats_.extended_highest_sequence_number);
EXPECT_EQ(statistics.fraction_lost, callback.stats_.fraction_lost);
EXPECT_EQ(statistics.jitter, callback.stats_.jitter);
EXPECT_EQ(51, statistics.fraction_lost);
EXPECT_EQ(1, statistics.packets_lost);
EXPECT_EQ(5u, statistics.extended_highest_sequence_number);
EXPECT_EQ(177u, statistics.jitter);
}
TEST_F(ReceiveStatisticsTest, SimpleLossComputation) {
packet1_.SetSequenceNumber(1);
receive_statistics_->OnRtpPacket(packet1_);
@ -558,7 +492,7 @@ class RtpTestCallback : public StreamDataCountersCallback {
TEST_F(ReceiveStatisticsTest, RtpCallbacks) {
RtpTestCallback callback;
receive_statistics_ = ReceiveStatistics::Create(&clock_, nullptr, &callback);
receive_statistics_ = ReceiveStatistics::Create(&clock_, &callback);
receive_statistics_->EnableRetransmitDetection(kSsrc1, true);
const size_t kHeaderLength = 20;
@ -621,7 +555,7 @@ TEST_F(ReceiveStatisticsTest, RtpCallbacks) {
TEST_F(ReceiveStatisticsTest, LastPacketReceivedTimestamp) {
RtpTestCallback callback;
receive_statistics_ = ReceiveStatistics::Create(&clock_, nullptr, &callback);
receive_statistics_ = ReceiveStatistics::Create(&clock_, &callback);
clock_.AdvanceTimeMilliseconds(42);
receive_statistics_->OnRtpPacket(packet1_);
@ -634,7 +568,7 @@ TEST_F(ReceiveStatisticsTest, LastPacketReceivedTimestamp) {
TEST_F(ReceiveStatisticsTest, RtpCallbacksFecFirst) {
RtpTestCallback callback;
receive_statistics_ = ReceiveStatistics::Create(&clock_, nullptr, &callback);
receive_statistics_ = ReceiveStatistics::Create(&clock_, &callback);
const uint32_t kHeaderLength = 20;
RtpPacketReceived packet =

View File

@ -661,15 +661,6 @@ void ReceiveStatisticsProxy::RtcpPacketTypesCounterUpdated(
stats_.rtcp_packet_type_counts = packet_counter;
}
void ReceiveStatisticsProxy::StatisticsUpdated(
const webrtc::RtcpStatistics& statistics,
uint32_t ssrc) {
rtc::CritScope lock(&crit_);
if (stats_.ssrc != ssrc)
return;
stats_.rtcp_stats = statistics;
}
void ReceiveStatisticsProxy::OnCname(uint32_t ssrc, absl::string_view cname) {
rtc::CritScope lock(&crit_);
// TODO(pbos): Handle both local and remote ssrcs here and RTC_DCHECK that we

View File

@ -37,7 +37,6 @@ class Clock;
struct CodecSpecificInfo;
class ReceiveStatisticsProxy : public VCMReceiveStatisticsCallback,
public RtcpStatisticsCallback,
public RtcpCnameCallback,
public RtcpPacketTypeCounterObserver,
public StreamDataCountersCallback,
@ -78,9 +77,6 @@ class ReceiveStatisticsProxy : public VCMReceiveStatisticsCallback,
void OnTimingFrameInfoUpdated(const TimingFrameInfo& info) override;
// Overrides RtcpStatisticsCallback.
void StatisticsUpdated(const webrtc::RtcpStatistics& statistics,
uint32_t ssrc) override;
// Overrides RtcpCnameCallback.
void OnCname(uint32_t ssrc, absl::string_view cname) override;

View File

@ -463,26 +463,6 @@ TEST_F(ReceiveStatisticsProxyTest, GetStatsReportsFrameCounts) {
EXPECT_EQ(kDeltaFrames, stats.frame_counts.delta_frames);
}
TEST_F(ReceiveStatisticsProxyTest, GetStatsReportsRtcpStats) {
const uint8_t kFracLost = 0;
const int32_t kCumLost = 1;
const uint32_t kExtSeqNum = 10;
const uint32_t kJitter = 4;
RtcpStatistics rtcp_stats;
rtcp_stats.fraction_lost = kFracLost;
rtcp_stats.packets_lost = kCumLost;
rtcp_stats.extended_highest_sequence_number = kExtSeqNum;
rtcp_stats.jitter = kJitter;
statistics_proxy_->StatisticsUpdated(rtcp_stats, kRemoteSsrc);
VideoReceiveStream::Stats stats = statistics_proxy_->GetStats();
EXPECT_EQ(kFracLost, stats.rtcp_stats.fraction_lost);
EXPECT_EQ(kCumLost, stats.rtcp_stats.packets_lost);
EXPECT_EQ(kExtSeqNum, stats.rtcp_stats.extended_highest_sequence_number);
EXPECT_EQ(kJitter, stats.rtcp_stats.jitter);
}
TEST_F(ReceiveStatisticsProxyTest, GetStatsReportsCName) {
const char* kName = "cName";
statistics_proxy_->OnCname(kRemoteSsrc, kName);

View File

@ -193,8 +193,7 @@ VideoReceiveStream::VideoReceiveStream(
call_stats_(call_stats),
source_tracker_(clock_),
stats_proxy_(&config_, clock_),
rtp_receive_statistics_(
ReceiveStatistics::Create(clock_, &stats_proxy_, &stats_proxy_)),
rtp_receive_statistics_(ReceiveStatistics::Create(clock_, &stats_proxy_)),
timing_(timing),
video_receiver_(clock_, timing_.get()),
rtp_video_stream_receiver_(clock_,
@ -459,7 +458,12 @@ void VideoReceiveStream::Stop() {
}
VideoReceiveStream::Stats VideoReceiveStream::GetStats() const {
return stats_proxy_.GetStats();
VideoReceiveStream::Stats stats = stats_proxy_.GetStats();
StreamStatistician* statistician =
rtp_receive_statistics_->GetStatistician(stats.ssrc);
if (statistician)
statistician->GetStatistics(&stats.rtcp_stats, /*reset=*/false);
return stats;
}
void VideoReceiveStream::UpdateHistograms() {