<html>
<head>
<base href="https://bugs.webkit.org/" />
</head>
<body>
<p>
<div>
<b><a class="bz_bug_link
bz_status_NEW "
title="NEW - [GStreamer] extra-headers and keep-alive properties for HTTP source element"
href="https://bugs.webkit.org/show_bug.cgi?id=143480#c4">Comment # 4</a>
on <a class="bz_bug_link
bz_status_NEW "
title="NEW - [GStreamer] extra-headers and keep-alive properties for HTTP source element"
href="https://bugs.webkit.org/show_bug.cgi?id=143480">bug 143480</a>
from <span class="vcard"><a class="email" href="mailto:cgarcia@igalia.com" title="Carlos Garcia Campos <cgarcia@igalia.com>"> <span class="fn">Carlos Garcia Campos</span></a>
</span></b>
<pre>Comment on <span class=""><a href="attachment.cgi?id=250265&action=diff" name="attach_250265" title="patch">attachment 250265</a> <a href="attachment.cgi?id=250265&action=edit" title="patch">[details]</a></span>
patch
View in context: <a href="https://bugs.webkit.org/attachment.cgi?id=250265&action=review">https://bugs.webkit.org/attachment.cgi?id=250265&action=review</a>
<span class="quote">> Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp:112
> + bool keep_alive;</span >
this should be keepAlive
<span class="quote">> Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp:252
> + FALSE, (GParamFlags) (G_PARAM_READWRITE | G_PARAM_STATIC_STRINGS)));</span >
Use C++ style cast for GParamFlags
<span class="quote">> Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp:256
> + GST_TYPE_STRUCTURE, (GParamFlags) (G_PARAM_READWRITE | G_PARAM_STATIC_STRINGS)));</span >
Use C++ style cast for GParamFlags
<span class="quote">> Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp:270
> + priv->keep_alive = FALSE;</span >
keep_alive -> keepAlive, FALSE -> false But this is false already since the private struct is filled with 0 on allocation
<span class="quote">> Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp:334
> + if (priv->extraHeaders) {
> + gst_structure_free(priv->extraHeaders);
> + priv->extraHeaders = nullptr;
> + }</span >
Don't we have a GUniquePtr impl for GstStructure?
<span class="quote">> Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp:471
> +static gboolean webKitWebSrcSetExtraHeader(GQuark fieldId, const GValue* value, gpointer userData)</span >
This could be bool
<span class="quote">> Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp:474
> + ResourceRequest* request = static_cast<ResourceRequest*>(userData);
> + const gchar* fieldName = g_quark_to_string(fieldId);</span >
Move this to where it's first needed.
<span class="quote">> Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp:477
> + if (G_VALUE_TYPE(value) == G_TYPE_STRING)</span >
G_VALUE_HOLDS_STRING?
<span class="quote">> Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp:504
> + return FALSE;</span >
Do we want to return if any of the elements fail, or should we try with the rest?
<span class="quote">> Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp:506
> + } else if (G_VALUE_TYPE(value) == GST_TYPE_LIST) {</span >
I would return TRUE in previous if instead of the else
<span class="quote">> Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp:511
> + return FALSE;</span >
Do we want to return if any of the elements fail, or should we try with the rest?
<span class="quote">> Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp:513
> + }
> + } else</span >
Ditto</pre>
</div>
</p>
<hr>
<span>You are receiving this mail because:</span>
<ul>
<li>You are the assignee for the bug.</li>
</ul>
</body>
</html>