drivers: video: common: fix video_closest_frmival() fie.index

drivers/video/video_common.c

@@ -128,16 +128,18 @@ void video_closest_frmival_stepwise(const struct video_frmival_stepwise *stepwis
 void video_closest_frmival(const struct device *dev, enum video_endpoint_id ep,
 			   struct video_frmival_enum *match)
 {
-	uint64_t best_diff_nsec = INT32_MAX;
 	struct video_frmival desired = match->discrete;
 	struct video_frmival_enum fie = {.format = match->format};
+	uint64_t best_diff_nsec = INT32_MAX;
+	uint64_t goal_nsec = video_frmival_nsec(&desired);
 
 	__ASSERT(match->type != VIDEO_FRMIVAL_TYPE_STEPWISE,
 		 "cannot find range matching the range, only a value matching the range");
 
-	while (video_enum_frmival(dev, ep, &fie) == 0) {
+	for (fie.index = 0; video_enum_frmival(dev, ep, &fie) == 0; fie.index++) {
 		struct video_frmival tmp = {0};
-		uint64_t diff_nsec = 0, a, b;
+		uint64_t diff_nsec = 0;
+		uint64_t tmp_nsec;
 
 		switch (fie.type) {
 		case VIDEO_FRMIVAL_TYPE_DISCRETE:
@@ -147,20 +149,21 @@ void video_closest_frmival(const struct device *dev, enum video_endpoint_id ep,
 			video_closest_frmival_stepwise(&fie.stepwise, &desired, &tmp);
 			break;
 		default:
-			__ASSERT(false, "invalid answer from the queried video device");
+			CODE_UNREACHABLE;
 		}
 
-		a = video_frmival_nsec(&desired);
-		b = video_frmival_nsec(&tmp);
-		diff_nsec = a > b ? a - b : b - a;
+		tmp_nsec = video_frmival_nsec(&tmp);
+		diff_nsec = tmp_nsec > goal_nsec ? tmp_nsec - goal_nsec : goal_nsec - tmp_nsec;
+
 		if (diff_nsec < best_diff_nsec) {
 			best_diff_nsec = diff_nsec;
-			memcpy(&match->discrete, &tmp, sizeof(tmp));
+			match->index = fie.index;
+			match->discrete = tmp;
+		}
 
-			/* The video_enum_frmival() function will increment fie.index every time.
-			 * Compensate for it to get the current index, not the next index.
-			 */
-			match->index = fie.index - 1;
+		if (diff_nsec == 0) {
+			/* Exact match, stop searching a better match */
+			break;
 		}
 	}
 }

drivers/video/video_emul_imager.c

@@ -277,7 +277,6 @@ static int emul_imager_enum_frmival(const struct device *dev, enum video_endpoin
 	fie->type = VIDEO_FRMIVAL_TYPE_DISCRETE;
 	fie->discrete.numerator = 1;
 	fie->discrete.denominator = mode->fps;
-	fie->index++;
 
 	return mode->fps == 0;
 }

tests/drivers/video/api/prj.conf

@@ -1,4 +1,5 @@
 CONFIG_ZTEST=y
+CONFIG_ASSERT=y
 CONFIG_VIDEO=y
 
 # Just enough for a single frame in RGB565 format: 320 * 420 * 2 + some margin

tests/drivers/video/api/src/video_emul.c

@@ -83,10 +83,10 @@ ZTEST(video_common, test_video_frmival)
 
 	/* Do a first enumeration of frame intervals, expected to work */
 	zexpect_ok(video_enum_frmival(imager_dev, VIDEO_EP_OUT, &fie));
-	zexpect_equal(fie.index, 1, "fie's index should increment by one at every iteration");
+	zexpect_equal(fie.index, 0, "fie's index should not increment on its own");
 
 	/* Test that every value of the frame interval enumerator can be applied */
-	do {
+	for (fie.index = 0; video_enum_frmival(imager_dev, VIDEO_EP_OUT, &fie) == 0; fie.index++) {
 		struct video_frmival q, a;
 		uint32_t min, max, step;
 
@@ -111,18 +111,25 @@ ZTEST(video_common, test_video_frmival)
 			for (q.numerator = min; q.numerator <= max; q.numerator += step) {
 				zexpect_ok(video_set_frmival(imager_dev, VIDEO_EP_OUT, &q));
 				zexpect_ok(video_get_frmival(imager_dev, VIDEO_EP_OUT, &a));
-				zexpect_equal(video_frmival_nsec(&q), video_frmival_nsec(&a));
+				zexpect_equal(video_frmival_nsec(&q), video_frmival_nsec(&a),
+					      "query %u/%u (%u nsec) answer %u/%u (%u nsec, sw)",
+					      q.numerator, q.denominator, video_frmival_nsec(&q),
+					      a.numerator, a.denominator, video_frmival_nsec(&a));
 			}
 			break;
 		case VIDEO_FRMIVAL_TYPE_DISCRETE:
 			/* There is just one frame interval to test */
-			zexpect_ok(video_set_frmival(imager_dev, VIDEO_EP_OUT, &fie.discrete));
+			memcpy(&q, &fie.discrete, sizeof(q));
+			zexpect_ok(video_set_frmival(imager_dev, VIDEO_EP_OUT, &q));
 			zexpect_ok(video_get_frmival(imager_dev, VIDEO_EP_OUT, &a));
 
-			zexpect_equal(video_frmival_nsec(&fie.discrete), video_frmival_nsec(&a));
+			zexpect_equal(video_frmival_nsec(&fie.discrete), video_frmival_nsec(&a),
+				      "query %u/%u (%u nsec) answer %u/%u (%u nsec, discrete)",
+				      q.numerator, q.denominator, video_frmival_nsec(&q),
+				      a.numerator, a.denominator, video_frmival_nsec(&a));
 			break;
 		}
-	} while (video_enum_frmival(imager_dev, VIDEO_EP_OUT, &fie) == 0);
+	}
 }
 
 ZTEST(video_common, test_video_ctrl)