mollyim/webrtc
1
0
Fork 0
mirror of https://github.com/mollyim/webrtc.git synced 2025-05-12 21:30:45 +01:00
Code Issues Projects Releases Packages Wiki Activity Actions

In VideoPlayoutDelay delete access to integer representation of min/max values

Browse source 
Bug: webrtc:13756
Change-Id: I1a81c25e5e3fab68a44e94a5ab93e8184c824683
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/316864
Commit-Queue: Danil Chapovalov <danilchap@webrtc.org>
Reviewed-by: Philip Eliasson <philipel@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#40612}
This commit is contained in:
Danil Chapovalov 2023-08-23 13:16:22 +02:00 committed by WebRTC LUCI CQ
parent 093a939572
commit 7084e1b6d9
11 changed files with 77 additions and 70 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

8
api/video/video_timing.cc
View file

@ -101,8 +101,8 @@ std::string TimingFrameInfo::ToString() const {
}
VideoPlayoutDelay::VideoPlayoutDelay(TimeDelta min, TimeDelta max)
: min_ms(std::clamp(min, TimeDelta::Zero(), kMax).ms()),
max_ms(std::clamp(max, min, kMax).ms()) {
: min_(std::clamp(min, TimeDelta::Zero(), kMax)),
max_(std::clamp(max, min_, kMax)) {
if (!(TimeDelta::Zero() <= min && min <= max && max <= kMax)) {
RTC_LOG(LS_ERROR) << "Invalid video playout delay: [" << min << "," << max
<< "]. Clamped to [" << this->min() << "," << this->max()
@ -112,8 +112,8 @@ VideoPlayoutDelay::VideoPlayoutDelay(TimeDelta min, TimeDelta max)
bool VideoPlayoutDelay::Set(TimeDelta min, TimeDelta max) {
if (TimeDelta::Zero() <= min && min <= max && max <= kMax) {
min_ms = min.ms();
max_ms = max.ms();
min_ = min;
max_ = max;
return true;
}
return false;

30
api/video/video_timing.h
View file

@ -112,7 +112,9 @@ struct RTC_EXPORT TimingFrameInfo {
//
// min = x, max = y indicates that the receiver is free to adapt
// in the range (x, y) based on network jitter.
struct RTC_EXPORT VideoPlayoutDelay {
// This class ensures invariant 0 <= min <= max <= kMax.
class RTC_EXPORT VideoPlayoutDelay {
public:
// Maximum supported value for the delay limit.
static constexpr TimeDelta kMax = TimeDelta::Millis(10) * 0xFFF;
@ -122,29 +124,25 @@ struct RTC_EXPORT VideoPlayoutDelay {
return VideoPlayoutDelay(TimeDelta::Zero(), TimeDelta::Zero());
}
// Creates valid, but unspecified limits.
VideoPlayoutDelay() = default;
VideoPlayoutDelay(int min_ms, int max_ms) : min_ms(min_ms), max_ms(max_ms) {}
VideoPlayoutDelay(const VideoPlayoutDelay&) = default;
VideoPlayoutDelay& operator=(const VideoPlayoutDelay&) = default;
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); }
TimeDelta min() const { return min_; }
TimeDelta max() const { return max_; }
// 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;
friend bool operator==(const VideoPlayoutDelay& lhs,
const VideoPlayoutDelay& rhs) {
return lhs.min_ == rhs.min_ && lhs.max_ == rhs.max_;
}
// TODO(bugs.webrtc.org/13756): Make members private and change their type to
// TimeDelta.
int min_ms = -1;
int max_ms = -1;
bool operator==(const VideoPlayoutDelay& rhs) const {
return min_ms == rhs.min_ms && max_ms == rhs.max_ms;
}
private:
TimeDelta min_ = TimeDelta::Zero();
TimeDelta max_ = kMax;
};
} // namespace webrtc

24
modules/rtp_rtcp/source/rtp_header_extensions.cc
View file

@ -400,22 +400,24 @@ bool PlayoutDelayLimits::Parse(rtc::ArrayView<const uint8_t> data,
uint32_t raw = ByteReader<uint32_t, 3>::ReadBigEndian(data.data());
uint16_t min_raw = (raw >> 12);
uint16_t max_raw = (raw & 0xfff);
if (min_raw > max_raw)
return false;
playout_delay->min_ms = min_raw * kGranularityMs;
playout_delay->max_ms = max_raw * kGranularityMs;
return true;
return playout_delay->Set(min_raw * kGranularity, max_raw * kGranularity);
}
bool PlayoutDelayLimits::Write(rtc::ArrayView<uint8_t> data,
const VideoPlayoutDelay& playout_delay) {
RTC_DCHECK_EQ(data.size(), 3);
RTC_DCHECK_LE(0, playout_delay.min_ms);
RTC_DCHECK_LE(playout_delay.min_ms, playout_delay.max_ms);
RTC_DCHECK_LE(playout_delay.max_ms, kMaxMs);
// Convert MS to value to be sent on extension header.
uint32_t min_delay = playout_delay.min_ms / kGranularityMs;
uint32_t max_delay = playout_delay.max_ms / kGranularityMs;
// Convert TimeDelta to value to be sent on extension header.
auto idiv = [](TimeDelta num, TimeDelta den) { return num.us() / den.us(); };
int64_t min_delay = idiv(playout_delay.min(), kGranularity);
int64_t max_delay = idiv(playout_delay.max(), kGranularity);
// Double check min/max boundaries guaranteed by the `VideoPlayouDelay` type.
RTC_DCHECK_GE(min_delay, 0);
RTC_DCHECK_LT(min_delay, 1 << 12);
RTC_DCHECK_GE(max_delay, 0);
RTC_DCHECK_LT(max_delay, 1 << 12);
ByteWriter<uint32_t, 3>::WriteBigEndian(data.data(),
(min_delay << 12) | max_delay);
return true;

4
modules/rtp_rtcp/source/rtp_header_extensions.h
View file

@ -197,9 +197,9 @@ class PlayoutDelayLimits {
// Playout delay in milliseconds. A playout delay limit (min or max)
// has 12 bits allocated. This allows a range of 0-4095 values which
// translates to a range of 0-40950 in milliseconds.
static constexpr int kGranularityMs = 10;
static constexpr TimeDelta kGranularity = TimeDelta::Millis(10);
// Maximum playout delay value in milliseconds.
static constexpr int kMaxMs = 0xfff * kGranularityMs; // 40950.
static constexpr TimeDelta kMax = 0xfff * kGranularity; // 40950.
static bool Parse(rtc::ArrayView<const uint8_t> data,
VideoPlayoutDelay* playout_delay);

10
modules/rtp_rtcp/source/rtp_packet_unittest.cc
View file

@ -253,8 +253,9 @@ TEST(RtpPacketTest, CreateWithTwoByteHeaderExtensionFirst) {
packet.SetTimestamp(kTimestamp);
packet.SetSsrc(kSsrc);
// Set extension that requires two-byte header.
VideoPlayoutDelay playoutDelay = {30, 340};
ASSERT_TRUE(packet.SetExtension<PlayoutDelayLimits>(playoutDelay));
VideoPlayoutDelay playout_delay(TimeDelta::Millis(30),
TimeDelta::Millis(340));
ASSERT_TRUE(packet.SetExtension<PlayoutDelayLimits>(playout_delay));
packet.SetExtension<TransmissionOffset>(kTimeOffset);
packet.SetExtension<AudioLevel>(kVoiceActive, kAudioLevel);
EXPECT_THAT(kPacketWithTwoByteExtensionIdFirst,
@ -277,8 +278,9 @@ TEST(RtpPacketTest, CreateWithTwoByteHeaderExtensionLast) {
EXPECT_THAT(kPacketWithTOAndAL,
ElementsAreArray(packet.data(), packet.size()));
// Set extension that requires two-byte header.
VideoPlayoutDelay playoutDelay = {30, 340};
ASSERT_TRUE(packet.SetExtension<PlayoutDelayLimits>(playoutDelay));
VideoPlayoutDelay playout_delay(TimeDelta::Millis(30),
TimeDelta::Millis(340));
ASSERT_TRUE(packet.SetExtension<PlayoutDelayLimits>(playout_delay));
EXPECT_THAT(kPacketWithTwoByteExtensionIdLast,
ElementsAreArray(packet.data(), packet.size()));
}

10
modules/rtp_rtcp/source/rtp_sender_video.cc
View file

@ -106,8 +106,9 @@ absl::optional<VideoPlayoutDelay> LoadVideoPlayoutDelayOverride(
ParseFieldTrial({&playout_delay_max_ms, &playout_delay_min_ms},
key_value_config->Lookup("WebRTC-ForceSendPlayoutDelay"));
return playout_delay_max_ms && playout_delay_min_ms
? absl::make_optional<VideoPlayoutDelay>(*playout_delay_min_ms,
*playout_delay_max_ms)
? absl::make_optional<VideoPlayoutDelay>(
TimeDelta::Millis(*playout_delay_min_ms),
TimeDelta::Millis(*playout_delay_max_ms))
: absl::nullopt;
}
@ -862,11 +863,6 @@ void RTPSenderVideo::MaybeUpdateCurrentPlayoutDelay(
return;
}
if (!requested_delay->Valid()) {
RTC_DLOG(LS_ERROR) << "Requested playout delay values out of order";
return;
}
current_playout_delay_ = requested_delay;
playout_delay_pending_ = true;
}

5
modules/rtp_rtcp/source/rtp_sender_video_unittest.cc
View file

@ -1333,7 +1333,8 @@ TEST_F(RtpSenderVideoTest, PopulatesPlayoutDelay) {
uint8_t kFrame[kPacketSize];
rtp_module_->RegisterRtpHeaderExtension(PlayoutDelayLimits::Uri(),
kPlayoutDelayExtensionId);
const VideoPlayoutDelay kExpectedDelay = {10, 20};
const VideoPlayoutDelay kExpectedDelay(TimeDelta::Millis(10),
TimeDelta::Millis(20));
// Send initial key-frame without playout delay.
RTPVideoHeader hdr;
@ -1362,7 +1363,7 @@ TEST_F(RtpSenderVideoTest, PopulatesPlayoutDelay) {
// 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 = VideoPlayoutDelay(); // Indicates "no change".
hdr.playout_delay = absl::nullopt; // Indicates "no change".
vp8_header.temporalIdx = 0;
rtp_sender_video_->SendVideo(
kPayload, kType, kTimestamp, fake_clock_.CurrentTime(), kFrame,

7
modules/video_coding/generic_decoder_unittest.cc
View file

@ -177,9 +177,10 @@ TEST_F(GenericDecoderTest, IsLowLatencyStreamFalseByDefault) {
TEST_F(GenericDecoderTest, IsLowLatencyStreamActivatedByPlayoutDelay) {
VCMEncodedFrame encoded_frame;
const VideoPlayoutDelay kPlayoutDelay = {0, 50};
timing_.set_min_playout_delay(TimeDelta::Millis(kPlayoutDelay.min_ms));
timing_.set_max_playout_delay(TimeDelta::Millis(kPlayoutDelay.max_ms));
const VideoPlayoutDelay kPlayoutDelay(TimeDelta::Zero(),
TimeDelta::Millis(50));
timing_.set_min_playout_delay(kPlayoutDelay.min());
timing_.set_max_playout_delay(kPlayoutDelay.max());
generic_decoder_.Decode(encoded_frame, clock_->CurrentTime());
time_controller_.AdvanceTime(TimeDelta::Millis(10));
absl::optional<VideoFrame> decoded_frame = user_callback_.PopLastFrame();

24
video/rtp_video_stream_receiver2_unittest.cc
View file

@ -1174,17 +1174,23 @@ TEST_F(RtpVideoStreamReceiver2Test, TransformFrame) {
}
// Test default behavior and when playout delay is overridden by field trial.
const VideoPlayoutDelay kTransmittedPlayoutDelay = {100, 200};
const VideoPlayoutDelay kForcedPlayoutDelay = {70, 90};
VideoPlayoutDelay TransmittedPlayoutDelay() {
return {TimeDelta::Millis(100), TimeDelta::Millis(200)};
}
VideoPlayoutDelay ForcedPlayoutDelay() {
return {TimeDelta::Millis(70), TimeDelta::Millis(90)};
}
struct PlayoutDelayOptions {
std::string field_trial;
VideoPlayoutDelay expected_delay;
};
const PlayoutDelayOptions kDefaultBehavior = {
/*field_trial=*/"", /*expected_delay=*/kTransmittedPlayoutDelay};
const PlayoutDelayOptions kOverridePlayoutDelay = {
/*field_trial=*/"WebRTC-ForcePlayoutDelay/min_ms:70,max_ms:90/",
/*expected_delay=*/kForcedPlayoutDelay};
PlayoutDelayOptions DefaultBehavior() {
return {.field_trial = "", .expected_delay = TransmittedPlayoutDelay()};
}
PlayoutDelayOptions OverridePlayoutDelay() {
return {.field_trial = "WebRTC-ForcePlayoutDelay/min_ms:70,max_ms:90/",
.expected_delay = ForcedPlayoutDelay()};
}
class RtpVideoStreamReceiver2TestPlayoutDelay
: public RtpVideoStreamReceiver2Test,
@ -1196,7 +1202,7 @@ class RtpVideoStreamReceiver2TestPlayoutDelay
INSTANTIATE_TEST_SUITE_P(PlayoutDelay,
RtpVideoStreamReceiver2TestPlayoutDelay,
Values(kDefaultBehavior, kOverridePlayoutDelay));
Values(DefaultBehavior(), OverridePlayoutDelay()));
TEST_P(RtpVideoStreamReceiver2TestPlayoutDelay, PlayoutDelay) {
rtc::CopyOnWriteBuffer payload_data({'1', '2', '3', '4'});
@ -1208,7 +1214,7 @@ TEST_P(RtpVideoStreamReceiver2TestPlayoutDelay, PlayoutDelay) {
// Set playout delay on outgoing packet.
EXPECT_TRUE(packet_to_send.SetExtension<PlayoutDelayLimits>(
kTransmittedPlayoutDelay));
TransmittedPlayoutDelay()));
uint8_t* payload = packet_to_send.AllocatePayload(payload_data.size());
memcpy(payload, payload_data.data(), payload_data.size());

17
video/video_receive_stream2_unittest.cc
View file

@ -314,18 +314,19 @@ TEST_P(VideoReceiveStream2Test, CreateFrameFromH264FmtpSpropAndIdr) {
}
TEST_P(VideoReceiveStream2Test, PlayoutDelay) {
const VideoPlayoutDelay kPlayoutDelayMs = {123, 321};
const VideoPlayoutDelay kPlayoutDelay(TimeDelta::Millis(123),
TimeDelta::Millis(321));
std::unique_ptr<test::FakeEncodedFrame> test_frame =
test::FakeFrameBuilder()
.Id(0)
.PlayoutDelay(kPlayoutDelayMs)
.PlayoutDelay(kPlayoutDelay)
.AsLast()
.Build();
video_receive_stream_->OnCompleteFrame(std::move(test_frame));
auto timings = timing_->GetTimings();
EXPECT_EQ(kPlayoutDelayMs.min_ms, timings.min_playout_delay.ms());
EXPECT_EQ(kPlayoutDelayMs.max_ms, timings.max_playout_delay.ms());
EXPECT_EQ(kPlayoutDelay.min(), timings.min_playout_delay);
EXPECT_EQ(kPlayoutDelay.max(), timings.max_playout_delay);
// Check that the biggest minimum delay is chosen.
video_receive_stream_->SetMinimumPlayoutDelay(400);
@ -373,7 +374,7 @@ TEST_P(VideoReceiveStream2Test, UseLowLatencyRenderingSetFromPlayoutDelay) {
std::unique_ptr<test::FakeEncodedFrame> test_frame1 =
test::FakeFrameBuilder()
.Id(1)
.PlayoutDelay({/*min_ms=*/0, /*max_ms=*/500})
.PlayoutDelay({TimeDelta::Zero(), TimeDelta::Millis(500)})
.AsLast()
.Build();
video_receive_stream_->OnCompleteFrame(std::move(test_frame1));
@ -404,7 +405,7 @@ TEST_P(VideoReceiveStream2Test, MaxCompositionDelaySetFromMaxPlayoutDelay) {
.Id(1)
.Time(RtpTimestampForFrame(1))
.ReceivedTime(ReceiveTimeForFrame(1))
.PlayoutDelay({0, 0})
.PlayoutDelay(VideoPlayoutDelay::Minimal())
.AsLast()
.Build();
video_receive_stream_->OnCompleteFrame(std::move(test_frame1));
@ -418,7 +419,7 @@ TEST_P(VideoReceiveStream2Test, MaxCompositionDelaySetFromMaxPlayoutDelay) {
.Id(2)
.Time(RtpTimestampForFrame(2))
.ReceivedTime(ReceiveTimeForFrame(2))
.PlayoutDelay({10, 30})
.PlayoutDelay({TimeDelta::Millis(10), TimeDelta::Millis(30)})
.AsLast()
.Build();
video_receive_stream_->OnCompleteFrame(std::move(test_frame2));
@ -434,7 +435,7 @@ TEST_P(VideoReceiveStream2Test, MaxCompositionDelaySetFromMaxPlayoutDelay) {
.Id(3)
.Time(RtpTimestampForFrame(3))
.ReceivedTime(ReceiveTimeForFrame(3))
.PlayoutDelay({0, 50})
.PlayoutDelay({TimeDelta::Zero(), TimeDelta::Millis(50)})
.AsLast()
.Build();
video_receive_stream_->OnCompleteFrame(std::move(test_frame3));

8
video/video_stream_buffer_controller_unittest.cc
View file

@ -827,7 +827,7 @@ TEST_P(LowLatencyVideoStreamBufferControllerTest,
auto frame = test::FakeFrameBuilder()
.Id(0)
.Time(0)
.PlayoutDelay({0, 10})
.PlayoutDelay({TimeDelta::Zero(), TimeDelta::Millis(10)})
.AsLast()
.Build();
buffer_->InsertFrame(std::move(frame));
@ -839,7 +839,7 @@ TEST_P(LowLatencyVideoStreamBufferControllerTest,
frame = test::FakeFrameBuilder()
.Id(1)
.Time(kFps30Rtp)
.PlayoutDelay({0, 10})
.PlayoutDelay({TimeDelta::Zero(), TimeDelta::Millis(10)})
.AsLast()
.Build();
buffer_->InsertFrame(std::move(frame));
@ -857,7 +857,7 @@ TEST_P(LowLatencyVideoStreamBufferControllerTest, ZeroPlayoutDelayFullQueue) {
auto frame = test::FakeFrameBuilder()
.Id(0)
.Time(0)
.PlayoutDelay({0, 10})
.PlayoutDelay({TimeDelta::Zero(), TimeDelta::Millis(10)})
.AsLast()
.Build();
// Playout delay of 0 implies low-latency rendering.
@ -869,7 +869,7 @@ TEST_P(LowLatencyVideoStreamBufferControllerTest, ZeroPlayoutDelayFullQueue) {
frame = test::FakeFrameBuilder()
.Id(id)
.Time(kFps30Rtp * id)
.PlayoutDelay({0, 10})
.PlayoutDelay({TimeDelta::Zero(), TimeDelta::Millis(10)})
.AsLast()
.Build();
buffer_->InsertFrame(std::move(frame));