Skip to content

Commit

Permalink
Revert of Use webrtc::VideoFrame timestamp in RTCVideoEncoder (patchset
Browse files Browse the repository at this point in the history
#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<media::VideoFrame>'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}
  • Loading branch information
uysalere authored and Commit bot committed Aug 31, 2016
1 parent 8809c44 commit eb436dd
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 35 deletions.
32 changes: 15 additions & 17 deletions content/renderer/media/gpu/rtc_video_encoder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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<uint32_t>(
capture_time_us * 90 / base::Time::kMicrosecondsPerMillisecond);

webrtc::EncodedImage image(
reinterpret_cast<uint8_t*>(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<uint32_t>(rtp_timestamp);
image._timeStamp = rtp_timestamp;
image.capture_time_ms_ = capture_time_ms;
image._frameType =
(key_frame ? webrtc::kVideoFrameKey : webrtc::kVideoFrameDelta);
Expand Down Expand Up @@ -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<uint8_t*>(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);
Expand Down Expand Up @@ -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);
Expand Down
20 changes: 2 additions & 18 deletions media/gpu/video_encode_accelerator_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -570,9 +570,6 @@ class H264Validator : public StreamValidator {
void H264Validator::ProcessStreamBuffer(const uint8_t* stream, size_t size) {
h264_parser_.SetStream(stream, static_cast<off_t>(size));

// Run |frame_cb_| for only first nalu.
bool frame_cb_called = false;

while (1) {
H264NALU nalu;
H264Parser::Result result;
Expand All @@ -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;
}

Expand Down Expand Up @@ -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,
Expand Down

0 comments on commit eb436dd

Please sign in to comment.