GNOME Bugzilla – Bug 762177
v4l2src: Potential race condition with pool release on renegotiation
Last modified: 2016-02-22 20:54:27 UTC
Created attachment 321474 [details] A simple demo showing how the race condition causes the v4l2src to crash Hello, I have stumbled upon a potential race condition with v4l2bufferpool reconstruction when trying to renegotiate a pipeline containing v4l2src. If a pipeline fires a renegotiation, the way it is currently being handled is: - gst_v4l2_object_stop is called. the pool is deactivated, unreffed and released. - renegotiation is performed. gst_v4l2src_set_format_full is called, setting the new format with V4L2_S_FMT. - a new pool is setup using gst_v4l2_object_setup_pool after V4L2_S_FMT. The problem is that before a V4L2_S_FMT is run, all buffers need to be returned to the driver via VIDIOC_REQBUFS. This is performed by v4l2allocator in gst_v4l2_allocator_stop, which is called by gst_v4l2_buffer_pool_stop when it is done releasing all the buffers that have been allocated previously. This works fine on simpler pipelines. However, if the v4l2src is running on a processing pipeline that holds the buffers for prolonged periods, the following happens: - gst_v4l2_object_stop deactivates the pool, the pool starts releasing the buffers. - renegotiation begins. - after renegotiation, the target format is set using V4L2_S_FMT. An implementation of the driver call can be seen in vivid at: https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/media/platform/vivid/vivid-vid-cap.c#n633 - The v4l2 driver checks if the device is busy. The function in v4l2-core, vb2_is_busy is called, checking if the number of buffers that the driver has are > 0. You can verify that at: https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/include/media/videobuf2-core.h#n621 - Because of the increased processing, the previous pool has not released all the buffers. gst_v4l2_allocator_stop has not yet been called. The buffers are still set. Thus, V4L2_S_FMT fails with an EBUSY signal, causing the pipeline to stop running. I have also attached a simple demo of how to replicate the issue using the vivid driver. This example is not perfect since it obviously refs indefinitely the buffers, but it is simple enough to illustrate the problem. I am a bit over my head in trying to find an easy way to fix this so any help would be much appreciated.
Also, the function that causes the deactivation and release of the v4l2pool is at: https://cgit.freedesktop.org/gstreamer/gst-plugins-good/tree/sys/v4l2/gstv4l2object.c#n3707
You forgot to mention that between stop, and star, there is an allocation query being done. In supported pipeline, the allocation query will cause the buffers to be returned. Failing if that does not happen in you pipeline is expected, and is a limitation of the v4l2 driver. Note, I still need to try your example. You said it "crash", if so, provide a backtrace please. Normally it should just error out, there is a reported crash on infinite recursion that is already reported (notice sometimes when you unplug an USB camera). If that's what you hit, I'll mark as duplicate. If not, I'll have to look at your pipeline, if it's candidate for renegotiation, we'll try to fix.
Comment on attachment 321474 [details] A simple demo showing how the race condition causes the v4l2src to crash +/* buffers are refed to simpulate buffers that are being utilized for processing. + * This is of course incorrect and will lead to the pipeline being stalled. + * However, the framerate should be low enough that the race condition is triggered + * before starvation becomes an issue. + */ +static GstPadProbeReturn +gst_qtec_camera_src_imgsrc_probe (GstPad * pad, GstPadProbeInfo * info, + gpointer data) +{ + GstBuffer *buffer; + + buffer = GST_PAD_PROBE_INFO_BUFFER (info); + gst_buffer_ref (buffer); + return GST_PAD_PROBE_OK; +} So you leak all buffers, and don't given them back, this is not a supported scenario for renegotiation (v4l2 driver limitation). What could be done is to add a property to force copies, so you can freely leak and renegotiate. Patch welcome.