[webkit-reviews] review denied: [Bug 173306] [GStreamer] Spreaker live shows won't play : [Attachment 313420] Patch

[bugzilla-daemon at webkit.org]

Xabier Rodríguez Calvar <calvaris at igalia.com> has denied Charlie Turner
<cturner at igalia.com>'s request for review:
Bug 173306: [GStreamer] Spreaker live shows won't play
https://bugs.webkit.org/show_bug.cgi?id=173306

Attachment 313420: Patch

https://bugs.webkit.org/attachment.cgi?id=313420&action=review




--- Comment #3 from Xabier Rodríguez Calvar <calvaris at igalia.com> ---
Comment on attachment 313420
  --> https://bugs.webkit.org/attachment.cgi?id=313420
Patch

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

The problem of this structure is that there is going to be the case of
(parsedIcyMetaInt && metadataInterval > 0) being false and you want to set the
caps to nullptr there too. You need to rework this to avoid it.

> Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp:956
> +    String icyMetaInt =
response.httpHeaderField(HTTPHeaderName::IcyMetaInt);

icyMetaInt -> metadataIntervalAsString

> Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp:958
> +	   bool parsedIcyMetaInt;

isMetatadaIntervalParsed

> Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp:959
> +	   gint metadataInterval = icyMetaInt.toInt(&parsedIcyMetaInt);

Please use plain int.

> Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp:962
> +	       GRefPtr<GstCaps> caps = adoptGRef(
> +		   gst_caps_new_simple("application/x-icy",
"metadata-interval", G_TYPE_INT, metadataInterval, nullptr));

You can have this in just one line. No problem with long lines in WK.