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..facd3ed1dd40b86863165b5eb1d1d69016d99578 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,8 @@ void VideoCaptureImpl::OnBufferDestroyed(int buffer_id) {
 
 void VideoCaptureImpl::OnBufferReceived(
     int buffer_id,
-    base::TimeTicks timestamp,
+    base::TimeTicks reference_time,
+    base::TimeDelta timestamp,
     const base::DictionaryValue& metadata,
     media::VideoPixelFormat pixel_format,
     media::VideoFrame::StorageType storage_type,
@@ -324,14 +324,12 @@ void VideoCaptureImpl::OnBufferReceived(
                                              gpu::SyncToken(), -1.0));
     return;
   }
-  if (first_frame_timestamp_.is_null())
-    first_frame_timestamp_ = timestamp;
 
   // 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());
+                       TRACE_EVENT_SCOPE_THREAD, "reference_time",
+                       reference_time.ToInternalValue(), "timestamp",

[miu, 2016/06/07 20:03:47]

Hmm...These uses of ToInternalValue() are questionable. It would be more
readable to write these as:

  TRACE_EVENT_INSTANT2(
      "cast_perf_test", "OnBufferReceived",
      TRACE_EVENT_SCOPE_THREAD, "reference_time",
      (reference_time - base::TimeTicks()).InMicroseconds(),
      "timestamp", timestamp.InMicroseconds());

Also, this trace event is read by a performance test (see comment on line 328).
If you change the names here, you'll need to update that test code as well.

[qiangchen, 2016/06/08 18:04:30]

Change the names back to be consistent with other metrics. Changing the metric
name would be out of this CL's scope.

+                       timestamp.ToInternalValue());
 
   scoped_refptr<media::VideoFrame> frame;
   BufferFinishedCallback buffer_finished_callback;
@@ -343,11 +341,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 +350,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 +365,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));
@@ -388,7 +382,7 @@ void VideoCaptureImpl::OnBufferReceived(
   }
 
   frame->metadata()->SetTimeTicks(media::VideoFrameMetadata::REFERENCE_TIME,

[miu, 2016/06/07 20:03:47]

This can become:

  DCHECK(frame->metadata()->HasKey(media::VideoFrameMetadata::REFERENCE_TIME));

(Since this field should have been set in the browser process.)

Oh, and it'll need to be moved down, after line 390 below, where the metadata is
copied from the IPC message.

[qiangchen, 2016/06/08 18:04:30]

It is removed, as the metadata is already prepared with this field.

-                                  timestamp);
+                                  reference_time);
   frame->AddDestructionObserver(
       base::Bind(&VideoCaptureImpl::DidFinishConsumingFrame, frame->metadata(),
                  base::Passed(&release_sync_token), buffer_finished_callback));
@@ -396,7 +390,7 @@ void VideoCaptureImpl::OnBufferReceived(
   frame->metadata()->MergeInternalValuesFrom(metadata);
 
   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(