Remove redundant I420-to-YV12 conversion in MediaStreamVideoTrack.

cc/resources/video_resource_updater.cc

 // 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.
 
 #include "cc/resources/video_resource_updater.h"
 
 #include "base/bind.h"
 #include "cc/output/gl_renderer.h"
 #include "cc/resources/resource_provider.h"
 #include "gpu/GLES2/gl2extchromium.h"
@@ skipping 43 matching lines @@
     return CreateForHardwarePlanes(video_frame);
   else
     return CreateForSoftwarePlanes(video_frame);
 }
 
 bool VideoResourceUpdater::VerifyFrame(
     const scoped_refptr<media::VideoFrame>& video_frame) {
   switch (video_frame->format()) {
     // Acceptable inputs.
     case media::VideoFrame::YV12:
+    case media::VideoFrame::I420:

[danakj, 2014/04/02 17:37:57]

Won't you need to make some changes in the gl_renderer/software_renderer, or
elsewhere to get the planes in the correct order?

Can you add a pixel test with this video format? See
https://code.google.com/p/chromium/codesearch#chromium/src/cc/output/renderer...

[hshi1, 2014/04/02 17:47:34]

No changes are necessary in gl_renderer/software_renderer because they all go
through media::VideoFrame::data(size_t plane) - which handles YV12 and I420
correctly - to obtain the pointers to the U and V planes. They never rely on any
assumption on how Y,U and V planes are ordered in memory.

For example see skcanvas_video_renderer.cc: function ConvertVideoFrameToBitmap()
calls media::ConvertYUVToRGB32() using separate
video_frame->data(media::VideoFrame::kYPlane),
video_frame->data(media::VideoFrame::kUPlane) and
video_frame->data(media::VideoFrame::kVPlane) pointers.

For example see gl_video_renderer.cc: GlVideoRenderer::Paint() sets up
glTexImage2D for each of the Y,U,V planes, also using video_frame->data(i).

That's why I claimed that YV12 and I420 are effectively the same. It only makes
a difference if the renderer tries to manually calculate U and V plane addresses
based on surface width/height and the base address, which it should NEVER do.

[hshi1, 2014/04/02 17:51:17]

Thanks for the suggestion and I looked at the renderer_pixeltest.cc, however I'm
afraid this is already testing at a lower level and the test itself is agnostic
of the "media" video formats (e.g. YV12 vs I420). You can see in the test that
it doesn't use media::VideoFrame class. The CreateTestYUVVideoDrawQuad() sets up
the Y, U and V planes directly.

[danakj, 2014/04/02 17:51:59]

Ahh, okay. Gotcha now. The pixel test I pointed to would be no different then
since it's not using VideoResourceUpdater.

We should have some pixel-test coverage of this to ensure the
VideoResourceUpdater does the right thing forever. Think you can modify those
tests to use VideoResourceUpdater instead of doing it manually?

     case media::VideoFrame::YV12A:
     case media::VideoFrame::YV16:
     case media::VideoFrame::YV12J:
     case media::VideoFrame::NATIVE_TEXTURE:
 #if defined(VIDEO_HOLE)
     case media::VideoFrame::HOLE:
 #endif  // defined(VIDEO_HOLE)
       return true;
 
     // Unacceptable inputs. ¯\(°_o)/¯
     case media::VideoFrame::UNKNOWN:
-    case media::VideoFrame::I420:
       break;
   }
   return false;
 }
 
 // For frames that we receive in software format, determine the dimensions of
 // each plane in the frame.
 static gfx::Size SoftwarePlaneDimension(
     const scoped_refptr<media::VideoFrame>& input_frame,
     ResourceFormat output_resource_format,
@@ skipping 14 matching lines @@
 #if defined(VIDEO_HOLE)
   if (input_frame_format == media::VideoFrame::HOLE) {
     VideoFrameExternalResources external_resources;
     external_resources.type = VideoFrameExternalResources::HOLE;
     return external_resources;
   }
 #endif  // defined(VIDEO_HOLE)
 
   // Only YUV software video frames are supported.
   DCHECK(input_frame_format == media::VideoFrame::YV12 ||
+         input_frame_format == media::VideoFrame::I420 ||
          input_frame_format == media::VideoFrame::YV12A ||
          input_frame_format == media::VideoFrame::YV12J ||
          input_frame_format == media::VideoFrame::YV16);
   if (input_frame_format != media::VideoFrame::YV12 &&
+      input_frame_format != media::VideoFrame::I420 &&
       input_frame_format != media::VideoFrame::YV12A &&
       input_frame_format != media::VideoFrame::YV12J &&
       input_frame_format != media::VideoFrame::YV16)
     return VideoFrameExternalResources();
 
   bool software_compositor = context_provider_ == NULL;
 
   ResourceFormat output_resource_format = kYUVResourceFormat;
   size_t output_plane_count = media::VideoFrame::NumPlanes(input_frame_format);
 
@@ skipping 228 matching lines @@
   }
 
   PlaneResource recycled_resource(data.resource_id,
                                   data.resource_size,
                                   data.resource_format,
                                   data.mailbox);
   updater->recycled_resources_.push_back(recycled_resource);
 }
 
 }  // namespace cc