Bug 34317 – [GStreamer] Add a WebKit HTTP/HTTPS source that uses WebKit's network infrastructure

• Status: RESOLVED
• Resolution: FIXED
• Component: Media

[Sebastian Dröge (slomo)]

Hi,
the attached patch adds a WebKit HTTP/HTTPS GStreamer source, that uses WebKit's network infrastructure. There's only two places where soup specific things are done, which can probably be replaced too. As such this should be useful for other ports too if they decide to use GStreamer at some point.

It automatically sets cookies as required, uses proxy/authentication/certificate information from WebKit, sets referer, etc. It also works with Vimeo's HTML5 stuff.

Please test this and report any issues, it's still not 100% completed.

[Sebastian Dröge (slomo)]

Created attachment 47689 [details]
0001-Add-webkitwebsrc-a-GStreamer-HTTP-HTTPS-source-using.patch

[Sebastian Dröge (slomo)]

One issue here right now is bug #34318 and that seeking on vimeo.com doesn't work: They don't allow range requests but don't say in the initial response that it's not supported. Seeking on vimeo.com thus silently stops playback.

[Sebastian Dröge (slomo)]

Created attachment 47760 [details]
0001-Add-webkitwebsrc-a-GStreamer-HTTP-HTTPS-source-using.patch

This now seems to work as good as the GStreamer HTTP/HTTPS sources.

[Sebastian Dröge (slomo)]

Created attachment 47761 [details]
0001-Add-webkitwebsrc-a-GStreamer-HTTP-HTTPS-source-using.patch

Rebased against latest master

[WebKit Review Bot]

Attachment 47760 [details] did not pass style-queue:

Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1
WebCore/platform/graphics/gtk/MediaPlayerPrivateGStreamer.cpp:46:  Alphabetical sorting problem.  [build/include_order] [4]
WebCore/platform/graphics/gtk/MediaPlayerPrivateGStreamer.cpp:145:  Tab found; better to use spaces  [whitespace/tab] [1]
Total errors found: 2


If any of these errors are false positives, please file a bug against check-webkit-style.

[WebKit Review Bot]

