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 269 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->coded_size());

[Ami GONE FROM CHROMIUM, 2012/11/16 22:15:43]

lolwat?

[sheu, 2012/11/17 00:35:39]

You're looking at a CL 2 revisions old :-)

     switch (frame->format()) {
     case media::VideoFrame::YV12:
         if (plane != media::VideoFrame::kYPlane) {
             dimensions.set_width(dimensions.width() / 2);
             dimensions.set_height(dimensions.height() / 2);
         }
         break;
     case media::VideoFrame::YV16:
         if (plane != media::VideoFrame::kYPlane) {
             dimensions.set_width(dimensions.width() / 2);
@@ skipping 64 matching lines @@
         ResourceProvider::ScopedWriteLockSoftware lock(resourceProvider, plane.resourceId);
         m_videoRenderer->Paint(m_frame, lock.skCanvas(), m_frame->visible_rect(), 0xFF);
         return true;
     }
 
     for (size_t planeIndex = 0; planeIndex < planeCount; ++planeIndex) {
         VideoLayerImpl::FramePlane& plane = m_framePlanes[planeIndex];
         // Only non-FormatNativeTexture planes should need upload.
         DCHECK_EQ(plane.format, GL_LUMINANCE);
         const uint8_t* softwarePlanePixels = m_frame->data(planeIndex);
-        gfx::Rect planeRect(gfx::Point(), plane.size);
-        resourceProvider->setPixels(plane.resourceId, softwarePlanePixels, planeRect, planeRect, gfx::Vector2d());
+        gfx::Rect imageRect(0, 0, m_frame->stride(planeIndex), plane.size.height());

[Ami GONE FROM CHROMIUM, 2012/11/16 22:15:43]

since this is going to copy anyway why not crop the stride/padding at this
point?

[sheu, 2012/11/17 00:35:39]

Using m_frame->stride() here is just slightly hacky, since the frame data size
should be exactly coded_size.  So when I do the upload, I make the size
coded_size to "true things up" again.

I suppose I could upload to visible_rect, and not have to do scaling on render,
but then the texture size no longer reflects coded_size, and I'd like to avoid
that.

[Ami GONE FROM CHROMIUM, 2012/11/17 00:41:37]

Why?  ISTM the cleanest thing would be to make the texture reflect visible_rect;
and make *this* the only place that needs to think about VideoFrame::stride().
Having more than one place in cc:: know about media::'s notions of stride seems
like a recipe for later confusion (and possibly missing cases now).

[sheu, 2012/11/17 01:50:38]

This is the only place that knows about stride now.  If you diff against the
base now, it's also the only three lines changed in the file.  So, with this
change:

coded_size refers to the frame data size, just like everywhere else in the code.
visible_rect refers to the visible rect, just like everywhere else in the code.

If I pre-cropped, then visible_rect would be incorrect in the case that there
was an x,y offset, and coded_size would refer to a size larger than the actual
frame data size.

+        gfx::Rect sourceRect(gfx::Point(), plane.size);
+        resourceProvider->setPixels(plane.resourceId, softwarePlanePixels, imageRect, sourceRect, gfx::Vector2d());
     }
     return true;
 }
 
 void VideoLayerImpl::freePlaneData(ResourceProvider* resourceProvider)
 {
     for (size_t i = 0; i < media::VideoFrame::kMaxPlanes; ++i)
         m_framePlanes[i].freeData(resourceProvider);
 }
 
@@ skipping 28 matching lines @@
 {
     layerTreeHostImpl()->setNeedsRedraw();
 }
 
 const char* VideoLayerImpl::layerTypeAsString() const
 {
     return "VideoLayer";
 }
 
 }  // namespace cc