[PATCH 01/12] utils: fix compilation with C++98

[PATCH 01/12] utils: fix compilation with C++98

[Rosen Penev]

Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 utils/cec-compliance/cec-test.cpp | 4 ++--
 utils/rds-ctl/rds-ctl.cpp         | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/utils/cec-compliance/cec-test.cpp b/utils/cec-compliance/cec-test.cpp
index 032ff5ad..cc07998a 100644
--- a/utils/cec-compliance/cec-test.cpp
+++ b/utils/cec-compliance/cec-test.cpp
@@ -908,7 +908,7 @@ static int tuner_ctl_test(struct node *node, unsigned me, unsigned la, bool inte
 		cec_msg_give_tuner_device_status(&msg, true, CEC_OP_STATUS_REQ_ONCE);
 		fail_on_test(!transmit_timeout(node, &msg));
 		fail_on_test(timed_out_or_abort(&msg));
-		info = {};
+		memset(&info, 0, sizeof(info));
 		cec_ops_tuner_device_status(&msg, &info);
 		if (!memcmp(&info, &info_vec[0], sizeof(info)))
 			break;
@@ -935,7 +935,7 @@ static int tuner_ctl_test(struct node *node, unsigned me, unsigned la, bool inte
 		cec_msg_give_tuner_device_status(&msg, true, CEC_OP_STATUS_REQ_ONCE);
 		fail_on_test(!transmit_timeout(node, &msg));
 		fail_on_test(timed_out_or_abort(&msg));
-		info = {};
+		memset(&info, 0, sizeof(info));
 		cec_ops_tuner_device_status(&msg, &info);
 		if (memcmp(&info, &(*iter), sizeof(info))) {
 			log_tuner_service(info);
diff --git a/utils/rds-ctl/rds-ctl.cpp b/utils/rds-ctl/rds-ctl.cpp
index 0511fe5d..0e497b2a 100644
--- a/utils/rds-ctl/rds-ctl.cpp
+++ b/utils/rds-ctl/rds-ctl.cpp
@@ -918,7 +918,7 @@ static void get_options(const int fd, const int capabilities, struct v4l2_freque
 				printf("\tFrequency range      : %.1f MHz - %.1f MHz\n",
 					 vt.rangelow / 16.0, vt.rangehigh / 16.0);
 			printf("\tSignal strength/AFC  : %ld%%/%d\n",
-				std::lround(vt.signal / 655.35), vt.afc);
+				lround(vt.signal / 655.25), vt.afc);
 			printf("\tCurrent audio mode   : %s\n", audmode2s(vt.audmode));
 			printf("\tAvailable subchannels: %s\n",
 					rxsubchans2s(vt.rxsubchans).c_str());
-- 
2.25.2

[PATCH 02/12] utils: remove extra commas

[Rosen Penev]

Found with -Wextra-semi-stmt

Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 utils/cec-compliance/cec-compliance.cpp     | 2 +-
 utils/cec-follower/cec-follower.cpp         | 2 +-
 utils/v4l2-compliance/v4l2-test-buffers.cpp | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/utils/cec-compliance/cec-compliance.cpp b/utils/cec-compliance/cec-compliance.cpp
index 713e2a58..1c62b5c9 100644
--- a/utils/cec-compliance/cec-compliance.cpp
+++ b/utils/cec-compliance/cec-compliance.cpp
@@ -336,7 +336,7 @@ static std::string audio_format_code2s(__u8 format_code)
 		return "Extended";
 	default:
 		return "Illegal";
-	};
+	}
 }
 
 std::string extension_type_code2s(__u8 type_code)
diff --git a/utils/cec-follower/cec-follower.cpp b/utils/cec-follower/cec-follower.cpp
index 36bd2fbb..1eeae2d4 100644
--- a/utils/cec-follower/cec-follower.cpp
+++ b/utils/cec-follower/cec-follower.cpp
@@ -189,7 +189,7 @@ static std::string audio_format_code2s(__u8 format_code)
 		return "Extended";
 	default:
 		return "Illegal";
-	};
+	}
 }
 
 std::string extension_type_code2s(__u8 type_code)
