From 812186d0604032bf671924252c492a11e8f8311b Mon Sep 17 00:00:00 2001 From: Danil Chapovalov Date: Mon, 6 Mar 2023 17:44:30 +0100 Subject: [PATCH] Unassign personal TODOs in modules/rtp_rtcp MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 1st TODO is removed because there are no plans to use RtcpSender for multiple media streams, RtcpTransceiver designed for that instead 2nd TODO is removed because benefits of not producing same report blocks is unclear, while there is risk to break RTT calculation for remote receivers TODOs in RtcpTransceiver are reassigned to general bug about it instead, in particular to note low priority, same as that issues has. No-Try: true Bug: webrtc:10198 Change-Id: Ibcc3691265459c0732019ceba7903e27d57d543a Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/296360 Auto-Submit: Danil Chapovalov Commit-Queue: Åsa Persson Reviewed-by: Åsa Persson Cr-Commit-Position: refs/heads/main@{#39488} --- modules/rtp_rtcp/source/rtcp_sender.cc | 8 +------- modules/rtp_rtcp/source/rtcp_transceiver_impl.cc | 5 +++-- modules/rtp_rtcp/source/rtcp_transceiver_impl.h | 4 ++-- 3 files changed, 6 insertions(+), 11 deletions(-) diff --git a/modules/rtp_rtcp/source/rtcp_sender.cc b/modules/rtp_rtcp/source/rtcp_sender.cc index 1f58a24c04..75f403d6fa 100644 --- a/modules/rtp_rtcp/source/rtcp_sender.cc +++ b/modules/rtp_rtcp/source/rtcp_sender.cc @@ -816,20 +816,14 @@ std::vector RTCPSender::CreateReportBlocks( if (!receive_statistics_) return result; - // TODO(danilchap): Support sending more than `RTCP_MAX_REPORT_BLOCKS` per - // compound rtcp packet when single rtcp module is used for multiple media - // streams. result = receive_statistics_->RtcpReportBlocks(RTCP_MAX_REPORT_BLOCKS); if (!result.empty() && feedback_state.last_rr.Valid()) { // Get our NTP as late as possible to avoid a race. uint32_t now = CompactNtp(clock_->CurrentNtpTime()); uint32_t receive_time = CompactNtp(feedback_state.last_rr); - uint32_t delay_since_last_sr = now - receive_time; - // TODO(danilchap): Instead of setting same value on all report blocks, - // set only when media_ssrc match sender ssrc of the sender report - // remote times were taken from. + for (auto& report_block : result) { report_block.SetLastSr(feedback_state.remote_sr); report_block.SetDelayLastSr(delay_since_last_sr); diff --git a/modules/rtp_rtcp/source/rtcp_transceiver_impl.cc b/modules/rtp_rtcp/source/rtcp_transceiver_impl.cc index bb4f96b970..d32880e37e 100644 --- a/modules/rtp_rtcp/source/rtcp_transceiver_impl.cc +++ b/modules/rtp_rtcp/source/rtcp_transceiver_impl.cc @@ -64,8 +64,9 @@ struct RtcpTransceiverImpl::LocalSenderState { // Helper to put several RTCP packets into lower layer datagram composing // Compound or Reduced-Size RTCP packet, as defined by RFC 5506 section 2. -// TODO(danilchap): When in compound mode and packets are so many that several -// compound RTCP packets need to be generated, ensure each packet is compound. +// TODO(bugs.webrtc.org/8239): When in compound mode and packets are so many +// that several compound RTCP packets need to be generated, ensure each packet +// is compound. class RtcpTransceiverImpl::PacketSender { public: PacketSender(rtcp::RtcpPacket::PacketReadyCallback callback, diff --git a/modules/rtp_rtcp/source/rtcp_transceiver_impl.h b/modules/rtp_rtcp/source/rtcp_transceiver_impl.h index 8a3333d45c..5c147b1f34 100644 --- a/modules/rtp_rtcp/source/rtcp_transceiver_impl.h +++ b/modules/rtp_rtcp/source/rtcp_transceiver_impl.h @@ -155,8 +155,8 @@ class RtcpTransceiverImpl { bool ready_to_send_; absl::optional remb_; - // TODO(danilchap): Remove entries from remote_senders_ that are no longer - // needed. + // TODO(bugs.webrtc.org/8239): Remove entries from remote_senders_ that are no + // longer needed. flat_map remote_senders_; std::list local_senders_; flat_map::iterator>