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 735386 - bufferpool: size check on release triggers unwanted buffer free in many situations
bufferpool: size check on release triggers unwanted buffer free in many situa...
Status: RESOLVED DUPLICATE of bug 741420
Product: GStreamer
Classification: Platform
Component: gstreamer (core)
1.4.0
Other Linux
: Normal normal
: git master
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2014-08-25 14:08 UTC by Arnaud Vrac
Modified: 2014-12-16 03:20 UTC
See Also:
GNOME target: ---
GNOME version: ---



Description Arnaud Vrac 2014-08-25 14:08:51 UTC
Since commit ec69ad4e8add (buffer: Only set TAG_MEMORY if the memory has been replaced) in gstreamer, buffers allocated in buffer pools are freed on release in multiple cases where they shouldn't. For example in this pipeline this issue happens:

gst-launch-1.0 --gst-debug='bufferpool:6' souphttpsrc location= http://absolut.zogzog.org/share/samples/avi/big_buck_bunny_720p_surround.avi ! avidemux ! avdec_mpeg4 ! videoconvert ! 'video/x-raw, format=NV12' ! fakesink

The problem is that the size set on the config parameter for video frame buffer pools is usually set to the *theoritical* size of the frame for the video format, ie without taking the stride into account.

I also see this issue on my decoder plugin elements, since I have no way of knowing in advance the stride of each video frame. So in my case the size is actually useless. IMHO it is actually generally useless when dealing with video frames when they have the video meta attached to them. Unfortunately the check added on size on buffer release hurts performance in my case.
Comment 1 Nicolas Dufresne (ndufresne) 2014-08-25 14:45:40 UTC
(In reply to comment #0)
> Since commit ec69ad4e8add (buffer: Only set TAG_MEMORY if the memory has been
> replaced) in gstreamer, buffers allocated in buffer pools are freed on release
> in multiple cases where they shouldn't. For example in this pipeline this issue
> happens:

Just a note that this patch do the opposite, it relax the amount of discarded buffers in order to reduce the performance hit. I have though, left this "light" size check, as otherwise a freshly allocated buffer may not have the negotiated size on _acquire(). It was said that where resize is expected, associated buffer pool should correct the buffer size in reset in order to preserve the allocation.

The negotiated size isn't supposed to be theoretical. The decoder and converter (in this example) should have negotiated the right size, otherwise it would endup with potential short allocation by the pool. We might have introduced a regression in avdec pool negotation. The size in videopool is obtained from the video alignment (through gst_video_info_align()). The freshly calculated size should match avdec expected size. This need further investigation, but is unlikely a bufferpool bug. Though, I'm starting to think that the video pool should validate that the provided size matches the calculate size.
Comment 2 Nicolas Dufresne (ndufresne) 2014-12-16 03:20:34 UTC

*** This bug has been marked as a duplicate of bug 741420 ***