Fix VideoLayerImpl upload of YUV->RGBA converted frames.

cc/layers/video_layer_impl.cc

Index: cc/layers/video_layer_impl.cc
diff --git a/cc/layers/video_layer_impl.cc b/cc/layers/video_layer_impl.cc
index fbef0a5bc855939c21c3868c289e52e3ce261586..8c0d8f579febecb97fbf2e727f5e5de02b71fd68 100644
--- a/cc/layers/video_layer_impl.cc
+++ b/cc/layers/video_layer_impl.cc
@@ -366,19 +366,18 @@ void VideoLayerImpl::FramePlane::FreeData(ResourceProvider* resource_provider) {
 }
 
 // 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();
@@ -387,23 +386,13 @@ static GLenum ConvertVFCFormatToGLenum(const media::VideoFrame& frame) {
   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];
 
@@ -464,7 +453,8 @@ void VideoLayerImpl::FreeFramePlanes(ResourceProvider* 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);
 }