Attachment 47761 [details] did not pass style-queue:

Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1
Last 3072 characters of output:
gtk/WebKitWebSourceGStreamer.cpp:560:  webkit_web_src_set_frame is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/platform/graphics/gtk/WebKitWebSourceGStreamer.cpp:562:  Declaration has space between type name and * in WebkitWebSrcPrivate *priv  [whitespace/declaration] [3]
WebCore/platform/graphics/gtk/WebKitWebSourceGStreamer.cpp:584:  Declaration has space between type name and * in WebkitWebSrcPrivate *priv  [whitespace/declaration] [3]
WebCore/platform/graphics/gtk/WebKitWebSourceGStreamer.cpp:590:  Use 0 instead of NULL.  [readability/null] [5]
WebCore/platform/graphics/gtk/WebKitWebSourceGStreamer.cpp:602:  Extra space before ( in function call  [whitespace/parens] [4]
WebCore/platform/graphics/gtk/WebKitWebSourceGStreamer.cpp:602:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
WebCore/platform/graphics/gtk/WebKitWebSourceGStreamer.cpp:607:  Declaration has space between type name and * in gchar *endptr  [whitespace/declaration] [3]
WebCore/platform/graphics/gtk/WebKitWebSourceGStreamer.cpp:607:  Use 0 instead of NULL.  [readability/null] [5]
WebCore/platform/graphics/gtk/WebKitWebSourceGStreamer.cpp:608:  icy_metaint is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/platform/graphics/gtk/WebKitWebSourceGStreamer.cpp:611:  Declaration has space between type name and * in GstCaps *caps  [whitespace/declaration] [3]
WebCore/platform/graphics/gtk/WebKitWebSourceGStreamer.cpp:611:  Use 0 instead of NULL.  [readability/null] [5]
WebCore/platform/graphics/gtk/WebKitWebSourceGStreamer.cpp:624:  Use 0 instead of NULL.  [readability/null] [5]
WebCore/platform/graphics/gtk/WebKitWebSourceGStreamer.cpp:631:  Use 0 instead of NULL.  [readability/null] [5]
WebCore/platform/graphics/gtk/WebKitWebSourceGStreamer.cpp:638:  Use 0 instead of NULL.  [readability/null] [5]
WebCore/platform/graphics/gtk/WebKitWebSourceGStreamer.cpp:645:  Use 0 instead of NULL.  [readability/null] [5]
WebCore/platform/graphics/gtk/WebKitWebSourceGStreamer.cpp:656:  Declaration has space between type name and * in WebkitWebSrcPrivate *priv  [whitespace/declaration] [3]
WebCore/platform/graphics/gtk/WebKitWebSourceGStreamer.cpp:665:  Declaration has space between type name and * in GstBuffer *buffer  [whitespace/declaration] [3]
WebCore/platform/graphics/gtk/WebKitWebSourceGStreamer.cpp:674:  Use 0 instead of NULL.  [readability/null] [5]
WebCore/platform/graphics/gtk/WebKitWebSourceGStreamer.cpp:675:  One line control clauses should not use braces.  [whitespace/braces] [4]
WebCore/platform/graphics/gtk/WebKitWebSourceGStreamer.cpp:687:  Use 0 instead of NULL.  [readability/null] [5]
WebCore/platform/graphics/gtk/MediaPlayerPrivateGStreamer.cpp:46:  Alphabetical sorting problem.  [build/include_order] [4]
WebCore/platform/graphics/gtk/MediaPlayerPrivateGStreamer.cpp:145:  Tab found; better to use spaces  [whitespace/tab] [1]
Total errors found: 138


If any of these errors are false positives, please file a bug against check-webkit-style.

[WebKit Review Bot]

Attachment 47760 [details] did not build on gtk:
Build output: http://webkit-commit-queue.appspot.com/results/220417

[Sebastian Dröge (slomo)]

Created attachment 47764 [details]
0001-Add-webkitwebsrc-a-GStreamer-HTTP-HTTPS-source-using.patch

Lots of style fixes... for the remaining ones, I don't think they should be fixed. Using camel case function names is not really GLib'ish and using 0 instead of NULL in varargs functions is not a good idea either (on x86-64 0 would be passed as 32 bit value while the function itself wants a 64 bit value).

[WebKit Review Bot]

Attachment 47764 [details] did not pass style-queue:

Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1
Last 3072 characters of output:
eadability/naming] [4]
WebCore/platform/graphics/gtk/WebKitWebSourceGStreamer.cpp:452:  webkit_web_src_uri_set_uri is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/platform/graphics/gtk/WebKitWebSourceGStreamer.cpp:457:  An else if statement should be written as an if statement when the prior "if" concludes with a return, break, continue or goto statement.  [readability/control_flow] [4]
WebCore/platform/graphics/gtk/WebKitWebSourceGStreamer.cpp:460:  Extra space before ( in function call  [whitespace/parens] [4]
WebCore/platform/graphics/gtk/WebKitWebSourceGStreamer.cpp:465:  Extra space before ( in function call  [whitespace/parens] [4]
WebCore/platform/graphics/gtk/WebKitWebSourceGStreamer.cpp:471:  webkit_web_src_uri_handler_init is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/platform/graphics/gtk/WebKitWebSourceGStreamer.cpp:483:  webkit_web_src_need_data_main_cb is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/platform/graphics/gtk/WebKitWebSourceGStreamer.cpp:496:  webkit_web_src_need_data_cb is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/platform/graphics/gtk/WebKitWebSourceGStreamer.cpp:508:  webkit_web_src_enough_data_main_cb is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/platform/graphics/gtk/WebKitWebSourceGStreamer.cpp:520:  webkit_web_src_enough_data_cb is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/platform/graphics/gtk/WebKitWebSourceGStreamer.cpp:532:  webkit_web_src_seek_main_cb is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/platform/graphics/gtk/WebKitWebSourceGStreamer.cpp:540:  webkit_web_src_seek_data_cb is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/platform/graphics/gtk/WebKitWebSourceGStreamer.cpp:563:  webkit_web_src_set_frame is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/platform/graphics/gtk/WebKitWebSourceGStreamer.cpp:614:  Use 0 instead of NULL.  [readability/null] [5]
WebCore/platform/graphics/gtk/WebKitWebSourceGStreamer.cpp:627:  Use 0 instead of NULL.  [readability/null] [5]
WebCore/platform/graphics/gtk/WebKitWebSourceGStreamer.cpp:634:  Use 0 instead of NULL.  [readability/null] [5]
WebCore/platform/graphics/gtk/WebKitWebSourceGStreamer.cpp:641:  Use 0 instead of NULL.  [readability/null] [5]
WebCore/platform/graphics/gtk/WebKitWebSourceGStreamer.cpp:648:  Use 0 instead of NULL.  [readability/null] [5]
WebCore/platform/graphics/gtk/WebKitWebSourceGStreamer.cpp:678:  One line control clauses should not use braces.  [whitespace/braces] [4]
WebCore/platform/graphics/gtk/MediaPlayerPrivateGStreamer.cpp:145:  Tab found; better to use spaces  [whitespace/tab] [1]
Total errors found: 45


If any of these errors are false positives, please file a bug against check-webkit-style.

[WebKit Review Bot]

Attachment 47764 [details] did not build on gtk:
Build output: http://webkit-commit-queue.appspot.com/results/221300

[Sebastian Dröge (slomo)]

Created attachment 47765 [details]
0001-Add-webkitwebsrc-a-GStreamer-HTTP-HTTPS-source-using.patch

...and now it should even compile again and some more style issues should be fixed.

[WebKit Review Bot]

Attachment 47765 [details] did not build on gtk:
Build output: http://webkit-commit-queue.appspot.com/results/221308

[WebKit Review Bot]

Attachment 47765 [details] did not pass style-queue:

Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1
Last 3072 characters of output:
r.cpp:433:  webkit_web_src_uri_get_type is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/platform/graphics/gtk/WebKitWebSourceGStreamer.cpp:438:  webkit_web_src_uri_get_protocols is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/platform/graphics/gtk/WebKitWebSourceGStreamer.cpp:445:  webkit_web_src_uri_get_uri is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/platform/graphics/gtk/WebKitWebSourceGStreamer.cpp:453:  webkit_web_src_uri_set_uri is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/platform/graphics/gtk/WebKitWebSourceGStreamer.cpp:463:  Extra space before ( in function call  [whitespace/parens] [4]
WebCore/platform/graphics/gtk/WebKitWebSourceGStreamer.cpp:468:  Extra space before ( in function call  [whitespace/parens] [4]
WebCore/platform/graphics/gtk/WebKitWebSourceGStreamer.cpp:474:  webkit_web_src_uri_handler_init is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/platform/graphics/gtk/WebKitWebSourceGStreamer.cpp:486:  webkit_web_src_need_data_main_cb is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/platform/graphics/gtk/WebKitWebSourceGStreamer.cpp:499:  webkit_web_src_need_data_cb is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/platform/graphics/gtk/WebKitWebSourceGStreamer.cpp:511:  webkit_web_src_enough_data_main_cb is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/platform/graphics/gtk/WebKitWebSourceGStreamer.cpp:523:  webkit_web_src_enough_data_cb is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/platform/graphics/gtk/WebKitWebSourceGStreamer.cpp:535:  webkit_web_src_seek_main_cb is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/platform/graphics/gtk/WebKitWebSourceGStreamer.cpp:543:  webkit_web_src_seek_data_cb is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/platform/graphics/gtk/WebKitWebSourceGStreamer.cpp:566:  webkit_web_src_set_frame is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/platform/graphics/gtk/WebKitWebSourceGStreamer.cpp:617:  Use 0 instead of NULL.  [readability/null] [5]
WebCore/platform/graphics/gtk/WebKitWebSourceGStreamer.cpp:630:  Use 0 instead of NULL.  [readability/null] [5]
WebCore/platform/graphics/gtk/WebKitWebSourceGStreamer.cpp:637:  Use 0 instead of NULL.  [readability/null] [5]
WebCore/platform/graphics/gtk/WebKitWebSourceGStreamer.cpp:644:  Use 0 instead of NULL.  [readability/null] [5]
WebCore/platform/graphics/gtk/WebKitWebSourceGStreamer.cpp:651:  Use 0 instead of NULL.  [readability/null] [5]
Total errors found: 42


If any of these errors are false positives, please file a bug against check-webkit-style.

[Sebastian Dröge (slomo)]

Created attachment 47766 [details]
0001-Add-webkitwebsrc-a-GStreamer-HTTP-HTTPS-source-using.patch

[WebKit Review Bot]

Attachment 47766 [details] did not pass style-queue:

Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1
Last 3072 characters of output:
y/naming] [4]
WebCore/platform/graphics/gtk/WebKitWebSourceGStreamer.cpp:389:  webkit_web_src_change_state is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/platform/graphics/gtk/WebKitWebSourceGStreamer.cpp:433:  webkit_web_src_uri_get_type is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/platform/graphics/gtk/WebKitWebSourceGStreamer.cpp:438:  webkit_web_src_uri_get_protocols is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/platform/graphics/gtk/WebKitWebSourceGStreamer.cpp:445:  webkit_web_src_uri_get_uri is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/platform/graphics/gtk/WebKitWebSourceGStreamer.cpp:453:  webkit_web_src_uri_set_uri is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/platform/graphics/gtk/WebKitWebSourceGStreamer.cpp:474:  webkit_web_src_uri_handler_init is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/platform/graphics/gtk/WebKitWebSourceGStreamer.cpp:486:  webkit_web_src_need_data_main_cb is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/platform/graphics/gtk/WebKitWebSourceGStreamer.cpp:499:  webkit_web_src_need_data_cb is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/platform/graphics/gtk/WebKitWebSourceGStreamer.cpp:511:  webkit_web_src_enough_data_main_cb is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/platform/graphics/gtk/WebKitWebSourceGStreamer.cpp:523:  webkit_web_src_enough_data_cb is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/platform/graphics/gtk/WebKitWebSourceGStreamer.cpp:535:  webkit_web_src_seek_main_cb is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/platform/graphics/gtk/WebKitWebSourceGStreamer.cpp:543:  webkit_web_src_seek_data_cb is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/platform/graphics/gtk/WebKitWebSourceGStreamer.cpp:566:  webkit_web_src_set_frame is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/platform/graphics/gtk/WebKitWebSourceGStreamer.cpp:617:  Use 0 instead of NULL.  [readability/null] [5]
WebCore/platform/graphics/gtk/WebKitWebSourceGStreamer.cpp:630:  Use 0 instead of NULL.  [readability/null] [5]
WebCore/platform/graphics/gtk/WebKitWebSourceGStreamer.cpp:637:  Use 0 instead of NULL.  [readability/null] [5]
WebCore/platform/graphics/gtk/WebKitWebSourceGStreamer.cpp:644:  Use 0 instead of NULL.  [readability/null] [5]
WebCore/platform/graphics/gtk/WebKitWebSourceGStreamer.cpp:651:  Use 0 instead of NULL.  [readability/null] [5]
Total errors found: 40


If any of these errors are false positives, please file a bug against check-webkit-style.

[Philippe Normand]

Nice patch :) Had a quick look at the code and found a little thing to fix, the error message if appsrc could not be created mentions giostreamsrc ;)

[Sebastian Dröge (slomo)]

(In reply to comment #16)
> Nice patch :) Had a quick look at the code and found a little thing to fix, the
> error message if appsrc could not be created mentions giostreamsrc ;)

