On 2/13/23 10:49, Tomi Valkeinen wrote:
> Hi,
>
> On 10/02/2023 13:55, 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 | 288 +++++++++++++++++++++++++----
>> utils/v4l2-ctl/v4l2-ctl.cpp | 2 +
>> utils/v4l2-ctl/v4l2-ctl.h | 2 +
>> 3 files changed, 259 insertions(+), 33 deletions(-)
>>
>> diff --git a/utils/v4l2-ctl/v4l2-ctl-subdev.cpp b/utils/v4l2-ctl/v4l2-ctl-subdev.cpp
>> index 33cc1342..81236451 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"
>> + " --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"
>
> Laurent noticed that the above option, and a few others, break backward
> compatibility.
>
> If no one has other suggestions, I'll additionally add back the old
> option format. In other words, if the parameter is a number, it's a pad.
> Otherwise we look for pad=<pad>,stream=<stream> style option.
That will work. Only mention the new arguments in the usage text, though.
And add a comment in the code why you check if the arg is just a number.
>
> Well, another alternative would be to use an "extended" pad format
> instead. I.e. for each <pad> in the old style, we'd accept something
> like <pad[/stream]>. E.g. "--list-subdev-framesizes
> pad=<pad[/stream]>,code=<code>"
No, that's inconsistent with the argument style of v4l2-ctl.
Note that I am not terribly concerned about backwards compatibility here,
especially w.r.t. the subdev options. So if it turns out to be too much
work to support the old option format, then I'm OK with breaking the
compatibility.
Regards,
Hans
>
> Opinions?
>
> Tomi
>
