Represent unset VideoPlayoutDelay with nullopt rather than special value

Remove support for setting one limit without another limit
because related rtp header extension doesn't support such values.

Start morphing VideoPlayouDelay into a class and stricter type: add accessors returning TimeDelta

Bug: webrtc:13756
Change-Id: If0dd02620528dc870b015beeff3a8103e04022ee
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/315921
Commit-Queue: Danil Chapovalov <danilchap@webrtc.org>
Reviewed-by: Philip Eliasson <philipel@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#40570}
This commit is contained in:
Danil Chapovalov 2023-08-16 11:42:52 +02:00 committed by WebRTC LUCI CQ
parent 0250b69fd4
commit c146b5f77b
13 changed files with 146 additions and 137 deletions

View File

@ -138,6 +138,22 @@ class RTC_EXPORT EncodedImage {
color_space_ = color_space;
}
absl::optional<VideoPlayoutDelay> PlayoutDelay() const {
if (playout_delay_.Valid()) {
return playout_delay_;
}
return absl::nullopt;
}
void SetPlayoutDelay(absl::optional<VideoPlayoutDelay> playout_delay) {
if (playout_delay.has_value()) {
playout_delay_ = *playout_delay;
} else {
playout_delay_.min_ms = -1;
playout_delay_.max_ms = -1;
}
}
// These methods along with the private member video_frame_tracking_id_ are
// meant for media quality testing purpose only.
absl::optional<uint16_t> VideoFrameTrackingId() const {
@ -207,6 +223,8 @@ class RTC_EXPORT EncodedImage {
// When an application indicates non-zero values here, it is taken as an
// indication that all future frames will be constrained with those limits
// until the application indicates a change again.
// TODO(bugs.webrc.org/13756): Make private and represent unset value with
// optional.
VideoPlayoutDelay playout_delay_;
struct Timing {

View File

@ -10,6 +10,8 @@
#include "api/video/video_timing.h"
#include <algorithm>
#include "api/array_view.h"
#include "api/units/time_delta.h"
#include "rtc_base/logging.h"
@ -98,4 +100,23 @@ std::string TimingFrameInfo::ToString() const {
return sb.str();
}
VideoPlayoutDelay::VideoPlayoutDelay(TimeDelta min, TimeDelta max)
: min_ms(std::clamp(min, TimeDelta::Zero(), kMax).ms()),
max_ms(std::clamp(max, min, kMax).ms()) {
if (!(TimeDelta::Zero() <= min && min <= max && max <= kMax)) {
RTC_LOG(LS_ERROR) << "Invalid video playout delay: [" << min << "," << max
<< "]. Clamped to [" << this->min() << "," << this->max()
<< "]";
}
}
bool VideoPlayoutDelay::Set(TimeDelta min, TimeDelta max) {
if (TimeDelta::Zero() <= min && min <= max && max <= kMax) {
min_ms = min.ms();
max_ms = max.ms();
return true;
}
return false;
}
} // namespace webrtc

View File

@ -106,16 +106,38 @@ struct TimingFrameInfo {
// Minimum and maximum playout delay values from capture to render.
// These are best effort values.
//
// A value < 0 indicates no change from previous valid value.
//
// min = max = 0 indicates that the receiver should try and render
// frame as soon as possible.
//
// min = x, max = y indicates that the receiver is free to adapt
// in the range (x, y) based on network jitter.
struct VideoPlayoutDelay {
// Maximum supported value for the delay limit.
static constexpr TimeDelta kMax = TimeDelta::Millis(10) * 0xFFF;
// Creates delay limits that indicates receiver should try to render frame
// as soon as possible.
static VideoPlayoutDelay Minimal() {
return VideoPlayoutDelay(TimeDelta::Zero(), TimeDelta::Zero());
}
VideoPlayoutDelay() = default;
VideoPlayoutDelay(int min_ms, int max_ms) : min_ms(min_ms), max_ms(max_ms) {}
VideoPlayoutDelay(TimeDelta min, TimeDelta max);
bool Set(TimeDelta min, TimeDelta max);
TimeDelta min() const { return TimeDelta::Millis(min_ms); }
TimeDelta max() const { return TimeDelta::Millis(max_ms); }
// TODO(bugs.webrtc.org/13756): Make members private and enforce the invariant
// in the constructors and setter.
bool Valid() const {
return TimeDelta::Zero() <= min() && min() <= max() && max() <= kMax;
}
// TODO(bugs.webrtc.org/13756): Make members private and change their type to
// TimeDelta.
int min_ms = -1;
int max_ms = -1;

View File

@ -98,10 +98,6 @@ bool IsBaseLayer(const RTPVideoHeader& video_header) {
return true;
}
bool IsNoopDelay(const VideoPlayoutDelay& delay) {
return delay.min_ms == -1 && delay.max_ms == -1;
}
absl::optional<VideoPlayoutDelay> LoadVideoPlayoutDelayOverride(
const FieldTrialsView* key_value_config) {
RTC_DCHECK(key_value_config);
@ -139,7 +135,6 @@ RTPSenderVideo::RTPSenderVideo(const Config& config)
last_rotation_(kVideoRotation_0),
transmit_color_space_next_frame_(false),
send_allocation_(SendVideoLayersAllocation::kDontSend),
current_playout_delay_{-1, -1},
playout_delay_pending_(false),
forced_playout_delay_(LoadVideoPlayoutDelayOverride(config.field_trials)),
red_payload_type_(config.red_payload_type),
@ -349,8 +344,8 @@ void RTPSenderVideo::AddRtpHeaderExtensions(const RTPVideoHeader& video_header,
packet->SetExtension<VideoTimingExtension>(video_header.video_timing);
// If transmitted, add to all packets; ack logic depends on this.
if (playout_delay_pending_) {
packet->SetExtension<PlayoutDelayLimits>(current_playout_delay_);
if (playout_delay_pending_ && current_playout_delay_.has_value()) {
packet->SetExtension<PlayoutDelayLimits>(*current_playout_delay_);
}
if (first_packet && video_header.absolute_capture_time.has_value()) {
@ -494,7 +489,7 @@ bool RTPSenderVideo::SendVideo(int payload_type,
MaybeUpdateCurrentPlayoutDelay(video_header);
if (video_header.frame_type == VideoFrameType::kVideoFrameKey) {
if (!IsNoopDelay(current_playout_delay_)) {
if (current_playout_delay_.has_value()) {
// Force playout delay on key-frames, if set.
playout_delay_pending_ = true;
}
@ -859,49 +854,19 @@ bool RTPSenderVideo::UpdateConditionalRetransmit(
void RTPSenderVideo::MaybeUpdateCurrentPlayoutDelay(
const RTPVideoHeader& header) {
VideoPlayoutDelay requested_delay =
forced_playout_delay_.value_or(header.playout_delay);
absl::optional<VideoPlayoutDelay> requested_delay =
forced_playout_delay_.has_value() ? forced_playout_delay_
: header.playout_delay;
if (IsNoopDelay(requested_delay)) {
if (!requested_delay.has_value()) {
return;
}
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) {
if (!requested_delay->Valid()) {
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;
}

View File

@ -211,7 +211,8 @@ class RTPSenderVideo : public RTPVideoFrameSenderInterface {
RTC_GUARDED_BY(send_checker_);
// Current target playout delay.
VideoPlayoutDelay current_playout_delay_ RTC_GUARDED_BY(send_checker_);
absl::optional<VideoPlayoutDelay> current_playout_delay_
RTC_GUARDED_BY(send_checker_);
// Flag indicating if we need to send `current_playout_delay_` in order
// to guarantee it gets delivered.
bool playout_delay_pending_;

View File

@ -82,7 +82,7 @@ struct RTPVideoHeader {
uint8_t simulcastIdx = 0;
VideoCodecType codec = VideoCodecType::kVideoCodecGeneric;
VideoPlayoutDelay playout_delay;
absl::optional<VideoPlayoutDelay> playout_delay;
VideoSendTiming video_timing;
absl::optional<ColorSpace> color_space;
// This field is meant for media quality testing purpose only. When enabled it

View File

@ -173,7 +173,7 @@ VCMFrameBufferEnum VCMFrameBuffer::InsertPacket(const VCMPacket& packet,
}
if (packet.is_first_packet_in_frame()) {
playout_delay_ = packet.video_header.playout_delay;
SetPlayoutDelay(packet.video_header.playout_delay);
}
if (_sessionInfo.complete()) {

View File

@ -80,27 +80,21 @@ int32_t VCMReceiver::InsertPacket(const VCMPacket& packet) {
VCMEncodedFrame* VCMReceiver::FrameForDecoding(uint16_t max_wait_time_ms,
bool prefer_late_decoding) {
const int64_t start_time_ms = clock_->TimeInMilliseconds();
uint32_t frame_timestamp = 0;
int min_playout_delay_ms = -1;
int max_playout_delay_ms = -1;
int64_t render_time_ms = 0;
// Exhaust wait time to get a complete frame for decoding.
VCMEncodedFrame* found_frame =
jitter_buffer_.NextCompleteFrame(max_wait_time_ms);
if (found_frame) {
frame_timestamp = found_frame->Timestamp();
min_playout_delay_ms = found_frame->EncodedImage().playout_delay_.min_ms;
max_playout_delay_ms = found_frame->EncodedImage().playout_delay_.max_ms;
} else {
if (found_frame == nullptr) {
return nullptr;
}
uint32_t frame_timestamp = found_frame->Timestamp();
if (min_playout_delay_ms >= 0)
timing_->set_min_playout_delay(TimeDelta::Millis(min_playout_delay_ms));
if (max_playout_delay_ms >= 0)
timing_->set_max_playout_delay(TimeDelta::Millis(max_playout_delay_ms));
if (absl::optional<VideoPlayoutDelay> playout_delay =
found_frame->EncodedImage().PlayoutDelay()) {
timing_->set_min_playout_delay(playout_delay->min());
timing_->set_max_playout_delay(playout_delay->max());
}
// We have a frame - Set timing and render timestamp.
timing_->SetJitterDelay(

View File

@ -34,12 +34,6 @@ class RTC_EXPORT VCMEncodedFrame : public EncodedImage {
_renderTimeMs = renderTimeMs;
}
VideoPlayoutDelay PlayoutDelay() const { return playout_delay_; }
void SetPlayoutDelay(VideoPlayoutDelay playout_delay) {
playout_delay_ = playout_delay;
}
/**
* Get the encoded image
*/

View File

@ -574,10 +574,13 @@ void RtpVideoStreamReceiver2::OnReceivedPayloadData(
&video_header.content_type);
rtp_packet.GetExtension<VideoTimingExtension>(&video_header.video_timing);
if (forced_playout_delay_max_ms_ && forced_playout_delay_min_ms_) {
video_header.playout_delay.max_ms = *forced_playout_delay_max_ms_;
video_header.playout_delay.min_ms = *forced_playout_delay_min_ms_;
if (!video_header.playout_delay.emplace().Set(
TimeDelta::Millis(*forced_playout_delay_min_ms_),
TimeDelta::Millis(*forced_playout_delay_max_ms_))) {
video_header.playout_delay = absl::nullopt;
}
} else {
rtp_packet.GetExtension<PlayoutDelayLimits>(&video_header.playout_delay);
video_header.playout_delay = rtp_packet.GetExtension<PlayoutDelayLimits>();
}
ParseGenericDependenciesResult generic_descriptor_state =

View File

@ -690,13 +690,10 @@ void VideoReceiveStream2::RequestKeyFrame(Timestamp now) {
void VideoReceiveStream2::OnCompleteFrame(std::unique_ptr<EncodedFrame> frame) {
RTC_DCHECK_RUN_ON(&worker_sequence_checker_);
const VideoPlayoutDelay& playout_delay = frame->EncodedImage().playout_delay_;
if (playout_delay.min_ms >= 0) {
frame_minimum_playout_delay_ = TimeDelta::Millis(playout_delay.min_ms);
UpdatePlayoutDelays();
}
if (playout_delay.max_ms >= 0) {
frame_maximum_playout_delay_ = TimeDelta::Millis(playout_delay.max_ms);
if (absl::optional<VideoPlayoutDelay> playout_delay =
frame->EncodedImage().PlayoutDelay()) {
frame_minimum_playout_delay_ = playout_delay->min();
frame_maximum_playout_delay_ = playout_delay->max();
UpdatePlayoutDelays();
}

View File

@ -316,8 +316,11 @@ TEST_P(VideoReceiveStream2Test, CreateFrameFromH264FmtpSpropAndIdr) {
TEST_P(VideoReceiveStream2Test, PlayoutDelay) {
const VideoPlayoutDelay kPlayoutDelayMs = {123, 321};
std::unique_ptr<test::FakeEncodedFrame> test_frame =
test::FakeFrameBuilder().Id(0).AsLast().Build();
test_frame->SetPlayoutDelay(kPlayoutDelayMs);
test::FakeFrameBuilder()
.Id(0)
.PlayoutDelay(kPlayoutDelayMs)
.AsLast()
.Build();
video_receive_stream_->OnCompleteFrame(std::move(test_frame));
auto timings = timing_->GetTimings();
@ -347,42 +350,6 @@ TEST_P(VideoReceiveStream2Test, PlayoutDelay) {
EXPECT_EQ(123, timings.min_playout_delay.ms());
}
TEST_P(VideoReceiveStream2Test, PlayoutDelayPreservesDefaultMaxValue) {
const TimeDelta default_max_playout_latency =
timing_->GetTimings().max_playout_delay;
const VideoPlayoutDelay kPlayoutDelayMs = {123, -1};
std::unique_ptr<test::FakeEncodedFrame> test_frame =
test::FakeFrameBuilder().Id(0).AsLast().Build();
test_frame->SetPlayoutDelay(kPlayoutDelayMs);
video_receive_stream_->OnCompleteFrame(std::move(test_frame));
// Ensure that -1 preserves default maximum value from `timing_`.
auto timings = timing_->GetTimings();
EXPECT_EQ(kPlayoutDelayMs.min_ms, timings.min_playout_delay.ms());
EXPECT_NE(kPlayoutDelayMs.max_ms, timings.max_playout_delay.ms());
EXPECT_EQ(default_max_playout_latency, timings.max_playout_delay);
}
TEST_P(VideoReceiveStream2Test, PlayoutDelayPreservesDefaultMinValue) {
const TimeDelta default_min_playout_latency =
timing_->GetTimings().min_playout_delay;
const VideoPlayoutDelay kPlayoutDelayMs = {-1, 321};
std::unique_ptr<test::FakeEncodedFrame> test_frame =
test::FakeFrameBuilder().Id(0).AsLast().Build();
test_frame->SetPlayoutDelay(kPlayoutDelayMs);
video_receive_stream_->OnCompleteFrame(std::move(test_frame));
// Ensure that -1 preserves default minimum value from `timing_`.
auto timings = timing_->GetTimings();
EXPECT_NE(kPlayoutDelayMs.min_ms, timings.min_playout_delay.ms());
EXPECT_EQ(kPlayoutDelayMs.max_ms, timings.max_playout_delay.ms());
EXPECT_EQ(default_min_playout_latency, timings.min_playout_delay);
}
TEST_P(VideoReceiveStream2Test, RenderParametersSetToDefaultValues) {
// Default render parameters.
const VideoFrame::RenderParameters kDefaultRenderParameters;
@ -394,16 +361,21 @@ TEST_P(VideoReceiveStream2Test, RenderParametersSetToDefaultValues) {
}
TEST_P(VideoReceiveStream2Test, UseLowLatencyRenderingSetFromPlayoutDelay) {
// use_low_latency_rendering set if playout delay set to min=0, max<=500 ms.
std::unique_ptr<test::FakeEncodedFrame> test_frame0 =
test::FakeFrameBuilder().Id(0).AsLast().Build();
test_frame0->SetPlayoutDelay({/*min_ms=*/0, /*max_ms=*/0});
test::FakeFrameBuilder()
.Id(0)
.PlayoutDelay(VideoPlayoutDelay::Minimal())
.AsLast()
.Build();
video_receive_stream_->OnCompleteFrame(std::move(test_frame0));
EXPECT_TRUE(timing_->RenderParameters().use_low_latency_rendering);
std::unique_ptr<test::FakeEncodedFrame> test_frame1 =
test::FakeFrameBuilder().Id(1).AsLast().Build();
test_frame1->SetPlayoutDelay({/*min_ms=*/0, /*max_ms=*/500});
test::FakeFrameBuilder()
.Id(1)
.PlayoutDelay({/*min_ms=*/0, /*max_ms=*/500})
.AsLast()
.Build();
video_receive_stream_->OnCompleteFrame(std::move(test_frame1));
EXPECT_TRUE(timing_->RenderParameters().use_low_latency_rendering);
}
@ -432,9 +404,9 @@ TEST_P(VideoReceiveStream2Test, MaxCompositionDelaySetFromMaxPlayoutDelay) {
.Id(1)
.Time(RtpTimestampForFrame(1))
.ReceivedTime(ReceiveTimeForFrame(1))
.PlayoutDelay({0, 0})
.AsLast()
.Build();
test_frame1->SetPlayoutDelay({0, 0});
video_receive_stream_->OnCompleteFrame(std::move(test_frame1));
EXPECT_THAT(timing_->RenderParameters().max_composition_delay_in_frames,
Eq(absl::nullopt));
@ -446,9 +418,9 @@ TEST_P(VideoReceiveStream2Test, MaxCompositionDelaySetFromMaxPlayoutDelay) {
.Id(2)
.Time(RtpTimestampForFrame(2))
.ReceivedTime(ReceiveTimeForFrame(2))
.PlayoutDelay({10, 30})
.AsLast()
.Build();
test_frame2->SetPlayoutDelay({10, 30});
video_receive_stream_->OnCompleteFrame(std::move(test_frame2));
EXPECT_THAT(timing_->RenderParameters().max_composition_delay_in_frames,
Eq(absl::nullopt));
@ -462,9 +434,9 @@ TEST_P(VideoReceiveStream2Test, MaxCompositionDelaySetFromMaxPlayoutDelay) {
.Id(3)
.Time(RtpTimestampForFrame(3))
.ReceivedTime(ReceiveTimeForFrame(3))
.PlayoutDelay({0, 50})
.AsLast()
.Build();
test_frame3->SetPlayoutDelay({0, 50});
video_receive_stream_->OnCompleteFrame(std::move(test_frame3));
EXPECT_THAT(timing_->RenderParameters().max_composition_delay_in_frames,
Optional(kExpectedMaxCompositionDelayInFrames));

View File

@ -57,7 +57,6 @@ namespace {
constexpr size_t kFrameSize = 10;
constexpr uint32_t kFps30Rtp = 90000 / 30;
constexpr TimeDelta kFps30Delay = 1 / Frequency::Hertz(30);
const VideoPlayoutDelay kZeroPlayoutDelay = {0, 0};
constexpr Timestamp kClockStart = Timestamp::Millis(1000);
auto TimedOut() {
@ -824,17 +823,25 @@ TEST_P(LowLatencyVideoStreamBufferControllerTest,
StartNextDecodeForceKeyframe();
timing_.set_min_playout_delay(TimeDelta::Zero());
timing_.set_max_playout_delay(TimeDelta::Millis(10));
auto frame = test::FakeFrameBuilder().Id(0).Time(0).AsLast().Build();
// Playout delay of 0 implies low-latency rendering.
frame->SetPlayoutDelay({0, 10});
auto frame = test::FakeFrameBuilder()
.Id(0)
.Time(0)
.PlayoutDelay({0, 10})
.AsLast()
.Build();
buffer_->InsertFrame(std::move(frame));
EXPECT_THAT(WaitForFrameOrTimeout(TimeDelta::Zero()), Frame(test::WithId(0)));
// Delta frame would normally wait here, but should decode at the pacing rate
// in low-latency mode.
StartNextDecode();
frame = test::FakeFrameBuilder().Id(1).Time(kFps30Rtp).AsLast().Build();
frame->SetPlayoutDelay({0, 10});
frame = test::FakeFrameBuilder()
.Id(1)
.Time(kFps30Rtp)
.PlayoutDelay({0, 10})
.AsLast()
.Build();
buffer_->InsertFrame(std::move(frame));
// Pacing is set to 16ms in the field trial so we should not decode yet.
EXPECT_THAT(WaitForFrameOrTimeout(TimeDelta::Zero()), Eq(absl::nullopt));
@ -847,17 +854,24 @@ TEST_P(LowLatencyVideoStreamBufferControllerTest, ZeroPlayoutDelayFullQueue) {
StartNextDecodeForceKeyframe();
timing_.set_min_playout_delay(TimeDelta::Zero());
timing_.set_max_playout_delay(TimeDelta::Millis(10));
auto frame = test::FakeFrameBuilder().Id(0).Time(0).AsLast().Build();
auto frame = test::FakeFrameBuilder()
.Id(0)
.Time(0)
.PlayoutDelay({0, 10})
.AsLast()
.Build();
// Playout delay of 0 implies low-latency rendering.
frame->SetPlayoutDelay({0, 10});
buffer_->InsertFrame(std::move(frame));
EXPECT_THAT(WaitForFrameOrTimeout(TimeDelta::Zero()), Frame(test::WithId(0)));
// Queue up 5 frames (configured max queue size for 0-playout delay pacing).
for (int id = 1; id <= 6; ++id) {
frame =
test::FakeFrameBuilder().Id(id).Time(kFps30Rtp * id).AsLast().Build();
frame->SetPlayoutDelay({0, 10});
frame = test::FakeFrameBuilder()
.Id(id)
.Time(kFps30Rtp * id)
.PlayoutDelay({0, 10})
.AsLast()
.Build();
buffer_->InsertFrame(std::move(frame));
}
@ -873,17 +887,25 @@ TEST_P(LowLatencyVideoStreamBufferControllerTest,
StartNextDecodeForceKeyframe();
timing_.set_min_playout_delay(TimeDelta::Zero());
timing_.set_max_playout_delay(TimeDelta::Zero());
auto frame = test::FakeFrameBuilder().Id(0).Time(0).AsLast().Build();
// Playout delay of 0 implies low-latency rendering.
frame->SetPlayoutDelay({0, 0});
auto frame = test::FakeFrameBuilder()
.Id(0)
.Time(0)
.PlayoutDelay(VideoPlayoutDelay::Minimal())
.AsLast()
.Build();
buffer_->InsertFrame(std::move(frame));
EXPECT_THAT(WaitForFrameOrTimeout(TimeDelta::Zero()), Frame(test::WithId(0)));
// Delta frame would normally wait here, but should decode at the pacing rate
// in low-latency mode.
StartNextDecode();
frame = test::FakeFrameBuilder().Id(1).Time(kFps30Rtp).AsLast().Build();
frame->SetPlayoutDelay({0, 0});
frame = test::FakeFrameBuilder()
.Id(1)
.Time(kFps30Rtp)
.PlayoutDelay(VideoPlayoutDelay::Minimal())
.AsLast()
.Build();
buffer_->InsertFrame(std::move(frame));
// The min/max=0 version of low-latency rendering will result in a large
// negative decode wait time, so the frame should be ready right away.