Bug 127376

Summary: GStreamer not buffering loading on Windows
Product: WebKit Reporter: Alex Christensen <alex.christensen>
Component: MediaAssignee: Nobody <webkit-unassigned>
Status: RESOLVED INVALID    
Severity: Normal CC: cgarcia, commit-queue, gustavo, menard, mrobinson, pnormand
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch cgarcia: review-

Alex Christensen
Reported 2014-01-21 15:51:29 PST
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.
Attachments
Patch (1.83 KB, patch)
2014-01-21 15:53 PST, Alex Christensen
cgarcia: review-
Alex Christensen
Comment 1 2014-01-21 15:53:49 PST
Martin Robinson
Comment 2 2014-01-21 15:58:51 PST
Comment on attachment 221799 [details] 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.
Carlos Garcia Campos
Comment 3 2014-01-21 23:29:20 PST
(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?
Alex Christensen
Comment 4 2014-01-22 13:55:29 PST
> 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?
Philippe Normand
Comment 5 2014-01-26 20:24:49 PST
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?
Carlos Garcia Campos
Comment 6 2014-01-27 00:08:18 PST
Comment on attachment 221799 [details] 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.
Alex Christensen
Comment 7 2014-03-03 09:14:49 PST
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.
Alex Christensen
Comment 8 2014-04-03 13:27:20 PDT
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.
Philippe Normand
Comment 9 2014-04-04 00:12:12 PDT
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.
Philippe Normand
Comment 10 2018-03-02 07:37:39 PST
Closing because the informations requested were not provided. Feel free to reopen if you want to provide answers :)
Note You need to log in before you can comment on or make changes to this bug.