127376 2014-01-21 15:51:29 -0800 GStreamer not buffering loading on Windows 2018-03-02 07:37:39 -0800 1 1 1 Unclassified WebKit Media 528+ (Nightly build) Unspecified Unspecified RESOLVED INVALID P2 Normal --- 1 alex.christensen webkit-unassigned cgarcia commit-queue gustavo menard mrobinson pnormand oldest_to_newest 970456 0 alex.christensen 2014-01-21 15:51:29 -0800 I'm not sure what a blob protocol is, but this is the fix I'm using to prevent an assertion failure. It's not the cleanest fix, and I'm sure it breaks something else, but it fixes my assertion failure. 970457 1 221799 alex.christensen 2014-01-21 15:53:49 -0800 Created attachment 221799 Patch 970459 2 221799 mrobinson 2014-01-21 15:58:51 -0800 Comment on attachment 221799 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=221799&action=review > Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp:1022 > + DataBufferingPolicy bufferingPolicy = BufferData; You can see why we do this here: https://bugs.webkit.org/show_bug.cgi?id=102586 Perhaps Carlos has more information. He should probably review this patch. 970592 3 cgarcia 2014-01-21 23:29:20 -0800 (In reply to comment #0) > I'm not sure what a blob protocol is, but this is the fix I'm using to prevent an assertion failure. It's not the cleanest fix, and I'm sure it breaks something else, but it fixes my assertion failure. what assertion failure exactly? 970857 4 alex.christensen 2014-01-22 13:55:29 -0800 > what assertion failure exactly? ASSERT(m_options.dataBufferingPolicy == BufferData) in CachedRawResource::addDataBuffer is failing. I'm using curl instead of libsoup, and I'm having trouble with MediaPlayerPrivateGStreamer::fillTimerFired. Do you think this could be related? 972316 5 pnormand 2014-01-26 20:24:49 -0800 I suspect this bug uncovered a difference of implementation in the Soup/Curl backends. If that's the case it'd be better to fix the Curl backend instead of adding a workaround in the gstreamer backend. Do the blob layout tests pass fine in WinCairo? 972348 6 221799 cgarcia 2014-01-27 00:08:18 -0800 Comment on attachment 221799 Patch Yes, we really want to set the policy as not buffering to avoid buffer copies while reading the stream. I don't understand why that assert is hit even using curl, and I don't have a windows to debug it. CachedRawResource::addDataBuffer() should never be called when the policy is DoNotBuffer. The resource handle calls ResourceHandleclient::didReceiveData (what curl backend does) or ResourceHandleclient::didReceiveBuffer (what soup backend does) when it has read a data chunk. The SubresourceLoader then calls didReceiveDataOrBuffer() and passes it to the parent ResourceLoader. In ResourceLoader::didReceiveDataOrBuffer() calls addDataOrBuffer which returns early when the policy is DoNotBuffer to make sure the m_resourceData is not set. And at this point is when SubresourceLoader calls CachedResource::addBuffer or CachedResource::addData, depending on whether there's cached data for the resource (m_resourceData). So, the only think I can think of is either something is changing the buffering policy once the resource load has started, or the m_resourceData of the resource is not correctly cleaned up at some point. 986369 7 alex.christensen 2014-03-03 09:14:49 -0800 Commenting out all calls to gst_element_register in initializeGStreamerAndRegisterWebKitElements makes it buffer correctly and wait when it reaches the end of the downloaded buffer, but it always says it has 100% loaded because gst_query_parse_buffering_range in fillTimerFired always sets start and stop to -1. Using a gst_query_new_duration gives the correct duration, so I think it might be a problem with gst_query_new_buffering on Windows. Using m_bufferingPercentage instead of fillStatus works close to how it should, though, but there are some quirks. I don't think that's a good solution. 997278 8 alex.christensen 2014-04-03 13:27:20 -0700 For some reason, gst_query_parse_buffering_range always returns -1, but gst_structure_get_int works with "buffer-percent". There is also a "buffering-ranges" string in gstquark.c. Could these be used instead of querying? Is there a specific reason a timer is used instead of just relying on GstMessages? It's been this way for a long time. 997489 9 pnormand 2014-04-04 00:12:12 -0700 Which httpsrc element is used then? You need to make sure it has a GstQuery handler answering to the GST_QUERY_SCHEDULING query and setting the GST_SCHEDULING_FLAG_BANDWIDTH_LIMITED flag. If the element doesn't have that feature please send a patch to bugzilla.gnome.org. (In reply to comment #8) > For some reason, gst_query_parse_buffering_range always returns -1, but gst_structure_get_int works with "buffer-percent". There is also a "buffering-ranges" string in gstquark.c. Could these be used instead of querying? Is there a specific reason a timer is used instead of just relying on GstMessages? It's been this way for a long time. Yes of course there is a specific reason for this design, this is how on-disk buffering is supposed to be implemented and not to be confused with the GST_QUERY_BUFFERING which is used to know if the pipeline has enough data to handle efficient seeking. 1403215 10 pnormand 2018-03-02 07:37:39 -0800 Closing because the informations requested were not provided. Feel free to reopen if you want to provide answers :) 221799 2014-01-21 15:53:49 -0800 2014-01-27 00:08:16 -0800 Patch bug-127376-20140121165348.patch text/plain 1871 alex.christensen SW5kZXg6IFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDE2MjQ4MCkKKysrIFNvdXJjZS9XZWJDb3JlL0NoYW5n ZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDE0IEBACisyMDE0LTAxLTIxICBBbGV4IENo cmlzdGVuc2VuICA8YWNocmlzdGVuc2VuQHdlYmtpdC5vcmc+CisKKyAgICAgICAgRml4IGFzc2Vy dGlvbiBmYWlsdXJlIHdoZW4gbm90IGJ1ZmZlcmluZyB3aXRoIEdTdHJlYW1lciBvbiBXaW5kb3dz LgorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTI3Mzc2 CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgKiBwbGF0 Zm9ybS9ncmFwaGljcy9nc3RyZWFtZXIvV2ViS2l0V2ViU291cmNlR1N0cmVhbWVyLmNwcDoKKyAg ICAgICAgKENhY2hlZFJlc291cmNlU3RyZWFtaW5nQ2xpZW50OjpDYWNoZWRSZXNvdXJjZVN0cmVh bWluZ0NsaWVudCk6CisgICAgICAgIEFsd2F5cyB1c2UgQnVmZmVyRGF0YSBpbnN0ZWFkIG9mIERv Tm90QnVmZmVyRGF0YS4KKwogMjAxNC0wMS0yMSAgU2ltb24gRnJhc2VyICA8c2ltb24uZnJhc2Vy QGFwcGxlLmNvbT4KIAogICAgICAgICBSZW1vdmUgI2lmIFBMQVRGT1JNKElPUykgaW4gdmFyaW91 cyBwbGFjZXMgYXJvdW5kIGN1c3RvbUZpeGVkUG9zaXRpb25MYXlvdXRSZWN0KCkgY29kZQpJbmRl eDogU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vZ3JhcGhpY3MvZ3N0cmVhbWVyL1dlYktpdFdlYlNv dXJjZUdTdHJlYW1lci5jcHAKPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291cmNlL1dlYkNvcmUvcGxhdGZvcm0v Z3JhcGhpY3MvZ3N0cmVhbWVyL1dlYktpdFdlYlNvdXJjZUdTdHJlYW1lci5jcHAJKHJldmlzaW9u IDE2MjQ3OCkKKysrIFNvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL2dyYXBoaWNzL2dzdHJlYW1lci9X ZWJLaXRXZWJTb3VyY2VHU3RyZWFtZXIuY3BwCSh3b3JraW5nIGNvcHkpCkBAIC0xMDE5LDcgKzEw MTksNyBAQCB2b2lkIFN0cmVhbWluZ0NsaWVudDo6aGFuZGxlTm90aWZ5RmluaXNoCiBDYWNoZWRS ZXNvdXJjZVN0cmVhbWluZ0NsaWVudDo6Q2FjaGVkUmVzb3VyY2VTdHJlYW1pbmdDbGllbnQoV2Vi S2l0V2ViU3JjKiBzcmMsIENhY2hlZFJlc291cmNlTG9hZGVyKiByZXNvdXJjZUxvYWRlciwgY29u c3QgUmVzb3VyY2VSZXF1ZXN0JiByZXF1ZXN0KQogICAgIDogU3RyZWFtaW5nQ2xpZW50KHNyYykK IHsKLSAgICBEYXRhQnVmZmVyaW5nUG9saWN5IGJ1ZmZlcmluZ1BvbGljeSA9IHJlcXVlc3QudXJs KCkucHJvdG9jb2xJcygiYmxvYiIpID8gQnVmZmVyRGF0YSA6IERvTm90QnVmZmVyRGF0YTsKKyAg ICBEYXRhQnVmZmVyaW5nUG9saWN5IGJ1ZmZlcmluZ1BvbGljeSA9IEJ1ZmZlckRhdGE7CiAgICAg Q2FjaGVkUmVzb3VyY2VSZXF1ZXN0IGNhY2hlUmVxdWVzdChyZXF1ZXN0LCBSZXNvdXJjZUxvYWRl ck9wdGlvbnMoU2VuZENhbGxiYWNrcywgRG9Ob3RTbmlmZkNvbnRlbnQsIGJ1ZmZlcmluZ1BvbGlj eSwgRG9Ob3RBbGxvd1N0b3JlZENyZWRlbnRpYWxzLCBEb05vdEFza0NsaWVudEZvckNyb3NzT3Jp Z2luQ3JlZGVudGlhbHMsIERvU2VjdXJpdHlDaGVjaywgVXNlRGVmYXVsdE9yaWdpblJlc3RyaWN0 aW9uc0ZvclR5cGUpKTsKICAgICBtX3Jlc291cmNlID0gcmVzb3VyY2VMb2FkZXItPnJlcXVlc3RS YXdSZXNvdXJjZShjYWNoZVJlcXVlc3QpOwogICAgIGlmIChtX3Jlc291cmNlKQo=