From bfc33f6b664906f008dae8220b1a9fa336ae582a Mon Sep 17 00:00:00 2001 From: Philippe Normand Date: Fri, 2 Oct 2009 11:09:12 +0200 Subject: [PATCH] sink: replace the idle with a timeout --- WebCore/ChangeLog | 19 +++++++ .../graphics/gtk/MediaPlayerPrivateGStreamer.cpp | 11 ---- .../platform/graphics/gtk/VideoSinkGStreamer.cpp | 58 ++++++++++++-------- 3 files changed, 53 insertions(+), 35 deletions(-) diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog index 8b07193..1dae1c8 100644 --- a/WebCore/ChangeLog +++ b/WebCore/ChangeLog @@ -8,6 +8,25 @@ * platform/qt/PlatformKeyboardEventQt.cpp: (WebCore::windowsKeyCodeForKeyEvent): +2009-10-05 Philippe Normand + + Reviewed by NOBODY (OOPS!). + + https://bugs.webkit.org/show_bug.cgi?id=29959 + [GTK] QoS support in the video sink + + Replace the idle with a timeout function in the sink. Allows + smoother video playback. This patch also adds basic QoS support + but we will improve things on that front progressively. + + * platform/graphics/gtk/VideoSinkGStreamer.cpp: + (webkit_video_sink_init): + (webkit_video_sink_timeout_func): + (destroy_timeout_data): + (webkit_video_sink_render): + (webkit_video_sink_dispose): + (webkit_video_sink_stop): + 2009-10-05 Fumitoshi Ukai Unreviewed build fix for ENABLE(WEB_SOCKETS) and v8. diff --git a/WebCore/platform/graphics/gtk/MediaPlayerPrivateGStreamer.cpp b/WebCore/platform/graphics/gtk/MediaPlayerPrivateGStreamer.cpp index 65c64b4..bf89376 100644 --- a/WebCore/platform/graphics/gtk/MediaPlayerPrivateGStreamer.cpp +++ b/WebCore/platform/graphics/gtk/MediaPlayerPrivateGStreamer.cpp @@ -174,17 +174,6 @@ MediaPlayerPrivate::~MediaPlayerPrivate() gst_object_unref(GST_OBJECT(m_playBin)); } - // FIXME: We should find a better way to handle the lifetime of this object; this is - // needed because the object is sometimes being destroyed inbetween a call to - // webkit_video_sink_render, and the idle it schedules. Adding a ref in - // webkit_video_sink_render that would be balanced by the idle is not an option, - // because in some cases the destruction of the sink may happen in time for the idle - // to be removed from the queue, so it may not run. It would also cause lots of ref - // counting churn (render/idle are called many times). This is an ugly race. - if (m_videoSink) { - g_idle_add(idleUnref, m_videoSink); - m_videoSink = 0; - } } void MediaPlayerPrivate::load(const String& url) diff --git a/WebCore/platform/graphics/gtk/VideoSinkGStreamer.cpp b/WebCore/platform/graphics/gtk/VideoSinkGStreamer.cpp index bb269cd..b734842 100644 --- a/WebCore/platform/graphics/gtk/VideoSinkGStreamer.cpp +++ b/WebCore/platform/graphics/gtk/VideoSinkGStreamer.cpp @@ -65,7 +65,6 @@ static guint webkit_video_sink_signals[LAST_SIGNAL] = { 0, }; struct _WebKitVideoSinkPrivate { cairo_surface_t* surface; - GAsyncQueue* async_queue; gboolean rgb_ordering; int width; int height; @@ -73,6 +72,10 @@ struct _WebKitVideoSinkPrivate { int fps_d; int par_n; int par_d; + GstBuffer* buffer; + guint timeout_id; + GMutex* buffer_mutex; + GCond* data_cond; }; #define _do_init(bla) \ @@ -102,25 +105,27 @@ webkit_video_sink_init(WebKitVideoSink* sink, WebKitVideoSinkClass* klass) WebKitVideoSinkPrivate* priv; sink->priv = priv = G_TYPE_INSTANCE_GET_PRIVATE(sink, WEBKIT_TYPE_VIDEO_SINK, WebKitVideoSinkPrivate); - priv->async_queue = g_async_queue_new(); + priv->data_cond = g_cond_new(); + priv->buffer_mutex = g_mutex_new(); } static gboolean -webkit_video_sink_idle_func(gpointer data) +webkit_video_sink_timeout_func(gpointer data) { WebKitVideoSink* sink = reinterpret_cast(data); WebKitVideoSinkPrivate* priv = sink->priv; GstBuffer* buffer; GstCaps* caps; GstVideoFormat* format; + guint timeout_id; gint par_n, par_d; gfloat par; gint bwidth, bheight; - if (!priv->async_queue) - return FALSE; + g_mutex_lock(priv->buffer_mutex); + buffer = priv->buffer; + timeout_id = priv->timeout_id; - buffer = (GstBuffer*)g_async_queue_try_pop(priv->async_queue); if (!buffer || G_UNLIKELY(!GST_IS_BUFFER(buffer))) return FALSE; @@ -136,7 +141,6 @@ webkit_video_sink_idle_func(gpointer data) par = (gfloat) par_n / (gfloat) par_d; - // TODO: consider priv->rgb_ordering? cairo_surface_t* src = cairo_image_surface_create_for_data(GST_BUFFER_DATA(buffer), CAIRO_FORMAT_RGB24, bwidth, bheight, @@ -151,9 +155,13 @@ webkit_video_sink_idle_func(gpointer data) cairo_rectangle(cr, 0, 0, priv->width, priv->height); cairo_fill(cr); cairo_destroy(cr); + + g_source_remove(timeout_id); gst_buffer_unref(buffer); - g_async_queue_unref(priv->async_queue); + priv->buffer = 0; + g_cond_signal(priv->data_cond); + g_mutex_unlock(priv->buffer_mutex); g_signal_emit(sink, webkit_video_sink_signals[REPAINT_REQUESTED], 0); return FALSE; @@ -165,9 +173,16 @@ webkit_video_sink_render(GstBaseSink* bsink, GstBuffer* buffer) WebKitVideoSink* sink = WEBKIT_VIDEO_SINK(bsink); WebKitVideoSinkPrivate* priv = sink->priv; - g_async_queue_ref(priv->async_queue); - g_async_queue_push(priv->async_queue, gst_buffer_ref(buffer)); - g_idle_add_full(G_PRIORITY_HIGH_IDLE, webkit_video_sink_idle_func, sink, 0); + g_mutex_lock(priv->buffer_mutex); + priv->buffer = gst_buffer_ref(buffer); + // use HIGH_IDLE+20 priority, like Gtk+ for redrawing operations + priv->timeout_id = g_timeout_add_full(G_PRIORITY_HIGH_IDLE + 20, 100, + webkit_video_sink_timeout_func, + sink, 0); + while(priv->buffer) + g_cond_wait(priv->data_cond, priv->buffer_mutex); + g_mutex_unlock(priv->buffer_mutex); + return GST_FLOW_OK; } @@ -225,9 +240,14 @@ webkit_video_sink_dispose(GObject* object) priv->surface = 0; } - if (priv->async_queue) { - g_async_queue_unref(priv->async_queue); - priv->async_queue = 0; + if (priv->data_cond) { + g_cond_free(priv->data_cond); + priv->data_cond = 0; + } + + if (priv->buffer_mutex) { + g_mutex_free(priv->buffer_mutex); + priv->buffer_mutex = 0; } G_OBJECT_CLASS(parent_class)->dispose(object); @@ -277,16 +297,6 @@ webkit_video_sink_stop(GstBaseSink* base_sink) { WebKitVideoSinkPrivate* priv = WEBKIT_VIDEO_SINK(base_sink)->priv; - g_async_queue_lock(priv->async_queue); - - /* Remove all remaining objects from the queue */ - while (GstBuffer* buffer = (GstBuffer*)g_async_queue_try_pop_unlocked(priv->async_queue)) - gst_buffer_unref(buffer); - - g_async_queue_unlock(priv->async_queue); - - g_idle_remove_by_data(base_sink); - return TRUE; } -- 1.6.0.4