[webkit-reviews] review granted: [Bug 54627] [GStreamer] URI queries support in webkitwebsrc : [Attachment 82964] Patch

[bugzilla-daemon at webkit.org]

Martin Robinson <mrobinson at webkit.org> has granted Andoni <ylatuya at gmail.com>'s
request for review:
Bug 54627: [GStreamer] URI queries support in webkitwebsrc
https://bugs.webkit.org/show_bug.cgi?id=54627

Attachment 82964: Patch
https://bugs.webkit.org/attachment.cgi?id=82964&action=review

------- Additional Comments from Martin Robinson <mrobinson at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=82964&action=review

Great stuff. Philippe do you mind landing this with the corrected ChangeLog?

> WebCore/ChangeLog:11
> +	   No new tests. (OOPS!)

Just need to replace this with an explanation of why there are no tests.

> WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp:511
> +    WebKitWebSrc* src = WEBKIT_WEB_SRC(gst_pad_get_parent_element(pad));
> +    gboolean success;
> +
> +    switch (GST_QUERY_TYPE(query)) {
> +    case GST_QUERY_URI:
> +	   gst_query_set_uri(query, src->priv->uri);
> +	   success = TRUE;
> +	   break;
> +    default:
> +	   success = FALSE;
> +	   break;
> +    }
> +
> +    if (!success)
> +	   success = gst_element_query(GST_ELEMENT(src->priv->appsrc), query);
> +
> +    gst_object_unref(src);
> +    return success;

This looks fine, but adding a GRefPtr specialization for GstObjects would make
it even cleaner:

{
    GRefPtr<GstObject*> src =
adoptGRef(WEBKIT_WEB_SRC(gst_pad_get_parent_element(pad)));
    if (GST_QUERY_TYPE(query) == GST_QUERY_URI) {
	gst_query_set_uri(query, src->priv->uri)
	return TRUE;
    }
    return gst_element_query(GST_ELEMENT(src->priv->appsrc), query);
}