[Webkit-unassigned] [Bug 54628] [GStreamer] Add 'location' property in webkitwebsrc

[bugzilla-daemon at webkit.org]

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


Martin Robinson <mrobinson at webkit.org> changed:

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




--- Comment #5 from Martin Robinson <mrobinson at webkit.org>  2011-02-17 07:41:29 PST ---
(From update of attachment 82776)
View in context: https://bugs.webkit.org/attachment.cgi?id=82776&action=review

Looks good, but I have just a few small suggestions.

> WebCore/ChangeLog:14
> +2011-02-17  Andoni Morales Alastruey  <amorales at flumotion.com>
> +
> +        Reviewed by NOBODY (OOPS!).
> +
> +        Added 'location' property
> +        https://bugs.webkit.org/show_bug.cgi?id=54628
> +
> +        The 'location' property is used by gst_element_make_from_uri()
> +        to set the uri in the source element
> +
> +        * platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp:
> +        (webkit_web_src_class_init):
> +        (webKitWebSrcSetProperty):
> +        (webKitWebSrcGetProperty):

Please generate your ChangeLog with prepare-ChangeLog --bug <bugid>. Some of the tools will unfotunately choke if the format is incorrect.

> WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp:119
> +static gboolean webKitWebSrcSetUri(GstURIHandler* handler, const gchar* uri);
> +static const gchar* webKitWebSrcGetUri(GstURIHandler* handler);

The argument names aren't necessary here.

> WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp:206
> +    /* location property for gst_element_make_from_uri */

Please use complete sentences for comments.

> WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp:298
> +        webKitWebSrcSetUri((GstURIHandler *)src, g_value_get_string(value));

Please use either static_cast or reinterpret_cast where appropriate.

> WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp:328
> +        g_value_set_string(value, webKitWebSrcGetUri((GstURIHandler *) src));

Ditto.

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