linux-media.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [yavta PATCH v2 1/1] v4l2-ctl: Add support for META_OUTPUT buffer type
@ 2019-01-22 12:05 Sakari Ailus
  2019-01-24  8:44 ` Hans Verkuil
  0 siblings, 1 reply; 3+ messages in thread
From: Sakari Ailus @ 2019-01-22 12:05 UTC (permalink / raw)
  To: linux-media; +Cc: rajmohan.mani, yong.zhi, hverkuil

Add support for META_OUTPUT buffer type to v4l2-ctl.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
Hi Hans,

Here's an update for the meta output buffer type in v4l2-ctl.

Since v1:

- Merge help text for meta and meta out buffer types

- Unify implementation for meta format handling

 utils/v4l2-ctl/v4l2-ctl-meta.cpp | 71 ++++++++++++++++++++++++++++++++++++----
 utils/v4l2-ctl/v4l2-ctl.cpp      |  7 ++++
 utils/v4l2-ctl/v4l2-ctl.h        |  5 +++
 3 files changed, 76 insertions(+), 7 deletions(-)

diff --git a/utils/v4l2-ctl/v4l2-ctl-meta.cpp b/utils/v4l2-ctl/v4l2-ctl-meta.cpp
index 37c91940a8..f4aa434937 100644
--- a/utils/v4l2-ctl/v4l2-ctl-meta.cpp
+++ b/utils/v4l2-ctl/v4l2-ctl-meta.cpp
@@ -21,14 +21,22 @@ static struct v4l2_format vfmt;	/* set_format/get_format */
 void meta_usage(void)
 {
 	printf("\nMetadata Formats options:\n"
-	       "  --list-formats-meta display supported metadata formats [VIDIOC_ENUM_FMT]\n"
-	       "  --get-fmt-meta      query the metadata format [VIDIOC_G_FMT]\n"
-	       "  --set-fmt-meta <f>  set the metadata format [VIDIOC_S_FMT]\n"
+	       "  --list-formats-meta display supported metadata capture formats [VIDIOC_ENUM_FMT]\n"
+	       "  --get-fmt-meta      query the metadata capture format [VIDIOC_G_FMT]\n"
+	       "  --set-fmt-meta <f>  set the metadata capture format [VIDIOC_S_FMT]\n"
 	       "                     parameter is either the format index as reported by\n"
 	       "                     --list-formats-meta, or the fourcc value as a string\n"
-	       "  --try-fmt-meta <f>  try the metadata format [VIDIOC_TRY_FMT]\n"
+	       "  --try-fmt-meta <f>  try the metadata capture format [VIDIOC_TRY_FMT]\n"
 	       "                     parameter is either the format index as reported by\n"
 	       "                     --list-formats-meta, or the fourcc value as a string\n"
+	       "  --list-formats-meta-out display supported metadata output formats [VIDIOC_ENUM_FMT]\n"
+	       "  --get-fmt-meta-out      query the metadata output format [VIDIOC_G_FMT]\n"
+	       "  --set-fmt-meta-out <f>  set the metadata output format [VIDIOC_S_FMT]\n"
+	       "                          parameter is either the format index as reported by\n"
+	       "                          --list-formats-meta-out, or the fourcc value as a string\n"
+	       "  --try-fmt-meta-out <f>  try the metadata output format [VIDIOC_TRY_FMT]\n"
+	       "                          parameter is either the format index as reported by\n"
+	       "                          --list-formats-meta-out, or the fourcc value as a string\n"
 	       );
 }
 
@@ -37,6 +45,8 @@ void meta_cmd(int ch, char *optarg)
 	switch (ch) {
 	case OptSetMetaFormat:
 	case OptTryMetaFormat:
+	case OptSetMetaOutFormat:
+	case OptTryMetaOutFormat:
 		if (strlen(optarg) == 0) {
 			meta_usage();
 			exit(1);
@@ -55,8 +65,38 @@ void meta_set(cv4l_fd &_fd)
 	int fd = _fd.g_fd();
 	int ret;
 
+	if (!v4l_type_is_meta(_fd.g_type()))
+		return;
+
 	if ((options[OptSetMetaFormat] || options[OptTryMetaFormat]) &&
-	    v4l_type_is_meta(_fd.g_type())) {
+	    v4l_type_is_capture(_fd.g_type())) {
+		struct v4l2_format in_vfmt;
+
+		in_vfmt.type = _fd.g_type();
+		in_vfmt.fmt.meta.dataformat = vfmt.fmt.meta.dataformat;
+
+		if (in_vfmt.fmt.meta.dataformat < 256) {
+			struct v4l2_fmtdesc fmt;
+
+			fmt.index = in_vfmt.fmt.meta.dataformat;
+			fmt.type = in_vfmt.type;
+
+			if (doioctl(fd, VIDIOC_ENUM_FMT, &fmt))
+				fmt.pixelformat = 0;
+
+			in_vfmt.fmt.meta.dataformat = fmt.pixelformat;
+		}
+
+		if (options[OptSetMetaFormat])
+			ret = doioctl(fd, VIDIOC_S_FMT, &in_vfmt);
+		else
+			ret = doioctl(fd, VIDIOC_TRY_FMT, &in_vfmt);
+		if (ret == 0 && (verbose || options[OptTryMetaFormat]))
+			printfmt(fd, in_vfmt);
+	}
+
+	if ((options[OptSetMetaOutFormat] || options[OptTryMetaOutFormat]) &&
+	    v4l_type_is_output(_fd.g_type())) {
 		struct v4l2_format in_vfmt;
 
 		in_vfmt.type = _fd.g_type();
@@ -85,7 +125,16 @@ void meta_set(cv4l_fd &_fd)
 
 void meta_get(cv4l_fd &fd)
 {
-	if (options[OptGetMetaFormat] && v4l_type_is_meta(fd.g_type())) {
+	if (!v4l_type_is_meta(fd.g_type()))
+		return;
+
+	if (options[OptGetMetaFormat] && v4l_type_is_capture(fd.g_type())) {
+		vfmt.type = fd.g_type();
+		if (doioctl(fd.g_fd(), VIDIOC_G_FMT, &vfmt) == 0)
+			printfmt(fd.g_fd(), vfmt);
+	}
+
+	if (options[OptGetMetaOutFormat] && v4l_type_is_output(fd.g_type())) {
 		vfmt.type = fd.g_type();
 		if (doioctl(fd.g_fd(), VIDIOC_G_FMT, &vfmt) == 0)
 			printfmt(fd.g_fd(), vfmt);
@@ -94,7 +143,15 @@ void meta_get(cv4l_fd &fd)
 
 void meta_list(cv4l_fd &fd)
 {
-	if (options[OptListMetaFormats] && v4l_type_is_meta(fd.g_type())) {
+	if (!v4l_type_is_meta(fd.g_type()))
+		return;
+
+	if (options[OptListMetaFormats] && v4l_type_is_capture(fd.g_type())) {
+		printf("ioctl: VIDIOC_ENUM_FMT\n");
+		print_video_formats(fd, fd.g_type());
+	}
+
+	if (options[OptListMetaOutFormats] && v4l_type_is_output(fd.g_type())) {
 		printf("ioctl: VIDIOC_ENUM_FMT\n");
 		print_video_formats(fd, fd.g_type());
 	}
diff --git a/utils/v4l2-ctl/v4l2-ctl.cpp b/utils/v4l2-ctl/v4l2-ctl.cpp
index 1783979d76..ffac8716d0 100644
--- a/utils/v4l2-ctl/v4l2-ctl.cpp
+++ b/utils/v4l2-ctl/v4l2-ctl.cpp
@@ -122,6 +122,7 @@ static struct option long_options[] = {
 	{"list-formats-out", no_argument, 0, OptListOutFormats},
 	{"list-formats-out-ext", no_argument, 0, OptListOutFormatsExt},
 	{"list-formats-meta", no_argument, 0, OptListMetaFormats},
+	{"list-formats-meta-out", no_argument, 0, OptListMetaOutFormats},
 	{"list-subdev-mbus-codes", optional_argument, 0, OptListSubDevMBusCodes},
 	{"list-subdev-framesizes", required_argument, 0, OptListSubDevFrameSizes},
 	{"list-subdev-frameintervals", required_argument, 0, OptListSubDevFrameIntervals},
@@ -174,6 +175,9 @@ static struct option long_options[] = {
 	{"get-fmt-meta", no_argument, 0, OptGetMetaFormat},
 	{"set-fmt-meta", required_argument, 0, OptSetMetaFormat},
 	{"try-fmt-meta", required_argument, 0, OptTryMetaFormat},
+	{"get-fmt-meta-out", no_argument, 0, OptGetMetaOutFormat},
+	{"set-fmt-meta-out", required_argument, 0, OptSetMetaOutFormat},
+	{"try-fmt-meta-out", required_argument, 0, OptTryMetaOutFormat},
 	{"get-subdev-fmt", optional_argument, 0, OptGetSubDevFormat},
 	{"set-subdev-fmt", required_argument, 0, OptSetSubDevFormat},
 	{"try-subdev-fmt", required_argument, 0, OptTrySubDevFormat},
@@ -238,6 +242,7 @@ static struct option long_options[] = {
 	{"list-buffers-sdr", no_argument, 0, OptListBuffersSdr},
 	{"list-buffers-sdr-out", no_argument, 0, OptListBuffersSdrOut},
 	{"list-buffers-meta", no_argument, 0, OptListBuffersMeta},
+	{"list-buffers-meta-out", no_argument, 0, OptListBuffersMetaOut},
 	{"stream-count", required_argument, 0, OptStreamCount},
 	{"stream-skip", required_argument, 0, OptStreamSkip},
 	{"stream-loop", no_argument, 0, OptStreamLoop},
@@ -507,6 +512,7 @@ void printfmt(int fd, const struct v4l2_format &vfmt)
 		printf("\tBuffer Size     : %u\n", vfmt.fmt.sdr.buffersize);
 		break;
 	case V4L2_BUF_TYPE_META_CAPTURE:
+	case V4L2_BUF_TYPE_META_OUTPUT:
 		printf("\tSample Format   : '%s'%s\n", fcc2s(vfmt.fmt.meta.dataformat).c_str(),
 		       printfmtname(fd, vfmt.type, vfmt.fmt.meta.dataformat).c_str());
 		printf("\tBuffer Size     : %u\n", vfmt.fmt.meta.buffersize);
@@ -1247,6 +1253,7 @@ int main(int argc, char **argv)
 		options[OptGetSdrFormat] = 1;
 		options[OptGetSdrOutFormat] = 1;
 		options[OptGetMetaFormat] = 1;
+		options[OptGetMetaOutFormat] = 1;
 		options[OptGetFBuf] = 1;
 		options[OptGetCropCap] = 1;
 		options[OptGetOutputCropCap] = 1;
diff --git a/utils/v4l2-ctl/v4l2-ctl.h b/utils/v4l2-ctl/v4l2-ctl.h
index 5a52a0a48f..fc51cd1b97 100644
--- a/utils/v4l2-ctl/v4l2-ctl.h
+++ b/utils/v4l2-ctl/v4l2-ctl.h
@@ -89,6 +89,7 @@ enum Option {
 	OptGetSdrFormat,
 	OptGetSdrOutFormat,
 	OptGetMetaFormat,
+	OptGetMetaOutFormat,
 	OptGetSubDevFormat,
 	OptSetSlicedVbiOutFormat,
 	OptSetOverlayFormat,
@@ -97,6 +98,7 @@ enum Option {
 	OptSetSdrFormat,
 	OptSetSdrOutFormat,
 	OptSetMetaFormat,
+	OptSetMetaOutFormat,
 	OptSetSubDevFormat,
 	OptTryVideoOutFormat,
 	OptTrySlicedVbiOutFormat,
@@ -108,6 +110,7 @@ enum Option {
 	OptTrySdrFormat,
 	OptTrySdrOutFormat,
 	OptTryMetaFormat,
+	OptTryMetaOutFormat,
 	OptTrySubDevFormat,
 	OptAll,
 	OptListStandards,
@@ -122,6 +125,7 @@ enum Option {
 	OptListOutFormats,
 	OptListOutFormatsExt,
 	OptListMetaFormats,
+	OptListMetaOutFormats,
 	OptListSubDevMBusCodes,
 	OptListSubDevFrameSizes,
 	OptListSubDevFrameIntervals,
@@ -205,6 +209,7 @@ enum Option {
 	OptListBuffersSdr,
 	OptListBuffersSdrOut,
 	OptListBuffersMeta,
+	OptListBuffersMetaOut,
 	OptStreamCount,
 	OptStreamSkip,
 	OptStreamLoop,
-- 
2.11.0


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [yavta PATCH v2 1/1] v4l2-ctl: Add support for META_OUTPUT buffer type
  2019-01-22 12:05 [yavta PATCH v2 1/1] v4l2-ctl: Add support for META_OUTPUT buffer type Sakari Ailus
@ 2019-01-24  8:44 ` Hans Verkuil
  2019-01-24 10:25   ` Sakari Ailus
  0 siblings, 1 reply; 3+ messages in thread
From: Hans Verkuil @ 2019-01-24  8:44 UTC (permalink / raw)
  To: Sakari Ailus, linux-media; +Cc: rajmohan.mani, yong.zhi

Note: the subject it wrong, it's v4l-utils, not yavta.

On 1/22/19 1:05 PM, Sakari Ailus wrote:
> Add support for META_OUTPUT buffer type to v4l2-ctl.
> 
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> ---
> Hi Hans,
> 
> Here's an update for the meta output buffer type in v4l2-ctl.
> 
> Since v1:
> 
> - Merge help text for meta and meta out buffer types
> 
> - Unify implementation for meta format handling
> 
>  utils/v4l2-ctl/v4l2-ctl-meta.cpp | 71 ++++++++++++++++++++++++++++++++++++----
>  utils/v4l2-ctl/v4l2-ctl.cpp      |  7 ++++
>  utils/v4l2-ctl/v4l2-ctl.h        |  5 +++
>  3 files changed, 76 insertions(+), 7 deletions(-)
> 
> diff --git a/utils/v4l2-ctl/v4l2-ctl-meta.cpp b/utils/v4l2-ctl/v4l2-ctl-meta.cpp
> index 37c91940a8..f4aa434937 100644
> --- a/utils/v4l2-ctl/v4l2-ctl-meta.cpp
> +++ b/utils/v4l2-ctl/v4l2-ctl-meta.cpp
> @@ -21,14 +21,22 @@ static struct v4l2_format vfmt;	/* set_format/get_format */
>  void meta_usage(void)
>  {
>  	printf("\nMetadata Formats options:\n"
> -	       "  --list-formats-meta display supported metadata formats [VIDIOC_ENUM_FMT]\n"
> -	       "  --get-fmt-meta      query the metadata format [VIDIOC_G_FMT]\n"
> -	       "  --set-fmt-meta <f>  set the metadata format [VIDIOC_S_FMT]\n"
> +	       "  --list-formats-meta display supported metadata capture formats [VIDIOC_ENUM_FMT]\n"
> +	       "  --get-fmt-meta      query the metadata capture format [VIDIOC_G_FMT]\n"
> +	       "  --set-fmt-meta <f>  set the metadata capture format [VIDIOC_S_FMT]\n"
>  	       "                     parameter is either the format index as reported by\n"
>  	       "                     --list-formats-meta, or the fourcc value as a string\n"
> -	       "  --try-fmt-meta <f>  try the metadata format [VIDIOC_TRY_FMT]\n"
> +	       "  --try-fmt-meta <f>  try the metadata capture format [VIDIOC_TRY_FMT]\n"
>  	       "                     parameter is either the format index as reported by\n"
>  	       "                     --list-formats-meta, or the fourcc value as a string\n"
> +	       "  --list-formats-meta-out display supported metadata output formats [VIDIOC_ENUM_FMT]\n"
> +	       "  --get-fmt-meta-out      query the metadata output format [VIDIOC_G_FMT]\n"
> +	       "  --set-fmt-meta-out <f>  set the metadata output format [VIDIOC_S_FMT]\n"
> +	       "                          parameter is either the format index as reported by\n"
> +	       "                          --list-formats-meta-out, or the fourcc value as a string\n"
> +	       "  --try-fmt-meta-out <f>  try the metadata output format [VIDIOC_TRY_FMT]\n"
> +	       "                          parameter is either the format index as reported by\n"
> +	       "                          --list-formats-meta-out, or the fourcc value as a string\n"
>  	       );
>  }
>  
> @@ -37,6 +45,8 @@ void meta_cmd(int ch, char *optarg)
>  	switch (ch) {
>  	case OptSetMetaFormat:
>  	case OptTryMetaFormat:
> +	case OptSetMetaOutFormat:
> +	case OptTryMetaOutFormat:
>  		if (strlen(optarg) == 0) {
>  			meta_usage();
>  			exit(1);
> @@ -55,8 +65,38 @@ void meta_set(cv4l_fd &_fd)
>  	int fd = _fd.g_fd();
>  	int ret;
>  
> +	if (!v4l_type_is_meta(_fd.g_type()))
> +		return;
> +
>  	if ((options[OptSetMetaFormat] || options[OptTryMetaFormat]) &&
> -	    v4l_type_is_meta(_fd.g_type())) {
> +	    v4l_type_is_capture(_fd.g_type())) {
> +		struct v4l2_format in_vfmt;
> +
> +		in_vfmt.type = _fd.g_type();
> +		in_vfmt.fmt.meta.dataformat = vfmt.fmt.meta.dataformat;
> +
> +		if (in_vfmt.fmt.meta.dataformat < 256) {
> +			struct v4l2_fmtdesc fmt;
> +
> +			fmt.index = in_vfmt.fmt.meta.dataformat;
> +			fmt.type = in_vfmt.type;
> +
> +			if (doioctl(fd, VIDIOC_ENUM_FMT, &fmt))
> +				fmt.pixelformat = 0;
> +
> +			in_vfmt.fmt.meta.dataformat = fmt.pixelformat;
> +		}
> +
> +		if (options[OptSetMetaFormat])
> +			ret = doioctl(fd, VIDIOC_S_FMT, &in_vfmt);
> +		else
> +			ret = doioctl(fd, VIDIOC_TRY_FMT, &in_vfmt);
> +		if (ret == 0 && (verbose || options[OptTryMetaFormat]))
> +			printfmt(fd, in_vfmt);

This is exactly the same code for meta output. Why not just do:

  	if ((options[OptSetMetaFormat] || options[OptTryMetaFormat] ||
	     options[OptSetMetaOutFormat] || options[OptTryMetaOutFormat]) &&
	    v4l_type_is_meta(_fd.g_type())

You can add tests for capture/output if you like, but it is not really necessary.


> +	}
> +
> +	if ((options[OptSetMetaOutFormat] || options[OptTryMetaOutFormat]) &&
> +	    v4l_type_is_output(_fd.g_type())) {
>  		struct v4l2_format in_vfmt;
>  
>  		in_vfmt.type = _fd.g_type();
> @@ -85,7 +125,16 @@ void meta_set(cv4l_fd &_fd)
>  
>  void meta_get(cv4l_fd &fd)
>  {
> -	if (options[OptGetMetaFormat] && v4l_type_is_meta(fd.g_type())) {
> +	if (!v4l_type_is_meta(fd.g_type()))
> +		return;
> +
> +	if (options[OptGetMetaFormat] && v4l_type_is_capture(fd.g_type())) {
> +		vfmt.type = fd.g_type();
> +		if (doioctl(fd.g_fd(), VIDIOC_G_FMT, &vfmt) == 0)
> +			printfmt(fd.g_fd(), vfmt);
> +	}
> +
> +	if (options[OptGetMetaOutFormat] && v4l_type_is_output(fd.g_type())) {
>  		vfmt.type = fd.g_type();
>  		if (doioctl(fd.g_fd(), VIDIOC_G_FMT, &vfmt) == 0)
>  			printfmt(fd.g_fd(), vfmt);
> @@ -94,7 +143,15 @@ void meta_get(cv4l_fd &fd)
>  
>  void meta_list(cv4l_fd &fd)
>  {
> -	if (options[OptListMetaFormats] && v4l_type_is_meta(fd.g_type())) {
> +	if (!v4l_type_is_meta(fd.g_type()))
> +		return;
> +
> +	if (options[OptListMetaFormats] && v4l_type_is_capture(fd.g_type())) {
> +		printf("ioctl: VIDIOC_ENUM_FMT\n");
> +		print_video_formats(fd, fd.g_type());
> +	}
> +
> +	if (options[OptListMetaOutFormats] && v4l_type_is_output(fd.g_type())) {
>  		printf("ioctl: VIDIOC_ENUM_FMT\n");
>  		print_video_formats(fd, fd.g_type());
>  	}

Same for these two functions: the same code works for both meta capture and output.

To be honest, I would really like to completely rework the way v4l2-ctl handles this.
There are way too many options to list, get, set, try formats. But you really don't
need to specify the type (video, meta, sdr, vbi, etc) as that can be determined
automatically.

But let's get this done first, and then I'll take another look at this.

Regards,

	Hans


> diff --git a/utils/v4l2-ctl/v4l2-ctl.cpp b/utils/v4l2-ctl/v4l2-ctl.cpp
> index 1783979d76..ffac8716d0 100644
> --- a/utils/v4l2-ctl/v4l2-ctl.cpp
> +++ b/utils/v4l2-ctl/v4l2-ctl.cpp
> @@ -122,6 +122,7 @@ static struct option long_options[] = {
>  	{"list-formats-out", no_argument, 0, OptListOutFormats},
>  	{"list-formats-out-ext", no_argument, 0, OptListOutFormatsExt},
>  	{"list-formats-meta", no_argument, 0, OptListMetaFormats},
> +	{"list-formats-meta-out", no_argument, 0, OptListMetaOutFormats},
>  	{"list-subdev-mbus-codes", optional_argument, 0, OptListSubDevMBusCodes},
>  	{"list-subdev-framesizes", required_argument, 0, OptListSubDevFrameSizes},
>  	{"list-subdev-frameintervals", required_argument, 0, OptListSubDevFrameIntervals},
> @@ -174,6 +175,9 @@ static struct option long_options[] = {
>  	{"get-fmt-meta", no_argument, 0, OptGetMetaFormat},
>  	{"set-fmt-meta", required_argument, 0, OptSetMetaFormat},
>  	{"try-fmt-meta", required_argument, 0, OptTryMetaFormat},
> +	{"get-fmt-meta-out", no_argument, 0, OptGetMetaOutFormat},
> +	{"set-fmt-meta-out", required_argument, 0, OptSetMetaOutFormat},
> +	{"try-fmt-meta-out", required_argument, 0, OptTryMetaOutFormat},
>  	{"get-subdev-fmt", optional_argument, 0, OptGetSubDevFormat},
>  	{"set-subdev-fmt", required_argument, 0, OptSetSubDevFormat},
>  	{"try-subdev-fmt", required_argument, 0, OptTrySubDevFormat},
> @@ -238,6 +242,7 @@ static struct option long_options[] = {
>  	{"list-buffers-sdr", no_argument, 0, OptListBuffersSdr},
>  	{"list-buffers-sdr-out", no_argument, 0, OptListBuffersSdrOut},
>  	{"list-buffers-meta", no_argument, 0, OptListBuffersMeta},
> +	{"list-buffers-meta-out", no_argument, 0, OptListBuffersMetaOut},
>  	{"stream-count", required_argument, 0, OptStreamCount},
>  	{"stream-skip", required_argument, 0, OptStreamSkip},
>  	{"stream-loop", no_argument, 0, OptStreamLoop},
> @@ -507,6 +512,7 @@ void printfmt(int fd, const struct v4l2_format &vfmt)
>  		printf("\tBuffer Size     : %u\n", vfmt.fmt.sdr.buffersize);
>  		break;
>  	case V4L2_BUF_TYPE_META_CAPTURE:
> +	case V4L2_BUF_TYPE_META_OUTPUT:
>  		printf("\tSample Format   : '%s'%s\n", fcc2s(vfmt.fmt.meta.dataformat).c_str(),
>  		       printfmtname(fd, vfmt.type, vfmt.fmt.meta.dataformat).c_str());
>  		printf("\tBuffer Size     : %u\n", vfmt.fmt.meta.buffersize);
> @@ -1247,6 +1253,7 @@ int main(int argc, char **argv)
>  		options[OptGetSdrFormat] = 1;
>  		options[OptGetSdrOutFormat] = 1;
>  		options[OptGetMetaFormat] = 1;
> +		options[OptGetMetaOutFormat] = 1;
>  		options[OptGetFBuf] = 1;
>  		options[OptGetCropCap] = 1;
>  		options[OptGetOutputCropCap] = 1;
> diff --git a/utils/v4l2-ctl/v4l2-ctl.h b/utils/v4l2-ctl/v4l2-ctl.h
> index 5a52a0a48f..fc51cd1b97 100644
> --- a/utils/v4l2-ctl/v4l2-ctl.h
> +++ b/utils/v4l2-ctl/v4l2-ctl.h
> @@ -89,6 +89,7 @@ enum Option {
>  	OptGetSdrFormat,
>  	OptGetSdrOutFormat,
>  	OptGetMetaFormat,
> +	OptGetMetaOutFormat,
>  	OptGetSubDevFormat,
>  	OptSetSlicedVbiOutFormat,
>  	OptSetOverlayFormat,
> @@ -97,6 +98,7 @@ enum Option {
>  	OptSetSdrFormat,
>  	OptSetSdrOutFormat,
>  	OptSetMetaFormat,
> +	OptSetMetaOutFormat,
>  	OptSetSubDevFormat,
>  	OptTryVideoOutFormat,
>  	OptTrySlicedVbiOutFormat,
> @@ -108,6 +110,7 @@ enum Option {
>  	OptTrySdrFormat,
>  	OptTrySdrOutFormat,
>  	OptTryMetaFormat,
> +	OptTryMetaOutFormat,
>  	OptTrySubDevFormat,
>  	OptAll,
>  	OptListStandards,
> @@ -122,6 +125,7 @@ enum Option {
>  	OptListOutFormats,
>  	OptListOutFormatsExt,
>  	OptListMetaFormats,
> +	OptListMetaOutFormats,
>  	OptListSubDevMBusCodes,
>  	OptListSubDevFrameSizes,
>  	OptListSubDevFrameIntervals,
> @@ -205,6 +209,7 @@ enum Option {
>  	OptListBuffersSdr,
>  	OptListBuffersSdrOut,
>  	OptListBuffersMeta,
> +	OptListBuffersMetaOut,
>  	OptStreamCount,
>  	OptStreamSkip,
>  	OptStreamLoop,
> 


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [yavta PATCH v2 1/1] v4l2-ctl: Add support for META_OUTPUT buffer type
  2019-01-24  8:44 ` Hans Verkuil
