Plumb through cropped output size for VideoFrame

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 86cf037eb8baa87676451710629a73fdde0bc619..f9cf075721ebccaf7bd86c93dbc8ddbc288f067b 100644
--- a/cc/video_layer_impl.cc
+++ b/cc/video_layer_impl.cc
@@ -122,6 +122,12 @@ void VideoLayerImpl::willDrawInternal(ResourceProvider* resourceProvider)
     if (!m_frame)
         return;
 
+    // If these fail, we'll have to add draw logic that handles offset bitmap/
+    // texture UVs.
+    DCHECK(m_frame->visibleX() == 0 && m_frame->visibleY() == 0);

[jamesr, 2012/10/25 00:26:34]

Can you make these separate DCHECK()s and use DCHECK_EQ() so if they do fail
it'll be possible to see what is actually different?

[Ami GONE FROM CHROMIUM, 2012/10/25 00:43:07]

DCHECKs are for programming errors, not creatively produced files downloaded
from the web.
If a file from the web can end up triggering this then it should be an error-out
condition.

[sheu, 2012/10/25 01:53:54]

Updated the comment a bit.  It's not really an error, it's just that we've never
handled that case before and I'd like to explicitly document that.  We could
easily add the code to handle it, but there's not been a requirement yet.

+
+    // The visible contents might not cover the entire texture; e.g. padding

[Ami GONE FROM CHROMIUM, 2012/10/25 00:43:07]

This comment seems oddly placed.

[sheu, 2012/10/25 01:53:54]

Done.

