[Webkit-unassigned] [Bug 73743] [GStreamer] webkitwebsrc: use SubResourceLoader

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Jul 18 09:57:45 PDT 2013


https://bugs.webkit.org/show_bug.cgi?id=73743





--- Comment #22 from Carlos Garcia Campos <cgarcia at igalia.com>  2013-07-18 09:57:38 PST ---
(In reply to comment #21)
> (In reply to comment #20)
> > The platform specific code is indeed part of the ResourceHandle, the soup implementation allows the client to provide its own read buffer, which is used by the gstreamer backend to avoid buffer copies while loading media, but it's not a gstreamer feature. 
> 
> But you just said that it's a gstreamer feature.

It's a ResourceHandleSoup feature used by the streamer media backend.

> Each media backend tends to do things radically differently from each other, but we've successfully kept the implementation details of ours out of the general purpose, cross-platform loader code.
> 
> > The same way the mac port has platform specific support for loading the data in a CFArrayRef. 
> 
> Which is an unfortunate case and one that we hope to rectify going forward.
> 
> But also, note that this callback is *only* in SubresourceLoader, and it was not necessary to pipe it through to the entire CachedResource/CachedRawResource/CachedRawResourceClient stack of classes.
> 
> They are absolutely *not* a place for platform loading details.  It's wrong wrong wrong.

That was just an example, there's also a mac platform ifdef in CachedResource, tryReplaceEncodedData, that is only used by the network process. But anyway, as I said, I agree with you that ideally there shouldn't be platform ifdefs there. And that's why I also proposed to simply remove the #ifdefs, and leave it unimplemented in platforms that don't need it, or even add a feature flag. I think letting the client provide the read buffer is useful feature in any case.

> > Except for the headers, all other platform specific code is in platform-specific files. I don't see how the gstreamer loader can provide the read buffer without using the CachedRawResource client. I agree that ideally, there shouldn't be platform ifdefs in those files. Another possibility is adding getOrCreateReadBuffer unconditionally, but unimplemented in other platforms, since the method itself doesn't have any platform specific types.
> 
> Let me ask this - What advantage is gained by this bug?  The bug is titled "use SubresourceLoader", but why is that important?  Didn't things work just fine when this code was in ResourceHandle?  Why is this move important to even be contemplating?

Phil can probably answer it better than me. I started to work on this to reduce the amount of layering violations in gstreamer media player. Using CachedResourceLoader is still a layering violation, but also used by mac port, so it would be a common point to look for a solution valid for both ports. 
It gives us more control over mixed content and security related stuff, so It should allow us to unskip some layout tests (which I forgot to do it).
It also allows to track the progress of the media loaded in the inspector like any other resource. 
And also allows us to easily support other non HTTP sources like data or blob urls, see bug #102586.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.



More information about the webkit-unassigned mailing list