v4l2-ctl: add function vidcap_get_and_update_fmt

Commit Message

Dafna Hirschfeld Jan. 30, 2019, 1 p.m. UTC

add a function vidcap_get_and_update_fmt to set
the format from cmd params. Use it in capture_setup.

Signed-off-by: Dafna Hirschfeld <[e-mail]>
---
 utils/v4l2-ctl/v4l2-ctl-streaming.cpp |  13 +++
 utils/v4l2-ctl/v4l2-ctl-vidcap.cpp    | 140 ++++++++++++++------------
 utils/v4l2-ctl/v4l2-ctl.h             |   1 +
 3 files changed, 91 insertions(+), 63 deletions(-)

Comments

Hans Verkuil Jan. 30, 2019, 1:30 p.m. UTC | #1

Hi Dafna,

One simple comment:

On 1/30/19 2:00 PM, Dafna Hirschfeld wrote:
> add a function vidcap_get_and_update_fmt to set
> the format from cmd params. Use it in capture_setup.
> 
> Signed-off-by: Dafna Hirschfeld <[e-mail]>
> ---
>  utils/v4l2-ctl/v4l2-ctl-streaming.cpp |  13 +++
>  utils/v4l2-ctl/v4l2-ctl-vidcap.cpp    | 140 ++++++++++++++------------
>  utils/v4l2-ctl/v4l2-ctl.h             |   1 +
>  3 files changed, 91 insertions(+), 63 deletions(-)
> 
> diff --git a/utils/v4l2-ctl/v4l2-ctl-streaming.cpp b/utils/v4l2-ctl/v4l2-ctl-streaming.cpp
> index d6c3f6a9..2f66e052 100644
> --- a/utils/v4l2-ctl/v4l2-ctl-streaming.cpp
> +++ b/utils/v4l2-ctl/v4l2-ctl-streaming.cpp
> @@ -1902,17 +1902,30 @@ static int capture_setup(cv4l_fd &fd, cv4l_queue &in, cv4l_fd *exp_fd)
>  		return -1;
>  	}
>  
> +	if (options[OptSetVideoFormat]) {
> +		cv4l_fmt fmt;
> +
> +		if (vidcap_get_and_update_fmt(fd, fmt)) {
> +			fprintf(stderr, "%s: vidcap_get_and_update_fmt error\n",
> +				__func__);
> +			return -1;
> +		}
> +		fd.s_fmt(fmt, in.g_type());
> +	}
> +
>  	if (in.reqbufs(&fd, reqbufs_count_cap)) {
>  		fprintf(stderr, "%s: in.reqbufs %u error\n", __func__,
>  			reqbufs_count_cap);
>  		return -1;
>  	}
> +
>  	if (exp_fd && in.export_bufs(exp_fd, exp_fd->g_type()))
>  		return -1;
>  	if (in.obtain_bufs(&fd) || in.queue_all(&fd)) {
>  		fprintf(stderr, "%s: in.obtain_bufs error\n", __func__);
>  		return -1;
>  	}
> +
>  	if (fd.streamon(in.g_type())) {
>  		fprintf(stderr, "%s: fd.streamon error\n", __func__);
>  		return -1;
> diff --git a/utils/v4l2-ctl/v4l2-ctl-vidcap.cpp b/utils/v4l2-ctl/v4l2-ctl-vidcap.cpp
> index dc17a868..df1203a9 100644
> --- a/utils/v4l2-ctl/v4l2-ctl-vidcap.cpp
> +++ b/utils/v4l2-ctl/v4l2-ctl-vidcap.cpp
> @@ -155,72 +155,12 @@ void vidcap_cmd(int ch, char *optarg)
>  
>  void vidcap_set(cv4l_fd &_fd)
>  {
> -	int fd = _fd.g_fd();
> -	int ret;
> -
>  	if (options[OptSetVideoFormat] || options[OptTryVideoFormat]) {
> +		int fd = _fd.g_fd();
> +		int ret;
>  		struct v4l2_format vfmt;
>  
> -		memset(&vfmt, 0, sizeof(vfmt));
> -		vfmt.fmt.pix.priv = priv_magic;
> -		vfmt.type = vidcap_buftype;
> -
> -		if (doioctl(fd, VIDIOC_G_FMT, &vfmt) == 0) {
> -			if (is_multiplanar) {
> -				if (set_fmts & FmtWidth)
> -					vfmt.fmt.pix_mp.width = width;
> -				if (set_fmts & FmtHeight)
> -					vfmt.fmt.pix_mp.height = height;
> -				if (set_fmts & FmtPixelFormat) {
> -					vfmt.fmt.pix_mp.pixelformat = pixfmt;
> -					if (vfmt.fmt.pix_mp.pixelformat < 256) {
> -						vfmt.fmt.pix_mp.pixelformat =
> -							find_pixel_format(fd, vfmt.fmt.pix_mp.pixelformat,
> -									false, true);
> -					}
> -				}
> -				if (set_fmts & FmtField)
> -					vfmt.fmt.pix_mp.field = field;
> -				if (set_fmts & FmtFlags)
> -					vfmt.fmt.pix_mp.flags = flags;
> -				if (set_fmts & FmtBytesPerLine) {
> -					for (unsigned i = 0; i < VIDEO_MAX_PLANES; i++)
> -						vfmt.fmt.pix_mp.plane_fmt[i].bytesperline =
> -							bytesperline[i];
> -				} else {
> -					/* G_FMT might return bytesperline values > width,
> -					 * reset them to 0 to force the driver to update them
> -					 * to the closest value for the new width. */
> -					for (unsigned i = 0; i < vfmt.fmt.pix_mp.num_planes; i++)
> -						vfmt.fmt.pix_mp.plane_fmt[i].bytesperline = 0;
> -				}
> -			} else {
> -				if (set_fmts & FmtWidth)
> -					vfmt.fmt.pix.width = width;
> -				if (set_fmts & FmtHeight)
> -					vfmt.fmt.pix.height = height;
> -				if (set_fmts & FmtPixelFormat) {
> -					vfmt.fmt.pix.pixelformat = pixfmt;
> -					if (vfmt.fmt.pix.pixelformat < 256) {
> -						vfmt.fmt.pix.pixelformat =
> -							find_pixel_format(fd, vfmt.fmt.pix.pixelformat,
> -									false, false);
> -					}
> -				}
> -				if (set_fmts & FmtField)
> -					vfmt.fmt.pix.field = field;
> -				if (set_fmts & FmtFlags)
> -					vfmt.fmt.pix.flags = flags;
> -				if (set_fmts & FmtBytesPerLine) {
> -					vfmt.fmt.pix.bytesperline = bytesperline[0];
> -				} else {
> -					/* G_FMT might return a bytesperline value > width,
> -					 * reset this to 0 to force the driver to update it
> -					 * to the closest value for the new width. */
> -					vfmt.fmt.pix.bytesperline = 0;
> -				}
> -			}
> -
> +		if (vidcap_get_and_update_fmt(_fd, vfmt) == 0) {
>  			if (options[OptSetVideoFormat])
>  				ret = doioctl(fd, VIDIOC_S_FMT, &vfmt);
>  			else
> @@ -231,6 +171,80 @@ void vidcap_set(cv4l_fd &_fd)
>  	}
>  }
>  
> +int vidcap_get_and_update_fmt(cv4l_fd &_fd, struct v4l2_format &vfmt)
> +{
> +	int fd = _fd.g_fd();
> +	int ret;
> +
> +	memset(&vfmt, 0, sizeof(vfmt));
> +	vfmt.fmt.pix.priv = priv_magic;
> +	vfmt.type = vidcap_buftype;
> +
> +	ret = doioctl(fd, VIDIOC_G_FMT, &vfmt);
> +	if (ret)
> +		return ret;
> +
> +	if (is_multiplanar) {
> +		if (set_fmts & FmtWidth)
> +			vfmt.fmt.pix_mp.width = width;
> +		if (set_fmts & FmtHeight)
> +			vfmt.fmt.pix_mp.height = height;
> +		if (set_fmts & FmtPixelFormat) {
> +			vfmt.fmt.pix_mp.pixelformat = pixfmt;
> +			if (vfmt.fmt.pix_mp.pixelformat < 256) {
> +				vfmt.fmt.pix_mp.pixelformat =
> +					find_pixel_format(fd, vfmt.fmt.pix_mp.pixelformat,
> +							  false, true);
> +			}
> +		}
> +		if (set_fmts & FmtField)
> +			vfmt.fmt.pix_mp.field = field;
> +		if (set_fmts & FmtFlags)
> +			vfmt.fmt.pix_mp.flags = flags;
> +		if (set_fmts & FmtBytesPerLine) {
> +			for (unsigned i = 0; i < VIDEO_MAX_PLANES; i++)
> +				vfmt.fmt.pix_mp.plane_fmt[i].bytesperline =
> +					bytesperline[i];
> +		} else {
> +			/*
> +			 * G_FMT might return bytesperline values > width,
> +			 * reset them to 0 to force the driver to update them
> +			 * to the closest value for the new width.
> +			 */
> +			for (unsigned i = 0; i < vfmt.fmt.pix_mp.num_planes; i++)
> +				vfmt.fmt.pix_mp.plane_fmt[i].bytesperline = 0;
> +		}
> +	} else {
> +		if (set_fmts & FmtWidth)
> +			vfmt.fmt.pix.width = width;
> +		if (set_fmts & FmtHeight)
> +			vfmt.fmt.pix.height = height;
> +		if (set_fmts & FmtPixelFormat) {
> +			vfmt.fmt.pix.pixelformat = pixfmt;
> +			if (vfmt.fmt.pix.pixelformat < 256) {
> +				vfmt.fmt.pix.pixelformat =
> +					find_pixel_format(fd, vfmt.fmt.pix.pixelformat,
> +							  false, false);
> +			}
> +		}
> +		if (set_fmts & FmtField)
> +			vfmt.fmt.pix.field = field;
> +		if (set_fmts & FmtFlags)
> +			vfmt.fmt.pix.flags = flags;
> +		if (set_fmts & FmtBytesPerLine) {
> +			vfmt.fmt.pix.bytesperline = bytesperline[0];
> +		} else {
> +			/*
> +			 * G_FMT might return a bytesperline value > width,
> +			 * reset this to 0 to force the driver to update it
> +			 * to the closest value for the new width.
> +			 */
> +			vfmt.fmt.pix.bytesperline = 0;
> +		}
> +	}
> +	return 0;
> +}
> +

Looks good, but please move vidcap_get_and_update_fmt() before vidcap_set instead
of after it. It is generally good practice to have lower level functions appear
before higher level functions.

Regards,

	Hans

>  void vidcap_get(cv4l_fd &fd)
>  {
>  	if (options[OptGetVideoFormat]) {
> diff --git a/utils/v4l2-ctl/v4l2-ctl.h b/utils/v4l2-ctl/v4l2-ctl.h
> index dcc39b51..739dc5a9 100644
> --- a/utils/v4l2-ctl/v4l2-ctl.h
> +++ b/utils/v4l2-ctl/v4l2-ctl.h
> @@ -348,6 +348,7 @@ void stds_list(cv4l_fd &fd);
>  // v4l2-ctl-vidcap.cpp
>  void vidcap_usage(void);
>  void vidcap_cmd(int ch, char *optarg);
> +int vidcap_get_and_update_fmt(cv4l_fd &_fd, struct v4l2_format &vfmt);
>  void vidcap_set(cv4l_fd &fd);
>  void vidcap_get(cv4l_fd &fd);
>  void vidcap_list(cv4l_fd &fd);
>

Patch

diff --git a/utils/v4l2-ctl/v4l2-ctl-streaming.cpp b/utils/v4l2-ctl/v4l2-ctl-streaming.cpp
index d6c3f6a9..2f66e052 100644
--- a/utils/v4l2-ctl/v4l2-ctl-streaming.cpp
+++ b/utils/v4l2-ctl/v4l2-ctl-streaming.cpp
@@ -1902,17 +1902,30 @@  static int capture_setup(cv4l_fd &fd, cv4l_queue &in, cv4l_fd *exp_fd)
 		return -1;
 	}
 
+	if (options[OptSetVideoFormat]) {
+		cv4l_fmt fmt;
+
+		if (vidcap_get_and_update_fmt(fd, fmt)) {
+			fprintf(stderr, "%s: vidcap_get_and_update_fmt error\n",
+				__func__);
+			return -1;
+		}
+		fd.s_fmt(fmt, in.g_type());
+	}
+
 	if (in.reqbufs(&fd, reqbufs_count_cap)) {
 		fprintf(stderr, "%s: in.reqbufs %u error\n", __func__,
 			reqbufs_count_cap);
 		return -1;
 	}
+
 	if (exp_fd && in.export_bufs(exp_fd, exp_fd->g_type()))
 		return -1;
 	if (in.obtain_bufs(&fd) || in.queue_all(&fd)) {
 		fprintf(stderr, "%s: in.obtain_bufs error\n", __func__);
 		return -1;
 	}
+
 	if (fd.streamon(in.g_type())) {
 		fprintf(stderr, "%s: fd.streamon error\n", __func__);
 		return -1;
diff --git a/utils/v4l2-ctl/v4l2-ctl-vidcap.cpp b/utils/v4l2-ctl/v4l2-ctl-vidcap.cpp
index dc17a868..df1203a9 100644
--- a/utils/v4l2-ctl/v4l2-ctl-vidcap.cpp
+++ b/utils/v4l2-ctl/v4l2-ctl-vidcap.cpp
@@ -155,72 +155,12 @@  void vidcap_cmd(int ch, char *optarg)
 
 void vidcap_set(cv4l_fd &_fd)
 {
-	int fd = _fd.g_fd();
-	int ret;
-
 	if (options[OptSetVideoFormat] || options[OptTryVideoFormat]) {
+		int fd = _fd.g_fd();
+		int ret;
 		struct v4l2_format vfmt;
 
-		memset(&vfmt, 0, sizeof(vfmt));
-		vfmt.fmt.pix.priv = priv_magic;
-		vfmt.type = vidcap_buftype;
-
-		if (doioctl(fd, VIDIOC_G_FMT, &vfmt) == 0) {
-			if (is_multiplanar) {
-				if (set_fmts & FmtWidth)
-					vfmt.fmt.pix_mp.width = width;
-				if (set_fmts & FmtHeight)
-					vfmt.fmt.pix_mp.height = height;
-				if (set_fmts & FmtPixelFormat) {
-					vfmt.fmt.pix_mp.pixelformat = pixfmt;
-					if (vfmt.fmt.pix_mp.pixelformat < 256) {
-						vfmt.fmt.pix_mp.pixelformat =
-							find_pixel_format(fd, vfmt.fmt.pix_mp.pixelformat,
-									false, true);
-					}
-				}
-				if (set_fmts & FmtField)
-					vfmt.fmt.pix_mp.field = field;
-				if (set_fmts & FmtFlags)
-					vfmt.fmt.pix_mp.flags = flags;
-				if (set_fmts & FmtBytesPerLine) {
-					for (unsigned i = 0; i < VIDEO_MAX_PLANES; i++)
-						vfmt.fmt.pix_mp.plane_fmt[i].bytesperline =
-							bytesperline[i];
-				} else {
-					/* G_FMT might return bytesperline values > width,
-					 * reset them to 0 to force the driver to update them
-					 * to the closest value for the new width. */
-					for (unsigned i = 0; i < vfmt.fmt.pix_mp.num_planes; i++)
-						vfmt.fmt.pix_mp.plane_fmt[i].bytesperline = 0;
-				}
-			} else {
-				if (set_fmts & FmtWidth)
-					vfmt.fmt.pix.width = width;
-				if (set_fmts & FmtHeight)
-					vfmt.fmt.pix.height = height;
-				if (set_fmts & FmtPixelFormat) {
-					vfmt.fmt.pix.pixelformat = pixfmt;
-					if (vfmt.fmt.pix.pixelformat < 256) {
-						vfmt.fmt.pix.pixelformat =
-							find_pixel_format(fd, vfmt.fmt.pix.pixelformat,
-									false, false);
-					}
-				}
-				if (set_fmts & FmtField)
-					vfmt.fmt.pix.field = field;
-				if (set_fmts & FmtFlags)
-					vfmt.fmt.pix.flags = flags;
-				if (set_fmts & FmtBytesPerLine) {
-					vfmt.fmt.pix.bytesperline = bytesperline[0];
-				} else {
-					/* G_FMT might return a bytesperline value > width,
-					 * reset this to 0 to force the driver to update it
-					 * to the closest value for the new width. */
-					vfmt.fmt.pix.bytesperline = 0;
-				}
-			}
-
+		if (vidcap_get_and_update_fmt(_fd, vfmt) == 0) {
 			if (options[OptSetVideoFormat])
 				ret = doioctl(fd, VIDIOC_S_FMT, &vfmt);
 			else
@@ -231,6 +171,80 @@  void vidcap_set(cv4l_fd &_fd)
 	}
 }
 
+int vidcap_get_and_update_fmt(cv4l_fd &_fd, struct v4l2_format &vfmt)
+{
+	int fd = _fd.g_fd();
+	int ret;
+
+	memset(&vfmt, 0, sizeof(vfmt));
+	vfmt.fmt.pix.priv = priv_magic;
+	vfmt.type = vidcap_buftype;
+
+	ret = doioctl(fd, VIDIOC_G_FMT, &vfmt);
+	if (ret)
+		return ret;
+
+	if (is_multiplanar) {
+		if (set_fmts & FmtWidth)
+			vfmt.fmt.pix_mp.width = width;
+		if (set_fmts & FmtHeight)
+			vfmt.fmt.pix_mp.height = height;
+		if (set_fmts & FmtPixelFormat) {
+			vfmt.fmt.pix_mp.pixelformat = pixfmt;
+			if (vfmt.fmt.pix_mp.pixelformat < 256) {
+				vfmt.fmt.pix_mp.pixelformat =
+					find_pixel_format(fd, vfmt.fmt.pix_mp.pixelformat,
+							  false, true);
+			}
+		}
+		if (set_fmts & FmtField)
+			vfmt.fmt.pix_mp.field = field;
+		if (set_fmts & FmtFlags)
+			vfmt.fmt.pix_mp.flags = flags;
+		if (set_fmts & FmtBytesPerLine) {
+			for (unsigned i = 0; i < VIDEO_MAX_PLANES; i++)
+				vfmt.fmt.pix_mp.plane_fmt[i].bytesperline =
+					bytesperline[i];
+		} else {
+			/*
+			 * G_FMT might return bytesperline values > width,
+			 * reset them to 0 to force the driver to update them
+			 * to the closest value for the new width.
+			 */
+			for (unsigned i = 0; i < vfmt.fmt.pix_mp.num_planes; i++)
+				vfmt.fmt.pix_mp.plane_fmt[i].bytesperline = 0;
+		}
+	} else {
+		if (set_fmts & FmtWidth)
+			vfmt.fmt.pix.width = width;
+		if (set_fmts & FmtHeight)
+			vfmt.fmt.pix.height = height;
+		if (set_fmts & FmtPixelFormat) {
+			vfmt.fmt.pix.pixelformat = pixfmt;
+			if (vfmt.fmt.pix.pixelformat < 256) {
+				vfmt.fmt.pix.pixelformat =
+					find_pixel_format(fd, vfmt.fmt.pix.pixelformat,
+							  false, false);
+			}
+		}
+		if (set_fmts & FmtField)
+			vfmt.fmt.pix.field = field;
+		if (set_fmts & FmtFlags)
+			vfmt.fmt.pix.flags = flags;
+		if (set_fmts & FmtBytesPerLine) {
+			vfmt.fmt.pix.bytesperline = bytesperline[0];
+		} else {
+			/*
+			 * G_FMT might return a bytesperline value > width,
+			 * reset this to 0 to force the driver to update it
+			 * to the closest value for the new width.
+			 */
+			vfmt.fmt.pix.bytesperline = 0;
+		}
+	}
+	return 0;
+}
+
 void vidcap_get(cv4l_fd &fd)
 {
 	if (options[OptGetVideoFormat]) {
diff --git a/utils/v4l2-ctl/v4l2-ctl.h b/utils/v4l2-ctl/v4l2-ctl.h
index dcc39b51..739dc5a9 100644
--- a/utils/v4l2-ctl/v4l2-ctl.h
+++ b/utils/v4l2-ctl/v4l2-ctl.h
@@ -348,6 +348,7 @@  void stds_list(cv4l_fd &fd);
 // v4l2-ctl-vidcap.cpp
 void vidcap_usage(void);
 void vidcap_cmd(int ch, char *optarg);
+int vidcap_get_and_update_fmt(cv4l_fd &_fd, struct v4l2_format &vfmt);
 void vidcap_set(cv4l_fd &fd);
 void vidcap_get(cv4l_fd &fd);
 void vidcap_list(cv4l_fd &fd);