Thread (34 messages) 34 messages, 5 authors, 2025-08-21

Re: [PATCH v11 02/19] media: v4l2-dev: Add helpers to run media_pipeline_[started|stopped]()

From: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
Date: 2025-08-05 08:52:18
Also in: linux-arm-kernel, linux-media

Hi Dan

On Mon, Jul 14, 2025 at 04:06:28PM +0100, Daniel Scally wrote:
quoted hunk ↗ jump to hunk
Add helpers to run the new media_pipeline_started() and
media_pipeline_stopped() functions. The helpers iterate over the
entities in the pipeline and count the number of video devices and
compare that to the pipeline's start_count() before acting. This
allows us to only run the media pipeline callbacks in the event that
the pipeline has had video_pipeline_start() called for each video
device.

Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com>

---
Changes in v3:
	- Revised the documentation a bit, detailing the specific error
	  codes and rewording the descriptions
	- Renamed __video_device_pipeline_started()
	- List the possible return values explicitly

Changes in v2:

	- Adapted now media_pipeline_for_each_entity() takes an iter
	  variable
	- Fixed the Return: section of the kerneldoc comments
---
 drivers/media/v4l2-core/v4l2-dev.c | 57 ++++++++++++++++++++++++++++++++++++++
 include/media/v4l2-dev.h           | 52 ++++++++++++++++++++++++++++++++++
 2 files changed, 109 insertions(+)
diff --git a/drivers/media/v4l2-core/v4l2-dev.c b/drivers/media/v4l2-core/v4l2-dev.c
index c369235113d98ae26c30a1aa386e7d60d541a66e..a25038f3c9cc9213b840e2b96d1ef49d17d22fb8 100644
--- a/drivers/media/v4l2-core/v4l2-dev.c
+++ b/drivers/media/v4l2-core/v4l2-dev.c
@@ -1200,6 +1200,63 @@ struct media_pipeline *video_device_pipeline(struct video_device *vdev)
 }
 EXPORT_SYMBOL_GPL(video_device_pipeline);

