RidgeRun / gst-interpipe

GStreamer plug-in for interpipeline communication
Other
143 stars 64 forks source link

Add parent meta to compensated buffers #132

Closed JassonRM closed 1 year ago

JassonRM commented 2 years ago

Adding parent meta to the compensated buffers avoided upstream bufferpools from trying to recycle the buffers before the memory was unreferenced. Solving issue #131.

michaelgruner commented 2 years ago

Thanks for the fix @JassonRM . However I don't understand how this fixes the original problem, the buffer is still not available, why isn't the pool allocating new buffers with this approach?

JassonRM commented 2 years ago

@michaelgruner the problem was because the original buffer was unreferenced but the underlying memory wasn't. So the bufferpool would try to recycle the buffer right away but fail due to the memory being used. Adding the parent meta the buffer is only recycled once all the child buffers have been unreferenced. And there is no need to allocate more since the bufferpool contains enough to keep recycling them after they stopped being used.

michaelgruner commented 2 years ago

This should also be true for RESTART_TIMESTAMP isn't it?

JassonRM commented 2 years ago

It is, I just added it. However, I was wondering why RESTART_TIMESTAMP never used gst_buffer_make_writable in the first place?

michaelgruner commented 2 years ago

It was an error, and it got past the reviews. Thanks for adding it.

JassonRM commented 2 years ago

Hi @michaelgruner @rrcarlosrodriguez, to remind you I don't have permission to merge this pull request.

dzhang28 commented 1 year ago

I think there might be some side-effect could be introduced by this change.

The gst_buffer_ref would increase the refcount of given buffer. This could cause the gst_buffer_make_writable always return a copy of given buffer which in some case is unnecessary and might degrade peformance.