Mac: Use AVSampleBufferDisplayLayer for 4:2:0 surfaces

content/common/gpu/ca_layer_tree_mac.mm

Index: content/common/gpu/ca_layer_tree_mac.mm
diff --git a/content/common/gpu/ca_layer_tree_mac.mm b/content/common/gpu/ca_layer_tree_mac.mm
index 49d0218f9b9fa9ba6621ff772aa527c987176d31..a150d72122c5640fc9cadc7af31d7f87f68fa814 100644
--- a/content/common/gpu/ca_layer_tree_mac.mm
+++ b/content/common/gpu/ca_layer_tree_mac.mm
@@ -4,6 +4,9 @@
 
 #include "content/common/gpu/ca_layer_tree_mac.h"
 
+#include <AVFoundation/AVFoundation.h>
+#include <CoreVideo/CoreVideo.h>
+
 #include "base/command_line.h"
 #include "base/mac/sdk_forward_declarations.h"
 #include "base/trace_event/trace_event.h"
@@ -13,8 +16,99 @@
 #include "ui/base/ui_base_switches.h"
 #include "ui/gfx/geometry/dip_util.h"
 

[ccameron, 2016/03/23 08:57:10]

I tried putting this into sdk_forward_declarations.h and that seemed to be out
of control (it started pulling in headers from CoreVideo, CoreMedia, and
AVFoundation) ... this localized version seems more reasonable to me.

+#if !defined(MAC_OS_X_VERSION_10_8) || \
+    MAC_OS_X_VERSION_MIN_REQUIRED < MAC_OS_X_VERSION_10_8

[erikchen, 2016/03/23 17:44:59]

I assume this will no longer be required after we bump the deployment target to
10.8+?

[ccameron, 2016/03/23 18:26:21]

Exactly.

+BASE_EXPORT extern NSString* const AVLayerVideoGravityResize;

[erikchen, 2016/03/23 17:45:00]

BASE_EXPORT?

[ccameron, 2016/03/23 18:26:21]

Mmh, should have removed that when I removed these from base. Gone now.

