<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[192094] trunk/Source/WebCore</title>
</head>
<body>
<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; }
#msg dl a { font-weight: bold}
#msg dl a:link { color:#fc3; }
#msg dl a:active { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/192094">192094</a></dd>
<dt>Author</dt> <dd>carlosgc@webkit.org</dd>
<dt>Date</dt> <dd>2015-11-05 23:28:00 -0800 (Thu, 05 Nov 2015)</dd>
</dl>
<h3>Log Message</h3>
<pre>[GStreamer] Use RunLoop::Timer instead of GMainLoopSource in video sink
https://bugs.webkit.org/show_bug.cgi?id=150807
Reviewed by Žan Doberšek.
Since we always wait until the sample is actually rendered we
don't really need either a thread safe main loop source, nor
cancelling if already requested and other things GMainLoopSource does.
This adds a helper class VideoRenderRequestScheduler to use the
RunLoop::Timer. All the logic to syncronize between threads has
been moved to this helper class too.
* platform/graphics/gstreamer/VideoSinkGStreamer.cpp:
(VideoRenderRequestScheduler::VideoRenderRequestScheduler):
(VideoRenderRequestScheduler::start):
(VideoRenderRequestScheduler::stop):
(VideoRenderRequestScheduler::requestRender):
(VideoRenderRequestScheduler::isUnlocked):
(VideoRenderRequestScheduler::render):
(_WebKitVideoSinkPrivate::_WebKitVideoSinkPrivate):
(webkitVideoSinkRepaintRequested):
(webkitVideoSinkRender):
(webkitVideoSinkUnlock):
(webkitVideoSinkUnlockStop):
(webkitVideoSinkStop):
(webkitVideoSinkStart):
(_WebKitVideoSinkPrivate::~_WebKitVideoSinkPrivate): Deleted.
(webkitVideoSinkTimeoutCallback): Deleted.
(unlockSampleMutex): Deleted.</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceWebCoreChangeLog">trunk/Source/WebCore/ChangeLog</a></li>
<li><a href="#trunkSourceWebCoreplatformgraphicsgstreamerVideoSinkGStreamercpp">trunk/Source/WebCore/platform/graphics/gstreamer/VideoSinkGStreamer.cpp</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/ChangeLog (192093 => 192094)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/ChangeLog        2015-11-06 05:58:52 UTC (rev 192093)
+++ trunk/Source/WebCore/ChangeLog        2015-11-06 07:28:00 UTC (rev 192094)
</span><span class="lines">@@ -1,3 +1,35 @@
</span><ins>+2015-11-05 Carlos Garcia Campos <cgarcia@igalia.com>
+
+ [GStreamer] Use RunLoop::Timer instead of GMainLoopSource in video sink
+ https://bugs.webkit.org/show_bug.cgi?id=150807
+
+ Reviewed by Žan Doberšek.
+
+ Since we always wait until the sample is actually rendered we
+ don't really need either a thread safe main loop source, nor
+ cancelling if already requested and other things GMainLoopSource does.
+ This adds a helper class VideoRenderRequestScheduler to use the
+ RunLoop::Timer. All the logic to syncronize between threads has
+ been moved to this helper class too.
+
+ * platform/graphics/gstreamer/VideoSinkGStreamer.cpp:
+ (VideoRenderRequestScheduler::VideoRenderRequestScheduler):
+ (VideoRenderRequestScheduler::start):
+ (VideoRenderRequestScheduler::stop):
+ (VideoRenderRequestScheduler::requestRender):
+ (VideoRenderRequestScheduler::isUnlocked):
+ (VideoRenderRequestScheduler::render):
+ (_WebKitVideoSinkPrivate::_WebKitVideoSinkPrivate):
+ (webkitVideoSinkRepaintRequested):
+ (webkitVideoSinkRender):
+ (webkitVideoSinkUnlock):
+ (webkitVideoSinkUnlockStop):
+ (webkitVideoSinkStop):
+ (webkitVideoSinkStart):
+ (_WebKitVideoSinkPrivate::~_WebKitVideoSinkPrivate): Deleted.
+ (webkitVideoSinkTimeoutCallback): Deleted.
+ (unlockSampleMutex): Deleted.
+
</ins><span class="cx"> 2015-11-05 Nikos Andronikos <nikos.andronikos-webkit@cisra.canon.com.au>
</span><span class="cx">
</span><span class="cx"> Add runtime and compile time flags for enabling Web Animations API and model.
</span></span></pre></div>
<a id="trunkSourceWebCoreplatformgraphicsgstreamerVideoSinkGStreamercpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/platform/graphics/gstreamer/VideoSinkGStreamer.cpp (192093 => 192094)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/platform/graphics/gstreamer/VideoSinkGStreamer.cpp        2015-11-06 05:58:52 UTC (rev 192093)
+++ trunk/Source/WebCore/platform/graphics/gstreamer/VideoSinkGStreamer.cpp        2015-11-06 07:28:00 UTC (rev 192094)
</span><span class="lines">@@ -35,8 +35,8 @@
</span><span class="cx"> #include <glib.h>
</span><span class="cx"> #include <gst/gst.h>
</span><span class="cx"> #include <gst/video/gstvideometa.h>
</span><del>-#include <wtf/glib/GMutexLocker.h>
-#include <wtf/glib/GThreadSafeMainLoopSource.h>
</del><ins>+#include <wtf/Condition.h>
+#include <wtf/RunLoop.h>
</ins><span class="cx">
</span><span class="cx"> using namespace WebCore;
</span><span class="cx">
</span><span class="lines">@@ -67,40 +67,97 @@
</span><span class="cx">
</span><span class="cx"> static guint webkitVideoSinkSignals[LAST_SIGNAL] = { 0, };
</span><span class="cx">
</span><del>-struct _WebKitVideoSinkPrivate {
- _WebKitVideoSinkPrivate()
</del><ins>+static void webkitVideoSinkRepaintRequested(WebKitVideoSink*, GstSample*);
+static GRefPtr<GstSample> webkitVideoSinkRequestRender(WebKitVideoSink*, GstBuffer*);
+
+class VideoRenderRequestScheduler {
+public:
+ VideoRenderRequestScheduler()
+ : m_timer(RunLoop::main(), this, &VideoRenderRequestScheduler::render)
</ins><span class="cx"> {
</span><del>- g_mutex_init(&sampleMutex);
- g_cond_init(&dataCondition);
- gst_video_info_init(&info);
</del><ins>+#if PLATFORM(GTK)
+ // Use a higher priority than WebCore timers (G_PRIORITY_HIGH_IDLE + 20).
+ m_timer.setPriority(G_PRIORITY_HIGH_IDLE + 19);
+#endif
</ins><span class="cx"> }
</span><span class="cx">
</span><del>- ~_WebKitVideoSinkPrivate()
</del><ins>+ void start()
</ins><span class="cx"> {
</span><del>- g_mutex_clear(&sampleMutex);
- g_cond_clear(&dataCondition);
</del><ins>+ LockHolder locker(m_sampleMutex);
+ m_unlocked = false;
</ins><span class="cx"> }
</span><span class="cx">
</span><del>- GstSample* sample;
- GThreadSafeMainLoopSource timeoutSource;
- GMutex sampleMutex;
- GCond dataCondition;
</del><ins>+ void stop()
+ {
+ LockHolder locker(m_sampleMutex);
+ m_timer.stop();
+ m_sample = nullptr;
+ m_unlocked = true;
+ m_dataCondition.notifyOne();
+ }
</ins><span class="cx">
</span><del>- GstVideoInfo info;
</del><ins>+ bool requestRender(WebKitVideoSink* sink, GstBuffer* buffer)
+ {
+ LockHolder locker(m_sampleMutex);
+ if (m_unlocked)
+ return true;
</ins><span class="cx">
</span><del>- GstCaps* currentCaps;
</del><ins>+ m_sample = webkitVideoSinkRequestRender(sink, buffer);
+ if (!m_sample)
+ return false;
</ins><span class="cx">
</span><del>- // If this is TRUE all processing should finish ASAP
</del><ins>+ m_sink = sink;
+ m_timer.startOneShot(0);
+ m_dataCondition.wait(m_sampleMutex);
+ return true;
+ }
+
+private:
+
+ void render()
+ {
+ LockHolder locker(m_sampleMutex);
+ GRefPtr<GstSample> sample = WTF::move(m_sample);
+ GRefPtr<WebKitVideoSink> sink = WTF::move(m_sink);
+ if (sample && !m_unlocked && LIKELY(GST_IS_SAMPLE(sample.get())))
+ webkitVideoSinkRepaintRequested(sink.get(), sample.get());
+ m_dataCondition.notifyOne();
+ }
+
+ RunLoop::Timer<VideoRenderRequestScheduler> m_timer;
+ Lock m_sampleMutex;
+ Condition m_dataCondition;
+ GRefPtr<GstSample> m_sample;
+ GRefPtr<WebKitVideoSink> m_sink;
+
+ // If this is true all processing should finish ASAP
</ins><span class="cx"> // This is necessary because there could be a race between
</span><span class="cx"> // unlock() and render(), where unlock() wins, signals the
</span><del>- // GCond, then render() tries to render a frame although
</del><ins>+ // Condition, then render() tries to render a frame although
</ins><span class="cx"> // everything else isn't running anymore. This will lead
</span><span class="cx"> // to deadlocks because render() holds the stream lock.
</span><span class="cx"> //
</span><span class="cx"> // Protected by the sample mutex
</span><del>- bool unlocked;
</del><ins>+ bool m_unlocked { false };
</ins><span class="cx"> };
</span><span class="cx">
</span><ins>+struct _WebKitVideoSinkPrivate {
+ _WebKitVideoSinkPrivate()
+ {
+ gst_video_info_init(&info);
+ }
+
+ ~_WebKitVideoSinkPrivate()
+ {
+ if (currentCaps)
+ gst_caps_unref(currentCaps);
+ }
+
+ VideoRenderRequestScheduler scheduler;
+ GstVideoInfo info;
+ GstCaps* currentCaps;
+};
+
</ins><span class="cx"> #define webkit_video_sink_parent_class parent_class
</span><span class="cx"> G_DEFINE_TYPE_WITH_CODE(WebKitVideoSink, webkit_video_sink, GST_TYPE_VIDEO_SINK, GST_DEBUG_CATEGORY_INIT(webkitVideoSinkDebug, "webkitsink", 0, "webkit video sink"));
</span><span class="cx">
</span><span class="lines">@@ -112,42 +169,20 @@
</span><span class="cx"> new (sink->priv) WebKitVideoSinkPrivate();
</span><span class="cx"> }
</span><span class="cx">
</span><del>-static void webkitVideoSinkTimeoutCallback(WebKitVideoSink* sink)
</del><ins>+static void webkitVideoSinkRepaintRequested(WebKitVideoSink* sink, GstSample* sample)
</ins><span class="cx"> {
</span><del>- WebKitVideoSinkPrivate* priv = sink->priv;
-
- WTF::GMutexLocker<GMutex> lock(priv->sampleMutex);
- GstSample* sample = priv->sample;
- priv->sample = 0;
-
- if (!sample || priv->unlocked || UNLIKELY(!GST_IS_SAMPLE(sample))) {
- g_cond_signal(&priv->dataCondition);
- return;
- }
-
</del><span class="cx"> g_signal_emit(sink, webkitVideoSinkSignals[REPAINT_REQUESTED], 0, sample);
</span><del>- gst_sample_unref(sample);
- g_cond_signal(&priv->dataCondition);
</del><span class="cx"> }
</span><span class="cx">
</span><del>-static GstFlowReturn webkitVideoSinkRender(GstBaseSink* baseSink, GstBuffer* buffer)
</del><ins>+static GRefPtr<GstSample> webkitVideoSinkRequestRender(WebKitVideoSink* sink, GstBuffer* buffer)
</ins><span class="cx"> {
</span><del>- WebKitVideoSink* sink = WEBKIT_VIDEO_SINK(baseSink);
</del><span class="cx"> WebKitVideoSinkPrivate* priv = sink->priv;
</span><ins>+ GRefPtr<GstSample> sample = adoptGRef(gst_sample_new(buffer, priv->currentCaps, nullptr, nullptr));
</ins><span class="cx">
</span><del>- WTF::GMutexLocker<GMutex> lock(priv->sampleMutex);
-
- if (priv->unlocked)
- return GST_FLOW_OK;
-
- priv->sample = gst_sample_new(buffer, priv->currentCaps, 0, 0);
-
</del><span class="cx"> // The video info structure is valid only if the sink handled an allocation query.
</span><span class="cx"> GstVideoFormat format = GST_VIDEO_INFO_FORMAT(&priv->info);
</span><del>- if (format == GST_VIDEO_FORMAT_UNKNOWN) {
- gst_sample_unref(priv->sample);
- return GST_FLOW_ERROR;
- }
</del><ins>+ if (format == GST_VIDEO_FORMAT_UNKNOWN)
+ return nullptr;
</ins><span class="cx">
</span><span class="cx"> #if !(USE(TEXTURE_MAPPER_GL) && !USE(COORDINATED_GRAPHICS))
</span><span class="cx"> // Cairo's ARGB has pre-multiplied alpha while GStreamer's doesn't.
</span><span class="lines">@@ -163,7 +198,7 @@
</span><span class="cx"> // Check if allocation failed.
</span><span class="cx"> if (UNLIKELY(!newBuffer)) {
</span><span class="cx"> gst_buffer_unref(buffer);
</span><del>- return GST_FLOW_ERROR;
</del><ins>+ return nullptr;
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> // We don't use Color::premultipliedARGBFromColor() here because
</span><span class="lines">@@ -174,14 +209,13 @@
</span><span class="cx"> GstVideoFrame destinationFrame;
</span><span class="cx">
</span><span class="cx"> if (!gst_video_frame_map(&sourceFrame, &priv->info, buffer, GST_MAP_READ)) {
</span><del>- gst_sample_unref(priv->sample);
</del><span class="cx"> gst_buffer_unref(newBuffer);
</span><del>- return GST_FLOW_ERROR;
</del><ins>+ return nullptr;
</ins><span class="cx"> }
</span><span class="cx"> if (!gst_video_frame_map(&destinationFrame, &priv->info, newBuffer, GST_MAP_WRITE)) {
</span><span class="cx"> gst_video_frame_unmap(&sourceFrame);
</span><span class="cx"> gst_buffer_unref(newBuffer);
</span><del>- return GST_FLOW_ERROR;
</del><ins>+ return nullptr;
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> const guint8* source = static_cast<guint8*>(GST_VIDEO_FRAME_PLANE_DATA(&sourceFrame, 0));
</span><span class="lines">@@ -209,20 +243,17 @@
</span><span class="cx">
</span><span class="cx"> gst_video_frame_unmap(&sourceFrame);
</span><span class="cx"> gst_video_frame_unmap(&destinationFrame);
</span><del>- gst_sample_unref(priv->sample);
- priv->sample = gst_sample_new(newBuffer, priv->currentCaps, 0, 0);
</del><ins>+ sample = adoptGRef(gst_sample_new(newBuffer, priv->currentCaps, nullptr, nullptr));
</ins><span class="cx"> }
</span><span class="cx"> #endif
</span><span class="cx">
</span><del>- // This should likely use a lower priority, but glib currently starves
- // lower priority sources.
- // See: https://bugzilla.gnome.org/show_bug.cgi?id=610830.
- GRefPtr<WebKitVideoSink> protector(sink);
- priv->timeoutSource.schedule("[WebKit] webkitVideoSinkTimeoutCallback",
- std::function<void()>([protector] { webkitVideoSinkTimeoutCallback(protector.get()); }));
</del><ins>+ return sample;
+}
</ins><span class="cx">
</span><del>- g_cond_wait(&priv->dataCondition, &priv->sampleMutex);
- return GST_FLOW_OK;
</del><ins>+static GstFlowReturn webkitVideoSinkRender(GstBaseSink* baseSink, GstBuffer* buffer)
+{
+ WebKitVideoSink* sink = WEBKIT_VIDEO_SINK(baseSink);
+ return sink->priv->scheduler.requestRender(sink, buffer) ? GST_FLOW_OK : GST_FLOW_ERROR;
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> static void webkitVideoSinkFinalize(GObject* object)
</span><span class="lines">@@ -231,25 +262,11 @@
</span><span class="cx"> G_OBJECT_CLASS(parent_class)->finalize(object);
</span><span class="cx"> }
</span><span class="cx">
</span><del>-static void unlockSampleMutex(WebKitVideoSinkPrivate* priv)
-{
- WTF::GMutexLocker<GMutex> lock(priv->sampleMutex);
-
- if (priv->sample) {
- gst_sample_unref(priv->sample);
- priv->sample = 0;
- }
-
- priv->unlocked = true;
-
- g_cond_signal(&priv->dataCondition);
-}
-
</del><span class="cx"> static gboolean webkitVideoSinkUnlock(GstBaseSink* baseSink)
</span><span class="cx"> {
</span><del>- WebKitVideoSink* sink = WEBKIT_VIDEO_SINK(baseSink);
</del><ins>+ WebKitVideoSinkPrivate* priv = WEBKIT_VIDEO_SINK(baseSink)->priv;
</ins><span class="cx">
</span><del>- unlockSampleMutex(sink->priv);
</del><ins>+ priv->scheduler.stop();
</ins><span class="cx">
</span><span class="cx"> return GST_CALL_PARENT_WITH_DEFAULT(GST_BASE_SINK_CLASS, unlock, (baseSink), TRUE);
</span><span class="cx"> }
</span><span class="lines">@@ -258,10 +275,7 @@
</span><span class="cx"> {
</span><span class="cx"> WebKitVideoSinkPrivate* priv = WEBKIT_VIDEO_SINK(baseSink)->priv;
</span><span class="cx">
</span><del>- {
- WTF::GMutexLocker<GMutex> lock(priv->sampleMutex);
- priv->unlocked = false;
- }
</del><ins>+ priv->scheduler.start();
</ins><span class="cx">
</span><span class="cx"> return GST_CALL_PARENT_WITH_DEFAULT(GST_BASE_SINK_CLASS, unlock_stop, (baseSink), TRUE);
</span><span class="cx"> }
</span><span class="lines">@@ -270,11 +284,10 @@
</span><span class="cx"> {
</span><span class="cx"> WebKitVideoSinkPrivate* priv = WEBKIT_VIDEO_SINK(baseSink)->priv;
</span><span class="cx">
</span><del>- unlockSampleMutex(priv);
-
</del><ins>+ priv->scheduler.stop();
</ins><span class="cx"> if (priv->currentCaps) {
</span><span class="cx"> gst_caps_unref(priv->currentCaps);
</span><del>- priv->currentCaps = 0;
</del><ins>+ priv->currentCaps = nullptr;
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> return TRUE;
</span><span class="lines">@@ -284,8 +297,8 @@
</span><span class="cx"> {
</span><span class="cx"> WebKitVideoSinkPrivate* priv = WEBKIT_VIDEO_SINK(baseSink)->priv;
</span><span class="cx">
</span><del>- WTF::GMutexLocker<GMutex> lock(priv->sampleMutex);
- priv->unlocked = false;
</del><ins>+ priv->scheduler.start();
+
</ins><span class="cx"> return TRUE;
</span><span class="cx"> }
</span><span class="cx">
</span></span></pre>
</div>
</div>
</body>
</html>