From 3b67672af749918e1113e8eafc9efe63cd36476a Mon Sep 17 00:00:00 2001 From: Mirko Bonadei Date: Fri, 12 Jul 2019 17:35:05 +0000 Subject: [PATCH] Reland "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 4d68314ec87b689792c9db9e2e50b76659bd42d9. Reason for revert: The culprit was https://webrtc-review.googlesource.com/c/src/+/133169. Original change's description: > Revert "Pass RtpRtcp::Configuration to RtcpReceiver ctor and initialize ssrcs" > > 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} TBR=mbonadei@webrtc.org,asapersson@webrtc.org,sprang@webrtc.org Change-Id: Ib59a7f716a58ca8082fe69020c56054e21646cdf No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: webrtc:10774 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/145402 Reviewed-by: Mirko Bonadei Commit-Queue: Mirko Bonadei Cr-Commit-Position: refs/heads/master@{#28564} --- 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, 57 insertions(+), 62 deletions(-) diff --git a/modules/rtp_rtcp/source/rtcp_receiver.cc b/modules/rtp_rtcp/source/rtcp_receiver.cc index c79143421a..99b55efb49 100644 --- a/modules/rtp_rtcp/source/rtcp_receiver.cc +++ b/modules/rtp_rtcp/source/rtcp_receiver.cc @@ -63,6 +63,8 @@ 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 { @@ -118,27 +120,21 @@ struct RTCPReceiver::LastFirStatus { uint8_t sequence_number; }; -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), +RTCPReceiver::RTCPReceiver(const RtpRtcp::Configuration& config, + ModuleRtpRtcp* owner) + : clock_(config.clock), + receiver_only_(config.receiver_only), rtp_rtcp_(owner), - 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), + 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)), remote_ssrc_(0), remote_sender_rtp_time_(0), xr_rrtr_status_(false), @@ -148,10 +144,19 @@ RTCPReceiver::RTCPReceiver( last_increased_sequence_number_ms_(0), stats_callback_(nullptr), report_block_data_observer_(nullptr), - packet_type_counter_observer_(packet_type_counter_observer), + packet_type_counter_observer_(config.rtcp_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 e971c15765..f49b7480c5 100644 --- a/modules/rtp_rtcp/source/rtcp_receiver.h +++ b/modules/rtp_rtcp/source/rtcp_receiver.h @@ -19,6 +19,7 @@ #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" @@ -51,16 +52,7 @@ class RTCPReceiver { virtual ~ModuleRtpRtcp() = default; }; - 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); + RTCPReceiver(const RtpRtcp::Configuration& config, 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 6fa2cdf900..8a2a89e892 100644 --- a/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc +++ b/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc @@ -132,20 +132,28 @@ class RtcpReceiverTest : public ::testing::Test { protected: RtcpReceiverTest() : system_clock_(1335900000), - 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_) {} + 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_) {} 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 13f1b354e1..1fdb35650d 100644 --- a/modules/rtp_rtcp/source/rtp_rtcp_impl.cc +++ b/modules/rtp_rtcp/source/rtp_rtcp_impl.cc @@ -36,8 +36,6 @@ 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; @@ -62,19 +60,7 @@ RtpRtcp* RtpRtcp::CreateRtpRtcp(const RtpRtcp::Configuration& configuration) { ModuleRtpRtcpImpl::ModuleRtpRtcpImpl(const Configuration& configuration) : rtcp_sender_(configuration), - 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), + rtcp_receiver_(configuration, 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 f6861214be..46bb9eb68b 100644 --- a/test/fuzzers/rtcp_receiver_fuzzer.cc +++ b/test/fuzzers/rtcp_receiver_fuzzer.cc @@ -7,6 +7,7 @@ * 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" @@ -39,8 +40,11 @@ void FuzzOneInput(const uint8_t* data, size_t size) { NullModuleRtpRtcp rtp_rtcp_module; SimulatedClock clock(1234); - RTCPReceiver receiver(&clock, false, nullptr, nullptr, nullptr, nullptr, - nullptr, nullptr, kRtcpIntervalMs, &rtp_rtcp_module); + RtpRtcp::Configuration config; + config.clock = &clock; + config.rtcp_report_interval_ms = kRtcpIntervalMs; + + RTCPReceiver receiver(config, &rtp_rtcp_module); receiver.IncomingPacket(data, size); }