Attachment 342657 Details for Bug 186590 – Patch

[patch] Patch

bug-186590-20180613161008.patch (text/plain), 7.94 KB, created by Philippe Normand on 2018-06-13 08:10:09 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Philippe Normand

Created: 2018-06-13 08:10:09 PDT

Size: 7.94 KB

patch

obsolete

>Subversion Revision: 232790
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 316a0e76163307c3ae51f801fe0ecdfb5a928b3d..7c95ecb61b6f3341fd08c2b4d4332c01861576e2 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,24 @@
>+2018-06-13  Philippe Normand  <pnormand@igalia.com>
>+
>+        [GStreamer] fast/canvas/webgl crashes
>+        https://bugs.webkit.org/show_bug.cgi?id=186590
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        After r232747 the sample managed by the player can be empty,
>+        without buffer. So we need to check for this before mapping video
>+        frames. Also use the GstVideoFrameHolder in more places to reduce
>+        copy-paste churn.
>+
>+        * platform/graphics/gstreamer/ImageGStreamer.h:
>+        * platform/graphics/gstreamer/ImageGStreamerCairo.cpp:
>+        (ImageGStreamer::ImageGStreamer):
>+        (ImageGStreamer::~ImageGStreamer):
>+        * platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:
>+        (WebCore::GstVideoFrameHolder::GstVideoFrameHolder):
>+        (WebCore::MediaPlayerPrivateGStreamerBase::copyVideoTextureToPlatformTexture):
>+        (WebCore::MediaPlayerPrivateGStreamerBase::nativeImageForCurrentTime):
>+
> 2018-06-12  Philippe Normand  <pnormand@igalia.com>
> 
>         [GStreamer] Video freezes when GStreamerGL is not installed
>diff --git a/Source/WebCore/platform/graphics/gstreamer/ImageGStreamer.h b/Source/WebCore/platform/graphics/gstreamer/ImageGStreamer.h
>index b6449a7bc6c4d527b9f87c2aaec22c8a48ad07e0..40724ea725129ec510810b22b11684f3e309c25e 100644
>--- a/Source/WebCore/platform/graphics/gstreamer/ImageGStreamer.h
>+++ b/Source/WebCore/platform/graphics/gstreamer/ImageGStreamer.h
>@@ -63,9 +63,9 @@ class ImageGStreamer : public RefCounted<ImageGStreamer> {
>         ImageGStreamer(GstSample*);
>         RefPtr<BitmapImage> m_image;
>         FloatRect m_cropRect;
>-
> #if USE(CAIRO)
>         GstVideoFrame m_videoFrame;
>+        bool m_frameMapped { false };
> #endif
>     };
> }
>diff --git a/Source/WebCore/platform/graphics/gstreamer/ImageGStreamerCairo.cpp b/Source/WebCore/platform/graphics/gstreamer/ImageGStreamerCairo.cpp
>index a3d5a5526029cb9335382de4c1ca990f2a3ed73e..bed66043e69a3d0267c710ac3e2565bd696a0697 100644
>--- a/Source/WebCore/platform/graphics/gstreamer/ImageGStreamerCairo.cpp
>+++ b/Source/WebCore/platform/graphics/gstreamer/ImageGStreamerCairo.cpp
>@@ -43,7 +43,11 @@ ImageGStreamer::ImageGStreamer(GstSample* sample)
>     ASSERT(GST_VIDEO_INFO_N_PLANES(&videoInfo) == 1);
> 
>     GstBuffer* buffer = gst_sample_get_buffer(sample);
>-    if (!gst_video_frame_map(&m_videoFrame, &videoInfo, buffer, GST_MAP_READ))
>+    if (UNLIKELY(!GST_IS_BUFFER(buffer)))
>+        return;
>+
>+    m_frameMapped = gst_video_frame_map(&m_videoFrame, &videoInfo, buffer, GST_MAP_READ);
>+    if (!m_frameMapped)
>         return;
> 
>     unsigned char* bufferData = reinterpret_cast<unsigned char*>(GST_VIDEO_FRAME_PLANE_DATA(&m_videoFrame, 0));
>@@ -110,6 +114,7 @@ ImageGStreamer::~ImageGStreamer()
> 
>     // We keep the buffer memory mapped until the image is destroyed because the internal
>     // cairo_surface_t was created using cairo_image_surface_create_for_data().
>-    gst_video_frame_unmap(&m_videoFrame);
>+    if (m_frameMapped)
>+        gst_video_frame_unmap(&m_videoFrame);
> }
> #endif // USE(GSTREAMER)
>diff --git a/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp b/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp
>index 4f8aaa4ade9d07475076b568c97a44c759d4d88e..efb76b20904b27cf8e2404a956e42869365a9f23 100644
>--- a/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp
>+++ b/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp
>@@ -211,6 +211,8 @@ public:
>         m_size = IntSize(GST_VIDEO_INFO_WIDTH(&videoInfo), GST_VIDEO_INFO_HEIGHT(&videoInfo));
>         m_hasAlphaChannel = GST_VIDEO_INFO_HAS_ALPHA(&videoInfo);
>         m_buffer = gst_sample_get_buffer(sample);
>+        if (UNLIKELY(!GST_IS_BUFFER(m_buffer)))
>+            return;
> 
> #if USE(GSTREAMER_GL)
>         m_flags = flags | (m_hasAlphaChannel ? TextureMapperGL::ShouldBlend : 0) | TEXTURE_MAPPER_COLOR_CONVERT_FLAG;
>@@ -928,28 +930,24 @@ bool MediaPlayerPrivateGStreamerBase::copyVideoTextureToPlatformTexture(Graphics
> 
>     auto sampleLocker = holdLock(m_sampleMutex);
> 
>-    GstVideoInfo videoInfo;
>-    if (!getSampleVideoInfo(m_sample.get(), videoInfo))
>+    if (!GST_IS_SAMPLE(m_sample.get()))
>         return false;
> 
>-    GstBuffer* buffer = gst_sample_get_buffer(m_sample.get());
>-    GstVideoFrame videoFrame;
>-    if (!gst_video_frame_map(&videoFrame, &videoInfo, buffer, static_cast<GstMapFlags>(GST_MAP_READ | GST_MAP_GL)))
>+    std::unique_ptr<GstVideoFrameHolder> frameHolder = std::make_unique<GstVideoFrameHolder>(m_sample.get(), texMapFlagFromOrientation(m_videoSourceOrientation), true);
>+
>+    auto textureID = frameHolder->textureID();
>+    ASSERT(textureID);
>+    if (!textureID)
>         return false;
> 
>-    IntSize size(GST_VIDEO_INFO_WIDTH(&videoInfo), GST_VIDEO_INFO_HEIGHT(&videoInfo));
>+    auto size = frameHolder->size();
>     if (m_videoSourceOrientation.usesWidthAsHeight())
>         size = size.transposedSize();
>-    unsigned textureID = *reinterpret_cast<unsigned*>(videoFrame.data[0]);
> 
>     if (!m_videoTextureCopier)
>         m_videoTextureCopier = std::make_unique<VideoTextureCopierGStreamer>(TEXTURE_COPIER_COLOR_CONVERT_FLAG);
> 
>-    bool copied = m_videoTextureCopier->copyVideoTextureToPlatformTexture(textureID, size, outputTexture, outputTarget, level, internalFormat, format, type, flipY, m_videoSourceOrientation);
>-
>-    gst_video_frame_unmap(&videoFrame);
>-
>-    return copied;
>+    return m_videoTextureCopier->copyVideoTextureToPlatformTexture(textureID, size, outputTexture, outputTarget, level, internalFormat, format, type, flipY, m_videoSourceOrientation);
> }
> 
> NativeImagePtr MediaPlayerPrivateGStreamerBase::nativeImageForCurrentTime()
>@@ -960,16 +958,17 @@ NativeImagePtr MediaPlayerPrivateGStreamerBase::nativeImageForCurrentTime()
> 
>     auto sampleLocker = holdLock(m_sampleMutex);
> 
>-    GstVideoInfo videoInfo;
>-    if (!getSampleVideoInfo(m_sample.get(), videoInfo))
>+    if (!GST_IS_SAMPLE(m_sample.get()))
>         return nullptr;
> 
>-    GstBuffer* buffer = gst_sample_get_buffer(m_sample.get());
>-    GstVideoFrame videoFrame;
>-    if (!gst_video_frame_map(&videoFrame, &videoInfo, buffer, static_cast<GstMapFlags>(GST_MAP_READ | GST_MAP_GL)))
>+    std::unique_ptr<GstVideoFrameHolder> frameHolder = std::make_unique<GstVideoFrameHolder>(m_sample.get(), texMapFlagFromOrientation(m_videoSourceOrientation), true);
>+
>+    auto textureID = frameHolder->textureID();
>+    ASSERT(textureID);
>+    if (!textureID)
>         return nullptr;
> 
>-    IntSize size(GST_VIDEO_INFO_WIDTH(&videoInfo), GST_VIDEO_INFO_HEIGHT(&videoInfo));
>+    auto size = frameHolder->size();
>     if (m_videoSourceOrientation.usesWidthAsHeight())
>         size = size.transposedSize();
> 
>@@ -979,11 +978,7 @@ NativeImagePtr MediaPlayerPrivateGStreamerBase::nativeImageForCurrentTime()
>     if (!m_videoTextureCopier)
>         m_videoTextureCopier = std::make_unique<VideoTextureCopierGStreamer>(TEXTURE_COPIER_COLOR_CONVERT_FLAG);
> 
>-    unsigned textureID = *reinterpret_cast<unsigned*>(videoFrame.data[0]);
>-    bool copied = m_videoTextureCopier->copyVideoTextureToPlatformTexture(textureID, size, 0, GraphicsContext3D::TEXTURE_2D, 0, GraphicsContext3D::RGBA, GraphicsContext3D::RGBA, GraphicsContext3D::UNSIGNED_BYTE, false, m_videoSourceOrientation);
>-    gst_video_frame_unmap(&videoFrame);
>-
>-    if (!copied)
>+    if (!m_videoTextureCopier->copyVideoTextureToPlatformTexture(textureID, size, 0, GraphicsContext3D::TEXTURE_2D, 0, GraphicsContext3D::RGBA, GraphicsContext3D::RGBA, GraphicsContext3D::UNSIGNED_BYTE, false, m_videoSourceOrientation))
>         return nullptr;
> 
>     return adoptRef(cairo_gl_surface_create_for_texture(context->cairoDevice(), CAIRO_CONTENT_COLOR_ALPHA, m_videoTextureCopier->resultTexture(), size.width(), size.height()));

Flags:

clopez: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 186590: 342656 | 342657