Calculate NtpCaptureStartMs using clock, not the frame rtp timestamp

If the first frame rtp timestamp got corrupted somehow, the introduced
error would stay there for the duration of the call. Using realtime
clock to calculate elapsed time instead of rtp timestamps resolves that
problem. The error will go away once ntp time would be estimated
correctly from the correct timestamps.

Bug: webrtc:9698
Change-Id: Ifa4c3f55f280fae8ec9f1826a89c251ec61b965e
Reviewed-on: https://webrtc-review.googlesource.com/97101
Reviewed-by: Fredrik Solenberg <solenberg@webrtc.org>
Reviewed-by: Stefan Holmer <stefan@webrtc.org>
Commit-Queue: Ilya Nikolaevskiy <ilnik@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#24726}
This commit is contained in:
Ilya Nikolaevskiy 2018-09-03 16:11:42 +02:00 committed by Commit Bot
parent bfb72ad4f4
commit 9c38c47630
2 changed files with 13 additions and 7 deletions

View File

@ -2429,13 +2429,10 @@ void WebRtcVideoChannel::WebRtcVideoReceiveStream::OnFrame(
const webrtc::VideoFrame& frame) {
rtc::CritScope crit(&sink_lock_);
int64_t time_now_ms = rtc::TimeMillis();
if (first_frame_timestamp_ < 0)
first_frame_timestamp_ = frame.timestamp();
int64_t rtp_time_elapsed_since_first_frame =
(timestamp_wraparound_handler_.Unwrap(frame.timestamp()) -
first_frame_timestamp_);
int64_t elapsed_time_ms = rtp_time_elapsed_since_first_frame /
(cricket::kVideoCodecClockrate / 1000);
first_frame_timestamp_ = time_now_ms;
int64_t elapsed_time_ms = time_now_ms - first_frame_timestamp_;
if (frame.ntp_time_ms() > 0)
estimated_remote_start_ntp_time_ms_ = frame.ntp_time_ms() - elapsed_time_ms;

View File

@ -40,6 +40,7 @@
#include "media/engine/webrtcvoiceengine.h"
#include "modules/rtp_rtcp/include/rtp_header_parser.h"
#include "rtc_base/arraysize.h"
#include "rtc_base/fakeclock.h"
#include "rtc_base/gunit.h"
#include "rtc_base/numerics/safe_conversions.h"
#include "rtc_base/stringutils.h"
@ -210,7 +211,11 @@ class WebRtcVideoEngineTest : public ::testing::Test {
engine_(std::unique_ptr<cricket::FakeWebRtcVideoEncoderFactory>(
encoder_factory_),
std::unique_ptr<cricket::FakeWebRtcVideoDecoderFactory>(
decoder_factory_)) {}
decoder_factory_)) {
// Ensure fake clock doesn't return 0, which will cause some initializations
// fail inside RTP senders.
fake_clock_.AdvanceTimeMicros(1);
}
protected:
void AssignDefaultAptRtxTypes();
@ -231,6 +236,9 @@ class WebRtcVideoEngineTest : public ::testing::Test {
void TestExtendedEncoderOveruse(bool use_external_encoder);
// Has to be the first one, so it is initialized before the call or there is a
// race condition in the clock access.
rtc::ScopedFakeClock fake_clock_;
webrtc::test::ScopedFieldTrials override_field_trials_;
webrtc::RtcEventLogNullImpl event_log_;
// Used in WebRtcVideoEngineVoiceTest, but defined here so it's properly
@ -3480,6 +3488,7 @@ TEST_F(WebRtcVideoChannelTest, EstimatesNtpStartTimeCorrectly) {
// This timestamp is kInitialTimestamp (-1) + kFrameOffsetMs * 90, which
// triggers a constant-overflow warning, hence we're calculating it explicitly
// here.
fake_clock_.AdvanceTimeMicros(kFrameOffsetMs * rtc::kNumMicrosecsPerMillisec);
video_frame.set_timestamp(kFrameOffsetMs * 90 - 1);
video_frame.set_ntp_time_ms(kInitialNtpTimeMs + kFrameOffsetMs);
stream->InjectFrame(video_frame);