Bug 45069

Summary: VideoLayerChromium uses GPU YUV to RGB color conversion
Product: WebKit Reporter: Victoria Kirst <vrk>
Component: New BugsAssignee: Victoria Kirst <vrk>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, hclam, jamesr, kbr, scherkus, tony, vangelis, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Other   
OS: OS X 10.5   
Bug Depends on: 45912    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch none

Description Victoria Kirst 2010-09-01 16:18:24 PDT
VideoLayerChromium uses GPU YUV to RGB color conversion
Comment 1 Victoria Kirst 2010-09-01 16:19:57 PDT
Created attachment 66298 [details]
Patch
Comment 2 Victoria Kirst 2010-09-01 16:25:18 PDT
Hi Vangelis!

Can you take a look at my code utilizing your changes to LayerRendererChromium? It no longer uses the GraphicsContext and instead gets frames from Chromium through the plumbing I created in the last few weeks.

One thing to note: I have only implemented cases for YV12 and RGBA frames, and actually I have not tested the implementation for RGBA frames as I believe we only encounter YV12 frames in the chromium code right now. But we will be using RGBA frames once hardware decode is working, and I wanted to give you an example of what the code looks like when there are multiple file formats implemented.

Thanks, and let me know whatever questions you have!
Comment 3 WebKit Review Bot 2010-09-01 16:26:21 PDT
Attachment 66298 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1
WebCore/platform/graphics/chromium/VideoLayerChromium.cpp:300:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
WebCore/platform/graphics/chromium/VideoLayerChromium.cpp:311:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Total errors found: 2 in 7 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 4 Vangelis Kokkevis 2010-09-08 23:26:54 PDT
Comment on attachment 66298 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=66298&action=prettypatch

Looks good!  A few comments inline.

> WebCore/platform/graphics/chromium/VideoLayerChromium.cpp:90
> +        "  gl_FragColor = vec4(rgb, 1);                       \n"
I think you still need to pass in an alpha value to the shader so that we can get respect the layer's opacity.

> WebCore/platform/graphics/chromium/VideoLayerChromium.cpp:185
> +    if (!frame)
Shouldn't the required texture size be the size of the frame rather than m_bounds?  m_bounds reflects the size that the layer will be displayed at which could be different than the frame size. You actually seem to ignore the requiredTextureSize value passed into the update*Contents method anyway.

> WebCore/platform/graphics/chromium/VideoLayerChromium.cpp:215
> +    glActiveTexture(GL_TEXTURE0);
If you remove the glActiveTexture calls from the update methods then you won't need to call this here either.

> WebCore/platform/graphics/chromium/VideoLayerChromium.cpp:228
> +            glDeleteTextures(1, &m_vTextureId);
This could be somewhat optimized if you put all 3 texture values in an array and call glDeleteTextures(3, array).  Also, for good measure, maybe put glDeleteTextures within a GLC() macro?

