mollyim/webrtc
1
0
Fork 0
mirror of https://github.com/mollyim/webrtc.git synced 2025-05-14 06:10:40 +01:00
Code Issues Projects Releases Packages Wiki Activity Actions

Deflake simulcast flow tests: prevent negative Timestamp exception.

Browse source 
These tests often fail in 'ExtrapolateLocalTime' because the result gives a negative Timestamp.

Here is the stack from https://chromium-swarm.appspot.com/task?id=6173230e67897b10:
PC: @     0x7f03afdb8e87  (unknown)  raise
    ...
    @     0x55f4a360ba71        352  webrtc::Timestamp::operator+()
    @     0x55f4a47ecaf3        160  webrtc::TimestampExtrapolator::ExtrapolateLocalTime()

Low-Coverage-Reason: coverage isn't that low.
Change-Id: If3e7cbf31d6c4800727b24352ed2c6edc425fc73
Bug: webrtc:15022
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/300600
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Reviewed-by: Rasmus Brandt <brandtr@webrtc.org>
Commit-Queue: Jeremy Leconte <jleconte@google.com>
Cr-Commit-Position: refs/heads/main@{#39853}
This commit is contained in:
Jeremy Leconte 2023-04-07 10:09:12 +02:00 committed by WebRTC LUCI CQ
parent c087d0cce5
commit 06e2148889
2 changed files with 28 additions and 4 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

15
modules/video_coding/timing/timestamp_extrapolator.cc
View file

@ -132,13 +132,24 @@ absl::optional<Timestamp> TimestampExtrapolator::ExtrapolateLocalTime(
constexpr double kRtpTicksPerMs = 90; constexpr double kRtpTicksPerMs = 90;
TimeDelta diff = TimeDelta::Millis( TimeDelta diff = TimeDelta::Millis(
(unwrapped_ts90khz - *prev_unwrapped_timestamp_) / kRtpTicksPerMs); (unwrapped_ts90khz - *prev_unwrapped_timestamp_) / kRtpTicksPerMs);
if (prev_.us() + diff.us() < 0) {
// Prevent the construction of a negative Timestamp.
// This scenario can occur when the RTP timestamp wraps around.
return absl::nullopt;
}
return prev_ + diff; return prev_ + diff;
} else if (w_[0] < 1e-3) { } else if (w_[0] < 1e-3) {
return start_; return start_;
} else { } else {
double timestampDiff = unwrapped_ts90khz - *first_unwrapped_timestamp_; double timestampDiff = unwrapped_ts90khz - *first_unwrapped_timestamp_;
auto diff_ms = static_cast<int64_t>((timestampDiff - w_[1]) / w_[0] + 0.5); TimeDelta diff = TimeDelta::Millis(
return start_ + TimeDelta::Millis(diff_ms); static_cast<int64_t>((timestampDiff - w_[1]) / w_[0] + 0.5));
if (start_.us() + diff.us() < 0) {
// Prevent the construction of a negative Timestamp.
// This scenario can occur when the RTP timestamp wraps around.
return absl::nullopt;
}
return start_ + diff;
} }
} }

17
modules/video_coding/timing/timestamp_extrapolator_unittest.cc
View file

@ -127,12 +127,25 @@ TEST(TimestampExtrapolatorTest, NegativeRtpTimestampWrapAround) {
ts_extrapolator.Update(clock.CurrentTime(), rtp); ts_extrapolator.Update(clock.CurrentTime(), rtp);
EXPECT_THAT(ts_extrapolator.ExtrapolateLocalTime(rtp), EXPECT_THAT(ts_extrapolator.ExtrapolateLocalTime(rtp),
Optional(clock.CurrentTime())); Optional(clock.CurrentTime()));
// Go backwards! // Go backwards! Static cast to avoid undefined behaviour with -=.
rtp -= kRtpHz.hertz(); rtp -= static_cast<uint32_t>(kRtpHz.hertz());
EXPECT_THAT(ts_extrapolator.ExtrapolateLocalTime(rtp), EXPECT_THAT(ts_extrapolator.ExtrapolateLocalTime(rtp),
Optional(clock.CurrentTime() - TimeDelta::Seconds(1))); Optional(clock.CurrentTime() - TimeDelta::Seconds(1)));
} }
TEST(TimestampExtrapolatorTest, NegativeRtpTimestampWrapAroundSecondScenario) {
SimulatedClock clock(Timestamp::Millis(1337));
TimestampExtrapolator ts_extrapolator(clock.CurrentTime());
uint32_t rtp = 0;
ts_extrapolator.Update(clock.CurrentTime(), rtp);
EXPECT_THAT(ts_extrapolator.ExtrapolateLocalTime(rtp),
Optional(clock.CurrentTime()));
// Go backwards! Static cast to avoid undefined behaviour with -=.
rtp -= static_cast<uint32_t>(kRtpHz * TimeDelta::Seconds(10));
ts_extrapolator.Update(clock.CurrentTime(), rtp);
EXPECT_THAT(ts_extrapolator.ExtrapolateLocalTime(rtp), absl::nullopt);
}
TEST(TimestampExtrapolatorTest, Slow90KHzClock) { TEST(TimestampExtrapolatorTest, Slow90KHzClock) {
// This simulates a slow camera, which produces frames at 24Hz instead of // This simulates a slow camera, which produces frames at 24Hz instead of
// 25Hz. The extrapolator should be able to resolve this with enough data. // 25Hz. The extrapolator should be able to resolve this with enough data.