+
     m_format = convertVFCFormatToGC3DFormat(*m_frame);
 
     if (m_format == GL_INVALID_VALUE) {
@@ -167,23 +173,29 @@ void VideoLayerImpl::appendQuads(QuadSink& quadSink, AppendQuadsData& appendQuad
 
     IntRect quadRect(IntPoint(), contentBounds());
 
+    // pixels for macroblocked formats.
+    const float texWidthScale =
+        static_cast<float>(m_frame->visibleWidth()) / m_frame->textureWidth();
+    const float texHeightScale =
+        static_cast<float>(m_frame->visibleHeight()) / m_frame->textureHeight();
+
     switch (m_format) {
     case GL_LUMINANCE: {
         // YUV software decoder.

[Ami GONE FROM CHROMIUM, 2012/10/25 00:43:07]

I don't believe it's possible to get here with scales != 1.
DCHECK that?

[sheu, 2012/10/25 01:53:54]

It's not an error condition though, and we can handle it.

         const FramePlane& yPlane = m_framePlanes[WebKit::WebVideoFrame::yPlane];
         const FramePlane& uPlane = m_framePlanes[WebKit::WebVideoFrame::uPlane];
         const FramePlane& vPlane = m_framePlanes[WebKit::WebVideoFrame::vPlane];
-        scoped_ptr<YUVVideoDrawQuad> yuvVideoQuad = YUVVideoDrawQuad::create(sharedQuadState, quadRect, yPlane, uPlane, vPlane);
+        FloatSize texScale(texWidthScale, texHeightScale);
+        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[WebKit::WebVideoFrame::rgbPlane];
-        float widthScaleFactor = static_cast<float>(plane.visibleSize.width()) / plane.size.width();
-
         bool premultipliedAlpha = true;
-        FloatRect uvRect(0, 0, widthScaleFactor, 1);
+        FloatRect uvRect(0, 0, texWidthScale, texHeightScale);
         bool flipped = false;
         scoped_ptr<TextureDrawQuad> textureQuad = TextureDrawQuad::create(sharedQuadState, quadRect, plane.resourceId, premultipliedAlpha, uvRect, flipped);
         quadSink.append(textureQuad.PassAs<DrawQuad>(), appendQuadsData);
@@ -192,21 +204,26 @@ void VideoLayerImpl::appendQuads(QuadSink& quadSink, AppendQuadsData& appendQuad
     case GL_TEXTURE_2D: {
         // NativeTexture hardware decoder.
         bool premultipliedAlpha = true;
-        FloatRect uvRect(0, 0, 1, 1);
+        FloatRect uvRect(0, 0, texWidthScale, texHeightScale);
         bool flipped = false;
         scoped_ptr<TextureDrawQuad> textureQuad = TextureDrawQuad::create(sharedQuadState, quadRect, m_externalTextureResource, premultipliedAlpha, uvRect, flipped);
         quadSink.append(textureQuad.PassAs<DrawQuad>(), appendQuadsData);
         break;
     }
     case GL_TEXTURE_RECTANGLE_ARB: {
-        IntSize textureSize(m_frame->width(), m_frame->height()); 
-        scoped_ptr<IOSurfaceDrawQuad> ioSurfaceQuad = IOSurfaceDrawQuad::create(sharedQuadState, quadRect, textureSize, m_frame->textureId(), IOSurfaceDrawQuad::Unflipped);
+        IntSize visibleSize(m_frame->visibleWidth(), m_frame->visibleHeight());
+        scoped_ptr<IOSurfaceDrawQuad> ioSurfaceQuad = IOSurfaceDrawQuad::create(sharedQuadState, quadRect, visibleSize, m_frame->textureId(), IOSurfaceDrawQuad::Unflipped);
         quadSink.append(ioSurfaceQuad.PassAs<DrawQuad>(), appendQuadsData);
         break;
     }
     case GL_TEXTURE_EXTERNAL_OES: {
         // StreamTexture hardware decoder.
-        scoped_ptr<StreamVideoDrawQuad> streamVideoQuad = StreamVideoDrawQuad::create(sharedQuadState, quadRect, m_frame->textureId(), m_streamTextureMatrix);
+        WebKit::WebTransformationMatrix transform(m_streamTextureMatrix);
+        transform.scaleNonUniform(texWidthScale, texHeightScale);
+        scoped_ptr<StreamVideoDrawQuad> streamVideoQuad =
+            StreamVideoDrawQuad::create(sharedQuadState, quadRect,
+                                        m_frame->textureId(),
+                                        m_streamTextureMatrix);
         quadSink.append(streamVideoQuad.PassAs<DrawQuad>(), appendQuadsData);
         break;
     }
@@ -239,43 +256,8 @@ void VideoLayerImpl::didDraw(ResourceProvider* resourceProvider)
     m_providerLock.Release();
 }
 
-static int videoFrameDimension(int originalDimension, unsigned plane, int format)
-{
-    if (format == WebKit::WebVideoFrame::FormatYV12 && plane != WebKit::WebVideoFrame::yPlane)
-        return originalDimension / 2;
-    return originalDimension;
-}
-
-static bool hasPaddingBytes(const WebKit::WebVideoFrame& frame, unsigned plane)
-{
-    return frame.stride(plane) > videoFrameDimension(frame.width(), plane, frame.format());
-}
-
-IntSize VideoLayerImpl::computeVisibleSize(const WebKit::WebVideoFrame& frame, unsigned plane)
-{
-    int visibleWidth = videoFrameDimension(frame.width(), plane, frame.format());
-    int originalWidth = visibleWidth;
-    int visibleHeight = videoFrameDimension(frame.height(), plane, frame.format());
-
-    // When there are dead pixels at the edge of the texture, decrease
-    // the frame width by 1 to prevent the rightmost pixels from
-    // interpolating with the dead pixels.
-    if (hasPaddingBytes(frame, plane))
-        --visibleWidth;
-
-    // In YV12, every 2x2 square of Y values corresponds to one U and
-    // one V value. If we decrease the width of the UV plane, we must decrease the
-    // width of the Y texture by 2 for proper alignment. This must happen
-    // always, even if Y's texture does not have padding bytes.
-    if (plane == WebKit::WebVideoFrame::yPlane && frame.format() == WebKit::WebVideoFrame::FormatYV12) {
-        if (hasPaddingBytes(frame, WebKit::WebVideoFrame::uPlane))
-            visibleWidth = originalWidth - 2;
-    }
-
-    return IntSize(visibleWidth, visibleHeight);
-}
-
-bool VideoLayerImpl::FramePlane::allocateData(ResourceProvider* resourceProvider)
+bool VideoLayerImpl::FramePlane::allocateData(
+    ResourceProvider* resourceProvider)
 {
     if (resourceId)
         return true;
@@ -296,12 +278,16 @@ void VideoLayerImpl::FramePlane::freeData(ResourceProvider* resourceProvider)
 bool VideoLayerImpl::allocatePlaneData(ResourceProvider* resourceProvider)
 {
     int maxTextureSize = resourceProvider->maxTextureSize();
-    for (unsigned planeIndex = 0; planeIndex < m_frame->planes(); ++planeIndex) {
-        VideoLayerImpl::FramePlane& plane = m_framePlanes[planeIndex];
-
-        IntSize requiredTextureSize(m_frame->stride(planeIndex), videoFrameDimension(m_frame->height(), planeIndex, m_frame->format()));
-        // FIXME: Remove the test against maxTextureSize when tiled layers are implemented.
-        if (requiredTextureSize.isZero() || requiredTextureSize.width() > maxTextureSize || requiredTextureSize.height() > maxTextureSize)
+    for (unsigned planeIdx = 0; planeIdx < m_frame->planes(); ++planeIdx) {
+        VideoLayerImpl::FramePlane& plane = m_framePlanes[planeIdx];
+
+        IntSize requiredTextureSize(m_frame->textureWidth(),
+                                    m_frame->textureHeight());
+        // FIXME: Remove the test against maxTextureSize when tiled layers are
+        // implemented.
+        if (requiredTextureSize.isZero() ||
+            requiredTextureSize.width() > maxTextureSize ||
+            requiredTextureSize.height() > maxTextureSize)
             return false;
 
         if (plane.size != requiredTextureSize || plane.format != m_format) {
@@ -310,26 +296,27 @@ bool VideoLayerImpl::allocatePlaneData(ResourceProvider* resourceProvider)
             plane.format = m_format;
         }
 
-        if (!plane.resourceId) {
-            if (!plane.allocateData(resourceProvider))
-                return false;
-            plane.visibleSize = computeVisibleSize(*m_frame, planeIndex);
-        }
+        if (!plane.allocateData(resourceProvider))
+            return false;
     }
     return true;
 }
 
 bool VideoLayerImpl::copyPlaneData(ResourceProvider* resourceProvider)
 {
-    size_t softwarePlaneCount = m_frame->planes();
-    if (!softwarePlaneCount)
-        return true;
-
-    for (size_t softwarePlaneIndex = 0; softwarePlaneIndex < softwarePlaneCount; ++softwarePlaneIndex) {
-        VideoLayerImpl::FramePlane& plane = m_framePlanes[softwarePlaneIndex];
-        const uint8_t* softwarePlanePixels = static_cast<const uint8_t*>(m_frame->data(softwarePlaneIndex));
-        IntRect planeRect(IntPoint(), plane.size);
-        resourceProvider->upload(plane.resourceId, softwarePlanePixels, planeRect, planeRect, IntSize());
+    for (unsigned planeIdx = 0; planeIdx < m_frame->planes(); ++planeIdx) {
+        VideoLayerImpl::FramePlane& plane = m_framePlanes[planeIdx];
+        const uint8_t* planePixels = static_cast<const uint8_t*>(
+            m_frame->data(planeIdx));

[jamesr, 2012/10/25 00:26:34]

General note for stuff in src/cc/ - we're still using 4-space indents here and
need to reformat, so don't worry about being too careful to hit 80col if it
breaks the line in an odd place.  we'll bulk reformat the whole directory at
some point

[sheu, 2012/10/25 01:53:54]

Done.

+
+        // Only non-FormatNativeTexture planes should need upload.
+        DCHECK(plane.format == GL_LUMINANCE);

[jamesr, 2012/10/25 00:26:34]

DCHECK_EQ please, it'll print the format if the check fails

[sheu, 2012/10/25 01:53:54]

Done.

+
+        IntRect planeRect(0, 0, plane.size.width(), plane.size.height());
+        IntRect visibleRect(0, 0, m_frame->textureWidth(),
+                            m_frame->textureHeight());
+        resourceProvider->upload(plane.resourceId, planePixels, planeRect,
+                                 visibleRect, IntSize());
     }
     return true;
 }