Diff
Modified: trunk/Source/WebCore/ChangeLog (140442 => 140443)
--- trunk/Source/WebCore/ChangeLog 2013-01-22 19:13:58 UTC (rev 140442)
+++ trunk/Source/WebCore/ChangeLog 2013-01-22 19:15:38 UTC (rev 140443)
@@ -1,3 +1,38 @@
+2013-01-22 Christophe Dumez <[email protected]>
+
+ [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 <[email protected]>
BackgroundHTMLParser should simulate tree building in a separate function
Modified: trunk/Source/WebCore/platform/audio/gstreamer/AudioDestinationGStreamer.cpp (140442 => 140443)
--- trunk/Source/WebCore/platform/audio/gstreamer/AudioDestinationGStreamer.cpp 2013-01-22 19:13:58 UTC (rev 140442)
+++ trunk/Source/WebCore/platform/audio/gstreamer/AudioDestinationGStreamer.cpp 2013-01-22 19:15:38 UTC (rev 140443)
@@ -26,6 +26,7 @@
#include "AudioSourceProvider.h"
#include <wtf/gobject/GOwnPtr.h>
#include "GRefPtrGStreamer.h"
+#include "GStreamerVersioning.h"
#include "Logging.h"
#include "WebKitWebAudioSourceGStreamer.h"
#include <gst/gst.h>
@@ -74,11 +75,10 @@
, m_isPlaying(false)
{
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,
"rate", sampleRate,
@@ -107,10 +107,11 @@
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);
}
Modified: trunk/Source/WebCore/platform/audio/gstreamer/AudioFileReaderGStreamer.cpp (140442 => 140443)
--- trunk/Source/WebCore/platform/audio/gstreamer/AudioFileReaderGStreamer.cpp 2013-01-22 19:13:58 UTC (rev 140442)
+++ trunk/Source/WebCore/platform/audio/gstreamer/AudioFileReaderGStreamer.cpp 2013-01-22 19:15:38 UTC (rev 140443)
@@ -181,10 +181,11 @@
AudioFileReader::~AudioFileReader()
{
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));
}
@@ -413,11 +414,10 @@
// A deinterleave element is added once a src pad becomes available in decodebin.
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;
if (m_data) {
Modified: trunk/Source/WebCore/platform/graphics/gstreamer/GStreamerGWorld.cpp (140442 => 140443)
--- trunk/Source/WebCore/platform/graphics/gstreamer/GStreamerGWorld.cpp 2013-01-22 19:13:58 UTC (rev 140442)
+++ trunk/Source/WebCore/platform/graphics/gstreamer/GStreamerGWorld.cpp 2013-01-22 19:15:38 UTC (rev 140443)
@@ -22,6 +22,7 @@
#if ENABLE(VIDEO) && USE(GSTREAMER) && !defined(GST_API_VERSION_1)
#include "GRefPtrGStreamer.h"
+#include "GStreamerVersioning.h"
#include <gst/gst.h>
#include <gst/interfaces/xoverlay.h>
#include <gst/pbutils/pbutils.h>
@@ -59,10 +60,9 @@
: m_pipeline(pipeline)
{
// 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);
}
GStreamerGWorld::~GStreamerGWorld()
Modified: trunk/Source/WebCore/platform/graphics/gstreamer/GStreamerVersioning.cpp (140442 => 140443)
--- trunk/Source/WebCore/platform/graphics/gstreamer/GStreamerVersioning.cpp 2013-01-22 19:13:58 UTC (rev 140442)
+++ trunk/Source/WebCore/platform/graphics/gstreamer/GStreamerVersioning.cpp 2013-01-22 19:15:38 UTC (rev 140443)
@@ -70,6 +70,17 @@
#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
+}
+
#if ENABLE(VIDEO)
bool getVideoSizeAndFormatFromCaps(GstCaps* caps, WebCore::IntSize& size, GstVideoFormat& format, int& pixelAspectRatioNumerator, int& pixelAspectRatioDenominator, int& stride)
{
Modified: trunk/Source/WebCore/platform/graphics/gstreamer/GStreamerVersioning.h (140442 => 140443)
--- trunk/Source/WebCore/platform/graphics/gstreamer/GStreamerVersioning.h 2013-01-22 19:13:58 UTC (rev 140442)
+++ trunk/Source/WebCore/platform/graphics/gstreamer/GStreamerVersioning.h 2013-01-22 19:15:38 UTC (rev 140443)
@@ -32,6 +32,7 @@
void webkitGstObjectRefSink(GstObject*);
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);
#endif
Modified: trunk/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp (140442 => 140443)
--- trunk/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp 2013-01-22 19:13:58 UTC (rev 140442)
+++ trunk/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp 2013-01-22 19:15:38 UTC (rev 140443)
@@ -275,6 +275,11 @@
#endif
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;
}
@@ -1801,10 +1806,9 @@
m_gstGWorld = GStreamerGWorld::createGWorld(m_playBin.get());
#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);