> WebCore/platform/graphics/chromium/VideoLayerChromium.cpp:241
> +void VideoLayerChromium::createYUVTextures(VideoFrameChromium* frame, const IntSize& requiredTextureSize,
requiredTextureSize isn't used in this method

> WebCore/platform/graphics/chromium/VideoLayerChromium.cpp:246
> +    GLC(glActiveTexture(GL_TEXTURE1));
I don't believe you need to call glActiveTexture here and below.

> WebCore/platform/graphics/chromium/VideoLayerChromium.cpp:259
> +void VideoLayerChromium::updateYUVTextures(VideoFrameChromium* frame, const IntRect& updateRect)
Do you ever do partial frame updates or is the updateRect always the same as the size of the frame? I'm wondering whether this parameter could be eliminated.

> WebCore/platform/graphics/chromium/VideoLayerChromium.cpp:263
> +    GLC(glActiveTexture(GL_TEXTURE1));
Same here:  glActiveTexture isn't necessary.

> WebCore/platform/graphics/chromium/VideoLayerChromium.cpp:296
> +void VideoLayerChromium::createTexture(unsigned textureId, const void* data, unsigned width, unsigned height, GLenum format)
Maybe better to name this method allocateTexture or initializeTexture as it doesn't really create a texture?

> WebCore/platform/graphics/chromium/VideoLayerChromium.cpp:303
> +void VideoLayerChromium::updateTexture(unsigned textureId, const void* data, unsigned xOffset, unsigned yOffset,
it might be more convenient to pass an IntRect here instead of 4 separate values

> WebCore/platform/graphics/chromium/VideoLayerChromium.cpp:343
> +    glActiveTexture(GL_TEXTURE0);
Maybe this call could be moved in the drawYUV method as drawRGB doesn't change the active texture.

> WebCore/platform/graphics/chromium/VideoLayerChromium.h:49
>      virtual void updateContents();
You should probably also define a destructor which calls glDeleteTexture on all the textures used by the layer, otherwise they will never be freed.
Comment 5 Victoria Kirst 2010-09-13 11:32:19 PDT
Created attachment 67440 [details]
Patch
Comment 6 WebKit Review Bot 2010-09-13 11:35:05 PDT
Attachment 67440 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1
WebCore/platform/graphics/chromium/VideoLayerChromium.cpp:307:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
WebCore/platform/graphics/chromium/VideoLayerChromium.cpp:317:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Total errors found: 2 in 7 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 7 Victoria Kirst 2010-09-13 12:34:09 PDT
Created attachment 67456 [details]
Patch
Comment 8 Victoria Kirst 2010-09-13 13:29:03 PDT
Thanks so much for the helpful comments, Vangelis! I fixed my code to fit all of your suggestions.

I also made a small but important change: in my original patch, I had forgot to address the stride of the video frames! Unfortunately, it doesn't seem like there is an easy way to set the stride for the texture in GLES2... the way I've seen this done in other GL code is by calling the command glPixelStorei(GL_UNPACK_ALIGNMENT, <stride>);, but GLES2 doesn't recognize GL_UNPACK_ALIGNMENT. So instead what I do is I allocate textures to the width of the stride, copy the entire data into the texture, then in the vertex shader I adjust the x component of the v_texCoord accordingly to clip off the padding between the edge of the frame and the stride. (Big thanks to Alpha for the idea!) I was planning to talk to Gregg and James to see if they could get the GL_UNPACK_ALIGNMENT parameter working, but if that's not possible, I think this is an okay solution.

Please take a look at my patch again and let me know what you think!

(In reply to comment #4)
> > WebCore/platform/graphics/chromium/VideoLayerChromium.cpp:90
> I think you still need to pass in an alpha value to the shader so that we can get respect the layer's opacity.
Done.

> > WebCore/platform/graphics/chromium/VideoLayerChromium.cpp:185
> Shouldn't the required texture size be the size of the frame rather than m_bounds?  m_bounds reflects the size that the layer will be displayed at which could be different than the frame size.
Done.

> > WebCore/platform/graphics/chromium/VideoLayerChromium.cpp:215
> If you remove the glActiveTexture calls from the update methods then you won't need to call this here either.
Done (as well as other places mentioned).

> > WebCore/platform/graphics/chromium/VideoLayerChromium.cpp:228
> This could be somewhat optimized if you put all 3 texture values in an array and call glDeleteTextures(3, array).  Also, for good measure, maybe put glDeleteTextures within a GLC() macro?
Done.

> > WebCore/platform/graphics/chromium/VideoLayerChromium.cpp:259
> > +void VideoLayerChromium::updateYUVTextures(VideoFrameChromium* frame, const IntRect& updateRect)
> Do you ever do partial frame updates or is the updateRect always the same as the size of the frame? I'm wondering whether this parameter could be eliminated.
That is a good point! I don't think we ever do partial frame updates; the whole updateRect/dirtyFrame/etc stuff was mostly carryover from when I was copying most of my logic from LayerChromium. I modified the code (in here and other places as appropriate) to get rid of all the partial frame stuff.

> > WebCore/platform/graphics/chromium/VideoLayerChromium.cpp:296
> Maybe better to name this method allocateTexture or initializeTexture as it doesn't really create a texture?
Done here and elsewhere.

> > WebCore/platform/graphics/chromium/VideoLayerChromium.cpp:303
> > +void VideoLayerChromium::updateTexture(unsigned textureId, const void* data, unsigned xOffset, unsigned yOffset,
> it might be more convenient to pass an IntRect here instead of 4 separate values
I decided to pass an IntSize for the width and height since the xOffset/yOffset only makes sense for updating partial frames.

> > WebCore/platform/graphics/chromium/VideoLayerChromium.cpp:343
> > +    glActiveTexture(GL_TEXTURE0);
> Maybe this call could be moved in the drawYUV method as drawRGB doesn't change the active texture.
Done. 

> > WebCore/platform/graphics/chromium/VideoLayerChromium.h:49
> >      virtual void updateContents();
> You should probably also define a destructor which calls glDeleteTexture on all the textures used by the layer, otherwise they will never be freed.
Thanks for the catch! Done.
Comment 9 Vangelis Kokkevis 2010-09-20 12:42:22 PDT
Comment on attachment 67456 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=67456&action=prettypatch

Looks pretty good. Just a few inline comments.

> WebCore/platform/graphics/chromium/VideoLayerChromium.cpp:103
> +        "  gl_FragColor = vec4(rgb, alpha);                   \n"

The compositor assumes that layers use premultipled alpha so all the channels need to be multiplied by the alpha value.  You can look at ContentLayerChromium for an example.

> WebCore/platform/graphics/chromium/VideoLayerChromium.cpp:113
> +        "  gl_FragColor = vec4(texColor.x, texColor.y, texColor.z, texColor.w); \n"

Alpha here?

> WebCore/platform/graphics/chromium/VideoLayerChromium.cpp:184
> +    if (m_yuvTextures)

Here you should be testing if each individual texture is not 0 before trying to delete it.

> WebCore/platform/graphics/chromium/VideoLayerChromium.cpp:209
> +    IntSize requiredTextureSize(frame->stride(VideoFrameChromium::cYPlane), frame->height());

what about the case of an RGBA frame? Will this give you the right stride?

> WebCore/platform/graphics/chromium/VideoLayerChromium.cpp:248
> +            GLC(glDeleteTextures(3, static_cast<const GLuint*>(m_yuvTextures)));

Same as before.  You need to make sure that each individual texture id is not equal to zero before deleting.

> WebCore/platform/graphics/chromium/VideoLayerChromium.cpp:267
> +    IntSize uvDimensions(frame->stride(VideoFrameChromium::cUPlane), frame->height() / 2);

Does the integer division by 2 work ok for odd heights?

> WebCore/platform/graphics/chromium/VideoLayerChromium.cpp:288
> +    ASSERT(frame->stride(VideoFrameChromium::cYPlane) == m_allocatedTextureSize.width());

VideoFrameChromium::cRGBPlane ?

> WebCore/platform/graphics/chromium/VideoLayerChromium.cpp:294
> +            glDeleteTextures(1, &m_rgbaTexture);

If you already have a texture ID you can just call allocateTexture() with it and adjust it to its new size.  Is there a reason why you delete it and re-create it?

> WebCore/platform/graphics/chromium/VideoLayerChromium.cpp:295
> +        m_rgbaTexture = layerRenderer()->createLayerTexture();

I wonder if the logic here could be simplified by making allocateTexture do only the allocation (passing a NULL pointer for data) and then always calling the updateTexture. That way, if in the future updateTexture() does something different, you won't have to change how allocateTexture also sends its bits to the texture.

> WebCore/platform/graphics/chromium/VideoLayerChromium.cpp:317
> +        ASSERT(glGetError() == GL_NO_ERROR);

Any reason why you're not using the GLC() macro for these two gl calls but rather explicitly call glGetError() ?
Comment 10 Kenneth Russell 2010-09-20 15:32:23 PDT
Apologies, but https://bugs.webkit.org/show_bug.cgi?id=45912 will require this patch to be regenerated. The transformation to use GraphicsContext3D rather than the command buffer client GL calls directly should be mechanical. Please let me know if you have any problems making the changes.
Comment 11 Vangelis Kokkevis 2010-09-20 16:55:29 PDT
Comment on attachment 67456 [details]
Patch

One more thing that just occurred to me while looking at another bug:

View in context: https://bugs.webkit.org/attachment.cgi?id=67456&action=prettypatch

> WebCore/platform/graphics/chromium/VideoLayerChromium.h:88
> +    ~VideoLayerChromium();

The destructor should be virtual.
Comment 12 Victoria Kirst 2010-09-27 10:48:29 PDT
Created attachment 68932 [details]
Patch
Comment 13 Victoria Kirst 2010-09-27 10:56:20 PDT
Thanks for the review. I addressed all vangelis's comments and I also made the necessary adjustments to work with kbr's GraphicsContext3D changes. 

I also did a little bit of refactoring of my code to make things cleaner and more generalized. vangelis, I will send you an email briefly detailing these things in case they are unclear!
Comment 14 Vangelis Kokkevis 2010-09-27 13:44:14 PDT
Comment on attachment 68932 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=68932&action=review

Looking good.  Just a couple of small comments.

> WebCore/platform/graphics/chromium/VideoLayerChromium.cpp:216
> +        notImplemented();

Should you also be setting m_skipsDraw = true here?

> WebKit/chromium/src/VideoFrameChromiumImpl.cpp:39
> +const unsigned VideoFrameChromium::cMaxPlanes = 3;

Not sure if using the "c" prefix for constants is standard in WebKit.  Is it being used elsewhere too?

> WebKit/chromium/src/VideoFrameChromiumImpl.cpp:121
> +            return IntSize(stride(plane), height() / 2);

Does this work ok with both odd and even height values?
Comment 15 Victoria Kirst 2010-09-27 14:58:14 PDT
Created attachment 68968 [details]
Patch
Comment 16 Victoria Kirst 2010-09-27 15:02:54 PDT
(In reply to comment #14)
> > WebCore/platform/graphics/chromium/VideoLayerChromium.cpp:216
> > +        notImplemented();
> 
> Should you also be setting m_skipsDraw = true here?
Good catch, thanks! I should also be releasing the frame and returning from the function.

> > WebKit/chromium/src/VideoFrameChromiumImpl.cpp:39
> > +const unsigned VideoFrameChromium::cMaxPlanes = 3;
> 
> Not sure if using the "c" prefix for constants is standard in WebKit.  Is it being used elsewhere too?
I saw this here: https://cs.corp.google.com/p#chrome/trunk/src/third_party/WebKit/WebCore/platform/text/UnicodeRange.h and here: https://cs.corp.google.com/p#chrome/trunk/src/third_party/WebKit/WebCore/page/FrameView.cpp
But on second look, it seems like *not* using the c-prefix is far more standard. I modified the constants to omit the prefix.


> > WebKit/chromium/src/VideoFrameChromiumImpl.cpp:121
> > +            return IntSize(stride(plane), height() / 2);
> 
> Does this work ok with both odd and even height values?
Yes, should be fine!

Thanks again!
Comment 17 James Robinson 2010-09-27 15:28:57 PDT
Comment on attachment 68968 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=68968&action=review

r- for nitpicks but overall looks good

> WebCore/platform/graphics/chromium/VideoLayerChromium.cpp:44
> +const float VideoLayerChromium::cYUV2RGB[9] = {

add a comment here indicating what these values are. if they are just PFM then note that

> WebCore/platform/graphics/chromium/VideoLayerChromium.cpp:201
> +    VideoFrameChromium* frame = 0;

Why not just declare this in the initialization 3 lines below?

> WebCore/platform/graphics/chromium/VideoLayerChromium.cpp:202
> +    bool needsAllocate = false;

this variable looks unused

> WebCore/platform/graphics/chromium/VideoLayerChromium.cpp:270
> +        if (planeTextureSize != IntSize() && planeTextureSize != m_textureSizes[plane]) {

There are .isEmpty() and .isZero() functions on IntSize, please use one of those

> WebCore/platform/graphics/chromium/VideoLayerChromium.cpp:294
> +    } else
> +        m_skipsDraw = true;

Can you leave a FIXME or notImplemented() here or something?  mapTexSubImage2D can fail in the normal course of operation and we should handle it.

> WebCore/platform/graphics/chromium/VideoLayerChromium.h:89
> +    explicit VideoLayerChromium(GraphicsLayerChromium* owner, VideoFrameProvider* provider);

nit: no explicit for a two-argument constructor, don't name the provider argument
Comment 18 Victoria Kirst 2010-09-27 16:08:26 PDT
Created attachment 68985 [details]
Patch
Comment 19 Victoria Kirst 2010-09-27 16:11:23 PDT
(In reply to comment #17)
> (From update of attachment 68968 [details])

Thanks James! Fixed all that you suggested, and also changed "cYUV2RGB" to "yuv2RGB" to match the style of the other constants.
Comment 20 James Robinson 2010-09-27 16:16:17 PDT
Comment on attachment 68985 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=68985&action=review

R=me

> WebCore/platform/graphics/chromium/VideoLayerChromium.cpp:66
> +    , m_yuvWidthScaleFactorLocation(0)
> +    , m_rgbaShaderMatrixLocation(0)
> +    , m_rgbaWidthScaleFactorLocation(0)
> +    , m_ccMatrixLocation(0)
> +    , m_yTextureLocation(0)
> +    , m_uTextureLocation(0)
> +    , m_vTextureLocation(0)
> +    , m_rgbaTextureLocation(0)
> +    , m_yuvAlphaLocation(0)
> +    , m_rgbaAlphaLocation(0)

nit: should these initialize to -1?
Comment 21 WebKit Commit Bot 2010-09-27 16:48:03 PDT
Comment on attachment 68985 [details]
Patch

Clearing flags on attachment: 68985

Committed r68447: <http://trac.webkit.org/changeset/68447>
Comment 22 WebKit Commit Bot 2010-09-27 16:48:10 PDT
All reviewed patches have been landed.  Closing bug.
Comment 23 Tony Chang 2010-09-27 17:43:33 PDT
This seems to have broken the Chromium Mac build on the canary bots (just compiler warnings):
http://build.chromium.org/buildbot/waterfall.fyi/builders/Webkit%20Mac%20(webkit.org)/builds/25113/steps/compile/logs/stdio

distcc[35339] ERROR: compile /b/slave/webkit-rel-mac-webkit-org/build/src/third_party/WebKit/WebCore/WebCore.gyp/../platform/graphics/chromium/VideoLayerChromium.cpp on codf42.jail/10,lzo,cpp failed
distcc[35339] (dcc_build_somewhere) Warning: remote compilation of '/b/slave/webkit-rel-mac-webkit-org/build/src/third_party/WebKit/WebCore/WebCore.gyp/../platform/graphics/chromium/VideoLayerChromium.cpp' failed, retrying locally
distcc[35339] Warning: failed to distribute /b/slave/webkit-rel-mac-webkit-org/build/src/third_party/WebKit/WebCore/WebCore.gyp/../platform/graphics/chromium/VideoLayerChromium.cpp to codf42.jail/10,lzo,cpp, running locally instead
cc1plus: warnings being treated as errors
/b/slave/webkit-rel-mac-webkit-org/build/src/third_party/WebKit/WebCore/WebCore.gyp/../platform/graphics/chromium/VideoLayerChromium.h: In constructor 'WebCore::VideoLayerChromium::SharedValues::SharedValues(WebCore::GraphicsContext3D*)':
/b/slave/webkit-rel-mac-webkit-org/build/src/third_party/WebKit/WebCore/WebCore.gyp/../platform/graphics/chromium/VideoLayerChromium.h:76:warning: 'WebCore::VideoLayerChromium::SharedValues::m_yuvWidthScaleFactorLocation' will be initialized after
/b/slave/webkit-rel-mac-webkit-org/build/src/third_party/WebKit/WebCore/WebCore.gyp/../platform/graphics/chromium/VideoLayerChromium.h:75:warning:   'int WebCore::VideoLayerChromium::SharedValues::m_rgbaShaderMatrixLocation'
/b/slave/webkit-rel-mac-webkit-org/build/src/third_party/WebKit/WebCore/WebCore.gyp/../platform/graphics/chromium/VideoLayerChromium.cpp:52:warning:   when initialized here
/b/slave/webkit-rel-mac-webkit-org/build/src/third_party/WebKit/WebCore/WebCore.gyp/../platform/graphics/chromium/VideoLayerChromium.h:82:warning: 'WebCore::VideoLayerChromium::SharedValues::m_ccMatrixLocation' will be initialized after
/b/slave/webkit-rel-mac-webkit-org/build/src/third_party/WebKit/WebCore/WebCore.gyp/../platform/graphics/chromium/VideoLayerChromium.h:78:warning:   'int WebCore::VideoLayerChromium::SharedValues::m_yTextureLocation'
/b/slave/webkit-rel-mac-webkit-org/build/src/third_party/WebKit/WebCore/WebCore.gyp/../platform/graphics/chromium/VideoLayerChromium.cpp:52:warning:   when initialized here
/b/slave/webkit-rel-mac-webkit-org/build/src/third_party/WebKit/WebCore/WebCore.gyp/../platform/graphics/chromium/VideoLayerChromium.h: In constructor 'WebCore::VideoLayerChromium::VideoLayerChromium(WebCore::GraphicsLayerChromium*, WebCore::VideoFrameProvider*)':
/b/slave/webkit-rel-mac-webkit-org/build/src/third_party/WebKit/WebCore/WebCore.gyp/../platform/graphics/chromium/VideoLayerChromium.h:103:warning: 'WebCore::VideoLayerChromium::m_skipsDraw' will be initialized after
/b/slave/webkit-rel-mac-webkit-org/build/src/third_party/WebKit/WebCore/WebCore.gyp/../platform/graphics/chromium/VideoLayerChromium.h:101:warning:   'WebCore::VideoFrameChromium::Format WebCore::VideoLayerChromium::m_frameFormat'
/b/slave/webkit-rel-mac-webkit-org/build/src/third_party/WebKit/WebCore/WebCore.gyp/../platform/graphics/chromium/VideoLayerChromium.cpp:173:warning:   when initialized here
/b/slave/webkit-rel-mac-webkit-org/build/src/third_party/WebKit/WebCore/WebCore.gyp/../platform/graphics/chromium/VideoLayerChromium.cpp:179:warning: comparison between signed and unsigned integer expressions
/b/slave/webkit-rel-mac-webkit-org/build/src/third_party/WebKit/WebCore/WebCore.gyp/../platform/graphics/chromium/VideoLayerChromium.cpp: In destructor 'virtual WebCore::VideoLayerChromium::~VideoLayerChromium()':
/b/slave/webkit-rel-mac-webkit-org/build/src/third_party/WebKit/WebCore/WebCore.gyp/../platform/graphics/chromium/VideoLayerChromium.cpp:189:warning: comparison between signed and unsigned integer expressions
/b/slave/webkit-rel-mac-webkit-org/build/src/third_party/WebKit/WebCore/WebCore.gyp/../platform/graphics/chromium/VideoLayerChromium.cpp: In member function 'virtual void WebCore::VideoLayerChromium::updateContents()':
/b/slave/webkit-rel-mac-webkit-org/build/src/third_party/WebKit/WebCore/WebCore.gyp/../platform/graphics/chromium/VideoLayerChromium.cpp:232:warning: comparison between signed and unsigned integer expressions
/b/slave/webkit-rel-mac-webkit-org/build/src/third_party/WebKit/WebCore/WebCore.gyp/../platform/graphics/chromium/VideoLayerChromium.cpp: In static member function 'static unsigned int WebCore::VideoLayerChromium::determineTextureFormat(WebCore::VideoFrameChromium*)':
/b/slave/webkit-rel-mac-webkit-org/build/src/third_party/WebKit/WebCore/WebCore.gyp/../platform/graphics/chromium/VideoLayerChromium.cpp:244:warning: enumeration value 'Invalid' not handled in switch
/b/slave/webkit-rel-mac-webkit-org/build/src/third_party/WebKit/WebCore/WebCore.gyp/../platform/graphics/chromium/VideoLayerChromium.cpp:244:warning: enumeration value 'RGB555' not handled in switch
/b/slave/webkit-rel-mac-webkit-org/build/src/third_party/WebKit/WebCore/WebCore.gyp/../platform/graphics/chromium/VideoLayerChromium.cpp:244:warning: enumeration value 'RGB565' not handled in switch
/b/slave/webkit-rel-mac-webkit-org/build/src/third_party/WebKit/WebCore/WebCore.gyp/../platform/graphics/chromium/VideoLayerChromium.cpp:244:warning: enumeration value 'RGB24' not handled in switch
/b/slave/webkit-rel-mac-webkit-org/build/src/third_party/WebKit/WebCore/WebCore.gyp/../platform/graphics/chromium/VideoLayerChromium.cpp:244:warning: enumeration value 'RGB32' not handled in switch
/b/slave/webkit-rel-mac-webkit-org/build/src/third_party/WebKit/WebCore/WebCore.gyp/../platform/graphics/chromium/VideoLayerChromium.cpp:244:warning: enumeration value 'YV16' not handled in switch
/b/slave/webkit-rel-mac-webkit-org/build/src/third_party/WebKit/WebCore/WebCore.gyp/../platform/graphics/chromium/VideoLayerChromium.cpp:244:warning: enumeration value 'NV12' not handled in switch
/b/slave/webkit-rel-mac-webkit-org/build/src/third_party/WebKit/WebCore/WebCore.gyp/../platform/graphics/chromium/VideoLayerChromium.cpp:244:warning: enumeration value 'Empty' not handled in switch
/b/slave/webkit-rel-mac-webkit-org/build/src/third_party/WebKit/WebCore/WebCore.gyp/../platform/graphics/chromium/VideoLayerChromium.cpp:244:warning: enumeration value 'ASCII' not handled in switch
/b/slave/webkit-rel-mac-webkit-org/build/src/third_party/WebKit/WebCore/WebCore.gyp/../platform/graphics/chromium/VideoLayerChromium.cpp: In member function 'bool WebCore::VideoLayerChromium::allocateTexturesIfNeeded(WebCore::GraphicsContext3D*, WebCore::VideoFrameChromium*, unsigned int)':
/b/slave/webkit-rel-mac-webkit-org/build/src/third_party/WebKit/WebCore/WebCore.gyp/../platform/graphics/chromium/VideoLayerChromium.cpp:258:warning: comparison between signed and unsigned integer expressions
distcc[35339] ERROR: compile /b/slave/webkit-rel-mac-webkit-org/build/src/third_party/WebKit/WebCore/WebCore.gyp/../platform/graphics/chromium/VideoLayerChromium.cpp on localhost failed