Bug 34317 - [GStreamer] Add a WebKit HTTP/HTTPS source that uses WebKit's network infrastructure
Summary: [GStreamer] Add a WebKit HTTP/HTTPS source that uses WebKit's network infrast...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Media (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-01-29 01:48 PST by Sebastian Dröge (slomo)
Modified: 2010-02-05 13:04 PST (History)
9 users (show)

See Also:


Attachments
0001-Add-webkitwebsrc-a-GStreamer-HTTP-HTTPS-source-using.patch (29.94 KB, patch)
2010-01-29 01:48 PST, Sebastian Dröge (slomo)
no flags Details | Formatted Diff | Diff
0001-Add-webkitwebsrc-a-GStreamer-HTTP-HTTPS-source-using.patch (6.60 KB, patch)
2010-01-30 02:38 PST, Sebastian Dröge (slomo)
no flags Details | Formatted Diff | Diff
0001-Add-webkitwebsrc-a-GStreamer-HTTP-HTTPS-source-using.patch (32.39 KB, patch)
2010-01-30 02:43 PST, Sebastian Dröge (slomo)
no flags Details | Formatted Diff | Diff
0001-Add-webkitwebsrc-a-GStreamer-HTTP-HTTPS-source-using.patch (32.18 KB, patch)
2010-01-30 03:03 PST, Sebastian Dröge (slomo)
no flags Details | Formatted Diff | Diff
0001-Add-webkitwebsrc-a-GStreamer-HTTP-HTTPS-source-using.patch (32.21 KB, patch)
2010-01-30 03:12 PST, Sebastian Dröge (slomo)
no flags Details | Formatted Diff | Diff
0001-Add-webkitwebsrc-a-GStreamer-HTTP-HTTPS-source-using.patch (32.23 KB, patch)
2010-01-30 03:20 PST, Sebastian Dröge (slomo)
zecke: review-
Details | Formatted Diff | Diff
0001-Add-webkitwebsrc-a-GStreamer-HTTP-HTTPS-source-using.patch (32.29 KB, patch)
2010-02-02 01:14 PST, Sebastian Dröge (slomo)
no flags Details | Formatted Diff | Diff
0001-Add-webkitwebsrc-a-GStreamer-HTTP-HTTPS-source-using.patch (32.74 KB, patch)
2010-02-05 03:37 PST, Sebastian Dröge (slomo)
no flags Details | Formatted Diff | Diff
0001-Add-webkitwebsrc-a-GStreamer-HTTP-HTTPS-source-using.patch (33.84 KB, patch)
2010-02-05 06:38 PST, Sebastian Dröge (slomo)
no flags Details | Formatted Diff | Diff
0001-Add-webkitwebsrc-a-GStreamer-HTTP-HTTPS-source-using.patch (33.84 KB, patch)
2010-02-05 06:58 PST, Sebastian Dröge (slomo)
gns: review+
gns: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Sebastian Dröge (slomo) 2010-01-29 01:48:05 PST
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.
Comment 1 Sebastian Dröge (slomo) 2010-01-29 01:48:51 PST
Created attachment 47689 [details]
0001-Add-webkitwebsrc-a-GStreamer-HTTP-HTTPS-source-using.patch
Comment 2 Sebastian Dröge (slomo) 2010-01-29 01:53:09 PST
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.
Comment 3 Sebastian Dröge (slomo) 2010-01-30 02:38:01 PST
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.
Comment 4 Sebastian Dröge (slomo) 2010-01-30 02:43:44 PST
Created attachment 47761 [details]
0001-Add-webkitwebsrc-a-GStreamer-HTTP-HTTPS-source-using.patch

Rebased against latest master
Comment 5 WebKit Review Bot 2010-01-30 02:44:21 PST
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.
Comment 6 WebKit Review Bot 2010-01-30 02:45:06 PST
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.
Comment 7 WebKit Review Bot 2010-01-30 02:57:40 PST
Attachment 47760 [details] did not build on gtk:
Build output: http://webkit-commit-queue.appspot.com/results/220417
Comment 8 Sebastian Dröge (slomo) 2010-01-30 03:03:51 PST
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).
Comment 9 WebKit Review Bot 2010-01-30 03:05:58 PST
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.
Comment 10 WebKit Review Bot 2010-01-30 03:07:31 PST
Attachment 47764 [details] did not build on gtk:
Build output: http://webkit-commit-queue.appspot.com/results/221300
Comment 11 Sebastian Dröge (slomo) 2010-01-30 03:12:25 PST
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.
Comment 12 WebKit Review Bot 2010-01-30 03:14:47 PST
Attachment 47765 [details] did not build on gtk:
Build output: http://webkit-commit-queue.appspot.com/results/221308
Comment 13 WebKit Review Bot 2010-01-30 03:16:33 PST
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.
Comment 14 Sebastian Dröge (slomo) 2010-01-30 03:20:16 PST
Created attachment 47766 [details]
0001-Add-webkitwebsrc-a-GStreamer-HTTP-HTTPS-source-using.patch
Comment 15 WebKit Review Bot 2010-01-30 03:22:04 PST
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.
Comment 16 Philippe Normand 2010-02-01 00:12:13 PST
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 ;)
Comment 17 Sebastian Dröge (slomo) 2010-02-01 01:46:26 PST
(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 :)
Comment 18 Holger Freyther 2010-02-02 00:49:38 PST
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?
Comment 19 Sebastian Dröge (slomo) 2010-02-02 01:10:50 PST
(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 :)
Comment 20 Sebastian Dröge (slomo) 2010-02-02 01:14:47 PST
Created attachment 47912 [details]
0001-Add-webkitwebsrc-a-GStreamer-HTTP-HTTPS-source-using.patch
Comment 21 WebKit Review Bot 2010-02-02 01:16:36 PST
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.
Comment 22 Sebastian Dröge (slomo) 2010-02-05 03:37:13 PST
Created attachment 48220 [details]
0001-Add-webkitwebsrc-a-GStreamer-HTTP-HTTPS-source-using.patch
Comment 23 WebKit Review Bot 2010-02-05 03:39:54 PST
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.
Comment 24 Gustavo Noronha (kov) 2010-02-05 04:02:05 PST
(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.
Comment 25 Gustavo Noronha (kov) 2010-02-05 05:22:38 PST
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.
Comment 26 Sebastian Dröge (slomo) 2010-02-05 06:38:48 PST
Created attachment 48229 [details]
0001-Add-webkitwebsrc-a-GStreamer-HTTP-HTTPS-source-using.patch
Comment 27 WebKit Review Bot 2010-02-05 06:43:44 PST
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.
Comment 28 Sebastian Dröge (slomo) 2010-02-05 06:58:52 PST
Created attachment 48231 [details]
0001-Add-webkitwebsrc-a-GStreamer-HTTP-HTTPS-source-using.patch
Comment 29 WebKit Review Bot 2010-02-05 07:02:45 PST
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.
Comment 30 Gustavo Noronha (kov) 2010-02-05 09:13:23 PST
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.
Comment 31 Gustavo Noronha (kov) 2010-02-05 09:25:32 PST
Landed as r54427.
Comment 32 Eric Seidel (no email) 2010-02-05 12:45:37 PST
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.
Comment 33 Csaba Osztrogonác 2010-02-05 13:04:18 PST
(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.