Fix VideoLayerImpl upload of YUV->RGBA converted frames.

cc/layers/video_layer_impl.cc

 // Copyright 2011 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 "cc/layers/video_layer_impl.h"
 
 #include "base/logging.h"
 #include "cc/base/math_util.h"
 #include "cc/layers/quad_sink.h"
 #include "cc/layers/video_frame_provider_client_impl.h"
@@ skipping 348 matching lines @@
 
 void VideoLayerImpl::FramePlane::FreeData(ResourceProvider* resource_provider) {
   if (!resource_id)
     return;
 
   resource_provider->DeleteResource(resource_id);
   resource_id = 0;
 }
 
 // Convert media::VideoFrame::Format to OpenGL enum values.
-static GLenum ConvertVFCFormatToGLenum(const media::VideoFrame& frame) {
-  switch (frame.format()) {
+static GLenum ConvertVFCFormatToGLenum(const media::VideoFrame::Format format) {
+  switch (format) {
     case media::VideoFrame::YV12:
     case media::VideoFrame::YV16:
       return GL_LUMINANCE;
+    case media::VideoFrame::RGB32:
+      return GL_RGBA;
     case media::VideoFrame::NATIVE_TEXTURE:
-      return frame.texture_target();
 #if defined(GOOGLE_TV)
     case media::VideoFrame::HOLE:

[danakj, 2013/03/21 22:38:31]

i don't see what prevents this from being called with HOLE?

[sheu, 2013/03/21 22:42:26]

NumPlanes will return 0 for HOLE, so SetupFramePlanes() won't get this far.

[danakj, 2013/03/21 22:47:53]

Ah okay.

-      return GL_INVALID_VALUE;
 #endif
     case media::VideoFrame::INVALID:
-    case media::VideoFrame::RGB32:
     case media::VideoFrame::EMPTY:
     case media::VideoFrame::I420:
       NOTREACHED();
       break;
   }
   return GL_INVALID_VALUE;
 }
 
-size_t VideoLayerImpl::NumPlanes() const {
-  if (!frame_)
-    return 0;
-
-  if (convert_yuv_)
-    return 1;
-
-  return media::VideoFrame::NumPlanes(frame_->format());
-}
-
 bool VideoLayerImpl::SetupFramePlanes(ResourceProvider* resource_provider) {
-  const size_t plane_count = NumPlanes();
+  const size_t plane_count = media::VideoFrame::NumPlanes(format_);
   if (!plane_count)
     return true;
 
   const int max_texture_size = resource_provider->max_texture_size();
-  const GLenum pixel_format = ConvertVFCFormatToGLenum(*frame_);
+  const GLenum pixel_format = ConvertVFCFormatToGLenum(format_);
   for (size_t plane_index = 0; plane_index < plane_count; ++plane_index) {
     VideoLayerImpl::FramePlane* plane = &frame_planes_[plane_index];
 
     gfx::Size required_texture_size = VideoFrameDimension(frame_, plane_index);
     // TODO: Remove the test against max_texture_size when tiled layers are
     // implemented.
     if (required_texture_size.IsEmpty() ||
         required_texture_size.width() > max_texture_size ||
         required_texture_size.height() > max_texture_size)
       return false;
@@ skipping 40 matching lines @@
   }
   return true;
 }
 
 void VideoLayerImpl::FreeFramePlanes(ResourceProvider* resource_provider) {
   for (size_t i = 0; i < media::VideoFrame::kMaxPlanes; ++i)
     frame_planes_[i].FreeData(resource_provider);
 }
 
 void VideoLayerImpl::FreeUnusedFramePlanes(ResourceProvider* resource_provider) {
-  size_t first_unused_plane = NumPlanes();
+  size_t first_unused_plane = (frame_ ? media::VideoFrame::NumPlanes(format_) :

[danakj, 2013/03/21 22:38:31]

how about just an early out if !frame_?

[sheu, 2013/03/21 22:42:26]

We'll still want to deallocate all the planes, for example if the previous frame
had planes, but the format changed for this frame to one that doesn't have
planes.

[danakj, 2013/03/21 22:47:53]

I see! Can you wrap this a little differently, by moving the ":" to the next
line just below the "?"

[sheu, 2013/03/21 22:50:39]

That's the way I would have done things, but I wasn't sure about the style nits
:-P

Done.

+                               0);
   for (size_t i = first_unused_plane; i < media::VideoFrame::kMaxPlanes; ++i)
     frame_planes_[i].FreeData(resource_provider);
 }
 
 void VideoLayerImpl::DidLoseOutputSurface() {
   FreeFramePlanes(layer_tree_impl()->resource_provider());
 }
 
 void VideoLayerImpl::SetNeedsRedraw() {
   layer_tree_impl()->SetNeedsRedraw();
 }
 
 void VideoLayerImpl::SetProviderClientImpl(
     scoped_refptr<VideoFrameProviderClientImpl> provider_client_impl) {
   provider_client_impl_ = provider_client_impl;
 }
 
 const char* VideoLayerImpl::LayerTypeAsString() const {
   return "VideoLayer";
 }
 
 }  // namespace cc