Oh, how embarrassing. Thanks, I'll fix this next time :)

[Holger Freyther]

Comment on attachment 47766 [details]
0001-Add-webkitwebsrc-a-GStreamer-HTTP-HTTPS-source-using.patch


> +	-gstapp-0.10

Are you sure, you don't want -lgstapp-0.10 here?



> +    priv->appsrc = GST_APP_SRC(gst_element_factory_make("appsrc", 0));
> +    if (!priv->appsrc) {
> +        GST_ERROR_OBJECT(src, "Failed to create giostreamsrc");
> +        return;
> +    }

> +static void webkit_web_src_stop(WebkitWebSrc* src, gboolean resetRequestedOffset)
> +{

> +    gst_app_src_set_caps(priv->appsrc, 0);

Does this mix well? Will it crash or just print a critical warning? Or will this never happen because the src_change_state will report a missing plugin and then stop is never called?

The next question is can the stop method be called multiple times? Or is there any gurantee that GStreamer will not do this?

[Sebastian Dröge (slomo)]

(In reply to comment #18)
> (From update of attachment 47766 [details])
> 
> > +	-gstapp-0.10
> 
> Are you sure, you don't want -lgstapp-0.10 here?

Yes, that's what I wanted. I wonder why it worked nonetheless...

> > +    priv->appsrc = GST_APP_SRC(gst_element_factory_make("appsrc", 0));
> > +    if (!priv->appsrc) {
> > +        GST_ERROR_OBJECT(src, "Failed to create giostreamsrc");
> > +        return;
> > +    }
> 
> > +static void webkit_web_src_stop(WebkitWebSrc* src, gboolean resetRequestedOffset)
> > +{
> 
> > +    gst_app_src_set_caps(priv->appsrc, 0);
> 
> Does this mix well? Will it crash or just print a critical warning? Or will
> this never happen because the src_change_state will report a missing plugin and
> then stop is never called?

Good catch, actually _stop() is called from the instance initialization function (webkit_web_src_init) and there appsrc could be NULL.

> The next question is can the stop method be called multiple times? Or is there
> any gurantee that GStreamer will not do this?

It can be called multiple times and probably will, all state changes from PAUSED to READY will go through the stop method. That's why I check if the things to be freed are not NULL before freeing and set them to NULL afterwards

Anyway, I'll attach a new patch shortly with the above issues fixed. Thanks for reviewing :)

[Sebastian Dröge (slomo)]

Created attachment 47912 [details]
0001-Add-webkitwebsrc-a-GStreamer-HTTP-HTTPS-source-using.patch

[WebKit Review Bot]

Attachment 47912 [details] did not pass style-queue:

Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1
Last 3072 characters of output:
y/naming] [4]
WebCore/platform/graphics/gtk/WebKitWebSourceGStreamer.cpp:390:  webkit_web_src_change_state is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/platform/graphics/gtk/WebKitWebSourceGStreamer.cpp:434:  webkit_web_src_uri_get_type is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/platform/graphics/gtk/WebKitWebSourceGStreamer.cpp:439:  webkit_web_src_uri_get_protocols is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/platform/graphics/gtk/WebKitWebSourceGStreamer.cpp:446:  webkit_web_src_uri_get_uri is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/platform/graphics/gtk/WebKitWebSourceGStreamer.cpp:454:  webkit_web_src_uri_set_uri is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/platform/graphics/gtk/WebKitWebSourceGStreamer.cpp:475:  webkit_web_src_uri_handler_init is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/platform/graphics/gtk/WebKitWebSourceGStreamer.cpp:487:  webkit_web_src_need_data_main_cb is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/platform/graphics/gtk/WebKitWebSourceGStreamer.cpp:500:  webkit_web_src_need_data_cb is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/platform/graphics/gtk/WebKitWebSourceGStreamer.cpp:512:  webkit_web_src_enough_data_main_cb is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/platform/graphics/gtk/WebKitWebSourceGStreamer.cpp:524:  webkit_web_src_enough_data_cb is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/platform/graphics/gtk/WebKitWebSourceGStreamer.cpp:536:  webkit_web_src_seek_main_cb is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/platform/graphics/gtk/WebKitWebSourceGStreamer.cpp:544:  webkit_web_src_seek_data_cb is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/platform/graphics/gtk/WebKitWebSourceGStreamer.cpp:567:  webkit_web_src_set_frame is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/platform/graphics/gtk/WebKitWebSourceGStreamer.cpp:618:  Use 0 instead of NULL.  [readability/null] [5]
WebCore/platform/graphics/gtk/WebKitWebSourceGStreamer.cpp:631:  Use 0 instead of NULL.  [readability/null] [5]
WebCore/platform/graphics/gtk/WebKitWebSourceGStreamer.cpp:638:  Use 0 instead of NULL.  [readability/null] [5]
WebCore/platform/graphics/gtk/WebKitWebSourceGStreamer.cpp:645:  Use 0 instead of NULL.  [readability/null] [5]
WebCore/platform/graphics/gtk/WebKitWebSourceGStreamer.cpp:652:  Use 0 instead of NULL.  [readability/null] [5]
Total errors found: 40


If any of these errors are false positives, please file a bug against check-webkit-style.

[Sebastian Dröge (slomo)]

Created attachment 48220 [details]
0001-Add-webkitwebsrc-a-GStreamer-HTTP-HTTPS-source-using.patch

[WebKit Review Bot]

Attachment 48220 [details] did not pass style-queue:

Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1
Last 3072 characters of output:
y/naming] [4]
WebCore/platform/graphics/gtk/WebKitWebSourceGStreamer.cpp:390:  webkit_web_src_change_state is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/platform/graphics/gtk/WebKitWebSourceGStreamer.cpp:434:  webkit_web_src_uri_get_type is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/platform/graphics/gtk/WebKitWebSourceGStreamer.cpp:439:  webkit_web_src_uri_get_protocols is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/platform/graphics/gtk/WebKitWebSourceGStreamer.cpp:446:  webkit_web_src_uri_get_uri is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/platform/graphics/gtk/WebKitWebSourceGStreamer.cpp:454:  webkit_web_src_uri_set_uri is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/platform/graphics/gtk/WebKitWebSourceGStreamer.cpp:475:  webkit_web_src_uri_handler_init is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/platform/graphics/gtk/WebKitWebSourceGStreamer.cpp:487:  webkit_web_src_need_data_main_cb is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/platform/graphics/gtk/WebKitWebSourceGStreamer.cpp:500:  webkit_web_src_need_data_cb is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/platform/graphics/gtk/WebKitWebSourceGStreamer.cpp:512:  webkit_web_src_enough_data_main_cb is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/platform/graphics/gtk/WebKitWebSourceGStreamer.cpp:524:  webkit_web_src_enough_data_cb is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/platform/graphics/gtk/WebKitWebSourceGStreamer.cpp:536:  webkit_web_src_seek_main_cb is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/platform/graphics/gtk/WebKitWebSourceGStreamer.cpp:544:  webkit_web_src_seek_data_cb is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/platform/graphics/gtk/WebKitWebSourceGStreamer.cpp:567:  webkit_web_src_set_frame is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/platform/graphics/gtk/WebKitWebSourceGStreamer.cpp:618:  Use 0 instead of NULL.  [readability/null] [5]
WebCore/platform/graphics/gtk/WebKitWebSourceGStreamer.cpp:631:  Use 0 instead of NULL.  [readability/null] [5]
WebCore/platform/graphics/gtk/WebKitWebSourceGStreamer.cpp:638:  Use 0 instead of NULL.  [readability/null] [5]
WebCore/platform/graphics/gtk/WebKitWebSourceGStreamer.cpp:645:  Use 0 instead of NULL.  [readability/null] [5]
WebCore/platform/graphics/gtk/WebKitWebSourceGStreamer.cpp:652:  Use 0 instead of NULL.  [readability/null] [5]
Total errors found: 40


If any of these errors are false positives, please file a bug against check-webkit-style.

[Gustavo Noronha (kov)]

(In reply to comment #8)
> Created an attachment (id=47764) [details]
> 0001-Add-webkitwebsrc-a-GStreamer-HTTP-HTTPS-source-using.patch
> 
> Lots of style fixes... for the remaining ones, I don't think they should be
> fixed. Using camel case function names is not really GLib'ish and using 0
> instead of NULL in varargs functions is not a good idea either (on x86-64 0
> would be passed as 32 bit value while the function itself wants a 64 bit
> value).

(In reply to comment #8)
> Created an attachment (id=47764) [details]
> 0001-Add-webkitwebsrc-a-GStreamer-HTTP-HTTPS-source-using.patch
> 
> Lots of style fixes... for the remaining ones, I don't think they should be
> fixed. Using camel case function names is not really GLib'ish and using 0
> instead of NULL in varargs functions is not a good idea either (on x86-64 0
> would be passed as 32 bit value while the function itself wants a 64 bit
> value).

While I agree with you on this, if we plan to use glibish style on the grounds that this is a "stand-alone" component that we may want to extract some day, then this should be considered as a separate API, and moved to WebKit/gtk/, like the soup http auth dialog was.

[Gustavo Noronha (kov)]

Comment on attachment 48220 [details]
0001-Add-webkitwebsrc-a-GStreamer-HTTP-HTTPS-source-using.patch

 142         Frame* frame = mp->m_player->frameView() ?
 143 mp->m_player->frameView()->frame() : 0;

Keep these in a single line, or indent the second one appropriately =).

 567 void webkit_web_src_set_frame(WebkitWebSrc* src, WebCore::Frame* frame)
 568 {
 569     WebkitWebSrcPrivate* priv = src->priv;
 570 
 571     frame->ref();
 572     priv->frame = frame;
 573 }

Instead of using manual refing/unrefing, I think you should make the object's private frame a RefPtr<Frame>, then just assigning 0 to it will make it go away, when it's no longer needed.

 22 #include "CString.h"
[...]
 29 #include "ResourceRequest.h"
 30 #include "ResourceResponse.h"
 31 
 32 #include <gst/app/gstappsrc.h>
 33 #include <gst/pbutils/missing-plugins.h>

I know this pattern is used in a bunch of places, but the right thing to do here is to add the system includes in the same list as the webkit ones. See, for instance, WebCore/platform/mac/GeolocationServiceMac.mm.

 104 static void webkit_web_src_finalize(GObject* object);
 105 static void webkit_web_src_set_property(GObject* object, guint propID,
 106     const GValue* value, GParamSpec* pspec);
 107 static void webkit_web_src_get_property(GObject* object, guint propID,
 108     GValue* value, GParamSpec* pspec);
 109 static GstStateChangeReturn webkit_web_src_change_state(GstElement* element,
 110                                                         GstStateChange transition);

Indentation is weird here. I would prefer a single line for each, but just being consistent works for me =D. I have noticed indentation issues like these also in other functions, like base_init, and src_init, but I won't clutter the review with them, just give everything a pass, I'd say.

223     gst_app_src_set_max_bytes(priv->appsrc, 512 * 1024);

Why is this the maximum? Does this depend on the amount of data soup will get before each got-chunk, for instance?

 234     g_free(priv->uri);

 339 static gboolean webkit_web_src_start(WebkitWebSrc* src)

You can use bool here instead of gboolean, because this is internal.

 368         gchar* val = g_strdup_printf("bytes=%" G_GUINT64_FORMAT "-", priv->requestedOffset);

Use GOwnPtr<gchar>, and throw the g_free away =).

 432 /*** GSTURIHANDLER INTERFACE *************************************************/

This is very alien for WebKit code =D.

// GstUriHandler Interface.

looks more like it =)

 441     static gchar* protocols[] = {(gchar*) "http", (gchar*) "https", 0 };