diff --git a/utils/v4l2-compliance/v4l2-test-buffers.cpp b/utils/v4l2-compliance/v4l2-test-buffers.cpp
index 3b45711d..93df7034 100644
--- a/utils/v4l2-compliance/v4l2-test-buffers.cpp
+++ b/utils/v4l2-compliance/v4l2-test-buffers.cpp
@@ -1752,7 +1752,7 @@ static int setupDmaBuf(struct node *expbuf_node, struct node *node,
 
 		fail_on_test(buf.qbuf(node, false));
 		for (unsigned p = 0; p < buf.g_num_planes(); p++) {
-			fail_on_test(buf.g_fd(p) != q.g_fd(i, p));;
+			fail_on_test(buf.g_fd(p) != q.g_fd(i, p));
 			fail_on_test(buf.g_length(p) != q.g_length(p));
 			if (v4l_type_is_output(q.g_type()))
 				fail_on_test(!buf.g_bytesused(p));
-- 
2.25.2

[PATCH 03/12] utils: fix float equal warning

[Rosen Penev]

Found with -Wfloat-equal

Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 utils/v4l2-ctl/v4l2-ctl-streaming.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/utils/v4l2-ctl/v4l2-ctl-streaming.cpp b/utils/v4l2-ctl/v4l2-ctl-streaming.cpp
index 663d0254..066a336a 100644
--- a/utils/v4l2-ctl/v4l2-ctl-streaming.cpp
+++ b/utils/v4l2-ctl/v4l2-ctl-streaming.cpp
@@ -154,7 +154,7 @@ void fps_timestamps::determine_field(int fd, unsigned type)
 
 bool fps_timestamps::add_ts(double ts_secs, unsigned sequence, unsigned field)
 {
-	if (ts_secs == 0) {
+	if (ts_secs <= 0) {
 		struct timespec ts_cur;
 
 		clock_gettime(CLOCK_MONOTONIC, &ts_cur);
-- 
2.25.2

[PATCH 04/12] utils: don't check unsigned for < 0.

[Rosen Penev]

Found with -Wtautological-unsigned-zero-compare

Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 utils/v4l2-compliance/v4l2-test-controls.cpp | 2 --
 1 file changed, 2 deletions(-)

diff --git a/utils/v4l2-compliance/v4l2-test-controls.cpp b/utils/v4l2-compliance/v4l2-test-controls.cpp
index 8c4480be..251a6049 100644
--- a/utils/v4l2-compliance/v4l2-test-controls.cpp
+++ b/utils/v4l2-compliance/v4l2-test-controls.cpp
@@ -80,8 +80,6 @@ static int checkQCtrl(struct node *node, struct test_query_ext_ctrl &qctrl)
 			return fail("min > max\n");
 		if (qctrl.step == 0)
 			return fail("step == 0\n");
-		if (qctrl.step < 0)
-			return fail("step < 0\n");
 		if (static_cast<unsigned>(qctrl.step) > static_cast<unsigned>(qctrl.maximum - qctrl.minimum) &&
 		    qctrl.maximum != qctrl.minimum)
 			return fail("step > max - min\n");
-- 
2.25.2

[PATCH 05/12] utils: add copy assignment operator

[Rosen Penev]

Found with -Wdeprecated

Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 utils/common/cv4l-helpers.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/utils/common/cv4l-helpers.h b/utils/common/cv4l-helpers.h
index a8089930..9505e334 100644
--- a/utils/common/cv4l-helpers.h
+++ b/utils/common/cv4l-helpers.h
@@ -830,6 +830,10 @@ public:
 	{
 		init(b);
 	}
+	cv4l_buffer operator= (const cv4l_buffer &b)
+	{
+		return *this;
+	}
 	virtual ~cv4l_buffer() {}
 
 	void init(unsigned type = 0, unsigned memory = 0, unsigned index = 0)
-- 
2.25.2

[PATCH 06/12] utils: add noreturn attribute and remove dead code

[Rosen Penev]

Found with -Wmissing-noreturn and -Wunreachable-code-return

Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 utils/v4l2-compliance/v4l2-compliance.cpp | 9 ++-------
 utils/v4l2-dbg/v4l2-dbg.cpp               | 7 +------
 2 files changed, 3 insertions(+), 13 deletions(-)

diff --git a/utils/v4l2-compliance/v4l2-compliance.cpp b/utils/v4l2-compliance/v4l2-compliance.cpp
index b3a18492..fbf34914 100644
--- a/utils/v4l2-compliance/v4l2-compliance.cpp
+++ b/utils/v4l2-compliance/v4l2-compliance.cpp
@@ -155,6 +155,7 @@ static struct option long_options[] = {
 	{0, 0, 0, 0}
 };
 
+__attribute__((noreturn))
 static void usage()
 {
 	printf("Usage:\n");
@@ -482,6 +483,7 @@ static void restoreState()
 	node->reopen();
 }
 
+__attribute__((noreturn))
 static void signal_handler_interrupt(int signum)
 {
 	restoreState();
@@ -1544,7 +1546,6 @@ int main(int argc, char **argv)
 		switch (ch) {
 		case OptHelp:
 			usage();
-			return 0;
 		case OptSetDevice:
 			device = make_devname(optarg, "video", media_bus_info);
 			break;
@@ -1619,7 +1620,6 @@ int main(int argc, char **argv)
 						color_component = 2;
 					else {
 						usage();
-						exit(1);
 					}
 					break;
 				case 1:
@@ -1634,7 +1634,6 @@ int main(int argc, char **argv)
 					break;
 				default:
 					usage();
-					exit(1);
 				}
 			}
 			break;
@@ -1647,7 +1646,6 @@ int main(int argc, char **argv)
 				show_colors = isatty(STDOUT_FILENO);
 			else {
 				usage();
-				exit(1);
 			}
 			break;
 		case OptNoWarnings:
@@ -1669,13 +1667,11 @@ int main(int argc, char **argv)
 			fprintf(stderr, "Option `%s' requires a value\n",
 				argv[optind]);
 			usage();
-			return 1;
 		case '?':
 			if (argv[optind])
 				fprintf(stderr, "Unknown argument `%s'\n",
 					argv[optind]);
 			usage();
-			return 1;
 		}
 	}
 	if (optind < argc) {
@@ -1684,7 +1680,6 @@ int main(int argc, char **argv)
 			fprintf(stderr, "%s ", argv[optind++]);
 		fprintf(stderr, "\n");
 		usage();
-		return 1;
 	}
 	bool direct = !options[OptUseWrapper];
 	int fd;
diff --git a/utils/v4l2-dbg/v4l2-dbg.cpp b/utils/v4l2-dbg/v4l2-dbg.cpp
index cd387d1d..7b986a50 100644
--- a/utils/v4l2-dbg/v4l2-dbg.cpp
+++ b/utils/v4l2-dbg/v4l2-dbg.cpp
@@ -162,6 +162,7 @@ static struct option long_options[] = {
 	{0, 0, 0, 0}
 };
 
+__attribute__((noreturn))
 static void usage()
 {
 	printf("Usage: v4l2-dbg [options] [values]\n"
@@ -387,13 +388,11 @@ static int parse_subopt(char **subs, const char * const *subopts, char **value)
 	if (opt == -1) {
 		fprintf(stderr, "Invalid suboptions specified\n");
 		usage();
-		exit(1);
 	}
 	if (*value == NULL) {
 		fprintf(stderr, "No value given to suboption <%s>\n",
 				subopts[opt]);
 		usage();
-		exit(1);
 	}
 	return opt;
 }
@@ -429,7 +428,6 @@ int main(int argc, char **argv)
 
 	if (argc == 1) {
 		usage();
-		return 0;
 	}
 	for (i = 0; long_options[i].name; i++) {
 		if (!isalpha(long_options[i].val))
@@ -467,7 +465,6 @@ int main(int argc, char **argv)
 		switch (ch) {
 		case OptHelp:
 			usage();
-			return 0;
 
 		case OptSetDevice:
 			device = optarg;
@@ -538,13 +535,11 @@ int main(int argc, char **argv)
 			fprintf(stderr, "Option `%s' requires a value\n",
 				argv[optind]);
 			usage();
-			return 1;
 
 		case '?':
 			fprintf(stderr, "Unknown argument `%s'\n",
 				argv[optind]);
 			usage();
-			return 1;
 		}
 	}
 
-- 
2.25.2

[PATCH 07/12] utils: fix implicit double promotion

[Rosen Penev]

Found with -Wimplicit-float-conversion

Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 utils/rds-ctl/rds-ctl.cpp | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/utils/rds-ctl/rds-ctl.cpp b/utils/rds-ctl/rds-ctl.cpp
index 0e497b2a..cc1d3bf7 100644
--- a/utils/rds-ctl/rds-ctl.cpp
+++ b/utils/rds-ctl/rds-ctl.cpp
@@ -525,7 +525,7 @@ static void print_rds_statistics(const struct v4l2_rds_statistics *statistics)
 
 	if (statistics->block_cnt)
 		block_error_percentage =
-			(static_cast<float>(statistics->block_error_cnt) / statistics->block_cnt) * 100.0;
+			(static_cast<float>(statistics->block_error_cnt) / statistics->block_cnt) * 100.0f;
 	printf("block errors / group errors: %u (%3.2f%%) / %u \n",
 		statistics->block_error_cnt,
 		block_error_percentage, statistics->group_error_cnt);
@@ -534,7 +534,7 @@ static void print_rds_statistics(const struct v4l2_rds_statistics *statistics)
 
 	if (statistics->block_cnt)
 		block_corrected_percentage = (
-			static_cast<float>(statistics->block_corrected_cnt) / statistics->block_cnt) * 100.0;
+			static_cast<float>(statistics->block_corrected_cnt) / statistics->block_cnt) * 100.0f;
 	printf("corrected blocks: %u (%3.2f%%)\n",
 		statistics->block_corrected_cnt, block_corrected_percentage);
 	for (int i = 0; i < 16; i++)
-- 
2.25.2

[PATCH 08/12] utils: initialize variable

[Rosen Penev]

clang is too stupid to figure out that this depends on the bool.

Fixes -Wconditional-uninitialized

Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 utils/cec-ctl/cec-ctl.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/utils/cec-ctl/cec-ctl.cpp b/utils/cec-ctl/cec-ctl.cpp
index 3acfe0b2..f4133494 100644
--- a/utils/cec-ctl/cec-ctl.cpp
+++ b/utils/cec-ctl/cec-ctl.cpp
@@ -2091,7 +2091,7 @@ int main(int argc, char **argv)
 			};
 			char *value, *endptr, *subs = optarg;
 			bool have_cmd = false;
-			__u8 cmd;
+			__u8 cmd = 0;
 			__u8 size = 0;
 			__u8 bytes[14];
 
-- 
2.25.2

[PATCH 09/12] utils: fix implicit float conversions

[Rosen Penev]

Found with -Wfloat-conversion

Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 utils/cec-ctl/cec-ctl.cpp                   | 4 ++--
 utils/cec-ctl/cec-pin.cpp                   | 2 +-
 utils/v4l2-compliance/v4l2-test-buffers.cpp | 2 +-
 utils/v4l2-ctl/v4l2-ctl-misc.cpp            | 4 ++--
 utils/v4l2-ctl/v4l2-ctl-streaming.cpp       | 2 +-
 utils/v4l2-ctl/v4l2-ctl-subdev.cpp          | 2 +-
 6 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/utils/cec-ctl/cec-ctl.cpp b/utils/cec-ctl/cec-ctl.cpp
index f4133494..f0e31aca 100644
--- a/utils/cec-ctl/cec-ctl.cpp
+++ b/utils/cec-ctl/cec-ctl.cpp
@@ -1507,8 +1507,8 @@ static void stress_test_power_cycle(struct node &node, unsigned cnt,
 	srandom(seed);
 
 	for (;;) {
-		unsigned usecs1 = mod_usleep ? random() % mod_usleep : sleep_before_on * 1000000;
-		unsigned usecs2 = mod_usleep ? random() % mod_usleep : sleep_before_off * 1000000;
+		unsigned usecs1 = mod_usleep ? random() % mod_usleep : (unsigned)(sleep_before_on * 1000000);
+		unsigned usecs2 = mod_usleep ? random() % mod_usleep : (unsigned)(sleep_before_off * 1000000);
 
 		usecs1 += min_usleep;
 		usecs2 += min_usleep;
diff --git a/utils/cec-ctl/cec-pin.cpp b/utils/cec-ctl/cec-pin.cpp
index 0322ad5e..10abea37 100644
--- a/utils/cec-ctl/cec-pin.cpp
+++ b/utils/cec-ctl/cec-pin.cpp
@@ -261,7 +261,7 @@ static void cec_pin_rx_data_bit_was_low(__u64 ev_ts, __u64 usecs, __u64 usecs_mi
 	 * If the low drive starts at the end of a 0 bit, then the actual
 	 * maximum time that the bus can be low is the two summed.
 	 */
-	const unsigned max_low_drive = CEC_TIM_LOW_DRIVE_ERROR_MAX +
+	const unsigned max_low_drive = (unsigned)CEC_TIM_LOW_DRIVE_ERROR_MAX +
 		CEC_TIM_DATA_BIT_0_LOW_MAX + CEC_TIM_MARGIN;
 
 	low_usecs = usecs;
diff --git a/utils/v4l2-compliance/v4l2-test-buffers.cpp b/utils/v4l2-compliance/v4l2-test-buffers.cpp
index 93df7034..87c2e523 100644
--- a/utils/v4l2-compliance/v4l2-test-buffers.cpp
+++ b/utils/v4l2-compliance/v4l2-test-buffers.cpp
@@ -2597,7 +2597,7 @@ static void streamFmt(struct node *node, __u32 pixelformat, __u32 w, __u32 h,
 	char hz[32] = "";
 
 	if (!frame_count)
-		frame_count = f ? 1.0 / fract2f(f) : 10;
+		frame_count = f ? (unsigned)(1.0f / fract2f(f)) : 10;
 	node->g_fmt(fmt);
 	fmt.s_pixelformat(pixelformat);
 	fmt.s_width(w);
diff --git a/utils/v4l2-ctl/v4l2-ctl-misc.cpp b/utils/v4l2-ctl/v4l2-ctl-misc.cpp
index cb933217..2851886a 100644
--- a/utils/v4l2-ctl/v4l2-ctl-misc.cpp
+++ b/utils/v4l2-ctl/v4l2-ctl-misc.cpp
@@ -320,7 +320,7 @@ void misc_set(cv4l_fd &_fd)
 		parm.type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
 		parm.parm.capture.timeperframe.numerator = 1000;
 		parm.parm.capture.timeperframe.denominator =
-			fps * parm.parm.capture.timeperframe.numerator;
+			(uint32_t)(fps * parm.parm.capture.timeperframe.numerator);
 
 		if (doioctl(fd, VIDIOC_S_PARM, &parm) == 0) {
 			struct v4l2_fract *tf = &parm.parm.capture.timeperframe;
@@ -338,7 +338,7 @@ void misc_set(cv4l_fd &_fd)
 		parm.type = V4L2_BUF_TYPE_VIDEO_OUTPUT;
 		parm.parm.output.timeperframe.numerator = 1000;
 		parm.parm.output.timeperframe.denominator =
-			output_fps * parm.parm.output.timeperframe.numerator;
+			(uint32_t)(output_fps * parm.parm.output.timeperframe.numerator);
 
 		if (doioctl(fd, VIDIOC_S_PARM, &parm) == 0) {
 			struct v4l2_fract *tf = &parm.parm.output.timeperframe;
diff --git a/utils/v4l2-ctl/v4l2-ctl-streaming.cpp b/utils/v4l2-ctl/v4l2-ctl-streaming.cpp
index 066a336a..6981a3cc 100644
--- a/utils/v4l2-ctl/v4l2-ctl-streaming.cpp
+++ b/utils/v4l2-ctl/v4l2-ctl-streaming.cpp
@@ -569,7 +569,7 @@ static void print_concise_buffer(FILE *f, cv4l_buffer &buf, cv4l_fmt &fmt,
 	if (!skip_ts && (buf.g_flags() & V4L2_BUF_FLAG_TIMESTAMP_MASK) != V4L2_BUF_FLAG_TIMESTAMP_COPY) {
 		double ts = buf.g_timestamp().tv_sec + buf.g_timestamp().tv_usec / 1000000.0;
 		fprintf(f, " ts: %.06f", ts);
-		if (last_ts)
+		if (last_ts <= 0.0)
 			fprintf(f, " delta: %.03f ms", (ts - last_ts) * 1000.0);
 		last_ts = ts;
 
diff --git a/utils/v4l2-ctl/v4l2-ctl-subdev.cpp b/utils/v4l2-ctl/v4l2-ctl-subdev.cpp
index 747f1699..f1223084 100644
--- a/utils/v4l2-ctl/v4l2-ctl-subdev.cpp
+++ b/utils/v4l2-ctl/v4l2-ctl-subdev.cpp
@@ -487,7 +487,7 @@ void subdev_set(cv4l_fd &_fd)
 			exit(1);
 		}
 		fival.interval.numerator = 1000;
-		fival.interval.denominator = set_fps * fival.interval.numerator;
+		fival.interval.denominator = (uint32_t)(set_fps * fival.interval.numerator);
 		printf("Note: --set-subdev-fps is only for testing.\n"
 		       "Normally media-ctl is used to configure the video pipeline.\n");
 		printf("ioctl: VIDIOC_SUBDEV_S_FRAME_INTERVAL (pad=%u)\n", fival.pad);
-- 
2.25.2

[PATCH 10/12] utils: fix fallthrough warnings

[Rosen Penev]

Comments are not enough for clang. Changed to use the C++ attribute.

Found with -Wimplicit-fallthrough

Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 utils/cec-compliance/cec-compliance.cpp      |  7 +++++++
 utils/cec-ctl/cec-ctl.cpp                    |  8 +++++++-
 utils/cec-follower/cec-follower.cpp          |  7 +++++++
 utils/cec-follower/cec-processing.cpp        |  9 ++++++++-
 utils/cec-follower/cec-tuner.cpp             | 12 +++++++++---
 utils/libcecutil/cec-log.cpp                 |  8 +++++++-
 utils/v4l2-compliance/v4l2-test-colors.cpp   | 16 +++++++++++-----
 utils/v4l2-compliance/v4l2-test-controls.cpp | 16 +++++++++++-----
 utils/v4l2-compliance/v4l2-test-formats.cpp  | 10 ++++++++--
 utils/v4l2-ctl/v4l2-ctl-streaming.cpp        | 10 ++++++++--
 utils/v4l2-ctl/v4l2-ctl-vbi.cpp              | 10 ++++++++--
 11 files changed, 91 insertions(+), 22 deletions(-)

diff --git a/utils/cec-compliance/cec-compliance.cpp b/utils/cec-compliance/cec-compliance.cpp
index 1c62b5c9..e9fce337 100644
--- a/utils/cec-compliance/cec-compliance.cpp
+++ b/utils/cec-compliance/cec-compliance.cpp
@@ -24,6 +24,12 @@
 #include "version.h"
 #endif
 
+#ifdef __clang__
+#define FALLTHROUGH [[clang::fallthrough]]
+#else
+#define FALLTHROUGH [[gnu::fallthrough]]
+#endif
+
 /* Short option list
 
    Please keep in alphabetical order.
@@ -486,6 +492,7 @@ void sad_decode(struct short_audio_desc *sad, __u32 descriptor)
 		case SAD_EXT_TYPE_MPEG_H_3D_AUDIO:
 		case SAD_EXT_TYPE_AC_4:
 			sad->format_dependent = b3 & 0x07;
+			FALLTHROUGH;
 		case SAD_EXT_TYPE_LPCM_3D_AUDIO:
 			sad->bit_depth_mask = b3 & 0x07;
 			break;
diff --git a/utils/cec-ctl/cec-ctl.cpp b/utils/cec-ctl/cec-ctl.cpp
index f0e31aca..a3698a67 100644
--- a/utils/cec-ctl/cec-ctl.cpp
+++ b/utils/cec-ctl/cec-ctl.cpp
@@ -37,6 +37,12 @@
 
 #include "cec-ctl.h"
 
+#ifdef __clang__
+#define FALLTHROUGH [[clang::fallthrough]]
+#else
+#define FALLTHROUGH [[gnu::fallthrough]]
+#endif
+
 static struct timespec start_monotonic;
 static struct timeval start_timeofday;
 static bool ignore_la[16];
@@ -1967,7 +1973,7 @@ int main(int argc, char **argv)
 			break;
 		case OptPhysAddrFromEDIDPoll:
 			edid_path = optarg;
-			/* fall-through */
+			FALLTHROUGH;
 		case OptPhysAddrFromEDID:
 			phys_addr = parse_phys_addr_from_edid(optarg);
 			break;
diff --git a/utils/cec-follower/cec-follower.cpp b/utils/cec-follower/cec-follower.cpp
index 1eeae2d4..2d61d5f9 100644
--- a/utils/cec-follower/cec-follower.cpp
+++ b/utils/cec-follower/cec-follower.cpp
@@ -23,6 +23,12 @@
 #include "version.h"
 #endif
 
+#ifdef __clang__
+#define FALLTHROUGH [[clang::fallthrough]]
+#else
+#define FALLTHROUGH [[gnu::fallthrough]]
+#endif
+
 /* Short option list
 
    Please keep in alphabetical order.
@@ -142,6 +148,7 @@ void sad_encode(const struct short_audio_desc *sad, __u32 *descriptor)
 		case SAD_EXT_TYPE_MPEG_H_3D_AUDIO:
 		case SAD_EXT_TYPE_AC_4:
 			b3 |= sad->format_dependent & 0x07;
+			FALLTHROUGH;
 		case SAD_EXT_TYPE_LPCM_3D_AUDIO:
 			b3 |= sad->bit_depth_mask & 0x07;
 			break;
diff --git a/utils/cec-follower/cec-processing.cpp b/utils/cec-follower/cec-processing.cpp
index 8e3a33a2..68bfdca3 100644
--- a/utils/cec-follower/cec-processing.cpp
+++ b/utils/cec-follower/cec-processing.cpp
@@ -19,6 +19,12 @@
 
 #include "cec-follower.h"
 
+#ifdef __clang__
+#define FALLTHROUGH [[clang::fallthrough]]
+#else
+#define FALLTHROUGH [[gnu::fallthrough]]
+#endif
+
 #define VOLUME_MAX 0x64
 #define VOLUME_MIN 0
 
@@ -352,7 +358,7 @@ static void processMsg(struct node *node, struct cec_msg &msg, unsigned me)
 		if (cec_has_tv(1 << la) && la_info[la].phys_addr == 0)
 			warn("TV (0) at 0.0.0.0 sent Routing Information.");
 	}
-
+	FALLTHROUGH;
 
 		/* System Information */
 
@@ -708,6 +714,7 @@ static void processMsg(struct node *node, struct cec_msg &msg, unsigned me)
 		cec_msg_system_audio_mode_status(&msg, node->state.sac_active ? CEC_OP_SYS_AUD_STATUS_ON :
 						 CEC_OP_SYS_AUD_STATUS_OFF);
 		transmit(node, &msg);
+		FALLTHROUGH;
 	case CEC_MSG_GIVE_AUDIO_STATUS:
 		if (!cec_has_audiosystem(1 << me))
 			break;
diff --git a/utils/cec-follower/cec-tuner.cpp b/utils/cec-follower/cec-tuner.cpp
index aa19f55d..af7c6437 100644
--- a/utils/cec-follower/cec-tuner.cpp
+++ b/utils/cec-follower/cec-tuner.cpp
@@ -8,6 +8,12 @@
 
 #include "cec-follower.h"
 
+#ifdef __clang__
+#define FALLTHROUGH [[clang::fallthrough]]
+#else
+#define FALLTHROUGH [[gnu::fallthrough]]
+#endif
+
 #define NUM_ANALOG_FREQS 3
 #define NUM_DIGITAL_CHANS 3
 #define TOT_ANALOG_FREQS (sizeof(analog_freqs_khz) / sizeof(analog_freqs_khz[0][0][0]))
@@ -303,21 +309,21 @@ static int digital_get_service_idx(struct cec_op_digital_service_id *digital)
 	switch (digital->dig_bcast_system) {
 	case CEC_OP_DIG_SERVICE_BCAST_SYSTEM_ARIB_T:
 		is_terrestrial = true;
-		/* fall through */
+		FALLTHROUGH;
 	case CEC_OP_DIG_SERVICE_BCAST_SYSTEM_ARIB_BS:
 		info = &digital_arib_data[is_terrestrial][0];
 		offset = is_terrestrial * NUM_DIGITAL_CHANS;
 		break;
 	case CEC_OP_DIG_SERVICE_BCAST_SYSTEM_ATSC_T:
 		is_terrestrial = true;
-		/* fall through */
+		FALLTHROUGH;
 	case CEC_OP_DIG_SERVICE_BCAST_SYSTEM_ATSC_SAT:
 		info = &digital_atsc_data[is_terrestrial][0];
 		offset = (2 + is_terrestrial) * NUM_DIGITAL_CHANS;
 		break;
 	case CEC_OP_DIG_SERVICE_BCAST_SYSTEM_DVB_T:
 		is_terrestrial = true;
-		/* fall through */
+		FALLTHROUGH;
 	case CEC_OP_DIG_SERVICE_BCAST_SYSTEM_DVB_S2:
 		info = &digital_dvb_data[is_terrestrial][0];
 		offset = (4 + is_terrestrial) * NUM_DIGITAL_CHANS;
diff --git a/utils/libcecutil/cec-log.cpp b/utils/libcecutil/cec-log.cpp
index 28a1ecac..9844ac43 100644
--- a/utils/libcecutil/cec-log.cpp
+++ b/utils/libcecutil/cec-log.cpp
@@ -15,6 +15,12 @@
 #include "cec-info.h"
 #include "cec-log.h"
 
+#ifdef __clang__
+#define FALLTHROUGH [[clang::fallthrough]]
+#else
+#define FALLTHROUGH [[gnu::fallthrough]]
+#endif
+
 static const struct cec_arg arg_u8 = {
 	CEC_ARG_TYPE_U8,
 };
@@ -44,7 +50,7 @@ static void log_arg(const struct cec_arg *arg, const char *arg_name, __u32 val)
 				return;
 			}
 		}
-		/* fall through */
+		FALLTHROUGH;
 	case CEC_ARG_TYPE_U8:
 		if (!strcmp(arg_name, "video-latency") ||
 		    !strcmp(arg_name, "audio-out-delay")) {
diff --git a/utils/v4l2-compliance/v4l2-test-colors.cpp b/utils/v4l2-compliance/v4l2-test-colors.cpp
index 887b198b..d37300e9 100644
--- a/utils/v4l2-compliance/v4l2-test-colors.cpp
+++ b/utils/v4l2-compliance/v4l2-test-colors.cpp
@@ -30,6 +30,12 @@
 #include <math.h>
 #include "v4l2-compliance.h"
 
+#ifdef __clang__
+#define FALLTHROUGH [[clang::fallthrough]]
+#else
+#define FALLTHROUGH [[gnu::fallthrough]]
+#endif
+
 static void setupPlanes(const cv4l_fmt &fmt, __u8 *planes[3])
 {
 	if (fmt.g_num_planes() > 1)
@@ -328,7 +334,7 @@ static void getColor(const cv4l_fmt &fmt, __u8 * const planes[3],
 	case V4L2_PIX_FMT_RGBA444:
 	case V4L2_PIX_FMT_BGRA444:
 		c.a = (v16 >> 12) / 15.0;
-		/* fall through */
+		FALLTHROUGH;
 	case V4L2_PIX_FMT_RGB444:
 	case V4L2_PIX_FMT_XRGB444:
 	case V4L2_PIX_FMT_XBGR444:
@@ -345,7 +351,7 @@ static void getColor(const cv4l_fmt &fmt, __u8 * const planes[3],
 	case V4L2_PIX_FMT_ABGR555:
 	case V4L2_PIX_FMT_BGRA555:
 		c.a = v16 >> 15;
-		/* fall through */
+		FALLTHROUGH;
 	case V4L2_PIX_FMT_YUV555:
 	case V4L2_PIX_FMT_RGB555:
 	case V4L2_PIX_FMT_XRGB555:
@@ -376,7 +382,7 @@ static void getColor(const cv4l_fmt &fmt, __u8 * const planes[3],
 	case V4L2_PIX_FMT_RGBA32:
 	case V4L2_PIX_FMT_BGRA32:
 		c.a = ((v32 >> 24) & 0xff) / 255.0;
-		/* fall through */
+		FALLTHROUGH;
 	default:
 		c.r = ((v32 >> 16) & 0xff) / 255.0;
 		c.g = ((v32 >> 8) & 0xff) / 255.0;
@@ -444,7 +450,7 @@ static void getColor(const cv4l_fmt &fmt, __u8 * const planes[3],
 		Y = (Y - 16.0 / 255.0) * 255.0 / 219.0;
 		cb *= 255.0 / 224.0;
 		cr *= 255.0 / 224.0;
-		/* fall through */
+		FALLTHROUGH;
 	case V4L2_YCBCR_ENC_601:
 	default:
 		ycbcr2rgb(bt601, Y, cb, cr, c);
@@ -453,7 +459,7 @@ static void getColor(const cv4l_fmt &fmt, __u8 * const planes[3],
 		Y = (Y - 16.0 / 255.0) * 255.0 / 219.0;
 		cb *= 255.0 / 224.0;
 		cr *= 255.0 / 224.0;
-		/* fall through */
+		FALLTHROUGH;
 	case V4L2_YCBCR_ENC_709:
 		ycbcr2rgb(rec709, Y, cb, cr, c);
 		break;
diff --git a/utils/v4l2-compliance/v4l2-test-controls.cpp b/utils/v4l2-compliance/v4l2-test-controls.cpp
index 251a6049..2de888cf 100644
--- a/utils/v4l2-compliance/v4l2-test-controls.cpp
+++ b/utils/v4l2-compliance/v4l2-test-controls.cpp
@@ -34,6 +34,12 @@
 
 #define V4L2_CTRL_CLASS_VIVID 0x00f00000
 
+#ifdef __clang__
+#define FALLTHROUGH [[clang::fallthrough]]
+#else
+#define FALLTHROUGH [[gnu::fallthrough]]
+#endif
+
 static int checkQCtrl(struct node *node, struct test_query_ext_ctrl &qctrl)
 {
 	struct v4l2_querymenu qmenu;
@@ -61,20 +67,20 @@ static int checkQCtrl(struct node *node, struct test_query_ext_ctrl &qctrl)
 	case V4L2_CTRL_TYPE_BOOLEAN:
 		if (qctrl.maximum > 1)
 			return fail("invalid boolean max value\n");
-		/* fall through */
+		FALLTHROUGH;
 	case V4L2_CTRL_TYPE_MENU:
 	case V4L2_CTRL_TYPE_INTEGER_MENU:
 		if (qctrl.step != 1)
 			return fail("invalid step value %lld\n", qctrl.step);
 		if (qctrl.minimum < 0)
 			return fail("min < 0\n");
-		/* fall through */
+		FALLTHROUGH;
 	case V4L2_CTRL_TYPE_INTEGER:
 	case V4L2_CTRL_TYPE_INTEGER64:
 		if (qctrl.default_value < qctrl.minimum ||
 		    qctrl.default_value > qctrl.maximum)
 			return fail("def < min || def > max\n");
-		/* fall through */
+		FALLTHROUGH;
 	case V4L2_CTRL_TYPE_STRING:
 		if (qctrl.minimum > qctrl.maximum)
 			return fail("min > max\n");
@@ -116,7 +122,7 @@ static int checkQCtrl(struct node *node, struct test_query_ext_ctrl &qctrl)
 	case V4L2_CTRL_TYPE_BUTTON:
 		if ((fl & rw_mask) != V4L2_CTRL_FLAG_WRITE_ONLY)
 			return fail("button control not write only\n");
-		/* fall through */
+		FALLTHROUGH;
 	case V4L2_CTRL_TYPE_BOOLEAN:
 	case V4L2_CTRL_TYPE_MENU:
 	case V4L2_CTRL_TYPE_INTEGER_MENU:
@@ -124,7 +130,7 @@ static int checkQCtrl(struct node *node, struct test_query_ext_ctrl &qctrl)
 	case V4L2_CTRL_TYPE_BITMASK:
 		if (fl & V4L2_CTRL_FLAG_SLIDER)
 			return fail("slider makes only sense for integer controls\n");
-		/* fall through */
+		FALLTHROUGH;
 	case V4L2_CTRL_TYPE_INTEGER64:
 	case V4L2_CTRL_TYPE_INTEGER:
 		if ((fl & rw_mask) == rw_mask)
diff --git a/utils/v4l2-compliance/v4l2-test-formats.cpp b/utils/v4l2-compliance/v4l2-test-formats.cpp
index 3fc87316..9e98cf99 100644
--- a/utils/v4l2-compliance/v4l2-test-formats.cpp
+++ b/utils/v4l2-compliance/v4l2-test-formats.cpp
@@ -32,6 +32,12 @@
 #include <assert.h>
 #include "v4l2-compliance.h"
 
+#ifdef __clang__
+#define FALLTHROUGH [[clang::fallthrough]]
+#else
+#define FALLTHROUGH [[gnu::fallthrough]]
+#endif
+
 static const __u32 buftype2cap[] = {
 	0,
 	V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_VIDEO_M2M,
@@ -97,7 +103,7 @@ static int testEnumFrameIntervals(struct node *node, __u32 pixfmt,
 		case V4L2_FRMIVAL_TYPE_CONTINUOUS:
 			if (sw->step.numerator != 1 || sw->step.denominator != 1)
 				return fail("invalid step for continuous frameinterval\n");
-			/* fallthrough */
+			FALLTHROUGH;
 		case V4L2_FRMIVAL_TYPE_STEPWISE:
 			if (frmival.index)
 				return fail("index must be 0 for stepwise/continuous frameintervals\n");
@@ -183,7 +189,7 @@ static int testEnumFrameSizes(struct node *node, __u32 pixfmt)
 		case V4L2_FRMSIZE_TYPE_CONTINUOUS:
 			if (frmsize.stepwise.step_width != 1 || frmsize.stepwise.step_height != 1)
 				return fail("invalid step_width/height for continuous framesize\n");
-			/* fallthrough */
+			FALLTHROUGH;
 		case V4L2_FRMSIZE_TYPE_STEPWISE:
 			if (frmsize.index)
 				return fail("index must be 0 for stepwise/continuous framesizes\n");
diff --git a/utils/v4l2-ctl/v4l2-ctl-streaming.cpp b/utils/v4l2-ctl/v4l2-ctl-streaming.cpp
index 6981a3cc..bb464e99 100644
--- a/utils/v4l2-ctl/v4l2-ctl-streaming.cpp
+++ b/utils/v4l2-ctl/v4l2-ctl-streaming.cpp
@@ -24,6 +24,12 @@
 #include <media-info.h>
 #include <fwht-ctrls.h>
 
+#ifdef __clang__
+#define FALLTHROUGH [[clang::fallthrough]]
+#else
+#define FALLTHROUGH [[gnu::fallthrough]]
+#endif
+
 extern "C" {
 #include "v4l2-tpg.h"
 }
@@ -763,7 +769,7 @@ void streaming_cmd(int ch, char *optarg)
 		break;
 	case OptStreamUser:
 		memory = V4L2_MEMORY_USERPTR;
-		/* fall through */
+		FALLTHROUGH;
 	case OptStreamMmap:
 		if (optarg) {
 			reqbufs_count_cap = strtoul(optarg, 0L, 0);
@@ -776,7 +782,7 @@ void streaming_cmd(int ch, char *optarg)
 		break;
 	case OptStreamOutUser:
 		out_memory = V4L2_MEMORY_USERPTR;
-		/* fall through */
+		FALLTHROUGH;
 	case OptStreamOutMmap:
 		if (optarg) {
 			reqbufs_count_out = strtoul(optarg, 0L, 0);
diff --git a/utils/v4l2-ctl/v4l2-ctl-vbi.cpp b/utils/v4l2-ctl/v4l2-ctl-vbi.cpp
index c5960f78..82d01b58 100644
--- a/utils/v4l2-ctl/v4l2-ctl-vbi.cpp
+++ b/utils/v4l2-ctl/v4l2-ctl-vbi.cpp
@@ -16,6 +16,12 @@
 
 #include "v4l2-ctl.h"
 
+#ifdef __clang__
+#define FALLTHROUGH [[clang::fallthrough]]
+#else
+#define FALLTHROUGH [[gnu::fallthrough]]
+#endif
+
 static struct v4l2_format sliced_fmt;	  /* set_format/get_format for sliced VBI */
 static struct v4l2_format sliced_fmt_out; /* set_format/get_format for sliced VBI output */
 static struct v4l2_format raw_fmt;	  /* set_format/get_format for VBI */
@@ -87,7 +93,7 @@ void vbi_cmd(int ch, char *optarg)
 	case OptSetSlicedVbiOutFormat:
 	case OptTrySlicedVbiOutFormat:
 		sliced = &sliced_fmt_out;
-		/* fall through */
+		FALLTHROUGH;
 	case OptSetSlicedVbiFormat:
 	case OptTrySlicedVbiFormat:
 		sliced->fmt.sliced.service_set = 0;
@@ -130,7 +136,7 @@ void vbi_cmd(int ch, char *optarg)
 	case OptSetVbiOutFormat:
 	case OptTryVbiOutFormat:
 		raw = &raw_fmt_out;
-		/* fall through */
+		FALLTHROUGH;
 	case OptSetVbiFormat:
 	case OptTryVbiFormat:
 		subs = optarg;
-- 
2.25.2

[PATCH 11/12] utils: fix double promotions

[Rosen Penev]

Found with -Wdouble-promotion

Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 utils/rds-ctl/rds-ctl.cpp                   | 4 ++--
 utils/v4l2-compliance/v4l2-test-buffers.cpp | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/utils/rds-ctl/rds-ctl.cpp b/utils/rds-ctl/rds-ctl.cpp
index cc1d3bf7..cd6ece38 100644
--- a/utils/rds-ctl/rds-ctl.cpp
+++ b/utils/rds-ctl/rds-ctl.cpp
@@ -528,7 +528,7 @@ static void print_rds_statistics(const struct v4l2_rds_statistics *statistics)
 			(static_cast<float>(statistics->block_error_cnt) / statistics->block_cnt) * 100.0f;
 	printf("block errors / group errors: %u (%3.2f%%) / %u \n",
 		statistics->block_error_cnt,
-		block_error_percentage, statistics->group_error_cnt);
+		(double)block_error_percentage, statistics->group_error_cnt);
 
 	float block_corrected_percentage = 0;
 
@@ -536,7 +536,7 @@ static void print_rds_statistics(const struct v4l2_rds_statistics *statistics)
 		block_corrected_percentage = (
 			static_cast<float>(statistics->block_corrected_cnt) / statistics->block_cnt) * 100.0f;
 	printf("corrected blocks: %u (%3.2f%%)\n",
-		statistics->block_corrected_cnt, block_corrected_percentage);
+		statistics->block_corrected_cnt, (double)block_corrected_percentage);
 	for (int i = 0; i < 16; i++)
 		printf("Group %02d: %u\n", i, statistics->group_type_cnt[i]);
 }
diff --git a/utils/v4l2-compliance/v4l2-test-buffers.cpp b/utils/v4l2-compliance/v4l2-test-buffers.cpp
index 87c2e523..abf39cc7 100644
--- a/utils/v4l2-compliance/v4l2-test-buffers.cpp
+++ b/utils/v4l2-compliance/v4l2-test-buffers.cpp
@@ -2597,7 +2597,7 @@ static void streamFmt(struct node *node, __u32 pixelformat, __u32 w, __u32 h,
 	char hz[32] = "";
 
 	if (!frame_count)
-		frame_count = f ? (unsigned)(1.0f / fract2f(f)) : 10;
+		frame_count = f ? (unsigned)(1.0 / fract2f(f)) : 10;
 	node->g_fmt(fmt);
 	fmt.s_pixelformat(pixelformat);
 	fmt.s_width(w);
-- 
2.25.2

[PATCH 12/12] utils: fix wrong format

[Rosen Penev]

Found with -Wformat-pedantic

Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 utils/v4l2-compliance/v4l2-test-formats.cpp | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/utils/v4l2-compliance/v4l2-test-formats.cpp b/utils/v4l2-compliance/v4l2-test-formats.cpp
index 9e98cf99..390702bc 100644
--- a/utils/v4l2-compliance/v4l2-test-formats.cpp
+++ b/utils/v4l2-compliance/v4l2-test-formats.cpp
@@ -664,10 +664,10 @@ static bool matchFormats(const struct v4l2_format &f1, const struct v4l2_format
 			return true;
 		printf("\t\tG_FMT:     %dx%d@%dx%d, %d, %x, %p, %d, %p, %x\n",
 			win1.w.width, win1.w.height, win1.w.left, win1.w.top, win1.field,
-			win1.chromakey, win1.clips, win1.clipcount, win1.bitmap, win1.global_alpha);
+			win1.chromakey, (void *)win1.clips, win1.clipcount, win1.bitmap, win1.global_alpha);
 		printf("\t\tTRY/S_FMT: %dx%d@%dx%d, %d, %x, %p, %d, %p, %x\n",
 			win2.w.width, win2.w.height, win2.w.left, win2.w.top, win2.field,
-			win2.chromakey, win2.clips, win2.clipcount, win2.bitmap, win2.global_alpha);
+			win2.chromakey, (void *)win2.clips, win2.clipcount, win2.bitmap, win2.global_alpha);
 		return false;
 	case V4L2_BUF_TYPE_VBI_CAPTURE:
 	case V4L2_BUF_TYPE_VBI_OUTPUT:
-- 
2.25.2

Re: [PATCH 01/12] utils: fix compilation with C++98

[Hans Verkuil]

On 22/04/2020 02:37, Rosen Penev wrote:
> Signed-off-by: Rosen Penev <rosenp@gmail.com>
> ---
>  utils/cec-compliance/cec-test.cpp | 4 ++--
>  utils/rds-ctl/rds-ctl.cpp         | 2 +-
>  2 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/utils/cec-compliance/cec-test.cpp b/utils/cec-compliance/cec-test.cpp
> index 032ff5ad..cc07998a 100644
> --- a/utils/cec-compliance/cec-test.cpp
> +++ b/utils/cec-compliance/cec-test.cpp
> @@ -908,7 +908,7 @@ static int tuner_ctl_test(struct node *node, unsigned me, unsigned la, bool inte
>  		cec_msg_give_tuner_device_status(&msg, true, CEC_OP_STATUS_REQ_ONCE);
>  		fail_on_test(!transmit_timeout(node, &msg));
>  		fail_on_test(timed_out_or_abort(&msg));
> -		info = {};
> +		memset(&info, 0, sizeof(info));
>  		cec_ops_tuner_device_status(&msg, &info);
>  		if (!memcmp(&info, &info_vec[0], sizeof(info)))
>  			break;
> @@ -935,7 +935,7 @@ static int tuner_ctl_test(struct node *node, unsigned me, unsigned la, bool inte
>  		cec_msg_give_tuner_device_status(&msg, true, CEC_OP_STATUS_REQ_ONCE);
>  		fail_on_test(!transmit_timeout(node, &msg));
>  		fail_on_test(timed_out_or_abort(&msg));
> -		info = {};
> +		memset(&info, 0, sizeof(info));
>  		cec_ops_tuner_device_status(&msg, &info);
>  		if (memcmp(&info, &(*iter), sizeof(info))) {
>  			log_tuner_service(info);
> diff --git a/utils/rds-ctl/rds-ctl.cpp b/utils/rds-ctl/rds-ctl.cpp
> index 0511fe5d..0e497b2a 100644
> --- a/utils/rds-ctl/rds-ctl.cpp
> +++ b/utils/rds-ctl/rds-ctl.cpp
> @@ -918,7 +918,7 @@ static void get_options(const int fd, const int capabilities, struct v4l2_freque
>  				printf("\tFrequency range      : %.1f MHz - %.1f MHz\n",
>  					 vt.rangelow / 16.0, vt.rangehigh / 16.0);
>  			printf("\tSignal strength/AFC  : %ld%%/%d\n",
> -				std::lround(vt.signal / 655.35), vt.afc);
> +				lround(vt.signal / 655.25), vt.afc);

v4l2-ctl-tuner.cpp also uses std::lround, but that one isn't changed back.

Is rds-ctl.cpp perhaps just missing a header?

Regards,

	Hans

>  			printf("\tCurrent audio mode   : %s\n", audmode2s(vt.audmode));
>  			printf("\tAvailable subchannels: %s\n",
>  					rxsubchans2s(vt.rxsubchans).c_str());
> 

Re: [PATCH 01/12] utils: fix compilation with C++98

[Rosen Penev]

> On Apr 23, 2020, at 1:19 AM, Hans Verkuil <hverkuil@xs4all.nl> wrote:
> 
> On 22/04/2020 02:37, Rosen Penev wrote:
>> Signed-off-by: Rosen Penev <rosenp@gmail.com>
>> ---
>> utils/cec-compliance/cec-test.cpp | 4 ++--
>> utils/rds-ctl/rds-ctl.cpp         | 2 +-
>> 2 files changed, 3 insertions(+), 3 deletions(-)
>> 
>> diff --git a/utils/cec-compliance/cec-test.cpp b/utils/cec-compliance/cec-test.cpp
>> index 032ff5ad..cc07998a 100644
>> --- a/utils/cec-compliance/cec-test.cpp
>> +++ b/utils/cec-compliance/cec-test.cpp
>> @@ -908,7 +908,7 @@ static int tuner_ctl_test(struct node *node, unsigned me, unsigned la, bool inte
>>        cec_msg_give_tuner_device_status(&msg, true, CEC_OP_STATUS_REQ_ONCE);
>>        fail_on_test(!transmit_timeout(node, &msg));
>>        fail_on_test(timed_out_or_abort(&msg));
>> -        info = {};
>> +        memset(&info, 0, sizeof(info));
>>        cec_ops_tuner_device_status(&msg, &info);
>>        if (!memcmp(&info, &info_vec[0], sizeof(info)))
>>            break;
>> @@ -935,7 +935,7 @@ static int tuner_ctl_test(struct node *node, unsigned me, unsigned la, bool inte
>>        cec_msg_give_tuner_device_status(&msg, true, CEC_OP_STATUS_REQ_ONCE);
>>        fail_on_test(!transmit_timeout(node, &msg));
>>        fail_on_test(timed_out_or_abort(&msg));
>> -        info = {};
>> +        memset(&info, 0, sizeof(info));
>>        cec_ops_tuner_device_status(&msg, &info);
>>        if (memcmp(&info, &(*iter), sizeof(info))) {
>>            log_tuner_service(info);
>> diff --git a/utils/rds-ctl/rds-ctl.cpp b/utils/rds-ctl/rds-ctl.cpp
>> index 0511fe5d..0e497b2a 100644
>> --- a/utils/rds-ctl/rds-ctl.cpp
>> +++ b/utils/rds-ctl/rds-ctl.cpp
>> @@ -918,7 +918,7 @@ static void get_options(const int fd, const int capabilities, struct v4l2_freque
>>                printf("\tFrequency range      : %.1f MHz - %.1f MHz\n",
>>                     vt.rangelow / 16.0, vt.rangehigh / 16.0);
>>            printf("\tSignal strength/AFC  : %ld%%/%d\n",
>> -                std::lround(vt.signal / 655.35), vt.afc);
>> +                lround(vt.signal / 655.25), vt.afc);
> 
> v4l2-ctl-tuner.cpp also uses std::lround, but that one isn't changed back.
> 
> Is rds-ctl.cpp perhaps just missing a header?
Nope. std::lround is C++11.
> 
> Regards,
> 
>    Hans
> 
>>            printf("\tCurrent audio mode   : %s\n", audmode2s(vt.audmode));
>>            printf("\tAvailable subchannels: %s\n",
>>                    rxsubchans2s(vt.rxsubchans).c_str());
>> 
> 

Re: [PATCH 04/12] utils: don't check unsigned for < 0.

[Hans Verkuil]

On 22/04/2020 02:37, Rosen Penev wrote:
> Found with -Wtautological-unsigned-zero-compare
> 
> Signed-off-by: Rosen Penev <rosenp@gmail.com>
> ---
>  utils/v4l2-compliance/v4l2-test-controls.cpp | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/utils/v4l2-compliance/v4l2-test-controls.cpp b/utils/v4l2-compliance/v4l2-test-controls.cpp
> index 8c4480be..251a6049 100644
> --- a/utils/v4l2-compliance/v4l2-test-controls.cpp
> +++ b/utils/v4l2-compliance/v4l2-test-controls.cpp
> @@ -80,8 +80,6 @@ static int checkQCtrl(struct node *node, struct test_query_ext_ctrl &qctrl)
>  			return fail("min > max\n");
>  		if (qctrl.step == 0)
>  			return fail("step == 0\n");
> -		if (qctrl.step < 0)
> -			return fail("step < 0\n");

Ah, nice. This is actually a bug since this test needs to be done for
struct v4l2_queryctl (where step is signed) instead of struct v4l2_query_ext_ctrl
(where step is unsigned).

I've made a patch fixing this correctly.

Regards,

	Hans

>  		if (static_cast<unsigned>(qctrl.step) > static_cast<unsigned>(qctrl.maximum - qctrl.minimum) &&
>  		    qctrl.maximum != qctrl.minimum)
>  			return fail("step > max - min\n");
> 

Re: [PATCH 04/12] utils: don't check unsigned for < 0.

[Rosen Penev]

> On Apr 23, 2020, at 1:30 AM, Hans Verkuil <hverkuil@xs4all.nl> wrote:
> 
> On 22/04/2020 02:37, Rosen Penev wrote:
>> Found with -Wtautological-unsigned-zero-compare
>> 
>> Signed-off-by: Rosen Penev <rosenp@gmail.com>
>> ---
>> utils/v4l2-compliance/v4l2-test-controls.cpp | 2 --
>> 1 file changed, 2 deletions(-)
>> 
>> diff --git a/utils/v4l2-compliance/v4l2-test-controls.cpp b/utils/v4l2-compliance/v4l2-test-controls.cpp
>> index 8c4480be..251a6049 100644
>> --- a/utils/v4l2-compliance/v4l2-test-controls.cpp
>> +++ b/utils/v4l2-compliance/v4l2-test-controls.cpp
>> @@ -80,8 +80,6 @@ static int checkQCtrl(struct node *node, struct test_query_ext_ctrl &qctrl)
>>            return fail("min > max\n");
>>        if (qctrl.step == 0)
>>            return fail("step == 0\n");
>> -        if (qctrl.step < 0)
>> -            return fail("step < 0\n");
> 
> Ah, nice. This is actually a bug since this test needs to be done for
> struct v4l2_queryctl (where step is signed) instead of struct v4l2_query_ext_ctrl
> (where step is unsigned).
> 
> I've made a patch fixing this correctly.
Sounds good.
> 
> Regards,
> 
>    Hans
> 
>>        if (static_cast<unsigned>(qctrl.step) > static_cast<unsigned>(qctrl.maximum - qctrl.minimum) &&
>>            qctrl.maximum != qctrl.minimum)
>>            return fail("step > max - min\n");
>> 
> 

Re: [PATCH 06/12] utils: add noreturn attribute and remove dead code

[Hans Verkuil]

On 22/04/2020 02:37, Rosen Penev wrote:
> Found with -Wmissing-noreturn and -Wunreachable-code-return
> 
> Signed-off-by: Rosen Penev <rosenp@gmail.com>
> ---
>  utils/v4l2-compliance/v4l2-compliance.cpp | 9 ++-------
>  utils/v4l2-dbg/v4l2-dbg.cpp               | 7 +------
>  2 files changed, 3 insertions(+), 13 deletions(-)
> 
> diff --git a/utils/v4l2-compliance/v4l2-compliance.cpp b/utils/v4l2-compliance/v4l2-compliance.cpp
> index b3a18492..fbf34914 100644
> --- a/utils/v4l2-compliance/v4l2-compliance.cpp
> +++ b/utils/v4l2-compliance/v4l2-compliance.cpp
> @@ -155,6 +155,7 @@ static struct option long_options[] = {
>  	{0, 0, 0, 0}
>  };
>  
> +__attribute__((noreturn))
>  static void usage()
>  {
>  	printf("Usage:\n");

I think usage() shouldn't exit, instead leave that to the caller.

> @@ -482,6 +483,7 @@ static void restoreState()
>  	node->reopen();
>  }
>  
> +__attribute__((noreturn))
>  static void signal_handler_interrupt(int signum)
>  {
>  	restoreState();
> @@ -1544,7 +1546,6 @@ int main(int argc, char **argv)
>  		switch (ch) {
>  		case OptHelp:
>  			usage();
> -			return 0;

So instead of doing 'return 0;', do 'exit(0);' here and exit(1) elsewhere.

Clearly in many cases I already assumed that usage() would return, so let's
change usage() accordingly.

Same for v4l2-dbg.cpp.

Regards,

	Hans

>  		case OptSetDevice:
>  			device = make_devname(optarg, "video", media_bus_info);
>  			break;
> @@ -1619,7 +1620,6 @@ int main(int argc, char **argv)
>  						color_component = 2;
>  					else {
>  						usage();
> -						exit(1);
>  					}
>  					break;
>  				case 1:
> @@ -1634,7 +1634,6 @@ int main(int argc, char **argv)
>  					break;
>  				default:
>  					usage();
> -					exit(1);
>  				}
>  			}
>  			break;
> @@ -1647,7 +1646,6 @@ int main(int argc, char **argv)
>  				show_colors = isatty(STDOUT_FILENO);
>  			else {
>  				usage();
> -				exit(1);
>  			}
>  			break;
>  		case OptNoWarnings:
> @@ -1669,13 +1667,11 @@ int main(int argc, char **argv)
>  			fprintf(stderr, "Option `%s' requires a value\n",
>  				argv[optind]);
>  			usage();
> -			return 1;
>  		case '?':
>  			if (argv[optind])
>  				fprintf(stderr, "Unknown argument `%s'\n",
>  					argv[optind]);
>  			usage();
> -			return 1;
>  		}
>  	}
>  	if (optind < argc) {
> @@ -1684,7 +1680,6 @@ int main(int argc, char **argv)
>  			fprintf(stderr, "%s ", argv[optind++]);
>  		fprintf(stderr, "\n");
>  		usage();
> -		return 1;
>  	}
>  	bool direct = !options[OptUseWrapper];
>  	int fd;
> diff --git a/utils/v4l2-dbg/v4l2-dbg.cpp b/utils/v4l2-dbg/v4l2-dbg.cpp
> index cd387d1d..7b986a50 100644
> --- a/utils/v4l2-dbg/v4l2-dbg.cpp
> +++ b/utils/v4l2-dbg/v4l2-dbg.cpp
> @@ -162,6 +162,7 @@ static struct option long_options[] = {
>  	{0, 0, 0, 0}
>  };
>  
> +__attribute__((noreturn))
>  static void usage()
>  {
>  	printf("Usage: v4l2-dbg [options] [values]\n"
> @@ -387,13 +388,11 @@ static int parse_subopt(char **subs, const char * const *subopts, char **value)
>  	if (opt == -1) {
>  		fprintf(stderr, "Invalid suboptions specified\n");
>  		usage();
> -		exit(1);
>  	}
>  	if (*value == NULL) {
>  		fprintf(stderr, "No value given to suboption <%s>\n",
>  				subopts[opt]);
>  		usage();
> -		exit(1);
>  	}
>  	return opt;
>  }
> @@ -429,7 +428,6 @@ int main(int argc, char **argv)
>  
>  	if (argc == 1) {
>  		usage();
> -		return 0;
>  	}
>  	for (i = 0; long_options[i].name; i++) {
>  		if (!isalpha(long_options[i].val))
> @@ -467,7 +465,6 @@ int main(int argc, char **argv)
>  		switch (ch) {
>  		case OptHelp:
>  			usage();
> -			return 0;
>  
>  		case OptSetDevice:
>  			device = optarg;
> @@ -538,13 +535,11 @@ int main(int argc, char **argv)
>  			fprintf(stderr, "Option `%s' requires a value\n",
>  				argv[optind]);
>  			usage();
> -			return 1;
>  
>  		case '?':
>  			fprintf(stderr, "Unknown argument `%s'\n",
>  				argv[optind]);
>  			usage();
> -			return 1;
>  		}
>  	}
>  
> 

Re: [PATCH 09/12] utils: fix implicit float conversions

[Hans Verkuil]

On 22/04/2020 02:37, Rosen Penev wrote:
> Found with -Wfloat-conversion
> 
> Signed-off-by: Rosen Penev <rosenp@gmail.com>
> ---
>  utils/cec-ctl/cec-ctl.cpp                   | 4 ++--
>  utils/cec-ctl/cec-pin.cpp                   | 2 +-
>  utils/v4l2-compliance/v4l2-test-buffers.cpp | 2 +-
>  utils/v4l2-ctl/v4l2-ctl-misc.cpp            | 4 ++--
>  utils/v4l2-ctl/v4l2-ctl-streaming.cpp       | 2 +-
>  utils/v4l2-ctl/v4l2-ctl-subdev.cpp          | 2 +-
>  6 files changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/utils/cec-ctl/cec-ctl.cpp b/utils/cec-ctl/cec-ctl.cpp
> index f4133494..f0e31aca 100644
> --- a/utils/cec-ctl/cec-ctl.cpp
> +++ b/utils/cec-ctl/cec-ctl.cpp
> @@ -1507,8 +1507,8 @@ static void stress_test_power_cycle(struct node &node, unsigned cnt,
>  	srandom(seed);
>  
>  	for (;;) {
> -		unsigned usecs1 = mod_usleep ? random() % mod_usleep : sleep_before_on * 1000000;
> -		unsigned usecs2 = mod_usleep ? random() % mod_usleep : sleep_before_off * 1000000;
> +		unsigned usecs1 = mod_usleep ? random() % mod_usleep : (unsigned)(sleep_before_on * 1000000);
> +		unsigned usecs2 = mod_usleep ? random() % mod_usleep : (unsigned)(sleep_before_off * 1000000);

Shouldn't this be static_cast<unsigned>? Same elsewhere.

Regards,

	Hans

>  
>  		usecs1 += min_usleep;
>  		usecs2 += min_usleep;
> diff --git a/utils/cec-ctl/cec-pin.cpp b/utils/cec-ctl/cec-pin.cpp
> index 0322ad5e..10abea37 100644
> --- a/utils/cec-ctl/cec-pin.cpp
> +++ b/utils/cec-ctl/cec-pin.cpp
> @@ -261,7 +261,7 @@ static void cec_pin_rx_data_bit_was_low(__u64 ev_ts, __u64 usecs, __u64 usecs_mi
>  	 * If the low drive starts at the end of a 0 bit, then the actual
>  	 * maximum time that the bus can be low is the two summed.
>  	 */
> -	const unsigned max_low_drive = CEC_TIM_LOW_DRIVE_ERROR_MAX +
> +	const unsigned max_low_drive = (unsigned)CEC_TIM_LOW_DRIVE_ERROR_MAX +
>  		CEC_TIM_DATA_BIT_0_LOW_MAX + CEC_TIM_MARGIN;
>  
>  	low_usecs = usecs;
> diff --git a/utils/v4l2-compliance/v4l2-test-buffers.cpp b/utils/v4l2-compliance/v4l2-test-buffers.cpp
> index 93df7034..87c2e523 100644
> --- a/utils/v4l2-compliance/v4l2-test-buffers.cpp
> +++ b/utils/v4l2-compliance/v4l2-test-buffers.cpp
> @@ -2597,7 +2597,7 @@ static void streamFmt(struct node *node, __u32 pixelformat, __u32 w, __u32 h,
>  	char hz[32] = "";
>  
>  	if (!frame_count)
> -		frame_count = f ? 1.0 / fract2f(f) : 10;
> +		frame_count = f ? (unsigned)(1.0f / fract2f(f)) : 10;
>  	node->g_fmt(fmt);
>  	fmt.s_pixelformat(pixelformat);
>  	fmt.s_width(w);
> diff --git a/utils/v4l2-ctl/v4l2-ctl-misc.cpp b/utils/v4l2-ctl/v4l2-ctl-misc.cpp
> index cb933217..2851886a 100644
> --- a/utils/v4l2-ctl/v4l2-ctl-misc.cpp
> +++ b/utils/v4l2-ctl/v4l2-ctl-misc.cpp
> @@ -320,7 +320,7 @@ void misc_set(cv4l_fd &_fd)
>  		parm.type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
>  		parm.parm.capture.timeperframe.numerator = 1000;
>  		parm.parm.capture.timeperframe.denominator =
> -			fps * parm.parm.capture.timeperframe.numerator;
> +			(uint32_t)(fps * parm.parm.capture.timeperframe.numerator);
>  
>  		if (doioctl(fd, VIDIOC_S_PARM, &parm) == 0) {
>  			struct v4l2_fract *tf = &parm.parm.capture.timeperframe;
> @@ -338,7 +338,7 @@ void misc_set(cv4l_fd &_fd)
>  		parm.type = V4L2_BUF_TYPE_VIDEO_OUTPUT;
>  		parm.parm.output.timeperframe.numerator = 1000;
>  		parm.parm.output.timeperframe.denominator =
> -			output_fps * parm.parm.output.timeperframe.numerator;
> +			(uint32_t)(output_fps * parm.parm.output.timeperframe.numerator);
>  
>  		if (doioctl(fd, VIDIOC_S_PARM, &parm) == 0) {
>  			struct v4l2_fract *tf = &parm.parm.output.timeperframe;
> diff --git a/utils/v4l2-ctl/v4l2-ctl-streaming.cpp b/utils/v4l2-ctl/v4l2-ctl-streaming.cpp
> index 066a336a..6981a3cc 100644
> --- a/utils/v4l2-ctl/v4l2-ctl-streaming.cpp
> +++ b/utils/v4l2-ctl/v4l2-ctl-streaming.cpp
> @@ -569,7 +569,7 @@ static void print_concise_buffer(FILE *f, cv4l_buffer &buf, cv4l_fmt &fmt,
>  	if (!skip_ts && (buf.g_flags() & V4L2_BUF_FLAG_TIMESTAMP_MASK) != V4L2_BUF_FLAG_TIMESTAMP_COPY) {
>  		double ts = buf.g_timestamp().tv_sec + buf.g_timestamp().tv_usec / 1000000.0;
>  		fprintf(f, " ts: %.06f", ts);
> -		if (last_ts)
> +		if (last_ts <= 0.0)
>  			fprintf(f, " delta: %.03f ms", (ts - last_ts) * 1000.0);
>  		last_ts = ts;
>  
> diff --git a/utils/v4l2-ctl/v4l2-ctl-subdev.cpp b/utils/v4l2-ctl/v4l2-ctl-subdev.cpp
> index 747f1699..f1223084 100644
> --- a/utils/v4l2-ctl/v4l2-ctl-subdev.cpp
> +++ b/utils/v4l2-ctl/v4l2-ctl-subdev.cpp
> @@ -487,7 +487,7 @@ void subdev_set(cv4l_fd &_fd)
>  			exit(1);
>  		}
>  		fival.interval.numerator = 1000;
> -		fival.interval.denominator = set_fps * fival.interval.numerator;
> +		fival.interval.denominator = (uint32_t)(set_fps * fival.interval.numerator);
>  		printf("Note: --set-subdev-fps is only for testing.\n"
>  		       "Normally media-ctl is used to configure the video pipeline.\n");
>  		printf("ioctl: VIDIOC_SUBDEV_S_FRAME_INTERVAL (pad=%u)\n", fival.pad);
> 

Re: [PATCH 10/12] utils: fix fallthrough warnings

[Hans Verkuil]

On 22/04/2020 02:37, Rosen Penev wrote:
> Comments are not enough for clang. Changed to use the C++ attribute.
> 
> Found with -Wimplicit-fallthrough
> 
> Signed-off-by: Rosen Penev <rosenp@gmail.com>
> ---
>  utils/cec-compliance/cec-compliance.cpp      |  7 +++++++
>  utils/cec-ctl/cec-ctl.cpp                    |  8 +++++++-
>  utils/cec-follower/cec-follower.cpp          |  7 +++++++
>  utils/cec-follower/cec-processing.cpp        |  9 ++++++++-
>  utils/cec-follower/cec-tuner.cpp             | 12 +++++++++---
>  utils/libcecutil/cec-log.cpp                 |  8 +++++++-
>  utils/v4l2-compliance/v4l2-test-colors.cpp   | 16 +++++++++++-----
>  utils/v4l2-compliance/v4l2-test-controls.cpp | 16 +++++++++++-----
>  utils/v4l2-compliance/v4l2-test-formats.cpp  | 10 ++++++++--
>  utils/v4l2-ctl/v4l2-ctl-streaming.cpp        | 10 ++++++++--
>  utils/v4l2-ctl/v4l2-ctl-vbi.cpp              | 10 ++++++++--
>  11 files changed, 91 insertions(+), 22 deletions(-)
> 
> diff --git a/utils/cec-compliance/cec-compliance.cpp b/utils/cec-compliance/cec-compliance.cpp
> index 1c62b5c9..e9fce337 100644
> --- a/utils/cec-compliance/cec-compliance.cpp
> +++ b/utils/cec-compliance/cec-compliance.cpp
> @@ -24,6 +24,12 @@
>  #include "version.h"
>  #endif
>  
> +#ifdef __clang__
> +#define FALLTHROUGH [[clang::fallthrough]]
> +#else
> +#define FALLTHROUGH [[gnu::fallthrough]]
> +#endif

Yuck, no uppercase please.

I am OK with lowercase 'fallthrough', since that's what the linux kernel settled on
as well.

This #ifdef/#else/#endif is replicated in several sources, so let's put this in a
header instead.

Regards,

	Hans

> +
>  /* Short option list
>  
>     Please keep in alphabetical order.
> @@ -486,6 +492,7 @@ void sad_decode(struct short_audio_desc *sad, __u32 descriptor)
>  		case SAD_EXT_TYPE_MPEG_H_3D_AUDIO:
>  		case SAD_EXT_TYPE_AC_4:
>  			sad->format_dependent = b3 & 0x07;
> +			FALLTHROUGH;
>  		case SAD_EXT_TYPE_LPCM_3D_AUDIO:
>  			sad->bit_depth_mask = b3 & 0x07;
>  			break;
> diff --git a/utils/cec-ctl/cec-ctl.cpp b/utils/cec-ctl/cec-ctl.cpp
> index f0e31aca..a3698a67 100644
> --- a/utils/cec-ctl/cec-ctl.cpp
> +++ b/utils/cec-ctl/cec-ctl.cpp
> @@ -37,6 +37,12 @@
>  
>  #include "cec-ctl.h"
>  
> +#ifdef __clang__
> +#define FALLTHROUGH [[clang::fallthrough]]
> +#else
> +#define FALLTHROUGH [[gnu::fallthrough]]
> +#endif
> +
>  static struct timespec start_monotonic;
>  static struct timeval start_timeofday;
>  static bool ignore_la[16];
> @@ -1967,7 +1973,7 @@ int main(int argc, char **argv)
>  			break;
>  		case OptPhysAddrFromEDIDPoll:
>  			edid_path = optarg;
> -			/* fall-through */
> +			FALLTHROUGH;
>  		case OptPhysAddrFromEDID:
>  			phys_addr = parse_phys_addr_from_edid(optarg);
>  			break;
> diff --git a/utils/cec-follower/cec-follower.cpp b/utils/cec-follower/cec-follower.cpp
> index 1eeae2d4..2d61d5f9 100644
> --- a/utils/cec-follower/cec-follower.cpp
> +++ b/utils/cec-follower/cec-follower.cpp
> @@ -23,6 +23,12 @@
>  #include "version.h"
>  #endif
>  
> +#ifdef __clang__
> +#define FALLTHROUGH [[clang::fallthrough]]
> +#else
> +#define FALLTHROUGH [[gnu::fallthrough]]
> +#endif
> +
>  /* Short option list
>  
>     Please keep in alphabetical order.
> @@ -142,6 +148,7 @@ void sad_encode(const struct short_audio_desc *sad, __u32 *descriptor)
>  		case SAD_EXT_TYPE_MPEG_H_3D_AUDIO:
>  		case SAD_EXT_TYPE_AC_4:
>  			b3 |= sad->format_dependent & 0x07;
> +			FALLTHROUGH;
>  		case SAD_EXT_TYPE_LPCM_3D_AUDIO:
>  			b3 |= sad->bit_depth_mask & 0x07;
>  			break;
> diff --git a/utils/cec-follower/cec-processing.cpp b/utils/cec-follower/cec-processing.cpp
> index 8e3a33a2..68bfdca3 100644
> --- a/utils/cec-follower/cec-processing.cpp
> +++ b/utils/cec-follower/cec-processing.cpp
> @@ -19,6 +19,12 @@
>  
>  #include "cec-follower.h"
>  
> +#ifdef __clang__
> +#define FALLTHROUGH [[clang::fallthrough]]
> +#else
> +#define FALLTHROUGH [[gnu::fallthrough]]
> +#endif
> +
>  #define VOLUME_MAX 0x64
>  #define VOLUME_MIN 0
>  
> @@ -352,7 +358,7 @@ static void processMsg(struct node *node, struct cec_msg &msg, unsigned me)
>  		if (cec_has_tv(1 << la) && la_info[la].phys_addr == 0)
>  			warn("TV (0) at 0.0.0.0 sent Routing Information.");
>  	}
> -
> +	FALLTHROUGH;
>  
>  		/* System Information */
>  
> @@ -708,6 +714,7 @@ static void processMsg(struct node *node, struct cec_msg &msg, unsigned me)
>  		cec_msg_system_audio_mode_status(&msg, node->state.sac_active ? CEC_OP_SYS_AUD_STATUS_ON :
>  						 CEC_OP_SYS_AUD_STATUS_OFF);
>  		transmit(node, &msg);
> +		FALLTHROUGH;
>  	case CEC_MSG_GIVE_AUDIO_STATUS:
>  		if (!cec_has_audiosystem(1 << me))
>  			break;
> diff --git a/utils/cec-follower/cec-tuner.cpp b/utils/cec-follower/cec-tuner.cpp
> index aa19f55d..af7c6437 100644
> --- a/utils/cec-follower/cec-tuner.cpp
> +++ b/utils/cec-follower/cec-tuner.cpp
> @@ -8,6 +8,12 @@
>  
>  #include "cec-follower.h"
>  
> +#ifdef __clang__
> +#define FALLTHROUGH [[clang::fallthrough]]
> +#else
> +#define FALLTHROUGH [[gnu::fallthrough]]
> +#endif
> +
>  #define NUM_ANALOG_FREQS 3
>  #define NUM_DIGITAL_CHANS 3
>  #define TOT_ANALOG_FREQS (sizeof(analog_freqs_khz) / sizeof(analog_freqs_khz[0][0][0]))
> @@ -303,21 +309,21 @@ static int digital_get_service_idx(struct cec_op_digital_service_id *digital)
>  	switch (digital->dig_bcast_system) {
>  	case CEC_OP_DIG_SERVICE_BCAST_SYSTEM_ARIB_T:
>  		is_terrestrial = true;
> -		/* fall through */
> +		FALLTHROUGH;
>  	case CEC_OP_DIG_SERVICE_BCAST_SYSTEM_ARIB_BS:
>  		info = &digital_arib_data[is_terrestrial][0];
>  		offset = is_terrestrial * NUM_DIGITAL_CHANS;
>  		break;
>  	case CEC_OP_DIG_SERVICE_BCAST_SYSTEM_ATSC_T:
>  		is_terrestrial = true;
> -		/* fall through */
> +		FALLTHROUGH;
>  	case CEC_OP_DIG_SERVICE_BCAST_SYSTEM_ATSC_SAT:
>  		info = &digital_atsc_data[is_terrestrial][0];
>  		offset = (2 + is_terrestrial) * NUM_DIGITAL_CHANS;
>  		break;
>  	case CEC_OP_DIG_SERVICE_BCAST_SYSTEM_DVB_T:
>  		is_terrestrial = true;
> -		/* fall through */
> +		FALLTHROUGH;
>  	case CEC_OP_DIG_SERVICE_BCAST_SYSTEM_DVB_S2:
>  		info = &digital_dvb_data[is_terrestrial][0];
>  		offset = (4 + is_terrestrial) * NUM_DIGITAL_CHANS;
> diff --git a/utils/libcecutil/cec-log.cpp b/utils/libcecutil/cec-log.cpp
> index 28a1ecac..9844ac43 100644
> --- a/utils/libcecutil/cec-log.cpp
> +++ b/utils/libcecutil/cec-log.cpp
> @@ -15,6 +15,12 @@
>  #include "cec-info.h"
>  #include "cec-log.h"
>  
> +#ifdef __clang__
> +#define FALLTHROUGH [[clang::fallthrough]]
> +#else
> +#define FALLTHROUGH [[gnu::fallthrough]]
> +#endif
> +
>  static const struct cec_arg arg_u8 = {
>  	CEC_ARG_TYPE_U8,
>  };
> @@ -44,7 +50,7 @@ static void log_arg(const struct cec_arg *arg, const char *arg_name, __u32 val)
>  				return;
>  			}
>  		}
> -		/* fall through */
> +		FALLTHROUGH;
>  	case CEC_ARG_TYPE_U8:
>  		if (!strcmp(arg_name, "video-latency") ||
>  		    !strcmp(arg_name, "audio-out-delay")) {
> diff --git a/utils/v4l2-compliance/v4l2-test-colors.cpp b/utils/v4l2-compliance/v4l2-test-colors.cpp
> index 887b198b..d37300e9 100644
> --- a/utils/v4l2-compliance/v4l2-test-colors.cpp
> +++ b/utils/v4l2-compliance/v4l2-test-colors.cpp
> @@ -30,6 +30,12 @@
>  #include <math.h>
>  #include "v4l2-compliance.h"
>  
> +#ifdef __clang__
> +#define FALLTHROUGH [[clang::fallthrough]]
> +#else
> +#define FALLTHROUGH [[gnu::fallthrough]]
> +#endif
> +
>  static void setupPlanes(const cv4l_fmt &fmt, __u8 *planes[3])
>  {
>  	if (fmt.g_num_planes() > 1)
> @@ -328,7 +334,7 @@ static void getColor(const cv4l_fmt &fmt, __u8 * const planes[3],
>  	case V4L2_PIX_FMT_RGBA444:
>  	case V4L2_PIX_FMT_BGRA444:
>  		c.a = (v16 >> 12) / 15.0;
> -		/* fall through */
> +		FALLTHROUGH;
>  	case V4L2_PIX_FMT_RGB444:
>  	case V4L2_PIX_FMT_XRGB444:
>  	case V4L2_PIX_FMT_XBGR444:
> @@ -345,7 +351,7 @@ static void getColor(const cv4l_fmt &fmt, __u8 * const planes[3],
>  	case V4L2_PIX_FMT_ABGR555:
>  	case V4L2_PIX_FMT_BGRA555:
>  		c.a = v16 >> 15;
> -		/* fall through */
> +		FALLTHROUGH;
>  	case V4L2_PIX_FMT_YUV555:
>  	case V4L2_PIX_FMT_RGB555:
>  	case V4L2_PIX_FMT_XRGB555:
> @@ -376,7 +382,7 @@ static void getColor(const cv4l_fmt &fmt, __u8 * const planes[3],
>  	case V4L2_PIX_FMT_RGBA32:
>  	case V4L2_PIX_FMT_BGRA32:
>  		c.a = ((v32 >> 24) & 0xff) / 255.0;
> -		/* fall through */
> +		FALLTHROUGH;
>  	default:
>  		c.r = ((v32 >> 16) & 0xff) / 255.0;
>  		c.g = ((v32 >> 8) & 0xff) / 255.0;
> @@ -444,7 +450,7 @@ static void getColor(const cv4l_fmt &fmt, __u8 * const planes[3],
>  		Y = (Y - 16.0 / 255.0) * 255.0 / 219.0;
>  		cb *= 255.0 / 224.0;
>  		cr *= 255.0 / 224.0;
> -		/* fall through */
> +		FALLTHROUGH;
>  	case V4L2_YCBCR_ENC_601:
>  	default:
>  		ycbcr2rgb(bt601, Y, cb, cr, c);
> @@ -453,7 +459,7 @@ static void getColor(const cv4l_fmt &fmt, __u8 * const planes[3],
>  		Y = (Y - 16.0 / 255.0) * 255.0 / 219.0;
>  		cb *= 255.0 / 224.0;
>  		cr *= 255.0 / 224.0;
> -		/* fall through */
> +		FALLTHROUGH;
>  	case V4L2_YCBCR_ENC_709:
>  		ycbcr2rgb(rec709, Y, cb, cr, c);
>  		break;
> diff --git a/utils/v4l2-compliance/v4l2-test-controls.cpp b/utils/v4l2-compliance/v4l2-test-controls.cpp
> index 251a6049..2de888cf 100644
> --- a/utils/v4l2-compliance/v4l2-test-controls.cpp
> +++ b/utils/v4l2-compliance/v4l2-test-controls.cpp
> @@ -34,6 +34,12 @@
>  
>  #define V4L2_CTRL_CLASS_VIVID 0x00f00000
>  
> +#ifdef __clang__
> +#define FALLTHROUGH [[clang::fallthrough]]
> +#else
> +#define FALLTHROUGH [[gnu::fallthrough]]
> +#endif
> +
>  static int checkQCtrl(struct node *node, struct test_query_ext_ctrl &qctrl)
>  {
>  	struct v4l2_querymenu qmenu;
> @@ -61,20 +67,20 @@ static int checkQCtrl(struct node *node, struct test_query_ext_ctrl &qctrl)
>  	case V4L2_CTRL_TYPE_BOOLEAN:
>  		if (qctrl.maximum > 1)
>  			return fail("invalid boolean max value\n");
> -		/* fall through */
> +		FALLTHROUGH;
>  	case V4L2_CTRL_TYPE_MENU:
>  	case V4L2_CTRL_TYPE_INTEGER_MENU:
>  		if (qctrl.step != 1)
>  			return fail("invalid step value %lld\n", qctrl.step);
>  		if (qctrl.minimum < 0)
>  			return fail("min < 0\n");
> -		/* fall through */
> +		FALLTHROUGH;
>  	case V4L2_CTRL_TYPE_INTEGER:
>  	case V4L2_CTRL_TYPE_INTEGER64:
>  		if (qctrl.default_value < qctrl.minimum ||
>  		    qctrl.default_value > qctrl.maximum)
>  			return fail("def < min || def > max\n");
> -		/* fall through */
> +		FALLTHROUGH;
>  	case V4L2_CTRL_TYPE_STRING:
>  		if (qctrl.minimum > qctrl.maximum)
>  			return fail("min > max\n");
> @@ -116,7 +122,7 @@ static int checkQCtrl(struct node *node, struct test_query_ext_ctrl &qctrl)
>  	case V4L2_CTRL_TYPE_BUTTON:
>  		if ((fl & rw_mask) != V4L2_CTRL_FLAG_WRITE_ONLY)
>  			return fail("button control not write only\n");
> -		/* fall through */
> +		FALLTHROUGH;
>  	case V4L2_CTRL_TYPE_BOOLEAN:
>  	case V4L2_CTRL_TYPE_MENU:
>  	case V4L2_CTRL_TYPE_INTEGER_MENU:
> @@ -124,7 +130,7 @@ static int checkQCtrl(struct node *node, struct test_query_ext_ctrl &qctrl)
>  	case V4L2_CTRL_TYPE_BITMASK:
>  		if (fl & V4L2_CTRL_FLAG_SLIDER)
>  			return fail("slider makes only sense for integer controls\n");
> -		/* fall through */
> +		FALLTHROUGH;
>  	case V4L2_CTRL_TYPE_INTEGER64:
>  	case V4L2_CTRL_TYPE_INTEGER:
>  		if ((fl & rw_mask) == rw_mask)
> diff --git a/utils/v4l2-compliance/v4l2-test-formats.cpp b/utils/v4l2-compliance/v4l2-test-formats.cpp
> index 3fc87316..9e98cf99 100644
> --- a/utils/v4l2-compliance/v4l2-test-formats.cpp
> +++ b/utils/v4l2-compliance/v4l2-test-formats.cpp
> @@ -32,6 +32,12 @@
>  #include <assert.h>
>  #include "v4l2-compliance.h"
>  
> +#ifdef __clang__
> +#define FALLTHROUGH [[clang::fallthrough]]
> +#else
> +#define FALLTHROUGH [[gnu::fallthrough]]
> +#endif
> +
>  static const __u32 buftype2cap[] = {
>  	0,
>  	V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_VIDEO_M2M,
> @@ -97,7 +103,7 @@ static int testEnumFrameIntervals(struct node *node, __u32 pixfmt,
>  		case V4L2_FRMIVAL_TYPE_CONTINUOUS:
>  			if (sw->step.numerator != 1 || sw->step.denominator != 1)
>  				return fail("invalid step for continuous frameinterval\n");
> -			/* fallthrough */
> +			FALLTHROUGH;
>  		case V4L2_FRMIVAL_TYPE_STEPWISE:
>  			if (frmival.index)
>  				return fail("index must be 0 for stepwise/continuous frameintervals\n");
> @@ -183,7 +189,7 @@ static int testEnumFrameSizes(struct node *node, __u32 pixfmt)
>  		case V4L2_FRMSIZE_TYPE_CONTINUOUS:
>  			if (frmsize.stepwise.step_width != 1 || frmsize.stepwise.step_height != 1)
>  				return fail("invalid step_width/height for continuous framesize\n");
> -			/* fallthrough */
> +			FALLTHROUGH;
>  		case V4L2_FRMSIZE_TYPE_STEPWISE:
>  			if (frmsize.index)
>  				return fail("index must be 0 for stepwise/continuous framesizes\n");
> diff --git a/utils/v4l2-ctl/v4l2-ctl-streaming.cpp b/utils/v4l2-ctl/v4l2-ctl-streaming.cpp
> index 6981a3cc..bb464e99 100644
> --- a/utils/v4l2-ctl/v4l2-ctl-streaming.cpp
> +++ b/utils/v4l2-ctl/v4l2-ctl-streaming.cpp
> @@ -24,6 +24,12 @@
>  #include <media-info.h>
>  #include <fwht-ctrls.h>
>  
> +#ifdef __clang__
> +#define FALLTHROUGH [[clang::fallthrough]]
> +#else
> +#define FALLTHROUGH [[gnu::fallthrough]]
> +#endif
> +
>  extern "C" {
>  #include "v4l2-tpg.h"
>  }
> @@ -763,7 +769,7 @@ void streaming_cmd(int ch, char *optarg)
>  		break;
>  	case OptStreamUser:
>  		memory = V4L2_MEMORY_USERPTR;
> -		/* fall through */
> +		FALLTHROUGH;
>  	case OptStreamMmap:
>  		if (optarg) {
>  			reqbufs_count_cap = strtoul(optarg, 0L, 0);
> @@ -776,7 +782,7 @@ void streaming_cmd(int ch, char *optarg)
>  		break;
>  	case OptStreamOutUser:
>  		out_memory = V4L2_MEMORY_USERPTR;
> -		/* fall through */
> +		FALLTHROUGH;
>  	case OptStreamOutMmap:
>  		if (optarg) {
>  			reqbufs_count_out = strtoul(optarg, 0L, 0);
> diff --git a/utils/v4l2-ctl/v4l2-ctl-vbi.cpp b/utils/v4l2-ctl/v4l2-ctl-vbi.cpp
> index c5960f78..82d01b58 100644
> --- a/utils/v4l2-ctl/v4l2-ctl-vbi.cpp
> +++ b/utils/v4l2-ctl/v4l2-ctl-vbi.cpp
> @@ -16,6 +16,12 @@
>  
>  #include "v4l2-ctl.h"
>  
> +#ifdef __clang__
> +#define FALLTHROUGH [[clang::fallthrough]]
> +#else
> +#define FALLTHROUGH [[gnu::fallthrough]]
> +#endif
> +
>  static struct v4l2_format sliced_fmt;	  /* set_format/get_format for sliced VBI */
>  static struct v4l2_format sliced_fmt_out; /* set_format/get_format for sliced VBI output */
>  static struct v4l2_format raw_fmt;	  /* set_format/get_format for VBI */
> @@ -87,7 +93,7 @@ void vbi_cmd(int ch, char *optarg)
>  	case OptSetSlicedVbiOutFormat:
>  	case OptTrySlicedVbiOutFormat:
>  		sliced = &sliced_fmt_out;
> -		/* fall through */
> +		FALLTHROUGH;
>  	case OptSetSlicedVbiFormat:
>  	case OptTrySlicedVbiFormat:
>  		sliced->fmt.sliced.service_set = 0;
> @@ -130,7 +136,7 @@ void vbi_cmd(int ch, char *optarg)
>  	case OptSetVbiOutFormat:
>  	case OptTryVbiOutFormat:
>  		raw = &raw_fmt_out;
> -		/* fall through */
> +		FALLTHROUGH;
>  	case OptSetVbiFormat:
>  	case OptTryVbiFormat:
>  		subs = optarg;
> 

Re: [PATCH 09/12] utils: fix implicit float conversions

[Rosen Penev]

> On Apr 23, 2020, at 1:36 AM, Hans Verkuil <hverkuil@xs4all.nl> wrote:
> 
> On 22/04/2020 02:37, Rosen Penev wrote:
>> Found with -Wfloat-conversion
>> 
>> Signed-off-by: Rosen Penev <rosenp@gmail.com>
>> ---
>> utils/cec-ctl/cec-ctl.cpp                   | 4 ++--
>> utils/cec-ctl/cec-pin.cpp                   | 2 +-
>> utils/v4l2-compliance/v4l2-test-buffers.cpp | 2 +-
>> utils/v4l2-ctl/v4l2-ctl-misc.cpp            | 4 ++--
>> utils/v4l2-ctl/v4l2-ctl-streaming.cpp       | 2 +-
>> utils/v4l2-ctl/v4l2-ctl-subdev.cpp          | 2 +-
>> 6 files changed, 8 insertions(+), 8 deletions(-)
>> 
>> diff --git a/utils/cec-ctl/cec-ctl.cpp b/utils/cec-ctl/cec-ctl.cpp
>> index f4133494..f0e31aca 100644
>> --- a/utils/cec-ctl/cec-ctl.cpp
>> +++ b/utils/cec-ctl/cec-ctl.cpp
>> @@ -1507,8 +1507,8 @@ static void stress_test_power_cycle(struct node &node, unsigned cnt,
>>    srandom(seed);
>> 
>>    for (;;) {
>> -        unsigned usecs1 = mod_usleep ? random() % mod_usleep : sleep_before_on * 1000000;
>> -        unsigned usecs2 = mod_usleep ? random() % mod_usleep : sleep_before_off * 1000000;
>> +        unsigned usecs1 = mod_usleep ? random() % mod_usleep : (unsigned)(sleep_before_on * 1000000);
>> +        unsigned usecs2 = mod_usleep ? random() % mod_usleep : (unsigned)(sleep_before_off * 1000000);
> 
> Shouldn't this be static_cast<unsigned>? Same elsewhere.
Sure, but that’s beyond this patch. It should be unsigned int actually. Implicit int is bad.
> 
> Regards,
> 
>    Hans
> 
>> 
>>        usecs1 += min_usleep;
>>        usecs2 += min_usleep;
>> diff --git a/utils/cec-ctl/cec-pin.cpp b/utils/cec-ctl/cec-pin.cpp
>> index 0322ad5e..10abea37 100644
>> --- a/utils/cec-ctl/cec-pin.cpp
>> +++ b/utils/cec-ctl/cec-pin.cpp
>> @@ -261,7 +261,7 @@ static void cec_pin_rx_data_bit_was_low(__u64 ev_ts, __u64 usecs, __u64 usecs_mi
>>     * If the low drive starts at the end of a 0 bit, then the actual
>>     * maximum time that the bus can be low is the two summed.
>>     */
>> -    const unsigned max_low_drive = CEC_TIM_LOW_DRIVE_ERROR_MAX +
>> +    const unsigned max_low_drive = (unsigned)CEC_TIM_LOW_DRIVE_ERROR_MAX +
>>        CEC_TIM_DATA_BIT_0_LOW_MAX + CEC_TIM_MARGIN;
>> 
>>    low_usecs = usecs;
>> diff --git a/utils/v4l2-compliance/v4l2-test-buffers.cpp b/utils/v4l2-compliance/v4l2-test-buffers.cpp
>> index 93df7034..87c2e523 100644
>> --- a/utils/v4l2-compliance/v4l2-test-buffers.cpp
>> +++ b/utils/v4l2-compliance/v4l2-test-buffers.cpp
>> @@ -2597,7 +2597,7 @@ static void streamFmt(struct node *node, __u32 pixelformat, __u32 w, __u32 h,
>>    char hz[32] = "";
>> 
>>    if (!frame_count)
>> -        frame_count = f ? 1.0 / fract2f(f) : 10;
>> +        frame_count = f ? (unsigned)(1.0f / fract2f(f)) : 10;
>>    node->g_fmt(fmt);
>>    fmt.s_pixelformat(pixelformat);
>>    fmt.s_width(w);
>> diff --git a/utils/v4l2-ctl/v4l2-ctl-misc.cpp b/utils/v4l2-ctl/v4l2-ctl-misc.cpp
>> index cb933217..2851886a 100644
>> --- a/utils/v4l2-ctl/v4l2-ctl-misc.cpp
>> +++ b/utils/v4l2-ctl/v4l2-ctl-misc.cpp
>> @@ -320,7 +320,7 @@ void misc_set(cv4l_fd &_fd)
>>        parm.type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
>>        parm.parm.capture.timeperframe.numerator = 1000;
>>        parm.parm.capture.timeperframe.denominator =
>> -            fps * parm.parm.capture.timeperframe.numerator;
>> +            (uint32_t)(fps * parm.parm.capture.timeperframe.numerator);
>> 
>>        if (doioctl(fd, VIDIOC_S_PARM, &parm) == 0) {
>>            struct v4l2_fract *tf = &parm.parm.capture.timeperframe;
>> @@ -338,7 +338,7 @@ void misc_set(cv4l_fd &_fd)
>>        parm.type = V4L2_BUF_TYPE_VIDEO_OUTPUT;
>>        parm.parm.output.timeperframe.numerator = 1000;
>>        parm.parm.output.timeperframe.denominator =
>> -            output_fps * parm.parm.output.timeperframe.numerator;
>> +            (uint32_t)(output_fps * parm.parm.output.timeperframe.numerator);
>> 
>>        if (doioctl(fd, VIDIOC_S_PARM, &parm) == 0) {
>>            struct v4l2_fract *tf = &parm.parm.output.timeperframe;
>> diff --git a/utils/v4l2-ctl/v4l2-ctl-streaming.cpp b/utils/v4l2-ctl/v4l2-ctl-streaming.cpp
>> index 066a336a..6981a3cc 100644
>> --- a/utils/v4l2-ctl/v4l2-ctl-streaming.cpp
>> +++ b/utils/v4l2-ctl/v4l2-ctl-streaming.cpp
>> @@ -569,7 +569,7 @@ static void print_concise_buffer(FILE *f, cv4l_buffer &buf, cv4l_fmt &fmt,
>>    if (!skip_ts && (buf.g_flags() & V4L2_BUF_FLAG_TIMESTAMP_MASK) != V4L2_BUF_FLAG_TIMESTAMP_COPY) {
>>        double ts = buf.g_timestamp().tv_sec + buf.g_timestamp().tv_usec / 1000000.0;
>>        fprintf(f, " ts: %.06f", ts);
>> -        if (last_ts)
>> +        if (last_ts <= 0.0)
>>            fprintf(f, " delta: %.03f ms", (ts - last_ts) * 1000.0);
>>        last_ts = ts;
>> 
>> diff --git a/utils/v4l2-ctl/v4l2-ctl-subdev.cpp b/utils/v4l2-ctl/v4l2-ctl-subdev.cpp
>> index 747f1699..f1223084 100644
>> --- a/utils/v4l2-ctl/v4l2-ctl-subdev.cpp
>> +++ b/utils/v4l2-ctl/v4l2-ctl-subdev.cpp
>> @@ -487,7 +487,7 @@ void subdev_set(cv4l_fd &_fd)
>>            exit(1);
>>        }
>>        fival.interval.numerator = 1000;
>> -        fival.interval.denominator = set_fps * fival.interval.numerator;
>> +        fival.interval.denominator = (uint32_t)(set_fps * fival.interval.numerator);
>>        printf("Note: --set-subdev-fps is only for testing.\n"
>>               "Normally media-ctl is used to configure the video pipeline.\n");
>>        printf("ioctl: VIDIOC_SUBDEV_S_FRAME_INTERVAL (pad=%u)\n", fival.pad);
>> 
> 

Re: [PATCH 11/12] utils: fix double promotions

[Hans Verkuil]

On 22/04/2020 02:37, Rosen Penev wrote:
> Found with -Wdouble-promotion
> 
> Signed-off-by: Rosen Penev <rosenp@gmail.com>
> ---
>  utils/rds-ctl/rds-ctl.cpp                   | 4 ++--
>  utils/v4l2-compliance/v4l2-test-buffers.cpp | 2 +-
>  2 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/utils/rds-ctl/rds-ctl.cpp b/utils/rds-ctl/rds-ctl.cpp
> index cc1d3bf7..cd6ece38 100644
> --- a/utils/rds-ctl/rds-ctl.cpp
> +++ b/utils/rds-ctl/rds-ctl.cpp
> @@ -528,7 +528,7 @@ static void print_rds_statistics(const struct v4l2_rds_statistics *statistics)
>  			(static_cast<float>(statistics->block_error_cnt) / statistics->block_cnt) * 100.0f;
>  	printf("block errors / group errors: %u (%3.2f%%) / %u \n",
>  		statistics->block_error_cnt,
> -		block_error_percentage, statistics->group_error_cnt);
> +		(double)block_error_percentage, statistics->group_error_cnt);

Rather than adding a cast, I think the block_error_percentage and block_corrected_percentage
type should just be changed to double.

Regards,

	Hans

>  
>  	float block_corrected_percentage = 0;
>  
> @@ -536,7 +536,7 @@ static void print_rds_statistics(const struct v4l2_rds_statistics *statistics)
>  		block_corrected_percentage = (
>  			static_cast<float>(statistics->block_corrected_cnt) / statistics->block_cnt) * 100.0f;
>  	printf("corrected blocks: %u (%3.2f%%)\n",
> -		statistics->block_corrected_cnt, block_corrected_percentage);
> +		statistics->block_corrected_cnt, (double)block_corrected_percentage);
>  	for (int i = 0; i < 16; i++)
>  		printf("Group %02d: %u\n", i, statistics->group_type_cnt[i]);
>  }
> diff --git a/utils/v4l2-compliance/v4l2-test-buffers.cpp b/utils/v4l2-compliance/v4l2-test-buffers.cpp
> index 87c2e523..abf39cc7 100644
> --- a/utils/v4l2-compliance/v4l2-test-buffers.cpp
> +++ b/utils/v4l2-compliance/v4l2-test-buffers.cpp
> @@ -2597,7 +2597,7 @@ static void streamFmt(struct node *node, __u32 pixelformat, __u32 w, __u32 h,
>  	char hz[32] = "";
>  
>  	if (!frame_count)
> -		frame_count = f ? (unsigned)(1.0f / fract2f(f)) : 10;
> +		frame_count = f ? (unsigned)(1.0 / fract2f(f)) : 10;
>  	node->g_fmt(fmt);
>  	fmt.s_pixelformat(pixelformat);
>  	fmt.s_width(w);
> 

Re: [PATCH 10/12] utils: fix fallthrough warnings

[Rosen Penev]

> On Apr 23, 2020, at 1:43 AM, Hans Verkuil <hverkuil@xs4all.nl> wrote:
> 
> On 22/04/2020 02:37, Rosen Penev wrote:
>> Comments are not enough for clang. Changed to use the C++ attribute.
>> 
>> Found with -Wimplicit-fallthrough
>> 
>> Signed-off-by: Rosen Penev <rosenp@gmail.com>
>> ---
>> utils/cec-compliance/cec-compliance.cpp      |  7 +++++++
>> utils/cec-ctl/cec-ctl.cpp                    |  8 +++++++-
>> utils/cec-follower/cec-follower.cpp          |  7 +++++++
>> utils/cec-follower/cec-processing.cpp        |  9 ++++++++-
>> utils/cec-follower/cec-tuner.cpp             | 12 +++++++++---
>> utils/libcecutil/cec-log.cpp                 |  8 +++++++-
>> utils/v4l2-compliance/v4l2-test-colors.cpp   | 16 +++++++++++-----
>> utils/v4l2-compliance/v4l2-test-controls.cpp | 16 +++++++++++-----
>> utils/v4l2-compliance/v4l2-test-formats.cpp  | 10 ++++++++--
>> utils/v4l2-ctl/v4l2-ctl-streaming.cpp        | 10 ++++++++--
>> utils/v4l2-ctl/v4l2-ctl-vbi.cpp              | 10 ++++++++--
>> 11 files changed, 91 insertions(+), 22 deletions(-)
>> 
>> diff --git a/utils/cec-compliance/cec-compliance.cpp b/utils/cec-compliance/cec-compliance.cpp
>> index 1c62b5c9..e9fce337 100644
>> --- a/utils/cec-compliance/cec-compliance.cpp
>> +++ b/utils/cec-compliance/cec-compliance.cpp
>> @@ -24,6 +24,12 @@
>> #include "version.h"
>> #endif
>> 
>> +#ifdef __clang__
>> +#define FALLTHROUGH [[clang::fallthrough]]
>> +#else
>> +#define FALLTHROUGH [[gnu::fallthrough]]
>> +#endif
> 
> Yuck, no uppercase please.
> 
> I am OK with lowercase 'fallthrough', since that's what the linux kernel settled on
> as well.
> 
> This #ifdef/#else/#endif is replicated in several sources, so let's put this in a
> header instead.
Which one?
> 
> Regards,
> 
>    Hans
> 
>> +
>> /* Short option list
>> 
>>    Please keep in alphabetical order.
>> @@ -486,6 +492,7 @@ void sad_decode(struct short_audio_desc *sad, __u32 descriptor)
>>        case SAD_EXT_TYPE_MPEG_H_3D_AUDIO:
>>        case SAD_EXT_TYPE_AC_4:
>>            sad->format_dependent = b3 & 0x07;
>> +            FALLTHROUGH;
>>        case SAD_EXT_TYPE_LPCM_3D_AUDIO:
>>            sad->bit_depth_mask = b3 & 0x07;
>>            break;
>> diff --git a/utils/cec-ctl/cec-ctl.cpp b/utils/cec-ctl/cec-ctl.cpp
>> index f0e31aca..a3698a67 100644
>> --- a/utils/cec-ctl/cec-ctl.cpp
>> +++ b/utils/cec-ctl/cec-ctl.cpp
>> @@ -37,6 +37,12 @@
>> 
>> #include "cec-ctl.h"
>> 
>> +#ifdef __clang__
>> +#define FALLTHROUGH [[clang::fallthrough]]
>> +#else
>> +#define FALLTHROUGH [[gnu::fallthrough]]
>> +#endif
>> +
>> static struct timespec start_monotonic;
>> static struct timeval start_timeofday;
>> static bool ignore_la[16];
>> @@ -1967,7 +1973,7 @@ int main(int argc, char **argv)
>>            break;
>>        case OptPhysAddrFromEDIDPoll:
>>            edid_path = optarg;
>> -            /* fall-through */
>> +            FALLTHROUGH;
>>        case OptPhysAddrFromEDID:
>>            phys_addr = parse_phys_addr_from_edid(optarg);
>>            break;
>> diff --git a/utils/cec-follower/cec-follower.cpp b/utils/cec-follower/cec-follower.cpp
>> index 1eeae2d4..2d61d5f9 100644
>> --- a/utils/cec-follower/cec-follower.cpp
>> +++ b/utils/cec-follower/cec-follower.cpp
>> @@ -23,6 +23,12 @@
>> #include "version.h"
>> #endif
>> 
>> +#ifdef __clang__
>> +#define FALLTHROUGH [[clang::fallthrough]]
>> +#else
>> +#define FALLTHROUGH [[gnu::fallthrough]]
>> +#endif
>> +
>> /* Short option list
>> 
>>    Please keep in alphabetical order.
>> @@ -142,6 +148,7 @@ void sad_encode(const struct short_audio_desc *sad, __u32 *descriptor)
>>        case SAD_EXT_TYPE_MPEG_H_3D_AUDIO:
>>        case SAD_EXT_TYPE_AC_4:
>>            b3 |= sad->format_dependent & 0x07;
>> +            FALLTHROUGH;
>>        case SAD_EXT_TYPE_LPCM_3D_AUDIO:
>>            b3 |= sad->bit_depth_mask & 0x07;
>>            break;
>> diff --git a/utils/cec-follower/cec-processing.cpp b/utils/cec-follower/cec-processing.cpp
>> index 8e3a33a2..68bfdca3 100644
>> --- a/utils/cec-follower/cec-processing.cpp
>> +++ b/utils/cec-follower/cec-processing.cpp
>> @@ -19,6 +19,12 @@
>> 
>> #include "cec-follower.h"
>> 
>> +#ifdef __clang__
>> +#define FALLTHROUGH [[clang::fallthrough]]
>> +#else
>> +#define FALLTHROUGH [[gnu::fallthrough]]
>> +#endif
>> +
>> #define VOLUME_MAX 0x64
>> #define VOLUME_MIN 0
>> 
>> @@ -352,7 +358,7 @@ static void processMsg(struct node *node, struct cec_msg &msg, unsigned me)
>>        if (cec_has_tv(1 << la) && la_info[la].phys_addr == 0)
>>            warn("TV (0) at 0.0.0.0 sent Routing Information.");
>>    }
>> -
>> +    FALLTHROUGH;
>> 
>>        /* System Information */
>> 
>> @@ -708,6 +714,7 @@ static void processMsg(struct node *node, struct cec_msg &msg, unsigned me)
>>        cec_msg_system_audio_mode_status(&msg, node->state.sac_active ? CEC_OP_SYS_AUD_STATUS_ON :
>>                         CEC_OP_SYS_AUD_STATUS_OFF);
>>        transmit(node, &msg);
>> +        FALLTHROUGH;
>>    case CEC_MSG_GIVE_AUDIO_STATUS:
>>        if (!cec_has_audiosystem(1 << me))
>>            break;
>> diff --git a/utils/cec-follower/cec-tuner.cpp b/utils/cec-follower/cec-tuner.cpp
>> index aa19f55d..af7c6437 100644
>> --- a/utils/cec-follower/cec-tuner.cpp
>> +++ b/utils/cec-follower/cec-tuner.cpp
>> @@ -8,6 +8,12 @@
>> 
>> #include "cec-follower.h"
>> 
>> +#ifdef __clang__
>> +#define FALLTHROUGH [[clang::fallthrough]]
>> +#else
>> +#define FALLTHROUGH [[gnu::fallthrough]]
>> +#endif
>> +
>> #define NUM_ANALOG_FREQS 3
>> #define NUM_DIGITAL_CHANS 3
>> #define TOT_ANALOG_FREQS (sizeof(analog_freqs_khz) / sizeof(analog_freqs_khz[0][0][0]))
>> @@ -303,21 +309,21 @@ static int digital_get_service_idx(struct cec_op_digital_service_id *digital)
>>    switch (digital->dig_bcast_system) {
>>    case CEC_OP_DIG_SERVICE_BCAST_SYSTEM_ARIB_T:
>>        is_terrestrial = true;
>> -        /* fall through */
>> +        FALLTHROUGH;
>>    case CEC_OP_DIG_SERVICE_BCAST_SYSTEM_ARIB_BS:
>>        info = &digital_arib_data[is_terrestrial][0];
>>        offset = is_terrestrial * NUM_DIGITAL_CHANS;
>>        break;
>>    case CEC_OP_DIG_SERVICE_BCAST_SYSTEM_ATSC_T:
>>        is_terrestrial = true;
>> -        /* fall through */
>> +        FALLTHROUGH;
>>    case CEC_OP_DIG_SERVICE_BCAST_SYSTEM_ATSC_SAT:
>>        info = &digital_atsc_data[is_terrestrial][0];
>>        offset = (2 + is_terrestrial) * NUM_DIGITAL_CHANS;
>>        break;
>>    case CEC_OP_DIG_SERVICE_BCAST_SYSTEM_DVB_T:
>>        is_terrestrial = true;
>> -        /* fall through */
>> +        FALLTHROUGH;
>>    case CEC_OP_DIG_SERVICE_BCAST_SYSTEM_DVB_S2:
>>        info = &digital_dvb_data[is_terrestrial][0];
>>        offset = (4 + is_terrestrial) * NUM_DIGITAL_CHANS;
>> diff --git a/utils/libcecutil/cec-log.cpp b/utils/libcecutil/cec-log.cpp
>> index 28a1ecac..9844ac43 100644
>> --- a/utils/libcecutil/cec-log.cpp
>> +++ b/utils/libcecutil/cec-log.cpp
>> @@ -15,6 +15,12 @@
>> #include "cec-info.h"
>> #include "cec-log.h"
>> 
>> +#ifdef __clang__
>> +#define FALLTHROUGH [[clang::fallthrough]]
>> +#else
>> +#define FALLTHROUGH [[gnu::fallthrough]]
>> +#endif
>> +
>> static const struct cec_arg arg_u8 = {
>>    CEC_ARG_TYPE_U8,
>> };
>> @@ -44,7 +50,7 @@ static void log_arg(const struct cec_arg *arg, const char *arg_name, __u32 val)
>>                return;
>>            }
>>        }
>> -        /* fall through */
>> +        FALLTHROUGH;
>>    case CEC_ARG_TYPE_U8:
>>        if (!strcmp(arg_name, "video-latency") ||
>>            !strcmp(arg_name, "audio-out-delay")) {
>> diff --git a/utils/v4l2-compliance/v4l2-test-colors.cpp b/utils/v4l2-compliance/v4l2-test-colors.cpp
>> index 887b198b..d37300e9 100644
>> --- a/utils/v4l2-compliance/v4l2-test-colors.cpp
>> +++ b/utils/v4l2-compliance/v4l2-test-colors.cpp
>> @@ -30,6 +30,12 @@
>> #include <math.h>
>> #include "v4l2-compliance.h"
>> 
>> +#ifdef __clang__
>> +#define FALLTHROUGH [[clang::fallthrough]]
>> +#else
>> +#define FALLTHROUGH [[gnu::fallthrough]]
>> +#endif
>> +
>> static void setupPlanes(const cv4l_fmt &fmt, __u8 *planes[3])
>> {
>>    if (fmt.g_num_planes() > 1)
>> @@ -328,7 +334,7 @@ static void getColor(const cv4l_fmt &fmt, __u8 * const planes[3],
>>    case V4L2_PIX_FMT_RGBA444:
>>    case V4L2_PIX_FMT_BGRA444:
>>        c.a = (v16 >> 12) / 15.0;
>> -        /* fall through */
>> +        FALLTHROUGH;
>>    case V4L2_PIX_FMT_RGB444:
>>    case V4L2_PIX_FMT_XRGB444:
>>    case V4L2_PIX_FMT_XBGR444:
>> @@ -345,7 +351,7 @@ static void getColor(const cv4l_fmt &fmt, __u8 * const planes[3],
>>    case V4L2_PIX_FMT_ABGR555:
>>    case V4L2_PIX_FMT_BGRA555:
>>        c.a = v16 >> 15;
>> -        /* fall through */
>> +        FALLTHROUGH;
>>    case V4L2_PIX_FMT_YUV555:
>>    case V4L2_PIX_FMT_RGB555:
>>    case V4L2_PIX_FMT_XRGB555:
>> @@ -376,7 +382,7 @@ static void getColor(const cv4l_fmt &fmt, __u8 * const planes[3],
>>    case V4L2_PIX_FMT_RGBA32:
>>    case V4L2_PIX_FMT_BGRA32:
>>        c.a = ((v32 >> 24) & 0xff) / 255.0;
>> -        /* fall through */
>> +        FALLTHROUGH;
>>    default:
>>        c.r = ((v32 >> 16) & 0xff) / 255.0;
>>        c.g = ((v32 >> 8) & 0xff) / 255.0;
>> @@ -444,7 +450,7 @@ static void getColor(const cv4l_fmt &fmt, __u8 * const planes[3],
>>        Y = (Y - 16.0 / 255.0) * 255.0 / 219.0;
>>        cb *= 255.0 / 224.0;
>>        cr *= 255.0 / 224.0;
>> -        /* fall through */
>> +        FALLTHROUGH;
>>    case V4L2_YCBCR_ENC_601:
>>    default:
>>        ycbcr2rgb(bt601, Y, cb, cr, c);
>> @@ -453,7 +459,7 @@ static void getColor(const cv4l_fmt &fmt, __u8 * const planes[3],
>>        Y = (Y - 16.0 / 255.0) * 255.0 / 219.0;
>>        cb *= 255.0 / 224.0;
>>        cr *= 255.0 / 224.0;
>> -        /* fall through */
>> +        FALLTHROUGH;
>>    case V4L2_YCBCR_ENC_709:
>>        ycbcr2rgb(rec709, Y, cb, cr, c);
>>        break;
>> diff --git a/utils/v4l2-compliance/v4l2-test-controls.cpp b/utils/v4l2-compliance/v4l2-test-controls.cpp
>> index 251a6049..2de888cf 100644
>> --- a/utils/v4l2-compliance/v4l2-test-controls.cpp
>> +++ b/utils/v4l2-compliance/v4l2-test-controls.cpp
>> @@ -34,6 +34,12 @@
>> 
>> #define V4L2_CTRL_CLASS_VIVID 0x00f00000
>> 
>> +#ifdef __clang__
>> +#define FALLTHROUGH [[clang::fallthrough]]
>> +#else
>> +#define FALLTHROUGH [[gnu::fallthrough]]
>> +#endif
>> +
>> static int checkQCtrl(struct node *node, struct test_query_ext_ctrl &qctrl)
>> {
>>    struct v4l2_querymenu qmenu;
>> @@ -61,20 +67,20 @@ static int checkQCtrl(struct node *node, struct test_query_ext_ctrl &qctrl)
>>    case V4L2_CTRL_TYPE_BOOLEAN:
>>        if (qctrl.maximum > 1)
>>            return fail("invalid boolean max value\n");
>> -        /* fall through */
>> +        FALLTHROUGH;
>>    case V4L2_CTRL_TYPE_MENU:
>>    case V4L2_CTRL_TYPE_INTEGER_MENU:
>>        if (qctrl.step != 1)
>>            return fail("invalid step value %lld\n", qctrl.step);
>>        if (qctrl.minimum < 0)
>>            return fail("min < 0\n");
>> -        /* fall through */
>> +        FALLTHROUGH;
>>    case V4L2_CTRL_TYPE_INTEGER:
>>    case V4L2_CTRL_TYPE_INTEGER64:
>>        if (qctrl.default_value < qctrl.minimum ||
>>            qctrl.default_value > qctrl.maximum)
>>            return fail("def < min || def > max\n");
>> -        /* fall through */
>> +        FALLTHROUGH;
>>    case V4L2_CTRL_TYPE_STRING:
>>        if (qctrl.minimum > qctrl.maximum)
>>            return fail("min > max\n");
>> @@ -116,7 +122,7 @@ static int checkQCtrl(struct node *node, struct test_query_ext_ctrl &qctrl)
>>    case V4L2_CTRL_TYPE_BUTTON:
>>        if ((fl & rw_mask) != V4L2_CTRL_FLAG_WRITE_ONLY)
>>            return fail("button control not write only\n");
>> -        /* fall through */
>> +        FALLTHROUGH;
>>    case V4L2_CTRL_TYPE_BOOLEAN:
>>    case V4L2_CTRL_TYPE_MENU:
>>    case V4L2_CTRL_TYPE_INTEGER_MENU:
>> @@ -124,7 +130,7 @@ static int checkQCtrl(struct node *node, struct test_query_ext_ctrl &qctrl)
>>    case V4L2_CTRL_TYPE_BITMASK:
>>        if (fl & V4L2_CTRL_FLAG_SLIDER)
>>            return fail("slider makes only sense for integer controls\n");
>> -        /* fall through */
>> +        FALLTHROUGH;
>>    case V4L2_CTRL_TYPE_INTEGER64:
>>    case V4L2_CTRL_TYPE_INTEGER:
>>        if ((fl & rw_mask) == rw_mask)
>> diff --git a/utils/v4l2-compliance/v4l2-test-formats.cpp b/utils/v4l2-compliance/v4l2-test-formats.cpp
>> index 3fc87316..9e98cf99 100644
>> --- a/utils/v4l2-compliance/v4l2-test-formats.cpp
>> +++ b/utils/v4l2-compliance/v4l2-test-formats.cpp
>> @@ -32,6 +32,12 @@
>> #include <assert.h>
>> #include "v4l2-compliance.h"
>> 
>> +#ifdef __clang__
>> +#define FALLTHROUGH [[clang::fallthrough]]
>> +#else
>> +#define FALLTHROUGH [[gnu::fallthrough]]
>> +#endif
>> +
>> static const __u32 buftype2cap[] = {
>>    0,
>>    V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_VIDEO_M2M,
>> @@ -97,7 +103,7 @@ static int testEnumFrameIntervals(struct node *node, __u32 pixfmt,
>>        case V4L2_FRMIVAL_TYPE_CONTINUOUS:
>>            if (sw->step.numerator != 1 || sw->step.denominator != 1)
>>                return fail("invalid step for continuous frameinterval\n");
>> -            /* fallthrough */
>> +            FALLTHROUGH;
>>        case V4L2_FRMIVAL_TYPE_STEPWISE:
>>            if (frmival.index)
>>                return fail("index must be 0 for stepwise/continuous frameintervals\n");
>> @@ -183,7 +189,7 @@ static int testEnumFrameSizes(struct node *node, __u32 pixfmt)
>>        case V4L2_FRMSIZE_TYPE_CONTINUOUS:
>>            if (frmsize.stepwise.step_width != 1 || frmsize.stepwise.step_height != 1)
>>                return fail("invalid step_width/height for continuous framesize\n");
>> -            /* fallthrough */
>> +            FALLTHROUGH;
>>        case V4L2_FRMSIZE_TYPE_STEPWISE:
>>            if (frmsize.index)
>>                return fail("index must be 0 for stepwise/continuous framesizes\n");
>> diff --git a/utils/v4l2-ctl/v4l2-ctl-streaming.cpp b/utils/v4l2-ctl/v4l2-ctl-streaming.cpp
>> index 6981a3cc..bb464e99 100644
>> --- a/utils/v4l2-ctl/v4l2-ctl-streaming.cpp
>> +++ b/utils/v4l2-ctl/v4l2-ctl-streaming.cpp
>> @@ -24,6 +24,12 @@
>> #include <media-info.h>
>> #include <fwht-ctrls.h>
>> 
>> +#ifdef __clang__
>> +#define FALLTHROUGH [[clang::fallthrough]]
>> +#else
>> +#define FALLTHROUGH [[gnu::fallthrough]]
>> +#endif
>> +
>> extern "C" {
>> #include "v4l2-tpg.h"
>> }
>> @@ -763,7 +769,7 @@ void streaming_cmd(int ch, char *optarg)
>>        break;
>>    case OptStreamUser:
>>        memory = V4L2_MEMORY_USERPTR;
>> -        /* fall through */
>> +        FALLTHROUGH;
>>    case OptStreamMmap:
>>        if (optarg) {
>>            reqbufs_count_cap = strtoul(optarg, 0L, 0);
>> @@ -776,7 +782,7 @@ void streaming_cmd(int ch, char *optarg)
>>        break;
>>    case OptStreamOutUser:
>>        out_memory = V4L2_MEMORY_USERPTR;
>> -        /* fall through */
>> +        FALLTHROUGH;
>>    case OptStreamOutMmap:
>>        if (optarg) {
>>            reqbufs_count_out = strtoul(optarg, 0L, 0);
>> diff --git a/utils/v4l2-ctl/v4l2-ctl-vbi.cpp b/utils/v4l2-ctl/v4l2-ctl-vbi.cpp
>> index c5960f78..82d01b58 100644
>> --- a/utils/v4l2-ctl/v4l2-ctl-vbi.cpp
>> +++ b/utils/v4l2-ctl/v4l2-ctl-vbi.cpp
>> @@ -16,6 +16,12 @@
>> 
>> #include "v4l2-ctl.h"
>> 
>> +#ifdef __clang__
>> +#define FALLTHROUGH [[clang::fallthrough]]
>> +#else
>> +#define FALLTHROUGH [[gnu::fallthrough]]
>> +#endif
>> +
>> static struct v4l2_format sliced_fmt;      /* set_format/get_format for sliced VBI */
>> static struct v4l2_format sliced_fmt_out; /* set_format/get_format for sliced VBI output */
>> static struct v4l2_format raw_fmt;      /* set_format/get_format for VBI */
>> @@ -87,7 +93,7 @@ void vbi_cmd(int ch, char *optarg)
>>    case OptSetSlicedVbiOutFormat:
>>    case OptTrySlicedVbiOutFormat:
>>        sliced = &sliced_fmt_out;
>> -        /* fall through */
>> +        FALLTHROUGH;
>>    case OptSetSlicedVbiFormat:
>>    case OptTrySlicedVbiFormat:
>>        sliced->fmt.sliced.service_set = 0;
>> @@ -130,7 +136,7 @@ void vbi_cmd(int ch, char *optarg)
>>    case OptSetVbiOutFormat:
>>    case OptTryVbiOutFormat:
>>        raw = &raw_fmt_out;
>> -        /* fall through */
>> +        FALLTHROUGH;
>>    case OptSetVbiFormat:
>>    case OptTryVbiFormat:
>>        subs = optarg;
>> 
> 

Re: [PATCH 01/12] utils: fix compilation with C++98

[Hans Verkuil]

On 23/04/2020 10:30, Rosen Penev wrote:
> 
> 
>> On Apr 23, 2020, at 1:19 AM, Hans Verkuil <hverkuil@xs4all.nl> wrote:
>>
>> On 22/04/2020 02:37, Rosen Penev wrote:
>>> Signed-off-by: Rosen Penev <rosenp@gmail.com>
>>> ---
>>> utils/cec-compliance/cec-test.cpp | 4 ++--
>>> utils/rds-ctl/rds-ctl.cpp         | 2 +-
>>> 2 files changed, 3 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/utils/cec-compliance/cec-test.cpp b/utils/cec-compliance/cec-test.cpp
>>> index 032ff5ad..cc07998a 100644
>>> --- a/utils/cec-compliance/cec-test.cpp
>>> +++ b/utils/cec-compliance/cec-test.cpp
>>> @@ -908,7 +908,7 @@ static int tuner_ctl_test(struct node *node, unsigned me, unsigned la, bool inte
>>>        cec_msg_give_tuner_device_status(&msg, true, CEC_OP_STATUS_REQ_ONCE);
>>>        fail_on_test(!transmit_timeout(node, &msg));
>>>        fail_on_test(timed_out_or_abort(&msg));
>>> -        info = {};
>>> +        memset(&info, 0, sizeof(info));
>>>        cec_ops_tuner_device_status(&msg, &info);
>>>        if (!memcmp(&info, &info_vec[0], sizeof(info)))
>>>            break;
>>> @@ -935,7 +935,7 @@ static int tuner_ctl_test(struct node *node, unsigned me, unsigned la, bool inte
>>>        cec_msg_give_tuner_device_status(&msg, true, CEC_OP_STATUS_REQ_ONCE);
>>>        fail_on_test(!transmit_timeout(node, &msg));
>>>        fail_on_test(timed_out_or_abort(&msg));
>>> -        info = {};
>>> +        memset(&info, 0, sizeof(info));
>>>        cec_ops_tuner_device_status(&msg, &info);
>>>        if (memcmp(&info, &(*iter), sizeof(info))) {
>>>            log_tuner_service(info);
>>> diff --git a/utils/rds-ctl/rds-ctl.cpp b/utils/rds-ctl/rds-ctl.cpp
>>> index 0511fe5d..0e497b2a 100644
>>> --- a/utils/rds-ctl/rds-ctl.cpp
>>> +++ b/utils/rds-ctl/rds-ctl.cpp
>>> @@ -918,7 +918,7 @@ static void get_options(const int fd, const int capabilities, struct v4l2_freque
>>>                printf("\tFrequency range      : %.1f MHz - %.1f MHz\n",
>>>                     vt.rangelow / 16.0, vt.rangehigh / 16.0);
>>>            printf("\tSignal strength/AFC  : %ld%%/%d\n",
>>> -                std::lround(vt.signal / 655.35), vt.afc);
>>> +                lround(vt.signal / 655.25), vt.afc);
>>
>> v4l2-ctl-tuner.cpp also uses std::lround, but that one isn't changed back.
>>
>> Is rds-ctl.cpp perhaps just missing a header?
> Nope. std::lround is C++11.

So, why isn't v4l2-ctl-tuner.cpp changed to lround as well?

Regards,

	Hans

>>
>> Regards,
>>
>>    Hans
>>
>>>            printf("\tCurrent audio mode   : %s\n", audmode2s(vt.audmode));
>>>            printf("\tAvailable subchannels: %s\n",
>>>                    rxsubchans2s(vt.rxsubchans).c_str());
>>>
>>

Re: [PATCH 01/12] utils: fix compilation with C++98

[Rosen Penev]

> On Apr 23, 2020, at 1:48 AM, Hans Verkuil <hverkuil@xs4all.nl> wrote:
> 
> On 23/04/2020 10:30, Rosen Penev wrote:
>> 
>> 
>>>> On Apr 23, 2020, at 1:19 AM, Hans Verkuil <hverkuil@xs4all.nl> wrote:
>>> 
>>> On 22/04/2020 02:37, Rosen Penev wrote:
>>>> Signed-off-by: Rosen Penev <rosenp@gmail.com>
>>>> ---
>>>> utils/cec-compliance/cec-test.cpp | 4 ++--
>>>> utils/rds-ctl/rds-ctl.cpp         | 2 +-
>>>> 2 files changed, 3 insertions(+), 3 deletions(-)
>>>> 
>>>> diff --git a/utils/cec-compliance/cec-test.cpp b/utils/cec-compliance/cec-test.cpp
>>>> index 032ff5ad..cc07998a 100644
>>>> --- a/utils/cec-compliance/cec-test.cpp
>>>> +++ b/utils/cec-compliance/cec-test.cpp
>>>> @@ -908,7 +908,7 @@ static int tuner_ctl_test(struct node *node, unsigned me, unsigned la, bool inte
>>>>       cec_msg_give_tuner_device_status(&msg, true, CEC_OP_STATUS_REQ_ONCE);
>>>>       fail_on_test(!transmit_timeout(node, &msg));
>>>>       fail_on_test(timed_out_or_abort(&msg));
>>>> -        info = {};
>>>> +        memset(&info, 0, sizeof(info));
>>>>       cec_ops_tuner_device_status(&msg, &info);
>>>>       if (!memcmp(&info, &info_vec[0], sizeof(info)))
>>>>           break;
>>>> @@ -935,7 +935,7 @@ static int tuner_ctl_test(struct node *node, unsigned me, unsigned la, bool inte
>>>>       cec_msg_give_tuner_device_status(&msg, true, CEC_OP_STATUS_REQ_ONCE);
>>>>       fail_on_test(!transmit_timeout(node, &msg));
>>>>       fail_on_test(timed_out_or_abort(&msg));
>>>> -        info = {};
>>>> +        memset(&info, 0, sizeof(info));
>>>>       cec_ops_tuner_device_status(&msg, &info);
>>>>       if (memcmp(&info, &(*iter), sizeof(info))) {
>>>>           log_tuner_service(info);
>>>> diff --git a/utils/rds-ctl/rds-ctl.cpp b/utils/rds-ctl/rds-ctl.cpp
>>>> index 0511fe5d..0e497b2a 100644
>>>> --- a/utils/rds-ctl/rds-ctl.cpp
>>>> +++ b/utils/rds-ctl/rds-ctl.cpp
>>>> @@ -918,7 +918,7 @@ static void get_options(const int fd, const int capabilities, struct v4l2_freque
>>>>               printf("\tFrequency range      : %.1f MHz - %.1f MHz\n",
>>>>                    vt.rangelow / 16.0, vt.rangehigh / 16.0);
>>>>           printf("\tSignal strength/AFC  : %ld%%/%d\n",
>>>> -                std::lround(vt.signal / 655.35), vt.afc);
>>>> +                lround(vt.signal / 655.25), vt.afc);
>>> 
>>> v4l2-ctl-tuner.cpp also uses std::lround, but that one isn't changed back.
>>> 
>>> Is rds-ctl.cpp perhaps just missing a header?
>> Nope. std::lround is C++11.
> 
> So, why isn't v4l2-ctl-tuner.cpp changed to lround as well?
Ugh I think it’s in some other local patch of mine. I’ll need to resend this one.
> 
> Regards,
> 
>    Hans
> 
>>> 
>>> Regards,
>>> 
>>>   Hans
>>> 
>>>>           printf("\tCurrent audio mode   : %s\n", audmode2s(vt.audmode));
>>>>           printf("\tAvailable subchannels: %s\n",
>>>>                   rxsubchans2s(vt.rxsubchans).c_str());
>>>> 
>>> 
> 

Re: [PATCH 10/12] utils: fix fallthrough warnings

[Hans Verkuil]

On 23/04/2020 10:46, Rosen Penev wrote:
> 
> 
>> On Apr 23, 2020, at 1:43 AM, Hans Verkuil <hverkuil@xs4all.nl> wrote:
>>
>> On 22/04/2020 02:37, Rosen Penev wrote:
>>> Comments are not enough for clang. Changed to use the C++ attribute.
>>>
>>> Found with -Wimplicit-fallthrough
>>>
>>> Signed-off-by: Rosen Penev <rosenp@gmail.com>
>>> ---
>>> utils/cec-compliance/cec-compliance.cpp      |  7 +++++++
>>> utils/cec-ctl/cec-ctl.cpp                    |  8 +++++++-
>>> utils/cec-follower/cec-follower.cpp          |  7 +++++++
>>> utils/cec-follower/cec-processing.cpp        |  9 ++++++++-
>>> utils/cec-follower/cec-tuner.cpp             | 12 +++++++++---
>>> utils/libcecutil/cec-log.cpp                 |  8 +++++++-
>>> utils/v4l2-compliance/v4l2-test-colors.cpp   | 16 +++++++++++-----
>>> utils/v4l2-compliance/v4l2-test-controls.cpp | 16 +++++++++++-----
>>> utils/v4l2-compliance/v4l2-test-formats.cpp  | 10 ++++++++--
>>> utils/v4l2-ctl/v4l2-ctl-streaming.cpp        | 10 ++++++++--
>>> utils/v4l2-ctl/v4l2-ctl-vbi.cpp              | 10 ++++++++--
>>> 11 files changed, 91 insertions(+), 22 deletions(-)
>>>
>>> diff --git a/utils/cec-compliance/cec-compliance.cpp b/utils/cec-compliance/cec-compliance.cpp
>>> index 1c62b5c9..e9fce337 100644
>>> --- a/utils/cec-compliance/cec-compliance.cpp
>>> +++ b/utils/cec-compliance/cec-compliance.cpp
>>> @@ -24,6 +24,12 @@
>>> #include "version.h"
>>> #endif
>>>
>>> +#ifdef __clang__
>>> +#define FALLTHROUGH [[clang::fallthrough]]
>>> +#else
>>> +#define FALLTHROUGH [[gnu::fallthrough]]
>>> +#endif
>>
>> Yuck, no uppercase please.
>>
>> I am OK with lowercase 'fallthrough', since that's what the linux kernel settled on
>> as well.
>>
>> This #ifdef/#else/#endif is replicated in several sources, so let's put this in a
>> header instead.
> Which one?

How about v4l-utils/include/compiler.h?

Regards,

	Hans

>>
>> Regards,
>>
>>    Hans
>>
>>> +
>>> /* Short option list
>>>
>>>    Please keep in alphabetical order.
>>> @@ -486,6 +492,7 @@ void sad_decode(struct short_audio_desc *sad, __u32 descriptor)
>>>        case SAD_EXT_TYPE_MPEG_H_3D_AUDIO:
>>>        case SAD_EXT_TYPE_AC_4:
>>>            sad->format_dependent = b3 & 0x07;
>>> +            FALLTHROUGH;
>>>        case SAD_EXT_TYPE_LPCM_3D_AUDIO:
>>>            sad->bit_depth_mask = b3 & 0x07;
>>>            break;
>>> diff --git a/utils/cec-ctl/cec-ctl.cpp b/utils/cec-ctl/cec-ctl.cpp
>>> index f0e31aca..a3698a67 100644
>>> --- a/utils/cec-ctl/cec-ctl.cpp
>>> +++ b/utils/cec-ctl/cec-ctl.cpp
>>> @@ -37,6 +37,12 @@
>>>
>>> #include "cec-ctl.h"
>>>
>>> +#ifdef __clang__
>>> +#define FALLTHROUGH [[clang::fallthrough]]
>>> +#else
>>> +#define FALLTHROUGH [[gnu::fallthrough]]
>>> +#endif
>>> +
>>> static struct timespec start_monotonic;
>>> static struct timeval start_timeofday;
>>> static bool ignore_la[16];
>>> @@ -1967,7 +1973,7 @@ int main(int argc, char **argv)
>>>            break;
>>>        case OptPhysAddrFromEDIDPoll:
>>>            edid_path = optarg;
>>> -            /* fall-through */
>>> +            FALLTHROUGH;
>>>        case OptPhysAddrFromEDID:
>>>            phys_addr = parse_phys_addr_from_edid(optarg);
>>>            break;
>>> diff --git a/utils/cec-follower/cec-follower.cpp b/utils/cec-follower/cec-follower.cpp
>>> index 1eeae2d4..2d61d5f9 100644
>>> --- a/utils/cec-follower/cec-follower.cpp
>>> +++ b/utils/cec-follower/cec-follower.cpp
>>> @@ -23,6 +23,12 @@
>>> #include "version.h"
>>> #endif
>>>
>>> +#ifdef __clang__
>>> +#define FALLTHROUGH [[clang::fallthrough]]
>>> +#else
>>> +#define FALLTHROUGH [[gnu::fallthrough]]
>>> +#endif
>>> +
>>> /* Short option list
>>>
>>>    Please keep in alphabetical order.
>>> @@ -142,6 +148,7 @@ void sad_encode(const struct short_audio_desc *sad, __u32 *descriptor)
>>>        case SAD_EXT_TYPE_MPEG_H_3D_AUDIO:
>>>        case SAD_EXT_TYPE_AC_4:
>>>            b3 |= sad->format_dependent & 0x07;
>>> +            FALLTHROUGH;
>>>        case SAD_EXT_TYPE_LPCM_3D_AUDIO:
>>>            b3 |= sad->bit_depth_mask & 0x07;
>>>            break;
>>> diff --git a/utils/cec-follower/cec-processing.cpp b/utils/cec-follower/cec-processing.cpp
>>> index 8e3a33a2..68bfdca3 100644
>>> --- a/utils/cec-follower/cec-processing.cpp
>>> +++ b/utils/cec-follower/cec-processing.cpp
>>> @@ -19,6 +19,12 @@
>>>
>>> #include "cec-follower.h"
>>>
>>> +#ifdef __clang__
>>> +#define FALLTHROUGH [[clang::fallthrough]]
>>> +#else
>>> +#define FALLTHROUGH [[gnu::fallthrough]]
>>> +#endif
>>> +
>>> #define VOLUME_MAX 0x64
>>> #define VOLUME_MIN 0
>>>
>>> @@ -352,7 +358,7 @@ static void processMsg(struct node *node, struct cec_msg &msg, unsigned me)
>>>        if (cec_has_tv(1 << la) && la_info[la].phys_addr == 0)
>>>            warn("TV (0) at 0.0.0.0 sent Routing Information.");
>>>    }
>>> -
>>> +    FALLTHROUGH;
>>>
>>>        /* System Information */
>>>
>>> @@ -708,6 +714,7 @@ static void processMsg(struct node *node, struct cec_msg &msg, unsigned me)
>>>        cec_msg_system_audio_mode_status(&msg, node->state.sac_active ? CEC_OP_SYS_AUD_STATUS_ON :
>>>                         CEC_OP_SYS_AUD_STATUS_OFF);
>>>        transmit(node, &msg);
>>> +        FALLTHROUGH;
>>>    case CEC_MSG_GIVE_AUDIO_STATUS:
>>>        if (!cec_has_audiosystem(1 << me))
>>>            break;
>>> diff --git a/utils/cec-follower/cec-tuner.cpp b/utils/cec-follower/cec-tuner.cpp
>>> index aa19f55d..af7c6437 100644
>>> --- a/utils/cec-follower/cec-tuner.cpp
>>> +++ b/utils/cec-follower/cec-tuner.cpp
>>> @@ -8,6 +8,12 @@
>>>
>>> #include "cec-follower.h"
>>>
>>> +#ifdef __clang__
>>> +#define FALLTHROUGH [[clang::fallthrough]]
>>> +#else
>>> +#define FALLTHROUGH [[gnu::fallthrough]]
>>> +#endif
>>> +
>>> #define NUM_ANALOG_FREQS 3
>>> #define NUM_DIGITAL_CHANS 3
>>> #define TOT_ANALOG_FREQS (sizeof(analog_freqs_khz) / sizeof(analog_freqs_khz[0][0][0]))
>>> @@ -303,21 +309,21 @@ static int digital_get_service_idx(struct cec_op_digital_service_id *digital)
>>>    switch (digital->dig_bcast_system) {
>>>    case CEC_OP_DIG_SERVICE_BCAST_SYSTEM_ARIB_T:
>>>        is_terrestrial = true;
>>> -        /* fall through */
>>> +        FALLTHROUGH;
>>>    case CEC_OP_DIG_SERVICE_BCAST_SYSTEM_ARIB_BS:
>>>        info = &digital_arib_data[is_terrestrial][0];
>>>        offset = is_terrestrial * NUM_DIGITAL_CHANS;
>>>        break;
>>>    case CEC_OP_DIG_SERVICE_BCAST_SYSTEM_ATSC_T:
>>>        is_terrestrial = true;
>>> -        /* fall through */
>>> +        FALLTHROUGH;
>>>    case CEC_OP_DIG_SERVICE_BCAST_SYSTEM_ATSC_SAT:
>>>        info = &digital_atsc_data[is_terrestrial][0];
>>>        offset = (2 + is_terrestrial) * NUM_DIGITAL_CHANS;
>>>        break;
>>>    case CEC_OP_DIG_SERVICE_BCAST_SYSTEM_DVB_T:
>>>        is_terrestrial = true;
>>> -        /* fall through */
>>> +        FALLTHROUGH;
>>>    case CEC_OP_DIG_SERVICE_BCAST_SYSTEM_DVB_S2:
>>>        info = &digital_dvb_data[is_terrestrial][0];
>>>        offset = (4 + is_terrestrial) * NUM_DIGITAL_CHANS;
>>> diff --git a/utils/libcecutil/cec-log.cpp b/utils/libcecutil/cec-log.cpp
>>> index 28a1ecac..9844ac43 100644
>>> --- a/utils/libcecutil/cec-log.cpp
>>> +++ b/utils/libcecutil/cec-log.cpp
>>> @@ -15,6 +15,12 @@
>>> #include "cec-info.h"
>>> #include "cec-log.h"
>>>
>>> +#ifdef __clang__
>>> +#define FALLTHROUGH [[clang::fallthrough]]
>>> +#else
>>> +#define FALLTHROUGH [[gnu::fallthrough]]
>>> +#endif
>>> +
>>> static const struct cec_arg arg_u8 = {
>>>    CEC_ARG_TYPE_U8,
>>> };
>>> @@ -44,7 +50,7 @@ static void log_arg(const struct cec_arg *arg, const char *arg_name, __u32 val)
>>>                return;
>>>            }
>>>        }
>>> -        /* fall through */
>>> +        FALLTHROUGH;
>>>    case CEC_ARG_TYPE_U8:
>>>        if (!strcmp(arg_name, "video-latency") ||
>>>            !strcmp(arg_name, "audio-out-delay")) {
>>> diff --git a/utils/v4l2-compliance/v4l2-test-colors.cpp b/utils/v4l2-compliance/v4l2-test-colors.cpp
>>> index 887b198b..d37300e9 100644
>>> --- a/utils/v4l2-compliance/v4l2-test-colors.cpp
>>> +++ b/utils/v4l2-compliance/v4l2-test-colors.cpp
>>> @@ -30,6 +30,12 @@
>>> #include <math.h>
>>> #include "v4l2-compliance.h"
>>>
>>> +#ifdef __clang__
>>> +#define FALLTHROUGH [[clang::fallthrough]]
>>> +#else
>>> +#define FALLTHROUGH [[gnu::fallthrough]]
>>> +#endif
>>> +
>>> static void setupPlanes(const cv4l_fmt &fmt, __u8 *planes[3])
>>> {
>>>    if (fmt.g_num_planes() > 1)
>>> @@ -328,7 +334,7 @@ static void getColor(const cv4l_fmt &fmt, __u8 * const planes[3],
>>>    case V4L2_PIX_FMT_RGBA444:
>>>    case V4L2_PIX_FMT_BGRA444:
>>>        c.a = (v16 >> 12) / 15.0;
>>> -        /* fall through */
>>> +        FALLTHROUGH;
>>>    case V4L2_PIX_FMT_RGB444:
>>>    case V4L2_PIX_FMT_XRGB444:
>>>    case V4L2_PIX_FMT_XBGR444:
>>> @@ -345,7 +351,7 @@ static void getColor(const cv4l_fmt &fmt, __u8 * const planes[3],
>>>    case V4L2_PIX_FMT_ABGR555:
>>>    case V4L2_PIX_FMT_BGRA555:
>>>        c.a = v16 >> 15;
>>> -        /* fall through */
>>> +        FALLTHROUGH;
>>>    case V4L2_PIX_FMT_YUV555:
>>>    case V4L2_PIX_FMT_RGB555:
>>>    case V4L2_PIX_FMT_XRGB555:
>>> @@ -376,7 +382,7 @@ static void getColor(const cv4l_fmt &fmt, __u8 * const planes[3],
>>>    case V4L2_PIX_FMT_RGBA32:
>>>    case V4L2_PIX_FMT_BGRA32:
>>>        c.a = ((v32 >> 24) & 0xff) / 255.0;
>>> -        /* fall through */
>>> +        FALLTHROUGH;
>>>    default:
>>>        c.r = ((v32 >> 16) & 0xff) / 255.0;
>>>        c.g = ((v32 >> 8) & 0xff) / 255.0;
>>> @@ -444,7 +450,7 @@ static void getColor(const cv4l_fmt &fmt, __u8 * const planes[3],
>>>        Y = (Y - 16.0 / 255.0) * 255.0 / 219.0;
>>>        cb *= 255.0 / 224.0;
>>>        cr *= 255.0 / 224.0;
>>> -        /* fall through */
>>> +        FALLTHROUGH;
>>>    case V4L2_YCBCR_ENC_601:
>>>    default:
>>>        ycbcr2rgb(bt601, Y, cb, cr, c);
>>> @@ -453,7 +459,7 @@ static void getColor(const cv4l_fmt &fmt, __u8 * const planes[3],
>>>        Y = (Y - 16.0 / 255.0) * 255.0 / 219.0;
>>>        cb *= 255.0 / 224.0;
>>>        cr *= 255.0 / 224.0;
>>> -        /* fall through */
>>> +        FALLTHROUGH;
>>>    case V4L2_YCBCR_ENC_709:
>>>        ycbcr2rgb(rec709, Y, cb, cr, c);
>>>        break;
>>> diff --git a/utils/v4l2-compliance/v4l2-test-controls.cpp b/utils/v4l2-compliance/v4l2-test-controls.cpp
>>> index 251a6049..2de888cf 100644
>>> --- a/utils/v4l2-compliance/v4l2-test-controls.cpp
>>> +++ b/utils/v4l2-compliance/v4l2-test-controls.cpp
>>> @@ -34,6 +34,12 @@
>>>
>>> #define V4L2_CTRL_CLASS_VIVID 0x00f00000
>>>
>>> +#ifdef __clang__
>>> +#define FALLTHROUGH [[clang::fallthrough]]
>>> +#else
>>> +#define FALLTHROUGH [[gnu::fallthrough]]
>>> +#endif
>>> +
>>> static int checkQCtrl(struct node *node, struct test_query_ext_ctrl &qctrl)
>>> {
>>>    struct v4l2_querymenu qmenu;
>>> @@ -61,20 +67,20 @@ static int checkQCtrl(struct node *node, struct test_query_ext_ctrl &qctrl)
>>>    case V4L2_CTRL_TYPE_BOOLEAN:
>>>        if (qctrl.maximum > 1)
>>>            return fail("invalid boolean max value\n");
>>> -        /* fall through */
>>> +        FALLTHROUGH;
>>>    case V4L2_CTRL_TYPE_MENU:
>>>    case V4L2_CTRL_TYPE_INTEGER_MENU:
>>>        if (qctrl.step != 1)
>>>            return fail("invalid step value %lld\n", qctrl.step);
>>>        if (qctrl.minimum < 0)
>>>            return fail("min < 0\n");
>>> -        /* fall through */
>>> +        FALLTHROUGH;
>>>    case V4L2_CTRL_TYPE_INTEGER:
>>>    case V4L2_CTRL_TYPE_INTEGER64:
>>>        if (qctrl.default_value < qctrl.minimum ||
>>>            qctrl.default_value > qctrl.maximum)
>>>            return fail("def < min || def > max\n");
>>> -        /* fall through */
>>> +        FALLTHROUGH;
>>>    case V4L2_CTRL_TYPE_STRING:
>>>        if (qctrl.minimum > qctrl.maximum)
>>>            return fail("min > max\n");
>>> @@ -116,7 +122,7 @@ static int checkQCtrl(struct node *node, struct test_query_ext_ctrl &qctrl)
>>>    case V4L2_CTRL_TYPE_BUTTON:
>>>        if ((fl & rw_mask) != V4L2_CTRL_FLAG_WRITE_ONLY)
>>>            return fail("button control not write only\n");
>>> -        /* fall through */
>>> +        FALLTHROUGH;
>>>    case V4L2_CTRL_TYPE_BOOLEAN:
>>>    case V4L2_CTRL_TYPE_MENU:
>>>    case V4L2_CTRL_TYPE_INTEGER_MENU:
>>> @@ -124,7 +130,7 @@ static int checkQCtrl(struct node *node, struct test_query_ext_ctrl &qctrl)
>>>    case V4L2_CTRL_TYPE_BITMASK:
>>>        if (fl & V4L2_CTRL_FLAG_SLIDER)
>>>            return fail("slider makes only sense for integer controls\n");
>>> -        /* fall through */
>>> +        FALLTHROUGH;
>>>    case V4L2_CTRL_TYPE_INTEGER64:
>>>    case V4L2_CTRL_TYPE_INTEGER:
>>>        if ((fl & rw_mask) == rw_mask)
>>> diff --git a/utils/v4l2-compliance/v4l2-test-formats.cpp b/utils/v4l2-compliance/v4l2-test-formats.cpp
>>> index 3fc87316..9e98cf99 100644
>>> --- a/utils/v4l2-compliance/v4l2-test-formats.cpp
>>> +++ b/utils/v4l2-compliance/v4l2-test-formats.cpp
>>> @@ -32,6 +32,12 @@
>>> #include <assert.h>
>>> #include "v4l2-compliance.h"
>>>
>>> +#ifdef __clang__
>>> +#define FALLTHROUGH [[clang::fallthrough]]
>>> +#else
>>> +#define FALLTHROUGH [[gnu::fallthrough]]
>>> +#endif
>>> +
>>> static const __u32 buftype2cap[] = {
>>>    0,
>>>    V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_VIDEO_M2M,
>>> @@ -97,7 +103,7 @@ static int testEnumFrameIntervals(struct node *node, __u32 pixfmt,
>>>        case V4L2_FRMIVAL_TYPE_CONTINUOUS:
>>>            if (sw->step.numerator != 1 || sw->step.denominator != 1)
>>>                return fail("invalid step for continuous frameinterval\n");
>>> -            /* fallthrough */
>>> +            FALLTHROUGH;
>>>        case V4L2_FRMIVAL_TYPE_STEPWISE:
>>>            if (frmival.index)
>>>                return fail("index must be 0 for stepwise/continuous frameintervals\n");
>>> @@ -183,7 +189,7 @@ static int testEnumFrameSizes(struct node *node, __u32 pixfmt)
>>>        case V4L2_FRMSIZE_TYPE_CONTINUOUS:
>>>            if (frmsize.stepwise.step_width != 1 || frmsize.stepwise.step_height != 1)
>>>                return fail("invalid step_width/height for continuous framesize\n");
>>> -            /* fallthrough */
>>> +            FALLTHROUGH;
>>>        case V4L2_FRMSIZE_TYPE_STEPWISE:
>>>            if (frmsize.index)
>>>                return fail("index must be 0 for stepwise/continuous framesizes\n");
>>> diff --git a/utils/v4l2-ctl/v4l2-ctl-streaming.cpp b/utils/v4l2-ctl/v4l2-ctl-streaming.cpp
>>> index 6981a3cc..bb464e99 100644
>>> --- a/utils/v4l2-ctl/v4l2-ctl-streaming.cpp
>>> +++ b/utils/v4l2-ctl/v4l2-ctl-streaming.cpp
>>> @@ -24,6 +24,12 @@
>>> #include <media-info.h>
>>> #include <fwht-ctrls.h>
>>>
>>> +#ifdef __clang__
>>> +#define FALLTHROUGH [[clang::fallthrough]]
>>> +#else
>>> +#define FALLTHROUGH [[gnu::fallthrough]]
>>> +#endif
>>> +
>>> extern "C" {
>>> #include "v4l2-tpg.h"
>>> }
>>> @@ -763,7 +769,7 @@ void streaming_cmd(int ch, char *optarg)
>>>        break;
>>>    case OptStreamUser:
>>>        memory = V4L2_MEMORY_USERPTR;
>>> -        /* fall through */
>>> +        FALLTHROUGH;
>>>    case OptStreamMmap:
>>>        if (optarg) {
>>>            reqbufs_count_cap = strtoul(optarg, 0L, 0);
>>> @@ -776,7 +782,7 @@ void streaming_cmd(int ch, char *optarg)
>>>        break;
>>>    case OptStreamOutUser:
>>>        out_memory = V4L2_MEMORY_USERPTR;
>>> -        /* fall through */
>>> +        FALLTHROUGH;
>>>    case OptStreamOutMmap:
>>>        if (optarg) {
>>>            reqbufs_count_out = strtoul(optarg, 0L, 0);
>>> diff --git a/utils/v4l2-ctl/v4l2-ctl-vbi.cpp b/utils/v4l2-ctl/v4l2-ctl-vbi.cpp
>>> index c5960f78..82d01b58 100644
>>> --- a/utils/v4l2-ctl/v4l2-ctl-vbi.cpp
>>> +++ b/utils/v4l2-ctl/v4l2-ctl-vbi.cpp
>>> @@ -16,6 +16,12 @@
>>>
>>> #include "v4l2-ctl.h"
>>>
>>> +#ifdef __clang__
>>> +#define FALLTHROUGH [[clang::fallthrough]]
>>> +#else
>>> +#define FALLTHROUGH [[gnu::fallthrough]]
>>> +#endif
>>> +
>>> static struct v4l2_format sliced_fmt;      /* set_format/get_format for sliced VBI */
>>> static struct v4l2_format sliced_fmt_out; /* set_format/get_format for sliced VBI output */
>>> static struct v4l2_format raw_fmt;      /* set_format/get_format for VBI */
>>> @@ -87,7 +93,7 @@ void vbi_cmd(int ch, char *optarg)
>>>    case OptSetSlicedVbiOutFormat:
>>>    case OptTrySlicedVbiOutFormat:
>>>        sliced = &sliced_fmt_out;
>>> -        /* fall through */
>>> +        FALLTHROUGH;
>>>    case OptSetSlicedVbiFormat:
>>>    case OptTrySlicedVbiFormat:
>>>        sliced->fmt.sliced.service_set = 0;
>>> @@ -130,7 +136,7 @@ void vbi_cmd(int ch, char *optarg)
>>>    case OptSetVbiOutFormat:
>>>    case OptTryVbiOutFormat:
>>>        raw = &raw_fmt_out;
>>> -        /* fall through */
>>> +        FALLTHROUGH;
>>>    case OptSetVbiFormat:
>>>    case OptTryVbiFormat:
>>>        subs = optarg;
>>>
>>

Re: [PATCH 09/12] utils: fix implicit float conversions

[Hans Verkuil]

On 23/04/2020 10:44, Rosen Penev wrote:
> 
> 
>> On Apr 23, 2020, at 1:36 AM, Hans Verkuil <hverkuil@xs4all.nl> wrote:
>>
>> On 22/04/2020 02:37, Rosen Penev wrote:
>>> Found with -Wfloat-conversion
>>>
>>> Signed-off-by: Rosen Penev <rosenp@gmail.com>
>>> ---
>>> utils/cec-ctl/cec-ctl.cpp                   | 4 ++--
>>> utils/cec-ctl/cec-pin.cpp                   | 2 +-
>>> utils/v4l2-compliance/v4l2-test-buffers.cpp | 2 +-
>>> utils/v4l2-ctl/v4l2-ctl-misc.cpp            | 4 ++--
>>> utils/v4l2-ctl/v4l2-ctl-streaming.cpp       | 2 +-
>>> utils/v4l2-ctl/v4l2-ctl-subdev.cpp          | 2 +-
>>> 6 files changed, 8 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/utils/cec-ctl/cec-ctl.cpp b/utils/cec-ctl/cec-ctl.cpp
>>> index f4133494..f0e31aca 100644
>>> --- a/utils/cec-ctl/cec-ctl.cpp
>>> +++ b/utils/cec-ctl/cec-ctl.cpp
>>> @@ -1507,8 +1507,8 @@ static void stress_test_power_cycle(struct node &node, unsigned cnt,
>>>    srandom(seed);
>>>
>>>    for (;;) {
>>> -        unsigned usecs1 = mod_usleep ? random() % mod_usleep : sleep_before_on * 1000000;
>>> -        unsigned usecs2 = mod_usleep ? random() % mod_usleep : sleep_before_off * 1000000;
>>> +        unsigned usecs1 = mod_usleep ? random() % mod_usleep : (unsigned)(sleep_before_on * 1000000);
>>> +        unsigned usecs2 = mod_usleep ? random() % mod_usleep : (unsigned)(sleep_before_off * 1000000);
>>
>> Shouldn't this be static_cast<unsigned>? Same elsewhere.
> Sure, but that’s beyond this patch. It should be unsigned int actually. Implicit int is bad.

Why is that beyond this patch?

I never understood why unsigned int is somehow magically better than unsigned.

In any case, just keep unsigned since that's consistent with the existing practice
in these sources.

Regards,

	Hans

>>
>> Regards,
>>
>>    Hans
>>
>>>
>>>        usecs1 += min_usleep;
>>>        usecs2 += min_usleep;
>>> diff --git a/utils/cec-ctl/cec-pin.cpp b/utils/cec-ctl/cec-pin.cpp
>>> index 0322ad5e..10abea37 100644
>>> --- a/utils/cec-ctl/cec-pin.cpp
>>> +++ b/utils/cec-ctl/cec-pin.cpp
>>> @@ -261,7 +261,7 @@ static void cec_pin_rx_data_bit_was_low(__u64 ev_ts, __u64 usecs, __u64 usecs_mi
>>>     * If the low drive starts at the end of a 0 bit, then the actual
>>>     * maximum time that the bus can be low is the two summed.
>>>     */
>>> -    const unsigned max_low_drive = CEC_TIM_LOW_DRIVE_ERROR_MAX +
>>> +    const unsigned max_low_drive = (unsigned)CEC_TIM_LOW_DRIVE_ERROR_MAX +
>>>        CEC_TIM_DATA_BIT_0_LOW_MAX + CEC_TIM_MARGIN;
>>>
>>>    low_usecs = usecs;
>>> diff --git a/utils/v4l2-compliance/v4l2-test-buffers.cpp b/utils/v4l2-compliance/v4l2-test-buffers.cpp
>>> index 93df7034..87c2e523 100644
>>> --- a/utils/v4l2-compliance/v4l2-test-buffers.cpp
>>> +++ b/utils/v4l2-compliance/v4l2-test-buffers.cpp
>>> @@ -2597,7 +2597,7 @@ static void streamFmt(struct node *node, __u32 pixelformat, __u32 w, __u32 h,
>>>    char hz[32] = "";
>>>
>>>    if (!frame_count)
>>> -        frame_count = f ? 1.0 / fract2f(f) : 10;
>>> +        frame_count = f ? (unsigned)(1.0f / fract2f(f)) : 10;
>>>    node->g_fmt(fmt);
>>>    fmt.s_pixelformat(pixelformat);
>>>    fmt.s_width(w);
>>> diff --git a/utils/v4l2-ctl/v4l2-ctl-misc.cpp b/utils/v4l2-ctl/v4l2-ctl-misc.cpp
>>> index cb933217..2851886a 100644
>>> --- a/utils/v4l2-ctl/v4l2-ctl-misc.cpp
>>> +++ b/utils/v4l2-ctl/v4l2-ctl-misc.cpp
>>> @@ -320,7 +320,7 @@ void misc_set(cv4l_fd &_fd)
>>>        parm.type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
>>>        parm.parm.capture.timeperframe.numerator = 1000;
>>>        parm.parm.capture.timeperframe.denominator =
>>> -            fps * parm.parm.capture.timeperframe.numerator;
>>> +            (uint32_t)(fps * parm.parm.capture.timeperframe.numerator);
>>>
>>>        if (doioctl(fd, VIDIOC_S_PARM, &parm) == 0) {
>>>            struct v4l2_fract *tf = &parm.parm.capture.timeperframe;
>>> @@ -338,7 +338,7 @@ void misc_set(cv4l_fd &_fd)
>>>        parm.type = V4L2_BUF_TYPE_VIDEO_OUTPUT;
>>>        parm.parm.output.timeperframe.numerator = 1000;
>>>        parm.parm.output.timeperframe.denominator =
>>> -            output_fps * parm.parm.output.timeperframe.numerator;
>>> +            (uint32_t)(output_fps * parm.parm.output.timeperframe.numerator);
>>>
>>>        if (doioctl(fd, VIDIOC_S_PARM, &parm) == 0) {
>>>            struct v4l2_fract *tf = &parm.parm.output.timeperframe;
>>> diff --git a/utils/v4l2-ctl/v4l2-ctl-streaming.cpp b/utils/v4l2-ctl/v4l2-ctl-streaming.cpp
>>> index 066a336a..6981a3cc 100644
>>> --- a/utils/v4l2-ctl/v4l2-ctl-streaming.cpp
>>> +++ b/utils/v4l2-ctl/v4l2-ctl-streaming.cpp
>>> @@ -569,7 +569,7 @@ static void print_concise_buffer(FILE *f, cv4l_buffer &buf, cv4l_fmt &fmt,
>>>    if (!skip_ts && (buf.g_flags() & V4L2_BUF_FLAG_TIMESTAMP_MASK) != V4L2_BUF_FLAG_TIMESTAMP_COPY) {
>>>        double ts = buf.g_timestamp().tv_sec + buf.g_timestamp().tv_usec / 1000000.0;
>>>        fprintf(f, " ts: %.06f", ts);
>>> -        if (last_ts)
>>> +        if (last_ts <= 0.0)
>>>            fprintf(f, " delta: %.03f ms", (ts - last_ts) * 1000.0);
>>>        last_ts = ts;
>>>
>>> diff --git a/utils/v4l2-ctl/v4l2-ctl-subdev.cpp b/utils/v4l2-ctl/v4l2-ctl-subdev.cpp
>>> index 747f1699..f1223084 100644
>>> --- a/utils/v4l2-ctl/v4l2-ctl-subdev.cpp
>>> +++ b/utils/v4l2-ctl/v4l2-ctl-subdev.cpp
>>> @@ -487,7 +487,7 @@ void subdev_set(cv4l_fd &_fd)
>>>            exit(1);
>>>        }
>>>        fival.interval.numerator = 1000;
>>> -        fival.interval.denominator = set_fps * fival.interval.numerator;
>>> +        fival.interval.denominator = (uint32_t)(set_fps * fival.interval.numerator);
>>>        printf("Note: --set-subdev-fps is only for testing.\n"
>>>               "Normally media-ctl is used to configure the video pipeline.\n");
>>>        printf("ioctl: VIDIOC_SUBDEV_S_FRAME_INTERVAL (pad=%u)\n", fival.pad);
>>>
>>

Re: [PATCH 07/12] utils: fix implicit double promotion

[Hans Verkuil]

On 22/04/2020 02:37, Rosen Penev wrote:
> Found with -Wimplicit-float-conversion
> 
> Signed-off-by: Rosen Penev <rosenp@gmail.com>
> ---
>  utils/rds-ctl/rds-ctl.cpp | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/utils/rds-ctl/rds-ctl.cpp b/utils/rds-ctl/rds-ctl.cpp
> index 0e497b2a..cc1d3bf7 100644
> --- a/utils/rds-ctl/rds-ctl.cpp
> +++ b/utils/rds-ctl/rds-ctl.cpp
> @@ -525,7 +525,7 @@ static void print_rds_statistics(const struct v4l2_rds_statistics *statistics)
>  
>  	if (statistics->block_cnt)
>  		block_error_percentage =
> -			(static_cast<float>(statistics->block_error_cnt) / statistics->block_cnt) * 100.0;
> +			(static_cast<float>(statistics->block_error_cnt) / statistics->block_cnt) * 100.0f;
>  	printf("block errors / group errors: %u (%3.2f%%) / %u \n",
>  		statistics->block_error_cnt,
>  		block_error_percentage, statistics->group_error_cnt);
> @@ -534,7 +534,7 @@ static void print_rds_statistics(const struct v4l2_rds_statistics *statistics)
>  
>  	if (statistics->block_cnt)
>  		block_corrected_percentage = (
> -			static_cast<float>(statistics->block_corrected_cnt) / statistics->block_cnt) * 100.0;
> +			static_cast<float>(statistics->block_corrected_cnt) / statistics->block_cnt) * 100.0f;
>  	printf("corrected blocks: %u (%3.2f%%)\n",
>  		statistics->block_corrected_cnt, block_corrected_percentage);
>  	for (int i = 0; i < 16; i++)
> 

I think this patch can be removed once the type of block_error_percentage and
block_corrected_percentage is changed to double, so I am skipping this patch.

Regards,

	Hans