[Webkit-unassigned] [Bug 115364] [SOUP] Move default buffer handling from ResourceHandleClient to ResourceHandleSoup

[bugzilla-daemon at webkit.org]

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





--- Comment #3 from Carlos Garcia Campos <cgarcia at igalia.com>  2013-04-30 00:50:18 PST ---
(In reply to comment #2)
> (From update of attachment 200029 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=200029&action=review
> 
> This is a really good change! I have just a couple small questions.
> 
> > Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp:56
> > -        virtual char* getBuffer(int, int*);
> > +        virtual char* getOrCreateReadBuffer(size_t requestedSize, size_t& actualSize);
> 
> I wonder if this should return a PassRefPtr<SharedBuffer> which would make it more similar to the rest of the networking code and allow for removing the actualSize argument.

This was added for performance issues when reading media streams. What we want is a pointer to a buffer that can be used with read operations (g_input_stream_read_async). A shared buffer has its own memory handling and a lot of logic we don't really need. What we want here is that the GST source element can create a GstBuffer and pass the pointer to the network layer to be used directly for reading, avoiding any intermediate allocation-copy-deallocation. I think other networks bakends receive already a buffer from their network layer, so they have to copy it anyway. In such cases a SharedBuffer is perfect to fill it with data and read it incrementally. 

> Another small comment is that you probably want this to be called createBuffer. It seems that the ResourceHandle is always responsible for freeing the buffer (unless I have misread this patch), so this method is essentially a constructor since it passes ownership. That's another reason why I suggested a return value wrapped in PassRefPtr above. If we had a PassGOwnPtr it would make sense here. In fact, this reminds me to investigate removing GOwnPtr completely again. :)

You have misread the patch :-) The method is in the client and the owner of the buffer is the client. But if the client doesn't provide a buffer, the resource handle creates its own one, and is the owner of that one, of course. Depending on how the buffer handling is implemented, the function will return a new buffer or reuse a existing one (like we do with the default buffer), that's why it's called getOrCreateReadBuffer.

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