summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorLaurent Pinchart <laurent.pinchart@ideasonboard.com>2022-06-21 22:47:53 +0300
committerLaurent Pinchart <laurent.pinchart@ideasonboard.com>2022-07-04 23:07:59 +0300
commitb8708937bf68eaf5ff5f5560d5af778d1878248d (patch)
tree78b4fd67b5a5e88acffde8febc07af5f86f462c9
parent06bd05beced313c79a72bad53870e5dc233f563e (diff)
gstreamer: Fix pads locking
The srcpads_ vector is protected by two different locks, the GstObject lock of the libcamerasrc element, and the stream_lock that covers the run function of the thread. This isn't correct. Use the stream_lock consistently to protect the pads. Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com> Reviewed-by: Umang Jain <umang.jain@ideasonboard.com>
-rw-r--r--src/gstreamer/gstlibcamerasrc.cpp69
1 files changed, 38 insertions, 31 deletions
diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp
index 6f9a03c5..c92ca7d2 100644
--- a/src/gstreamer/gstlibcamerasrc.cpp
+++ b/src/gstreamer/gstlibcamerasrc.cpp
@@ -112,12 +112,18 @@ struct GstLibcameraSrcState {
std::shared_ptr<CameraManager> cm_;
std::shared_ptr<Camera> cam_;
std::unique_ptr<CameraConfiguration> config_;
- std::vector<GstPad *> srcpads_;
+
+ std::vector<GstPad *> srcpads_; /* Protected by stream_lock */
/*
* Contention on this lock_ must be minimized, as it has to be taken in
* the realtime-sensitive requestCompleted() handler to protect
* queuedRequests_ and completedRequests_.
+ *
+ * stream_lock must be taken before lock_ in contexts where both locks
+ * need to be taken. In particular, this means that the lock_ must not
+ * be held while calling into other graph elements (e.g. when calling
+ * gst_pad_query()).
*/
Mutex lock_;
std::queue<std::unique_ptr<RequestWrap>> queuedRequests_
@@ -354,36 +360,34 @@ gst_libcamera_src_task_run(gpointer user_data)
srcpad, ret);
}
- {
- if (ret != GST_FLOW_OK) {
- if (ret == GST_FLOW_EOS) {
- g_autoptr(GstEvent) eos = gst_event_new_eos();
- guint32 seqnum = gst_util_seqnum_next();
- gst_event_set_seqnum(eos, seqnum);
- for (GstPad *srcpad : state->srcpads_)
- gst_pad_push_event(srcpad, gst_event_ref(eos));
- } else if (ret != GST_FLOW_FLUSHING) {
- GST_ELEMENT_FLOW_ERROR(self, ret);
- }
- gst_task_stop(self->task);
- return;
+ if (ret != GST_FLOW_OK) {
+ if (ret == GST_FLOW_EOS) {
+ g_autoptr(GstEvent) eos = gst_event_new_eos();
+ guint32 seqnum = gst_util_seqnum_next();
+ gst_event_set_seqnum(eos, seqnum);
+ for (GstPad *srcpad : state->srcpads_)
+ gst_pad_push_event(srcpad, gst_event_ref(eos));
+ } else if (ret != GST_FLOW_FLUSHING) {
+ GST_ELEMENT_FLOW_ERROR(self, ret);
}
+ gst_task_stop(self->task);
+ return;
+ }
- /*
- * Here we need to decide if we want to pause. This needs to
- * happen in lock step with the callback thread which may want
- * to resume the task and might push pending buffers.
- */
- bool do_pause;
-
- {
- MutexLocker locker(state->lock_);
- do_pause = state->completedRequests_.empty();
- }
+ /*
+ * Here we need to decide if we want to pause. This needs to
+ * happen in lock step with the callback thread which may want
+ * to resume the task and might push pending buffers.
+ */
+ bool do_pause;
- if (do_pause)
- gst_task_pause(self->task);
+ {
+ MutexLocker locker(state->lock_);
+ do_pause = state->completedRequests_.empty();
}
+
+ if (do_pause)
+ gst_task_pause(self->task);
}
static void
@@ -537,8 +541,11 @@ gst_libcamera_src_task_leave([[maybe_unused]] GstTask *task,
state->completedRequests_ = {};
}
- for (GstPad *srcpad : state->srcpads_)
- gst_libcamera_pad_set_pool(srcpad, nullptr);
+ {
+ GLibRecLocker locker(&self->stream_lock);
+ for (GstPad *srcpad : state->srcpads_)
+ gst_libcamera_pad_set_pool(srcpad, nullptr);
+ }
g_clear_object(&self->allocator);
g_clear_pointer(&self->flow_combiner,
@@ -697,7 +704,7 @@ gst_libcamera_src_request_new_pad(GstElement *element, GstPadTemplate *templ,
g_object_ref_sink(pad);
if (gst_element_add_pad(element, pad)) {
- GLibLocker lock(GST_OBJECT(self));
+ GLibRecLocker lock(&self->stream_lock);
self->state->srcpads_.push_back(reinterpret_cast<GstPad *>(g_object_ref(pad)));
} else {
GST_ELEMENT_ERROR(element, STREAM, FAILED,
@@ -717,7 +724,7 @@ gst_libcamera_src_release_pad(GstElement *element, GstPad *pad)
GST_DEBUG_OBJECT(self, "Pad %" GST_PTR_FORMAT " being released", pad);
{
- GLibLocker lock(GST_OBJECT(self));
+ GLibRecLocker lock(&self->stream_lock);
std::vector<GstPad *> &pads = self->state->srcpads_;
auto begin_iterator = pads.begin();
auto end_iterator = pads.end();