[GStreamer] fast/canvas/webgl crashes
authorphiln@webkit.org <philn@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 13 Jun 2018 17:58:05 +0000 (17:58 +0000)
committerphiln@webkit.org <philn@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 13 Jun 2018 17:58:05 +0000 (17:58 +0000)
https://bugs.webkit.org/show_bug.cgi?id=186590

Reviewed by Carlos Alberto Lopez Perez.

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):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@232795 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/platform/graphics/gstreamer/ImageGStreamer.h
Source/WebCore/platform/graphics/gstreamer/ImageGStreamerCairo.cpp
Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp

index 50328f1..84f8ca7 100644 (file)
@@ -1,5 +1,26 @@
 2018-06-13  Philippe Normand  <pnormand@igalia.com>
 
+        [GStreamer] fast/canvas/webgl crashes
+        https://bugs.webkit.org/show_bug.cgi?id=186590
+
+        Reviewed by Carlos Alberto Lopez Perez.
+
+        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-13  Philippe Normand  <pnormand@igalia.com>
+
         Unreviewed GTK build fix for --cmakeargs=-DUSE_GSTREAMER_GL=OFF
 
         * platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:
index b6449a7..40724ea 100644 (file)
@@ -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
     };
 }
index a3d5a55..bed6604 100644 (file)
@@ -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)
index 7beb1d2..16d2340 100644 (file)
@@ -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;
@@ -931,28 +933,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()
@@ -963,16 +961,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();
 
@@ -982,11 +981,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()));