Title: [232795] trunk/Source/WebCore
Revision
232795
Author
ph...@webkit.org
Date
2018-06-13 10:58:05 -0700 (Wed, 13 Jun 2018)

Log Message

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

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (232794 => 232795)


--- trunk/Source/WebCore/ChangeLog	2018-06-13 17:06:30 UTC (rev 232794)
+++ trunk/Source/WebCore/ChangeLog	2018-06-13 17:58:05 UTC (rev 232795)
@@ -1,5 +1,26 @@
 2018-06-13  Philippe Normand  <pnorm...@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  <pnorm...@igalia.com>
+
         Unreviewed GTK build fix for --cmakeargs=-DUSE_GSTREAMER_GL=OFF
 
         * platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:

Modified: trunk/Source/WebCore/platform/graphics/gstreamer/ImageGStreamer.h (232794 => 232795)


--- trunk/Source/WebCore/platform/graphics/gstreamer/ImageGStreamer.h	2018-06-13 17:06:30 UTC (rev 232794)
+++ trunk/Source/WebCore/platform/graphics/gstreamer/ImageGStreamer.h	2018-06-13 17:58:05 UTC (rev 232795)
@@ -63,9 +63,9 @@
         ImageGStreamer(GstSample*);
         RefPtr<BitmapImage> m_image;
         FloatRect m_cropRect;
-
 #if USE(CAIRO)
         GstVideoFrame m_videoFrame;
+        bool m_frameMapped { false };
 #endif
     };
 }

Modified: trunk/Source/WebCore/platform/graphics/gstreamer/ImageGStreamerCairo.cpp (232794 => 232795)


--- trunk/Source/WebCore/platform/graphics/gstreamer/ImageGStreamerCairo.cpp	2018-06-13 17:06:30 UTC (rev 232794)
+++ trunk/Source/WebCore/platform/graphics/gstreamer/ImageGStreamerCairo.cpp	2018-06-13 17:58:05 UTC (rev 232795)
@@ -43,9 +43,13 @@
     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));
     int stride = GST_VIDEO_FRAME_PLANE_STRIDE(&m_videoFrame, 0);
     int width = GST_VIDEO_FRAME_WIDTH(&m_videoFrame);
@@ -110,6 +114,7 @@
 
     // 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)

Modified: trunk/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp (232794 => 232795)


--- trunk/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp	2018-06-13 17:06:30 UTC (rev 232794)
+++ trunk/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp	2018-06-13 17:58:05 UTC (rev 232795)
@@ -211,6 +211,8 @@
         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 @@
 
     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 @@
 
     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 @@
     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()));
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to