diff --git a/call/rtp_video_sender.cc b/call/rtp_video_sender.cc index 3ae0794631..413171fa67 100644 --- a/call/rtp_video_sender.cc +++ b/call/rtp_video_sender.cc @@ -24,6 +24,7 @@ #include "modules/pacing/packet_router.h" #include "modules/rtp_rtcp/include/rtp_rtcp.h" #include "modules/rtp_rtcp/include/rtp_rtcp_defines.h" +#include "modules/rtp_rtcp/source/playout_delay_oracle.h" #include "modules/rtp_rtcp/source/rtp_sender.h" #include "modules/utility/include/process_thread.h" #include "modules/video_coding/include/video_codec_interface.h" @@ -36,9 +37,13 @@ namespace webrtc { namespace webrtc_internal_rtp_video_sender { -RtpStreamSender::RtpStreamSender(std::unique_ptr rtp_rtcp, - std::unique_ptr sender_video) - : rtp_rtcp(std::move(rtp_rtcp)), sender_video(std::move(sender_video)) {} +RtpStreamSender::RtpStreamSender( + std::unique_ptr playout_delay_oracle, + std::unique_ptr rtp_rtcp, + std::unique_ptr sender_video) + : playout_delay_oracle(std::move(playout_delay_oracle)), + rtp_rtcp(std::move(rtp_rtcp)), + sender_video(std::move(sender_video)) {} RtpStreamSender::~RtpStreamSender() = default; @@ -172,7 +177,9 @@ std::vector CreateRtpStreamSenders( configuration.local_media_ssrc) != flexfec_protected_ssrcs.end(); configuration.flexfec_sender = enable_flexfec ? flexfec_sender : nullptr; + auto playout_delay_oracle = std::make_unique(); + configuration.ack_observer = playout_delay_oracle.get(); if (rtp_config.rtx.ssrcs.size() > i) { configuration.rtx_send_ssrc = rtp_config.rtx.ssrcs[i]; } @@ -189,6 +196,7 @@ std::vector CreateRtpStreamSenders( video_config.clock = configuration.clock; video_config.rtp_sender = rtp_rtcp->RtpSender(); video_config.flexfec_sender = configuration.flexfec_sender; + video_config.playout_delay_oracle = playout_delay_oracle.get(); video_config.frame_encryptor = frame_encryptor; video_config.require_frame_encryption = crypto_options.sframe.require_frame_encryption; @@ -206,7 +214,8 @@ std::vector CreateRtpStreamSenders( video_config.ulpfec_payload_type = rtp_config.ulpfec.ulpfec_payload_type; } auto sender_video = std::make_unique(video_config); - rtp_streams.emplace_back(std::move(rtp_rtcp), std::move(sender_video)); + rtp_streams.emplace_back(std::move(playout_delay_oracle), + std::move(rtp_rtcp), std::move(sender_video)); } return rtp_streams; } diff --git a/call/rtp_video_sender.h b/call/rtp_video_sender.h index 620c975810..eb7e4315be 100644 --- a/call/rtp_video_sender.h +++ b/call/rtp_video_sender.h @@ -50,7 +50,8 @@ namespace webrtc_internal_rtp_video_sender { // RTP state for a single simulcast stream. Internal to the implementation of // RtpVideoSender. struct RtpStreamSender { - RtpStreamSender(std::unique_ptr rtp_rtcp, + RtpStreamSender(std::unique_ptr playout_delay_oracle, + std::unique_ptr rtp_rtcp, std::unique_ptr sender_video); ~RtpStreamSender(); @@ -58,6 +59,7 @@ struct RtpStreamSender { RtpStreamSender& operator=(RtpStreamSender&&) = default; // Note: Needs pointer stability. + std::unique_ptr playout_delay_oracle; std::unique_ptr rtp_rtcp; std::unique_ptr sender_video; }; diff --git a/common_types.h b/common_types.h index dedcbd5460..aadda4fb99 100644 --- a/common_types.h +++ b/common_types.h @@ -89,16 +89,8 @@ typedef SpatialLayer SimulcastStream; // Note: Given that this gets embedded in a union, it is up-to the owner to // initialize these values. struct PlayoutDelay { - PlayoutDelay(int min_ms, int max_ms) : min_ms(min_ms), max_ms(max_ms) {} int min_ms; int max_ms; - - static PlayoutDelay Noop() { return PlayoutDelay(-1, -1); } - - bool IsNoop() const { return min_ms == -1 && max_ms == -1; } - bool operator==(const PlayoutDelay& rhs) const { - return min_ms == rhs.min_ms && max_ms == rhs.max_ms; - } }; } // namespace webrtc diff --git a/modules/rtp_rtcp/BUILD.gn b/modules/rtp_rtcp/BUILD.gn index b8dd23ed86..099c0663d2 100644 --- a/modules/rtp_rtcp/BUILD.gn +++ b/modules/rtp_rtcp/BUILD.gn @@ -156,6 +156,7 @@ rtc_library("rtp_rtcp") { "source/forward_error_correction_internal.h", "source/packet_loss_stats.cc", "source/packet_loss_stats.h", + "source/playout_delay_oracle.cc", "source/playout_delay_oracle.h", "source/receive_statistics_impl.cc", "source/receive_statistics_impl.h", @@ -428,6 +429,7 @@ if (rtc_include_tests) { "source/flexfec_sender_unittest.cc", "source/nack_rtx_unittest.cc", "source/packet_loss_stats_unittest.cc", + "source/playout_delay_oracle_unittest.cc", "source/receive_statistics_unittest.cc", "source/remote_ntp_time_estimator_unittest.cc", "source/rtcp_nack_stats_unittest.cc", diff --git a/modules/rtp_rtcp/include/rtp_rtcp.h b/modules/rtp_rtcp/include/rtp_rtcp.h index fbb3bb3241..b3cd8f6418 100644 --- a/modules/rtp_rtcp/include/rtp_rtcp.h +++ b/modules/rtp_rtcp/include/rtp_rtcp.h @@ -101,6 +101,7 @@ class RtpRtcp : public Module, public RtcpFeedbackSenderInterface { SendPacketObserver* send_packet_observer = nullptr; RateLimiter* retransmission_rate_limiter = nullptr; OverheadObserver* overhead_observer = nullptr; + RtcpAckObserver* ack_observer = nullptr; StreamDataCountersCallback* rtp_stats_callback = nullptr; int rtcp_report_interval_ms = 0; diff --git a/modules/rtp_rtcp/include/rtp_rtcp_defines.h b/modules/rtp_rtcp/include/rtp_rtcp_defines.h index bdee7b45ed..8cd402e227 100644 --- a/modules/rtp_rtcp/include/rtp_rtcp_defines.h +++ b/modules/rtp_rtcp/include/rtp_rtcp_defines.h @@ -392,6 +392,19 @@ struct RtpReceiveStats { RtpPacketCounter packet_counter; }; +class RtcpAckObserver { + public: + // This method is called on received report blocks matching the sender ssrc. + // TODO(nisse): Use of "extended" sequence number is a bit brittle, since the + // observer for this callback typically has its own sequence number unwrapper, + // and there's no guarantee that they are in sync. Change to pass raw sequence + // number, possibly augmented with timestamp (if available) to aid + // disambiguation. + virtual void OnReceivedAck(int64_t extended_highest_sequence_number) = 0; + + virtual ~RtcpAckObserver() = default; +}; + // Callback, used to notify an observer whenever new rates have been estimated. class BitrateStatisticsObserver { public: diff --git a/modules/rtp_rtcp/source/nack_rtx_unittest.cc b/modules/rtp_rtcp/source/nack_rtx_unittest.cc index 55e1e44ebe..17601dd966 100644 --- a/modules/rtp_rtcp/source/nack_rtx_unittest.cc +++ b/modules/rtp_rtcp/source/nack_rtx_unittest.cc @@ -21,6 +21,7 @@ #include "modules/rtp_rtcp/include/receive_statistics.h" #include "modules/rtp_rtcp/include/rtp_rtcp.h" #include "modules/rtp_rtcp/include/rtp_rtcp_defines.h" +#include "modules/rtp_rtcp/source/playout_delay_oracle.h" #include "modules/rtp_rtcp/source/rtp_packet_received.h" #include "modules/rtp_rtcp/source/rtp_sender_video.h" #include "rtc_base/rate_limiter.h" @@ -139,6 +140,7 @@ class RtpRtcpRtxNackTest : public ::testing::Test { RTPSenderVideo::Config video_config; video_config.clock = &fake_clock; video_config.rtp_sender = rtp_rtcp_module_->RtpSender(); + video_config.playout_delay_oracle = &playout_delay_oracle_; video_config.field_trials = &field_trials; rtp_sender_video_ = std::make_unique(video_config); rtp_rtcp_module_->SetRTCPStatus(RtcpMode::kCompound); @@ -225,6 +227,7 @@ class RtpRtcpRtxNackTest : public ::testing::Test { std::unique_ptr receive_statistics_; std::unique_ptr rtp_rtcp_module_; + PlayoutDelayOracle playout_delay_oracle_; std::unique_ptr rtp_sender_video_; RtxLoopBackTransport transport_; const std::map rtx_associated_payload_types_ = { diff --git a/modules/rtp_rtcp/source/playout_delay_oracle.cc b/modules/rtp_rtcp/source/playout_delay_oracle.cc new file mode 100644 index 0000000000..f234759678 --- /dev/null +++ b/modules/rtp_rtcp/source/playout_delay_oracle.cc @@ -0,0 +1,90 @@ +/* + * Copyright (c) 2016 The WebRTC project authors. All Rights Reserved. + * + * Use of this source code is governed by a BSD-style license + * that can be found in the LICENSE file in the root of the source + * tree. An additional intellectual property rights grant can be found + * in the file PATENTS. All contributing project authors may + * be found in the AUTHORS file in the root of the source tree. + */ + +#include "modules/rtp_rtcp/source/playout_delay_oracle.h" + +#include + +#include "modules/rtp_rtcp/source/rtp_header_extensions.h" +#include "rtc_base/checks.h" +#include "rtc_base/logging.h" + +namespace webrtc { + +PlayoutDelayOracle::PlayoutDelayOracle() = default; + +PlayoutDelayOracle::~PlayoutDelayOracle() = default; + +absl::optional PlayoutDelayOracle::PlayoutDelayToSend( + PlayoutDelay requested_delay) const { + rtc::CritScope lock(&crit_sect_); + if (requested_delay.min_ms > PlayoutDelayLimits::kMaxMs || + requested_delay.max_ms > PlayoutDelayLimits::kMaxMs) { + RTC_DLOG(LS_ERROR) + << "Requested playout delay values out of range, ignored"; + return absl::nullopt; + } + if (requested_delay.max_ms != -1 && + requested_delay.min_ms > requested_delay.max_ms) { + RTC_DLOG(LS_ERROR) << "Requested playout delay values out of order"; + return absl::nullopt; + } + if ((requested_delay.min_ms == -1 || + requested_delay.min_ms == latest_delay_.min_ms) && + (requested_delay.max_ms == -1 || + requested_delay.max_ms == latest_delay_.max_ms)) { + // Unchanged. + return unacked_sequence_number_ ? absl::make_optional(latest_delay_) + : absl::nullopt; + } + if (requested_delay.min_ms == -1) { + RTC_DCHECK_GE(requested_delay.max_ms, 0); + requested_delay.min_ms = + std::min(latest_delay_.min_ms, requested_delay.max_ms); + } + if (requested_delay.max_ms == -1) { + requested_delay.max_ms = + std::max(latest_delay_.max_ms, requested_delay.min_ms); + } + return requested_delay; +} + +void PlayoutDelayOracle::OnSentPacket(uint16_t sequence_number, + absl::optional delay) { + rtc::CritScope lock(&crit_sect_); + int64_t unwrapped_sequence_number = unwrapper_.Unwrap(sequence_number); + + if (!delay) { + return; + } + + RTC_DCHECK_LE(0, delay->min_ms); + RTC_DCHECK_LE(delay->max_ms, PlayoutDelayLimits::kMaxMs); + RTC_DCHECK_LE(delay->min_ms, delay->max_ms); + + if (delay->min_ms != latest_delay_.min_ms || + delay->max_ms != latest_delay_.max_ms) { + latest_delay_ = *delay; + unacked_sequence_number_ = unwrapped_sequence_number; + } +} + +// If an ACK is received on the packet containing the playout delay extension, +// we stop sending the extension on future packets. +void PlayoutDelayOracle::OnReceivedAck( + int64_t extended_highest_sequence_number) { + rtc::CritScope lock(&crit_sect_); + if (unacked_sequence_number_ && + extended_highest_sequence_number > *unacked_sequence_number_) { + unacked_sequence_number_ = absl::nullopt; + } +} + +} // namespace webrtc diff --git a/modules/rtp_rtcp/source/playout_delay_oracle.h b/modules/rtp_rtcp/source/playout_delay_oracle.h index 04465e3cfc..6451be4cdc 100644 --- a/modules/rtp_rtcp/source/playout_delay_oracle.h +++ b/modules/rtp_rtcp/source/playout_delay_oracle.h @@ -11,12 +11,64 @@ #ifndef MODULES_RTP_RTCP_SOURCE_PLAYOUT_DELAY_ORACLE_H_ #define MODULES_RTP_RTCP_SOURCE_PLAYOUT_DELAY_ORACLE_H_ +#include + +#include "absl/types/optional.h" +#include "common_types.h" // NOLINT(build/include) +#include "modules/include/module_common_types_public.h" +#include "modules/rtp_rtcp/include/rtp_rtcp_defines.h" +#include "rtc_base/constructor_magic.h" +#include "rtc_base/critical_section.h" +#include "rtc_base/thread_annotations.h" + namespace webrtc { -// TODO(sprang): Remove once downstream usage is gone. -class PlayoutDelayOracle { +// This class tracks the application requests to limit minimum and maximum +// playout delay and makes a decision on whether the current RTP frame +// should include the playout out delay extension header. +// +// Playout delay can be defined in terms of capture and render time as follows: +// +// Render time = Capture time in receiver time + playout delay +// +// The application specifies a minimum and maximum limit for the playout delay +// which are both communicated to the receiver and the receiver can adapt +// the playout delay within this range based on observed network jitter. +class PlayoutDelayOracle : public RtcpAckObserver { public: - PlayoutDelayOracle() = default; + PlayoutDelayOracle(); + ~PlayoutDelayOracle() override; + + // The playout delay to be added to a packet. The input delays are provided by + // the application, with -1 meaning unchanged/unspecified. The output delay + // are the values to be attached to packets on the wire. Presence and value + // depends on the current input, previous inputs, and received acks from the + // remote end. + absl::optional PlayoutDelayToSend( + PlayoutDelay requested_delay) const; + + void OnSentPacket(uint16_t sequence_number, + absl::optional playout_delay); + + void OnReceivedAck(int64_t extended_highest_sequence_number) override; + + private: + // The playout delay information is updated from the encoder thread(s). + // The sequence number feedback is updated from the worker thread. + // Guards access to data across multiple threads. + rtc::CriticalSection crit_sect_; + // The oldest sequence number on which the current playout delay values have + // been sent. When set, it means we need to attach extension to sent packets. + absl::optional unacked_sequence_number_ RTC_GUARDED_BY(crit_sect_); + // Sequence number unwrapper for sent packets. + + // TODO(nisse): Could potentially get out of sync with the unwrapper used by + // the caller of OnReceivedAck. + SequenceNumberUnwrapper unwrapper_ RTC_GUARDED_BY(crit_sect_); + // Playout delay values on the next frame if |send_playout_delay_| is set. + PlayoutDelay latest_delay_ RTC_GUARDED_BY(crit_sect_) = {-1, -1}; + + RTC_DISALLOW_COPY_AND_ASSIGN(PlayoutDelayOracle); }; } // namespace webrtc diff --git a/modules/rtp_rtcp/source/playout_delay_oracle_unittest.cc b/modules/rtp_rtcp/source/playout_delay_oracle_unittest.cc new file mode 100644 index 0000000000..3857e9b211 --- /dev/null +++ b/modules/rtp_rtcp/source/playout_delay_oracle_unittest.cc @@ -0,0 +1,52 @@ +/* + * Copyright (c) 2016 The WebRTC project authors. All Rights Reserved. + * + * Use of this source code is governed by a BSD-style license + * that can be found in the LICENSE file in the root of the source + * tree. An additional intellectual property rights grant can be found + * in the file PATENTS. All contributing project authors may + * be found in the AUTHORS file in the root of the source tree. + */ + +#include "modules/rtp_rtcp/source/playout_delay_oracle.h" + +#include "rtc_base/logging.h" +#include "test/gtest.h" + +namespace webrtc { + +namespace { +constexpr int kSequenceNumber = 100; +constexpr int kMinPlayoutDelay = 0; +constexpr int kMaxPlayoutDelay = 150; +} // namespace + +TEST(PlayoutDelayOracleTest, DisabledByDefault) { + PlayoutDelayOracle playout_delay_oracle; + EXPECT_FALSE(playout_delay_oracle.PlayoutDelayToSend({-1, -1})); +} + +TEST(PlayoutDelayOracleTest, SendPlayoutDelayUntilSeqNumberExceeds) { + PlayoutDelayOracle playout_delay_oracle; + PlayoutDelay playout_delay = {kMinPlayoutDelay, kMaxPlayoutDelay}; + playout_delay_oracle.OnSentPacket(kSequenceNumber, playout_delay); + absl::optional delay_to_send = + playout_delay_oracle.PlayoutDelayToSend({-1, -1}); + ASSERT_TRUE(delay_to_send.has_value()); + EXPECT_EQ(kMinPlayoutDelay, delay_to_send->min_ms); + EXPECT_EQ(kMaxPlayoutDelay, delay_to_send->max_ms); + + // Oracle indicates playout delay should be sent if highest sequence number + // acked is lower than the sequence number of the first packet containing + // playout delay. + playout_delay_oracle.OnReceivedAck(kSequenceNumber - 1); + EXPECT_TRUE(playout_delay_oracle.PlayoutDelayToSend({-1, -1})); + + // Oracle indicates playout delay should not be sent if sequence number + // acked on a matching ssrc indicates the receiver has received the playout + // delay values. + playout_delay_oracle.OnReceivedAck(kSequenceNumber + 1); + EXPECT_FALSE(playout_delay_oracle.PlayoutDelayToSend({-1, -1})); +} + +} // namespace webrtc diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl.cc b/modules/rtp_rtcp/source/rtp_rtcp_impl.cc index dfbac29d03..987ae0ec59 100644 --- a/modules/rtp_rtcp/source/rtp_rtcp_impl.cc +++ b/modules/rtp_rtcp/source/rtp_rtcp_impl.cc @@ -68,6 +68,7 @@ ModuleRtpRtcpImpl::ModuleRtpRtcpImpl(const Configuration& configuration) nack_last_time_sent_full_ms_(0), nack_last_seq_number_sent_(0), remote_bitrate_(configuration.remote_bitrate_estimator), + ack_observer_(configuration.ack_observer), rtt_stats_(configuration.rtt_stats), rtt_ms_(0) { if (!configuration.receiver_only) { @@ -735,7 +736,7 @@ void ModuleRtpRtcpImpl::OnReceivedNack( void ModuleRtpRtcpImpl::OnReceivedRtcpReportBlocks( const ReportBlockList& report_blocks) { - if (rtp_sender_) { + if (ack_observer_) { uint32_t ssrc = SSRC(); absl::optional rtx_ssrc; if (rtp_sender_->packet_generator.RtxStatus() != kRtxOff) { @@ -746,6 +747,8 @@ void ModuleRtpRtcpImpl::OnReceivedRtcpReportBlocks( if (ssrc == report_block.source_ssrc) { rtp_sender_->packet_generator.OnReceivedAckOnSsrc( report_block.extended_highest_sequence_number); + ack_observer_->OnReceivedAck( + report_block.extended_highest_sequence_number); } else if (rtx_ssrc && *rtx_ssrc == report_block.source_ssrc) { rtp_sender_->packet_generator.OnReceivedAckOnRtxSsrc( report_block.extended_highest_sequence_number); diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl.h b/modules/rtp_rtcp/source/rtp_rtcp_impl.h index c03683f48e..976653a458 100644 --- a/modules/rtp_rtcp/source/rtp_rtcp_impl.h +++ b/modules/rtp_rtcp/source/rtp_rtcp_impl.h @@ -340,6 +340,8 @@ class ModuleRtpRtcpImpl : public RtpRtcp, public RTCPReceiver::ModuleRtpRtcp { RemoteBitrateEstimator* const remote_bitrate_; + RtcpAckObserver* const ack_observer_; + RtcpRttStats* const rtt_stats_; // The processed RTT from RtcpRttStats. diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl_unittest.cc b/modules/rtp_rtcp/source/rtp_rtcp_impl_unittest.cc index 5e4cce99a7..0b681cf183 100644 --- a/modules/rtp_rtcp/source/rtp_rtcp_impl_unittest.cc +++ b/modules/rtp_rtcp/source/rtp_rtcp_impl_unittest.cc @@ -17,6 +17,7 @@ #include "api/transport/field_trial_based_config.h" #include "api/video_codecs/video_codec.h" #include "modules/rtp_rtcp/include/rtp_rtcp_defines.h" +#include "modules/rtp_rtcp/source/playout_delay_oracle.h" #include "modules/rtp_rtcp/source/rtcp_packet.h" #include "modules/rtp_rtcp/source/rtcp_packet/nack.h" #include "modules/rtp_rtcp/source/rtp_packet_received.h" @@ -181,6 +182,7 @@ class RtpRtcpImplTest : public ::testing::Test { RTPSenderVideo::Config video_config; video_config.clock = &clock_; video_config.rtp_sender = sender_.impl_->RtpSender(); + video_config.playout_delay_oracle = &playout_delay_oracle_; video_config.field_trials = &field_trials; sender_video_ = std::make_unique(video_config); @@ -199,6 +201,7 @@ class RtpRtcpImplTest : public ::testing::Test { SimulatedClock clock_; RtpRtcpModule sender_; + PlayoutDelayOracle playout_delay_oracle_; std::unique_ptr sender_video_; RtpRtcpModule receiver_; VideoCodec codec_; diff --git a/modules/rtp_rtcp/source/rtp_sender_unittest.cc b/modules/rtp_rtcp/source/rtp_sender_unittest.cc index 458d3e7eb6..5ca4e70de8 100644 --- a/modules/rtp_rtcp/source/rtp_sender_unittest.cc +++ b/modules/rtp_rtcp/source/rtp_sender_unittest.cc @@ -649,10 +649,12 @@ TEST_P(RtpSenderTestWithoutPacer, OnSendSideDelayUpdated) { config.event_log = &mock_rtc_event_log_; rtp_sender_context_ = std::make_unique(config); + PlayoutDelayOracle playout_delay_oracle; FieldTrialBasedConfig field_trials; RTPSenderVideo::Config video_config; video_config.clock = &fake_clock_; video_config.rtp_sender = rtp_sender(); + video_config.playout_delay_oracle = &playout_delay_oracle; video_config.field_trials = &field_trials; RTPSenderVideo rtp_sender_video(video_config); @@ -1151,10 +1153,12 @@ TEST_P(RtpSenderTest, OnSendPacketNotUpdatedForRetransmits) { TEST_P(RtpSenderTestWithoutPacer, SendGenericVideo) { const uint8_t kPayloadType = 127; const VideoCodecType kCodecType = VideoCodecType::kVideoCodecGeneric; + PlayoutDelayOracle playout_delay_oracle; FieldTrialBasedConfig field_trials; RTPSenderVideo::Config video_config; video_config.clock = &fake_clock_; video_config.rtp_sender = rtp_sender(); + video_config.playout_delay_oracle = &playout_delay_oracle; video_config.field_trials = &field_trials; RTPSenderVideo rtp_sender_video(video_config); uint8_t payload[] = {47, 11, 32, 93, 89}; @@ -1193,10 +1197,12 @@ TEST_P(RtpSenderTestWithoutPacer, SendRawVideo) { const uint8_t kPayloadType = 111; const uint8_t payload[] = {11, 22, 33, 44, 55}; + PlayoutDelayOracle playout_delay_oracle; FieldTrialBasedConfig field_trials; RTPSenderVideo::Config video_config; video_config.clock = &fake_clock_; video_config.rtp_sender = rtp_sender(); + video_config.playout_delay_oracle = &playout_delay_oracle; video_config.field_trials = &field_trials; RTPSenderVideo rtp_sender_video(video_config); @@ -1238,11 +1244,13 @@ TEST_P(RtpSenderTest, SendFlexfecPackets) { rtp_sender_context_->packet_history_.SetStorePacketsStatus( RtpPacketHistory::StorageMode::kStoreAndCull, 10); + PlayoutDelayOracle playout_delay_oracle; FieldTrialBasedConfig field_trials; RTPSenderVideo::Config video_config; video_config.clock = &fake_clock_; video_config.rtp_sender = rtp_sender(); video_config.flexfec_sender = &flexfec_sender; + video_config.playout_delay_oracle = &playout_delay_oracle; video_config.field_trials = &field_trials; RTPSenderVideo rtp_sender_video(video_config); @@ -1322,11 +1330,13 @@ TEST_P(RtpSenderTestWithoutPacer, SendFlexfecPackets) { rtp_sender()->SetSequenceNumber(kSeqNum); + PlayoutDelayOracle playout_delay_oracle; FieldTrialBasedConfig field_trials; RTPSenderVideo::Config video_config; video_config.clock = &fake_clock_; video_config.rtp_sender = rtp_sender(); video_config.flexfec_sender = &flexfec_sender; + video_config.playout_delay_oracle = &playout_delay_oracle; video_config.field_trials = &field_trials; RTPSenderVideo rtp_sender_video(video_config); @@ -1594,11 +1604,13 @@ TEST_P(RtpSenderTest, FecOverheadRate) { rtp_sender()->SetSequenceNumber(kSeqNum); + PlayoutDelayOracle playout_delay_oracle; FieldTrialBasedConfig field_trials; RTPSenderVideo::Config video_config; video_config.clock = &fake_clock_; video_config.rtp_sender = rtp_sender(); video_config.flexfec_sender = &flexfec_sender; + video_config.playout_delay_oracle = &playout_delay_oracle; video_config.field_trials = &field_trials; RTPSenderVideo rtp_sender_video(video_config); // Parameters selected to generate a single FEC packet per media packet. @@ -1668,10 +1680,12 @@ TEST_P(RtpSenderTest, BitrateCallbacks) { config.retransmission_rate_limiter = &retransmission_rate_limiter_; rtp_sender_context_ = std::make_unique(config); + PlayoutDelayOracle playout_delay_oracle; FieldTrialBasedConfig field_trials; RTPSenderVideo::Config video_config; video_config.clock = &fake_clock_; video_config.rtp_sender = rtp_sender(); + video_config.playout_delay_oracle = &playout_delay_oracle; video_config.field_trials = &field_trials; RTPSenderVideo rtp_sender_video(video_config); const VideoCodecType kCodecType = VideoCodecType::kVideoCodecGeneric; @@ -1724,10 +1738,12 @@ TEST_P(RtpSenderTest, BitrateCallbacks) { TEST_P(RtpSenderTestWithoutPacer, StreamDataCountersCallbacks) { const uint8_t kPayloadType = 127; const VideoCodecType kCodecType = VideoCodecType::kVideoCodecGeneric; + PlayoutDelayOracle playout_delay_oracle; FieldTrialBasedConfig field_trials; RTPSenderVideo::Config video_config; video_config.clock = &fake_clock_; video_config.rtp_sender = rtp_sender(); + video_config.playout_delay_oracle = &playout_delay_oracle; video_config.field_trials = &field_trials; RTPSenderVideo rtp_sender_video(video_config); uint8_t payload[] = {47, 11, 32, 93, 89}; @@ -1779,10 +1795,12 @@ TEST_P(RtpSenderTestWithoutPacer, StreamDataCountersCallbacksUlpfec) { const uint8_t kUlpfecPayloadType = 97; const uint8_t kPayloadType = 127; const VideoCodecType kCodecType = VideoCodecType::kVideoCodecGeneric; + PlayoutDelayOracle playout_delay_oracle; FieldTrialBasedConfig field_trials; RTPSenderVideo::Config video_config; video_config.clock = &fake_clock_; video_config.rtp_sender = rtp_sender(); + video_config.playout_delay_oracle = &playout_delay_oracle; video_config.field_trials = &field_trials; video_config.red_payload_type = kRedPayloadType; video_config.ulpfec_payload_type = kUlpfecPayloadType; diff --git a/modules/rtp_rtcp/source/rtp_sender_video.cc b/modules/rtp_rtcp/source/rtp_sender_video.cc index 6c171c6d99..fc176c96cd 100644 --- a/modules/rtp_rtcp/source/rtp_sender_video.cc +++ b/modules/rtp_rtcp/source/rtp_sender_video.cc @@ -13,7 +13,6 @@ #include #include -#include #include #include #include @@ -241,10 +240,6 @@ const char* FrameTypeToString(VideoFrameType frame_type) { } #endif -bool IsNoopDelay(const PlayoutDelay& delay) { - return delay.min_ms == -1 && delay.max_ms == -1; -} - } // namespace RTPSenderVideo::RTPSenderVideo(Clock* clock, @@ -261,6 +256,7 @@ RTPSenderVideo::RTPSenderVideo(Clock* clock, config.clock = clock; config.rtp_sender = rtp_sender; config.flexfec_sender = flexfec_sender; + config.playout_delay_oracle = playout_delay_oracle; config.frame_encryptor = frame_encryptor; config.require_frame_encryption = require_frame_encryption; config.need_rtp_packet_infos = need_rtp_packet_infos; @@ -278,8 +274,7 @@ RTPSenderVideo::RTPSenderVideo(const Config& config) : (kRetransmitBaseLayer | kConditionallyRetransmitHigherLayers)), last_rotation_(kVideoRotation_0), transmit_color_space_next_frame_(false), - current_playout_delay_{-1, -1}, - playout_delay_pending_(false), + playout_delay_oracle_(config.playout_delay_oracle), rtp_sequence_number_map_(config.need_rtp_packet_infos ? std::make_unique( kRtpSequenceNumberMapMaxEntries) @@ -301,7 +296,9 @@ RTPSenderVideo::RTPSenderVideo(const Config& config) config.field_trials ->Lookup(kExcludeTransportSequenceNumberFromFecFieldTrial) .find("Enabled") == 0), - absolute_capture_time_sender_(config.clock) {} + absolute_capture_time_sender_(config.clock) { + RTC_DCHECK(playout_delay_oracle_); +} RTPSenderVideo::~RTPSenderVideo() {} @@ -524,16 +521,8 @@ bool RTPSenderVideo::SendVideo( video_header.codec == kVideoCodecH264 && video_header.frame_marking.temporal_id != kNoTemporalIdx; - MaybeUpdateCurrentPlayoutDelay(video_header); - if (video_header.frame_type == VideoFrameType::kVideoFrameKey && - !IsNoopDelay(current_playout_delay_)) { - // Force playout delay on key-frames, if set. - playout_delay_pending_ = true; - } const absl::optional playout_delay = - playout_delay_pending_ - ? absl::optional(current_playout_delay_) - : absl::nullopt; + playout_delay_oracle_->PlayoutDelayToSend(video_header.playout_delay); // According to // http://www.etsi.org/deliver/etsi_ts/126100_126199/126114/12.07.00_60/ @@ -662,15 +651,6 @@ bool RTPSenderVideo::SendVideo( MinimizeDescriptor(&video_header); } - if (video_header.frame_type == VideoFrameType::kVideoFrameKey || - (IsBaseLayer(video_header) && - !(video_header.generic.has_value() ? video_header.generic->discardable - : false))) { - // This frame has guaranteed delivery, no need to populate playout - // delay extensions until it changes again. - playout_delay_pending_ = false; - } - // TODO(benwright@webrtc.org) - Allocate enough to always encrypt inline. rtc::Buffer encrypted_video_payload; if (frame_encryptor_ != nullptr) { @@ -765,6 +745,10 @@ bool RTPSenderVideo::SendVideo( first_sequence_number = packet->SequenceNumber(); } + if (i == 0) { + playout_delay_oracle_->OnSentPacket(packet->SequenceNumber(), + playout_delay); + } // No FEC protection for upper temporal layers, if used. bool protect_packet = temporal_id == 0 || temporal_id == kNoTemporalIdx; @@ -958,52 +942,4 @@ bool RTPSenderVideo::UpdateConditionalRetransmit( return false; } -void RTPSenderVideo::MaybeUpdateCurrentPlayoutDelay( - const RTPVideoHeader& header) { - if (IsNoopDelay(header.playout_delay)) { - return; - } - - PlayoutDelay requested_delay = header.playout_delay; - - if (requested_delay.min_ms > PlayoutDelayLimits::kMaxMs || - requested_delay.max_ms > PlayoutDelayLimits::kMaxMs) { - RTC_DLOG(LS_ERROR) - << "Requested playout delay values out of range, ignored"; - return; - } - if (requested_delay.max_ms != -1 && - requested_delay.min_ms > requested_delay.max_ms) { - RTC_DLOG(LS_ERROR) << "Requested playout delay values out of order"; - return; - } - - if (!playout_delay_pending_) { - current_playout_delay_ = requested_delay; - playout_delay_pending_ = true; - return; - } - - if ((requested_delay.min_ms == -1 || - requested_delay.min_ms == current_playout_delay_.min_ms) && - (requested_delay.max_ms == -1 || - requested_delay.max_ms == current_playout_delay_.max_ms)) { - // No change, ignore. - return; - } - - if (requested_delay.min_ms == -1) { - RTC_DCHECK_GE(requested_delay.max_ms, 0); - requested_delay.min_ms = - std::min(current_playout_delay_.min_ms, requested_delay.max_ms); - } - if (requested_delay.max_ms == -1) { - requested_delay.max_ms = - std::max(current_playout_delay_.max_ms, requested_delay.min_ms); - } - - current_playout_delay_ = requested_delay; - playout_delay_pending_ = true; -} - } // namespace webrtc diff --git a/modules/rtp_rtcp/source/rtp_sender_video.h b/modules/rtp_rtcp/source/rtp_sender_video.h index 0f42d25a76..053877ef28 100644 --- a/modules/rtp_rtcp/source/rtp_sender_video.h +++ b/modules/rtp_rtcp/source/rtp_sender_video.h @@ -70,6 +70,7 @@ class RTPSenderVideo { Clock* clock = nullptr; RTPSender* rtp_sender = nullptr; FlexfecSender* flexfec_sender = nullptr; + PlayoutDelayOracle* playout_delay_oracle = nullptr; FrameEncryptorInterface* frame_encryptor = nullptr; bool require_frame_encryption = false; bool need_rtp_packet_infos = false; @@ -180,9 +181,6 @@ class RTPSenderVideo { int64_t expected_retransmission_time_ms) RTC_EXCLUSIVE_LOCKS_REQUIRED(stats_crit_); - void MaybeUpdateCurrentPlayoutDelay(const RTPVideoHeader& header) - RTC_EXCLUSIVE_LOCKS_REQUIRED(send_checker_); - RTPSender* const rtp_sender_; Clock* const clock_; @@ -197,11 +195,10 @@ class RTPSenderVideo { std::unique_ptr video_structure_ RTC_GUARDED_BY(send_checker_); - // Current target playout delay. - PlayoutDelay current_playout_delay_ RTC_GUARDED_BY(send_checker_); - // Flag indicating if we need to propagate |current_playout_delay_| in order - // to guarantee it gets delivered. - bool playout_delay_pending_; + // Tracks the current request for playout delay limits from application + // and decides whether the current RTP frame should include the playout + // delay extension on header. + PlayoutDelayOracle* const playout_delay_oracle_; // Should never be held when calling out of this class. rtc::CriticalSection crit_; diff --git a/modules/rtp_rtcp/source/rtp_sender_video_unittest.cc b/modules/rtp_rtcp/source/rtp_sender_video_unittest.cc index af235afe2a..867e05b60d 100644 --- a/modules/rtp_rtcp/source/rtp_sender_video_unittest.cc +++ b/modules/rtp_rtcp/source/rtp_sender_video_unittest.cc @@ -54,7 +54,6 @@ enum : int { // The first valid value is 1. kVideoRotationExtensionId, kVideoTimingExtensionId, kAbsoluteCaptureTimeExtensionId, - kPlayoutDelayExtensionId }; constexpr int kPayload = 100; @@ -88,8 +87,6 @@ class LoopbackTransportTest : public webrtc::Transport { kFrameMarkingExtensionId); receivers_extensions_.Register( kAbsoluteCaptureTimeExtensionId); - receivers_extensions_.Register( - kPlayoutDelayExtensionId); } bool SendRtp(const uint8_t* data, @@ -124,6 +121,7 @@ class TestRtpSenderVideo : public RTPSenderVideo { config.clock = clock; config.rtp_sender = rtp_sender; config.flexfec_sender = flexfec_sender; + config.playout_delay_oracle = &playout_delay_oracle_; config.field_trials = &field_trials; return config; }()) {} @@ -136,6 +134,7 @@ class TestRtpSenderVideo : public RTPSenderVideo { retransmission_settings, expected_retransmission_time_ms); } + PlayoutDelayOracle playout_delay_oracle_; }; class FieldTrials : public WebRtcKeyValueConfig { @@ -793,63 +792,6 @@ TEST_P(RtpSenderVideoTest, AbsoluteCaptureTime) { EXPECT_EQ(packets_with_abs_capture_time, 1); } -TEST_P(RtpSenderVideoTest, PopulatesPlayoutDelay) { - // Single packet frames. - constexpr size_t kPacketSize = 123; - uint8_t kFrame[kPacketSize]; - rtp_module_->RegisterRtpHeaderExtension(PlayoutDelayLimits::kUri, - kPlayoutDelayExtensionId); - const PlayoutDelay kExpectedDelay = {10, 20}; - - // Send initial key-frame without playout delay. - RTPVideoHeader hdr; - hdr.frame_type = VideoFrameType::kVideoFrameKey; - hdr.codec = VideoCodecType::kVideoCodecVP8; - auto& vp8_header = hdr.video_type_header.emplace(); - vp8_header.temporalIdx = 0; - - rtp_sender_video_.SendVideo(kPayload, kType, kTimestamp, 0, kFrame, nullptr, - hdr, kDefaultExpectedRetransmissionTimeMs); - EXPECT_FALSE( - transport_.last_sent_packet().HasExtension()); - - // Set playout delay on a discardable frame. - hdr.playout_delay = kExpectedDelay; - hdr.frame_type = VideoFrameType::kVideoFrameDelta; - vp8_header.temporalIdx = 1; - rtp_sender_video_.SendVideo(kPayload, kType, kTimestamp, 0, kFrame, nullptr, - hdr, kDefaultExpectedRetransmissionTimeMs); - PlayoutDelay received_delay = PlayoutDelay::Noop(); - ASSERT_TRUE(transport_.last_sent_packet().GetExtension( - &received_delay)); - EXPECT_EQ(received_delay, kExpectedDelay); - - // Set playout delay on a non-discardable frame, the extension should still - // be populated since dilvery wasn't guaranteed on the last one. - hdr.playout_delay = PlayoutDelay::Noop(); // Inidcates "no change". - vp8_header.temporalIdx = 0; - rtp_sender_video_.SendVideo(kPayload, kType, kTimestamp, 0, kFrame, nullptr, - hdr, kDefaultExpectedRetransmissionTimeMs); - ASSERT_TRUE(transport_.last_sent_packet().GetExtension( - &received_delay)); - EXPECT_EQ(received_delay, kExpectedDelay); - - // The next frame does not need the extensions since it's delivery has - // already been guaranteed. - rtp_sender_video_.SendVideo(kPayload, kType, kTimestamp, 0, kFrame, nullptr, - hdr, kDefaultExpectedRetransmissionTimeMs); - EXPECT_FALSE( - transport_.last_sent_packet().HasExtension()); - - // Insert key-frame, we need to refresh the state here. - hdr.frame_type = VideoFrameType::kVideoFrameKey; - rtp_sender_video_.SendVideo(kPayload, kType, kTimestamp, 0, kFrame, nullptr, - hdr, kDefaultExpectedRetransmissionTimeMs); - ASSERT_TRUE(transport_.last_sent_packet().GetExtension( - &received_delay)); - EXPECT_EQ(received_delay, kExpectedDelay); -} - INSTANTIATE_TEST_SUITE_P(WithAndWithoutOverhead, RtpSenderVideoTest, ::testing::Bool()); diff --git a/test/fuzzers/rtp_packet_fuzzer.cc b/test/fuzzers/rtp_packet_fuzzer.cc index 774be0871e..25fec2c094 100644 --- a/test/fuzzers/rtp_packet_fuzzer.cc +++ b/test/fuzzers/rtp_packet_fuzzer.cc @@ -99,11 +99,10 @@ void FuzzOneInput(const uint8_t* data, size_t size) { &feedback_request); break; } - case kRtpExtensionPlayoutDelay: { - PlayoutDelay playout = PlayoutDelay::Noop(); + case kRtpExtensionPlayoutDelay: + PlayoutDelay playout; packet.GetExtension(&playout); break; - } case kRtpExtensionVideoContentType: VideoContentType content_type; packet.GetExtension(&content_type);