[Bug 795021] Reduce amount of memory allocations when payloading fixed-size audio buffers

[GStreamer (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=795021

--- Comment #18 from Mathieu Duponchelle <mduponchelle1 at gmail.com> ---
> (In reply to Nicolas Dufresne (stormer) from comment #12)
> > Review of attachment 371192 [details] [review] [review]:
> > 
> > ::: gst-libs/gst/rtp/gstrtpbaseaudiopayload.c
> > @@ +130,3 @@
> > +   * released to the pool again, see the resize code in
> > +   * gst_rtp_buffer_pool_acquire_buffer */
> > +  gst_buffer_resize (buffer, offset * -1, size + offset);
> > 
> > You implemented that in GstBufferPool:
> >   gst_buffer_resize (buffer, 0, pool->priv->size);
> > 
> > Which now that I think of is incorrect, and now your are hiding this bug
> > here. You should drop this reset code and fix GstBufferPool instead.
> 
> I actually didn't implement the patch you were thinking of as discussed on
> IRC, I'll propose my amendment to that patch here, you are right that we
> shouldn't need to manipulate the offset here. The size however will still
> need to be manipulated, as at this point we are resetting the buffer to the
> state that acquire_buffer expects, which is the same as after alloc_buffer.


Disregard that last part, as it turns out the resize code in the reset_buffer
implementation, was doing exactly nothing, as the offset was 0 here already, so
in effect we were calling gst_buffer_resize (buffer, 0, size + 0);

As it turns out, we cannot resize here to pool_size + header_len anyway,
because the release function runs after that and discards the buffer as it has
a different size. Instead I updated the patch to simply ensure the buffer size
is as expected in acquire_buffer, before the call to initialize_header.

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.