diff --git a/modules/video_coding/timing/timestamp_extrapolator.cc b/modules/video_coding/timing/timestamp_extrapolator.cc index c91aa1a362..a90df8bf7f 100644 --- a/modules/video_coding/timing/timestamp_extrapolator.cc +++ b/modules/video_coding/timing/timestamp_extrapolator.cc @@ -132,13 +132,24 @@ absl::optional TimestampExtrapolator::ExtrapolateLocalTime( constexpr double kRtpTicksPerMs = 90; TimeDelta diff = TimeDelta::Millis( (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; } else if (w_[0] < 1e-3) { return start_; } else { double timestampDiff = unwrapped_ts90khz - *first_unwrapped_timestamp_; - auto diff_ms = static_cast((timestampDiff - w_[1]) / w_[0] + 0.5); - return start_ + TimeDelta::Millis(diff_ms); + TimeDelta diff = TimeDelta::Millis( + static_cast((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; } } diff --git a/modules/video_coding/timing/timestamp_extrapolator_unittest.cc b/modules/video_coding/timing/timestamp_extrapolator_unittest.cc index 0b5fd74a8e..d6c8fa9de1 100644 --- a/modules/video_coding/timing/timestamp_extrapolator_unittest.cc +++ b/modules/video_coding/timing/timestamp_extrapolator_unittest.cc @@ -127,12 +127,25 @@ TEST(TimestampExtrapolatorTest, NegativeRtpTimestampWrapAround) { ts_extrapolator.Update(clock.CurrentTime(), rtp); EXPECT_THAT(ts_extrapolator.ExtrapolateLocalTime(rtp), Optional(clock.CurrentTime())); - // Go backwards! - rtp -= kRtpHz.hertz(); + // Go backwards! Static cast to avoid undefined behaviour with -=. + rtp -= static_cast(kRtpHz.hertz()); EXPECT_THAT(ts_extrapolator.ExtrapolateLocalTime(rtp), 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(kRtpHz * TimeDelta::Seconds(10)); + ts_extrapolator.Update(clock.CurrentTime(), rtp); + EXPECT_THAT(ts_extrapolator.ExtrapolateLocalTime(rtp), absl::nullopt); +} + TEST(TimestampExtrapolatorTest, Slow90KHzClock) { // This simulates a slow camera, which produces frames at 24Hz instead of // 25Hz. The extrapolator should be able to resolve this with enough data.