Re: buffer management [was Re: libsoup-2.4 branch merged to trunk]

[Wouter Cloetens <wouter mind be>]

On Tue, Jan 29, 2008 at 08:57:11AM -0500, Dan Winship wrote:
> Wouter Cloetens wrote:
> > One thing I'm still missing is application buffer management. I'd like
> > to be able to (1) control the maximum size of the chunks and (2) provide
> > pre-allocated buffers to eliminate copying data.

(1) RESPONSE_BLOCK_SIZE in soup-message-io.c is the first issue. I would
make this dynamic by adding a buffer-size property, to end up in
SoupMessageIOData. Not for read_metadata() perhaps, but certainly for
read_body_chunk().

(2) 
> Another possibility would be to have a "gst_buffer_set_free_func()" or
> something, so that you could have the GstBuffer share the memory with
> the SoupBuffer directly, and then free the SoupBuffer when the GstBuffer
> is freed.

You'll have to dynamically allocate it then, instead of allocating it on
the stack.
GstBuffer allows me to provide a pointer to the buffer data, the length
of the data, and also a pointer to the allocated memory. When the GstBuffer
is no longer needed, g_free() is called on that last pointer. That's a
neat feature, because it allows you to specify that the useable data
starts at a certain offset in the allocated buffer, avoiding another
copy.
A simple alternative to the SOUP_MESSAGE_OVERWRITE_CHUNKS flag, e.g.
SOUP_MESSAGE_MALLOC_CHUNKS, could specify the new behaviour of
malloc'ing the buffer and letting the got-chunk signal callback worry
about freeing it. That may not correspond nicely to the SoupBuffer
logic, but AFAICT, none of its functionality is needed in this
particular instance.

> Benjamin Otte is running into the same problem in swfdec (except worse,
> because in some cases he ends up streaming from SoupBuffers to
> GstBuffers via SwfdecBuffers). We were talking on #gnome-hackers the
> other day about the possibility of having a refcounted buffer type
> directly in glib.

Oh that would be cool.

> Anyway, I'm still trying to figure out exactly how an API like this
> would work... currently the code isn't really written with that
> possibility in mind,

The suggestion above seems straightforward. API impact is also limited
to a new property and a new flag, neither of which break backward
compatibility.

> although the addition of GCancellable args to the
> SoupSocket calls will make it easier I think...
> 
> I'm assuming it would probably involve a new SoupSession subclass with
> some methods sort of like neon's
> ne_begin_request/ne_read_response_block, but if you have different ideas
> of what you want the API to look like, chime in (or write a "this is the
> patch I'd like to be able to apply to souphttpsrc" patch. :)

How does my simple suggestion sound? I'm willing to brew some code.

bfn, Wouter