[v4l-utils PATCH 0/4] List supported formats in libv4l2subdev

[v4l-utils PATCH 0/4] List supported formats in libv4l2subdev

[Sakari Ailus]

Hi,

This set moves libv4l2subdev to use media bus format definitions instead
of the old V4L2 mbus formats. In addition, support is added to all
existing formats, as the format list is generated instead of manually
adding formats to the list. The media-ctl test program is amended by the
list of the formats which media-ctl itself supports.

These patches go on the top of the previous set I posted ("[v4l-utils
PATCH 0/2] Add field support to libv4l2subdev").

-- 
Kind regards,
Sakari

[v4l-utils PATCH 1/4] libv4l2subdev: Switch to media bus formats

[Sakari Ailus]

No longer use the old V4L2 based MBUS_FMT definitions. Instead, use the
newer media bus format definitions.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 utils/media-ctl/libv4l2subdev.c | 54 ++++++++++++++++++++---------------------
 1 file changed, 27 insertions(+), 27 deletions(-)

diff --git a/utils/media-ctl/libv4l2subdev.c b/utils/media-ctl/libv4l2subdev.c
index 949eeff..b33d3fd 100644
--- a/utils/media-ctl/libv4l2subdev.c
+++ b/utils/media-ctl/libv4l2subdev.c
@@ -719,33 +719,33 @@ static struct {
 	const char *name;
 	enum v4l2_mbus_pixelcode code;
 } mbus_formats[] = {
-	{ "Y8", V4L2_MBUS_FMT_Y8_1X8},
-	{ "Y10", V4L2_MBUS_FMT_Y10_1X10 },
-	{ "Y12", V4L2_MBUS_FMT_Y12_1X12 },
-	{ "YUYV", V4L2_MBUS_FMT_YUYV8_1X16 },
-	{ "YUYV1_5X8", V4L2_MBUS_FMT_YUYV8_1_5X8 },
-	{ "YUYV2X8", V4L2_MBUS_FMT_YUYV8_2X8 },
-	{ "UYVY", V4L2_MBUS_FMT_UYVY8_1X16 },
-	{ "UYVY1_5X8", V4L2_MBUS_FMT_UYVY8_1_5X8 },
-	{ "UYVY2X8", V4L2_MBUS_FMT_UYVY8_2X8 },
-	{ "SBGGR8", V4L2_MBUS_FMT_SBGGR8_1X8 },
-	{ "SGBRG8", V4L2_MBUS_FMT_SGBRG8_1X8 },
-	{ "SGRBG8", V4L2_MBUS_FMT_SGRBG8_1X8 },
-	{ "SRGGB8", V4L2_MBUS_FMT_SRGGB8_1X8 },
-	{ "SBGGR10", V4L2_MBUS_FMT_SBGGR10_1X10 },
-	{ "SGBRG10", V4L2_MBUS_FMT_SGBRG10_1X10 },
-	{ "SGRBG10", V4L2_MBUS_FMT_SGRBG10_1X10 },
-	{ "SRGGB10", V4L2_MBUS_FMT_SRGGB10_1X10 },
-	{ "SBGGR10_DPCM8", V4L2_MBUS_FMT_SBGGR10_DPCM8_1X8 },
-	{ "SGBRG10_DPCM8", V4L2_MBUS_FMT_SGBRG10_DPCM8_1X8 },
-	{ "SGRBG10_DPCM8", V4L2_MBUS_FMT_SGRBG10_DPCM8_1X8 },
-	{ "SRGGB10_DPCM8", V4L2_MBUS_FMT_SRGGB10_DPCM8_1X8 },
-	{ "SBGGR12", V4L2_MBUS_FMT_SBGGR12_1X12 },
-	{ "SGBRG12", V4L2_MBUS_FMT_SGBRG12_1X12 },
-	{ "SGRBG12", V4L2_MBUS_FMT_SGRBG12_1X12 },
-	{ "SRGGB12", V4L2_MBUS_FMT_SRGGB12_1X12 },
-	{ "AYUV32", V4L2_MBUS_FMT_AYUV8_1X32 },
-	{ "ARGB32", V4L2_MBUS_FMT_ARGB8888_1X32 },
+	{ "Y8", MEDIA_BUS_FMT_Y8_1X8},
+	{ "Y10", MEDIA_BUS_FMT_Y10_1X10 },
+	{ "Y12", MEDIA_BUS_FMT_Y12_1X12 },
+	{ "YUYV", MEDIA_BUS_FMT_YUYV8_1X16 },
+	{ "YUYV1_5X8", MEDIA_BUS_FMT_YUYV8_1_5X8 },
+	{ "YUYV2X8", MEDIA_BUS_FMT_YUYV8_2X8 },
+	{ "UYVY", MEDIA_BUS_FMT_UYVY8_1X16 },
+	{ "UYVY1_5X8", MEDIA_BUS_FMT_UYVY8_1_5X8 },
+	{ "UYVY2X8", MEDIA_BUS_FMT_UYVY8_2X8 },
+	{ "SBGGR8", MEDIA_BUS_FMT_SBGGR8_1X8 },
+	{ "SGBRG8", MEDIA_BUS_FMT_SGBRG8_1X8 },
+	{ "SGRBG8", MEDIA_BUS_FMT_SGRBG8_1X8 },
+	{ "SRGGB8", MEDIA_BUS_FMT_SRGGB8_1X8 },
+	{ "SBGGR10", MEDIA_BUS_FMT_SBGGR10_1X10 },
+	{ "SGBRG10", MEDIA_BUS_FMT_SGBRG10_1X10 },
+	{ "SGRBG10", MEDIA_BUS_FMT_SGRBG10_1X10 },
+	{ "SRGGB10", MEDIA_BUS_FMT_SRGGB10_1X10 },
+	{ "SBGGR10_DPCM8", MEDIA_BUS_FMT_SBGGR10_DPCM8_1X8 },
+	{ "SGBRG10_DPCM8", MEDIA_BUS_FMT_SGBRG10_DPCM8_1X8 },
+	{ "SGRBG10_DPCM8", MEDIA_BUS_FMT_SGRBG10_DPCM8_1X8 },
+	{ "SRGGB10_DPCM8", MEDIA_BUS_FMT_SRGGB10_DPCM8_1X8 },
+	{ "SBGGR12", MEDIA_BUS_FMT_SBGGR12_1X12 },
+	{ "SGBRG12", MEDIA_BUS_FMT_SGBRG12_1X12 },
+	{ "SGRBG12", MEDIA_BUS_FMT_SGRBG12_1X12 },
+	{ "SRGGB12", MEDIA_BUS_FMT_SRGGB12_1X12 },
+	{ "AYUV32", MEDIA_BUS_FMT_AYUV8_1X32 },
+	{ "ARGB32", MEDIA_BUS_FMT_ARGB8888_1X32 },
 };
 
 const char *v4l2_subdev_pixelcode_to_string(enum v4l2_mbus_pixelcode code)
-- 
2.1.0.231.g7484e3b

[v4l-utils PATCH 2/4] libv4l2subdev: Use generated format definitions in libv4l2subdev

[Sakari Ailus]

Instead of manually adding each and every new media bus pixel code to
libv4l2subdev, generate the list automatically. The pre-existing formats
that do not match the list are not modified so that existing users are
unaffected by this change, with the exception of converting codes to
strings, which will use the new definitions.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 utils/media-ctl/.gitignore      | 1 +
 utils/media-ctl/Makefile.am     | 8 ++++++++
 utils/media-ctl/libv4l2subdev.c | 1 +
 3 files changed, 10 insertions(+)

diff --git a/utils/media-ctl/.gitignore b/utils/media-ctl/.gitignore
index 95b6a57..8c7d576 100644
--- a/utils/media-ctl/.gitignore
+++ b/utils/media-ctl/.gitignore
@@ -1 +1,2 @@
 media-ctl
+media-bus-formats.h
diff --git a/utils/media-ctl/Makefile.am b/utils/media-ctl/Makefile.am
index a3931fb..a1a9225 100644
--- a/utils/media-ctl/Makefile.am
+++ b/utils/media-ctl/Makefile.am
@@ -4,6 +4,14 @@ libmediactl_la_SOURCES = libmediactl.c mediactl-priv.h
 libmediactl_la_CFLAGS = -static $(LIBUDEV_CFLAGS)
 libmediactl_la_LDFLAGS = -static $(LIBUDEV_LIBS)
 
