zqz/platform-external-webrtc
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Delete RtcpStatisticsCallback in favor of ReportBlockDataObserver

Browse Source 
Bug: webrtc:10678
Change-Id: Ie016cbc47dbba15176fc5e7ad7d01a438db7dfb3
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/218842
Reviewed-by: Erik Språng <sprang@webrtc.org>
Commit-Queue: Danil Chapovalov <danilchap@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#34013}
This commit is contained in:
Danil Chapovalov
2021-05-14 15:39:23 +02:00
committed by WebRTC LUCI CQ
parent f217334d99
commit f01c2c96f2
12 changed files with 62 additions and 117 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
call/rtp_transport_controller_send_interface.h
View File

@ -52,7 +52,6 @@ struct RtpSenderObservers {
RtcpRttStats* rtcp_rtt_stats;
RtcpIntraFrameObserver* intra_frame_callback;
RtcpLossNotificationObserver* rtcp_loss_notification_observer;
RtcpStatisticsCallback* rtcp_stats;
ReportBlockDataObserver* report_block_data_observer;
StreamDataCountersCallback* rtp_stats;
BitrateStatisticsObserver* bitrate_observer;

1
call/rtp_video_sender.cc
View File

@ -216,7 +216,6 @@ std::vector<RtpStreamSender> CreateRtpStreamSenders(
configuration.rtt_stats = observers.rtcp_rtt_stats;
configuration.rtcp_packet_type_counter_observer =
observers.rtcp_type_observer;
configuration.rtcp_statistics_callback = observers.rtcp_stats;
configuration.report_block_data_observer =
observers.report_block_data_observer;
configuration.paced_sender = transport->packet_sender();

7
call/rtp_video_sender_unittest.cc
View File

@ -61,7 +61,6 @@ class MockRtcpIntraFrameObserver : public RtcpIntraFrameObserver {
RtpSenderObservers CreateObservers(
RtcpRttStats* rtcp_rtt_stats,
RtcpIntraFrameObserver* intra_frame_callback,
RtcpStatisticsCallback* rtcp_stats,
ReportBlockDataObserver* report_block_data_observer,
StreamDataCountersCallback* rtp_stats,
BitrateStatisticsObserver* bitrate_observer,
@ -73,7 +72,6 @@ RtpSenderObservers CreateObservers(
observers.rtcp_rtt_stats = rtcp_rtt_stats;
observers.intra_frame_callback = intra_frame_callback;
observers.rtcp_loss_notification_observer = nullptr;
observers.rtcp_stats = rtcp_stats;
observers.report_block_data_observer = report_block_data_observer;
observers.rtp_stats = rtp_stats;
observers.bitrate_observer = bitrate_observer;
@ -147,9 +145,8 @@ class RtpVideoSenderTestFixture {
time_controller_.GetClock(), suspended_ssrcs, suspended_payload_states,
config_.rtp, config_.rtcp_report_interval_ms, &transport_,
CreateObservers(nullptr, &encoder_feedback_, &stats_proxy_,
&stats_proxy_, &stats_proxy_, &stats_proxy_,
frame_count_observer, &stats_proxy_, &stats_proxy_,
&send_delay_stats_),
&stats_proxy_, &stats_proxy_, frame_count_observer,
&stats_proxy_, &stats_proxy_, &send_delay_stats_),
&transport_controller_, &event_log_, &retransmission_rate_limiter_,
std::make_unique<FecControllerDefault>(time_controller_.GetClock()),
nullptr, CryptoOptions{}, frame_transformer);

10
modules/rtp_rtcp/include/rtcp_statistics.h
View File

@ -18,6 +18,8 @@
namespace webrtc {
// Statistics for an RTCP channel
// TODO(bugs.webrtc.org/10678): Remove remaining usages of this struct in favor
// of RTCPReportBlock, rtcp::ReportBlock or other similar structs.
struct RtcpStatistics {
uint8_t fraction_lost = 0;
int32_t packets_lost = 0; // Defined as a 24 bit signed integer in RTCP
@ -25,14 +27,6 @@ struct RtcpStatistics {
uint32_t jitter = 0;
};
class RtcpStatisticsCallback {
public:
virtual ~RtcpStatisticsCallback() {}
virtual void StatisticsUpdated(const RtcpStatistics& statistics,
uint32_t ssrc) = 0;
};
// Statistics for RTCP packet types.
struct RtcpPacketTypeCounter {
RtcpPacketTypeCounter()

13
modules/rtp_rtcp/source/rtcp_receiver.cc
View File

@ -142,7 +142,6 @@ RTCPReceiver::RTCPReceiver(const RtpRtcpInterface::Configuration& config,
xr_rrtr_status_(config.non_sender_rtt_measurement),
xr_rr_rtt_ms_(0),
oldest_tmmbr_info_ms_(0),
stats_callback_(config.rtcp_statistics_callback),
cname_callback_(config.rtcp_cname_callback),
report_block_data_observer_(config.report_block_data_observer),
packet_type_counter_observer_(config.rtcp_packet_type_counter_observer),
@ -1105,18 +1104,6 @@ void RTCPReceiver::TriggerCallbacksFromRtcpPacket(
}
if (!receiver_only_) {
if (stats_callback_) {
for (const auto& report_block : packet_information.report_blocks) {
RtcpStatistics stats;
stats.packets_lost = report_block.packets_lost;
stats.extended_highest_sequence_number =
report_block.extended_highest_sequence_number;
stats.fraction_lost = report_block.fraction_lost;
stats.jitter = report_block.jitter;
stats_callback_->StatisticsUpdated(stats, report_block.source_ssrc);
}
}
if (report_block_data_observer_) {
for (const auto& report_block_data :
packet_information.report_block_datas) {

4
modules/rtp_rtcp/source/rtcp_receiver.h
View File

@ -340,11 +340,7 @@ class RTCPReceiver final {
// delivered RTP packet to the remote side.
Timestamp last_increased_sequence_number_ = Timestamp::PlusInfinity();
RtcpStatisticsCallback* const stats_callback_;
RtcpCnameCallback* const cname_callback_;
// TODO(hbos): Remove RtcpStatisticsCallback in favor of
// ReportBlockDataObserver; the ReportBlockData contains a superset of the
// RtcpStatistics data.
ReportBlockDataObserver* const report_block_data_observer_;
RtcpPacketTypeCounterObserver* const packet_type_counter_observer_;

45
modules/rtp_rtcp/source/rtcp_receiver_unittest.cc
View File

@ -87,14 +87,6 @@ class MockRtcpLossNotificationObserver : public RtcpLossNotificationObserver {
(override));
};
class MockRtcpCallbackImpl : public RtcpStatisticsCallback {
public:
MOCK_METHOD(void,
StatisticsUpdated,
(const RtcpStatistics&, uint32_t),
(override));
};
class MockCnameCallbackImpl : public RtcpCnameCallback {
public:
MOCK_METHOD(void, OnCname, (uint32_t, absl::string_view), (override));
@ -1280,43 +1272,6 @@ TEST(RtcpReceiverTest, TmmbrThreeConstraintsTimeOut) {
Property(&rtcp::TmmbItem::ssrc, Eq(kSenderSsrc + 2))));
}
TEST(RtcpReceiverTest, Callbacks) {
ReceiverMocks mocks;
MockRtcpCallbackImpl callback;
RtpRtcpInterface::Configuration config = DefaultConfiguration(&mocks);
config.rtcp_statistics_callback = &callback;
RTCPReceiver receiver(config, &mocks.rtp_rtcp_impl);
receiver.SetRemoteSSRC(kSenderSsrc);
const uint8_t kFractionLoss = 3;
const uint32_t kCumulativeLoss = 7;
const uint32_t kJitter = 9;
const uint16_t kSequenceNumber = 1234;
// First packet, all numbers should just propagate.
rtcp::ReportBlock rb1;
rb1.SetMediaSsrc(kReceiverMainSsrc);
rb1.SetExtHighestSeqNum(kSequenceNumber);
rb1.SetFractionLost(kFractionLoss);
rb1.SetCumulativeLost(kCumulativeLoss);
rb1.SetJitter(kJitter);
rtcp::ReceiverReport rr1;
rr1.SetSenderSsrc(kSenderSsrc);
rr1.AddReportBlock(rb1);
EXPECT_CALL(callback,
StatisticsUpdated(
AllOf(Field(&RtcpStatistics::fraction_lost, kFractionLoss),
Field(&RtcpStatistics::packets_lost, kCumulativeLoss),
Field(&RtcpStatistics::extended_highest_sequence_number,
kSequenceNumber),
Field(&RtcpStatistics::jitter, kJitter)),
kReceiverMainSsrc));
EXPECT_CALL(mocks.rtp_rtcp_impl, OnReceivedRtcpReportBlocks);
EXPECT_CALL(mocks.bandwidth_observer, OnReceivedRtcpReceiverReport);
receiver.IncomingPacket(rr1.Build());
}
TEST(RtcpReceiverTest,
VerifyBlockAndTimestampObtainedFromReportBlockDataObserver) {
ReceiverMocks mocks;

3
modules/rtp_rtcp/source/rtp_rtcp_interface.h
View File

@ -77,13 +77,10 @@ class RtpRtcpInterface : public RtcpFeedbackSenderInterface {
RtcpRttStats* rtt_stats = nullptr;
RtcpPacketTypeCounterObserver* rtcp_packet_type_counter_observer = nullptr;
// Called on receipt of RTCP report block from remote side.
// TODO(bugs.webrtc.org/10678): Remove RtcpStatisticsCallback in
// favor of ReportBlockDataObserver.
// TODO(bugs.webrtc.org/10679): Consider whether we want to use
// only getters or only callbacks. If we decide on getters, the
// ReportBlockDataObserver should also be removed in favor of
// GetLatestReportBlockData().
RtcpStatisticsCallback* rtcp_statistics_callback = nullptr;
RtcpCnameCallback* rtcp_cname_callback = nullptr;
ReportBlockDataObserver* report_block_data_observer = nullptr;

20
video/send_statistics_proxy.cc
View File

@ -1285,17 +1285,6 @@ void SendStatisticsProxy::RtcpPacketTypesCounterUpdated(
uma_container_->first_rtcp_stats_time_ms_ = clock_->TimeInMilliseconds();
}
void SendStatisticsProxy::StatisticsUpdated(const RtcpStatistics& statistics,
uint32_t ssrc) {
MutexLock lock(&mutex_);
VideoSendStream::StreamStats* stats = GetStatsEntry(ssrc);
if (!stats)
return;
stats->rtcp_stats = statistics;
uma_container_->report_block_stats_.Store(ssrc, statistics);
}
void SendStatisticsProxy::OnReportBlockDataUpdated(
ReportBlockData report_block_data) {
MutexLock lock(&mutex_);
@ -1303,6 +1292,15 @@ void SendStatisticsProxy::OnReportBlockDataUpdated(
GetStatsEntry(report_block_data.report_block().source_ssrc);
if (!stats)
return;
const RTCPReportBlock& report_block = report_block_data.report_block();
stats->rtcp_stats.fraction_lost = report_block.fraction_lost;
stats->rtcp_stats.packets_lost = report_block.packets_lost;
stats->rtcp_stats.extended_highest_sequence_number =
report_block.extended_highest_sequence_number;
stats->rtcp_stats.jitter = report_block.jitter;
uma_container_->report_block_stats_.Store(report_block.source_ssrc,
stats->rtcp_stats);
stats->report_block_data = std::move(report_block_data);
}

4
video/send_statistics_proxy.h
View File

@ -37,7 +37,6 @@
namespace webrtc {
class SendStatisticsProxy : public VideoStreamEncoderObserver,
public RtcpStatisticsCallback,
public ReportBlockDataObserver,
public RtcpPacketTypeCounterObserver,
public StreamDataCountersCallback,
@ -106,9 +105,6 @@ class SendStatisticsProxy : public VideoStreamEncoderObserver,
int GetSendFrameRate() const;
protected:
// From RtcpStatisticsCallback.
void StatisticsUpdated(const RtcpStatistics& statistics,
uint32_t ssrc) override;
// From ReportBlockDataObserver.
void OnReportBlockDataUpdated(ReportBlockData report_block_data) override;
// From RtcpPacketTypeCounterObserver.

70
video/send_statistics_proxy_unittest.cc
View File

@ -174,29 +174,51 @@ class SendStatisticsProxyTest : public ::testing::Test {
VideoSendStream::Stats expected_;
};
TEST_F(SendStatisticsProxyTest, RtcpStatistics) {
RtcpStatisticsCallback* callback = statistics_proxy_.get();
for (const auto& ssrc : config_.rtp.ssrcs) {
TEST_F(SendStatisticsProxyTest, ReportBlockDataObserver) {
ReportBlockDataObserver* callback = statistics_proxy_.get();
for (uint32_t ssrc : config_.rtp.ssrcs) {
VideoSendStream::StreamStats& ssrc_stats = expected_.substreams[ssrc];
// Add statistics with some arbitrary, but unique, numbers.
uint32_t offset = ssrc * sizeof(RtcpStatistics);
ssrc_stats.rtcp_stats.packets_lost = offset;
ssrc_stats.rtcp_stats.extended_highest_sequence_number = offset + 1;
ssrc_stats.rtcp_stats.fraction_lost = offset + 2;
ssrc_stats.rtcp_stats.jitter = offset + 3;
callback->StatisticsUpdated(ssrc_stats.rtcp_stats, ssrc);
RTCPReportBlock report_block;
report_block.source_ssrc = ssrc;
report_block.packets_lost = offset;
report_block.extended_highest_sequence_number = offset + 1;
report_block.fraction_lost = offset + 2;
report_block.jitter = offset + 3;
ssrc_stats.rtcp_stats.packets_lost = report_block.packets_lost;
ssrc_stats.rtcp_stats.extended_highest_sequence_number =
report_block.extended_highest_sequence_number;
ssrc_stats.rtcp_stats.fraction_lost = report_block.fraction_lost;
ssrc_stats.rtcp_stats.jitter = report_block.jitter;
ReportBlockData data;
data.SetReportBlock(report_block, 0);
callback->OnReportBlockDataUpdated(data);
}
for (const auto& ssrc : config_.rtp.rtx.ssrcs) {
for (uint32_t ssrc : config_.rtp.rtx.ssrcs) {
VideoSendStream::StreamStats& ssrc_stats = expected_.substreams[ssrc];
// Add statistics with some arbitrary, but unique, numbers.
uint32_t offset = ssrc * sizeof(RtcpStatistics);
ssrc_stats.rtcp_stats.packets_lost = offset;
ssrc_stats.rtcp_stats.extended_highest_sequence_number = offset + 1;
ssrc_stats.rtcp_stats.fraction_lost = offset + 2;
ssrc_stats.rtcp_stats.jitter = offset + 3;
callback->StatisticsUpdated(ssrc_stats.rtcp_stats, ssrc);
RTCPReportBlock report_block;
report_block.source_ssrc = ssrc;
report_block.packets_lost = offset;
report_block.extended_highest_sequence_number = offset + 1;
report_block.fraction_lost = offset + 2;
report_block.jitter = offset + 3;
ssrc_stats.rtcp_stats.packets_lost = report_block.packets_lost;
ssrc_stats.rtcp_stats.extended_highest_sequence_number =
report_block.extended_highest_sequence_number;
ssrc_stats.rtcp_stats.fraction_lost = report_block.fraction_lost;
ssrc_stats.rtcp_stats.jitter = report_block.jitter;
ReportBlockData data;
data.SetReportBlock(report_block, 0);
callback->OnReportBlockDataUpdated(data);
}
VideoSendStream::Stats stats = statistics_proxy_->GetStats();
ExpectEqual(expected_, stats);
@ -2171,10 +2193,13 @@ TEST_F(SendStatisticsProxyTest, NoSubstreams) {
std::max(*absl::c_max_element(config_.rtp.ssrcs),
*absl::c_max_element(config_.rtp.rtx.ssrcs)) +
1;
// From RtcpStatisticsCallback.
RtcpStatistics rtcp_stats;
RtcpStatisticsCallback* rtcp_callback = statistics_proxy_.get();
rtcp_callback->StatisticsUpdated(rtcp_stats, excluded_ssrc);
// From ReportBlockDataObserver.
ReportBlockDataObserver* rtcp_callback = statistics_proxy_.get();
RTCPReportBlock report_block;
report_block.source_ssrc = excluded_ssrc;
ReportBlockData data;
data.SetReportBlock(report_block, 0);
rtcp_callback->OnReportBlockDataUpdated(data);
// From BitrateStatisticsObserver.
uint32_t total = 0;
@ -2221,9 +2246,12 @@ TEST_F(SendStatisticsProxyTest, EncodedResolutionTimesOut) {
// Update the first SSRC with bogus RTCP stats to make sure that encoded
// resolution still times out (no global timeout for all stats).
RtcpStatistics rtcp_statistics;
RtcpStatisticsCallback* rtcp_stats = statistics_proxy_.get();
rtcp_stats->StatisticsUpdated(rtcp_statistics, config_.rtp.ssrcs[0]);
ReportBlockDataObserver* rtcp_callback = statistics_proxy_.get();
RTCPReportBlock report_block;
report_block.source_ssrc = config_.rtp.ssrcs[0];
ReportBlockData data;
data.SetReportBlock(report_block, 0);
rtcp_callback->OnReportBlockDataUpdated(data);
// Report stats for second SSRC to make sure it's not outdated along with the
// first SSRC.

1
video/video_send_stream_impl.cc
View File

@ -146,7 +146,6 @@ RtpSenderObservers CreateObservers(RtcpRttStats* call_stats,
observers.rtcp_rtt_stats = call_stats;
observers.intra_frame_callback = encoder_feedback;
observers.rtcp_loss_notification_observer = encoder_feedback;
observers.rtcp_stats = stats_proxy;
observers.report_block_data_observer = stats_proxy;
observers.rtp_stats = stats_proxy;
observers.bitrate_observer = stats_proxy;