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

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

[Sakari Ailus]

Hi,

I've changed the patches according to comments from Laurent.

since v2:

- Remove the guardian (format code zero at the end of the list).

- v4l2_subdev_pixelcode_list() called only once pre printing the list.

-- 
Kind regards,
Sakari

[v4l-utils PATCH 1/4] v4l: libv4lsubdev: Make mbus_formats array const

[Sakari Ailus]

The array is already static and may not be modified at runtime. Make it
const.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 utils/media-ctl/libv4l2subdev.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/utils/media-ctl/libv4l2subdev.c b/utils/media-ctl/libv4l2subdev.c
index dc2cd87..e45834f 100644
--- a/utils/media-ctl/libv4l2subdev.c
+++ b/utils/media-ctl/libv4l2subdev.c
@@ -715,7 +715,7 @@ int v4l2_subdev_parse_setup_formats(struct media_device *media, const char *p)
 	return *end ? -EINVAL : 0;
 }
 
-static struct {
+static const struct {
 	const char *name;
 	enum v4l2_mbus_pixelcode code;
 } mbus_formats[] = {
-- 
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>
Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.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..799ab33 100644
--- a/utils/media-ctl/.gitignore
+++ b/utils/media-ctl/.gitignore
@@ -1 +1,2 @@
 media-ctl
+media-bus-format-names.h
diff --git a/utils/media-ctl/Makefile.am b/utils/media-ctl/Makefile.am
index a3931fb..23ad90b 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-format-names.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-format-names.h
+CLEANFILES = $(BUILT_SOURCES)
+
+nodist_libv4l2subdev_la_SOURCES = $(BUILT_SOURCES)
 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 e45834f..f3c0a9a 100644
--- a/utils/media-ctl/libv4l2subdev.c
+++ b/utils/media-ctl/libv4l2subdev.c
@@ -719,6 +719,7 @@ static const struct {
 	const char *name;
 	enum v4l2_mbus_pixelcode code;
 } mbus_formats[] = {
+#include "media-bus-format-names.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/.gitignore      |  1 +
 utils/media-ctl/Makefile.am     |  6 +++-
 utils/media-ctl/libv4l2subdev.c | 72 ++++++++++++++++++++++++-----------------
 utils/media-ctl/v4l2subdev.h    | 11 +++++++
 4 files changed, 59 insertions(+), 31 deletions(-)

diff --git a/utils/media-ctl/.gitignore b/utils/media-ctl/.gitignore
index 799ab33..5354fec 100644
--- a/utils/media-ctl/.gitignore
+++ b/utils/media-ctl/.gitignore
@@ -1,2 +1,3 @@
 media-ctl
 media-bus-format-names.h
+media-bus-format-codes.h
diff --git a/utils/media-ctl/Makefile.am b/utils/media-ctl/Makefile.am
index 23ad90b..ee7dcc9 100644
--- a/utils/media-ctl/Makefile.am
+++ b/utils/media-ctl/Makefile.am
@@ -8,7 +8,11 @@ media-bus-format-names.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-format-names.h
+media-bus-format-codes.h: ../../include/linux/media-bus-format.h
+	sed -e '/#define MEDIA_BUS_FMT/ ! d; s/.*#define //; /FIXED/ d; s/\t.*//; s/.*/ &,/;' \
+	< $< > $@
+
+BUILT_SOURCES = media-bus-format-names.h media-bus-format-codes.h
 CLEANFILES = $(BUILT_SOURCES)
 
 nodist_libv4l2subdev_la_SOURCES = $(BUILT_SOURCES)
diff --git a/utils/media-ctl/libv4l2subdev.c b/utils/media-ctl/libv4l2subdev.c
index f3c0a9a..20d95b4 100644
--- a/utils/media-ctl/libv4l2subdev.c
+++ b/utils/media-ctl/libv4l2subdev.c
@@ -718,38 +718,43 @@ int v4l2_subdev_parse_setup_formats(struct media_device *media, const char *p)
 static const struct {
 	const char *name;
 	enum v4l2_mbus_pixelcode code;
+	bool compat;
 } mbus_formats[] = {
 #include "media-bus-format-names.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 },
-	{ "VUY24", MEDIA_BUS_FMT_VUY8_1X24 },
-	{ "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 },
-	{ "RBG24", MEDIA_BUS_FMT_RBG888_1X24 },
-	{ "RGB32", MEDIA_BUS_FMT_RGB888_1X32_PADHI },
-	{ "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 },
+	{ "VUY24", MEDIA_BUS_FMT_VUY8_1X24, 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 },
+	{ "RBG24", MEDIA_BUS_FMT_RBG888_1X24, true },
+	{ "RGB32", MEDIA_BUS_FMT_RGB888_1X32_PADHI, true },
+	{ "ARGB32", MEDIA_BUS_FMT_ARGB8888_1X32, true },
+};
+
+static const enum v4l2_mbus_pixelcode mbus_codes[] = {
+#include "media-bus-format-codes.h"
 };
 
 const char *v4l2_subdev_pixelcode_to_string(enum v4l2_mbus_pixelcode code)
@@ -821,3 +826,10 @@ enum v4l2_field v4l2_subdev_string_to_field(const char *string,
 
 	return fields[i].field;
 }
+
+const enum v4l2_mbus_pixelcode *v4l2_subdev_pixelcode_list(unsigned int *length)
+{
+	*length = ARRAY_SIZE(mbus_codes);
+
+	return mbus_codes;
+}
diff --git a/utils/media-ctl/v4l2subdev.h b/utils/media-ctl/v4l2subdev.h
index 104e420..97f46a8 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 length - the number of the supported pixel codes
+ *
+ * Obtain pixel codes supported by libv4l2subdev.
+ *
+ * @return A pointer to the pixel code array
+ */
+const enum v4l2_mbus_pixelcode *v4l2_subdev_pixelcode_list(
+	unsigned int *length);
+
 #endif
-- 
2.1.0.231.g7484e3b

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

[Hans Verkuil]

On 02/21/16 22:29, Sakari Ailus wrote:
> 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.

This new compat field doesn't seem to be used...

Regards,

	Hans

> 
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> ---
>  utils/media-ctl/.gitignore      |  1 +
>  utils/media-ctl/Makefile.am     |  6 +++-
>  utils/media-ctl/libv4l2subdev.c | 72 ++++++++++++++++++++++++-----------------
>  utils/media-ctl/v4l2subdev.h    | 11 +++++++
>  4 files changed, 59 insertions(+), 31 deletions(-)
> 
> diff --git a/utils/media-ctl/.gitignore b/utils/media-ctl/.gitignore
> index 799ab33..5354fec 100644
> --- a/utils/media-ctl/.gitignore
> +++ b/utils/media-ctl/.gitignore
> @@ -1,2 +1,3 @@
>  media-ctl
>  media-bus-format-names.h
> +media-bus-format-codes.h
> diff --git a/utils/media-ctl/Makefile.am b/utils/media-ctl/Makefile.am
> index 23ad90b..ee7dcc9 100644
> --- a/utils/media-ctl/Makefile.am
> +++ b/utils/media-ctl/Makefile.am
> @@ -8,7 +8,11 @@ media-bus-format-names.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-format-names.h
> +media-bus-format-codes.h: ../../include/linux/media-bus-format.h
> +	sed -e '/#define MEDIA_BUS_FMT/ ! d; s/.*#define //; /FIXED/ d; s/\t.*//; s/.*/ &,/;' \
> +	< $< > $@
> +
> +BUILT_SOURCES = media-bus-format-names.h media-bus-format-codes.h
>  CLEANFILES = $(BUILT_SOURCES)
>  
>  nodist_libv4l2subdev_la_SOURCES = $(BUILT_SOURCES)
> diff --git a/utils/media-ctl/libv4l2subdev.c b/utils/media-ctl/libv4l2subdev.c
> index f3c0a9a..20d95b4 100644
> --- a/utils/media-ctl/libv4l2subdev.c
> +++ b/utils/media-ctl/libv4l2subdev.c
> @@ -718,38 +718,43 @@ int v4l2_subdev_parse_setup_formats(struct media_device *media, const char *p)
>  static const struct {
>  	const char *name;
>  	enum v4l2_mbus_pixelcode code;
> +	bool compat;
>  } mbus_formats[] = {
>  #include "media-bus-format-names.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 },
> -	{ "VUY24", MEDIA_BUS_FMT_VUY8_1X24 },
> -	{ "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 },
> -	{ "RBG24", MEDIA_BUS_FMT_RBG888_1X24 },
> -	{ "RGB32", MEDIA_BUS_FMT_RGB888_1X32_PADHI },
> -	{ "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 },
> +	{ "VUY24", MEDIA_BUS_FMT_VUY8_1X24, 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 },
> +	{ "RBG24", MEDIA_BUS_FMT_RBG888_1X24, true },
> +	{ "RGB32", MEDIA_BUS_FMT_RGB888_1X32_PADHI, true },
> +	{ "ARGB32", MEDIA_BUS_FMT_ARGB8888_1X32, true },
> +};
> +
> +static const enum v4l2_mbus_pixelcode mbus_codes[] = {
> +#include "media-bus-format-codes.h"
>  };
>  
>  const char *v4l2_subdev_pixelcode_to_string(enum v4l2_mbus_pixelcode code)
> @@ -821,3 +826,10 @@ enum v4l2_field v4l2_subdev_string_to_field(const char *string,
>  
>  	return fields[i].field;
>  }
> +
> +const enum v4l2_mbus_pixelcode *v4l2_subdev_pixelcode_list(unsigned int *length)
> +{
> +	*length = ARRAY_SIZE(mbus_codes);
> +
> +	return mbus_codes;
> +}
> diff --git a/utils/media-ctl/v4l2subdev.h b/utils/media-ctl/v4l2subdev.h
> index 104e420..97f46a8 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 length - the number of the supported pixel codes
> + *
> + * Obtain pixel codes supported by libv4l2subdev.
> + *
> + * @return A pointer to the pixel code array
> + */
> +const enum v4l2_mbus_pixelcode *v4l2_subdev_pixelcode_list(
> +	unsigned int *length);
> +
>  #endif
> 

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

[Sakari Ailus]

On Tue, Feb 23, 2016 at 01:37:48PM +0100, Hans Verkuil wrote:
> On 02/21/16 22:29, Sakari Ailus wrote:
> > 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.
> 
> This new compat field doesn't seem to be used...

Good catch... that was an addition made in an earlier version of the patch;
the information is no longer needed once the table containing the formats is
available. I'll remove it.

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

[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

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':
> 

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/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

[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

[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 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: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

[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]

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

[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

[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':
>