Changeset 140443 in webkit

Timestamp:

Jan 22, 2013, 11:15:38 AM (14 years ago)

Author:

Christophe Dumez

Message:

[gstreamer] GstBus signal watch should be removed on clean up
​https://bugs.webkit.org/show_bug.cgi?id=107544

Reviewed by Philippe Normand.

Our gstreamer backend code currently calls gst_bus_add_signal_watch()
on GstBus to add a signal watch. As per the gstreamer documentation,
"To clean up, the caller is responsible for calling
gst_bus_remove_signal_watch() as many times as this function is
called". This is because gst_bus_add_signal_watch() causes the GstBus
object to be ref'd and gst_bus_remove_signal_watch() needs to be
called to properly unref it.

This patch makes sure that gst_bus_remove_signal_watch() is called
on the GstBus object when cleaning up. This patch also uses smart
pointers for GstBus objects for consistency.

No new tests, no behavior change for layout tests.

• platform/audio/gstreamer/AudioDestinationGStreamer.cpp:

(WebCore::AudioDestinationGStreamer::AudioDestinationGStreamer):
(WebCore::AudioDestinationGStreamer::~AudioDestinationGStreamer):

• platform/audio/gstreamer/AudioFileReaderGStreamer.cpp:

(WebCore::AudioFileReader::~AudioFileReader):
(WebCore::AudioFileReader::decodeAudioForBusCreation):

• platform/graphics/gstreamer/GStreamerGWorld.cpp:

(WebCore::GStreamerGWorld::GStreamerGWorld):

• platform/graphics/gstreamer/GStreamerVersioning.cpp:

(webkitGstPipelineGetBus):

• platform/graphics/gstreamer/GStreamerVersioning.h:
• platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:

