YUV software decode path stride fixes.

cc/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/video_layer_impl.h"
 
 #include "base/logging.h"
 #include "cc/io_surface_draw_quad.h"
 #include "cc/layer_tree_host_impl.h"
 #include "cc/quad_sink.h"
@@ skipping 186 matching lines @@
     appendDebugBorderQuad(quadSink, sharedQuadState, appendQuadsData);
 
     // FIXME: When we pass quads out of process, we need to double-buffer, or
     // otherwise synchonize use of all textures in the quad.
 
     gfx::Rect quadRect(gfx::Point(), contentBounds());
     gfx::Rect visibleRect = m_frame->visible_rect();
     gfx::Size codedSize = m_frame->coded_size();
 
     // pixels for macroblocked formats.
-    const float texWidthScale =
+    float texWidthScale =
         static_cast<float>(visibleRect.width()) / codedSize.width();
-    const float texHeightScale =
+    float texHeightScale =

[Ami GONE FROM CHROMIUM, 2012/11/15 18:15:36]

Undo these changes?

[sheu, 2012/11/15 19:46:38]

They're applicable for the rest of the paths, so I think it would be good to
calculate them once here.  The YUV software decoder path is an exception.

[Ami GONE FROM CHROMIUM, 2012/11/15 20:39:09]

My point was that all you changed was dropping the "const" but then you never
reassign.  So AFAICT dropping the const is not useful.

[sheu, 2012/11/15 22:20:53]

Ah right, no-op change.  Fixed.

         static_cast<float>(visibleRect.height()) / codedSize.height();
 
     switch (m_format) {
     case GL_LUMINANCE: {
         // YUV software decoder.
         const FramePlane& yPlane = m_framePlanes[media::VideoFrame::kYPlane];
         const FramePlane& uPlane = m_framePlanes[media::VideoFrame::kUPlane];
         const FramePlane& vPlane = m_framePlanes[media::VideoFrame::kVPlane];
-        gfx::SizeF texScale(texWidthScale, texHeightScale);
+        // YUV software decoder uses CPU-allocated textures that have extra
+        // padding past what coded_size_ reflects.

[Ami GONE FROM CHROMIUM, 2012/11/15 18:15:36]

I don't see how this CL changes things; how is yPlane.size not exactly equal to
codedSize?

[sheu, 2012/11/15 19:46:38]

See video_frame.cc, where we do some extra alignment to 16 bytes for
software-allocated YUV frames.

It would be better to get coded_size to match up, but that's more changes that
would complicate the immediate fix.

[Ami GONE FROM CHROMIUM, 2012/11/15 20:39:09]

I somehow missed the thunk below in videoFrameDimension.  Oops.

+        gfx::SizeF texScale(
+            static_cast<float>(visibleRect.width()) / yPlane.size.width(),

[Ami GONE FROM CHROMIUM, 2012/11/15 20:39:09]

What is this supposed to be scaling from/to?

My assumption _was_ that this is meant to scale from a VideoFrame's
visible_size() to a <video>'s height/width.  But AFAICT nothing in this class
knows about the web-visible dimensions, instead everything is keyed off the
VideoFrame's idea of its own size(s).
I must be misunderstanding something, because ISTM that (say) if you had a
visible width of 20 pixels and a stride of 200 pixels (because why not), this
would end up with a scaling factor of 0.1; something's gonna be multiplied or
divided by 10.  What is that something?

[sheu, 2012/11/15 22:20:53]

We're not scaling to <video> dimensions -- that's handled by quadRect, which is
constructed from contentBounds().  We're scaling the video texture that we're
sampling from, since the borders of the video texture might have extraneous data
that we don't use.  So with a stride of 200 pixels, and a visible width of 20,
we're scaling with a factor of 0.1, regardless of the <video> size; the size of
the <video> will just affect how big that 10% of the video texture we do use is
displayed in the final composited output.

+            static_cast<float>(visibleRect.height()) / yPlane.size.height());
         scoped_ptr<YUVVideoDrawQuad> yuvVideoQuad = YUVVideoDrawQuad::create(
             sharedQuadState, quadRect, texScale, yPlane, uPlane, vPlane);
         quadSink.append(yuvVideoQuad.PassAs<DrawQuad>(), appendQuadsData);
         break;
     }
     case GL_RGBA: {
         // RGBA software decoder.
         const FramePlane& plane = m_framePlanes[media::VideoFrame::kRGBPlane];
         bool premultipliedAlpha = true;
         gfx::RectF uvRect(0, 0, texWidthScale, texHeightScale);
@@ skipping 51 matching lines @@
         m_externalTextureResource = 0;
     }
 
     m_provider->putCurrentFrame(m_webFrame);
     m_frame = 0;
 
     m_providerLock.Release();
 }
 
 static gfx::Size videoFrameDimension(media::VideoFrame* frame, int plane) {
-    gfx::Size dimensions = frame->coded_size();
+    gfx::Size dimensions = gfx::Size(frame->stride(plane),
+                                     frame->coded_size().height());
     switch (frame->format()) {
     case media::VideoFrame::YV12:
-        if (plane != media::VideoFrame::kYPlane) {
-            dimensions.set_width(dimensions.width() / 2);
+        if (plane != media::VideoFrame::kYPlane)
             dimensions.set_height(dimensions.height() / 2);
-        }
-        break;
-    case media::VideoFrame::YV16:
-        if (plane != media::VideoFrame::kYPlane) {
-            dimensions.set_width(dimensions.width() / 2);
-        }
         break;
     default:
         break;
     }
     return dimensions;
 }
 
 bool VideoLayerImpl::FramePlane::allocateData(
     ResourceProvider* resourceProvider)
 {
@@ skipping 103 matching lines @@
 {
     layerTreeHostImpl()->setNeedsRedraw();
 }
 
 const char* VideoLayerImpl::layerTypeAsString() const
 {
     return "VideoLayer";
 }
 
 }  // namespace cc