After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 744303 - bufferpool: Don't free GstBuffer if memory isn't writable
bufferpool: Don't free GstBuffer if memory isn't writable
Status: RESOLVED OBSOLETE
Product: GStreamer
Classification: Platform
Component: gstreamer (core)
1.4.1
Other Linux
: Normal normal
: git master
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2015-02-11 02:56 UTC by kevin
Modified: 2018-11-03 12:25 UTC
See Also:
GNOME target: ---
GNOME version: ---



Description kevin 2015-02-11 02:56:45 UTC
GstBuffer will be freed if memory isn't writable. The reason of the memory can't writable is gst_buffer_copy_into() will increase the ref count of the memory. The expect is reuse video buffer without free it. The buffer will writable later. Do we have some general solution for it?

I have asked the question in gstreamer-dev mail list:
http://lists.freedesktop.org/archives/gstreamer-devel/2015-January/051137.html
Comment 1 Nicolas Dufresne (ndufresne) 2015-02-16 03:03:11 UTC
An idea (if it does not clash with something else) could be to use GstMiniObject parenting in order to create a link between buffers if we have GST_BUFFER_COPY_MEMORY flag (not GST_BUFFER_COPY_DEEP, which do a real copy, removing the bound between buffers).

This would be similar to gst_memory_share() at the GstMemory level. Same would apply to gst_buffer_copy() and gst_buffer_copy_region().

Any comment ?
Comment 2 Olivier Crête 2015-02-16 03:11:24 UTC
@nicolas: This is how sub-buffering worked in 0.10. I guess Wim had a good reason to remove it. 

I understand the current design is that the GstBuffer object just isn't poolable in that case, making the buffer pools pretty much useless for a lot of cases. That said, the GstMemory is just a small structure coming out of the slice allocator, so it shouldn't matter much. And frankly, I'd like to see some number about the usefulness of the pooling behavior of GstBufferPool in 1.x.

Parenting one buffer to another means that even if only one GstMemory of a GstBuffer is still in use, then all GstMemories in it would be stuck.
Comment 3 kevin 2015-02-16 07:01:00 UTC
The reasons of why need video buffer pooling without free:

1. V4L2 does support allocate memory dynamically currently on our platform. So memory is pre-allocated. We implement V4L2 memory allocator, and use default video buffer pool. And can't do memory pooling in V4L2 memory allocator as can't handle flushing.

2. VPU (video decoder unit) must register all video frame buffer in initialize on our SOC, and can't support dynamically change the video frame buffer during decoding. So video pool shouldn't free and allocate memory during playback.

Can add this kind of usage as one special usage to avoid broken buffer pool normal usage?
Comment 4 Nicolas Dufresne (ndufresne) 2015-02-16 13:55:54 UTC
(In reply to Olivier Crête from comment #2)
> @nicolas: This is how sub-buffering worked in 0.10. I guess Wim had a good
> reason to remove it. 

I'll ask.

> 
> I understand the current design is that the GstBuffer object just isn't
> poolable in that case, making the buffer pools pretty much useless for a lot
> of cases. That said, the GstMemory is just a small structure coming out of
> the slice allocator, so it shouldn't matter much. And frankly, I'd like to
> see some number about the usefulness of the pooling behavior of
> GstBufferPool in 1.x.

We can do measurement of course. The point here is to realign the design. Unlike what you seem to think, I doubt there is a way to remove buffers pools that would not break the API. Pooling is broken for many uses cases, and I think we have an opportunity to fix it. Pooling in most allocators makes very little sense, since the allocator does not know if allocation of size A still have a meaning compare to an allocation of size B. Pooling in the allocator is a limited option, and only apply to allocator with GST_ALLOCATOR_FLAG_CUSTOM_ALLOC flag in real life.

> 
> Parenting one buffer to another means that even if only one GstMemory of a
> GstBuffer is still in use, then all GstMemories in it would be stuck.

It won't be stuck for a very long time. Also, if you look at payloaders and GstAdapter, they append into a child, and leave the parent untouched. This is the most common case, and parenting is a good fit.

Kevin, I don't really get your arguments though. V4L2 and VPU are excellent fit for Allocator side pooling. As a proof, the generic v4l2 elements implement that. It does not change anything to the fact I think we should be able to pool GstBuffer as per design, and reduce as much as possible the amount of round-trip to the OS in order to re-allocate for.
Comment 5 kevin 2015-07-02 08:26:58 UTC
seems same with https://bugzilla.gnome.org/show_bug.cgi?id=750039
will verify it on GStreamer 1.6.
Comment 6 GStreamer system administrator 2018-11-03 12:25:27 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/gstreamer/gstreamer/issues/93.