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

Fix bug introduced in 46a262f where the
fie.index field was expected to be incremented by the driver, while it
is the responsibility of the caller to increment it.

Fixes #89663

Signed-off-by: Josuah Demangeon <[email protected]>

Author: josuah

drivers/video/video_common.c

@@ -135,7 +135,7 @@ void video_closest_frmival(const struct device *dev, enum video_endpoint_id ep,
 	__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;
 
@@ -155,12 +155,8 @@ void video_closest_frmival(const struct device *dev, enum video_endpoint_id ep,
 		diff_nsec = a > b ? a - b : b - a;
 		if (diff_nsec < best_diff_nsec) {
 			best_diff_nsec = diff_nsec;
+			match->index = fie.index;
 			memcpy(&match->discrete, &tmp, sizeof(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;
 		}
 	}
 }

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)