diff --git a/modules/pacing/packet_router.cc b/modules/pacing/packet_router.cc index ba1359a556..e3d935e65c 100644 --- a/modules/pacing/packet_router.cc +++ b/modules/pacing/packet_router.cc @@ -37,7 +37,7 @@ PacketRouter::PacketRouter() PacketRouter::~PacketRouter() { RTC_DCHECK(rtp_send_modules_.empty()); - RTC_DCHECK(rtp_receive_modules_.empty()); + RTC_DCHECK(rtcp_feedback_senders_.empty()); RTC_DCHECK(sender_remb_candidates_.empty()); RTC_DCHECK(receiver_remb_candidates_.empty()); RTC_DCHECK(active_remb_module_ == nullptr); @@ -56,39 +56,41 @@ void PacketRouter::AddSendRtpModule(RtpRtcp* rtp_module, bool remb_candidate) { } if (remb_candidate) { - AddRembModuleCandidate(rtp_module, true); + AddRembModuleCandidate(rtp_module, /* media_sender = */ true); } } void PacketRouter::RemoveSendRtpModule(RtpRtcp* rtp_module) { rtc::CritScope cs(&modules_crit_); - MaybeRemoveRembModuleCandidate(rtp_module, /* sender = */ true); + MaybeRemoveRembModuleCandidate(rtp_module, /* media_sender = */ true); auto it = std::find(rtp_send_modules_.begin(), rtp_send_modules_.end(), rtp_module); RTC_DCHECK(it != rtp_send_modules_.end()); rtp_send_modules_.erase(it); } -void PacketRouter::AddReceiveRtpModule(RtpRtcp* rtp_module, +void PacketRouter::AddReceiveRtpModule(RtcpFeedbackSenderInterface* rtcp_sender, bool remb_candidate) { rtc::CritScope cs(&modules_crit_); - RTC_DCHECK(std::find(rtp_receive_modules_.begin(), rtp_receive_modules_.end(), - rtp_module) == rtp_receive_modules_.end()); + RTC_DCHECK(std::find(rtcp_feedback_senders_.begin(), + rtcp_feedback_senders_.end(), + rtcp_sender) == rtcp_feedback_senders_.end()); - rtp_receive_modules_.push_back(rtp_module); + rtcp_feedback_senders_.push_back(rtcp_sender); if (remb_candidate) { - AddRembModuleCandidate(rtp_module, false); + AddRembModuleCandidate(rtcp_sender, /* media_sender = */ false); } } -void PacketRouter::RemoveReceiveRtpModule(RtpRtcp* rtp_module) { +void PacketRouter::RemoveReceiveRtpModule( + RtcpFeedbackSenderInterface* rtcp_sender) { rtc::CritScope cs(&modules_crit_); - MaybeRemoveRembModuleCandidate(rtp_module, /* sender = */ false); - const auto& it = std::find(rtp_receive_modules_.begin(), - rtp_receive_modules_.end(), rtp_module); - RTC_DCHECK(it != rtp_receive_modules_.end()); - rtp_receive_modules_.erase(it); + MaybeRemoveRembModuleCandidate(rtcp_sender, /* media_sender = */ false); + auto it = std::find(rtcp_feedback_senders_.begin(), + rtcp_feedback_senders_.end(), rtcp_sender); + RTC_DCHECK(it != rtcp_feedback_senders_.end()); + rtcp_feedback_senders_.erase(it); } bool PacketRouter::TimeToSendPacket(uint32_t ssrc, @@ -229,30 +231,32 @@ bool PacketRouter::SendTransportFeedback(rtcp::TransportFeedback* packet) { if (rtp_module->SendFeedbackPacket(*packet)) return true; } - for (auto* rtp_module : rtp_receive_modules_) { - packet->SetSenderSsrc(rtp_module->SSRC()); - if (rtp_module->SendFeedbackPacket(*packet)) + for (auto* rtcp_sender : rtcp_feedback_senders_) { + packet->SetSenderSsrc(rtcp_sender->SSRC()); + if (rtcp_sender->SendFeedbackPacket(*packet)) return true; } return false; } -void PacketRouter::AddRembModuleCandidate(RtpRtcp* candidate_module, - bool sender) { +void PacketRouter::AddRembModuleCandidate( + RtcpFeedbackSenderInterface* candidate_module, + bool media_sender) { RTC_DCHECK(candidate_module); - std::vector& candidates = - sender ? sender_remb_candidates_ : receiver_remb_candidates_; + std::vector& candidates = + media_sender ? sender_remb_candidates_ : receiver_remb_candidates_; RTC_DCHECK(std::find(candidates.cbegin(), candidates.cend(), candidate_module) == candidates.cend()); candidates.push_back(candidate_module); DetermineActiveRembModule(); } -void PacketRouter::MaybeRemoveRembModuleCandidate(RtpRtcp* candidate_module, - bool sender) { +void PacketRouter::MaybeRemoveRembModuleCandidate( + RtcpFeedbackSenderInterface* candidate_module, + bool media_sender) { RTC_DCHECK(candidate_module); - std::vector& candidates = - sender ? sender_remb_candidates_ : receiver_remb_candidates_; + std::vector& candidates = + media_sender ? sender_remb_candidates_ : receiver_remb_candidates_; auto it = std::find(candidates.begin(), candidates.end(), candidate_module); if (it == candidates.end()) { @@ -278,7 +282,7 @@ void PacketRouter::DetermineActiveRembModule() { // When adding the first sender module, we should change the active REMB // module to be that. Otherwise, we remain with the current active module. - RtpRtcp* new_active_remb_module; + RtcpFeedbackSenderInterface* new_active_remb_module; if (!sender_remb_candidates_.empty()) { new_active_remb_module = sender_remb_candidates_.front(); diff --git a/modules/pacing/packet_router.h b/modules/pacing/packet_router.h index 77d46d7717..9597896353 100644 --- a/modules/pacing/packet_router.h +++ b/modules/pacing/packet_router.h @@ -46,8 +46,9 @@ class PacketRouter : public PacedSender::PacketSender, void AddSendRtpModule(RtpRtcp* rtp_module, bool remb_candidate); void RemoveSendRtpModule(RtpRtcp* rtp_module); - void AddReceiveRtpModule(RtpRtcp* rtp_module, bool remb_candidate); - void RemoveReceiveRtpModule(RtpRtcp* rtp_module); + void AddReceiveRtpModule(RtcpFeedbackSenderInterface* rtcp_sender, + bool remb_candidate); + void RemoveReceiveRtpModule(RtcpFeedbackSenderInterface* rtcp_sender); // Implements PacedSender::Callback. bool TimeToSendPacket(uint32_t ssrc, @@ -81,17 +82,22 @@ class PacketRouter : public PacedSender::PacketSender, bool SendTransportFeedback(rtcp::TransportFeedback* packet) override; private: - void AddRembModuleCandidate(RtpRtcp* candidate_module, bool sender) - RTC_EXCLUSIVE_LOCKS_REQUIRED(modules_crit_); - void MaybeRemoveRembModuleCandidate(RtpRtcp* candidate_module, bool sender) + void AddRembModuleCandidate(RtcpFeedbackSenderInterface* candidate_module, + bool media_sender) RTC_EXCLUSIVE_LOCKS_REQUIRED(modules_crit_); + void MaybeRemoveRembModuleCandidate( + RtcpFeedbackSenderInterface* candidate_module, + bool media_sender) RTC_EXCLUSIVE_LOCKS_REQUIRED(modules_crit_); void UnsetActiveRembModule() RTC_EXCLUSIVE_LOCKS_REQUIRED(modules_crit_); void DetermineActiveRembModule() RTC_EXCLUSIVE_LOCKS_REQUIRED(modules_crit_); rtc::RaceChecker pacer_race_; rtc::CriticalSection modules_crit_; + // Rtp and Rtcp modules of the rtp senders. std::list rtp_send_modules_ RTC_GUARDED_BY(modules_crit_); - std::vector rtp_receive_modules_ RTC_GUARDED_BY(modules_crit_); + // Rtcp modules of the rtp receivers. + std::vector rtcp_feedback_senders_ + RTC_GUARDED_BY(modules_crit_); // TODO(eladalon): remb_crit_ only ever held from one function, and it's not // clear if that function can actually be called from more than one thread. @@ -105,9 +111,12 @@ class PacketRouter : public PacedSender::PacketSender, // Candidates for the REMB module can be RTP sender/receiver modules, with // the sender modules taking precedence. - std::vector sender_remb_candidates_ RTC_GUARDED_BY(modules_crit_); - std::vector receiver_remb_candidates_ RTC_GUARDED_BY(modules_crit_); - RtpRtcp* active_remb_module_ RTC_GUARDED_BY(modules_crit_); + std::vector sender_remb_candidates_ + RTC_GUARDED_BY(modules_crit_); + std::vector receiver_remb_candidates_ + RTC_GUARDED_BY(modules_crit_); + RtcpFeedbackSenderInterface* active_remb_module_ + RTC_GUARDED_BY(modules_crit_); volatile int transport_seq_; diff --git a/modules/rtp_rtcp/include/rtp_rtcp.h b/modules/rtp_rtcp/include/rtp_rtcp.h index 6e146229e4..7e85cb6983 100644 --- a/modules/rtp_rtcp/include/rtp_rtcp.h +++ b/modules/rtp_rtcp/include/rtp_rtcp.h @@ -42,7 +42,7 @@ namespace rtcp { class TransportFeedback; } -class RtpRtcp : public Module { +class RtpRtcp : public Module, public RtcpFeedbackSenderInterface { public: struct Configuration { Configuration(); @@ -165,7 +165,7 @@ class RtpRtcp : public Module { virtual RtpState GetRtxState() const = 0; // Returns SSRC. - virtual uint32_t SSRC() const = 0; + uint32_t SSRC() const override = 0; // Sets SSRC, default is a random number. virtual void SetSSRC(uint32_t ssrc) = 0; @@ -341,9 +341,9 @@ class RtpRtcp : public Module { // (REMB) Receiver Estimated Max Bitrate. // Schedules sending REMB on next and following sender/receiver reports. - virtual void SetRemb(int64_t bitrate_bps, std::vector ssrcs) = 0; + void SetRemb(int64_t bitrate_bps, std::vector ssrcs) override = 0; // Stops sending REMB on next and following sender/receiver reports. - virtual void UnsetRemb() = 0; + void UnsetRemb() override = 0; // (TMMBR) Temporary Max Media Bit Rate virtual bool TMMBR() const = 0; @@ -391,7 +391,7 @@ class RtpRtcp : public Module { RtcpStatisticsCallback* callback) = 0; virtual RtcpStatisticsCallback* GetRtcpStatisticsCallback() = 0; // BWE feedback packets. - virtual bool SendFeedbackPacket(const rtcp::TransportFeedback& packet) = 0; + bool SendFeedbackPacket(const rtcp::TransportFeedback& packet) override = 0; virtual void SetVideoBitrateAllocation(const BitrateAllocation& bitrate) = 0; diff --git a/modules/rtp_rtcp/include/rtp_rtcp_defines.h b/modules/rtp_rtcp/include/rtp_rtcp_defines.h index 97a2a98ff7..61b1577d7d 100644 --- a/modules/rtp_rtcp/include/rtp_rtcp_defines.h +++ b/modules/rtp_rtcp/include/rtp_rtcp_defines.h @@ -407,6 +407,19 @@ class TransportFeedbackObserver { virtual std::vector GetTransportFeedbackVector() const = 0; }; +// Interface for PacketRouter to send rtcp feedback on behalf of +// congestion controller. +// TODO(bugs.webrtc.org/8239): Remove and use RtcpTransceiver directly +// when RtcpTransceiver always present in rtp transport. +class RtcpFeedbackSenderInterface { + public: + virtual ~RtcpFeedbackSenderInterface() = default; + virtual uint32_t SSRC() const = 0; + virtual bool SendFeedbackPacket(const rtcp::TransportFeedback& feedback) = 0; + virtual void SetRemb(int64_t bitrate_bps, std::vector ssrcs) = 0; + virtual void UnsetRemb() = 0; +}; + class PacketFeedbackObserver { public: virtual ~PacketFeedbackObserver() = default; diff --git a/modules/rtp_rtcp/source/rtcp_transceiver.h b/modules/rtp_rtcp/source/rtcp_transceiver.h index 8ce2db5243..9df56d14e6 100644 --- a/modules/rtp_rtcp/source/rtcp_transceiver.h +++ b/modules/rtp_rtcp/source/rtcp_transceiver.h @@ -27,10 +27,10 @@ namespace webrtc { // Manage incoming and outgoing rtcp messages for multiple BUNDLED streams. // // This class is thread-safe wrapper of RtcpTransceiverImpl -class RtcpTransceiver { +class RtcpTransceiver : public RtcpFeedbackSenderInterface { public: explicit RtcpTransceiver(const RtcpTransceiverConfig& config); - ~RtcpTransceiver(); + ~RtcpTransceiver() override; // Registers observer to be notified about incoming rtcp packets. // Calls to observer will be done on the |config.task_queue|. @@ -51,17 +51,17 @@ class RtcpTransceiver { // (REMB) Receiver Estimated Max Bitrate. // Includes REMB in following compound packets. - void SetRemb(int64_t bitrate_bps, std::vector ssrcs); + void SetRemb(int64_t bitrate_bps, std::vector ssrcs) override; // Stops sending REMB in following compound packets. - void UnsetRemb(); + void UnsetRemb() override; // TODO(bugs.webrtc.org/8239): Remove SendFeedbackPacket and SSRC functions // and move generating of the TransportFeedback message inside // RtcpTransceiverImpl when there is one RtcpTransceiver per rtp transport. // Returns ssrc to put as sender ssrc into rtcp::TransportFeedback. - uint32_t SSRC() const; - bool SendFeedbackPacket(const rtcp::TransportFeedback& packet); + uint32_t SSRC() const override; + bool SendFeedbackPacket(const rtcp::TransportFeedback& packet) override; // Reports missing packets, https://tools.ietf.org/html/rfc4585#section-6.2.1 void SendNack(uint32_t ssrc, std::vector sequence_numbers);