+media-bus-formats.h: ../../include/linux/media-bus-format.h
+	sed -e '/#define MEDIA_BUS_FMT/ ! d; s/.*FMT_//; /FIXED/ d; s/\t.*//; s/.*/{ \"&\", MEDIA_BUS_FMT_& },/;' \
+	< $< > $@
+
+BUILT_SOURCES = media-bus-formats.h
+CLEANFILES = media-bus-formats.h
+
+nodist_libv4l2subdev_la_SOURCES = media-bus-formats.h
 libv4l2subdev_la_SOURCES = libv4l2subdev.c
 libv4l2subdev_la_LIBADD = libmediactl.la
 libv4l2subdev_la_CFLAGS = -static
diff --git a/utils/media-ctl/libv4l2subdev.c b/utils/media-ctl/libv4l2subdev.c
index b33d3fd..607e943 100644
--- a/utils/media-ctl/libv4l2subdev.c
+++ b/utils/media-ctl/libv4l2subdev.c
@@ -719,6 +719,7 @@ static struct {
 	const char *name;
 	enum v4l2_mbus_pixelcode code;
 } mbus_formats[] = {
+#include "media-bus-formats.h"
 	{ "Y8", MEDIA_BUS_FMT_Y8_1X8},
 	{ "Y10", MEDIA_BUS_FMT_Y10_1X10 },
 	{ "Y12", MEDIA_BUS_FMT_Y12_1X12 },
-- 
2.1.0.231.g7484e3b

[v4l-utils PATCH 3/4] libv4l2subdev: Add a function to list library supported pixel codes

[Sakari Ailus]

Also mark which format definitions are compat definitions for the
pre-existing codes. This way we don't end up listing the same formats
twice.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 utils/media-ctl/libv4l2subdev.c | 63 +++++++++++++++++++++++------------------
 utils/media-ctl/v4l2subdev.h    | 11 +++++++
 2 files changed, 47 insertions(+), 27 deletions(-)

diff --git a/utils/media-ctl/libv4l2subdev.c b/utils/media-ctl/libv4l2subdev.c
index 607e943..7e0da46 100644
--- a/utils/media-ctl/libv4l2subdev.c
+++ b/utils/media-ctl/libv4l2subdev.c
@@ -718,35 +718,36 @@ int v4l2_subdev_parse_setup_formats(struct media_device *media, const char *p)
 static struct {
 	const char *name;
 	enum v4l2_mbus_pixelcode code;
+	bool compat;
 } mbus_formats[] = {
 #include "media-bus-formats.h"
-	{ "Y8", MEDIA_BUS_FMT_Y8_1X8},
-	{ "Y10", MEDIA_BUS_FMT_Y10_1X10 },
-	{ "Y12", MEDIA_BUS_FMT_Y12_1X12 },
-	{ "YUYV", MEDIA_BUS_FMT_YUYV8_1X16 },
-	{ "YUYV1_5X8", MEDIA_BUS_FMT_YUYV8_1_5X8 },
-	{ "YUYV2X8", MEDIA_BUS_FMT_YUYV8_2X8 },
-	{ "UYVY", MEDIA_BUS_FMT_UYVY8_1X16 },
-	{ "UYVY1_5X8", MEDIA_BUS_FMT_UYVY8_1_5X8 },
-	{ "UYVY2X8", MEDIA_BUS_FMT_UYVY8_2X8 },
-	{ "SBGGR8", MEDIA_BUS_FMT_SBGGR8_1X8 },
-	{ "SGBRG8", MEDIA_BUS_FMT_SGBRG8_1X8 },
-	{ "SGRBG8", MEDIA_BUS_FMT_SGRBG8_1X8 },
-	{ "SRGGB8", MEDIA_BUS_FMT_SRGGB8_1X8 },
-	{ "SBGGR10", MEDIA_BUS_FMT_SBGGR10_1X10 },
-	{ "SGBRG10", MEDIA_BUS_FMT_SGBRG10_1X10 },
-	{ "SGRBG10", MEDIA_BUS_FMT_SGRBG10_1X10 },
-	{ "SRGGB10", MEDIA_BUS_FMT_SRGGB10_1X10 },
-	{ "SBGGR10_DPCM8", MEDIA_BUS_FMT_SBGGR10_DPCM8_1X8 },
-	{ "SGBRG10_DPCM8", MEDIA_BUS_FMT_SGBRG10_DPCM8_1X8 },
-	{ "SGRBG10_DPCM8", MEDIA_BUS_FMT_SGRBG10_DPCM8_1X8 },
-	{ "SRGGB10_DPCM8", MEDIA_BUS_FMT_SRGGB10_DPCM8_1X8 },
-	{ "SBGGR12", MEDIA_BUS_FMT_SBGGR12_1X12 },
-	{ "SGBRG12", MEDIA_BUS_FMT_SGBRG12_1X12 },
-	{ "SGRBG12", MEDIA_BUS_FMT_SGRBG12_1X12 },
-	{ "SRGGB12", MEDIA_BUS_FMT_SRGGB12_1X12 },
-	{ "AYUV32", MEDIA_BUS_FMT_AYUV8_1X32 },
-	{ "ARGB32", MEDIA_BUS_FMT_ARGB8888_1X32 },
+	{ "Y8", MEDIA_BUS_FMT_Y8_1X8, true },
+	{ "Y10", MEDIA_BUS_FMT_Y10_1X10, true },
+	{ "Y12", MEDIA_BUS_FMT_Y12_1X12, true },
+	{ "YUYV", MEDIA_BUS_FMT_YUYV8_1X16, true },
+	{ "YUYV1_5X8", MEDIA_BUS_FMT_YUYV8_1_5X8, true },
+	{ "YUYV2X8", MEDIA_BUS_FMT_YUYV8_2X8, true },
+	{ "UYVY", MEDIA_BUS_FMT_UYVY8_1X16, true },
+	{ "UYVY1_5X8", MEDIA_BUS_FMT_UYVY8_1_5X8, true },
+	{ "UYVY2X8", MEDIA_BUS_FMT_UYVY8_2X8, true },
+	{ "SBGGR8", MEDIA_BUS_FMT_SBGGR8_1X8, true },
+	{ "SGBRG8", MEDIA_BUS_FMT_SGBRG8_1X8, true },
+	{ "SGRBG8", MEDIA_BUS_FMT_SGRBG8_1X8, true },
+	{ "SRGGB8", MEDIA_BUS_FMT_SRGGB8_1X8, true },
+	{ "SBGGR10", MEDIA_BUS_FMT_SBGGR10_1X10, true },
+	{ "SGBRG10", MEDIA_BUS_FMT_SGBRG10_1X10, true },
+	{ "SGRBG10", MEDIA_BUS_FMT_SGRBG10_1X10, true },
+	{ "SRGGB10", MEDIA_BUS_FMT_SRGGB10_1X10, true },
+	{ "SBGGR10_DPCM8", MEDIA_BUS_FMT_SBGGR10_DPCM8_1X8, true },
+	{ "SGBRG10_DPCM8", MEDIA_BUS_FMT_SGBRG10_DPCM8_1X8, true },
+	{ "SGRBG10_DPCM8", MEDIA_BUS_FMT_SGRBG10_DPCM8_1X8, true },
+	{ "SRGGB10_DPCM8", MEDIA_BUS_FMT_SRGGB10_DPCM8_1X8, true },
+	{ "SBGGR12", MEDIA_BUS_FMT_SBGGR12_1X12, true },
+	{ "SGBRG12", MEDIA_BUS_FMT_SGBRG12_1X12, true },
+	{ "SGRBG12", MEDIA_BUS_FMT_SGRBG12_1X12, true },
+	{ "SRGGB12", MEDIA_BUS_FMT_SRGGB12_1X12, true },
+	{ "AYUV32", MEDIA_BUS_FMT_AYUV8_1X32, true },
+	{ "ARGB32", MEDIA_BUS_FMT_ARGB8888_1X32, true },
 };
 
 const char *v4l2_subdev_pixelcode_to_string(enum v4l2_mbus_pixelcode code)
@@ -820,3 +821,11 @@ enum v4l2_field v4l2_subdev_string_to_field(const char *string,
 
 	return fields[i].field;
 }
+
+enum v4l2_mbus_pixelcode v4l2_subdev_pixelcode_list(unsigned int i)
+{
+	if (i >= ARRAY_SIZE(mbus_formats) || mbus_formats[i].compat)
+		return (enum v4l2_mbus_pixelcode)-1;
+
+	return mbus_formats[i].code;
+}
diff --git a/utils/media-ctl/v4l2subdev.h b/utils/media-ctl/v4l2subdev.h
index 104e420..ef8ef95 100644
--- a/utils/media-ctl/v4l2subdev.h
+++ b/utils/media-ctl/v4l2subdev.h
@@ -279,4 +279,15 @@ const char *v4l2_subdev_field_to_string(enum v4l2_field field);
 enum v4l2_field v4l2_subdev_string_to_field(const char *string,
 					    unsigned int length);
 
+/**
+ * @brief Enumerate library supported media bus pixel codes.
+ * @param i - index starting from zero
+ *
+ * Enumerate pixel codes supported by libv4l2subdev starting from
+ * index 0.
+ *
+ * @return media bus pixelcode on success, -1 on failure.
+ */
+enum v4l2_mbus_pixelcode v4l2_subdev_pixelcode_list(unsigned int i);
+
 #endif
-- 
2.1.0.231.g7484e3b

[v4l-utils PATCH 4/4] media-ctl: List supported media bus formats

[Sakari Ailus]

Add a new topic option for -h to allow listing supported media bus codes
in conversion functions. This is useful in figuring out which media bus
codes are actually supported by the library. The numeric values of the
codes are listed as well.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 utils/media-ctl/options.c | 42 ++++++++++++++++++++++++++++++++++++++----
 1 file changed, 38 insertions(+), 4 deletions(-)

diff --git a/utils/media-ctl/options.c b/utils/media-ctl/options.c
index 75d7ad7..e0f1ff5 100644
--- a/utils/media-ctl/options.c
+++ b/utils/media-ctl/options.c
@@ -22,7 +22,9 @@
 #include <getopt.h>
 #include <stdio.h>
 #include <stdlib.h>
+#include <string.h>
 #include <unistd.h>
+#include <v4l2subdev.h>
 
 #include <linux/videodev2.h>
 
@@ -45,7 +47,8 @@ static void usage(const char *argv0)
 	printf("-V, --set-v4l2 v4l2	Comma-separated list of formats to setup\n");
 	printf("    --get-v4l2 pad	Print the active format on a given pad\n");
 	printf("    --set-dv pad	Configure DV timings on a given pad\n");
-	printf("-h, --help		Show verbose help and exit\n");
+	printf("-h, --help[=topic]	Show verbose help and exit\n");
+	printf("			topics:	mbus-fmt: List supported media bus pixel codes\n");
 	printf("-i, --interactive	Modify links interactively\n");
 	printf("-l, --links links	Comma-separated list of link descriptors to setup\n");
 	printf("-p, --print-topology	Print the device topology\n");
@@ -100,7 +103,7 @@ static struct option opts[] = {
 	{"get-format", 1, 0, OPT_GET_FORMAT},
 	{"get-v4l2", 1, 0, OPT_GET_FORMAT},
 	{"set-dv", 1, 0, OPT_SET_DV},
-	{"help", 0, 0, 'h'},
+	{"help", 2, 0, 'h'},
 	{"interactive", 0, 0, 'i'},
 	{"links", 1, 0, 'l'},
 	{"print-dot", 0, 0, OPT_PRINT_DOT},
@@ -109,6 +112,27 @@ static struct option opts[] = {
 	{"verbose", 0, 0, 'v'},
 };
 
+void list_mbus_formats(void)
+{
+	unsigned int i;
+
+	printf("Supported media bus pixel codes\n");
+
+	for (i = 0; ; i++) {
+		unsigned int code = v4l2_subdev_pixelcode_list(i);
+		const char *str = v4l2_subdev_pixelcode_to_string(code);
+		int spaces = 30 - (int)strlen(str);
+
+		if (code == -1)
+			break;
+
+		if (spaces < 0)
+			spaces = 0;
+
+		printf("\t%s %*c (0x%8.8x)\n", str, spaces, ' ', code);
+	}
+}
+
 int parse_cmdline(int argc, char **argv)
 {
 	int opt;
@@ -119,7 +143,8 @@ int parse_cmdline(int argc, char **argv)
 	}
 
 	/* parse options */
-	while ((opt = getopt_long(argc, argv, "d:e:f:hil:prvV:", opts, NULL)) != -1) {
+	while ((opt = getopt_long(argc, argv, "d:e:f:h::il:prvV:",
+				  opts, NULL)) != -1) {
 		switch (opt) {
 		case 'd':
 			media_opts.devname = optarg;
@@ -141,7 +166,16 @@ int parse_cmdline(int argc, char **argv)
 			break;
 
 		case 'h':
-			usage(argv[0]);
+			if (optarg) {
+				if (!strcmp(optarg, "mbus-fmt"))
+					list_mbus_formats();
+				else
+					fprintf(stderr,
+						"Unknown topic \"%s\"\n",
+						optarg);
+			} else {
+				usage(argv[0]);
+			}
 			exit(0);
 
 		case 'i':
-- 
2.1.0.231.g7484e3b

Re: [v4l-utils PATCH 4/4] media-ctl: List supported media bus formats

[Sakari Ailus]

Hi Hans,

On Wed, Feb 24, 2016 at 04:38:58PM +0100, Hans Verkuil wrote:
> On 02/23/16 21:30, Laurent Pinchart wrote:
> > Hi Sakari,
> > 
> > On Tuesday 23 February 2016 22:24:00 Sakari Ailus wrote:
> >> On Tue, Feb 23, 2016 at 10:15:46PM +0200, Laurent Pinchart wrote:
> >>> On Tuesday 23 February 2016 17:15:15 Hans Verkuil wrote:
> >>>> On 02/23/2016 05:11 PM, Sakari Ailus wrote:
> >>>>> On Tue, Feb 23, 2016 at 01:18:53PM +0100, Hans Verkuil wrote:
> >>>>>> On 02/21/16 22:29, Sakari Ailus wrote:
> >>>>>>> Add a new topic option for -h to allow listing supported media bus
> >>>>>>> codes in conversion functions. This is useful in figuring out which
> >>>>>>> media bus codes are actually supported by the library. The numeric
> >>>>>>> values of the codes are listed as well.
> >>>>>>>
> >>>>>>> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> >>>>>>> ---
> >>>>>>>
> >>>>>>>  utils/media-ctl/options.c | 42 ++++++++++++++++++++++++++++++++----
> >>>>>>>  1 file changed, 38 insertions(+), 4 deletions(-)
> >>>>>>>
> >>>>>>> diff --git a/utils/media-ctl/options.c b/utils/media-ctl/options.c
> >>>>>>> index 0afc9c2..55cdd29 100644
> >>>>>>> --- a/utils/media-ctl/options.c
> >>>>>>> +++ b/utils/media-ctl/options.c
> > 
> > [snip]
> > 
> >>>>>>> @@ -45,7 +47,8 @@ static void usage(const char *argv0)
> >>>>>>>
> >>>>>>>  	printf("-V, --set-v4l2 v4l2	Comma-separated list of formats to
> >>>>>>>  	setup\n");
> >>>>>>>  	printf("    --get-v4l2 pad	Print the active format on a given
> >>>>>>> pad\n");
> >>>>>>>  	printf("    --set-dv pad	Configure DV timings on a given pad\n");
> >>>>>>>
> >>>>>>> -	printf("-h, --help		Show verbose help and exit\n");
> >>>>>>> +	printf("-h, --help[=topic]	Show verbose help and exit\n");
> >>>>>>> +	printf("			topics:	mbus-fmt: List supported media bus pixel
> >>>>>>> codes\n");
> >>>>>>
> >>>>>> OK, this is ugly. It has nothing to do with usage help.
> >>>>>>
> >>>>>> Just make a new option --list-mbus-fmts to list supported media bus
> >>>>>> pixel codes.
> >>>>>>
> >>>>>> That would make much more sense.
> >>>>>
> >>>>> I added it as a --help option argument in order to imply it's a part
> >>>>> of the program's usage instructions, which is what it indeed is. It's
> >>>>> not a list of media bus formats supported by a device.
> >>>>>
> >>>>> A separate option is fine, but it should be clear that it's about just
> >>>>> listing supported formats. E.g. --list-supported-mbus-fmts. But that's
> >>>>> a long one. Long options are loooong.
> >>>>
> >>>> --list-known-mbus-fmts will do the trick.
> >>>
> >>> That doesn't feel right. Isn't it a help option, really, given that it
> >>> lists the formats you can use as command line arguments ?
> 
> The help arguments don't have 'options'. You could provide a --help-list-mbus-fmts,
> though.

You could ask that from GCC.

> 
> >>> Another option would actually be to always print the formats when the -h
> >>> switch is given. We could print them in a comma-separated list with
> >>> multiple formats per line, possibly dropping the numerical value, it
> >>> should hopefully not be horrible.
> 
> Just always printing the list when -h is given works for me too.

Well, I'm fine with that as well.

-- 
Sakari Ailus
e-mail: sakari.ailus@iki.fi	XMPP: sailus@retiisi.org.uk

Re: [v4l-utils PATCH 4/4] media-ctl: List supported media bus formats

[Sakari Ailus]

Hans Verkuil wrote:
> On 02/23/16 21:30, Laurent Pinchart wrote:
>> Hi Sakari,
>>
>> On Tuesday 23 February 2016 22:24:00 Sakari Ailus wrote:
>>> On Tue, Feb 23, 2016 at 10:15:46PM +0200, Laurent Pinchart wrote:
>>>> On Tuesday 23 February 2016 17:15:15 Hans Verkuil wrote:
>>>>> On 02/23/2016 05:11 PM, Sakari Ailus wrote:
>>>>>> On Tue, Feb 23, 2016 at 01:18:53PM +0100, Hans Verkuil wrote:
>>>>>>> On 02/21/16 22:29, Sakari Ailus wrote:
>>>>>>>> Add a new topic option for -h to allow listing supported media bus
>>>>>>>> codes in conversion functions. This is useful in figuring out which
>>>>>>>> media bus codes are actually supported by the library. The numeric
>>>>>>>> values of the codes are listed as well.
>>>>>>>>
>>>>>>>> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
>>>>>>>> ---
>>>>>>>>
>>>>>>>>  utils/media-ctl/options.c | 42 ++++++++++++++++++++++++++++++++----
>>>>>>>>  1 file changed, 38 insertions(+), 4 deletions(-)
>>>>>>>>
>>>>>>>> diff --git a/utils/media-ctl/options.c b/utils/media-ctl/options.c
>>>>>>>> index 0afc9c2..55cdd29 100644
>>>>>>>> --- a/utils/media-ctl/options.c
>>>>>>>> +++ b/utils/media-ctl/options.c
>>
>> [snip]
>>
>>>>>>>> @@ -45,7 +47,8 @@ static void usage(const char *argv0)
>>>>>>>>
>>>>>>>>  	printf("-V, --set-v4l2 v4l2	Comma-separated list of formats to
>>>>>>>>  	setup\n");
>>>>>>>>  	printf("    --get-v4l2 pad	Print the active format on a given
>>>>>>>> pad\n");
>>>>>>>>  	printf("    --set-dv pad	Configure DV timings on a given pad\n");
>>>>>>>>
>>>>>>>> -	printf("-h, --help		Show verbose help and exit\n");
>>>>>>>> +	printf("-h, --help[=topic]	Show verbose help and exit\n");
>>>>>>>> +	printf("			topics:	mbus-fmt: List supported media bus pixel
>>>>>>>> codes\n");
>>>>>>>
>>>>>>> OK, this is ugly. It has nothing to do with usage help.
>>>>>>>
>>>>>>> Just make a new option --list-mbus-fmts to list supported media bus
>>>>>>> pixel codes.
>>>>>>>
>>>>>>> That would make much more sense.
>>>>>>
>>>>>> I added it as a --help option argument in order to imply it's a part
>>>>>> of the program's usage instructions, which is what it indeed is. It's
>>>>>> not a list of media bus formats supported by a device.
>>>>>>
>>>>>> A separate option is fine, but it should be clear that it's about just
>>>>>> listing supported formats. E.g. --list-supported-mbus-fmts. But that's
>>>>>> a long one. Long options are loooong.
>>>>>
>>>>> --list-known-mbus-fmts will do the trick.
>>>>
>>>> That doesn't feel right. Isn't it a help option, really, given that it
>>>> lists the formats you can use as command line arguments ?
> 
> The help arguments don't have 'options'. You could provide a --help-list-mbus-fmts,
> though.
> 
>>>> Another option would actually be to always print the formats when the -h
>>>> switch is given. We could print them in a comma-separated list with
>>>> multiple formats per line, possibly dropping the numerical value, it
>>>> should hopefully not be horrible.
> 
> Just always printing the list when -h is given works for me too.
> 
> Regards,
> 
> 	Hans
> 
>>>
>>> I'd prefer to keep the numerical value as well; the link validation code in
>>> drivers may print the media bus code at each end in case they do not match.
>>> To debug that, it's easy to grep that from the list media-ctl prints.
>>
>> Grepping media-bus-formats.h shouldn't be difficult ;-)
>>
>> To shorten the output, how about printing the numerical values as 0x%04x or 
>> %04x ?
>>


-- 
Sakari Ailus
sakari.ailus@linux.intel.com

Re: [v4l-utils PATCH 4/4] media-ctl: List supported media bus formats

[Hans Verkuil]

On 02/23/16 21:30, Laurent Pinchart wrote:
> Hi Sakari,
> 
> On Tuesday 23 February 2016 22:24:00 Sakari Ailus wrote:
>> On Tue, Feb 23, 2016 at 10:15:46PM +0200, Laurent Pinchart wrote:
>>> On Tuesday 23 February 2016 17:15:15 Hans Verkuil wrote:
>>>> On 02/23/2016 05:11 PM, Sakari Ailus wrote:
>>>>> On Tue, Feb 23, 2016 at 01:18:53PM +0100, Hans Verkuil wrote:
>>>>>> On 02/21/16 22:29, Sakari Ailus wrote:
>>>>>>> Add a new topic option for -h to allow listing supported media bus
>>>>>>> codes in conversion functions. This is useful in figuring out which
>>>>>>> media bus codes are actually supported by the library. The numeric
>>>>>>> values of the codes are listed as well.
>>>>>>>
>>>>>>> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
>>>>>>> ---
>>>>>>>
>>>>>>>  utils/media-ctl/options.c | 42 ++++++++++++++++++++++++++++++++----
>>>>>>>  1 file changed, 38 insertions(+), 4 deletions(-)
>>>>>>>
>>>>>>> diff --git a/utils/media-ctl/options.c b/utils/media-ctl/options.c
>>>>>>> index 0afc9c2..55cdd29 100644
>>>>>>> --- a/utils/media-ctl/options.c
>>>>>>> +++ b/utils/media-ctl/options.c
> 
> [snip]
> 
>>>>>>> @@ -45,7 +47,8 @@ static void usage(const char *argv0)
>>>>>>>
>>>>>>>  	printf("-V, --set-v4l2 v4l2	Comma-separated list of formats to
>>>>>>>  	setup\n");
>>>>>>>  	printf("    --get-v4l2 pad	Print the active format on a given
>>>>>>> pad\n");
>>>>>>>  	printf("    --set-dv pad	Configure DV timings on a given pad\n");
>>>>>>>
>>>>>>> -	printf("-h, --help		Show verbose help and exit\n");
>>>>>>> +	printf("-h, --help[=topic]	Show verbose help and exit\n");
>>>>>>> +	printf("			topics:	mbus-fmt: List supported media bus pixel
>>>>>>> codes\n");
>>>>>>
>>>>>> OK, this is ugly. It has nothing to do with usage help.
>>>>>>
>>>>>> Just make a new option --list-mbus-fmts to list supported media bus
>>>>>> pixel codes.
>>>>>>
>>>>>> That would make much more sense.
>>>>>
>>>>> I added it as a --help option argument in order to imply it's a part
>>>>> of the program's usage instructions, which is what it indeed is. It's
>>>>> not a list of media bus formats supported by a device.
>>>>>
>>>>> A separate option is fine, but it should be clear that it's about just
>>>>> listing supported formats. E.g. --list-supported-mbus-fmts. But that's
>>>>> a long one. Long options are loooong.
>>>>
>>>> --list-known-mbus-fmts will do the trick.
>>>
>>> That doesn't feel right. Isn't it a help option, really, given that it
>>> lists the formats you can use as command line arguments ?

The help arguments don't have 'options'. You could provide a --help-list-mbus-fmts,
though.

>>> Another option would actually be to always print the formats when the -h
>>> switch is given. We could print them in a comma-separated list with
>>> multiple formats per line, possibly dropping the numerical value, it
>>> should hopefully not be horrible.

Just always printing the list when -h is given works for me too.

Regards,

	Hans

>>
>> I'd prefer to keep the numerical value as well; the link validation code in
>> drivers may print the media bus code at each end in case they do not match.
>> To debug that, it's easy to grep that from the list media-ctl prints.
> 
> Grepping media-bus-formats.h shouldn't be difficult ;-)
> 
> To shorten the output, how about printing the numerical values as 0x%04x or 
> %04x ?
> 

Re: [v4l-utils PATCH 4/4] media-ctl: List supported media bus formats

[Sakari Ailus]

Hi Laurent,

On Tue, Feb 23, 2016 at 10:30:35PM +0200, Laurent Pinchart wrote:
> Hi Sakari,
> 
> On Tuesday 23 February 2016 22:24:00 Sakari Ailus wrote:
> > On Tue, Feb 23, 2016 at 10:15:46PM +0200, Laurent Pinchart wrote:
> > > On Tuesday 23 February 2016 17:15:15 Hans Verkuil wrote:
> > >> On 02/23/2016 05:11 PM, Sakari Ailus wrote:
> > >>> On Tue, Feb 23, 2016 at 01:18:53PM +0100, Hans Verkuil wrote:
> > >>>> On 02/21/16 22:29, Sakari Ailus wrote:
> > >>>>> Add a new topic option for -h to allow listing supported media bus
> > >>>>> codes in conversion functions. This is useful in figuring out which
> > >>>>> media bus codes are actually supported by the library. The numeric
> > >>>>> values of the codes are listed as well.
> > >>>>> 
> > >>>>> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> > >>>>> ---
> > >>>>> 
> > >>>>>  utils/media-ctl/options.c | 42 ++++++++++++++++++++++++++++++++----
> > >>>>>  1 file changed, 38 insertions(+), 4 deletions(-)
> > >>>>> 
> > >>>>> diff --git a/utils/media-ctl/options.c b/utils/media-ctl/options.c
> > >>>>> index 0afc9c2..55cdd29 100644
> > >>>>> --- a/utils/media-ctl/options.c
> > >>>>> +++ b/utils/media-ctl/options.c
> 
> [snip]
> 
> > >>>>> @@ -45,7 +47,8 @@ static void usage(const char *argv0)
> > >>>>> 
> > >>>>>  	printf("-V, --set-v4l2 v4l2	Comma-separated list of formats to
> > >>>>>  	setup\n");
> > >>>>>  	printf("    --get-v4l2 pad	Print the active format on a given
> > >>>>> pad\n");
> > >>>>>  	printf("    --set-dv pad	Configure DV timings on a given pad\n");
> > >>>>> 
> > >>>>> -	printf("-h, --help		Show verbose help and exit\n");
> > >>>>> +	printf("-h, --help[=topic]	Show verbose help and exit\n");
> > >>>>> +	printf("			topics:	mbus-fmt: List supported media bus pixel
> > >>>>> codes\n");
> > >>>>
> > >>>> OK, this is ugly. It has nothing to do with usage help.
> > >>>> 
> > >>>> Just make a new option --list-mbus-fmts to list supported media bus
> > >>>> pixel codes.
> > >>>> 
> > >>>> That would make much more sense.
> > >>> 
> > >>> I added it as a --help option argument in order to imply it's a part
> > >>> of the program's usage instructions, which is what it indeed is. It's
> > >>> not a list of media bus formats supported by a device.
> > >>> 
> > >>> A separate option is fine, but it should be clear that it's about just
> > >>> listing supported formats. E.g. --list-supported-mbus-fmts. But that's
> > >>> a long one. Long options are loooong.
> > >> 
> > >> --list-known-mbus-fmts will do the trick.
> > > 
> > > That doesn't feel right. Isn't it a help option, really, given that it
> > > lists the formats you can use as command line arguments ?
> > > 
> > > Another option would actually be to always print the formats when the -h
> > > switch is given. We could print them in a comma-separated list with
> > > multiple formats per line, possibly dropping the numerical value, it
> > > should hopefully not be horrible.
> > 
> > I'd prefer to keep the numerical value as well; the link validation code in
> > drivers may print the media bus code at each end in case they do not match.
> > To debug that, it's easy to grep that from the list media-ctl prints.
> 
> Grepping media-bus-formats.h shouldn't be difficult ;-)
> 
> To shorten the output, how about printing the numerical values as 0x%04x or 
> %04x ?

Well, yes, you can do that. But you have to have the headers available; with
the numerical values printed by media-ctl you don't need any source code. I
presume user space developers would appreciate that.

We're not using more than 16 bits currently, I can sure change to print just
four digits.

-- 
Kind regards,

Sakari Ailus
e-mail: sakari.ailus@iki.fi	XMPP: sailus@retiisi.org.uk

Re: [v4l-utils PATCH 4/4] media-ctl: List supported media bus formats

[Laurent Pinchart]

Hi Sakari,

On Tuesday 23 February 2016 22:24:00 Sakari Ailus wrote:
> On Tue, Feb 23, 2016 at 10:15:46PM +0200, Laurent Pinchart wrote:
> > On Tuesday 23 February 2016 17:15:15 Hans Verkuil wrote:
> >> On 02/23/2016 05:11 PM, Sakari Ailus wrote:
> >>> On Tue, Feb 23, 2016 at 01:18:53PM +0100, Hans Verkuil wrote:
> >>>> On 02/21/16 22:29, Sakari Ailus wrote:
> >>>>> Add a new topic option for -h to allow listing supported media bus
> >>>>> codes in conversion functions. This is useful in figuring out which
> >>>>> media bus codes are actually supported by the library. The numeric
> >>>>> values of the codes are listed as well.
> >>>>> 
> >>>>> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> >>>>> ---
> >>>>> 
> >>>>>  utils/media-ctl/options.c | 42 ++++++++++++++++++++++++++++++++----
> >>>>>  1 file changed, 38 insertions(+), 4 deletions(-)
> >>>>> 
> >>>>> diff --git a/utils/media-ctl/options.c b/utils/media-ctl/options.c
> >>>>> index 0afc9c2..55cdd29 100644
> >>>>> --- a/utils/media-ctl/options.c
> >>>>> +++ b/utils/media-ctl/options.c

[snip]

> >>>>> @@ -45,7 +47,8 @@ static void usage(const char *argv0)
> >>>>> 
> >>>>>  	printf("-V, --set-v4l2 v4l2	Comma-separated list of formats to
> >>>>>  	setup\n");
> >>>>>  	printf("    --get-v4l2 pad	Print the active format on a given
> >>>>> pad\n");
> >>>>>  	printf("    --set-dv pad	Configure DV timings on a given pad\n");
> >>>>> 
> >>>>> -	printf("-h, --help		Show verbose help and exit\n");
> >>>>> +	printf("-h, --help[=topic]	Show verbose help and exit\n");
> >>>>> +	printf("			topics:	mbus-fmt: List supported media bus pixel
> >>>>> codes\n");
> >>>>
> >>>> OK, this is ugly. It has nothing to do with usage help.
> >>>> 
> >>>> Just make a new option --list-mbus-fmts to list supported media bus
> >>>> pixel codes.
> >>>> 
> >>>> That would make much more sense.
> >>> 
> >>> I added it as a --help option argument in order to imply it's a part
> >>> of the program's usage instructions, which is what it indeed is. It's
> >>> not a list of media bus formats supported by a device.
> >>> 
> >>> A separate option is fine, but it should be clear that it's about just
> >>> listing supported formats. E.g. --list-supported-mbus-fmts. But that's
> >>> a long one. Long options are loooong.
> >> 
> >> --list-known-mbus-fmts will do the trick.
> > 
> > That doesn't feel right. Isn't it a help option, really, given that it
> > lists the formats you can use as command line arguments ?
> > 
> > Another option would actually be to always print the formats when the -h
> > switch is given. We could print them in a comma-separated list with
> > multiple formats per line, possibly dropping the numerical value, it
> > should hopefully not be horrible.
> 
> I'd prefer to keep the numerical value as well; the link validation code in
> drivers may print the media bus code at each end in case they do not match.
> To debug that, it's easy to grep that from the list media-ctl prints.

Grepping media-bus-formats.h shouldn't be difficult ;-)

To shorten the output, how about printing the numerical values as 0x%04x or 
%04x ?

-- 
Regards,

Laurent Pinchart

Re: [v4l-utils PATCH 4/4] media-ctl: List supported media bus formats

[Sakari Ailus]

Hi Laurent,

On Tue, Feb 23, 2016 at 10:15:46PM +0200, Laurent Pinchart wrote:
> Hi Hans,
> 
> On Tuesday 23 February 2016 17:15:15 Hans Verkuil wrote:
> > On 02/23/2016 05:11 PM, Sakari Ailus wrote:
> > > On Tue, Feb 23, 2016 at 01:18:53PM +0100, Hans Verkuil wrote:
> > >> On 02/21/16 22:29, Sakari Ailus wrote:
> > >>> Add a new topic option for -h to allow listing supported media bus codes
> > >>> in conversion functions. This is useful in figuring out which media bus
> > >>> codes are actually supported by the library. The numeric values of the
> > >>> codes are listed as well.
> > >>> 
> > >>> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> > >>> ---
> > >>> 
> > >>>  utils/media-ctl/options.c | 42 ++++++++++++++++++++++++++++++++++++----
> > >>>  1 file changed, 38 insertions(+), 4 deletions(-)
> > >>> 
> > >>> diff --git a/utils/media-ctl/options.c b/utils/media-ctl/options.c
> > >>> index 0afc9c2..55cdd29 100644
> > >>> --- a/utils/media-ctl/options.c
> > >>> +++ b/utils/media-ctl/options.c
> > >>> @@ -22,7 +22,9 @@
> > >>>  #include <getopt.h>
> > >>>  #include <stdio.h>
> > >>>  #include <stdlib.h>
> > >>> +#include <string.h>
> > >>>  #include <unistd.h>
> > >>> +#include <v4l2subdev.h>
> > >>> 
> > >>>  #include <linux/videodev2.h>
> > >>> 
> > >>> @@ -45,7 +47,8 @@ static void usage(const char *argv0)
> > >>>  	printf("-V, --set-v4l2 v4l2	Comma-separated list of formats to
> > >>>  	setup\n");
> > >>>  	printf("    --get-v4l2 pad	Print the active format on a given 
> pad\n");
> > >>>  	printf("    --set-dv pad	Configure DV timings on a given pad\n");
> > >>> -	printf("-h, --help		Show verbose help and exit\n");
> > >>> +	printf("-h, --help[=topic]	Show verbose help and exit\n");
> > >>> +	printf("			topics:	mbus-fmt: List supported media bus pixel 
> codes\n");
> > >> 
> > >> OK, this is ugly. It has nothing to do with usage help.
> > >> 
> > >> Just make a new option --list-mbus-fmts to list supported media bus pixel
> > >> codes.
> > >> 
> > >> That would make much more sense.
> > > 
> > > I added it as a --help option argument in order to imply it's a part of
> > > the program's usage instructions, which is what it indeed is. It's not a
> > > list of media bus formats supported by a device.
> > > 
> > > A separate option is fine, but it should be clear that it's about just
> > > listing supported formats. E.g. --list-supported-mbus-fmts. But that's a
> > > long one. Long options are loooong.
> > 
> > --list-known-mbus-fmts will do the trick.
> 
> That doesn't feel right. Isn't it a help option, really, given that it lists 
> the formats you can use as command line arguments ?
> 
> Another option would actually be to always print the formats when the -h 
> switch is given. We could print them in a comma-separated list with multiple 
> formats per line, possibly dropping the numerical value, it should hopefully 
> not be horrible.

I'd prefer to keep the numerical value as well; the link validation code in
drivers may print the media bus code at each end in case they do not match.
To debug that, it's easy to grep that from the list media-ctl prints.

-- 
Regards,

Sakari Ailus
e-mail: sakari.ailus@iki.fi	XMPP: sailus@retiisi.org.uk

Re: [v4l-utils PATCH 4/4] media-ctl: List supported media bus formats

[Laurent Pinchart]

Hi Hans,

On Tuesday 23 February 2016 17:15:15 Hans Verkuil wrote:
> On 02/23/2016 05:11 PM, Sakari Ailus wrote:
> > On Tue, Feb 23, 2016 at 01:18:53PM +0100, Hans Verkuil wrote:
> >> On 02/21/16 22:29, Sakari Ailus wrote:
> >>> Add a new topic option for -h to allow listing supported media bus codes
> >>> in conversion functions. This is useful in figuring out which media bus
> >>> codes are actually supported by the library. The numeric values of the
> >>> codes are listed as well.
> >>> 
> >>> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> >>> ---
> >>> 
> >>>  utils/media-ctl/options.c | 42 ++++++++++++++++++++++++++++++++++++----
> >>>  1 file changed, 38 insertions(+), 4 deletions(-)
> >>> 
> >>> diff --git a/utils/media-ctl/options.c b/utils/media-ctl/options.c
> >>> index 0afc9c2..55cdd29 100644
> >>> --- a/utils/media-ctl/options.c
> >>> +++ b/utils/media-ctl/options.c
> >>> @@ -22,7 +22,9 @@
> >>>  #include <getopt.h>
> >>>  #include <stdio.h>
> >>>  #include <stdlib.h>
> >>> +#include <string.h>
> >>>  #include <unistd.h>
> >>> +#include <v4l2subdev.h>
> >>> 
> >>>  #include <linux/videodev2.h>
> >>> 
> >>> @@ -45,7 +47,8 @@ static void usage(const char *argv0)
> >>>  	printf("-V, --set-v4l2 v4l2	Comma-separated list of formats to
> >>>  	setup\n");
> >>>  	printf("    --get-v4l2 pad	Print the active format on a given 
pad\n");
> >>>  	printf("    --set-dv pad	Configure DV timings on a given pad\n");
> >>> -	printf("-h, --help		Show verbose help and exit\n");
> >>> +	printf("-h, --help[=topic]	Show verbose help and exit\n");
> >>> +	printf("			topics:	mbus-fmt: List supported media bus pixel 
codes\n");
> >> 
> >> OK, this is ugly. It has nothing to do with usage help.
> >> 
> >> Just make a new option --list-mbus-fmts to list supported media bus pixel
> >> codes.
> >> 
> >> That would make much more sense.
> > 
> > I added it as a --help option argument in order to imply it's a part of
> > the program's usage instructions, which is what it indeed is. It's not a
> > list of media bus formats supported by a device.
> > 
> > A separate option is fine, but it should be clear that it's about just
> > listing supported formats. E.g. --list-supported-mbus-fmts. But that's a
> > long one. Long options are loooong.
> 
> --list-known-mbus-fmts will do the trick.

That doesn't feel right. Isn't it a help option, really, given that it lists 
the formats you can use as command line arguments ?

Another option would actually be to always print the formats when the -h 
switch is given. We could print them in a comma-separated list with multiple 
formats per line, possibly dropping the numerical value, it should hopefully 
not be horrible.

-- 
Regards,

Laurent Pinchart

Re: [v4l-utils PATCH 4/4] media-ctl: List supported media bus formats

[Hans Verkuil]

On 02/23/2016 05:11 PM, Sakari Ailus wrote:
> Hi Hans,
> 
> On Tue, Feb 23, 2016 at 01:18:53PM +0100, Hans Verkuil wrote:
>> On 02/21/16 22:29, Sakari Ailus wrote:
>>> Add a new topic option for -h to allow listing supported media bus codes
>>> in conversion functions. This is useful in figuring out which media bus
>>> codes are actually supported by the library. The numeric values of the
>>> codes are listed as well.
>>>
>>> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
>>> ---
>>>  utils/media-ctl/options.c | 42 ++++++++++++++++++++++++++++++++++++++----
>>>  1 file changed, 38 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/utils/media-ctl/options.c b/utils/media-ctl/options.c
>>> index 0afc9c2..55cdd29 100644
>>> --- a/utils/media-ctl/options.c
>>> +++ b/utils/media-ctl/options.c
>>> @@ -22,7 +22,9 @@
>>>  #include <getopt.h>
>>>  #include <stdio.h>
>>>  #include <stdlib.h>
>>> +#include <string.h>
>>>  #include <unistd.h>
>>> +#include <v4l2subdev.h>
>>>  
>>>  #include <linux/videodev2.h>
>>>  
>>> @@ -45,7 +47,8 @@ static void usage(const char *argv0)
>>>  	printf("-V, --set-v4l2 v4l2	Comma-separated list of formats to setup\n");
>>>  	printf("    --get-v4l2 pad	Print the active format on a given pad\n");
>>>  	printf("    --set-dv pad	Configure DV timings on a given pad\n");
>>> -	printf("-h, --help		Show verbose help and exit\n");
>>> +	printf("-h, --help[=topic]	Show verbose help and exit\n");
>>> +	printf("			topics:	mbus-fmt: List supported media bus pixel codes\n");
>>
>> OK, this is ugly. It has nothing to do with usage help.
>>
>> Just make a new option --list-mbus-fmts to list supported media bus pixel
>> codes.
>>
>> That would make much more sense.
> 
> I added it as a --help option argument in order to imply it's a part of the
> program's usage instructions, which is what it indeed is. It's not a list of
> media bus formats supported by a device.
> 
> A separate option is fine, but it should be clear that it's about just
> listing supported formats. E.g. --list-supported-mbus-fmts. But that's a
> long one. Long options are loooong.

--list-known-mbus-fmts will do the trick.

Regards,

	Hans

Re: [v4l-utils PATCH 4/4] media-ctl: List supported media bus formats

[Sakari Ailus]

Hi Hans,

On Tue, Feb 23, 2016 at 01:18:53PM +0100, Hans Verkuil wrote:
> On 02/21/16 22:29, Sakari Ailus wrote:
> > Add a new topic option for -h to allow listing supported media bus codes
> > in conversion functions. This is useful in figuring out which media bus
> > codes are actually supported by the library. The numeric values of the
> > codes are listed as well.
> > 
> > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> > ---
> >  utils/media-ctl/options.c | 42 ++++++++++++++++++++++++++++++++++++++----
> >  1 file changed, 38 insertions(+), 4 deletions(-)
> > 
> > diff --git a/utils/media-ctl/options.c b/utils/media-ctl/options.c
> > index 0afc9c2..55cdd29 100644
> > --- a/utils/media-ctl/options.c
> > +++ b/utils/media-ctl/options.c
> > @@ -22,7 +22,9 @@
> >  #include <getopt.h>
> >  #include <stdio.h>
> >  #include <stdlib.h>
> > +#include <string.h>
> >  #include <unistd.h>
> > +#include <v4l2subdev.h>
> >  
> >  #include <linux/videodev2.h>
> >  
> > @@ -45,7 +47,8 @@ static void usage(const char *argv0)
> >  	printf("-V, --set-v4l2 v4l2	Comma-separated list of formats to setup\n");
> >  	printf("    --get-v4l2 pad	Print the active format on a given pad\n");
> >  	printf("    --set-dv pad	Configure DV timings on a given pad\n");
> > -	printf("-h, --help		Show verbose help and exit\n");
> > +	printf("-h, --help[=topic]	Show verbose help and exit\n");
> > +	printf("			topics:	mbus-fmt: List supported media bus pixel codes\n");
> 
> OK, this is ugly. It has nothing to do with usage help.
> 
> Just make a new option --list-mbus-fmts to list supported media bus pixel
> codes.
> 
> That would make much more sense.

I added it as a --help option argument in order to imply it's a part of the
program's usage instructions, which is what it indeed is. It's not a list of
media bus formats supported by a device.

A separate option is fine, but it should be clear that it's about just
listing supported formats. E.g. --list-supported-mbus-fmts. But that's a
long one. Long options are loooong.

-- 
Kind regards,

Sakari Ailus
e-mail: sakari.ailus@iki.fi	XMPP: sailus@retiisi.org.uk

Re: [v4l-utils PATCH 4/4] media-ctl: List supported media bus formats

[Hans Verkuil]

On 02/21/16 22:29, Sakari Ailus wrote:
> Add a new topic option for -h to allow listing supported media bus codes
> in conversion functions. This is useful in figuring out which media bus
> codes are actually supported by the library. The numeric values of the
> codes are listed as well.
> 
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> ---
>  utils/media-ctl/options.c | 42 ++++++++++++++++++++++++++++++++++++++----
>  1 file changed, 38 insertions(+), 4 deletions(-)
> 
> diff --git a/utils/media-ctl/options.c b/utils/media-ctl/options.c
> index 0afc9c2..55cdd29 100644
> --- a/utils/media-ctl/options.c
> +++ b/utils/media-ctl/options.c
> @@ -22,7 +22,9 @@
>  #include <getopt.h>
>  #include <stdio.h>
>  #include <stdlib.h>
> +#include <string.h>
>  #include <unistd.h>
> +#include <v4l2subdev.h>
>  
>  #include <linux/videodev2.h>
>  
> @@ -45,7 +47,8 @@ static void usage(const char *argv0)
>  	printf("-V, --set-v4l2 v4l2	Comma-separated list of formats to setup\n");
>  	printf("    --get-v4l2 pad	Print the active format on a given pad\n");
>  	printf("    --set-dv pad	Configure DV timings on a given pad\n");
> -	printf("-h, --help		Show verbose help and exit\n");
> +	printf("-h, --help[=topic]	Show verbose help and exit\n");
> +	printf("			topics:	mbus-fmt: List supported media bus pixel codes\n");
>  	printf("-i, --interactive	Modify links interactively\n");
>  	printf("-l, --links links	Comma-separated list of link descriptors to setup\n");
>  	printf("-p, --print-topology	Print the device topology\n");
> @@ -100,7 +103,7 @@ static struct option opts[] = {
>  	{"get-format", 1, 0, OPT_GET_FORMAT},
>  	{"get-v4l2", 1, 0, OPT_GET_FORMAT},
>  	{"set-dv", 1, 0, OPT_SET_DV},
> -	{"help", 0, 0, 'h'},
> +	{"help", 2, 0, 'h'},
>  	{"interactive", 0, 0, 'i'},
>  	{"links", 1, 0, 'l'},
>  	{"print-dot", 0, 0, OPT_PRINT_DOT},
> @@ -110,6 +113,27 @@ static struct option opts[] = {
>  	{ },
>  };
>  
> +void list_mbus_formats(void)
> +{
> +	unsigned int ncodes;
> +	const unsigned int *code = v4l2_subdev_pixelcode_list(&ncodes);
> +
> +	printf("Supported media bus pixel codes\n");
> +
> +	for (ncodes++; ncodes; ncodes--, code++) {

Huh? Just do:

	while (ncodes--) {
		....
		code++;
	}

That for loop is weird.

	Hans

> +		const char *str = v4l2_subdev_pixelcode_to_string(*code);
> +		int spaces = 30 - (int)strlen(str);
> +
> +		if (*code == 0)
> +			break;
> +
> +		if (spaces < 0)
> +			spaces = 0;
> +
> +		printf("\t%s %*c (0x%8.8x)\n", str, spaces, ' ', *code);
> +	}
> +}
> +
>  int parse_cmdline(int argc, char **argv)
>  {
>  	int opt;
> @@ -120,7 +144,8 @@ int parse_cmdline(int argc, char **argv)
>  	}
>  
>  	/* parse options */
> -	while ((opt = getopt_long(argc, argv, "d:e:f:hil:prvV:", opts, NULL)) != -1) {
> +	while ((opt = getopt_long(argc, argv, "d:e:f:h::il:prvV:",
> +				  opts, NULL)) != -1) {
>  		switch (opt) {
>  		case 'd':
>  			media_opts.devname = optarg;
> @@ -142,7 +167,16 @@ int parse_cmdline(int argc, char **argv)
>  			break;
>  
>  		case 'h':
> -			usage(argv[0]);
> +			if (optarg) {
> +				if (!strcmp(optarg, "mbus-fmt"))
> +					list_mbus_formats();
> +				else
> +					fprintf(stderr,
> +						"Unknown topic \"%s\"\n",
> +						optarg);
> +			} else {
> +				usage(argv[0]);
> +			}
>  			exit(0);
>  
>  		case 'i':
> 

Re: [v4l-utils PATCH 4/4] media-ctl: List supported media bus formats

[Hans Verkuil]

On 02/21/16 22:29, Sakari Ailus wrote:
> Add a new topic option for -h to allow listing supported media bus codes
> in conversion functions. This is useful in figuring out which media bus
> codes are actually supported by the library. The numeric values of the
> codes are listed as well.
> 
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> ---
>  utils/media-ctl/options.c | 42 ++++++++++++++++++++++++++++++++++++++----
>  1 file changed, 38 insertions(+), 4 deletions(-)
> 
> diff --git a/utils/media-ctl/options.c b/utils/media-ctl/options.c
> index 0afc9c2..55cdd29 100644
> --- a/utils/media-ctl/options.c
> +++ b/utils/media-ctl/options.c
> @@ -22,7 +22,9 @@
>  #include <getopt.h>
>  #include <stdio.h>
>  #include <stdlib.h>
> +#include <string.h>
>  #include <unistd.h>
> +#include <v4l2subdev.h>
>  
>  #include <linux/videodev2.h>
>  
> @@ -45,7 +47,8 @@ static void usage(const char *argv0)
>  	printf("-V, --set-v4l2 v4l2	Comma-separated list of formats to setup\n");
>  	printf("    --get-v4l2 pad	Print the active format on a given pad\n");
>  	printf("    --set-dv pad	Configure DV timings on a given pad\n");
> -	printf("-h, --help		Show verbose help and exit\n");
> +	printf("-h, --help[=topic]	Show verbose help and exit\n");
> +	printf("			topics:	mbus-fmt: List supported media bus pixel codes\n");

OK, this is ugly. It has nothing to do with usage help.

Just make a new option --list-mbus-fmts to list supported media bus pixel codes.

That would make much more sense.

Regards,

	Hans

>  	printf("-i, --interactive	Modify links interactively\n");
>  	printf("-l, --links links	Comma-separated list of link descriptors to setup\n");
>  	printf("-p, --print-topology	Print the device topology\n");
> @@ -100,7 +103,7 @@ static struct option opts[] = {
>  	{"get-format", 1, 0, OPT_GET_FORMAT},
>  	{"get-v4l2", 1, 0, OPT_GET_FORMAT},
>  	{"set-dv", 1, 0, OPT_SET_DV},
> -	{"help", 0, 0, 'h'},
> +	{"help", 2, 0, 'h'},
>  	{"interactive", 0, 0, 'i'},
>  	{"links", 1, 0, 'l'},
>  	{"print-dot", 0, 0, OPT_PRINT_DOT},
> @@ -110,6 +113,27 @@ static struct option opts[] = {
>  	{ },
>  };
>  
> +void list_mbus_formats(void)
> +{
> +	unsigned int ncodes;
> +	const unsigned int *code = v4l2_subdev_pixelcode_list(&ncodes);
> +
> +	printf("Supported media bus pixel codes\n");
> +
> +	for (ncodes++; ncodes; ncodes--, code++) {
> +		const char *str = v4l2_subdev_pixelcode_to_string(*code);
> +		int spaces = 30 - (int)strlen(str);
> +
> +		if (*code == 0)
> +			break;
> +
> +		if (spaces < 0)
> +			spaces = 0;
> +
> +		printf("\t%s %*c (0x%8.8x)\n", str, spaces, ' ', *code);
> +	}
> +}
> +
>  int parse_cmdline(int argc, char **argv)
>  {
>  	int opt;
> @@ -120,7 +144,8 @@ int parse_cmdline(int argc, char **argv)
>  	}
>  
>  	/* parse options */
> -	while ((opt = getopt_long(argc, argv, "d:e:f:hil:prvV:", opts, NULL)) != -1) {
> +	while ((opt = getopt_long(argc, argv, "d:e:f:h::il:prvV:",
> +				  opts, NULL)) != -1) {
>  		switch (opt) {
>  		case 'd':
>  			media_opts.devname = optarg;
> @@ -142,7 +167,16 @@ int parse_cmdline(int argc, char **argv)
>  			break;
>  
>  		case 'h':
> -			usage(argv[0]);
> +			if (optarg) {
> +				if (!strcmp(optarg, "mbus-fmt"))
> +					list_mbus_formats();
> +				else
> +					fprintf(stderr,
> +						"Unknown topic \"%s\"\n",
> +						optarg);
> +			} else {
> +				usage(argv[0]);
> +			}
>  			exit(0);
>  
>  		case 'i':
> 

[v4l-utils PATCH 4/4] media-ctl: List supported media bus formats

[Sakari Ailus]

Add a new topic option for -h to allow listing supported media bus codes
in conversion functions. This is useful in figuring out which media bus
codes are actually supported by the library. The numeric values of the
codes are listed as well.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 utils/media-ctl/options.c | 42 ++++++++++++++++++++++++++++++++++++++----
 1 file changed, 38 insertions(+), 4 deletions(-)

diff --git a/utils/media-ctl/options.c b/utils/media-ctl/options.c
index 0afc9c2..55cdd29 100644
--- a/utils/media-ctl/options.c
+++ b/utils/media-ctl/options.c
@@ -22,7 +22,9 @@
 #include <getopt.h>
 #include <stdio.h>
 #include <stdlib.h>
+#include <string.h>
 #include <unistd.h>
+#include <v4l2subdev.h>
 
 #include <linux/videodev2.h>
 
@@ -45,7 +47,8 @@ static void usage(const char *argv0)
 	printf("-V, --set-v4l2 v4l2	Comma-separated list of formats to setup\n");
 	printf("    --get-v4l2 pad	Print the active format on a given pad\n");
 	printf("    --set-dv pad	Configure DV timings on a given pad\n");
-	printf("-h, --help		Show verbose help and exit\n");
+	printf("-h, --help[=topic]	Show verbose help and exit\n");
+	printf("			topics:	mbus-fmt: List supported media bus pixel codes\n");
 	printf("-i, --interactive	Modify links interactively\n");
 	printf("-l, --links links	Comma-separated list of link descriptors to setup\n");
 	printf("-p, --print-topology	Print the device topology\n");
@@ -100,7 +103,7 @@ static struct option opts[] = {
 	{"get-format", 1, 0, OPT_GET_FORMAT},
 	{"get-v4l2", 1, 0, OPT_GET_FORMAT},
 	{"set-dv", 1, 0, OPT_SET_DV},
-	{"help", 0, 0, 'h'},
+	{"help", 2, 0, 'h'},
 	{"interactive", 0, 0, 'i'},
 	{"links", 1, 0, 'l'},
 	{"print-dot", 0, 0, OPT_PRINT_DOT},
@@ -110,6 +113,27 @@ static struct option opts[] = {
 	{ },
 };
 
+void list_mbus_formats(void)
+{
+	unsigned int ncodes;
+	const unsigned int *code = v4l2_subdev_pixelcode_list(&ncodes);
+
+	printf("Supported media bus pixel codes\n");
+
+	for (ncodes++; ncodes; ncodes--, code++) {
+		const char *str = v4l2_subdev_pixelcode_to_string(*code);
+		int spaces = 30 - (int)strlen(str);
+
+		if (*code == 0)
+			break;
+
+		if (spaces < 0)
+			spaces = 0;
+
+		printf("\t%s %*c (0x%8.8x)\n", str, spaces, ' ', *code);
+	}
+}
+
 int parse_cmdline(int argc, char **argv)
 {
 	int opt;
@@ -120,7 +144,8 @@ int parse_cmdline(int argc, char **argv)
 	}
 
 	/* parse options */
-	while ((opt = getopt_long(argc, argv, "d:e:f:hil:prvV:", opts, NULL)) != -1) {
+	while ((opt = getopt_long(argc, argv, "d:e:f:h::il:prvV:",
+				  opts, NULL)) != -1) {
 		switch (opt) {
 		case 'd':
 			media_opts.devname = optarg;
@@ -142,7 +167,16 @@ int parse_cmdline(int argc, char **argv)
 			break;
 
 		case 'h':
-			usage(argv[0]);
+			if (optarg) {
+				if (!strcmp(optarg, "mbus-fmt"))
+					list_mbus_formats();
+				else
+					fprintf(stderr,
+						"Unknown topic \"%s\"\n",
+						optarg);
+			} else {
+				usage(argv[0]);
+			}
 			exit(0);
 
 		case 'i':
-- 
2.1.0.231.g7484e3b