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

Fix bug introduced in 46a262ffe6 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 <me@josuah.net>
This commit is contained in:
Josuah Demangeon 2025-04-01 17:49:34 +00:00 committed by Benjamin Cabé
commit 84f0eec62e
4 changed files with 16 additions and 13 deletions

View file

@ -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;
}
}
}

View file

@ -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;
}

View file

@ -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

View file

@ -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)