Decouple capture timestamp and reference time

content/renderer/media/video_capture_impl.cc

Index: content/renderer/media/video_capture_impl.cc
diff --git a/content/renderer/media/video_capture_impl.cc b/content/renderer/media/video_capture_impl.cc
index 90026160a5e3b6aa5e8d3ba9180459836e2615d4..5795534e65cb9963074ae229f2983b4f8a0d262a 100644
--- a/content/renderer/media/video_capture_impl.cc
+++ b/content/renderer/media/video_capture_impl.cc
@@ -191,7 +191,6 @@ void VideoCaptureImpl::StartCapture(
       }
       DVLOG(1) << "StartCapture: starting with first resolution "
                << params_.requested_format.frame_size.ToString();
-      first_frame_timestamp_ = base::TimeTicks();
       StartCaptureInternal();
     }
   }
@@ -304,7 +303,7 @@ void VideoCaptureImpl::OnBufferDestroyed(int buffer_id) {
 
 void VideoCaptureImpl::OnBufferReceived(
     int buffer_id,
-    base::TimeTicks timestamp,
+    base::TimeDelta timestamp,
     const base::DictionaryValue& metadata,
     media::VideoPixelFormat pixel_format,
     media::VideoFrame::StorageType storage_type,
@@ -324,14 +323,29 @@ void VideoCaptureImpl::OnBufferReceived(
                                              gpu::SyncToken(), -1.0));
     return;
   }
-  if (first_frame_timestamp_.is_null())
-    first_frame_timestamp_ = timestamp;
 
+  base::TimeTicks reference_time;
+  media::VideoFrameMetadata frame_metadata;
+  frame_metadata.MergeInternalValuesFrom(metadata);
+  const bool success = frame_metadata.GetTimeTicks(
+      media::VideoFrameMetadata::REFERENCE_TIME, &reference_time);
+  DCHECK(success);
+
+  if (first_frame_ref_time_.is_null())
+    first_frame_ref_time_ = reference_time;
+
+  // If the timestamp is not prepared, we use reference time to make a rough

[miu, 2016/06/08 19:24:44]

This wasn't in PS1. Looks like there are some capturers that don't set the
VideoFrame::timestamp?

Actually, when I look up the code for tab capture, I see this is the case. Could
you add a "TODO(miu): Fix upstream capturers to always set timestamp and
reference time." here, with a link to this crbug: http://crbug.com/618407

[qiangchen, 2016/06/08 20:08:54]

Yeah, when I change the signature of OnIncomingCapturedVideoFrame,
I found ThreadSafeCaptureOracle::DidCaptureFrame does not prepare timestamp. I
do not know what is that, and it looks like that the code path is deep.

+  // estimate.
+  if (timestamp.is_zero())
+    timestamp = reference_time - first_frame_ref_time_;
+
+  // TODO(qiangchen): Change the metric name to "reference_time" and
+  // "timestamp", so that we have consistent naming everywhere.
   // Used by chrome/browser/extension/api/cast_streaming/performance_test.cc
   TRACE_EVENT_INSTANT2("cast_perf_test", "OnBufferReceived",
                        TRACE_EVENT_SCOPE_THREAD, "timestamp",
-                       timestamp.ToInternalValue(), "time_delta",
-                       (timestamp - first_frame_timestamp_).ToInternalValue());
+                       (reference_time - base::TimeTicks()).InMicroseconds(),
+                       "time_delta", timestamp.InMicroseconds());
 
   scoped_refptr<media::VideoFrame> frame;
   BufferFinishedCallback buffer_finished_callback;
@@ -343,11 +357,8 @@ void VideoCaptureImpl::OnBufferReceived(
       scoped_refptr<ClientBuffer2> buffer = iter->second;
       const auto& handles = buffer->gpu_memory_buffer_handles();
       frame = media::VideoFrame::WrapExternalYuvGpuMemoryBuffers(
-          media::PIXEL_FORMAT_I420,
-          coded_size,
-          gfx::Rect(coded_size),
-          coded_size,
-          buffer->stride(media::VideoFrame::kYPlane),
+          media::PIXEL_FORMAT_I420, coded_size, gfx::Rect(coded_size),
+          coded_size, buffer->stride(media::VideoFrame::kYPlane),
           buffer->stride(media::VideoFrame::kUPlane),
           buffer->stride(media::VideoFrame::kVPlane),
           buffer->data(media::VideoFrame::kYPlane),
@@ -355,8 +366,7 @@ void VideoCaptureImpl::OnBufferReceived(
           buffer->data(media::VideoFrame::kVPlane),
           handles[media::VideoFrame::kYPlane],
           handles[media::VideoFrame::kUPlane],
-          handles[media::VideoFrame::kVPlane],
-          timestamp - first_frame_timestamp_);
+          handles[media::VideoFrame::kVPlane], timestamp);
       buffer_finished_callback = media::BindToCurrentLoop(
           base::Bind(&VideoCaptureImpl::OnClientBufferFinished2,
                      weak_factory_.GetWeakPtr(), buffer_id, buffer));
@@ -371,7 +381,7 @@ void VideoCaptureImpl::OnBufferReceived(
           gfx::Size(visible_rect.width(), visible_rect.height()),
           reinterpret_cast<uint8_t*>(buffer->buffer()->memory()),
           buffer->buffer_size(), buffer->buffer()->handle(),
-          0 /* shared_memory_offset */, timestamp - first_frame_timestamp_);
+          0 /* shared_memory_offset */, timestamp);
       buffer_finished_callback = media::BindToCurrentLoop(
           base::Bind(&VideoCaptureImpl::OnClientBufferFinished,
                      weak_factory_.GetWeakPtr(), buffer_id, buffer));
@@ -387,16 +397,16 @@ void VideoCaptureImpl::OnBufferReceived(
     return;
   }
 
-  frame->metadata()->SetTimeTicks(media::VideoFrameMetadata::REFERENCE_TIME,
-                                  timestamp);
   frame->AddDestructionObserver(
       base::Bind(&VideoCaptureImpl::DidFinishConsumingFrame, frame->metadata(),
                  base::Passed(&release_sync_token), buffer_finished_callback));
 
   frame->metadata()->MergeInternalValuesFrom(metadata);
 
+  // TODO(qiangchen): Dive into the full code path to let frame metadata hold
+  // reference time rather than using an extra parameter.

[miu, 2016/06/08 19:24:44]

Yep, this "deliver callback" API was developed before VideoFrameMetadata even
existed.

[qiangchen, 2016/06/08 20:08:54]

Acknowledged.

   for (const auto& client : clients_)
-    client.second.deliver_frame_cb.Run(frame, timestamp);
+    client.second.deliver_frame_cb.Run(frame, reference_time);
 }
 
 void VideoCaptureImpl::OnClientBufferFinished(