[Webkit-unassigned] [Bug 143480] [GStreamer] extra-headers and keep-alive properties for HTTP source element

[bugzilla-daemon at webkit.org]

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

--- Comment #4 from Carlos Garcia Campos <cgarcia at igalia.com> ---
Comment on attachment 250265
  --> https://bugs.webkit.org/attachment.cgi?id=250265
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=250265&action=review

> Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp:112
> +    bool keep_alive;

this should be keepAlive

> Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp:252
> +            FALSE, (GParamFlags) (G_PARAM_READWRITE | G_PARAM_STATIC_STRINGS)));

Use C++ style cast for GParamFlags

> Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp:256
> +            GST_TYPE_STRUCTURE, (GParamFlags) (G_PARAM_READWRITE | G_PARAM_STATIC_STRINGS)));

Use C++ style cast for GParamFlags

> Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp:270
> +    priv->keep_alive = FALSE;

keep_alive -> keepAlive, FALSE -> false But this is false already since the private struct is filled with 0 on allocation

> Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp:334
> +    if (priv->extraHeaders) {
> +        gst_structure_free(priv->extraHeaders);
> +        priv->extraHeaders = nullptr;
> +    }

Don't we have a GUniquePtr impl for GstStructure?

> Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp:471
> +static gboolean webKitWebSrcSetExtraHeader(GQuark fieldId, const GValue* value, gpointer userData)

This could be bool

> Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp:474
> +    ResourceRequest* request = static_cast<ResourceRequest*>(userData);
> +    const gchar* fieldName = g_quark_to_string(fieldId);

Move this to where it's first needed.

> Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp:477
> +    if (G_VALUE_TYPE(value) == G_TYPE_STRING)

G_VALUE_HOLDS_STRING?

> Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp:504
> +                return FALSE;

Do we want to return if any of the elements fail, or should we try with the rest?

> Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp:506
> +    } else if (G_VALUE_TYPE(value) == GST_TYPE_LIST) {

I would return TRUE in previous if instead of the else

> Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp:511
> +                return FALSE;

Do we want to return if any of the elements fail, or should we try with the rest?

> Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp:513
> +        }
> +    } else

Ditto

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-unassigned/attachments/20150407/427ab21d/attachment.html>