Pass callbacks for RtcpReceiver at construction

Bug: webrtc:10680
Change-Id: Ic242008e63a5a86ac30ab5f4041a30dbdb7fc72b
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/170236
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Reviewed-by: Niels Moller <nisse@webrtc.org>
Reviewed-by: Sebastian Jansson <srte@webrtc.org>
Reviewed-by: Åsa Persson <asapersson@webrtc.org>
Commit-Queue: Danil Chapovalov <danilchap@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#30773}
This commit is contained in:
Danil Chapovalov
2020-03-12 09:22:44 +01:00
committed by Commit Bot
parent b8e69efcee
commit bd74d5ca6b
9 changed files with 40 additions and 132 deletions

View File

@ -211,6 +211,9 @@ 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();
configuration.send_bitrate_observer = observers.bitrate_observer;
configuration.send_side_delay_observer = observers.send_delay_observer;
@ -400,9 +403,6 @@ RtpVideoSender::RtpVideoSender(
for (const RtpStreamSender& stream : rtp_streams_) {
// Simulcast has one module for each layer. Set the CNAME on all modules.
stream.rtp_rtcp->SetCNAME(rtp_config_.c_name.c_str());
stream.rtp_rtcp->RegisterRtcpStatisticsCallback(observers.rtcp_stats);
stream.rtp_rtcp->SetReportBlockDataObserver(
observers.report_block_data_observer);
stream.rtp_rtcp->SetMaxRtpPacketSize(rtp_config_.max_packet_size);
stream.rtp_rtcp->RegisterSendPayloadFrequency(rtp_config_.payload_type,
kVideoPayloadTypeFrequency);

View File

@ -86,6 +86,16 @@ class RtpRtcp : public Module, public RtcpFeedbackSenderInterface {
VideoBitrateAllocationObserver* bitrate_allocation_observer = nullptr;
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;
// Estimates the bandwidth available for a set of streams from the same
// client.
@ -417,24 +427,6 @@ class RtpRtcp : public Module, public RtcpFeedbackSenderInterface {
// Returns true if the module is configured to store packets.
virtual bool StorePackets() const = 0;
// Called on receipt of RTCP report block from remote side.
// TODO(https://crbug.com/webrtc/10678): Remove RtcpStatisticsCallback in
// favor of ReportBlockDataObserver.
// TODO(https://crbug.com/webrtc/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().
// TODO(nisse): Replace RegisterRtcpStatisticsCallback and
// RegisterRtcpCnameCallback with construction-time settings in
// RtpRtcp::Configuration.
virtual void RegisterRtcpStatisticsCallback(
RtcpStatisticsCallback* callback) = 0;
virtual RtcpStatisticsCallback* GetRtcpStatisticsCallback() = 0;
virtual void RegisterRtcpCnameCallback(RtcpCnameCallback* callback) = 0;
// TODO(https://crbug.com/webrtc/10680): When callbacks are registered at
// construction, remove this setter.
virtual void SetReportBlockDataObserver(
ReportBlockDataObserver* observer) = 0;
virtual void SetVideoBitrateAllocation(
const VideoBitrateAllocation& bitrate) = 0;

View File

@ -149,10 +149,6 @@ class MockRtpRtcp : public RtpRtcp {
MOCK_METHOD2(SetStorePacketsStatus,
void(bool enable, uint16_t number_to_store));
MOCK_CONST_METHOD0(StorePackets, bool());
MOCK_METHOD1(RegisterRtcpStatisticsCallback, void(RtcpStatisticsCallback*));
MOCK_METHOD0(GetRtcpStatisticsCallback, RtcpStatisticsCallback*());
MOCK_METHOD1(RegisterRtcpCnameCallback, void(RtcpCnameCallback*));
MOCK_METHOD1(SetReportBlockDataObserver, void(ReportBlockDataObserver*));
MOCK_METHOD1(SendFeedbackPacket, bool(const rtcp::TransportFeedback& packet));
MOCK_METHOD1(SendNetworkStateEstimatePacket,
bool(const rtcp::RemoteEstimate& packet));

View File

@ -161,9 +161,9 @@ RTCPReceiver::RTCPReceiver(const RtpRtcp::Configuration& config,
oldest_tmmbr_info_ms_(0),
last_received_rb_ms_(0),
last_increased_sequence_number_ms_(0),
stats_callback_(nullptr),
cname_callback_(nullptr),
report_block_data_observer_(nullptr),
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),
num_skipped_packets_(0),
last_skipped_packets_warning_ms_(clock_->TimeInMilliseconds()) {
@ -662,11 +662,8 @@ void RTCPReceiver::HandleSdes(const CommonHeader& rtcp_block,
for (const rtcp::Sdes::Chunk& chunk : sdes.chunks()) {
received_cnames_[chunk.ssrc] = chunk.cname;
{
rtc::CritScope lock(&feedbacks_lock_);
if (cname_callback_)
cname_callback_->OnCname(chunk.ssrc, chunk.cname);
}
if (cname_callback_)
cname_callback_->OnCname(chunk.ssrc, chunk.cname);
}
packet_information->packet_type_flags |= kRtcpSdes;
}
@ -989,28 +986,6 @@ void RTCPReceiver::NotifyTmmbrUpdated() {
rtp_rtcp_->SetTmmbn(std::move(bounding));
}
void RTCPReceiver::RegisterRtcpStatisticsCallback(
RtcpStatisticsCallback* callback) {
rtc::CritScope cs(&feedbacks_lock_);
stats_callback_ = callback;
}
RtcpStatisticsCallback* RTCPReceiver::GetRtcpStatisticsCallback() {
rtc::CritScope cs(&feedbacks_lock_);
return stats_callback_;
}
void RTCPReceiver::RegisterRtcpCnameCallback(RtcpCnameCallback* callback) {
rtc::CritScope cs(&feedbacks_lock_);
cname_callback_ = callback;
}
void RTCPReceiver::SetReportBlockDataObserver(
ReportBlockDataObserver* observer) {
rtc::CritScope cs(&feedbacks_lock_);
report_block_data_observer_ = observer;
}
// Holding no Critical section.
void RTCPReceiver::TriggerCallbacksFromRtcpPacket(
const PacketInformation& packet_information) {
@ -1114,7 +1089,6 @@ void RTCPReceiver::TriggerCallbacksFromRtcpPacket(
}
if (!receiver_only_) {
rtc::CritScope cs(&feedbacks_lock_);
if (stats_callback_) {
for (const auto& report_block : packet_information.report_blocks) {
RtcpStatistics stats;

View File

@ -113,11 +113,6 @@ class RTCPReceiver final {
// Set new bandwidth and notify remote clients about it.
void NotifyTmmbrUpdated();
void RegisterRtcpStatisticsCallback(RtcpStatisticsCallback* callback);
void RegisterRtcpCnameCallback(RtcpCnameCallback* callback);
RtcpStatisticsCallback* GetRtcpStatisticsCallback();
void SetReportBlockDataObserver(ReportBlockDataObserver* observer);
private:
struct PacketInformation;
struct TmmbrInformation;
@ -220,7 +215,6 @@ class RTCPReceiver final {
const uint32_t main_ssrc_;
const std::set<uint32_t> registered_ssrcs_;
rtc::CriticalSection feedbacks_lock_;
RtcpBandwidthObserver* const rtcp_bandwidth_observer_;
RtcpIntraFrameObserver* const rtcp_intra_frame_observer_;
RtcpLossNotificationObserver* const rtcp_loss_notification_observer_;
@ -267,13 +261,12 @@ class RTCPReceiver final {
// delivered RTP packet to the remote side.
int64_t last_increased_sequence_number_ms_;
RtcpStatisticsCallback* stats_callback_ RTC_GUARDED_BY(feedbacks_lock_);
RtcpCnameCallback* cname_callback_ RTC_GUARDED_BY(feedbacks_lock_);
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* report_block_data_observer_
RTC_GUARDED_BY(feedbacks_lock_);
ReportBlockDataObserver* const report_block_data_observer_;
RtcpPacketTypeCounterObserver* const packet_type_counter_observer_;
RtcpPacketTypeCounter packet_type_counter_;

View File

@ -635,12 +635,13 @@ TEST(RtcpReceiverTest, InjectApp) {
TEST(RtcpReceiverTest, InjectSdesWithOneChunk) {
ReceiverMocks mocks;
RTCPReceiver receiver(DefaultConfiguration(&mocks), &mocks.rtp_rtcp_impl);
MockCnameCallbackImpl callback;
RtpRtcp::Configuration config = DefaultConfiguration(&mocks);
config.rtcp_cname_callback = &callback;
RTCPReceiver receiver(config, &mocks.rtp_rtcp_impl);
receiver.SetRemoteSSRC(kSenderSsrc);
const char kCname[] = "alice@host";
MockCnameCallbackImpl callback;
receiver.RegisterRtcpCnameCallback(&callback);
rtcp::Sdes sdes;
sdes.AddCName(kSenderSsrc, kCname);
@ -1308,11 +1309,11 @@ TEST(RtcpReceiverTest, TmmbrThreeConstraintsTimeOut) {
TEST(RtcpReceiverTest, Callbacks) {
ReceiverMocks mocks;
RTCPReceiver receiver(DefaultConfiguration(&mocks), &mocks.rtp_rtcp_impl);
receiver.SetRemoteSSRC(kSenderSsrc);
MockRtcpCallbackImpl callback;
receiver.RegisterRtcpStatisticsCallback(&callback);
RtpRtcp::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;
@ -1341,35 +1342,16 @@ TEST(RtcpReceiverTest, Callbacks) {
EXPECT_CALL(mocks.rtp_rtcp_impl, OnReceivedRtcpReportBlocks);
EXPECT_CALL(mocks.bandwidth_observer, OnReceivedRtcpReceiverReport);
receiver.IncomingPacket(rr1.Build());
receiver.RegisterRtcpStatisticsCallback(nullptr);
// Add arbitrary numbers, callback should not be called.
rtcp::ReportBlock rb2;
rb2.SetMediaSsrc(kReceiverMainSsrc);
rb2.SetExtHighestSeqNum(kSequenceNumber + 1);
rb2.SetFractionLost(42);
rb2.SetCumulativeLost(137);
rb2.SetJitter(4711);
rtcp::ReceiverReport rr2;
rr2.SetSenderSsrc(kSenderSsrc);
rr2.AddReportBlock(rb2);
EXPECT_CALL(mocks.rtp_rtcp_impl, OnReceivedRtcpReportBlocks);
EXPECT_CALL(mocks.bandwidth_observer, OnReceivedRtcpReceiverReport);
EXPECT_CALL(callback, StatisticsUpdated).Times(0);
receiver.IncomingPacket(rr2.Build());
}
TEST(RtcpReceiverTest,
VerifyBlockAndTimestampObtainedFromReportBlockDataObserver) {
ReceiverMocks mocks;
RTCPReceiver receiver(DefaultConfiguration(&mocks), &mocks.rtp_rtcp_impl);
receiver.SetRemoteSSRC(kSenderSsrc);
MockReportBlockDataObserverImpl observer;
receiver.SetReportBlockDataObserver(&observer);
RtpRtcp::Configuration config = DefaultConfiguration(&mocks);
config.report_block_data_observer = &observer;
RTCPReceiver receiver(config, &mocks.rtp_rtcp_impl);
receiver.SetRemoteSSRC(kSenderSsrc);
const uint8_t kFractionLoss = 3;
const uint32_t kCumulativeLoss = 7;
@ -1414,11 +1396,11 @@ TEST(RtcpReceiverTest,
TEST(RtcpReceiverTest, VerifyRttObtainedFromReportBlockDataObserver) {
ReceiverMocks mocks;
RTCPReceiver receiver(DefaultConfiguration(&mocks), &mocks.rtp_rtcp_impl);
receiver.SetRemoteSSRC(kSenderSsrc);
MockReportBlockDataObserverImpl observer;
receiver.SetReportBlockDataObserver(&observer);
RtpRtcp::Configuration config = DefaultConfiguration(&mocks);
config.report_block_data_observer = &observer;
RTCPReceiver receiver(config, &mocks.rtp_rtcp_impl);
receiver.SetRemoteSSRC(kSenderSsrc);
const int64_t kRttMs = 120;
const uint32_t kDelayNtp = 123000;

View File

@ -670,24 +670,6 @@ bool ModuleRtpRtcpImpl::StorePackets() const {
RtpPacketHistory::StorageMode::kDisabled;
}
void ModuleRtpRtcpImpl::RegisterRtcpStatisticsCallback(
RtcpStatisticsCallback* callback) {
rtcp_receiver_.RegisterRtcpStatisticsCallback(callback);
}
RtcpStatisticsCallback* ModuleRtpRtcpImpl::GetRtcpStatisticsCallback() {
return rtcp_receiver_.GetRtcpStatisticsCallback();
}
void ModuleRtpRtcpImpl::RegisterRtcpCnameCallback(RtcpCnameCallback* callback) {
rtcp_receiver_.RegisterRtcpCnameCallback(callback);
}
void ModuleRtpRtcpImpl::SetReportBlockDataObserver(
ReportBlockDataObserver* observer) {
return rtcp_receiver_.SetReportBlockDataObserver(observer);
}
void ModuleRtpRtcpImpl::SendCombinedRtcpPacket(
std::vector<std::unique_ptr<rtcp::RtcpPacket>> rtcp_packets) {
rtcp_sender_.SendCombinedRtcpPacket(std::move(rtcp_packets));

View File

@ -231,14 +231,6 @@ class ModuleRtpRtcpImpl : public RtpRtcp, public RTCPReceiver::ModuleRtpRtcp {
bool StorePackets() const override;
// Called on receipt of RTCP report block from remote side.
void RegisterRtcpStatisticsCallback(
RtcpStatisticsCallback* callback) override;
RtcpStatisticsCallback* GetRtcpStatisticsCallback() override;
void RegisterRtcpCnameCallback(RtcpCnameCallback* callback) override;
void SetReportBlockDataObserver(ReportBlockDataObserver* observer) override;
void SendCombinedRtcpPacket(
std::vector<std::unique_ptr<rtcp::RtcpPacket>> rtcp_packets) override;

View File

@ -84,7 +84,7 @@ std::unique_ptr<RtpRtcp> CreateRtpRtcpModule(
ReceiveStatistics* receive_statistics,
Transport* outgoing_transport,
RtcpRttStats* rtt_stats,
RtcpPacketTypeCounterObserver* rtcp_packet_type_counter_observer,
ReceiveStatisticsProxy* rtcp_statistics_observer,
uint32_t local_ssrc) {
RtpRtcp::Configuration configuration;
configuration.clock = clock;
@ -93,8 +93,8 @@ std::unique_ptr<RtpRtcp> CreateRtpRtcpModule(
configuration.receive_statistics = receive_statistics;
configuration.outgoing_transport = outgoing_transport;
configuration.rtt_stats = rtt_stats;
configuration.rtcp_packet_type_counter_observer =
rtcp_packet_type_counter_observer;
configuration.rtcp_packet_type_counter_observer = rtcp_statistics_observer;
configuration.rtcp_cname_callback = rtcp_statistics_observer;
configuration.local_media_ssrc = local_ssrc;
std::unique_ptr<RtpRtcp> rtp_rtcp = RtpRtcp::Create(configuration);
@ -256,9 +256,6 @@ RtpVideoStreamReceiver::RtpVideoStreamReceiver(
if (config_.rtp.rtcp_xr.receiver_reference_time_report)
rtp_rtcp_->SetRtcpXrRrtrStatus(true);
// Stats callback for CNAME changes.
rtp_rtcp_->RegisterRtcpCnameCallback(receive_stats_proxy);
process_thread_->RegisterModule(rtp_rtcp_.get(), RTC_FROM_HERE);
if (config_.rtp.lntf.enabled) {