From 4d68314ec87b689792c9db9e2e50b76659bd42d9 Mon Sep 17 00:00:00 2001 From: Mirko Bonadei Date: Fri, 12 Jul 2019 08:36:29 +0000 Subject: [PATCH] Revert "Pass RtpRtcp::Configuration to RtcpReceiver ctor and initialize ssrcs" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit 741b96b175cb20606d5f1aad6339beeaa424b719. Reason for revert: Speculative revert (some perf test are failing) Original change's description: > Pass RtpRtcp::Configuration to RtcpReceiver ctor and initialize ssrcs > > Bug: webrtc:10774 > Change-Id: Iaae717ed1b7373d5cb2246e3ba92fc6ace422b41 > Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/145206 > Commit-Queue: Erik Språng > Reviewed-by: Åsa Persson > Cr-Commit-Position: refs/heads/master@{#28536} TBR=asapersson@webrtc.org,sprang@webrtc.org Change-Id: I877c1e4c025717c3392bce96ef31591dc1ef5f0b No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: webrtc:10774 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/145325 Reviewed-by: Mirko Bonadei Commit-Queue: Mirko Bonadei Cr-Commit-Position: refs/heads/master@{#28551} --- modules/rtp_rtcp/source/rtcp_receiver.cc | 49 +++++++++---------- modules/rtp_rtcp/source/rtcp_receiver.h | 12 ++++- .../rtp_rtcp/source/rtcp_receiver_unittest.cc | 34 +++++-------- modules/rtp_rtcp/source/rtp_rtcp_impl.cc | 16 +++++- test/fuzzers/rtcp_receiver_fuzzer.cc | 8 +-- 5 files changed, 62 insertions(+), 57 deletions(-) diff --git a/modules/rtp_rtcp/source/rtcp_receiver.cc b/modules/rtp_rtcp/source/rtcp_receiver.cc index 99b55efb49..c79143421a 100644 --- a/modules/rtp_rtcp/source/rtcp_receiver.cc +++ b/modules/rtp_rtcp/source/rtcp_receiver.cc @@ -63,8 +63,6 @@ const int64_t kRtcpMinFrameLengthMs = 17; // Maximum number of received RRTRs that will be stored. const size_t kMaxNumberOfStoredRrtrs = 200; -constexpr int32_t kDefaultVideoReportInterval = 1000; -constexpr int32_t kDefaultAudioReportInterval = 5000; } // namespace struct RTCPReceiver::PacketInformation { @@ -120,21 +118,27 @@ struct RTCPReceiver::LastFirStatus { uint8_t sequence_number; }; -RTCPReceiver::RTCPReceiver(const RtpRtcp::Configuration& config, - ModuleRtpRtcp* owner) - : clock_(config.clock), - receiver_only_(config.receiver_only), +RTCPReceiver::RTCPReceiver( + Clock* clock, + bool receiver_only, + RtcpPacketTypeCounterObserver* packet_type_counter_observer, + RtcpBandwidthObserver* rtcp_bandwidth_observer, + RtcpIntraFrameObserver* rtcp_intra_frame_observer, + RtcpLossNotificationObserver* rtcp_loss_notification_observer, + TransportFeedbackObserver* transport_feedback_observer, + VideoBitrateAllocationObserver* bitrate_allocation_observer, + int report_interval_ms, + ModuleRtpRtcp* owner) + : clock_(clock), + receiver_only_(receiver_only), rtp_rtcp_(owner), - rtcp_bandwidth_observer_(config.bandwidth_callback), - rtcp_intra_frame_observer_(config.intra_frame_callback), - rtcp_loss_notification_observer_(config.rtcp_loss_notification_observer), - transport_feedback_observer_(config.transport_feedback_callback), - bitrate_allocation_observer_(config.bitrate_allocation_observer), - report_interval_ms_(config.rtcp_report_interval_ms > 0 - ? config.rtcp_report_interval_ms - : (config.audio ? kDefaultAudioReportInterval - : kDefaultVideoReportInterval)), - main_ssrc_(config.media_send_ssrc.value_or(0)), + rtcp_bandwidth_observer_(rtcp_bandwidth_observer), + rtcp_intra_frame_observer_(rtcp_intra_frame_observer), + rtcp_loss_notification_observer_(rtcp_loss_notification_observer), + transport_feedback_observer_(transport_feedback_observer), + bitrate_allocation_observer_(bitrate_allocation_observer), + report_interval_ms_(report_interval_ms), + main_ssrc_(0), remote_ssrc_(0), remote_sender_rtp_time_(0), xr_rrtr_status_(false), @@ -144,19 +148,10 @@ RTCPReceiver::RTCPReceiver(const RtpRtcp::Configuration& config, last_increased_sequence_number_ms_(0), stats_callback_(nullptr), report_block_data_observer_(nullptr), - packet_type_counter_observer_(config.rtcp_packet_type_counter_observer), + packet_type_counter_observer_(packet_type_counter_observer), num_skipped_packets_(0), - last_skipped_packets_warning_ms_(clock_->TimeInMilliseconds()) { + last_skipped_packets_warning_ms_(clock->TimeInMilliseconds()) { RTC_DCHECK(owner); - if (config.media_send_ssrc) { - registered_ssrcs_.insert(*config.media_send_ssrc); - } - if (config.rtx_send_ssrc) { - registered_ssrcs_.insert(*config.rtx_send_ssrc); - } - if (config.flexfec_sender) { - registered_ssrcs_.insert(config.flexfec_sender->ssrc()); - } } RTCPReceiver::~RTCPReceiver() {} diff --git a/modules/rtp_rtcp/source/rtcp_receiver.h b/modules/rtp_rtcp/source/rtcp_receiver.h index f49b7480c5..e971c15765 100644 --- a/modules/rtp_rtcp/source/rtcp_receiver.h +++ b/modules/rtp_rtcp/source/rtcp_receiver.h @@ -19,7 +19,6 @@ #include "modules/rtp_rtcp/include/report_block_data.h" #include "modules/rtp_rtcp/include/rtcp_statistics.h" -#include "modules/rtp_rtcp/include/rtp_rtcp.h" #include "modules/rtp_rtcp/include/rtp_rtcp_defines.h" #include "modules/rtp_rtcp/source/rtcp_nack_stats.h" #include "modules/rtp_rtcp/source/rtcp_packet/dlrr.h" @@ -52,7 +51,16 @@ class RTCPReceiver { virtual ~ModuleRtpRtcp() = default; }; - RTCPReceiver(const RtpRtcp::Configuration& config, ModuleRtpRtcp* owner); + RTCPReceiver(Clock* clock, + bool receiver_only, + RtcpPacketTypeCounterObserver* packet_type_counter_observer, + RtcpBandwidthObserver* rtcp_bandwidth_observer, + RtcpIntraFrameObserver* rtcp_intra_frame_observer, + RtcpLossNotificationObserver* rtcp_loss_notification_observer, + TransportFeedbackObserver* transport_feedback_observer, + VideoBitrateAllocationObserver* bitrate_allocation_observer, + int report_interval_ms, + ModuleRtpRtcp* owner); virtual ~RTCPReceiver(); void IncomingPacket(const uint8_t* packet, size_t packet_size); diff --git a/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc b/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc index 8a2a89e892..6fa2cdf900 100644 --- a/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc +++ b/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc @@ -132,28 +132,20 @@ class RtcpReceiverTest : public ::testing::Test { protected: RtcpReceiverTest() : system_clock_(1335900000), - rtcp_receiver_( - [&] { - RtpRtcp::Configuration config; - config.clock = &system_clock_; - config.receiver_only = false; - config.rtcp_packet_type_counter_observer = - &packet_type_counter_observer_; - config.bandwidth_callback = &bandwidth_observer_; - config.intra_frame_callback = &intra_frame_observer_; - config.rtcp_loss_notification_observer = - &rtcp_loss_notification_observer_; - config.transport_feedback_callback = - &transport_feedback_observer_; - config.bitrate_allocation_observer = - &bitrate_allocation_observer_; - config.rtcp_report_interval_ms = kRtcpIntervalMs; - config.media_send_ssrc = kReceiverMainSsrc; - config.rtx_send_ssrc = kReceiverExtraSsrc; - return config; - }(), - &rtp_rtcp_impl_) {} + rtcp_receiver_(&system_clock_, + false, + &packet_type_counter_observer_, + &bandwidth_observer_, + &intra_frame_observer_, + &rtcp_loss_notification_observer_, + &transport_feedback_observer_, + &bitrate_allocation_observer_, + kRtcpIntervalMs, + &rtp_rtcp_impl_) {} void SetUp() { + std::set ssrcs = {kReceiverMainSsrc, kReceiverExtraSsrc}; + rtcp_receiver_.SetSsrcs(kReceiverMainSsrc, ssrcs); + rtcp_receiver_.SetRemoteSSRC(kSenderSsrc); } diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl.cc b/modules/rtp_rtcp/source/rtp_rtcp_impl.cc index 1fdb35650d..13f1b354e1 100644 --- a/modules/rtp_rtcp/source/rtp_rtcp_impl.cc +++ b/modules/rtp_rtcp/source/rtp_rtcp_impl.cc @@ -36,6 +36,8 @@ const int64_t kRtpRtcpMaxIdleTimeProcessMs = 5; const int64_t kRtpRtcpRttProcessTimeMs = 1000; const int64_t kRtpRtcpBitrateProcessTimeMs = 10; const int64_t kDefaultExpectedRetransmissionTimeMs = 125; +constexpr int32_t kDefaultVideoReportInterval = 1000; +constexpr int32_t kDefaultAudioReportInterval = 5000; } // namespace RtpRtcp::Configuration::Configuration() = default; @@ -60,7 +62,19 @@ RtpRtcp* RtpRtcp::CreateRtpRtcp(const RtpRtcp::Configuration& configuration) { ModuleRtpRtcpImpl::ModuleRtpRtcpImpl(const Configuration& configuration) : rtcp_sender_(configuration), - rtcp_receiver_(configuration, this), + rtcp_receiver_(configuration.clock, + configuration.receiver_only, + configuration.rtcp_packet_type_counter_observer, + configuration.bandwidth_callback, + configuration.intra_frame_callback, + configuration.rtcp_loss_notification_observer, + configuration.transport_feedback_callback, + configuration.bitrate_allocation_observer, + configuration.rtcp_report_interval_ms > 0 + ? configuration.rtcp_report_interval_ms + : (configuration.audio ? kDefaultAudioReportInterval + : kDefaultVideoReportInterval), + this), clock_(configuration.clock), last_bitrate_process_time_(clock_->TimeInMilliseconds()), last_rtt_process_time_(clock_->TimeInMilliseconds()), diff --git a/test/fuzzers/rtcp_receiver_fuzzer.cc b/test/fuzzers/rtcp_receiver_fuzzer.cc index 46bb9eb68b..f6861214be 100644 --- a/test/fuzzers/rtcp_receiver_fuzzer.cc +++ b/test/fuzzers/rtcp_receiver_fuzzer.cc @@ -7,7 +7,6 @@ * in the file PATENTS. All contributing project authors may * be found in the AUTHORS file in the root of the source tree. */ -#include "modules/rtp_rtcp/include/rtp_rtcp.h" #include "modules/rtp_rtcp/source/rtcp_packet/tmmb_item.h" #include "modules/rtp_rtcp/source/rtcp_receiver.h" #include "rtc_base/checks.h" @@ -40,11 +39,8 @@ void FuzzOneInput(const uint8_t* data, size_t size) { NullModuleRtpRtcp rtp_rtcp_module; SimulatedClock clock(1234); - RtpRtcp::Configuration config; - config.clock = &clock; - config.rtcp_report_interval_ms = kRtcpIntervalMs; - - RTCPReceiver receiver(config, &rtp_rtcp_module); + RTCPReceiver receiver(&clock, false, nullptr, nullptr, nullptr, nullptr, + nullptr, nullptr, kRtcpIntervalMs, &rtp_rtcp_module); receiver.IncomingPacket(data, size); }