Narrow interface PacketRouter use to send Remb and TransportFeedback
This allows to use RtcpTransceiver implementation instead of RtpRtcp.
No functional changes.
Bug: webrtc:8239
Change-Id: I3c5bd23ff2136eb844e85b567b70380fc2a65929
Reviewed-on: https://webrtc-review.googlesource.com/33005
Reviewed-by: Niels Moller <nisse@webrtc.org>
Reviewed-by: Philip Eliasson <philipel@webrtc.org>
Commit-Queue: Danil Chapovalov <danilchap@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#21298}diff --git a/modules/pacing/packet_router.cc b/modules/pacing/packet_router.cc
index ba1359a..e3d935e 100644
--- a/modules/pacing/packet_router.cc
+++ b/modules/pacing/packet_router.cc
@@ -37,7 +37,7 @@
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 @@
}
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 @@
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<RtpRtcp*>& candidates =
- sender ? sender_remb_candidates_ : receiver_remb_candidates_;
+ std::vector<RtcpFeedbackSenderInterface*>& 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<RtpRtcp*>& candidates =
- sender ? sender_remb_candidates_ : receiver_remb_candidates_;
+ std::vector<RtcpFeedbackSenderInterface*>& 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 @@
// 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 77d46d7..9597896 100644
--- a/modules/pacing/packet_router.h
+++ b/modules/pacing/packet_router.h
@@ -46,8 +46,9 @@
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 @@
bool SendTransportFeedback(rtcp::TransportFeedback* packet) override;
private:
- void AddRembModuleCandidate(RtpRtcp* candidate_module, bool sender)
+ void AddRembModuleCandidate(RtcpFeedbackSenderInterface* candidate_module,
+ bool media_sender)
RTC_EXCLUSIVE_LOCKS_REQUIRED(modules_crit_);
- void MaybeRemoveRembModuleCandidate(RtpRtcp* candidate_module, bool 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<RtpRtcp*> rtp_send_modules_ RTC_GUARDED_BY(modules_crit_);
- std::vector<RtpRtcp*> rtp_receive_modules_ RTC_GUARDED_BY(modules_crit_);
+ // Rtcp modules of the rtp receivers.
+ std::vector<RtcpFeedbackSenderInterface*> 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 @@
// Candidates for the REMB module can be RTP sender/receiver modules, with
// the sender modules taking precedence.
- std::vector<RtpRtcp*> sender_remb_candidates_ RTC_GUARDED_BY(modules_crit_);
- std::vector<RtpRtcp*> receiver_remb_candidates_ RTC_GUARDED_BY(modules_crit_);
- RtpRtcp* active_remb_module_ RTC_GUARDED_BY(modules_crit_);
+ std::vector<RtcpFeedbackSenderInterface*> sender_remb_candidates_
+ RTC_GUARDED_BY(modules_crit_);
+ std::vector<RtcpFeedbackSenderInterface*> 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 6e14622..7e85cb6 100644
--- a/modules/rtp_rtcp/include/rtp_rtcp.h
+++ b/modules/rtp_rtcp/include/rtp_rtcp.h
@@ -42,7 +42,7 @@
class TransportFeedback;
}
-class RtpRtcp : public Module {
+class RtpRtcp : public Module, public RtcpFeedbackSenderInterface {
public:
struct Configuration {
Configuration();
@@ -165,7 +165,7 @@
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 @@
// (REMB) Receiver Estimated Max Bitrate.
// Schedules sending REMB on next and following sender/receiver reports.
- virtual void SetRemb(int64_t bitrate_bps, std::vector<uint32_t> ssrcs) = 0;
+ void SetRemb(int64_t bitrate_bps, std::vector<uint32_t> 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 @@
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 97a2a98..61b1577 100644
--- a/modules/rtp_rtcp/include/rtp_rtcp_defines.h
+++ b/modules/rtp_rtcp/include/rtp_rtcp_defines.h
@@ -407,6 +407,19 @@
virtual std::vector<PacketFeedback> 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<uint32_t> 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 8ce2db5..9df56d1 100644
--- a/modules/rtp_rtcp/source/rtcp_transceiver.h
+++ b/modules/rtp_rtcp/source/rtcp_transceiver.h
@@ -27,10 +27,10 @@
// 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 @@
// (REMB) Receiver Estimated Max Bitrate.
// Includes REMB in following compound packets.
- void SetRemb(int64_t bitrate_bps, std::vector<uint32_t> ssrcs);
+ void SetRemb(int64_t bitrate_bps, std::vector<uint32_t> 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<uint16_t> sequence_numbers);
