GNOME Bugzilla – Bug 701375
v4l2: rework sink buffer refcounting
Last modified: 2013-06-03 09:57:05 UTC
Created attachment 245757 [details] [review] patch This is a followup patch for #700781, which is not quite correct. The buffer handling is quite complicated here. The original code intended to the the following: - gst_v4l2_buffer_pool_process() calls QBUF and adds the buffer to the local list. - The sink calls gst_buffer_unref() which returns the buffer to the pool but not the 'free list'. - Some time later DQBUF returns the buffer and gst_v4l2_buffer_pool_release_buffer() puts in on the 'free list'. If the buffer must be copied then (parent_class)->acquire_buffer() is called directly to keep the buffer in the pool. This has two problems: 1. If gst_v4l2_buffer_pool_release_buffer() is called before the buffer is returned to the pool, then the buffer is put on the 'free list' twice. This can happen if a reference to the buffer is kept outside the sink, of if DQBUF returns the buffer, that was just queued with QBUF. 2. If buffers are copied, then all buffers are in the pool at all times. As a result gst_v4l2_buffer_pool_stop() and gst_v4l2_buffer_pool_dqbuf() can access pool->buffers at the same time, which can lead to memory corruption. The patch for #700781 fixes those problems, but with the side effect that there are always buffers outside the pool (because they are queued) and the pool is never stopped. This patch fixes this by releasing the reference to the buffer after handling it (to avoid problem 2.) so it can be returned to the pool. gst_v4l2_buffer_pool_release_buffer() is only called if the buffer is already in the pool (to avoid problem 1.).
commit 496995a7d52e74c9d73dd644e5ed7af4b2d5f15d Author: Michael Olbrich <m.olbrich@pengutronix.de> Date: Tue May 28 19:14:15 2013 +0200 v4l2: rework sink buffer refcounting This is a followup patch for #700781, which is not quite correct. The buffer handling is quite complicated here. The original code intended to the the following: - gst_v4l2_buffer_pool_process() calls QBUF and adds the buffer to the local list. - The sink calls gst_buffer_unref() which returns the buffer to the pool but not the 'free list'. - Some time later DQBUF returns the buffer and gst_v4l2_buffer_pool_release_buffer() puts in on the 'free list'. If the buffer must be copied then (parent_class)->acquire_buffer() is called directly to keep the buffer in the pool. This has two problems: 1. If gst_v4l2_buffer_pool_release_buffer() is called before the buffer is returned to the pool, then the buffer is put on the 'free list' twice. This can happen if a reference to the buffer is kept outside the sink, of if DQBUF returns the buffer, that was just queued with QBUF. 2. If buffers are copied, then all buffers are in the pool at all times. As a result gst_v4l2_buffer_pool_stop() and gst_v4l2_buffer_pool_dqbuf() can access pool->buffers at the same time, which can lead to memory corruption. The patch for #700781 fixes those problems, but with the side effect that there are always buffers outside the pool (because they are queued) and the pool is never stopped. This patch fixes this by releasing the reference to the buffer after handling it (to avoid problem 2.) so it can be returned to the pool. gst_v4l2_buffer_pool_release_buffer() is only called if the buffer is already in the pool (to avoid problem 1.). Fixes https://bugzilla.gnome.org/show_bug.cgi?id=701375