Modified: trunk/Source/WebCore/ChangeLog (151727 => 151728)
--- trunk/Source/WebCore/ChangeLog 2013-06-19 08:07:40 UTC (rev 151727)
+++ trunk/Source/WebCore/ChangeLog 2013-06-19 09:14:02 UTC (rev 151728)
@@ -1,3 +1,32 @@
+2013-06-19 Víctor Manuel Jáquez Leal <[email protected]>
+
+ [GStreamer] Potential race condition when getting the video sink caps.
+ https://bugs.webkit.org/show_bug.cgi?id=117736
+
+ Reviewed by Philippe Normand.
+
+ There is a potential race condition with the pad setting caps, as the
+ buffer and the caps won't match when renegotiation happens, and might
+ cause a crash.
+
+ This patch keeps an instance of the current caps in the video sink,
+ and it is accessible through a getter method. Hence the player can ask
+ for the current caps without running into a race condition.
+
+ No new tests. No change in behaviour.
+
+ * platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:
+ (WebCore::MediaPlayerPrivateGStreamerBase::naturalSize):
+ (WebCore::MediaPlayerPrivateGStreamerBase::paint):
+ (WebCore::MediaPlayerPrivateGStreamerBase::currentVideoSinkCaps):
+ (WebCore::MediaPlayerPrivateGStreamerBase::createVideoSink):
+ * platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.h:
+ * platform/graphics/gstreamer/VideoSinkGStreamer.cpp:
+ (webkitVideoSinkGetProperty):
+ (webkitVideoSinkStop):
+ (webkitVideoSinkSetCaps):
+ (webkit_video_sink_class_init):
+
2013-06-19 Kihong Kwon <[email protected]>
Vibration can be canceled even if page visibility status is hidden
Modified: trunk/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp (151727 => 151728)
--- trunk/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp 2013-06-19 08:07:40 UTC (rev 151727)
+++ trunk/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp 2013-06-19 09:14:02 UTC (rev 151728)
@@ -176,11 +176,7 @@
return m_videoSize;
#ifdef GST_API_VERSION_1
- /* FIXME this has a race with the pad setting caps as the buffer (m_buffer)
- * and the caps won't match and might cause a crash. (In case a
- * renegotiation happens)
- */
- GRefPtr<GstCaps> caps = webkitGstGetPadCaps(m_videoSinkPad.get());
+ GRefPtr<GstCaps> caps = currentVideoSinkCaps();
#else
g_mutex_lock(m_bufferMutex);
GRefPtr<GstCaps> caps = m_buffer ? GST_BUFFER_CAPS(m_buffer) : 0;
@@ -418,11 +414,7 @@
}
#ifdef GST_API_VERSION_1
- /* FIXME this has a race with the pad setting caps as the buffer (m_buffer)
- * and the caps won't match and might cause a crash. (In case a
- * renegotiation happens)
- */
- GRefPtr<GstCaps> caps = webkitGstGetPadCaps(m_videoSinkPad.get());
+ GRefPtr<GstCaps> caps = currentVideoSinkCaps();
#else
GRefPtr<GstCaps> caps = GST_BUFFER_CAPS(m_buffer);
#endif
@@ -503,6 +495,16 @@
return MediaPlayer::Download;
}
+GRefPtr<GstCaps> MediaPlayerPrivateGStreamerBase::currentVideoSinkCaps() const
+{
+ if (!m_webkitVideoSink)
+ return 0;
+
+ GstCaps* currentCaps = 0;
+ g_object_get(G_OBJECT(m_webkitVideoSink.get()), "current-caps", ¤tCaps, NULL);
+ return adoptGRef(currentCaps);
+}
+
// This function creates and initializes some internal variables, and returns a
// pointer to the element that should receive the data flow first
GstElement* MediaPlayerPrivateGStreamerBase::createVideoSink(GstElement* pipeline)
@@ -517,7 +519,6 @@
UNUSED_PARAM(pipeline);
m_webkitVideoSink = webkitVideoSinkNew();
#endif
- m_videoSinkPad = adoptGRef(gst_element_get_static_pad(m_webkitVideoSink.get(), "sink"));
m_repaintHandler = g_signal_connect(m_webkitVideoSink.get(), "repaint-requested", G_CALLBACK(mediaPlayerPrivateRepaintCallback), this);
Modified: trunk/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.h (151727 => 151728)
--- trunk/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.h 2013-06-19 08:07:40 UTC (rev 151727)
+++ trunk/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.h 2013-06-19 09:14:02 UTC (rev 151728)
@@ -114,6 +114,7 @@
GstElement* createVideoSink(GstElement* pipeline);
void setStreamVolumeElement(GstStreamVolume*);
virtual GstElement* audioSink() const { return 0; }
+ GRefPtr<GstCaps> currentVideoSinkCaps() const;
MediaPlayer* m_player;
GRefPtr<GstStreamVolume> m_volumeElement;
@@ -134,7 +135,6 @@
unsigned long m_repaintHandler;
unsigned long m_volumeSignalHandler;
unsigned long m_muteSignalHandler;
- GRefPtr<GstPad> m_videoSinkPad;
mutable IntSize m_videoSize;
#if USE(ACCELERATED_COMPOSITING) && USE(TEXTURE_MAPPER_GL) && !USE(COORDINATED_GRAPHICS)
void updateTexture(GstBuffer*);
Modified: trunk/Source/WebCore/platform/graphics/gstreamer/VideoSinkGStreamer.cpp (151727 => 151728)
--- trunk/Source/WebCore/platform/graphics/gstreamer/VideoSinkGStreamer.cpp 2013-06-19 08:07:40 UTC (rev 151727)
+++ trunk/Source/WebCore/platform/graphics/gstreamer/VideoSinkGStreamer.cpp 2013-06-19 09:14:02 UTC (rev 151728)
@@ -76,7 +76,8 @@
};
enum {
- PROP_0
+ PROP_0,
+ PROP_CAPS
};
static guint webkitVideoSinkSignals[LAST_SIGNAL] = { 0, };
@@ -95,6 +96,8 @@
WebCore::GStreamerGWorld* gstGWorld;
#endif
+ GstCaps* currentCaps;
+
// If this is TRUE all processing should finish ASAP
// This is necessary because there could be a race between
// unlock() and render(), where unlock() wins, signals the
@@ -296,6 +299,24 @@
G_OBJECT_CLASS(parent_class)->dispose(object);
}
+static void webkitVideoSinkGetProperty(GObject* object, guint propertyId, GValue* value, GParamSpec* parameterSpec)
+{
+ WebKitVideoSink* sink = WEBKIT_VIDEO_SINK(object);
+ WebKitVideoSinkPrivate* priv = sink->priv;
+
+ switch (propertyId) {
+ case PROP_CAPS: {
+ GstCaps* caps = priv->currentCaps;
+ if (caps)
+ gst_caps_ref(caps);
+ g_value_take_boxed(value, caps);
+ break;
+ }
+ default:
+ G_OBJECT_WARN_INVALID_PROPERTY_ID(object, propertyId, parameterSpec);
+ }
+}
+
static void unlockBufferMutex(WebKitVideoSinkPrivate* priv)
{
g_mutex_lock(priv->bufferMutex);
@@ -333,7 +354,15 @@
static gboolean webkitVideoSinkStop(GstBaseSink* baseSink)
{
- unlockBufferMutex(WEBKIT_VIDEO_SINK(baseSink)->priv);
+ WebKitVideoSinkPrivate* priv = WEBKIT_VIDEO_SINK(baseSink)->priv;
+
+ unlockBufferMutex(priv);
+
+ if (priv->currentCaps) {
+ gst_caps_unref(priv->currentCaps);
+ priv->currentCaps = 0;
+ }
+
return TRUE;
}
@@ -347,6 +376,14 @@
return TRUE;
}
+static gboolean webkitVideoSinkSetCaps(GstBaseSink* baseSink, GstCaps* caps)
+{
+ WebKitVideoSinkPrivate* priv = WEBKIT_VIDEO_SINK(baseSink)->priv;
+
+ gst_caps_replace(&priv->currentCaps, caps);
+ return TRUE;
+}
+
#ifdef GST_API_VERSION_1
static gboolean webkitVideoSinkProposeAllocation(GstBaseSink* baseSink, GstQuery* query)
{
@@ -403,6 +440,7 @@
g_type_class_add_private(klass, sizeof(WebKitVideoSinkPrivate));
gobjectClass->dispose = webkitVideoSinkDispose;
+ gobjectClass->get_property = webkitVideoSinkGetProperty;
baseSinkClass->unlock = webkitVideoSinkUnlock;
baseSinkClass->unlock_stop = webkitVideoSinkUnlockStop;
@@ -410,10 +448,14 @@
baseSinkClass->preroll = webkitVideoSinkRender;
baseSinkClass->stop = webkitVideoSinkStop;
baseSinkClass->start = webkitVideoSinkStart;
+ baseSinkClass->set_caps = webkitVideoSinkSetCaps;
#ifdef GST_API_VERSION_1
baseSinkClass->propose_allocation = webkitVideoSinkProposeAllocation;
#endif
+ g_object_class_install_property(gobjectClass, PROP_CAPS,
+ g_param_spec_boxed("current-caps", "Current-Caps", "Current caps", GST_TYPE_CAPS, G_PARAM_READABLE));
+
webkitVideoSinkSignals[REPAINT_REQUESTED] = g_signal_new("repaint-requested",
G_TYPE_FROM_CLASS(klass),
static_cast<GSignalFlags>(G_SIGNAL_RUN_LAST | G_SIGNAL_ACTION),