You can use char*, and lose the casts.

 464     if (uri && !g_ascii_strncasecmp("http", uri, 4) && !g_ascii_strncasecmp("http", uri, 5)) {

Looks like one of them is missing an s. You may consider storing priv->uri as a SoupURI, btw, and using SOUP_URI_VALID_FOR_HTTP() here instead. This has the advantage of only storing URIs that soup will actually accept, and saving us the trouble of finding bugs later for using invalid-for-soup URIs.

Let me push what I already have (since you'll likely work on this soonish =P), and I'll add another comment with the rest.

[Sebastian Dröge (slomo)]

Created attachment 48229 [details]
0001-Add-webkitwebsrc-a-GStreamer-HTTP-HTTPS-source-using.patch

[WebKit Review Bot]

Attachment 48229 [details] did not pass style-queue:

Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1
WebCore/platform/graphics/gtk/WebKitWebSourceGStreamer.h:47:  webkit_web_src_get_type is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/platform/graphics/gtk/WebKitWebSourceGStreamer.cpp:25:  Alphabetical sorting problem.  [build/include_order] [4]
WebCore/platform/graphics/gtk/WebKitWebSourceGStreamer.cpp:28:  Alphabetical sorting problem.  [build/include_order] [4]
WebCore/platform/graphics/gtk/WebKitWebSourceGStreamer.cpp:121:  _doInit is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/platform/graphics/gtk/WebKitWebSourceGStreamer.cpp:135:  webkit_web_src_base_init is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/platform/graphics/gtk/WebKitWebSourceGStreamer.cpp:148:  webkit_web_src_class_init is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/platform/graphics/gtk/WebKitWebSourceGStreamer.cpp:203:  webkit_web_src_init is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/platform/graphics/gtk/WebKitWebSourceGStreamer.cpp:640:  Use 0 instead of NULL.  [readability/null] [5]
WebCore/platform/graphics/gtk/WebKitWebSourceGStreamer.cpp:653:  Use 0 instead of NULL.  [readability/null] [5]
WebCore/platform/graphics/gtk/WebKitWebSourceGStreamer.cpp:660:  Use 0 instead of NULL.  [readability/null] [5]
WebCore/platform/graphics/gtk/WebKitWebSourceGStreamer.cpp:667:  Use 0 instead of NULL.  [readability/null] [5]
WebCore/platform/graphics/gtk/WebKitWebSourceGStreamer.cpp:674:  Use 0 instead of NULL.  [readability/null] [5]
Total errors found: 12


If any of these errors are false positives, please file a bug against check-webkit-style.

[Sebastian Dröge (slomo)]

Created attachment 48231 [details]
0001-Add-webkitwebsrc-a-GStreamer-HTTP-HTTPS-source-using.patch

[WebKit Review Bot]

Attachment 48231 [details] did not pass style-queue:

Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1
WebCore/platform/graphics/gtk/WebKitWebSourceGStreamer.h:47:  webkit_web_src_get_type is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/platform/graphics/gtk/WebKitWebSourceGStreamer.cpp:135:  webkit_web_src_base_init is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/platform/graphics/gtk/WebKitWebSourceGStreamer.cpp:148:  webkit_web_src_class_init is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/platform/graphics/gtk/WebKitWebSourceGStreamer.cpp:203:  webkit_web_src_init is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/platform/graphics/gtk/WebKitWebSourceGStreamer.cpp:640:  Use 0 instead of NULL.  [readability/null] [5]
WebCore/platform/graphics/gtk/WebKitWebSourceGStreamer.cpp:653:  Use 0 instead of NULL.  [readability/null] [5]
WebCore/platform/graphics/gtk/WebKitWebSourceGStreamer.cpp:660:  Use 0 instead of NULL.  [readability/null] [5]
WebCore/platform/graphics/gtk/WebKitWebSourceGStreamer.cpp:667:  Use 0 instead of NULL.  [readability/null] [5]
WebCore/platform/graphics/gtk/WebKitWebSourceGStreamer.cpp:674:  Use 0 instead of NULL.  [readability/null] [5]
Total errors found: 9


If any of these errors are false positives, please file a bug against check-webkit-style.

[Gustavo Noronha (kov)]

Comment on attachment 48231 [details]
0001-Add-webkitwebsrc-a-GStreamer-HTTP-HTTPS-source-using.patch

Looks good. I'll land this manually, since there are a couple small typos I will fix before landing.

[Gustavo Noronha (kov)]

Landed as r54427.

[Eric Seidel (no email)]

Tiger started failing after this checkin:
http://build.webkit.org/results/Tiger%20Intel%20Release/r54427%20(8544)/results.html

But I don't think it's related to this change, rather to general run-webkit-tests issues which we've been seeing recently.  Probably related to the "run httpd tests last" changes.

CCing Brian who has been dealing with these failures on the Windows bot.

[Csaba Osztrogonác]

(In reply to comment #32)
> Tiger started failing after this checkin:
> http://build.webkit.org/results/Tiger%20Intel%20Release/r54427%20(8544)/results.html
> 
> But I don't think it's related to this change, rather to general
> run-webkit-tests issues which we've been seeing recently.  Probably related to
> the "run httpd tests last" changes.
> 
> CCing Brian who has been dealing with these failures on the Windows bot.

http://build.webkit.org/results/Tiger%20Intel%20Release/r54427%20(8544)/pywebsocket_log.txt said that "[2010-02-05 10:41:05,703] [CRITICAL] root: (48, 'Address already in use')"

I think it means, that websocket server stucked, it should be killed/restarted.