Verify returned frames from media::VideoFrame::Wrap*() methods

media/video/gpu_memory_buffer_video_frame_pool.cc

 // Copyright 2015 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.
 
 #include "media/video/gpu_memory_buffer_video_frame_pool.h"
 
 #include <GLES2/gl2.h>
 #include <GLES2/gl2ext.h>
 
 #include <algorithm>
@@ skipping 562 matching lines @@
   // Create the VideoFrame backed by native textures.
   gfx::Size visible_size = video_frame->visible_rect().size();
   switch (output_format_) {
     case PIXEL_FORMAT_I420:
       frame = VideoFrame::WrapYUV420NativeTextures(
           mailbox_holders[VideoFrame::kYPlane],
           mailbox_holders[VideoFrame::kUPlane],
           mailbox_holders[VideoFrame::kVPlane], release_mailbox_callback,
           coded_size, gfx::Rect(visible_size), video_frame->natural_size(),
           video_frame->timestamp());
-      if (video_frame->metadata()->IsTrue(VideoFrameMetadata::ALLOW_OVERLAY))
+      if (frame &&
+          video_frame->metadata()->IsTrue(VideoFrameMetadata::ALLOW_OVERLAY))

[Pawel Osciak, 2015/11/26 01:19:03]

I'm wondering why this check, I don't think WrapYUV420NativeTextures() would
ever set it to true from what I can see in the code?

Also, wondering why here we need to check (and likely always fail since I think
it looks like it would always be false), but not check for NV12 and UYVY below?

Perhaps we should extract this and l.594 out of switch and always set allow
overlay.

[emircan, 2015/12/04 04:20:09]

It is checking the |video_frame| metadata rather than the new |frame| metadata.
|video_frame| might have it as true.

         frame->metadata()->SetBoolean(VideoFrameMetadata::ALLOW_OVERLAY, true);
       break;
     case PIXEL_FORMAT_NV12:
     case PIXEL_FORMAT_UYVY:
       frame = VideoFrame::WrapNativeTexture(
           output_format_, mailbox_holders[VideoFrame::kYPlane],
           release_mailbox_callback, coded_size, gfx::Rect(visible_size),
           video_frame->natural_size(), video_frame->timestamp());
-      frame->metadata()->SetBoolean(VideoFrameMetadata::ALLOW_OVERLAY, true);
+      if (frame)
+        frame->metadata()->SetBoolean(VideoFrameMetadata::ALLOW_OVERLAY, true);
       break;
     default:
       NOTREACHED();
   }
 
+  if (!frame) {
+    release_mailbox_callback.Run(gpu::SyncToken());
+    frame_ready_cb.Run(video_frame);
+    return;
+  }
+
   base::TimeTicks render_time;
   if (video_frame->metadata()->GetTimeTicks(VideoFrameMetadata::REFERENCE_TIME,
                                             &render_time)) {
     frame->metadata()->SetTimeTicks(VideoFrameMetadata::REFERENCE_TIME,
                                     render_time);
   }
 
   frame_ready_cb.Run(frame);
 }
 
@@ skipping 130 matching lines @@
 }
 
 void GpuMemoryBufferVideoFramePool::MaybeCreateHardwareFrame(
     const scoped_refptr<VideoFrame>& video_frame,
     const FrameReadyCB& frame_ready_cb) {
   DCHECK(video_frame);
   pool_impl_->CreateHardwareFrame(video_frame, frame_ready_cb);
 }
 
 }  // namespace media