Created attachment 389674 [details]
Patch

Comment on attachment 389674 [details]
Patch

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

> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:3172
> +    auto internalCompositingOperation = [this](TextureMapperPlatformLayerProxy& proxy, std::unique_ptr<GstVideoFrameHolder> frameHolder) {

You're moving the frameholder when calling this, so I guess you should make this && as well.

Anyway, why do you create a lambda instead of a method considering that we are only capturing this?

Comment on attachment 389674 [details]
Patch

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

> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:218
> +        static std::once_flag onceFlag;
> +        std::call_once(onceFlag, [] {

Nit: I'd recommend using `s_onceFlag` as the name, the `s_` prefix more explicitly describing the storage nature.

> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:372
> +        return m_dmabufFD;

This should test for m_dmabufFD being >= 0, i.e. a valid fd. Related to this, m_dmabufFD should be initialized to -1 and in handoffVideoDmaBuf() reset to -1 after the hand-off and closure.

> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:3203
> +            std::unique_ptr<GstVideoFrameHolder> frameHolder = makeUnique<GstVideoFrameHolder>(m_sample.get(), m_videoDecoderPlatform, m_textureMapperFlags, !m_isUsingFallbackVideoSink);
> +            if (frameHolder->hasDMABuf()) {
> +                std::unique_ptr<TextureMapperPlatformLayerBuffer> layerBuffer = makeUnique<TextureMapperPlatformLayerBuffer>(0, m_size, TextureMapperGL::ShouldNotBlend, GL_DONT_CARE);

Could use `auto object = makeUnique<>()` in both cases.

Comment on attachment 389674 [details]
Patch

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

>> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:3172
>> +    auto internalCompositingOperation = [this](TextureMapperPlatformLayerProxy& proxy, std::unique_ptr<GstVideoFrameHolder> frameHolder) {
> 
> You're moving the frameholder when calling this, so I guess you should make this && as well.
> 
> Anyway, why do you create a lambda instead of a method considering that we are only capturing this?

I don't really see the point of adding a method for this. As the logic remains self-contained in pushTextureToCompositor() anyway. This new closure is an implementation detail of it.

Committed r255790: <https://trac.webkit.org/changeset/255790>

<rdar://problem/59182626>