Summary: | [GStreamer] Add support to copy YUV video textures into platform textures | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Chris Lord <clord> | ||||||||||||
Component: | WPE WebKit | Assignee: | Chris Lord <clord> | ||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||
Severity: | Normal | CC: | bugs-noreply, calvaris, commit-queue, ggaren, magomez, sabouhallawa, simon.fraser, ysuzuki, zan | ||||||||||||
Priority: | P2 | ||||||||||||||
Version: | WebKit Local Build | ||||||||||||||
Hardware: | Unspecified | ||||||||||||||
OS: | Unspecified | ||||||||||||||
See Also: |
https://bugs.webkit.org/show_bug.cgi?id=201184 https://bugs.webkit.org/show_bug.cgi?id=201422 |
||||||||||||||
Bug Depends on: | 132869 | ||||||||||||||
Bug Blocks: | |||||||||||||||
Attachments: |
|
Description
Chris Lord
2019-08-20 01:24:01 PDT
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. |