Mac Video Capture: Sending the timestamps provided by the driver to the capture pipeline

media/capture/video/mac/video_capture_device_avfoundation_mac.mm

 // Copyright 2013 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.
 
 #import "media/capture/video/mac/video_capture_device_avfoundation_mac.h"
 
 #import <CoreVideo/CoreVideo.h>
+#import <CoreMedia/CoreMedia.h>

[mcasas, 2015/11/02 18:59:18]

Imports, same as includes, should follow alphabetical
order. So, swap l.7 and 8.

[qiangchen, 2015/11/02 21:35:47]

Done.

 
 #include "base/logging.h"
 #include "base/mac/foundation_util.h"
 #include "media/base/video_capture_types.h"
 #include "media/capture/video/mac/video_capture_device_mac.h"
 #include "ui/gfx/geometry/size.h"
 
 // Prefer MJPEG if frame width or height is larger than this.
 static const int kMjpegWidthThreshold = 640;
 static const int kMjpegHeightThreshold = 480;
@@ skipping 303 matching lines @@
       baseAddress = static_cast<char*>(CVPixelBufferGetBaseAddress(videoFrame));
       frameSize = CVPixelBufferGetHeight(videoFrame) *
                   CVPixelBufferGetBytesPerRow(videoFrame);
     } else {
       videoFrame = nil;
     }
   }
 
   {
     base::AutoLock lock(lock_);
+    const CoreMediaGlue::CMTime cm_timestamp =
+        CoreMediaGlue::CMSampleBufferGetPresentationTimeStamp(sampleBuffer);
+    base::TimeTicks timestamp;
+    if ((cm_timestamp.flags & kCMTimeFlags_Valid) && cm_timestamp.timescale) {

[mcasas, 2015/11/02 18:59:18]

What about 

  const base::TimeTicks timestamp =
      CMTIME_IS_INVALID(cm_timestamp)
          ? base::TimeTicks::Now()
          : base::TimeTicks() + base::TimeDelta::FromMicroseconds(
                                    cm_timestamp.value *
                                    base::TimeTicks::kMicrosecondsPerSecond /
                                    cm_timestamp.timescale);

I would think that |cm_timestamp.timescale != 0| 
is included in the validity.

Or even better:

  const base::TimeTicks timestamp =
      CMTIME_IS_VALID(cm_timestamp)
          ? base::TimeTicks() +
                base::TimeDelta::FromSecondsD(CMTimeGetSeconds(cm_timestamp))
          : base::TimeTicks::Now();

Which is more readable IMHO, WDYT?

[qiangchen, 2015/11/02 21:35:47]

Done.

But not using CMTimeGetSeconds, because CoreMediaGlue::CMTime is not the apple
defined CMTime, but we redefined the same struct in coremedia_glue.h. Not sure
why, but in order to use a CM*** function, we have to expose it from the library
in coremedia_glue.h(and .cc) in a tedious way as we did for
CMGetPresentationTime, which I do not think necessary for just time format
conversion.

+      timestamp =
+          base::TimeTicks() +
+          base::TimeDelta::FromMicroseconds(
+              cm_timestamp.value * base::TimeTicks::kMicrosecondsPerSecond /
+              cm_timestamp.timescale);
+    } else {
+      timestamp = base::TimeTicks::Now();
+    }
     if (frameReceiver_ && baseAddress) {
       frameReceiver_->ReceiveFrame(reinterpret_cast<uint8_t*>(baseAddress),
-                                   frameSize, captureFormat, 0, 0);
+                                   frameSize, captureFormat, 0, 0, timestamp);
     }
   }
 
   if (videoFrame)
     CVPixelBufferUnlockBaseAddress(videoFrame, kCVPixelBufferLock_ReadOnly);
 }
 
 - (void)onVideoError:(NSNotification*)errorNotification {
   NSError* error = base::mac::ObjCCast<NSError>([[errorNotification userInfo]
       objectForKey:AVFoundationGlue::AVCaptureSessionErrorKey()]);
   [self sendErrorString:[NSString
                             stringWithFormat:@"%@: %@",
                                              [error localizedDescription],
                                              [error localizedFailureReason]]];
 }
 
 - (void)sendErrorString:(NSString*)error {
   DLOG(ERROR) << [error UTF8String];
   base::AutoLock lock(lock_);
   if (frameReceiver_)
     frameReceiver_->ReceiveError([error UTF8String]);
 }
 
 @end