Bug 132869 adds shaders to render YUV video textures to avoid having to render to an extra off-screen surface. The patch on that bug only implements the playback portion and YUV videos become broken/non-working on accelerated and unaccelerated canvases. This bug tracks the work to enable YUV video textures in accelerated canvases (so WebGL and accelerated 2D canvas).
Created attachment 376753 [details] Patch
Created attachment 376858 [details] Patch
Comment on attachment 376858 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=376858&action=review Besides my review, I guess this should be checked by Miguel or Zan as well. > Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:219 > + std::unique_ptr<TextureMapperPlatformLayerBuffer> getPlatformLayerBuffer(bool treatAsRGB) platformLayerBuffer and shouldBeTreatedAsRGB > Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:229 > + Buffer::TextureVariant { Buffer::RGBTexture { *static_cast<GLuint*>(m_videoFrame.data[0]) } }, > + m_size, m_flags, GraphicsContext3D::RGBA); Nit: this can be just one line. > Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:236 > + int nPlanes = GST_VIDEO_INFO_N_PLANES(&m_videoFrame.info); numberOfPlanes > Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:239 > + std::array<int, 3> yuvPOffset; yuvPlaneOffset? > Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:255 > + // Default to bt601. This is the same behaviour as GStreamer's glcolorconvert element Period at the end. > Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:266 > + return makeUnique<Buffer>( > + Buffer::TextureVariant { > + Buffer::YUVTexture { nPlanes, planes, yuvPlane, yuvPOffset, yuvToRgb } }, > + m_size, m_flags, GraphicsContext3D::RGBA); Line lengths need to be reworked here. I think all could fit it one. > Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:998 > + frameHolder->SyncWaitCPU(); Where is this defined? besides the name should be a bit more explanatory (I guess adding more words will help) > Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:1030 > + frameHolder->SyncWaitCPU(); ditto > Source/WebCore/platform/graphics/gstreamer/VideoTextureCopierGStreamer.cpp:164 > + WTF::switchOn(inputTexture.getTextureVariant(), > + [&](const Buffer::RGBTexture&) > + { These three lines could be collapsed into one. Of course we'd need to reindent the function. > Source/WebCore/platform/graphics/gstreamer/VideoTextureCopierGStreamer.cpp:167 > + }, > + [&](const Buffer::YUVTexture& texture) Collapse. > Source/WebCore/platform/graphics/gstreamer/VideoTextureCopierGStreamer.cpp:226 > + [&](const Buffer::RGBTexture& texture) > + { lines > Source/WebCore/platform/graphics/gstreamer/VideoTextureCopierGStreamer.cpp:236 > + [&](const Buffer::YUVTexture& texture) > + { lines > Source/WebCore/platform/graphics/texmap/TextureMapperPlatformLayerBuffer.h:94 > + const TextureVariant& getTextureVariant() { return m_variant; } textureVariant
Created attachment 377334 [details] Patch
Comment on attachment 377334 [details] Patch Beautiful! Let's see what Miguel or Zan have to say about this cause they know more of the subject.
(In reply to Xabier Rodríguez Calvar from comment #5) > Comment on attachment 377334 [details] > Patch > > Beautiful! Let's see what Miguel or Zan have to say about this cause they > know more of the subject. LGTM. Awesome work Chris!
Created attachment 377883 [details] Patch
Created attachment 377891 [details] Patch
Comment on attachment 377891 [details] Patch Clearing flags on attachment: 377891 Committed r249428: <https://trac.webkit.org/changeset/249428>
All reviewed patches have been landed. Closing bug.