Description
Sebastian Dröge (slomo)
2010-01-29 01:48:05 PST
Created attachment 47689 [details]
0001-Add-webkitwebsrc-a-GStreamer-HTTP-HTTPS-source-using.patch
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. 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.
Created attachment 47761 [details]
0001-Add-webkitwebsrc-a-GStreamer-HTTP-HTTPS-source-using.patch
Rebased against latest master
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.
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.
Attachment 47760 [details] did not build on gtk: Build output: http://webkit-commit-queue.appspot.com/results/220417 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).
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.
Attachment 47764 [details] did not build on gtk: Build output: http://webkit-commit-queue.appspot.com/results/221300 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.
Attachment 47765 [details] did not build on gtk: Build output: http://webkit-commit-queue.appspot.com/results/221308 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.
Created attachment 47766 [details]
0001-Add-webkitwebsrc-a-GStreamer-HTTP-HTTPS-source-using.patch
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.
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 ;) (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 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? (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 :) Created attachment 47912 [details]
0001-Add-webkitwebsrc-a-GStreamer-HTTP-HTTPS-source-using.patch
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.
Created attachment 48220 [details]
0001-Add-webkitwebsrc-a-GStreamer-HTTP-HTTPS-source-using.patch
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.
(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 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.
Created attachment 48229 [details]
0001-Add-webkitwebsrc-a-GStreamer-HTTP-HTTPS-source-using.patch
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.
Created attachment 48231 [details]
0001-Add-webkitwebsrc-a-GStreamer-HTTP-HTTPS-source-using.patch
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 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.
Landed as r54427. 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. (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. |