From c6b9ac782ae6d3f8e84194880f4fe145c5ed5685 Mon Sep 17 00:00:00 2001 From: Markus Handell Date: Fri, 18 Jun 2021 13:44:51 +0200 Subject: [PATCH] RTCPSender: migrate to Timestamp. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This change migrates RTCPSender to use webrtc::Timestamp, preparing for later improvements regarding bugs.webrtc.org/11581. Fixed: webrtc:12873 Change-Id: I1159701dc373883367d9b2c86823f8fb59904d55 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/222324 Commit-Queue: Markus Handell Reviewed-by: Erik Språng Cr-Commit-Position: refs/heads/master@{#34346} --- modules/rtp_rtcp/source/rtcp_sender.cc | 75 +++++++++++-------- modules/rtp_rtcp/source/rtcp_sender.h | 15 +++- .../rtp_rtcp/source/rtcp_sender_unittest.cc | 5 +- modules/rtp_rtcp/source/rtp_rtcp_config.h | 10 ++- modules/rtp_rtcp/source/rtp_rtcp_impl.cc | 11 ++- modules/rtp_rtcp/source/rtp_rtcp_impl2.cc | 13 +++- 6 files changed, 85 insertions(+), 44 deletions(-) diff --git a/modules/rtp_rtcp/source/rtcp_sender.cc b/modules/rtp_rtcp/source/rtcp_sender.cc index 0e40d88e67..f531a30cd5 100644 --- a/modules/rtp_rtcp/source/rtcp_sender.cc +++ b/modules/rtp_rtcp/source/rtcp_sender.cc @@ -16,7 +16,9 @@ #include #include +#include "absl/types/optional.h" #include "api/rtc_event_log/rtc_event_log.h" +#include "api/rtp_headers.h" #include "api/units/time_delta.h" #include "api/units/timestamp.h" #include "logging/rtc_event_log/events/rtc_event_rtcp_packet_outgoing.h" @@ -144,16 +146,12 @@ RTCPSender::RTCPSender(const Configuration& config) method_(RtcpMode::kOff), event_log_(config.event_log), transport_(config.outgoing_transport), - report_interval_ms_(config.rtcp_report_interval - .value_or(TimeDelta::Millis( - config.audio ? kDefaultAudioReportInterval - : kDefaultVideoReportInterval)) - .ms()), + report_interval_(config.rtcp_report_interval.value_or( + TimeDelta::Millis(config.audio ? kDefaultAudioReportInterval + : kDefaultVideoReportInterval))), sending_(false), - next_time_to_send_rtcp_(0), timestamp_offset_(0), last_rtp_timestamp_(0), - last_frame_capture_time_ms_(-1), remote_ssrc_(0), receive_statistics_(config.receive_statistics), @@ -199,10 +197,11 @@ RtcpMode RTCPSender::Status() const { void RTCPSender::SetRTCPStatus(RtcpMode new_method) { MutexLock lock(&mutex_rtcp_sender_); - if (method_ == RtcpMode::kOff && new_method != RtcpMode::kOff) { + if (new_method == RtcpMode::kOff) { + next_time_to_send_rtcp_ = absl::nullopt; + } else if (method_ == RtcpMode::kOff) { // When switching on, reschedule the next packet - next_time_to_send_rtcp_ = - clock_->TimeInMilliseconds() + (report_interval_ms_ / 2); + next_time_to_send_rtcp_ = clock_->CurrentTime() + report_interval_ / 2; } method_ = new_method; } @@ -285,7 +284,7 @@ void RTCPSender::SetRemb(int64_t bitrate_bps, std::vector ssrcs) { SetFlag(kRtcpRemb, /*is_volatile=*/false); // Send a REMB immediately if we have a new REMB. The frequency of REMBs is // throttled by the caller. - next_time_to_send_rtcp_ = clock_->TimeInMilliseconds(); + next_time_to_send_rtcp_ = clock_->CurrentTime(); } void RTCPSender::UnsetRemb() { @@ -310,23 +309,33 @@ void RTCPSender::SetTimestampOffset(uint32_t timestamp_offset) { } void RTCPSender::SetLastRtpTime(uint32_t rtp_timestamp, - int64_t capture_time_ms, - int8_t payload_type) { + absl::optional capture_time, + absl::optional payload_type) { MutexLock lock(&mutex_rtcp_sender_); // For compatibility with clients who don't set payload type correctly on all // calls. - if (payload_type != -1) { - last_payload_type_ = payload_type; + if (payload_type.has_value()) { + last_payload_type_ = *payload_type; } last_rtp_timestamp_ = rtp_timestamp; - if (capture_time_ms <= 0) { + if (!capture_time.has_value()) { // We don't currently get a capture time from VoiceEngine. - last_frame_capture_time_ms_ = clock_->TimeInMilliseconds(); + last_frame_capture_time_ = clock_->CurrentTime(); } else { - last_frame_capture_time_ms_ = capture_time_ms; + last_frame_capture_time_ = *capture_time; } } +void RTCPSender::SetLastRtpTime(uint32_t rtp_timestamp, + int64_t capture_time_ms, + int8_t payload_type) { + absl::optional payload_type_optional; + if (payload_type != -1) + payload_type_optional = payload_type; + SetLastRtpTime(rtp_timestamp, Timestamp::Millis(capture_time_ms), + payload_type_optional); +} + void RTCPSender::SetRtpClockRate(int8_t payload_type, int rtp_clock_rate_hz) { MutexLock lock(&mutex_rtcp_sender_); rtp_clock_rates_khz_[payload_type] = rtp_clock_rate_hz / 1000; @@ -416,7 +425,7 @@ bool RTCPSender::TimeToSendRTCPReport(bool sendKeyframeBeforeRTP) const { a value of the RTCP bandwidth below the intended average */ - int64_t now = clock_->TimeInMilliseconds(); + Timestamp now = clock_->CurrentTime(); MutexLock lock(&mutex_rtcp_sender_); @@ -426,15 +435,15 @@ bool RTCPSender::TimeToSendRTCPReport(bool sendKeyframeBeforeRTP) const { if (!audio_ && sendKeyframeBeforeRTP) { // for video key-frames we want to send the RTCP before the large key-frame // if we have a 100 ms margin - now += RTCP_SEND_BEFORE_KEY_FRAME_MS; + now += RTCP_SEND_BEFORE_KEY_FRAME; } - return now >= next_time_to_send_rtcp_; + return now >= *next_time_to_send_rtcp_; } 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); + RTC_DCHECK(last_frame_capture_time_.has_value()); // The timestamp of this RTCP packet should be estimated as the timestamp of // the frame being captured at this moment. We are calculating that // timestamp as the last frame's timestamp + the time since the last frame @@ -449,7 +458,8 @@ void RTCPSender::BuildSR(const RtcpContext& ctx, PacketSender& sender) { // when converted to milliseconds, uint32_t rtp_timestamp = timestamp_offset_ + last_rtp_timestamp_ + - ((ctx.now_.us() + 500) / 1000 - last_frame_capture_time_ms_) * rtp_rate; + ((ctx.now_.us() + 500) / 1000 - last_frame_capture_time_->ms()) * + rtp_rate; rtcp::SenderReport report; report.SetSenderSsrc(ssrc_); @@ -688,7 +698,7 @@ absl::optional RTCPSender::ComputeCompoundRTCPPacket( 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); + const bool can_calculate_rtp_timestamp = last_frame_capture_time_.has_value(); if (!can_calculate_rtp_timestamp) { bool consumed_sr_flag = ConsumeFlag(kRtcpSr); bool consumed_report_flag = sending_ && ConsumeFlag(kRtcpReport); @@ -779,24 +789,25 @@ void RTCPSender::PrepareReport(const FeedbackState& feedback_state) { } // generate next time to send an RTCP report - int min_interval_ms = report_interval_ms_; + TimeDelta min_interval = report_interval_; if (!audio_ && sending_) { // Calculate bandwidth for video; 360 / send bandwidth in kbit/s. int send_bitrate_kbit = feedback_state.send_bitrate / 1000; if (send_bitrate_kbit != 0) { - min_interval_ms = 360000 / send_bitrate_kbit; - min_interval_ms = std::min(min_interval_ms, report_interval_ms_); + min_interval = std::min(TimeDelta::Millis(360000 / send_bitrate_kbit), + report_interval_); } } // The interval between RTCP packets is varied randomly over the // range [1/2,3/2] times the calculated interval. - int time_to_next = - random_.Rand(min_interval_ms * 1 / 2, min_interval_ms * 3 / 2); + int min_interval_int = rtc::dchecked_cast(min_interval.ms()); + TimeDelta time_to_next = TimeDelta::Millis( + random_.Rand(min_interval_int * 1 / 2, min_interval_int * 3 / 2)); - RTC_DCHECK_GT(time_to_next, 0); - next_time_to_send_rtcp_ = clock_->TimeInMilliseconds() + time_to_next; + RTC_DCHECK(!time_to_next.IsZero()); + next_time_to_send_rtcp_ = clock_->CurrentTime() + time_to_next; // RtcpSender expected to be used for sending either just sender reports // or just receiver reports. @@ -890,7 +901,7 @@ void RTCPSender::SetVideoBitrateAllocation( RTC_LOG(LS_INFO) << "Emitting TargetBitrate XR for SSRC " << ssrc_ << " with new layers enabled/disabled: " << video_bitrate_allocation_.ToString(); - next_time_to_send_rtcp_ = clock_->TimeInMilliseconds(); + next_time_to_send_rtcp_ = clock_->CurrentTime(); } else { video_bitrate_allocation_ = bitrate; } diff --git a/modules/rtp_rtcp/source/rtcp_sender.h b/modules/rtp_rtcp/source/rtcp_sender.h index aa13156a94..e3fda383ba 100644 --- a/modules/rtp_rtcp/source/rtcp_sender.h +++ b/modules/rtp_rtcp/source/rtcp_sender.h @@ -20,6 +20,7 @@ #include "absl/types/optional.h" #include "api/call/transport.h" #include "api/units/time_delta.h" +#include "api/units/timestamp.h" #include "api/video/video_bitrate_allocation.h" #include "modules/remote_bitrate_estimator/include/remote_bitrate_estimator.h" #include "modules/rtp_rtcp/include/receive_statistics.h" @@ -114,8 +115,14 @@ class RTCPSender final { void SetTimestampOffset(uint32_t timestamp_offset) RTC_LOCKS_EXCLUDED(mutex_rtcp_sender_); + void SetLastRtpTime(uint32_t rtp_timestamp, + absl::optional capture_time, + absl::optional payload_type) + RTC_LOCKS_EXCLUDED(mutex_rtcp_sender_); // TODO(bugs.webrtc.org/6458): Remove default parameter value when all the // depending projects are updated to correctly set payload type. + // TODO(bugs.webrtc.org/12873): Remove once downstream consumers migrates to + // the new version of SetLastRtpTime declared above. void SetLastRtpTime(uint32_t rtp_timestamp, int64_t capture_time_ms, int8_t payload_type = -1) @@ -230,16 +237,18 @@ class RTCPSender final { RtcEventLog* const event_log_; Transport* const transport_; - const int report_interval_ms_; + const TimeDelta report_interval_; mutable Mutex mutex_rtcp_sender_; bool sending_ RTC_GUARDED_BY(mutex_rtcp_sender_); - int64_t next_time_to_send_rtcp_ RTC_GUARDED_BY(mutex_rtcp_sender_); + absl::optional next_time_to_send_rtcp_ + RTC_GUARDED_BY(mutex_rtcp_sender_); uint32_t timestamp_offset_ RTC_GUARDED_BY(mutex_rtcp_sender_); uint32_t last_rtp_timestamp_ RTC_GUARDED_BY(mutex_rtcp_sender_); - int64_t last_frame_capture_time_ms_ RTC_GUARDED_BY(mutex_rtcp_sender_); + absl::optional last_frame_capture_time_ + RTC_GUARDED_BY(mutex_rtcp_sender_); // SSRC that we receive on our RTP channel uint32_t remote_ssrc_ RTC_GUARDED_BY(mutex_rtcp_sender_); std::string cname_ RTC_GUARDED_BY(mutex_rtcp_sender_); diff --git a/modules/rtp_rtcp/source/rtcp_sender_unittest.cc b/modules/rtp_rtcp/source/rtcp_sender_unittest.cc index 706b804d19..347be79398 100644 --- a/modules/rtp_rtcp/source/rtcp_sender_unittest.cc +++ b/modules/rtp_rtcp/source/rtcp_sender_unittest.cc @@ -78,8 +78,7 @@ std::unique_ptr CreateRtcpSender( rtcp_sender->SetRemoteSSRC(kRemoteSsrc); if (init_timestamps) { rtcp_sender->SetTimestampOffset(kStartRtpTimestamp); - rtcp_sender->SetLastRtpTime(kRtpTimestamp, - config.clock->TimeInMilliseconds(), + rtcp_sender->SetLastRtpTime(kRtpTimestamp, config.clock->CurrentTime(), /*payload_type=*/0); } return rtcp_sender; @@ -660,7 +659,7 @@ TEST_F(RtcpSenderTest, ByeMustBeLast) { auto rtcp_sender = CreateRtcpSender(config); rtcp_sender->SetTimestampOffset(kStartRtpTimestamp); - rtcp_sender->SetLastRtpTime(kRtpTimestamp, clock_.TimeInMilliseconds(), + rtcp_sender->SetLastRtpTime(kRtpTimestamp, clock_.CurrentTime(), /*payload_type=*/0); // Set up REMB info to be included with BYE. diff --git a/modules/rtp_rtcp/source/rtp_rtcp_config.h b/modules/rtp_rtcp/source/rtp_rtcp_config.h index 6863c4c353..66caadd578 100644 --- a/modules/rtp_rtcp/source/rtp_rtcp_config.h +++ b/modules/rtp_rtcp/source/rtp_rtcp_config.h @@ -11,13 +11,15 @@ #ifndef MODULES_RTP_RTCP_SOURCE_RTP_RTCP_CONFIG_H_ #define MODULES_RTP_RTCP_SOURCE_RTP_RTCP_CONFIG_H_ +#include "api/units/time_delta.h" + // Configuration file for RTP utilities (RTPSender, RTPReceiver ...) namespace webrtc { -enum { kDefaultMaxReorderingThreshold = 50 }; // In sequence numbers. -enum { kRtcpMaxNackFields = 253 }; +constexpr int kDefaultMaxReorderingThreshold = 5; // In sequence numbers. +constexpr int kRtcpMaxNackFields = 253; -enum { RTCP_SEND_BEFORE_KEY_FRAME_MS = 100 }; -enum { RTCP_MAX_REPORT_BLOCKS = 31 }; // RFC 3550 page 37 +constexpr TimeDelta RTCP_SEND_BEFORE_KEY_FRAME = TimeDelta::Millis(100); +constexpr int RTCP_MAX_REPORT_BLOCKS = 31; // RFC 3550 page 37 } // namespace webrtc #endif // MODULES_RTP_RTCP_SOURCE_RTP_RTCP_CONFIG_H_ diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl.cc b/modules/rtp_rtcp/source/rtp_rtcp_impl.cc index 8d5b3ac754..3f985e213a 100644 --- a/modules/rtp_rtcp/source/rtp_rtcp_impl.cc +++ b/modules/rtp_rtcp/source/rtp_rtcp_impl.cc @@ -381,7 +381,16 @@ bool ModuleRtpRtcpImpl::OnSendingRtpFrame(uint32_t timestamp, if (!Sending()) return false; - rtcp_sender_.SetLastRtpTime(timestamp, capture_time_ms, payload_type); + // TODO(bugs.webrtc.org/12873): Migrate this method and it's users to use + // optional Timestamps. + absl::optional capture_time; + if (capture_time_ms > 0) { + capture_time = Timestamp::Millis(capture_time_ms); + } + absl::optional payload_type_optional; + if (payload_type >= 0) + payload_type_optional = payload_type; + rtcp_sender_.SetLastRtpTime(timestamp, capture_time, payload_type_optional); // Make sure an RTCP report isn't queued behind a key frame. if (rtcp_sender_.TimeToSendRTCPReport(force_sender_report)) rtcp_sender_.SendRTCP(GetFeedbackState(), kRtcpReport); diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl2.cc b/modules/rtp_rtcp/source/rtp_rtcp_impl2.cc index 3a2ec81fdc..25c92b267a 100644 --- a/modules/rtp_rtcp/source/rtp_rtcp_impl2.cc +++ b/modules/rtp_rtcp/source/rtp_rtcp_impl2.cc @@ -19,7 +19,9 @@ #include #include +#include "absl/types/optional.h" #include "api/transport/field_trial_based_config.h" +#include "api/units/timestamp.h" #include "modules/rtp_rtcp/source/rtcp_packet/dlrr.h" #include "modules/rtp_rtcp/source/rtp_rtcp_config.h" #include "rtc_base/checks.h" @@ -337,7 +339,16 @@ bool ModuleRtpRtcpImpl2::OnSendingRtpFrame(uint32_t timestamp, if (!Sending()) return false; - rtcp_sender_.SetLastRtpTime(timestamp, capture_time_ms, payload_type); + // TODO(bugs.webrtc.org/12873): Migrate this method and it's users to use + // optional Timestamps. + absl::optional capture_time; + if (capture_time_ms > 0) { + capture_time = Timestamp::Millis(capture_time_ms); + } + absl::optional payload_type_optional; + if (payload_type >= 0) + payload_type_optional = payload_type; + rtcp_sender_.SetLastRtpTime(timestamp, capture_time, payload_type_optional); // Make sure an RTCP report isn't queued behind a key frame. if (rtcp_sender_.TimeToSendRTCPReport(force_sender_report)) rtcp_sender_.SendRTCP(GetFeedbackState(), kRtcpReport);