[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 #19 from Sebastian Dröge (slomo) <slomo at coaxion.net> ---
(In reply to Mathieu Duponchelle from comment #13)
> (In reply to Sebastian Dröge (slomo) from comment #9)
> > Review of attachment 371192 [details] [review] [review]:
> > 
> > ::: gst-libs/gst/rtp/gstrtpbaseaudiopayload.c
> > @@ +160,3 @@
> > +    goto wrong_config;
> > +
> > +  self->size = size;
> > 
> > What if this size is bigger than our MTU here, or does not comply with the
> > min/max ptime?
> > 
> 
> The input buffers are pushed to an adapter, then chopped up into new
> buffers, which we don't resize in prepare_output_buffer () (buffer->pool !=
> self->pool).
> 
> As these new buffers are taken with take_fast, the memories are likely to be
> shared / resized further, causing the memory flag to be set and the buffers
> being discarded when released.

Ok then we just go into the normal code path again. Can you add a comment about
that somewhere in the code?

Also I'm not entirely sure this is correct. In theory upstream could take one
of our buffers, have the single reference and add memories or resize it or
whatever. But that does not change buffer->pool, does it?

> (In reply to Sebastian Dröge (slomo) from comment #10)
> > Review of attachment 371193 [details] [review] [review]:
> > 
> > alawenc does not output fixed-size buffers, but depending on the input
> > buffer size, or not?
> > 
> 
> That is correct, it is the reason why I documented the new MTU property as
> "Set the expected maximum size of output buffers". If the input buffers end
> up resulting in different-from-MTU buffers being output, the buffers are
> either resized to a lower size, causing the buffers to be released back to
> the buffer pool, or the pool is not used (with my latest audioencoder patch
> :P).

How would alawenc now behave on input that is bigger than the MTU? Would it
split buffers or ...? Would it do something with the buffer pool?

> > ::: gst-libs/gst/audio/gstaudioencoder.c
> > @@ +2953,3 @@
> > +  if (enc->priv->ctx.pool) {
> > +    GstFlowReturn ret =
> > +        gst_buffer_pool_acquire_buffer (enc->priv->ctx.pool, &buffer, NULL);
> > 
> > Should probably sanity-check that size <= max_size here
> 
> I've actually updated this to only acquire buffers from the pool if size <=
> self->pool_size.

I guess this answers my previous question :)

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