From 7769dc87d71fb4b2406fbd10b2ee2fca89f7fed0 Mon Sep 17 00:00:00 2001 From: Danil Chapovalov Date: Tue, 28 Jun 2022 12:38:34 +0200 Subject: [PATCH] Detach legacy RtpRtcp from Module interface Bug: webrtc:7219 Change-Id: I5faf8f68b043994a86d227926c13b07d0141f382 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/267063 Auto-Submit: Danil Chapovalov Commit-Queue: Emil Lundmark Reviewed-by: Emil Lundmark Cr-Commit-Position: refs/heads/main@{#37353} --- modules/rtp_rtcp/BUILD.gn | 1 - modules/rtp_rtcp/include/rtp_rtcp.h | 6 ++++-- modules/rtp_rtcp/source/rtp_rtcp_impl.cc | 23 ----------------------- modules/rtp_rtcp/source/rtp_rtcp_impl.h | 5 ----- 4 files changed, 4 insertions(+), 31 deletions(-) diff --git a/modules/rtp_rtcp/BUILD.gn b/modules/rtp_rtcp/BUILD.gn index 300d68a950..d2ad6cb813 100644 --- a/modules/rtp_rtcp/BUILD.gn +++ b/modules/rtp_rtcp/BUILD.gn @@ -351,7 +351,6 @@ rtc_source_set("rtp_rtcp_legacy") { deps = [ ":rtp_rtcp", ":rtp_rtcp_format", - "..:module_api", "..:module_fec_api", "../../api:rtp_headers", "../../api:transport_api", diff --git a/modules/rtp_rtcp/include/rtp_rtcp.h b/modules/rtp_rtcp/include/rtp_rtcp.h index 727fc6e649..c71d7f0c3d 100644 --- a/modules/rtp_rtcp/include/rtp_rtcp.h +++ b/modules/rtp_rtcp/include/rtp_rtcp.h @@ -14,13 +14,12 @@ #include #include "absl/base/attributes.h" -#include "modules/include/module.h" #include "modules/rtp_rtcp/source/rtp_rtcp_interface.h" namespace webrtc { // DEPRECATED. Do not use. -class RtpRtcp : public Module, public RtpRtcpInterface { +class RtpRtcp : public RtpRtcpInterface { public: // Instantiates a deprecated version of the RtpRtcp module. static std::unique_ptr ABSL_DEPRECATED("") @@ -36,6 +35,9 @@ class RtpRtcp : public Module, public RtpRtcpInterface { void SendPictureLossIndication() { SendRTCP(kRtcpPli); } // using FIR, https://tools.ietf.org/html/rfc5104#section-4.3.1.2 void SendFullIntraRequest() { SendRTCP(kRtcpFir); } + + // Process any pending tasks such as timeouts. + virtual void Process() = 0; }; } // namespace webrtc diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl.cc b/modules/rtp_rtcp/source/rtp_rtcp_impl.cc index 0114c99edd..e138ebaf12 100644 --- a/modules/rtp_rtcp/source/rtp_rtcp_impl.cc +++ b/modules/rtp_rtcp/source/rtp_rtcp_impl.cc @@ -36,7 +36,6 @@ namespace webrtc { namespace { -const int64_t kRtpRtcpMaxIdleTimeProcessMs = 5; const int64_t kRtpRtcpRttProcessTimeMs = 1000; const int64_t kRtpRtcpBitrateProcessTimeMs = 10; const int64_t kDefaultExpectedRetransmissionTimeMs = 125; @@ -71,8 +70,6 @@ ModuleRtpRtcpImpl::ModuleRtpRtcpImpl(const Configuration& configuration) clock_(configuration.clock), last_bitrate_process_time_(clock_->TimeInMilliseconds()), last_rtt_process_time_(clock_->TimeInMilliseconds()), - next_process_time_(clock_->TimeInMilliseconds() + - kRtpRtcpMaxIdleTimeProcessMs), packet_overhead_(28), // IPV4 UDP. nack_last_time_sent_full_ms_(0), nack_last_seq_number_sent_(0), @@ -95,29 +92,14 @@ ModuleRtpRtcpImpl::ModuleRtpRtcpImpl(const Configuration& configuration) ModuleRtpRtcpImpl::~ModuleRtpRtcpImpl() = default; -// Returns the number of milliseconds until the module want a worker thread -// to call Process. -int64_t ModuleRtpRtcpImpl::TimeUntilNextProcess() { - return std::max(0, - next_process_time_ - clock_->TimeInMilliseconds()); -} - // Process any pending tasks such as timeouts (non time critical events). void ModuleRtpRtcpImpl::Process() { const int64_t now = clock_->TimeInMilliseconds(); - // TODO(bugs.webrtc.org/11581): Figure out why we need to call Process() 200 - // times a second. - next_process_time_ = now + kRtpRtcpMaxIdleTimeProcessMs; if (rtp_sender_) { if (now >= last_bitrate_process_time_ + kRtpRtcpBitrateProcessTimeMs) { rtp_sender_->packet_sender.ProcessBitrateAndNotifyObservers(); last_bitrate_process_time_ = now; - // TODO(bugs.webrtc.org/11581): Is this a bug? At the top of the function, - // next_process_time_ is incremented by 5ms, here we effectively do a - // std::min() of (now + 5ms, now + 10ms). Seems like this is a no-op? - next_process_time_ = - std::min(next_process_time_, now + kRtpRtcpBitrateProcessTimeMs); } } @@ -183,11 +165,6 @@ void ModuleRtpRtcpImpl::Process() { // Get processed rtt. if (process_rtt) { last_rtt_process_time_ = now; - // TODO(bugs.webrtc.org/11581): Is this a bug? At the top of the function, - // next_process_time_ is incremented by 5ms, here we effectively do a - // std::min() of (now + 5ms, now + 1000ms). Seems like this is a no-op? - next_process_time_ = std::min( - next_process_time_, last_rtt_process_time_ + kRtpRtcpRttProcessTimeMs); if (rtt_stats_) { // Make sure we have a valid RTT before setting. int64_t last_rtt = rtt_stats_->LastProcessedRtt(); diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl.h b/modules/rtp_rtcp/source/rtp_rtcp_impl.h index c135858156..7521b9d96a 100644 --- a/modules/rtp_rtcp/source/rtp_rtcp_impl.h +++ b/modules/rtp_rtcp/source/rtp_rtcp_impl.h @@ -51,10 +51,6 @@ class ModuleRtpRtcpImpl : public RtpRtcp, public RTCPReceiver::ModuleRtpRtcp { const RtpRtcpInterface::Configuration& configuration); ~ModuleRtpRtcpImpl() override; - // Returns the number of milliseconds until the module want a worker thread to - // call Process. - int64_t TimeUntilNextProcess() override; - // Process any pending tasks such as timeouts. void Process() override; @@ -309,7 +305,6 @@ class ModuleRtpRtcpImpl : public RtpRtcp, public RTCPReceiver::ModuleRtpRtcp { int64_t last_bitrate_process_time_; int64_t last_rtt_process_time_; - int64_t next_process_time_; uint16_t packet_overhead_; // Send side