(WebCore::MediaPlayerPrivateGStreamer::~MediaPlayerPrivateGStreamer):

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• platform/audio/gstreamer/AudioDestinationGStreamer.cpp (modified) (3 diffs)
• platform/audio/gstreamer/AudioFileReaderGStreamer.cpp (modified) (2 diffs)
• platform/graphics/gstreamer/GStreamerGWorld.cpp (modified) (2 diffs)
• platform/graphics/gstreamer/GStreamerVersioning.cpp (modified) (1 diff)
• platform/graphics/gstreamer/GStreamerVersioning.h (modified) (1 diff)
• platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp (modified) (2 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2013-01-22  Christophe Dumez  <christophe.dumez@intel.com>
+
+        [gstreamer] GstBus signal watch should be removed on clean up
+        https://bugs.webkit.org/show_bug.cgi?id=107544
+
+        Reviewed by Philippe Normand.
+
+        Our gstreamer backend code currently calls gst_bus_add_signal_watch()
+        on GstBus to add a signal watch. As per the gstreamer  documentation,
+        "To clean up, the caller is responsible for calling
+        gst_bus_remove_signal_watch() as many times as this function is
+        called". This is because gst_bus_add_signal_watch() causes the GstBus
+        object to be ref'd and gst_bus_remove_signal_watch() needs to be
+        called to properly unref it.
+
+        This patch makes sure that gst_bus_remove_signal_watch() is called
+        on the GstBus object when cleaning up. This patch also uses smart
+        pointers for GstBus objects for consistency.
+
+        No new tests, no behavior change for layout tests.
+
+        * platform/audio/gstreamer/AudioDestinationGStreamer.cpp:
+        (WebCore::AudioDestinationGStreamer::AudioDestinationGStreamer):
+        (WebCore::AudioDestinationGStreamer::~AudioDestinationGStreamer):
+        * platform/audio/gstreamer/AudioFileReaderGStreamer.cpp:
+        (WebCore::AudioFileReader::~AudioFileReader):
+        (WebCore::AudioFileReader::decodeAudioForBusCreation):
+        * platform/graphics/gstreamer/GStreamerGWorld.cpp:
+        (WebCore::GStreamerGWorld::GStreamerGWorld):
+        * platform/graphics/gstreamer/GStreamerVersioning.cpp:
+        (webkitGstPipelineGetBus):
+        * platform/graphics/gstreamer/GStreamerVersioning.h:
+        * platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:
+        (WebCore::MediaPlayerPrivateGStreamer::~MediaPlayerPrivateGStreamer):
+
 2013-01-22  Adam Barth  <abarth@webkit.org>
 

trunk/Source/WebCore/platform/audio/gstreamer/AudioDestinationGStreamer.cpp

@@ -27 +27 @@
 #include <wtf/gobject/GOwnPtr.h>
 #include "GRefPtrGStreamer.h"
+#include "GStreamerVersioning.h"
 #include "Logging.h"
 #include "WebKitWebAudioSourceGStreamer.h"
@@ -75 +76 @@
 {
     m_pipeline = gst_pipeline_new("play");
-    GstBus* bus = gst_pipeline_get_bus(GST_PIPELINE(m_pipeline));
+    GRefPtr<GstBus> bus = webkitGstPipelineGetBus(GST_PIPELINE(m_pipeline));
     ASSERT(bus);
-    gst_bus_add_signal_watch(bus);
-    g_signal_connect(bus, "message", G_CALLBACK(messageCallback), this);
-    gst_object_unref(bus);
+    gst_bus_add_signal_watch(bus.get());
+    g_signal_connect(bus.get(), "message", G_CALLBACK(messageCallback), this);
 
     GstElement* webkitAudioSrc = reinterpret_cast<GstElement*>(g_object_new(WEBKIT_TYPE_WEB_AUDIO_SRC,
@@ -108 +108 @@
 AudioDestinationGStreamer::~AudioDestinationGStreamer()
 {
-    GstBus* bus = gst_pipeline_get_bus(GST_PIPELINE(m_pipeline));
+    GRefPtr<GstBus> bus = webkitGstPipelineGetBus(GST_PIPELINE(m_pipeline));
     ASSERT(bus);
-    g_signal_handlers_disconnect_by_func(bus, reinterpret_cast<gpointer>(messageCallback), this);
-    gst_object_unref(bus);
+    g_signal_handlers_disconnect_by_func(bus.get(), reinterpret_cast<gpointer>(messageCallback), this);
+    gst_bus_remove_signal_watch(bus.get());
+
     gst_element_set_state(m_pipeline, GST_STATE_NULL);
     gst_object_unref(m_pipeline);

trunk/Source/WebCore/platform/audio/gstreamer/AudioFileReaderGStreamer.cpp

@@ -182 +182 @@
 {
     if (m_pipeline) {
-        GstBus* bus = gst_pipeline_get_bus(GST_PIPELINE(m_pipeline));
+        GRefPtr<GstBus> bus = webkitGstPipelineGetBus(GST_PIPELINE(m_pipeline));
         ASSERT(bus);
-        g_signal_handlers_disconnect_by_func(bus, reinterpret_cast<gpointer>(messageCallback), this);
-        gst_object_unref(bus);
+        g_signal_handlers_disconnect_by_func(bus.get(), reinterpret_cast<gpointer>(messageCallback), this);
+        gst_bus_remove_signal_watch(bus.get());
+
         gst_element_set_state(m_pipeline, GST_STATE_NULL);
         gst_object_unref(GST_OBJECT(m_pipeline));
@@ -414 +415 @@
     m_pipeline = gst_pipeline_new(0);
 
-    GstBus* bus = gst_pipeline_get_bus(GST_PIPELINE(m_pipeline));
+    GRefPtr<GstBus> bus = webkitGstPipelineGetBus(GST_PIPELINE(m_pipeline));
     ASSERT(bus);
-    gst_bus_add_signal_watch(bus);
-    g_signal_connect(bus, "message", G_CALLBACK(messageCallback), this);
-    gst_object_unref(bus);
+    gst_bus_add_signal_watch(bus.get());
+    g_signal_connect(bus.get(), "message", G_CALLBACK(messageCallback), this);
 
     GstElement* source;

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

@@ -23 +23 @@
 
 #include "GRefPtrGStreamer.h"
+#include "GStreamerVersioning.h"
 #include <gst/gst.h>
 #include <gst/interfaces/xoverlay.h>
@@ -60 +61 @@
 {
     // XOverlay messages need to be handled synchronously.
-    GstBus* bus = gst_pipeline_get_bus(GST_PIPELINE(m_pipeline));
-    gst_bus_set_sync_handler(bus, gst_bus_sync_signal_handler, this);
-    g_signal_connect(bus, "sync-message::element", G_CALLBACK(gstGWorldSyncMessageCallback), this);
-    gst_object_unref(bus);
+    GRefPtr<GstBus> bus = webkitGstPipelineGetBus(GST_PIPELINE(m_pipeline));
+    gst_bus_set_sync_handler(bus.get(), gst_bus_sync_signal_handler, this);
+    g_signal_connect(bus.get(), "sync-message::element", G_CALLBACK(gstGWorldSyncMessageCallback), this);
 }
 

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

@@ -68 +68 @@
 #else
     return GST_PAD_CAPS(pad);
+#endif
+}
+
+GRefPtr<GstBus> webkitGstPipelineGetBus(GstPipeline* pipeline)
+{
+#ifdef GST_API_VERSION_1
+    return adoptGRef(gst_pipeline_get_bus(pipeline));
+#else
+    // gst_pipeline_get_bus returns a floating reference in
+    // gstreamer 0.10 so we should not adopt.
+    return gst_pipeline_get_bus(pipeline);
 #endif
 }

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

@@ -33 +33 @@
 GstPad* webkitGstGhostPadFromStaticTemplate(GstStaticPadTemplate*, const gchar* name, GstPad* target);
 GRefPtr<GstCaps> webkitGstGetPadCaps(GstPad*);
+GRefPtr<GstBus> webkitGstPipelineGetBus(GstPipeline*);
 #if ENABLE(VIDEO)
 bool getVideoSizeAndFormatFromCaps(GstCaps*, WebCore::IntSize&, GstVideoFormat&, int& pixelAspectRatioNumerator, int& pixelAspectRatioDenominator, int& stride);

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

@@ -276 +276 @@
 
     if (m_playBin) {
+        GRefPtr<GstBus> bus = webkitGstPipelineGetBus(GST_PIPELINE(m_playBin.get()));
+        ASSERT(bus);
+        g_signal_handlers_disconnect_by_func(bus.get(), reinterpret_cast<gpointer>(mediaPlayerPrivateMessageCallback), this);
+        gst_bus_remove_signal_watch(bus.get());
+
         gst_element_set_state(m_playBin.get(), GST_STATE_NULL);
         m_playBin = 0;
@@ -1802 +1807 @@
 #endif
 
-    GstBus* bus = gst_pipeline_get_bus(GST_PIPELINE(m_playBin.get()));
-    gst_bus_add_signal_watch(bus);
-    g_signal_connect(bus, "message", G_CALLBACK(mediaPlayerPrivateMessageCallback), this);
-    gst_object_unref(bus);
+    GRefPtr<GstBus> bus = webkitGstPipelineGetBus(GST_PIPELINE(m_playBin.get()));
+    gst_bus_add_signal_watch(bus.get());
+    g_signal_connect(bus.get(), "message", G_CALLBACK(mediaPlayerPrivateMessageCallback), this);
 
     g_object_set(m_playBin.get(), "mute", m_player->muted(), NULL);