From eb436ddb8b313a81e758b5c5f97ca4349b5203a0 Mon Sep 17 00:00:00 2001 From: emircan Date: Wed, 31 Aug 2016 14:14:18 -0700 Subject: [PATCH] Revert of Use webrtc::VideoFrame timestamp in RTCVideoEncoder (patchset #7 id:260001 of https://codereview.chromium.org/2205623002/ ) Reason for revert: This CL caused some regressions regarding BWE stats and HW encoder performance. Reasons include: 1) Modifying scoped_refptr's timestamp causes problems for other clients using the same media::VideoFrame. 2) WebRTC's RTP timestamp isn't suitable for using as presentation timestamp in Mac and Win HW encoders. BUG=641230 Original issue's description: > Use webrtc::VideoFrame timestamp in RTCVideoEncoder > > This CL fixes input timestamp mismatch in RTCVideoEncoder, which > broke googAvgEncodeMs and googEncodeUsagePercent stats in webrtc-internals > for hardware encoders. > With this change, we start using WebRTC given timestamp() so that > OveruseFrameDetector can match the timestamps and calculate the stats. > > BUG=597087 > TEST=googAvgEncodeMs and googEncodeUsagePercent works on Mac(H264) and > veyron_jerry(VP8). > > Committed: https://crrev.com/e3195490a63d9545fb1bfe560aa21680ba0b5843 > Cr-Commit-Position: refs/heads/master@{#414589} TBR=wuchengli@chromium.org,pbos@chromium.org,posciak@chromium.org # Not skipping CQ checks because original CL landed more than 1 days ago. BUG=597087 Review-Url: https://codereview.chromium.org/2296273002 Cr-Commit-Position: refs/heads/master@{#415752} --- .../renderer/media/gpu/rtc_video_encoder.cc | 32 +++++++++---------- .../gpu/video_encode_accelerator_unittest.cc | 20 ++---------- 2 files changed, 17 insertions(+), 35 deletions(-) diff --git a/content/renderer/media/gpu/rtc_video_encoder.cc b/content/renderer/media/gpu/rtc_video_encoder.cc index 1718e9704e56e..da955a90f75a0 100644 --- a/content/renderer/media/gpu/rtc_video_encoder.cc +++ b/content/renderer/media/gpu/rtc_video_encoder.cc @@ -32,9 +32,6 @@ namespace content { namespace { -// Used for timestamp conversions. -static const int64_t kMsToRtpTimestamp = 90; - // Translate from webrtc::VideoCodecType and webrtc::VideoCodec to // media::VideoCodecProfile. media::VideoCodecProfile WebRTCVideoCodecToVideoCodecProfile( @@ -477,25 +474,27 @@ void RTCVideoEncoder::Impl::BitstreamBufferReady(int32_t bitstream_buffer_id, output_buffers_free_count_--; // Derive the capture time (in ms) and RTP timestamp (in 90KHz ticks). - int64_t rtp_timestamp, capture_time_ms; + int64_t capture_time_us, capture_time_ms; + uint32_t rtp_timestamp; + if (!timestamp.is_zero()) { - // Get RTP timestamp value. - rtp_timestamp = timestamp.ToInternalValue(); - capture_time_ms = rtp_timestamp / kMsToRtpTimestamp; + capture_time_us = timestamp.InMicroseconds();; + capture_time_ms = timestamp.InMilliseconds(); } else { // Fallback to the current time if encoder does not provide timestamp. - rtp_timestamp = rtc::TimeMicros() * kMsToRtpTimestamp / - base::Time::kMicrosecondsPerMillisecond; - capture_time_ms = - rtc::TimeMicros() / base::Time::kMicrosecondsPerMillisecond; + capture_time_us = rtc::TimeMicros(); + capture_time_ms = capture_time_us / base::Time::kMicrosecondsPerMillisecond; } + // RTP timestamp can wrap around. Get the lower 32 bits. + rtp_timestamp = static_cast( + capture_time_us * 90 / base::Time::kMicrosecondsPerMillisecond); webrtc::EncodedImage image( reinterpret_cast(output_buffer->memory()), payload_size, output_buffer->mapped_size()); image._encodedWidth = input_visible_size_.width(); image._encodedHeight = input_visible_size_.height(); - image._timeStamp = static_cast(rtp_timestamp); + image._timeStamp = rtp_timestamp; image.capture_time_ms_ = capture_time_ms; image._frameType = (key_frame ? webrtc::kVideoFrameKey : webrtc::kVideoFrameDelta); @@ -572,13 +571,15 @@ void RTCVideoEncoder::Impl::EncodeOneFrame() { } if (requires_copy) { + const base::TimeDelta timestamp = + frame ? frame->timestamp() + : base::TimeDelta::FromMilliseconds(next_frame->ntp_time_ms()); base::SharedMemory* input_buffer = input_buffers_[index]; frame = media::VideoFrame::WrapExternalSharedMemory( media::PIXEL_FORMAT_I420, input_frame_coded_size_, gfx::Rect(input_visible_size_), input_visible_size_, reinterpret_cast(input_buffer->memory()), - input_buffer->mapped_size(), input_buffer->handle(), 0, - base::TimeDelta()); + input_buffer->mapped_size(), input_buffer->handle(), 0, timestamp); if (!frame.get()) { LogAndNotifyError(FROM_HERE, "failed to create frame", media::VideoEncodeAccelerator::kPlatformFailureError); @@ -610,9 +611,6 @@ void RTCVideoEncoder::Impl::EncodeOneFrame() { return; } } - // Use the timestamp set from WebRTC and set it in 90 kHz. - frame->set_timestamp( - base::TimeDelta::FromInternalValue(next_frame->timestamp())); frame->AddDestructionObserver(media::BindToCurrentLoop( base::Bind(&RTCVideoEncoder::Impl::EncodeFrameFinished, this, index))); video_encoder_->Encode(frame, next_frame_keyframe); diff --git a/media/gpu/video_encode_accelerator_unittest.cc b/media/gpu/video_encode_accelerator_unittest.cc index 07a8b1caa3bf8..ae28ea813655d 100644 --- a/media/gpu/video_encode_accelerator_unittest.cc +++ b/media/gpu/video_encode_accelerator_unittest.cc @@ -570,9 +570,6 @@ class H264Validator : public StreamValidator { void H264Validator::ProcessStreamBuffer(const uint8_t* stream, size_t size) { h264_parser_.SetStream(stream, static_cast(size)); - // Run |frame_cb_| for only first nalu. - bool frame_cb_called = false; - while (1) { H264NALU nalu; H264Parser::Result result; @@ -593,14 +590,10 @@ void H264Validator::ProcessStreamBuffer(const uint8_t* stream, size_t size) { keyframe = true; // fallthrough case H264NALU::kNonIDRSlice: { - // Stream may contain at most one frame. ASSERT_TRUE(seen_idr_); seen_sps_ = seen_pps_ = false; - if (!frame_cb_called) { - frame_cb_called = true; - if (!frame_cb_.Run(keyframe)) - return; - } + if (!frame_cb_.Run(keyframe)) + return; break; } @@ -1850,15 +1843,6 @@ INSTANTIATE_TEST_CASE_P(MultipleEncoders, false, false, false))); - -#if defined(OS_MACOSX) -INSTANTIATE_TEST_CASE_P( - VerifyTimestamp, - VideoEncodeAcceleratorTest, - ::testing::Values( - std::make_tuple(1, false, 0, false, false, false, false, false, true))); -#endif // defined(OS_MACOSX) - #if defined(OS_WIN) INSTANTIATE_TEST_CASE_P( ForceBitrate,