Title: [236668] trunk/Source/WebCore
Revision
236668
Author
ab...@igalia.com
Date
2018-10-01 10:40:48 -0700 (Mon, 01 Oct 2018)

Log Message

[GStreamer] Fix abort in gst_sample_get_info()
https://bugs.webkit.org/show_bug.cgi?id=190135

Reviewed by Philippe Normand.

A flush can occur before any frame has finished decoding -- especially
in tests, where actions on the player often occur in quick succession.

Therefore, the code must not assume by the time a flush occurs any
frame has reached the sink. This patch fixes a case when such wrong
assumption was causing gst_sample_get_info() to abort (crashing
WebKit).

* platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:
(WebCore::MediaPlayerPrivateGStreamerBase::flushCurrentBuffer):
(WebCore::MediaPlayerPrivateGStreamerBase::createGLAppSink):

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (236667 => 236668)


--- trunk/Source/WebCore/ChangeLog	2018-10-01 17:22:54 UTC (rev 236667)
+++ trunk/Source/WebCore/ChangeLog	2018-10-01 17:40:48 UTC (rev 236668)
@@ -1,3 +1,22 @@
+2018-10-01  Alicia Boya GarcĂ­a  <ab...@igalia.com>
+
+        [GStreamer] Fix abort in gst_sample_get_info()
+        https://bugs.webkit.org/show_bug.cgi?id=190135
+
+        Reviewed by Philippe Normand.
+
+        A flush can occur before any frame has finished decoding -- especially
+        in tests, where actions on the player often occur in quick succession.
+
+        Therefore, the code must not assume by the time a flush occurs any
+        frame has reached the sink. This patch fixes a case when such wrong
+        assumption was causing gst_sample_get_info() to abort (crashing
+        WebKit).
+
+        * platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:
+        (WebCore::MediaPlayerPrivateGStreamerBase::flushCurrentBuffer):
+        (WebCore::MediaPlayerPrivateGStreamerBase::createGLAppSink):
+
 2018-10-01  Olivier Blin  <olivier.b...@softathome.com>
 
         [WPE] fix buffer over-read in RenderThemeWPE::mediaControlsStyleSheet()

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


--- trunk/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp	2018-10-01 17:22:54 UTC (rev 236667)
+++ trunk/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp	2018-10-01 17:40:48 UTC (rev 236668)
@@ -916,11 +916,13 @@
     GST_DEBUG_OBJECT(pipeline(), "Flushing video sample");
     auto sampleLocker = holdLock(m_sampleMutex);
 
-    // Replace by a new sample having only the caps, so this dummy sample is still useful to get the dimensions.
-    // This prevents resizing problems when the video changes its quality and a DRAIN is performed.
-    const GstStructure* info = gst_sample_get_info(m_sample.get());
-    m_sample = adoptGRef(gst_sample_new(nullptr, gst_sample_get_caps(m_sample.get()),
-        gst_sample_get_segment(m_sample.get()), info ? gst_structure_copy(info) : nullptr));
+    if (m_sample) {
+        // Replace by a new sample having only the caps, so this dummy sample is still useful to get the dimensions.
+        // This prevents resizing problems when the video changes its quality and a DRAIN is performed.
+        const GstStructure* info = gst_sample_get_info(m_sample.get());
+        m_sample = adoptGRef(gst_sample_new(nullptr, gst_sample_get_caps(m_sample.get()),
+            gst_sample_get_segment(m_sample.get()), info ? gst_structure_copy(info) : nullptr));
+    }
 
     auto proxyOperation =
         [](TextureMapperPlatformLayerProxy& proxy)
@@ -1076,6 +1078,11 @@
 
     GRefPtr<GstPad> pad = adoptGRef(gst_element_get_static_pad(appsink, "sink"));
     gst_pad_add_probe(pad.get(), static_cast<GstPadProbeType>(GST_PAD_PROBE_TYPE_QUERY_DOWNSTREAM | GST_PAD_PROBE_TYPE_EVENT_FLUSH), [] (GstPad*, GstPadProbeInfo* info,  gpointer userData) -> GstPadProbeReturn {
+        // In some platforms (e.g. OpenMAX on the Raspberry Pi) when a resolution change occurs the
+        // pipeline has to be drained before a frame with the new resolution can be decoded.
+        // In this context, it's important that we don't hold references to any previous frame
+        // (e.g. m_sample) so that decoding can continue.
+        // We are also not supposed to keep the original frame after a flush.
         if (info->type & GST_PAD_PROBE_TYPE_QUERY_DOWNSTREAM) {
             if (GST_QUERY_TYPE(GST_PAD_PROBE_INFO_QUERY(info)) != GST_QUERY_DRAIN)
                 return GST_PAD_PROBE_OK;
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to