+static int video_device_pipeline_unstarted_vdevs(struct media_pipeline *pipe)
+{
+	struct media_pipeline_entity_iter iter;
+	unsigned int n_video_devices = 0;
+	struct media_entity *entity;
+	int ret;
+
+	ret = media_pipeline_entity_iter_init(pipe, &iter);
+	if (ret)
+		return ret;
+
+	media_pipeline_for_each_entity(pipe, &iter, entity) {
+		if (entity->obj_type == MEDIA_ENTITY_TYPE_VIDEO_DEVICE)
+			n_video_devices++;
+	}
+
+	media_pipeline_entity_iter_cleanup(&iter);
+
+	return n_video_devices - pipe->start_count;
+}
+
+int video_device_pipeline_started(struct video_device *vdev)
+{
+	struct media_pipeline *pipe;
+	int ret;
+
+	pipe = video_device_pipeline(vdev);
+	if (!pipe)
+		return -ENODEV;
+
+	ret = video_device_pipeline_unstarted_vdevs(pipe);
+	if (ret)
+		return ret;
+
+	return media_pipeline_started(pipe);
+}
+EXPORT_SYMBOL_GPL(video_device_pipeline_started);
+
+int video_device_pipeline_stopped(struct video_device *vdev)
+{
+	struct media_pipeline *pipe;
+	int ret;
+
+	pipe = video_device_pipeline(vdev);
+	if (!pipe)
+		return -ENODEV;
+
+	ret = video_device_pipeline_unstarted_vdevs(pipe);
+	if (ret)
+		return ret;
+
+	media_pipeline_stopped(pipe);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(video_device_pipeline_stopped);
+
 #endif /* CONFIG_MEDIA_CONTROLLER */

 /*
diff --git a/include/media/v4l2-dev.h b/include/media/v4l2-dev.h
index 1b6222fab24eda96cbe459b435431c01f7259366..45a87a5a76e8782c79da9d4890b063b8f0cbb3d8 100644
--- a/include/media/v4l2-dev.h
+++ b/include/media/v4l2-dev.h
@@ -654,6 +654,58 @@ __must_check int video_device_pipeline_alloc_start(struct video_device *vdev);
  */
 struct media_pipeline *video_device_pipeline(struct video_device *vdev);

+/**
+ * video_device_pipeline_started - Run the pipeline_started() entity operation
+ *				   for a fully-started media pipeline
+ * @vdev: A video device that's part of the pipeline
+ *
+ * This function checks whether all MEDIA_ENTITY_TYPE_VIDEO_DEVICE entities
+ * connected to a given video device through enabled links, either directly or
+ * indirectly, have been marked as streaming through the use of
+ * video_device_pipeline_start() or one of its equivalent functions. If so,
+ * media_pipeline_started() is called to inform entities in the pipeline of that
+ * fact. The intention is to provide drivers with a mechanism for checking
+ * whether their pipeline is fully ready to start processing data and call the
+ * .pipeline_started() media entity operation on all the entities in the
+ * pipeline if so.
+ *
+ * Return: The number of video devices in the pipeline remaining to be started,
+ * or a negative error number on failure:
+ *
+ * * 0			- Success
+ * * n > 0		- n video devices in the pipeline remain unstarted
+ * * -ENODEV		- The video device has no pipeline
+ * * -ENOMEM		- failed to allocate pipeline iterator
+ * * n < 0		- Error passed through from a media entity's
+ *			  .pipeline_started() operation
+ */
+int video_device_pipeline_started(struct video_device *vdev);
+
+/**
+ * video_device_pipeline_stopped - Run the pipeline_stopped() entity operation
+ *				   for a fully-started media pipeline
+ * @vdev: A video device that's part of the pipeline
+ *
+ * This function checks whether the MEDIA_ENTITY_TYPE_VIDEO_DEVICE entities
+ * connected to a given video device through enabled links, either directly or
+ * indirectly, have been marked as not streaming through the use of
+ * video_device_pipeline_stop() or one of its equivalent functions. If none of
+ * the video devices in the pipeline have been stopped, then the function
+ * media_pipeline_stopped() is called. The intention is to provide drivers with
+ * a mechanism for checking whether this video device is the first device in the
+ * pipeline to be stopped and call the .pipeline_stopped() media entity
+ * operation on all the entities in the pipeline if so.
+ *
+ * Return: The number of video devices in the pipeline remaining to be started,
+ * or a negative error number on failure:
+ *
+ * * 0			- Success
+ * * n > 0		- n video devices in the pipeline already stopped
+ * * -ENODEV		- The video device has no pipeline
+ * * -ENOMEM		- failed to allocate pipeline iterator
+ */
+int video_device_pipeline_stopped(struct video_device *vdev);
I still have troubles here,

media_pipeline_stopped() is documented as

* media_pipeline_stopped - Inform entities in a pipeline that it has stopped

and the entity operation pipeline_stopped() is documented as

* @pipeline_stopped:  Optional: Notify this entity that the pipeline it is a
*                     part of has been stopped.

However video_device_pipeline_stopped() calls media_pipeline_stopped()
(and consequentially the entities' .pipeline_stopped() op) on the first call
to streamoff on any video devices in the pipeline.

At this point what confuses me is:

- If we're on the first call to streamoff, has the pipeline actually
  "stopped" ?
- if the fist call to streamoff triggers a pipeline halt, what happens
  to the non-yet-stopped video devices ? Will they stop sending
  buffers back ?
- I understand a post-pipeline-start hook is desirable, but I fail to
  clearly find a use case for this one, that to me looks like a
  pre-pipeline-stop hook.
+
 #endif /* CONFIG_MEDIA_CONTROLLER */

 #endif /* _V4L2_DEV_H */

--
2.34.1
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help