Re: [PATCH v11 34/36] media: v4l2-subdev: Add v4l2_subdev_state_xlate_streams() helper

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

 



Hi,

On 07/07/2022 13:27, Tomasz Figa wrote:
Hi Tomi, Laurent,

On Tue, Mar 01, 2022 at 06:11:54PM +0200, Tomi Valkeinen wrote:
From: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>

Add a helper function to translate streams between two pads of a subdev,
using the subdev's internal routing table.


Thanks for the patch! Please see my comments inline.

Signed-off-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@xxxxxxxxxxxxxxxx>
---
  drivers/media/v4l2-core/v4l2-subdev.c | 25 +++++++++++++++++++++++++
  include/media/v4l2-subdev.h           | 23 +++++++++++++++++++++++
  2 files changed, 48 insertions(+)

diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
index e30338a53088..6a9fc62dacbf 100644
--- a/drivers/media/v4l2-core/v4l2-subdev.c
+++ b/drivers/media/v4l2-core/v4l2-subdev.c
@@ -1529,6 +1529,31 @@ v4l2_subdev_state_get_opposite_stream_format(struct v4l2_subdev_state *state,
  }
  EXPORT_SYMBOL_GPL(v4l2_subdev_state_get_opposite_stream_format);
+u64 v4l2_subdev_state_xlate_streams(const struct v4l2_subdev_state *state,
+				    u32 pad0, u32 pad1, u64 *streams)
+{
+	const struct v4l2_subdev_krouting *routing = &state->routing;
+	struct v4l2_subdev_route *route;
+	u64 streams0 = 0;
+	u64 streams1 = 0;
+
+	for_each_active_route(routing, route) {
+		if (route->sink_pad == pad0 && route->source_pad == pad1 &&
+		    (*streams & BIT(route->sink_stream))) {
+			streams0 |= BIT(route->sink_stream);
+			streams1 |= BIT(route->source_stream);
+		}
+		if (route->source_pad == pad0 && route->sink_pad == pad1 &&
+		    (*streams & BIT(route->source_stream))) {
+			streams0 |= BIT(route->source_stream);
+			streams1 |= BIT(route->sink_stream);
+		}
+	}
+
+	*streams = streams0;
+	return streams1;
+}
+
  int v4l2_subdev_get_fmt(struct v4l2_subdev *sd, struct v4l2_subdev_state *state,
  			struct v4l2_subdev_format *format)
  {
diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
index 1eced0f47296..992debe116ac 100644
--- a/include/media/v4l2-subdev.h
+++ b/include/media/v4l2-subdev.h
@@ -1497,6 +1497,29 @@ struct v4l2_mbus_framefmt *
  v4l2_subdev_state_get_opposite_stream_format(struct v4l2_subdev_state *state,
  					     u32 pad, u32 stream);
+/**
+ * v4l2_subdev_state_xlate_streams() - Translate streams from one pad to another
+ *
+ * @state: Subdevice state
+ * @pad0: The first pad
+ * @pad1: The second pad
+ * @streams: Streams bitmask on the first pad
+ *
+ * Streams on sink pads of a subdev are routed to source pads as expressed in
+ * the subdev state routing table. Stream numbers don't necessarily match on
+ * the sink and source side of a route. This function translates stream numbers
+ * on @pad0, expressed as a bitmask in @streams, to the corresponding streams
+ * on @pad1 using the routing table from the @state. It returns the stream mask
+ * on @pad1, and updates @streams with the streams that have been found in the
+ * routing table.
+ *
+ * @pad0 and @pad1 must be a sink and a source, in any order.
+ *
+ * Return: The bitmask of streams of @pad1 that are routed to @streams on @pad0.

It might be just me, but somehow I associate "translate" with turning a
name from one namespace into a corresponding name from another
namespace. In this case I initially assumed that this function is
supposed to accept stream IDs from pad0 and return corresponding
stream IDs from pad1. However, it looks like this is move like - "tell
me which streams on pad1 are connected to the given pad0 streams". Is
this correct?

So is your point that as the function returns a bitmask, and not a mapping of the stream IDs, this is not a translate function?

If yes, maybe v4l2_subdev_state_get_routed_streams() be a better name?

I think this name would also fit quite well for a function that returns a mapping of the streams. So... I don't have a strong opinion on this. To me, neither the current name nor the proposed one clearly explains alone what the function does.

 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