On 29/05/2023 10:49, Tomi Valkeinen wrote:
> Add support to get and set subdev routes and to get and set
> configurations per stream.
>
> Based on work from Jacopo Mondi <jacopo@xxxxxxxxxx> and
> Niklas Söderlund <niklas.soderlund+renesas@xxxxxxxxxxxx>.
>
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@xxxxxxxxxxxxxxxx>
> ---
> utils/v4l2-ctl/v4l2-ctl-subdev.cpp | 310 ++++++++++++++++++++++++++---
> utils/v4l2-ctl/v4l2-ctl.cpp | 2 +
> utils/v4l2-ctl/v4l2-ctl.h | 2 +
> 3 files changed, 281 insertions(+), 33 deletions(-)
>
> diff --git a/utils/v4l2-ctl/v4l2-ctl-subdev.cpp b/utils/v4l2-ctl/v4l2-ctl-subdev.cpp
> index 33cc1342..fafb7d92 100644
> --- a/utils/v4l2-ctl/v4l2-ctl-subdev.cpp
> +++ b/utils/v4l2-ctl/v4l2-ctl-subdev.cpp
> @@ -1,5 +1,13 @@
> #include "v4l2-ctl.h"
>
> +#define ARRAY_SIZE(array) (sizeof(array) / sizeof((array)[0]))
> +
> +/*
> + * The max value comes from a check in the kernel source code
> + * drivers/media/v4l2-core/v4l2-ioctl.c check_array_args()
> + */
> +#define NUM_ROUTES_MAX 256
> +
> struct mbus_name {
> const char *name;
> __u32 code;
> @@ -19,45 +27,57 @@ static const struct mbus_name mbus_names[] = {
> #define SelectionFlags (1L<<4)
>
> static __u32 list_mbus_codes_pad;
> +static __u32 list_mbus_codes_stream = 0;
> static __u32 get_fmt_pad;
> +static __u32 get_fmt_stream = 0;
> static __u32 get_sel_pad;
> +static __u32 get_sel_stream = 0;
> static __u32 get_fps_pad;
> +static __u32 get_fps_stream = 0;
> static int get_sel_target = -1;
> static unsigned int set_selection;
> static struct v4l2_subdev_selection vsel;
> static unsigned int set_fmt;
> static __u32 set_fmt_pad;
> +static __u32 set_fmt_stream = 0;
> static struct v4l2_mbus_framefmt ffmt;
> static struct v4l2_subdev_frame_size_enum frmsize;
> static struct v4l2_subdev_frame_interval_enum frmival;
> static __u32 set_fps_pad;
> +static __u32 set_fps_stream = 0;
> static double set_fps;
> +static struct v4l2_subdev_routing routing;
> +static struct v4l2_subdev_route routes[NUM_ROUTES_MAX];
>
> void subdev_usage()
> {
> printf("\nSub-Device options:\n"
> - " --list-subdev-mbus-codes <pad>\n"
> + " --list-subdev-mbus-codes pad=<pad>,stream=<stream>\n"
> " display supported mediabus codes for this pad (0 is default)\n"
> " [VIDIOC_SUBDEV_ENUM_MBUS_CODE]\n"
> - " --list-subdev-framesizes pad=<pad>,code=<code>\n"
> + " --list-subdev-framesizes pad=<pad>,stream=<stream>,code=<code>\n"
> " list supported framesizes for this pad and code\n"
> " [VIDIOC_SUBDEV_ENUM_FRAME_SIZE]\n"
> " <code> is the value of the mediabus code\n"
> - " --list-subdev-frameintervals pad=<pad>,width=<w>,height=<h>,code=<code>\n"
> + " --list-subdev-frameintervals pad=<pad>,stream=<stream>,width=<w>,height=<h>,code=<code>\n"
> " list supported frame intervals for this pad and code and\n"
> " the given width and height [VIDIOC_SUBDEV_ENUM_FRAME_INTERVAL]\n"
> " <code> is the value of the mediabus code\n"
> - " --get-subdev-fmt [<pad>]\n"
> - " query the frame format for the given pad [VIDIOC_SUBDEV_G_FMT]\n"
> - " --get-subdev-selection pad=<pad>,target=<target>\n"
> + " --get-subdev-fmt pad=<pad>,stream=<stream>\n"
> + " query the frame format for the given pad and optional stream [VIDIOC_SUBDEV_G_FMT]\n"
> + " <pad> the pad to get the format from\n"
> + " <stream> the stream to get the format from (0 if not specified)\n"
Here you explicitly state that <stream> is optional, but you did not do
that for --list-subdev-mbus-codes, --list-subdev-framesizes and --list-subdev-frameintervals
and other options below (just go through the list).
I also see that the 'pad' suboption is only sometimes documented.
In particular it is not clear when 'pad' is optional (defaulting
to 0) or required.
In some places I used [<pad>] to indicate that it is optional,
but I think it is better to explicitly state that <pad> defaults
to 0, or something along those lines.
If it isn't already the case, my preference is that both pad and
stream are optional and both default to 0.
> + " --get-subdev-selection pad=<pad>,stream=<stream>,target=<target>\n"
> " query the frame selection rectangle [VIDIOC_SUBDEV_G_SELECTION]\n"
> " See --set-subdev-selection command for the valid <target> values.\n"
> - " --get-subdev-fps [<pad>]\n"
> + " --get-subdev-fps pad=<pad>,stream=<stream>\n"
> " query the frame rate [VIDIOC_SUBDEV_G_FRAME_INTERVAL]\n"
> " --set-subdev-fmt (for testing only, otherwise use media-ctl)\n"
> - " --try-subdev-fmt pad=<pad>,width=<w>,height=<h>,code=<code>,field=<f>,colorspace=<c>,\n"
> + " --try-subdev-fmt pad=<pad>,stream=<stream>,width=<w>,height=<h>,code=<code>,field=<f>,colorspace=<c>,\n"
> " xfer=<xf>,ycbcr=<y>,hsv=<hsv>,quantization=<q>\n"
> - " set the frame format [VIDIOC_SUBDEV_S_FMT]\n"
> + " set the frame format for the given pad and optional stream [VIDIOC_SUBDEV_S_FMT]\n"
> + " <pad> the pad to get the format from\n"
> + " <stream> the stream to get the format from (0 if not specified)\n"
> " <code> is the value of the mediabus code\n"
> " <f> can be one of the following field layouts:\n"
> " any, none, top, bottom, interlaced, seq_tb, seq_bt,\n"
> @@ -74,31 +94,74 @@ void subdev_usage()
> " <q> can be one of the following quantization methods:\n"
> " default, full-range, lim-range\n"
> " --set-subdev-selection (for testing only, otherwise use media-ctl)\n"
> - " --try-subdev-selection pad=<pad>,target=<target>,flags=<flags>,\n"
> + " --try-subdev-selection pad=<pad>,stream=<stream>,target=<target>,flags=<flags>,\n"
> " top=<x>,left=<y>,width=<w>,height=<h>\n"
> " set the video capture selection rectangle [VIDIOC_SUBDEV_S_SELECTION]\n"
> " target=crop|crop_bounds|crop_default|compose|compose_bounds|\n"
> " compose_default|compose_padded|native_size\n"
> " flags=le|ge|keep-config\n"
> - " --set-subdev-fps pad=<pad>,fps=<fps> (for testing only, otherwise use media-ctl)\n"
> + " --set-subdev-fps pad=<pad>,stream=<stream>,fps=<fps> (for testing only, otherwise use media-ctl)\n"
> " set the frame rate [VIDIOC_SUBDEV_S_FRAME_INTERVAL]\n"
> + " --get-routing Print the route topology\n"
> + " --set-routing <routes>\n"
> + " Comma-separated list of route descriptors to setup\n"
> + "\n"
> + "Routes are defined as\n"
> + " routes = route { ',' route } ;\n"
> + " route = sink '->' source '[' flags ']' ;\n"
> + " sink = sink-pad '/' sink-stream ;\n"
> + " source = source-pad '/' source-stream ;\n"
> + "\n"
> + "where\n"
> + " sink-pad = Pad numeric identifier for sink\n"
> + " sink-stream = Stream numeric identifier for sink\n"
> + " source-pad = Pad numeric identifier for source\n"
> + " source-stream = Stream numeric identifier for source\n"
> + " flags = Route flags (0: inactive, 1: active)\n"
> );
> }
Regards,
Hans
