Bug 47283 – [Chromium] Use RGBA instead of Luminance textures for YUV2RGB conversion

• Component: New Bugs
• Status: RESOLVED WONTFIX

[Victoria Kirst]

Use RGBA instead of Luminance textures for YUV2RGB conversion

[Victoria Kirst]

Created attachment 69965 [details]
Patch

[Kenneth Russell]

Comment on attachment 69965 [details]
Patch

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

> WebCore/platform/graphics/chromium/VideoLayerChromium.cpp:113
> +        "  vec4 attrib_y = attrib_matrix[y_index];            \n"

FYI, this indexing operation is not guaranteed to work on OpenGL ES 2.0 hardware. If you look at http://www.khronos.org/registry/gles/specs/2.0/GLSL_ES_Specification_1.0.17.pdf , Appendix A, Section 5, you will see that the only requirement is to support indexing expressions involving constants and loop indices. Is it possible to rephrase this logic (tersely) to work within this restriction?

[Victoria Kirst]

(In reply to comment #2)
> FYI, this indexing operation is not guaranteed to work on OpenGL ES 2.0 hardware. 

Thanks for the info! I have figured out a terse way to do it using only vectors and math, but it is pretty mathy :) I can upload the patch!

[Victoria Kirst]

Created attachment 69995 [details]
Patch

[Hin-Chung Lam]

Comment on attachment 69995 [details]
Patch

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

> WebKit/chromium/src/VideoFrameChromiumImpl.cpp:119
> +        unsigned packedWidth = (stride(plane) + 3) / 4;

Do so will affect the code path for rendering textures directly. I think this method is more getting the properties of a video frame, the logic for getting the texture width should really be in VideoLayerChromium.

[Kenneth Russell]

Victoria, is this patch still current and are you still looking for review?

[Adam Barth]

Comment on attachment 69995 [details]
Patch

Clearing the review flag while we wait for a response to kbr's question above.  Please feel free to re-nominate this patch if you'd like it reviewed.

[Victoria Kirst]

No need to review! Patch no longer relevant. Thanks for the reminder!