Re: [PATCH v1 1/5] media: mc: entity: Add pad iterator for media_pipeline

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 12/12/2022 16:16, Laurent Pinchart wrote:
Add a media_pipeline_for_each_pad() macro to iterate over pads in a
pipeline. This should be used by driver as a replacement of the
media_graph_walk API, as iterating over the media_pipeline uses the
cached list of pads and is thus more efficient.

Signed-off-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
---
  drivers/media/mc/mc-entity.c | 18 ++++++++++++++++++
  include/media/media-entity.h | 29 +++++++++++++++++++++++++++++
  2 files changed, 47 insertions(+)

diff --git a/drivers/media/mc/mc-entity.c b/drivers/media/mc/mc-entity.c
index b8bcbc734eaf..70df2050951c 100644
--- a/drivers/media/mc/mc-entity.c
+++ b/drivers/media/mc/mc-entity.c
@@ -932,6 +932,24 @@ __must_check int media_pipeline_alloc_start(struct media_pad *pad)
  }
  EXPORT_SYMBOL_GPL(media_pipeline_alloc_start);
+struct media_pad *
+__media_pipeline_pad_iter_next(struct media_pipeline *pipe,
+			       struct media_pipeline_pad_iter *iter,
+			       struct media_pad *pad)
+{
+	if (!pad)
+		iter->cursor = pipe->pads.next;
+
+	if (iter->cursor == &pipe->pads)
+		return NULL;
+
+	pad = list_entry(iter->cursor, struct media_pipeline_pad, list)->pad;
+	iter->cursor = iter->cursor->next;
+
+	return pad;
+}
+EXPORT_SYMBOL_GPL(__media_pipeline_pad_iter_next);
+
  /* -----------------------------------------------------------------------------
   * Links management
   */
diff --git a/include/media/media-entity.h b/include/media/media-entity.h
index 85ed08ddee9d..e881e483b550 100644
--- a/include/media/media-entity.h
+++ b/include/media/media-entity.h
@@ -130,6 +130,15 @@ struct media_pipeline_pad {
  	struct media_pad *pad;
  };
+/**
+ * struct media_pipeline_pad_iter - Iterator for media_pipeline_for_each_pad
+ *
+ * @cursor: The current element
+ */
+struct media_pipeline_pad_iter {
+	struct list_head *cursor;
+};
+

Is there any reason to have this iter struct in a public header, vs. having it in mc-entity.c?

  /**
   * struct media_link - A link object part of a media graph.
   *
@@ -1163,6 +1172,26 @@ void media_pipeline_stop(struct media_pad *pad);
   */
  void __media_pipeline_stop(struct media_pad *pad);
+struct media_pad *
+__media_pipeline_pad_iter_next(struct media_pipeline *pipe,
+			       struct media_pipeline_pad_iter *iter,
+			       struct media_pad *pad);
+
+/**
+ * media_pipeline_for_each_pad - Iterate on all pads in a media pipeline
+ * @pipe: The pipeline
+ * @iter: The iterator (struct media_pipeline_pad_iter)
+ * @pad: The iterator pad

If I understand this correctly, both iter and pad are just variables the macro will use. In other words, they are not used to pass any values.

Would it be better to declare those variables in the macro itself? Well, that has its downsides. But perhaps at least clarify in the doc that they are only variables used by the loop, and do not need to be initialized.

+ * Iterate on all pads in a media pipeline. This is only valid after the
+ * pipeline has been built with media_pipeline_start() and before it gets
+ * destroyed with media_pipeline_stop().
+ */
+#define media_pipeline_for_each_pad(pipe, iter, pad)			\
+	for (pad = __media_pipeline_pad_iter_next((pipe), iter, NULL);	\
+	     pad != NULL;						\
+	     pad = __media_pipeline_pad_iter_next((pipe), iter, pad))
+
  /**
   * media_pipeline_alloc_start - Mark a pipeline as streaming
   * @pad: Starting pad

 Tomi




[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux