Revert "Remove PlayoutDelayOracle and make RtpSenderVideo guarantee delivery"
This reverts commit 4f68f5398d7fa3d47c449e99893c9bea07bf7ca2.
Reason for revert: Breaks downstream project
Original change's description:
> Remove PlayoutDelayOracle and make RtpSenderVideo guarantee delivery
>
> The PlayoutDelayOracle was responsible for making sure the PlayoutDelay
> header extension was successfully propagated to the receiving side. Once
> it was determined that the receiver had received a frame with the new
> delay tag, it's no longer necessary to propagate.
>
> The issue with this implementation is that it is based on max
> extended sequence number reported via RTCP, which makes it often slow
> to react, could theoretically fail to produce desired outcome (max
> received > X does not guarantee X was fully received and decoded), and
> added a lot of code complexity.
>
> The guarantee of delivery can in fact be accomplished more reliably and
> with less code by making sure to tag each frame until an undiscardable
> frame is sent.
>
> This allows containing the logic fully within RTPSenderVideo.
>
> Bug: webrtc:11340
> Change-Id: I2d1d2b6b67f4f07b8b33336f8fcfcde724243eef
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/168221
> Reviewed-by: Stefan Holmer <stefan@webrtc.org>
> Reviewed-by: Sebastian Jansson <srte@webrtc.org>
> Commit-Queue: Erik Språng <sprang@webrtc.org>
> Cr-Commit-Position: refs/heads/master@{#30473}
TBR=sprang@webrtc.org,stefan@webrtc.org,srte@webrtc.org
Change-Id: Ide922e680ae36bb69b95e58002482cf5ed57e254
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: webrtc:11340
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/168304
Reviewed-by: Erik Språng <sprang@webrtc.org>
Commit-Queue: Erik Språng <sprang@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#30475}
diff --git a/call/rtp_video_sender.cc b/call/rtp_video_sender.cc
index 3ae0794..413171f 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_internal_rtp_video_sender {
-RtpStreamSender::RtpStreamSender(std::unique_ptr<RtpRtcp> rtp_rtcp,
- std::unique_ptr<RTPSenderVideo> sender_video)
- : rtp_rtcp(std::move(rtp_rtcp)), sender_video(std::move(sender_video)) {}
+RtpStreamSender::RtpStreamSender(
+ std::unique_ptr<PlayoutDelayOracle> playout_delay_oracle,
+ std::unique_ptr<RtpRtcp> rtp_rtcp,
+ std::unique_ptr<RTPSenderVideo> 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 @@
configuration.local_media_ssrc) !=
flexfec_protected_ssrcs.end();
configuration.flexfec_sender = enable_flexfec ? flexfec_sender : nullptr;
+ auto playout_delay_oracle = std::make_unique<PlayoutDelayOracle>();
+ 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 @@
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 @@
video_config.ulpfec_payload_type = rtp_config.ulpfec.ulpfec_payload_type;
}
auto sender_video = std::make_unique<RTPSenderVideo>(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 620c975..eb7e431 100644
--- a/call/rtp_video_sender.h
+++ b/call/rtp_video_sender.h
@@ -50,7 +50,8 @@
// RTP state for a single simulcast stream. Internal to the implementation of
// RtpVideoSender.
struct RtpStreamSender {
- RtpStreamSender(std::unique_ptr<RtpRtcp> rtp_rtcp,
+ RtpStreamSender(std::unique_ptr<PlayoutDelayOracle> playout_delay_oracle,
+ std::unique_ptr<RtpRtcp> rtp_rtcp,
std::unique_ptr<RTPSenderVideo> sender_video);
~RtpStreamSender();
@@ -58,6 +59,7 @@
RtpStreamSender& operator=(RtpStreamSender&&) = default;
// Note: Needs pointer stability.
+ std::unique_ptr<PlayoutDelayOracle> playout_delay_oracle;
std::unique_ptr<RtpRtcp> rtp_rtcp;
std::unique_ptr<RTPSenderVideo> sender_video;
};
diff --git a/common_types.h b/common_types.h
index dedcbd5..aadda4f 100644
--- a/common_types.h
+++ b/common_types.h
@@ -89,16 +89,8 @@
// 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 b8dd23e..099c066 100644
--- a/modules/rtp_rtcp/BUILD.gn
+++ b/modules/rtp_rtcp/BUILD.gn
@@ -156,6 +156,7 @@
"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 @@
"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 fbb3bb3..b3cd8f6 100644
--- a/modules/rtp_rtcp/include/rtp_rtcp.h
+++ b/modules/rtp_rtcp/include/rtp_rtcp.h
@@ -101,6 +101,7 @@
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 bdee7b4..8cd402e 100644
--- a/modules/rtp_rtcp/include/rtp_rtcp_defines.h
+++ b/modules/rtp_rtcp/include/rtp_rtcp_defines.h
@@ -392,6 +392,19 @@
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 55e1e44..17601dd 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 @@
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<RTPSenderVideo>(video_config);
rtp_rtcp_module_->SetRTCPStatus(RtcpMode::kCompound);
@@ -225,6 +227,7 @@
std::unique_ptr<ReceiveStatistics> receive_statistics_;
std::unique_ptr<RtpRtcp> rtp_rtcp_module_;
+ PlayoutDelayOracle playout_delay_oracle_;
std::unique_ptr<RTPSenderVideo> rtp_sender_video_;
RtxLoopBackTransport transport_;
const std::map<int, int> 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 0000000..f234759
--- /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 <algorithm>
+
+#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<PlayoutDelay> 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<PlayoutDelay> 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 04465e3..6451be4 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 <stdint.h>
+
+#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<PlayoutDelay> PlayoutDelayToSend(
+ PlayoutDelay requested_delay) const;
+
+ void OnSentPacket(uint16_t sequence_number,
+ absl::optional<PlayoutDelay> 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<int64_t> 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 0000000..3857e9b
--- /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<PlayoutDelay> 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 dfbac29..987ae0e 100644
--- a/modules/rtp_rtcp/source/rtp_rtcp_impl.cc
+++ b/modules/rtp_rtcp/source/rtp_rtcp_impl.cc
@@ -68,6 +68,7 @@
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::OnReceivedRtcpReportBlocks(
const ReportBlockList& report_blocks) {
- if (rtp_sender_) {
+ if (ack_observer_) {
uint32_t ssrc = SSRC();
absl::optional<uint32_t> rtx_ssrc;
if (rtp_sender_->packet_generator.RtxStatus() != kRtxOff) {
@@ -746,6 +747,8 @@
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 c03683f..976653a 100644
--- a/modules/rtp_rtcp/source/rtp_rtcp_impl.h
+++ b/modules/rtp_rtcp/source/rtp_rtcp_impl.h
@@ -340,6 +340,8 @@
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 5e4cce9..0b681cf 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 @@
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<RTPSenderVideo>(video_config);
@@ -199,6 +201,7 @@
SimulatedClock clock_;
RtpRtcpModule sender_;
+ PlayoutDelayOracle playout_delay_oracle_;
std::unique_ptr<RTPSenderVideo> 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 458d3e7..5ca4e70 100644
--- a/modules/rtp_rtcp/source/rtp_sender_unittest.cc
+++ b/modules/rtp_rtcp/source/rtp_sender_unittest.cc
@@ -649,10 +649,12 @@
config.event_log = &mock_rtc_event_log_;
rtp_sender_context_ = std::make_unique<RtpSenderContext>(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(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 @@
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 @@
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 @@
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 @@
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 @@
config.retransmission_rate_limiter = &retransmission_rate_limiter_;
rtp_sender_context_ = std::make_unique<RtpSenderContext>(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(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 @@
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 6c171c6..fc176c9 100644
--- a/modules/rtp_rtcp/source/rtp_sender_video.cc
+++ b/modules/rtp_rtcp/source/rtp_sender_video.cc
@@ -13,7 +13,6 @@
#include <stdlib.h>
#include <string.h>
-#include <algorithm>
#include <limits>
#include <memory>
#include <string>
@@ -241,10 +240,6 @@
}
#endif
-bool IsNoopDelay(const PlayoutDelay& delay) {
- return delay.min_ms == -1 && delay.max_ms == -1;
-}
-
} // namespace
RTPSenderVideo::RTPSenderVideo(Clock* clock,
@@ -261,6 +256,7 @@
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 @@
: (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<RtpSequenceNumberMap>(
kRtpSequenceNumberMapMaxEntries)
@@ -301,7 +296,9 @@
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 @@
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<PlayoutDelay> playout_delay =
- playout_delay_pending_
- ? absl::optional<PlayoutDelay>(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 @@
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 @@
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 @@
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 0f42d25..053877e 100644
--- a/modules/rtp_rtcp/source/rtp_sender_video.h
+++ b/modules/rtp_rtcp/source/rtp_sender_video.h
@@ -70,6 +70,7 @@
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 @@
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 @@
std::unique_ptr<FrameDependencyStructure> 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 af235af..867e05b 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 @@
kVideoRotationExtensionId,
kVideoTimingExtensionId,
kAbsoluteCaptureTimeExtensionId,
- kPlayoutDelayExtensionId
};
constexpr int kPayload = 100;
@@ -88,8 +87,6 @@
kFrameMarkingExtensionId);
receivers_extensions_.Register<AbsoluteCaptureTimeExtension>(
kAbsoluteCaptureTimeExtensionId);
- receivers_extensions_.Register<PlayoutDelayLimits>(
- kPlayoutDelayExtensionId);
}
bool SendRtp(const uint8_t* data,
@@ -124,6 +121,7 @@
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 @@
retransmission_settings,
expected_retransmission_time_ms);
}
+ PlayoutDelayOracle playout_delay_oracle_;
};
class FieldTrials : public WebRtcKeyValueConfig {
@@ -793,63 +792,6 @@
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<RTPVideoHeaderVP8>();
- vp8_header.temporalIdx = 0;
-
- rtp_sender_video_.SendVideo(kPayload, kType, kTimestamp, 0, kFrame, nullptr,
- hdr, kDefaultExpectedRetransmissionTimeMs);
- EXPECT_FALSE(
- transport_.last_sent_packet().HasExtension<PlayoutDelayLimits>());
-
- // 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<PlayoutDelayLimits>(
- &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<PlayoutDelayLimits>(
- &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<PlayoutDelayLimits>());
-
- // 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<PlayoutDelayLimits>(
- &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 774be08..25fec2c 100644
--- a/test/fuzzers/rtp_packet_fuzzer.cc
+++ b/test/fuzzers/rtp_packet_fuzzer.cc
@@ -99,11 +99,10 @@
&feedback_request);
break;
}
- case kRtpExtensionPlayoutDelay: {
- PlayoutDelay playout = PlayoutDelay::Noop();
+ case kRtpExtensionPlayoutDelay:
+ PlayoutDelay playout;
packet.GetExtension<PlayoutDelayLimits>(&playout);
break;
- }
case kRtpExtensionVideoContentType:
VideoContentType content_type;
packet.GetExtension<VideoContentTypeExtension>(&content_type);