[Webkit-unassigned] [Bug 29959] [GTK] QoS support in the video sink

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=29959


Gustavo Noronha (kov) <gns at gnome.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #40640|review?                     |review-
               Flag|                            |




--- Comment #5 from Gustavo Noronha (kov) <gns at gnome.org>  2009-10-08 16:27:52 PDT ---
(From update of attachment 40640)
Awesome stuff, and I'm happy to see the real fix for the race condition
happening! What do you think of moving this sink to WebKit/gtk/webkit/ like we
did with soup-dialog, by the way? The synchronization and object life time
handling look OK to me, I have just a couple nitpicks:

> +    // use HIGH_IDLE+20 priority, like Gtk+ for redrawing operations
> +    priv->timeout_id = g_timeout_add_full(G_PRIORITY_HIGH_IDLE + 20, 0,
> +                                          webkit_video_sink_timeout_func,
> +                                          gst_object_ref(sink),
> +                                          (GDestroyNotify) gst_object_unref);

Additional space on the cast, and I suggest a normal sentence in the comment,
with capital U and period =).

>  
>  static void
> -webkit_video_sink_finalize(GObject* object)
> +_unlock_buffer_mutex(WebKitVideoSinkPrivate* priv)
> +{
> +
> +    g_mutex_lock(priv->buffer_mutex);
> +

There seems to be an unnecessary line at the start of this function. Also, I
don't think we use this _name pattern anywhere else, so I'd suggest just
dropping the underscore here. You are already declaring it static, which should
be enough.

Thanks for your work on this!

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.