GNOME Bugzilla – Bug 744303
bufferpool: Don't free GstBuffer if memory isn't writable
Last modified: 2018-11-03 12:25:27 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
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 ?
@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.
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?
(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.
seems same with https://bugzilla.gnome.org/show_bug.cgi?id=750039 will verify it on GStreamer 1.6.
-- 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.