From ded6636cf43904448ee926d1f2b4352c8a957ca6 Mon Sep 17 00:00:00 2001 From: Danil Chapovalov Date: Mon, 25 Jan 2021 12:40:01 +0100 Subject: [PATCH] Cleanup RtcpSender from legacy functionality MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reduce amount of dynamic memory used to generate rtcp message Remove option to request several types of rtcp message as unused Deduplicated PacketContainer and PacketSender as constructs to create packets Bug: None Change-Id: Ib2e20a72a9bd73a441ae6b72a13e18ab5885f5c8 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/203261 Reviewed-by: Erik Språng Commit-Queue: Danil Chapovalov Cr-Commit-Position: refs/heads/master@{#33068} --- modules/rtp_rtcp/source/rtcp_sender.cc | 346 ++++++++---------- modules/rtp_rtcp/source/rtcp_sender.h | 62 +--- .../rtp_rtcp/source/rtcp_sender_unittest.cc | 15 - 3 files changed, 166 insertions(+), 257 deletions(-) diff --git a/modules/rtp_rtcp/source/rtcp_sender.cc b/modules/rtp_rtcp/source/rtcp_sender.cc index 79f5aa6c67..c4679e6534 100644 --- a/modules/rtp_rtcp/source/rtcp_sender.cc +++ b/modules/rtp_rtcp/source/rtcp_sender.cc @@ -50,36 +50,10 @@ const uint32_t kRtcpAnyExtendedReports = kRtcpXrReceiverReferenceTime | constexpr int32_t kDefaultVideoReportInterval = 1000; constexpr int32_t kDefaultAudioReportInterval = 5000; -class PacketContainer : public rtcp::CompoundPacket { - public: - PacketContainer(Transport* transport, RtcEventLog* event_log) - : transport_(transport), event_log_(event_log) {} - - PacketContainer() = delete; - PacketContainer(const PacketContainer&) = delete; - PacketContainer& operator=(const PacketContainer&) = delete; - - size_t SendPackets(size_t max_payload_length) { - size_t bytes_sent = 0; - Build(max_payload_length, [&](rtc::ArrayView packet) { - if (transport_->SendRtcp(packet.data(), packet.size())) { - bytes_sent += packet.size(); - if (event_log_) { - event_log_->Log(std::make_unique(packet)); - } - } - }); - return bytes_sent; - } - - private: - Transport* transport_; - RtcEventLog* const event_log_; -}; +} // namespace // Helper to put several RTCP packets into lower layer datagram RTCP packet. -// Prefer to use this class instead of PacketContainer. -class PacketSender { +class RTCPSender::PacketSender { public: PacketSender(rtcp::RtcpPacket::PacketReadyCallback callback, size_t max_packet_size) @@ -102,8 +76,6 @@ class PacketSender { } } - bool IsEmpty() const { return index_ == 0; } - private: const rtcp::RtcpPacket::PacketReadyCallback callback_; const size_t max_packet_size_; @@ -111,8 +83,6 @@ class PacketSender { uint8_t buffer_[IP_PACKET_SIZE]; }; -} // namespace - RTCPSender::FeedbackState::FeedbackState() : packets_sent(0), media_bytes_sent(0), @@ -241,21 +211,43 @@ int32_t RTCPSender::SendLossNotification(const FeedbackState& feedback_state, uint16_t last_received_seq_num, bool decodability_flag, bool buffering_allowed) { - MutexLock lock(&mutex_rtcp_sender_); + int32_t error_code = -1; + auto callback = [&](rtc::ArrayView packet) { + if (transport_->SendRtcp(packet.data(), packet.size())) { + error_code = 0; + if (event_log_) { + event_log_->Log(std::make_unique(packet)); + } + } + }; + absl::optional sender; + { + MutexLock lock(&mutex_rtcp_sender_); - loss_notification_state_.last_decoded_seq_num = last_decoded_seq_num; - loss_notification_state_.last_received_seq_num = last_received_seq_num; - loss_notification_state_.decodability_flag = decodability_flag; + if (!loss_notification_.Set(last_decoded_seq_num, last_received_seq_num, + decodability_flag)) { + return -1; + } - SetFlag(kRtcpLossNotification, /*is_volatile=*/true); + SetFlag(kRtcpLossNotification, /*is_volatile=*/true); - if (buffering_allowed) { - // The loss notification will be batched with additional feedback messages. - return 0; + if (buffering_allowed) { + // The loss notification will be batched with additional feedback + // messages. + return 0; + } + + sender.emplace(callback, max_packet_size_); + auto result = ComputeCompoundRTCPPacket( + feedback_state, RTCPPacketType::kRtcpLossNotification, 0, nullptr, + *sender); + if (result) { + return *result; + } } + sender->Send(); - return SendCompoundRTCPLocked( - feedback_state, {RTCPPacketType::kRtcpLossNotification}, 0, nullptr); + return error_code; } void RTCPSender::SetRemb(int64_t bitrate_bps, std::vector ssrcs) { @@ -438,7 +430,7 @@ bool RTCPSender::TimeToSendRTCPReport(bool sendKeyframeBeforeRTP) const { return now >= next_time_to_send_rtcp_; } -std::unique_ptr RTCPSender::BuildSR(const RtcpContext& ctx) { +void RTCPSender::BuildSR(const RtcpContext& ctx, PacketSender& sender) { // Timestamp shouldn't be estimated before first media frame. RTC_DCHECK_GE(last_frame_capture_time_ms_, 0); // The timestamp of this RTCP packet should be estimated as the timestamp of @@ -457,69 +449,62 @@ std::unique_ptr RTCPSender::BuildSR(const RtcpContext& ctx) { timestamp_offset_ + last_rtp_timestamp_ + ((ctx.now_us_ + 500) / 1000 - last_frame_capture_time_ms_) * rtp_rate; - rtcp::SenderReport* report = new rtcp::SenderReport(); - report->SetSenderSsrc(ssrc_); - report->SetNtp(TimeMicrosToNtp(ctx.now_us_)); - report->SetRtpTimestamp(rtp_timestamp); - report->SetPacketCount(ctx.feedback_state_.packets_sent); - report->SetOctetCount(ctx.feedback_state_.media_bytes_sent); - report->SetReportBlocks(CreateReportBlocks(ctx.feedback_state_)); - - return std::unique_ptr(report); + rtcp::SenderReport report; + report.SetSenderSsrc(ssrc_); + report.SetNtp(TimeMicrosToNtp(ctx.now_us_)); + report.SetRtpTimestamp(rtp_timestamp); + report.SetPacketCount(ctx.feedback_state_.packets_sent); + report.SetOctetCount(ctx.feedback_state_.media_bytes_sent); + report.SetReportBlocks(CreateReportBlocks(ctx.feedback_state_)); + sender.AppendPacket(report); } -std::unique_ptr RTCPSender::BuildSDES( - const RtcpContext& ctx) { +void RTCPSender::BuildSDES(const RtcpContext& ctx, PacketSender& sender) { size_t length_cname = cname_.length(); RTC_CHECK_LT(length_cname, RTCP_CNAME_SIZE); - rtcp::Sdes* sdes = new rtcp::Sdes(); - sdes->AddCName(ssrc_, cname_); + rtcp::Sdes sdes; + sdes.AddCName(ssrc_, cname_); for (const auto& it : csrc_cnames_) - RTC_CHECK(sdes->AddCName(it.first, it.second)); + RTC_CHECK(sdes.AddCName(it.first, it.second)); - return std::unique_ptr(sdes); + sender.AppendPacket(sdes); } -std::unique_ptr RTCPSender::BuildRR(const RtcpContext& ctx) { - rtcp::ReceiverReport* report = new rtcp::ReceiverReport(); - report->SetSenderSsrc(ssrc_); - report->SetReportBlocks(CreateReportBlocks(ctx.feedback_state_)); - - return std::unique_ptr(report); +void RTCPSender::BuildRR(const RtcpContext& ctx, PacketSender& sender) { + rtcp::ReceiverReport report; + report.SetSenderSsrc(ssrc_); + report.SetReportBlocks(CreateReportBlocks(ctx.feedback_state_)); + sender.AppendPacket(report); } -std::unique_ptr RTCPSender::BuildPLI(const RtcpContext& ctx) { - rtcp::Pli* pli = new rtcp::Pli(); - pli->SetSenderSsrc(ssrc_); - pli->SetMediaSsrc(remote_ssrc_); +void RTCPSender::BuildPLI(const RtcpContext& ctx, PacketSender& sender) { + rtcp::Pli pli; + pli.SetSenderSsrc(ssrc_); + pli.SetMediaSsrc(remote_ssrc_); ++packet_type_counter_.pli_packets; - - return std::unique_ptr(pli); + sender.AppendPacket(pli); } -std::unique_ptr RTCPSender::BuildFIR(const RtcpContext& ctx) { +void RTCPSender::BuildFIR(const RtcpContext& ctx, PacketSender& sender) { ++sequence_number_fir_; - rtcp::Fir* fir = new rtcp::Fir(); - fir->SetSenderSsrc(ssrc_); - fir->AddRequestTo(remote_ssrc_, sequence_number_fir_); + rtcp::Fir fir; + fir.SetSenderSsrc(ssrc_); + fir.AddRequestTo(remote_ssrc_, sequence_number_fir_); ++packet_type_counter_.fir_packets; - - return std::unique_ptr(fir); + sender.AppendPacket(fir); } -std::unique_ptr RTCPSender::BuildREMB( - const RtcpContext& ctx) { - rtcp::Remb* remb = new rtcp::Remb(); - remb->SetSenderSsrc(ssrc_); - remb->SetBitrateBps(remb_bitrate_); - remb->SetSsrcs(remb_ssrcs_); - - return std::unique_ptr(remb); +void RTCPSender::BuildREMB(const RtcpContext& ctx, PacketSender& sender) { + rtcp::Remb remb; + remb.SetSenderSsrc(ssrc_); + remb.SetBitrateBps(remb_bitrate_); + remb.SetSsrcs(remb_ssrcs_); + sender.AppendPacket(remb); } void RTCPSender::SetTargetBitrate(unsigned int target_bitrate) { @@ -527,10 +512,9 @@ void RTCPSender::SetTargetBitrate(unsigned int target_bitrate) { tmmbr_send_bps_ = target_bitrate; } -std::unique_ptr RTCPSender::BuildTMMBR( - const RtcpContext& ctx) { +void RTCPSender::BuildTMMBR(const RtcpContext& ctx, PacketSender& sender) { if (ctx.feedback_state_.receiver == nullptr) - return nullptr; + return; // Before sending the TMMBR check the received TMMBN, only an owner is // allowed to raise the bitrate: // * If the sender is an owner of the TMMBN -> send TMMBR @@ -550,7 +534,7 @@ std::unique_ptr RTCPSender::BuildTMMBR( if (candidate.bitrate_bps() == tmmbr_send_bps_ && candidate.packet_overhead() == packet_oh_send_) { // Do not send the same tuple. - return nullptr; + return; } } if (!tmmbr_owner) { @@ -564,62 +548,53 @@ std::unique_ptr RTCPSender::BuildTMMBR( tmmbr_owner = TMMBRHelp::IsOwner(bounding, ssrc_); if (!tmmbr_owner) { // Did not enter bounding set, no meaning to send this request. - return nullptr; + return; } } } if (!tmmbr_send_bps_) - return nullptr; + return; - rtcp::Tmmbr* tmmbr = new rtcp::Tmmbr(); - tmmbr->SetSenderSsrc(ssrc_); + rtcp::Tmmbr tmmbr; + tmmbr.SetSenderSsrc(ssrc_); rtcp::TmmbItem request; request.set_ssrc(remote_ssrc_); request.set_bitrate_bps(tmmbr_send_bps_); request.set_packet_overhead(packet_oh_send_); - tmmbr->AddTmmbr(request); - - return std::unique_ptr(tmmbr); + tmmbr.AddTmmbr(request); + sender.AppendPacket(tmmbr); } -std::unique_ptr RTCPSender::BuildTMMBN( - const RtcpContext& ctx) { - rtcp::Tmmbn* tmmbn = new rtcp::Tmmbn(); - tmmbn->SetSenderSsrc(ssrc_); +void RTCPSender::BuildTMMBN(const RtcpContext& ctx, PacketSender& sender) { + rtcp::Tmmbn tmmbn; + tmmbn.SetSenderSsrc(ssrc_); for (const rtcp::TmmbItem& tmmbr : tmmbn_to_send_) { if (tmmbr.bitrate_bps() > 0) { - tmmbn->AddTmmbr(tmmbr); + tmmbn.AddTmmbr(tmmbr); } } - - return std::unique_ptr(tmmbn); + sender.AppendPacket(tmmbn); } -std::unique_ptr RTCPSender::BuildAPP(const RtcpContext& ctx) { - rtcp::App* app = new rtcp::App(); - app->SetSenderSsrc(ssrc_); - - return std::unique_ptr(app); +void RTCPSender::BuildAPP(const RtcpContext& ctx, PacketSender& sender) { + rtcp::App app; + app.SetSenderSsrc(ssrc_); + sender.AppendPacket(app); } -std::unique_ptr RTCPSender::BuildLossNotification( - const RtcpContext& ctx) { - auto loss_notification = std::make_unique( - loss_notification_state_.last_decoded_seq_num, - loss_notification_state_.last_received_seq_num, - loss_notification_state_.decodability_flag); - loss_notification->SetSenderSsrc(ssrc_); - loss_notification->SetMediaSsrc(remote_ssrc_); - return std::move(loss_notification); +void RTCPSender::BuildLossNotification(const RtcpContext& ctx, + PacketSender& sender) { + loss_notification_.SetSenderSsrc(ssrc_); + loss_notification_.SetMediaSsrc(remote_ssrc_); + sender.AppendPacket(loss_notification_); } -std::unique_ptr RTCPSender::BuildNACK( - const RtcpContext& ctx) { - rtcp::Nack* nack = new rtcp::Nack(); - nack->SetSenderSsrc(ssrc_); - nack->SetMediaSsrc(remote_ssrc_); - nack->SetPacketIds(ctx.nack_list_, ctx.nack_size_); +void RTCPSender::BuildNACK(const RtcpContext& ctx, PacketSender& sender) { + rtcp::Nack nack; + nack.SetSenderSsrc(ssrc_); + nack.SetMediaSsrc(remote_ssrc_); + nack.SetPacketIds(ctx.nack_list_, ctx.nack_size_); // Report stats. for (int idx = 0; idx < ctx.nack_size_; ++idx) { @@ -629,31 +604,29 @@ std::unique_ptr RTCPSender::BuildNACK( packet_type_counter_.unique_nack_requests = nack_stats_.unique_requests(); ++packet_type_counter_.nack_packets; - - return std::unique_ptr(nack); + sender.AppendPacket(nack); } -std::unique_ptr RTCPSender::BuildBYE(const RtcpContext& ctx) { - rtcp::Bye* bye = new rtcp::Bye(); - bye->SetSenderSsrc(ssrc_); - bye->SetCsrcs(csrcs_); - - return std::unique_ptr(bye); +void RTCPSender::BuildBYE(const RtcpContext& ctx, PacketSender& sender) { + rtcp::Bye bye; + bye.SetSenderSsrc(ssrc_); + bye.SetCsrcs(csrcs_); + sender.AppendPacket(bye); } -std::unique_ptr RTCPSender::BuildExtendedReports( - const RtcpContext& ctx) { - std::unique_ptr xr(new rtcp::ExtendedReports()); - xr->SetSenderSsrc(ssrc_); +void RTCPSender::BuildExtendedReports(const RtcpContext& ctx, + PacketSender& sender) { + rtcp::ExtendedReports xr; + xr.SetSenderSsrc(ssrc_); if (!sending_ && xr_send_receiver_reference_time_enabled_) { rtcp::Rrtr rrtr; rrtr.SetNtp(TimeMicrosToNtp(ctx.now_us_)); - xr->SetRrtr(rrtr); + xr.SetRrtr(rrtr); } for (const rtcp::ReceiveTimeInfo& rti : ctx.feedback_state_.last_xr_rtis) { - xr->AddDlrrItem(rti); + xr.AddDlrrItem(rti); } if (send_video_bitrate_allocation_) { @@ -668,72 +641,53 @@ std::unique_ptr RTCPSender::BuildExtendedReports( } } - xr->SetTargetBitrate(target_bitrate); + xr.SetTargetBitrate(target_bitrate); send_video_bitrate_allocation_ = false; } - - return std::move(xr); + sender.AppendPacket(xr); } int32_t RTCPSender::SendRTCP(const FeedbackState& feedback_state, - RTCPPacketType packetType, + RTCPPacketType packet_type, int32_t nack_size, const uint16_t* nack_list) { - return SendCompoundRTCP( - feedback_state, std::set(&packetType, &packetType + 1), - nack_size, nack_list); -} - -int32_t RTCPSender::SendCompoundRTCP( - const FeedbackState& feedback_state, - const std::set& packet_types, - int32_t nack_size, - const uint16_t* nack_list) { - PacketContainer container(transport_, event_log_); - size_t max_packet_size; - + int32_t error_code = -1; + auto callback = [&](rtc::ArrayView packet) { + if (transport_->SendRtcp(packet.data(), packet.size())) { + error_code = 0; + if (event_log_) { + event_log_->Log(std::make_unique(packet)); + } + } + }; + absl::optional sender; { MutexLock lock(&mutex_rtcp_sender_); - auto result = ComputeCompoundRTCPPacket(feedback_state, packet_types, - nack_size, nack_list, &container); + sender.emplace(callback, max_packet_size_); + auto result = ComputeCompoundRTCPPacket(feedback_state, packet_type, + nack_size, nack_list, *sender); if (result) { return *result; } - max_packet_size = max_packet_size_; } + sender->Send(); - size_t bytes_sent = container.SendPackets(max_packet_size); - return bytes_sent == 0 ? -1 : 0; -} - -int32_t RTCPSender::SendCompoundRTCPLocked( - const FeedbackState& feedback_state, - const std::set& packet_types, - int32_t nack_size, - const uint16_t* nack_list) { - PacketContainer container(transport_, event_log_); - auto result = ComputeCompoundRTCPPacket(feedback_state, packet_types, - nack_size, nack_list, &container); - if (result) { - return *result; - } - size_t bytes_sent = container.SendPackets(max_packet_size_); - return bytes_sent == 0 ? -1 : 0; + return error_code; } absl::optional RTCPSender::ComputeCompoundRTCPPacket( const FeedbackState& feedback_state, - const std::set& packet_types, + RTCPPacketType packet_type, int32_t nack_size, const uint16_t* nack_list, - rtcp::CompoundPacket* out_packet) { + PacketSender& sender) { if (method_ == RtcpMode::kOff) { RTC_LOG(LS_WARNING) << "Can't send rtcp if it is disabled."; return -1; } - // Add all flags as volatile. Non volatile entries will not be overwritten. - // All new volatile flags added will be consumed by the end of this call. - SetFlags(packet_types, true); + // Add the flag as volatile. Non volatile entries will not be overwritten. + // The new volatile flag will be consumed by the end of this call. + SetFlag(packet_type, true); // Prevent sending streams to send SR before any media has been sent. const bool can_calculate_rtp_timestamp = (last_frame_capture_time_ms_ >= 0); @@ -760,37 +714,37 @@ absl::optional RTCPSender::ComputeCompoundRTCPPacket( PrepareReport(feedback_state); - std::unique_ptr packet_bye; + bool create_bye = false; auto it = report_flags_.begin(); while (it != report_flags_.end()) { - auto builder_it = builders_.find(it->type); + uint32_t rtcp_packet_type = it->type; + if (it->is_volatile) { report_flags_.erase(it++); } else { ++it; } + // If there is a BYE, don't append now - save it and append it + // at the end later. + if (rtcp_packet_type == kRtcpBye) { + create_bye = true; + continue; + } + auto builder_it = builders_.find(rtcp_packet_type); if (builder_it == builders_.end()) { - RTC_NOTREACHED() << "Could not find builder for packet type " << it->type; + RTC_NOTREACHED() << "Could not find builder for packet type " + << rtcp_packet_type; } else { BuilderFunc func = builder_it->second; - std::unique_ptr packet = (this->*func)(context); - if (packet == nullptr) - return -1; - // If there is a BYE, don't append now - save it and append it - // at the end later. - if (builder_it->first == kRtcpBye) { - packet_bye = std::move(packet); - } else { - out_packet->Append(std::move(packet)); - } + (this->*func)(context, sender); } } // Append the BYE now at the end - if (packet_bye) { - out_packet->Append(std::move(packet_bye)); + if (create_bye) { + BuildBYE(context, sender); } if (packet_type_counter_observer_ != nullptr) { @@ -904,12 +858,6 @@ void RTCPSender::SetFlag(uint32_t type, bool is_volatile) { } } -void RTCPSender::SetFlags(const std::set& types, - bool is_volatile) { - for (RTCPPacketType type : types) - SetFlag(type, is_volatile); -} - bool RTCPSender::IsFlagPresent(uint32_t type) const { return report_flags_.find(ReportFlag(type, false)) != report_flags_.end(); } diff --git a/modules/rtp_rtcp/source/rtcp_sender.h b/modules/rtp_rtcp/source/rtcp_sender.h index cc9091dfc7..10211435e9 100644 --- a/modules/rtp_rtcp/source/rtcp_sender.h +++ b/modules/rtp_rtcp/source/rtcp_sender.h @@ -27,6 +27,7 @@ #include "modules/rtp_rtcp/source/rtcp_packet.h" #include "modules/rtp_rtcp/source/rtcp_packet/compound_packet.h" #include "modules/rtp_rtcp/source/rtcp_packet/dlrr.h" +#include "modules/rtp_rtcp/source/rtcp_packet/loss_notification.h" #include "modules/rtp_rtcp/source/rtcp_packet/report_block.h" #include "modules/rtp_rtcp/source/rtcp_packet/tmmb_item.h" #include "modules/rtp_rtcp/source/rtp_rtcp_interface.h" @@ -114,12 +115,6 @@ class RTCPSender final { const uint16_t* nackList = 0) RTC_LOCKS_EXCLUDED(mutex_rtcp_sender_); - int32_t SendCompoundRTCP(const FeedbackState& feedback_state, - const std::set& packetTypes, - int32_t nackSize = 0, - const uint16_t* nackList = nullptr) - RTC_LOCKS_EXCLUDED(mutex_rtcp_sender_); - int32_t SendLossNotification(const FeedbackState& feedback_state, uint16_t last_decoded_seq_num, uint16_t last_received_seq_num, @@ -155,20 +150,14 @@ class RTCPSender final { private: class RtcpContext; - - int32_t SendCompoundRTCPLocked(const FeedbackState& feedback_state, - const std::set& packet_types, - int32_t nack_size, - const uint16_t* nack_list) - RTC_EXCLUSIVE_LOCKS_REQUIRED(mutex_rtcp_sender_); + class PacketSender; absl::optional ComputeCompoundRTCPPacket( const FeedbackState& feedback_state, - const std::set& packet_types, + RTCPPacketType packet_type, int32_t nack_size, const uint16_t* nack_list, - rtcp::CompoundPacket* out_packet) - RTC_EXCLUSIVE_LOCKS_REQUIRED(mutex_rtcp_sender_); + PacketSender& sender) RTC_EXCLUSIVE_LOCKS_REQUIRED(mutex_rtcp_sender_); // Determine which RTCP messages should be sent and setup flags. void PrepareReport(const FeedbackState& feedback_state) @@ -178,36 +167,33 @@ class RTCPSender final { const FeedbackState& feedback_state) RTC_EXCLUSIVE_LOCKS_REQUIRED(mutex_rtcp_sender_); - std::unique_ptr BuildSR(const RtcpContext& context) + void BuildSR(const RtcpContext& context, PacketSender& sender) RTC_EXCLUSIVE_LOCKS_REQUIRED(mutex_rtcp_sender_); - std::unique_ptr BuildRR(const RtcpContext& context) + void BuildRR(const RtcpContext& context, PacketSender& sender) RTC_EXCLUSIVE_LOCKS_REQUIRED(mutex_rtcp_sender_); - std::unique_ptr BuildSDES(const RtcpContext& context) + void BuildSDES(const RtcpContext& context, PacketSender& sender) RTC_EXCLUSIVE_LOCKS_REQUIRED(mutex_rtcp_sender_); - std::unique_ptr BuildPLI(const RtcpContext& context) + void BuildPLI(const RtcpContext& context, PacketSender& sender) RTC_EXCLUSIVE_LOCKS_REQUIRED(mutex_rtcp_sender_); - std::unique_ptr BuildREMB(const RtcpContext& context) + void BuildREMB(const RtcpContext& context, PacketSender& sender) RTC_EXCLUSIVE_LOCKS_REQUIRED(mutex_rtcp_sender_); - std::unique_ptr BuildTMMBR(const RtcpContext& context) + void BuildTMMBR(const RtcpContext& context, PacketSender& sender) RTC_EXCLUSIVE_LOCKS_REQUIRED(mutex_rtcp_sender_); - std::unique_ptr BuildTMMBN(const RtcpContext& context) + void BuildTMMBN(const RtcpContext& context, PacketSender& sender) RTC_EXCLUSIVE_LOCKS_REQUIRED(mutex_rtcp_sender_); - std::unique_ptr BuildAPP(const RtcpContext& context) + void BuildAPP(const RtcpContext& context, PacketSender& sender) RTC_EXCLUSIVE_LOCKS_REQUIRED(mutex_rtcp_sender_); - std::unique_ptr BuildLossNotification( - const RtcpContext& context) + void BuildLossNotification(const RtcpContext& context, PacketSender& sender) RTC_EXCLUSIVE_LOCKS_REQUIRED(mutex_rtcp_sender_); - std::unique_ptr BuildExtendedReports( - const RtcpContext& context) + void BuildExtendedReports(const RtcpContext& context, PacketSender& sender) RTC_EXCLUSIVE_LOCKS_REQUIRED(mutex_rtcp_sender_); - std::unique_ptr BuildBYE(const RtcpContext& context) + void BuildBYE(const RtcpContext& context, PacketSender& sender) RTC_EXCLUSIVE_LOCKS_REQUIRED(mutex_rtcp_sender_); - std::unique_ptr BuildFIR(const RtcpContext& context) + void BuildFIR(const RtcpContext& context, PacketSender& sender) RTC_EXCLUSIVE_LOCKS_REQUIRED(mutex_rtcp_sender_); - std::unique_ptr BuildNACK(const RtcpContext& context) + void BuildNACK(const RtcpContext& context, PacketSender& sender) RTC_EXCLUSIVE_LOCKS_REQUIRED(mutex_rtcp_sender_); - private: const bool audio_; const uint32_t ssrc_; Clock* const clock_; @@ -242,14 +228,7 @@ class RTCPSender final { // Full intra request uint8_t sequence_number_fir_ RTC_GUARDED_BY(mutex_rtcp_sender_); - // Loss Notification - struct LossNotificationState { - uint16_t last_decoded_seq_num; - uint16_t last_received_seq_num; - bool decodability_flag; - }; - LossNotificationState loss_notification_state_ - RTC_GUARDED_BY(mutex_rtcp_sender_); + rtcp::LossNotification loss_notification_ RTC_GUARDED_BY(mutex_rtcp_sender_); // REMB int64_t remb_bitrate_ RTC_GUARDED_BY(mutex_rtcp_sender_); @@ -281,8 +260,6 @@ class RTCPSender final { void SetFlag(uint32_t type, bool is_volatile) RTC_EXCLUSIVE_LOCKS_REQUIRED(mutex_rtcp_sender_); - void SetFlags(const std::set& types, bool is_volatile) - RTC_EXCLUSIVE_LOCKS_REQUIRED(mutex_rtcp_sender_); bool IsFlagPresent(uint32_t type) const RTC_EXCLUSIVE_LOCKS_REQUIRED(mutex_rtcp_sender_); bool ConsumeFlag(uint32_t type, bool forced = false) @@ -300,8 +277,7 @@ class RTCPSender final { std::set report_flags_ RTC_GUARDED_BY(mutex_rtcp_sender_); - typedef std::unique_ptr (RTCPSender::*BuilderFunc)( - const RtcpContext&); + typedef void (RTCPSender::*BuilderFunc)(const RtcpContext&, PacketSender&); // Map from RTCPPacketType to builder. std::map builders_; }; diff --git a/modules/rtp_rtcp/source/rtcp_sender_unittest.cc b/modules/rtp_rtcp/source/rtcp_sender_unittest.cc index 4c8038fd04..6bf75c6be1 100644 --- a/modules/rtp_rtcp/source/rtcp_sender_unittest.cc +++ b/modules/rtp_rtcp/source/rtcp_sender_unittest.cc @@ -648,21 +648,6 @@ TEST_F(RtcpSenderTest, SendsTmmbnIfSetAndEmpty) { EXPECT_EQ(0U, parser()->tmmbn()->items().size()); } -TEST_F(RtcpSenderTest, SendCompoundPliRemb) { - const int kBitrate = 261011; - auto rtcp_sender = CreateRtcpSender(GetDefaultConfig()); - std::vector ssrcs; - ssrcs.push_back(kRemoteSsrc); - rtcp_sender->SetRTCPStatus(RtcpMode::kCompound); - rtcp_sender->SetRemb(kBitrate, ssrcs); - std::set packet_types; - packet_types.insert(kRtcpRemb); - packet_types.insert(kRtcpPli); - EXPECT_EQ(0, rtcp_sender->SendCompoundRTCP(feedback_state(), packet_types)); - EXPECT_EQ(1, parser()->remb()->num_packets()); - EXPECT_EQ(1, parser()->pli()->num_packets()); -} - // This test is written to verify that BYE is always the last packet // type in a RTCP compoud packet. The rtcp_sender is recreated with // mock_transport, which is used to check for whether BYE at the end