+BASE_EXPORT extern "C" void NSAccessibilityPostNotificationWithUserInfo(
+    id object,
+    NSString* notification,
+    NSDictionary* user_info);
+BASE_EXPORT extern "C" OSStatus CMSampleBufferCreateForImageBuffer(
+    CFAllocatorRef,
+    CVImageBufferRef,
+    Boolean dataReady,
+    CMSampleBufferMakeDataReadyCallback,
+    void*,
+    CMVideoFormatDescriptionRef,
+    const CMSampleTimingInfo*,
+    CMSampleBufferRef*);
+BASE_EXPORT extern "C" CFArrayRef CMSampleBufferGetSampleAttachmentsArray(
+    CMSampleBufferRef,
+    Boolean);
+BASE_EXPORT extern "C" OSStatus CMVideoFormatDescriptionCreateForImageBuffer(
+    CFAllocatorRef,
+    CVImageBufferRef,
+    CMVideoFormatDescriptionRef*);
+BASE_EXPORT extern "C" CMTime CMTimeMake(int64_t, int32_t);
+BASE_EXPORT extern CFStringRef const kCMSampleAttachmentKey_DisplayImmediately;
+BASE_EXPORT extern const CMTime kCMTimeInvalid;
+#endif  // MAC_OS_X_VERSION_10_8
+
 namespace content {
 
+namespace {
+
+bool AVSampleBufferDisplayLayerEnqueueIOSurface(

[erikchen, 2016/03/23 17:45:00]

Can you add a comment on the ownership semantics of this function? It looks like
on success, io_surface gets retained by cv_pixel_buffer, which in turn becomes
owned by av_layer?

[ccameron, 2016/03/23 18:26:21]

Added a comment. I just said that the io_surface is retained and marked in-use
until it is displayed. In practice cv_pixel_buffer is an intermediate in that.

+    AVSampleBufferDisplayLayer* av_layer,
+    IOSurfaceRef io_surface) {
+  OSStatus os_status = noErr;
+  CVReturn cv_return = kCVReturnSuccess;
+
+  base::ScopedCFTypeRef<CFDictionaryRef> pixel_buffer_attributes;
+  base::ScopedCFTypeRef<CVPixelBufferRef> cv_pixel_buffer;
+  cv_return = CVPixelBufferCreateWithIOSurface(
+      kCFAllocatorDefault, io_surface, pixel_buffer_attributes.get(),

[erikchen, 2016/03/23 17:44:59]

Isn't pixel_buffer_attributes.get() always nullptr?

[erikchen, 2016/03/23 17:45:00]

shouldn't we pass null to use the defualt allocator? Mostly looking for
consistency between this and CMVideoFormatDescriptionCreateForImageBuffer

[ccameron, 2016/03/23 18:26:20]

I spent so long trying to put random things in that dictionary that I didn't
want to forget it completely. nullptr now.

[ccameron, 2016/03/23 18:26:21]

Changed to nullptr.

+      cv_pixel_buffer.InitializeInto());
+  if (cv_return) {

[erikchen, 2016/03/23 17:45:00]

I think you want cv_return != kCVReturnSuccess

[ccameron, 2016/03/23 18:26:20]

Done (kCVReturnSuccess == 0).

+    LOG(ERROR) << "CVPixelBufferCreateWithIOSurface failed with " << cv_return;
+    return false;
+  }
+
+  base::ScopedCFTypeRef<CMVideoFormatDescriptionRef> video_info;
+  os_status = CMVideoFormatDescriptionCreateForImageBuffer(
+      NULL, cv_pixel_buffer, video_info.InitializeInto());
+  if (os_status) {

[erikchen, 2016/03/23 17:45:00]

use proper conditional here and later in function.

[ccameron, 2016/03/23 18:26:21]

Done.

+    LOG(ERROR) << "CMVideoFormatDescriptionCreateForImageBuffer failed with "
+               << os_status;
+    return false;
+  }
+
+  CMTime frame_time = CMTimeMake(0, 30);

[erikchen, 2016/03/23 17:44:59]

Where did this number 30 come from?

[ccameron, 2016/03/23 18:26:21]

Added a comment saying that frame time doesn't matter because we're displaying
immediately. And changed the denominator to 1.

+  CMSampleTimingInfo timing_info = {frame_time, frame_time, kCMTimeInvalid};
+
+  base::ScopedCFTypeRef<CMSampleBufferRef> sample_buffer = NULL;

[erikchen, 2016/03/23 17:45:00]

s/NULL/nullptr

[ccameron, 2016/03/23 18:26:21]

Done.

+  os_status = CMSampleBufferCreateForImageBuffer(
+      kCFAllocatorDefault, cv_pixel_buffer, YES, NULL, NULL, video_info,
+      &timing_info, sample_buffer.InitializeInto());
+  if (os_status) {
+    LOG(ERROR) << "CMSampleBufferCreateForImageBuffer failed with "
+               << os_status;
+    return false;
+  }
+
+  CFArrayRef attachments =
+      CMSampleBufferGetSampleAttachmentsArray(sample_buffer, YES);
+  if (!attachments) {
+    LOG(ERROR) << "CMSampleBufferGetSampleAttachmentsArray failed";
+    return false;
+  }
+  CFMutableDictionaryRef attachments_dictionary =

[erikchen, 2016/03/23 17:45:00]

Check that array has size >=1 first

[ccameron, 2016/03/23 18:26:20]

Done.

+      reinterpret_cast<CFMutableDictionaryRef>(
+          const_cast<void*>(CFArrayGetValueAtIndex(attachments, 0)));
+  if (!attachments_dictionary) {
+    LOG(ERROR) << "Failed to get attachments dictionary";
+    return false;
+  }
+  CFDictionarySetValue(attachments_dictionary,
+                       kCMSampleAttachmentKey_DisplayImmediately,
+                       kCFBooleanTrue);
+
+  [av_layer enqueueSampleBuffer:sample_buffer];
+  return true;
+}
+
+}  // namespace
+
 CALayerTree::CALayerTree() {}
 CALayerTree::~CALayerTree() {}
 
@@ -154,6 +248,14 @@ CALayerTree::ContentLayer::ContentLayer(
   // marked as InUse.
   if (io_surface)
     IOSurfaceIncrementUseCount(io_surface);
+
+  // Only allow 4:2:0 frames which fill the layer's contents to be promoted to
+  // AV layers.
+  if (IOSurfaceGetPixelFormat(io_surface) ==
+          kCVPixelFormatType_420YpCbCr8BiPlanarVideoRange &&
+      contents_rect == gfx::RectF(0, 0, 1, 1)) {
+    use_av_layer = true;
+  }
 }
 
 CALayerTree::ContentLayer::ContentLayer(ContentLayer&& layer)
@@ -163,9 +265,12 @@ CALayerTree::ContentLayer::ContentLayer(ContentLayer&& layer)
       background_color(layer.background_color),
       ca_edge_aa_mask(layer.ca_edge_aa_mask),
       opacity(layer.opacity),
-      ca_layer(layer.ca_layer) {
+      ca_layer(layer.ca_layer),
+      av_layer(layer.av_layer),
+      use_av_layer(layer.use_av_layer) {
   DCHECK(!layer.ca_layer);
   layer.ca_layer.reset();
+  layer.av_layer.reset();
   // See remarks in the non-move constructor.
   if (io_surface)
     IOSurfaceIncrementUseCount(io_surface);
@@ -388,9 +493,10 @@ void CALayerTree::ContentLayer::CommitToCA(CALayer* superlayer,
   bool update_background_color = true;
   bool update_ca_edge_aa_mask = true;
   bool update_opacity = true;
-  if (old_layer) {
+  if (old_layer && old_layer->use_av_layer == use_av_layer) {
     DCHECK(old_layer->ca_layer);
     std::swap(ca_layer, old_layer->ca_layer);
+    std::swap(av_layer, old_layer->av_layer);
     update_contents = old_layer->io_surface != io_surface;
     update_contents_rect = old_layer->contents_rect != contents_rect;
     update_rect = old_layer->rect != rect;
@@ -398,7 +504,13 @@ void CALayerTree::ContentLayer::CommitToCA(CALayer* superlayer,
     update_ca_edge_aa_mask = old_layer->ca_edge_aa_mask != ca_edge_aa_mask;
     update_opacity = old_layer->opacity != opacity;
   } else {
-    ca_layer.reset([[CALayer alloc] init]);
+    if (use_av_layer) {
+      av_layer.reset([[AVSampleBufferDisplayLayer alloc] init]);
+      ca_layer.reset([av_layer retain]);
+      [av_layer setVideoGravity:AVLayerVideoGravityResize];
+    } else {
+      ca_layer.reset([[CALayer alloc] init]);

[erikchen, 2016/03/23 17:44:59]

Shouldn't we reset av_layer as well [to release a reference?]

[ccameron, 2016/03/23 18:26:20]

We construct a new CALayerTree every frame (yeah, crazy, I know), so everything
here gets re-set every frame. If it's going to have a CALayer, it needs allocate
it or pull it from |old_layer|.

+    }
     [ca_layer setAnchorPoint:CGPointZero];
     [superlayer addSublayer:ca_layer];
   }
@@ -406,14 +518,18 @@ void CALayerTree::ContentLayer::CommitToCA(CALayer* superlayer,
   bool update_anything = update_contents || update_contents_rect ||
                          update_rect || update_background_color ||
                          update_ca_edge_aa_mask || update_opacity;
-
-  if (update_contents) {
-    [ca_layer setContents:static_cast<id>(io_surface.get())];
-    if ([ca_layer respondsToSelector:(@selector(setContentsScale:))])
-      [ca_layer setContentsScale:scale_factor];
+  if (use_av_layer) {
+    if (update_contents)
+      AVSampleBufferDisplayLayerEnqueueIOSurface(av_layer, io_surface);
+  } else {
+    if (update_contents) {
+      [ca_layer setContents:static_cast<id>(io_surface.get())];
+      if ([ca_layer respondsToSelector:(@selector(setContentsScale:))])
+        [ca_layer setContentsScale:scale_factor];
+    }
+    if (update_contents_rect)

[erikchen, 2016/03/23 17:45:00]

What if we're using a new av layer and need to update the contents rect (to
become 0, 0, 1, 1)? 

[ccameron, 2016/03/23 18:26:20]

We only use_av_layer when the content rect is (0,0,1,1) -- we may be able to
relax this, but I didn't bother with it.

If the content rect changed from not-(0,0,1,1) to (0,0,1,1), then
"old_layer->use_av_layer == use_av_layer" will make us create a new layer and
set everything again.

+      [ca_layer setContentsRect:contents_rect.ToCGRect()];
   }
-  if (update_contents_rect)
-    [ca_layer setContentsRect:contents_rect.ToCGRect()];
   if (update_rect) {
     gfx::RectF dip_rect = gfx::RectF(rect);
     dip_rect.Scale(1 / scale_factor);
@@ -441,8 +557,13 @@ void CALayerTree::ContentLayer::CommitToCA(CALayer* superlayer,
   if (show_borders) {
     base::ScopedCFTypeRef<CGColorRef> color;
     if (update_anything) {
-      // Pink represents a CALayer that changed this frame.
-      color.reset(CGColorCreateGenericRGB(1, 0, 1, 1));
+      if (use_av_layer) {
+        // Green represents an AV layer that changed this frame.
+        color.reset(CGColorCreateGenericRGB(0, 1, 0, 1));
+      } else {
+        // Pink represents a CALayer that changed this frame.
+        color.reset(CGColorCreateGenericRGB(1, 0, 1, 1));
+      }
     } else {
       // Grey represents a CALayer that has not changed.
       color.reset(CGColorCreateGenericRGB(0, 0, 0, 0.1));