Changeset 139896 in webkit

Timestamp:

Jan 16, 2013, 10:44:18 AM (13 years ago)

Author:

Christophe Dumez

Message:

[gstreamer] Some media tests occasionally crash with gstreamer 1.0 backend
​https://bugs.webkit.org/show_bug.cgi?id=106551

Reviewed by Philippe Normand.

ImageGStreamerCairo was passing mapped memory to
cairo_image_surface_create_for_data() and then unmapping it straight
away even though the cairo_surface_t is still used. The cairo
documentation states:
"The output buffer must be kept around until the cairo_surface_t is
destroyed or cairo_surface_finish() is called on the surface."

This patch keeps the GstBuffer memory mapped until the ImageGStreamer
is destroyed so that the internal cairo_surface_t stays valid while
avoiding copying the image data.

No new tests, already covered by existing tests.

• platform/graphics/gstreamer/GRefPtrGStreamer.cpp:

(WTF::adoptGRef):
(WTF):
(WTF::GstBuffer):

• platform/graphics/gstreamer/GRefPtrGStreamer.h:

(WTF): Add support for using GRefPtr with GstBuffer.

• platform/graphics/gstreamer/ImageGStreamer.h:

(ImageGStreamer):

• platform/graphics/gstreamer/ImageGStreamerCairo.cpp:

(ImageGStreamer::ImageGStreamer):
(ImageGStreamer::~ImageGStreamer):

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• platform/graphics/gstreamer/GRefPtrGStreamer.cpp (modified) (1 diff)
• platform/graphics/gstreamer/GRefPtrGStreamer.h (modified) (2 diffs)
• platform/graphics/gstreamer/ImageGStreamer.h (modified) (1 diff)
• platform/graphics/gstreamer/ImageGStreamerCairo.cpp (modified) (4 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2013-01-16  Christophe Dumez  <christophe.dumez@intel.com>
+
+        [gstreamer] Some media tests occasionally crash with gstreamer 1.0 backend
+        https://bugs.webkit.org/show_bug.cgi?id=106551
+
+        Reviewed by Philippe Normand.
+
+        ImageGStreamerCairo was passing mapped memory to
+        cairo_image_surface_create_for_data() and then unmapping it straight
+        away even though the cairo_surface_t is still used. The cairo
+        documentation states:
+        "The output buffer must be kept around until the cairo_surface_t is
+        destroyed or cairo_surface_finish() is called on the surface."
+
+        This patch keeps the GstBuffer memory mapped until the ImageGStreamer
+        is destroyed so that the internal cairo_surface_t stays valid while
+        avoiding copying the image data.
+
+        No new tests, already covered by existing tests.
+
+        * platform/graphics/gstreamer/GRefPtrGStreamer.cpp:
+        (WTF::adoptGRef):
+        (WTF):
+        (WTF::GstBuffer):
+        * platform/graphics/gstreamer/GRefPtrGStreamer.h:
+        (WTF): Add support for using GRefPtr with GstBuffer.
+        * platform/graphics/gstreamer/ImageGStreamer.h:
+        (ImageGStreamer):
+        * platform/graphics/gstreamer/ImageGStreamerCairo.cpp:
+        (ImageGStreamer::ImageGStreamer):
+        (ImageGStreamer::~ImageGStreamer):
+
 2013-01-16  Mike Reed  <reed@google.com>
 

trunk/Source/WebCore/platform/graphics/gstreamer/GRefPtrGStreamer.cpp

@@ -166 +166 @@
 }
 
+template<> GRefPtr<GstBuffer> adoptGRef(GstBuffer* ptr)
+{
+    return GRefPtr<GstBuffer>(ptr, GRefPtrAdopt);
+}
+
+template<> GstBuffer* refGPtr<GstBuffer>(GstBuffer* ptr)
+{
+    if (ptr)
+        gst_buffer_ref(ptr);
+
+    return ptr;
+}
+
+template<> void derefGPtr<GstBuffer>(GstBuffer* ptr)
+{
+    if (ptr)
+        gst_buffer_unref(ptr);
+}
 }
 #endif // USE(GSTREAMER)

trunk/Source/WebCore/platform/graphics/gstreamer/GRefPtrGStreamer.h

@@ -31 +31 @@
 typedef struct _GstBus GstBus;
 typedef struct _GstElementFactory GstElementFactory;
+typedef struct _GstBuffer GstBuffer;
 
 namespace WTF {
@@ -62 +63 @@
 template<> void derefGPtr<GstElementFactory>(GstElementFactory* ptr);
 
+template<> GRefPtr<GstBuffer> adoptGRef(GstBuffer* ptr);
+template<> GstBuffer* refGPtr<GstBuffer>(GstBuffer* ptr);
+template<> void derefGPtr<GstBuffer>(GstBuffer* ptr);
 }
 

trunk/Source/WebCore/platform/graphics/gstreamer/ImageGStreamer.h

@@ -61 +61 @@
         RefPtr<BitmapImage> m_image;
         FloatRect m_cropRect;
+
+#if USE(CAIRO) && defined(GST_API_VERSION_1)
+        GRefPtr<GstBuffer> m_buffer;
+        GstMapInfo m_mapInfo;
+#endif
     };
 }

trunk/Source/WebCore/platform/graphics/gstreamer/ImageGStreamerCairo.cpp

@@ -37 +37 @@
 
 ImageGStreamer::ImageGStreamer(GstBuffer* buffer, GstCaps* caps)
+#ifdef GST_API_VERSION_1
+    : m_buffer(buffer)
+#endif
 {
     GstVideoFormat format;
@@ -44 +47 @@
 
 #ifdef GST_API_VERSION_1
-    GstMapInfo mapInfo;
-    gst_buffer_map(buffer, &mapInfo, GST_MAP_READ);
-    unsigned char* bufferData = reinterpret_cast<unsigned char*>(mapInfo.data);
+    gst_buffer_map(buffer, &m_mapInfo, GST_MAP_READ);
+    unsigned char* bufferData = reinterpret_cast<unsigned char*>(m_mapInfo.data);
 #else
     unsigned char* bufferData = reinterpret_cast<unsigned char*>(GST_BUFFER_DATA(buffer));
@@ -65 +67 @@
     if (GstVideoCropMeta* cropMeta = gst_buffer_get_video_crop_meta(buffer))
         setCropRect(FloatRect(cropMeta->x, cropMeta->y, cropMeta->width, cropMeta->height));
-
-    gst_buffer_unmap(buffer, &mapInfo);
 #endif
 }
@@ -76 +76 @@
 
     m_image = 0;
+
+#ifdef GST_API_VERSION_1
+    // 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_buffer_unmap(m_buffer.get(), &m_mapInfo);
+#endif
 }
 #endif // USE(GSTREAMER)