YUV software decode path stride fixes.

cc/video_layer_impl.cc

Index: cc/video_layer_impl.cc
diff --git a/cc/video_layer_impl.cc b/cc/video_layer_impl.cc
index 9930a0c4eac8bd4b7e0dac633d74d82951699d99..6d671adf97e699c22e36e53ced004895a3422e1d 100644
--- a/cc/video_layer_impl.cc
+++ b/cc/video_layer_impl.cc
@@ -204,9 +204,9 @@ void VideoLayerImpl::appendQuads(QuadSink& quadSink, AppendQuadsData& appendQuad
     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) {
@@ -215,7 +215,11 @@ void VideoLayerImpl::appendQuads(QuadSink& quadSink, AppendQuadsData& appendQuad
         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);
@@ -287,18 +291,12 @@ void VideoLayerImpl::didDraw(ResourceProvider* resourceProvider)
 }
 
 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;