Decouple capture timestamp and reference time

media/capture/video/video_capture_device.h

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 //
 // VideoCaptureDevice is the abstract base class for realizing video capture
 // device support in Chromium. It provides the interface for OS dependent
 // implementations.
 // The class is created and functions are invoked on a thread owned by
 // VideoCaptureManager. Capturing is done on other threads, depending on the OS
 // specific implementation.
@@ skipping 182 matching lines @@
 #endif
     };
 
     virtual ~Client() {}
 
     // Captured a new video frame, data for which is pointed to by |data|.
     //
     // The format of the frame is described by |frame_format|, and is assumed to
     // be tightly packed. This method will try to reserve an output buffer and
     // copy from |data| into the output buffer. If no output buffer is
-    // available, the frame will be silently dropped.
+    // available, the frame will be silently dropped. |referernce_time| is

[miu, 2016/05/18 22:35:40]

typo: s/referernce_time/reference_time/

[qiangchen, 2016/05/20 17:55:14]

Done.

+    // estimated system clock time when the capture happened. |timestamp|

[miu, 2016/05/18 22:35:40]

Remove "estimated" since we really *are* taking a snapshot of system clock time.
;-)

IMO, it's also worth mentioning: "The reference time is used to achieve A/V
sync, and not meant to be used to derive an exact presentation time for
playout."

[qiangchen, 2016/05/20 17:55:14]

Done.

+    // measures the ideal time span between the first frame in the stream and
+    // the current frame; however, the time source is not necessarily the

[miu, 2016/05/18 22:35:40]

nit: End of this sentence should be: "...however, the time source is determined
by the platform's device driver and is often not the system clock."

[qiangchen, 2016/05/20 17:55:14]

Done.

+    // system clock.
     virtual void OnIncomingCapturedData(const uint8_t* data,
                                         int length,
                                         const VideoCaptureFormat& frame_format,
                                         int clockwise_rotation,
-                                        const base::TimeTicks& timestamp) = 0;
+                                        base::TimeTicks reference_time,
+                                        base::TimeDelta timestamp) = 0;
 
     // Reserve an output buffer into which contents can be captured directly.
     // The returned Buffer will always be allocated with a memory size suitable
     // for holding a packed video frame with pixels of |format| format, of
     // |dimensions| frame dimensions. It is permissible for |dimensions| to be
     // zero; in which case the returned Buffer does not guarantee memory
     // backing, but functions as a reservation for external input for the
     // purposes of buffer throttling.
     //
     // The output buffer stays reserved and mapped for use until the Buffer
     // object is destroyed or returned.
     virtual std::unique_ptr<Buffer> ReserveOutputBuffer(
         const gfx::Size& dimensions,
         VideoPixelFormat format,
         VideoPixelStorage storage) = 0;
 
     // Captured new video data, held in |frame| or |buffer|, respectively for
     // OnIncomingCapturedVideoFrame() and  OnIncomingCapturedBuffer().
     //
     // In both cases, as the frame is backed by a reservation returned by
     // ReserveOutputBuffer(), delivery is guaranteed and will require no
     // additional copies in the browser process.
     virtual void OnIncomingCapturedBuffer(
         std::unique_ptr<Buffer> buffer,
         const VideoCaptureFormat& frame_format,
         const base::TimeTicks& timestamp) = 0;

[miu, 2016/05/18 22:35:40]

These two methods need similar changes. Do you intend to do that in a future
change?

[qiangchen, 2016/05/20 17:55:14]

I intended to do it in next CL, but now I think it is better to do it in this
CL, because it touches same subclasses and thus same files with the change for
OnIncomingCapturedData.

     virtual void OnIncomingCapturedVideoFrame(
         std::unique_ptr<Buffer> buffer,
         const scoped_refptr<VideoFrame>& frame,
         const base::TimeTicks& timestamp) = 0;

[miu, 2016/05/18 22:35:40]

BTW--For this method, the TimeTicks argument would be reference_time, and
instead of a separate |timestamp| TimeDelta argument,
VideoFrame::set_timestamp() should be used. Then, client code should
DCHECK(!frame->timestamp().is_null());

[qiangchen, 2016/05/20 17:55:14]

Done.

 
     // Attempts to reserve the same Buffer provided in the last call to one of
     // the OnIncomingCapturedXXX() methods. This will fail if the content of the
     // Buffer has not been preserved, or if the |dimensions|, |format|, or
     // |storage| disagree with how it was reserved via ReserveOutputBuffer().
     // When this operation fails, nullptr will be returned.
     virtual std::unique_ptr<Buffer> ResurrectLastOutputBuffer(
         const gfx::Size& dimensions,
         VideoPixelFormat format,
         VideoPixelStorage storage) = 0;
@@ skipping 67 matching lines @@
 
  private:
   // Gets the power line frequency from the current system time zone if this is
   // defined, otherwise returns 0.
   PowerLineFrequency GetPowerLineFrequencyForLocation() const;
 };
 
 }  // namespace media
 
 #endif  // MEDIA_VIDEO_CAPTURE_VIDEO_CAPTURE_DEVICE_H_