@ 2019-01-24 10:25   ` Sakari Ailus
  0 siblings, 0 replies; 3+ messages in thread
From: Sakari Ailus @ 2019-01-24 10:25 UTC (permalink / raw)
  To: Hans Verkuil; +Cc: linux-media, rajmohan.mani, yong.zhi

Hi Hans,

On Thu, Jan 24, 2019 at 09:44:13AM +0100, Hans Verkuil wrote:
> Note: the subject it wrong, it's v4l-utils, not yavta.

Oops, my bad. I must have been thinking of yavta when writing that. :-)

And thanks for the review.

> 
> On 1/22/19 1:05 PM, Sakari Ailus wrote:
> > Add support for META_OUTPUT buffer type to v4l2-ctl.
> > 
> > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> > ---
> > Hi Hans,
> > 
> > Here's an update for the meta output buffer type in v4l2-ctl.
> > 
> > Since v1:
> > 
> > - Merge help text for meta and meta out buffer types
> > 
> > - Unify implementation for meta format handling
> > 
> >  utils/v4l2-ctl/v4l2-ctl-meta.cpp | 71 ++++++++++++++++++++++++++++++++++++----
> >  utils/v4l2-ctl/v4l2-ctl.cpp      |  7 ++++
> >  utils/v4l2-ctl/v4l2-ctl.h        |  5 +++
> >  3 files changed, 76 insertions(+), 7 deletions(-)
> > 
> > diff --git a/utils/v4l2-ctl/v4l2-ctl-meta.cpp b/utils/v4l2-ctl/v4l2-ctl-meta.cpp
> > index 37c91940a8..f4aa434937 100644
> > --- a/utils/v4l2-ctl/v4l2-ctl-meta.cpp
> > +++ b/utils/v4l2-ctl/v4l2-ctl-meta.cpp
> > @@ -21,14 +21,22 @@ static struct v4l2_format vfmt;	/* set_format/get_format */
> >  void meta_usage(void)
> >  {
> >  	printf("\nMetadata Formats options:\n"
> > -	       "  --list-formats-meta display supported metadata formats [VIDIOC_ENUM_FMT]\n"
> > -	       "  --get-fmt-meta      query the metadata format [VIDIOC_G_FMT]\n"
> > -	       "  --set-fmt-meta <f>  set the metadata format [VIDIOC_S_FMT]\n"
> > +	       "  --list-formats-meta display supported metadata capture formats [VIDIOC_ENUM_FMT]\n"
> > +	       "  --get-fmt-meta      query the metadata capture format [VIDIOC_G_FMT]\n"
> > +	       "  --set-fmt-meta <f>  set the metadata capture format [VIDIOC_S_FMT]\n"
> >  	       "                     parameter is either the format index as reported by\n"
> >  	       "                     --list-formats-meta, or the fourcc value as a string\n"
> > -	       "  --try-fmt-meta <f>  try the metadata format [VIDIOC_TRY_FMT]\n"
> > +	       "  --try-fmt-meta <f>  try the metadata capture format [VIDIOC_TRY_FMT]\n"
> >  	       "                     parameter is either the format index as reported by\n"
> >  	       "                     --list-formats-meta, or the fourcc value as a string\n"
> > +	       "  --list-formats-meta-out display supported metadata output formats [VIDIOC_ENUM_FMT]\n"
> > +	       "  --get-fmt-meta-out      query the metadata output format [VIDIOC_G_FMT]\n"
> > +	       "  --set-fmt-meta-out <f>  set the metadata output format [VIDIOC_S_FMT]\n"
> > +	       "                          parameter is either the format index as reported by\n"
> > +	       "                          --list-formats-meta-out, or the fourcc value as a string\n"
> > +	       "  --try-fmt-meta-out <f>  try the metadata output format [VIDIOC_TRY_FMT]\n"
> > +	       "                          parameter is either the format index as reported by\n"
> > +	       "                          --list-formats-meta-out, or the fourcc value as a string\n"
> >  	       );
> >  }
> >  
> > @@ -37,6 +45,8 @@ void meta_cmd(int ch, char *optarg)
> >  	switch (ch) {
> >  	case OptSetMetaFormat:
> >  	case OptTryMetaFormat:
> > +	case OptSetMetaOutFormat:
> > +	case OptTryMetaOutFormat:
> >  		if (strlen(optarg) == 0) {
> >  			meta_usage();
> >  			exit(1);
> > @@ -55,8 +65,38 @@ void meta_set(cv4l_fd &_fd)
> >  	int fd = _fd.g_fd();
> >  	int ret;
> >  
> > +	if (!v4l_type_is_meta(_fd.g_type()))
> > +		return;
> > +
> >  	if ((options[OptSetMetaFormat] || options[OptTryMetaFormat]) &&
> > -	    v4l_type_is_meta(_fd.g_type())) {
> > +	    v4l_type_is_capture(_fd.g_type())) {
> > +		struct v4l2_format in_vfmt;
> > +
> > +		in_vfmt.type = _fd.g_type();
> > +		in_vfmt.fmt.meta.dataformat = vfmt.fmt.meta.dataformat;
> > +
> > +		if (in_vfmt.fmt.meta.dataformat < 256) {
> > +			struct v4l2_fmtdesc fmt;
> > +
> > +			fmt.index = in_vfmt.fmt.meta.dataformat;
> > +			fmt.type = in_vfmt.type;
> > +
> > +			if (doioctl(fd, VIDIOC_ENUM_FMT, &fmt))
> > +				fmt.pixelformat = 0;
> > +
> > +			in_vfmt.fmt.meta.dataformat = fmt.pixelformat;
> > +		}
> > +
> > +		if (options[OptSetMetaFormat])
> > +			ret = doioctl(fd, VIDIOC_S_FMT, &in_vfmt);
> > +		else
> > +			ret = doioctl(fd, VIDIOC_TRY_FMT, &in_vfmt);
> > +		if (ret == 0 && (verbose || options[OptTryMetaFormat]))
> > +			printfmt(fd, in_vfmt);
> 
> This is exactly the same code for meta output. Why not just do:
> 
>   	if ((options[OptSetMetaFormat] || options[OptTryMetaFormat] ||
> 	     options[OptSetMetaOutFormat] || options[OptTryMetaOutFormat]) &&
> 	    v4l_type_is_meta(_fd.g_type())
> 
> You can add tests for capture/output if you like, but it is not really necessary.

Ack, I'll fix that.

> 
> 
> > +	}
> > +
> > +	if ((options[OptSetMetaOutFormat] || options[OptTryMetaOutFormat]) &&
> > +	    v4l_type_is_output(_fd.g_type())) {
> >  		struct v4l2_format in_vfmt;
> >  
> >  		in_vfmt.type = _fd.g_type();
> > @@ -85,7 +125,16 @@ void meta_set(cv4l_fd &_fd)
> >  
> >  void meta_get(cv4l_fd &fd)
> >  {
> > -	if (options[OptGetMetaFormat] && v4l_type_is_meta(fd.g_type())) {
> > +	if (!v4l_type_is_meta(fd.g_type()))
> > +		return;
> > +
> > +	if (options[OptGetMetaFormat] && v4l_type_is_capture(fd.g_type())) {
> > +		vfmt.type = fd.g_type();
> > +		if (doioctl(fd.g_fd(), VIDIOC_G_FMT, &vfmt) == 0)
> > +			printfmt(fd.g_fd(), vfmt);
> > +	}
> > +
> > +	if (options[OptGetMetaOutFormat] && v4l_type_is_output(fd.g_type())) {
> >  		vfmt.type = fd.g_type();
> >  		if (doioctl(fd.g_fd(), VIDIOC_G_FMT, &vfmt) == 0)
> >  			printfmt(fd.g_fd(), vfmt);
> > @@ -94,7 +143,15 @@ void meta_get(cv4l_fd &fd)
> >  
> >  void meta_list(cv4l_fd &fd)
> >  {
> > -	if (options[OptListMetaFormats] && v4l_type_is_meta(fd.g_type())) {
> > +	if (!v4l_type_is_meta(fd.g_type()))
> > +		return;
> > +
> > +	if (options[OptListMetaFormats] && v4l_type_is_capture(fd.g_type())) {
> > +		printf("ioctl: VIDIOC_ENUM_FMT\n");
> > +		print_video_formats(fd, fd.g_type());
> > +	}
> > +
> > +	if (options[OptListMetaOutFormats] && v4l_type_is_output(fd.g_type())) {
> >  		printf("ioctl: VIDIOC_ENUM_FMT\n");
> >  		print_video_formats(fd, fd.g_type());
> >  	}
> 
> Same for these two functions: the same code works for both meta capture and output.
> 
> To be honest, I would really like to completely rework the way v4l2-ctl handles this.
> There are way too many options to list, get, set, try formats. But you really don't
> need to specify the type (video, meta, sdr, vbi, etc) as that can be determined
> automatically.

As long as a video node supports a single type only. Aren't there cases
that need multiple types? Vbi?

> 
> But let's get this done first, and then I'll take another look at this.
> 
> Regards,
> 
> 	Hans
> 
> 
> > diff --git a/utils/v4l2-ctl/v4l2-ctl.cpp b/utils/v4l2-ctl/v4l2-ctl.cpp
> > index 1783979d76..ffac8716d0 100644
> > --- a/utils/v4l2-ctl/v4l2-ctl.cpp
> > +++ b/utils/v4l2-ctl/v4l2-ctl.cpp
> > @@ -122,6 +122,7 @@ static struct option long_options[] = {
> >  	{"list-formats-out", no_argument, 0, OptListOutFormats},
> >  	{"list-formats-out-ext", no_argument, 0, OptListOutFormatsExt},
> >  	{"list-formats-meta", no_argument, 0, OptListMetaFormats},
> > +	{"list-formats-meta-out", no_argument, 0, OptListMetaOutFormats},
> >  	{"list-subdev-mbus-codes", optional_argument, 0, OptListSubDevMBusCodes},
> >  	{"list-subdev-framesizes", required_argument, 0, OptListSubDevFrameSizes},
> >  	{"list-subdev-frameintervals", required_argument, 0, OptListSubDevFrameIntervals},
> > @@ -174,6 +175,9 @@ static struct option long_options[] = {
> >  	{"get-fmt-meta", no_argument, 0, OptGetMetaFormat},
> >  	{"set-fmt-meta", required_argument, 0, OptSetMetaFormat},
> >  	{"try-fmt-meta", required_argument, 0, OptTryMetaFormat},
> > +	{"get-fmt-meta-out", no_argument, 0, OptGetMetaOutFormat},
> > +	{"set-fmt-meta-out", required_argument, 0, OptSetMetaOutFormat},
> > +	{"try-fmt-meta-out", required_argument, 0, OptTryMetaOutFormat},
> >  	{"get-subdev-fmt", optional_argument, 0, OptGetSubDevFormat},
> >  	{"set-subdev-fmt", required_argument, 0, OptSetSubDevFormat},
> >  	{"try-subdev-fmt", required_argument, 0, OptTrySubDevFormat},
> > @@ -238,6 +242,7 @@ static struct option long_options[] = {
> >  	{"list-buffers-sdr", no_argument, 0, OptListBuffersSdr},
> >  	{"list-buffers-sdr-out", no_argument, 0, OptListBuffersSdrOut},
> >  	{"list-buffers-meta", no_argument, 0, OptListBuffersMeta},
> > +	{"list-buffers-meta-out", no_argument, 0, OptListBuffersMetaOut},
> >  	{"stream-count", required_argument, 0, OptStreamCount},
> >  	{"stream-skip", required_argument, 0, OptStreamSkip},
> >  	{"stream-loop", no_argument, 0, OptStreamLoop},
> > @@ -507,6 +512,7 @@ void printfmt(int fd, const struct v4l2_format &vfmt)
> >  		printf("\tBuffer Size     : %u\n", vfmt.fmt.sdr.buffersize);
> >  		break;
> >  	case V4L2_BUF_TYPE_META_CAPTURE:
> > +	case V4L2_BUF_TYPE_META_OUTPUT:
> >  		printf("\tSample Format   : '%s'%s\n", fcc2s(vfmt.fmt.meta.dataformat).c_str(),
> >  		       printfmtname(fd, vfmt.type, vfmt.fmt.meta.dataformat).c_str());
> >  		printf("\tBuffer Size     : %u\n", vfmt.fmt.meta.buffersize);
> > @@ -1247,6 +1253,7 @@ int main(int argc, char **argv)
> >  		options[OptGetSdrFormat] = 1;
> >  		options[OptGetSdrOutFormat] = 1;
> >  		options[OptGetMetaFormat] = 1;
> > +		options[OptGetMetaOutFormat] = 1;
> >  		options[OptGetFBuf] = 1;
> >  		options[OptGetCropCap] = 1;
> >  		options[OptGetOutputCropCap] = 1;
> > diff --git a/utils/v4l2-ctl/v4l2-ctl.h b/utils/v4l2-ctl/v4l2-ctl.h
> > index 5a52a0a48f..fc51cd1b97 100644
> > --- a/utils/v4l2-ctl/v4l2-ctl.h
> > +++ b/utils/v4l2-ctl/v4l2-ctl.h
> > @@ -89,6 +89,7 @@ enum Option {
> >  	OptGetSdrFormat,
> >  	OptGetSdrOutFormat,
> >  	OptGetMetaFormat,
> > +	OptGetMetaOutFormat,
> >  	OptGetSubDevFormat,
> >  	OptSetSlicedVbiOutFormat,
> >  	OptSetOverlayFormat,
> > @@ -97,6 +98,7 @@ enum Option {
> >  	OptSetSdrFormat,
> >  	OptSetSdrOutFormat,
> >  	OptSetMetaFormat,
> > +	OptSetMetaOutFormat,
> >  	OptSetSubDevFormat,
> >  	OptTryVideoOutFormat,
> >  	OptTrySlicedVbiOutFormat,
> > @@ -108,6 +110,7 @@ enum Option {
> >  	OptTrySdrFormat,
> >  	OptTrySdrOutFormat,
> >  	OptTryMetaFormat,
> > +	OptTryMetaOutFormat,
> >  	OptTrySubDevFormat,
> >  	OptAll,
> >  	OptListStandards,
> > @@ -122,6 +125,7 @@ enum Option {
> >  	OptListOutFormats,
> >  	OptListOutFormatsExt,
> >  	OptListMetaFormats,
> > +	OptListMetaOutFormats,
> >  	OptListSubDevMBusCodes,
> >  	OptListSubDevFrameSizes,
> >  	OptListSubDevFrameIntervals,
> > @@ -205,6 +209,7 @@ enum Option {
> >  	OptListBuffersSdr,
> >  	OptListBuffersSdrOut,
> >  	OptListBuffersMeta,
> > +	OptListBuffersMetaOut,
> >  	OptStreamCount,
> >  	OptStreamSkip,
> >  	OptStreamLoop,
> > 
> 

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2019-01-24 10:25 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-22 12:05 [yavta PATCH v2 1/1] v4l2-ctl: Add support for META_OUTPUT buffer type Sakari Ailus
2019-01-24  8:44 ` Hans Verkuil
2019-01-24 10:25   ` Sakari Ailus

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).