Skip to content

Commit fd3677d

Browse files
committed
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. Signed-off-by: Josuah Demangeon <me@josuah.net>
1 parent 2ece06a commit fd3677d

File tree

3 files changed

+16
-12
lines changed

3 files changed

+16
-12
lines changed

drivers/video/video_common.c

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,7 @@ void video_closest_frmival(const struct device *dev, enum video_endpoint_id ep,
144144
__ASSERT(match->type != VIDEO_FRMIVAL_TYPE_STEPWISE,
145145
"cannot find range matching the range, only a value matching the range");
146146

147-
while (video_enum_frmival(dev, ep, &fie) == 0) {
147+
for (fie.index = 0; video_enum_frmival(dev, ep, &fie) == 0; fie.index++) {
148148
struct video_frmival tmp = {0};
149149
uint64_t diff_nsec = 0, a, b;
150150

@@ -164,12 +164,8 @@ void video_closest_frmival(const struct device *dev, enum video_endpoint_id ep,
164164
diff_nsec = a > b ? a - b : b - a;
165165
if (diff_nsec < best_diff_nsec) {
166166
best_diff_nsec = diff_nsec;
167+
match->index = fie.index;
167168
memcpy(&match->discrete, &tmp, sizeof(tmp));
168-
169-
/* The video_enum_frmival() function will increment fie.index every time.
170-
* Compensate for it to get the current index, not the next index.
171-
*/
172-
match->index = fie.index - 1;
173169
}
174170
}
175171
}

tests/drivers/video/api/prj.conf

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
CONFIG_ZTEST=y
2+
CONFIG_ASSERT=y
23
CONFIG_VIDEO=y
34

45
# Just enough for a single frame in RGB565 format: 320 * 420 * 2 + some margin

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

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -83,10 +83,10 @@ ZTEST(video_common, test_video_frmival)
8383

8484
/* Do a first enumeration of frame intervals, expected to work */
8585
zexpect_ok(video_enum_frmival(imager_dev, VIDEO_EP_OUT, &fie));
86-
zexpect_equal(fie.index, 1, "fie's index should increment by one at every iteration");
86+
zexpect_equal(fie.index, 0, "fie's index should not increment on its own");
8787

8888
/* Test that every value of the frame interval enumerator can be applied */
89-
do {
89+
for (fie.index = 0; video_enum_frmival(imager_dev, VIDEO_EP_OUT, &fie) == 0; fie.index++) {
9090
struct video_frmival q, a;
9191
uint32_t min, max, step;
9292

@@ -111,18 +111,25 @@ ZTEST(video_common, test_video_frmival)
111111
for (q.numerator = min; q.numerator <= max; q.numerator += step) {
112112
zexpect_ok(video_set_frmival(imager_dev, VIDEO_EP_OUT, &q));
113113
zexpect_ok(video_get_frmival(imager_dev, VIDEO_EP_OUT, &a));
114-
zexpect_equal(video_frmival_nsec(&q), video_frmival_nsec(&a));
114+
zexpect_equal(video_frmival_nsec(&q), video_frmival_nsec(&a),
115+
"query %u/%u (%u nsec) answer %u/%u (%u nsec, sw)",
116+
q.numerator, q.denominator, video_frmival_nsec(&q),
117+
a.numerator, a.denominator, video_frmival_nsec(&a));
115118
}
116119
break;
117120
case VIDEO_FRMIVAL_TYPE_DISCRETE:
118121
/* There is just one frame interval to test */
119-
zexpect_ok(video_set_frmival(imager_dev, VIDEO_EP_OUT, &fie.discrete));
122+
memcpy(&q, &fie.discrete, sizeof(q));
123+
zexpect_ok(video_set_frmival(imager_dev, VIDEO_EP_OUT, &q));
120124
zexpect_ok(video_get_frmival(imager_dev, VIDEO_EP_OUT, &a));
121125

122-
zexpect_equal(video_frmival_nsec(&fie.discrete), video_frmival_nsec(&a));
126+
zexpect_equal(video_frmival_nsec(&fie.discrete), video_frmival_nsec(&a),
127+
"query %u/%u (%u nsec) answer %u/%u (%u nsec, discrete)",
128+
q.numerator, q.denominator, video_frmival_nsec(&q),
129+
a.numerator, a.denominator, video_frmival_nsec(&a));
123130
break;
124131
}
125-
} while (video_enum_frmival(imager_dev, VIDEO_EP_OUT, &fie) == 0);
132+
}
126133
}
127134

128135
ZTEST(video_common, test_video_ctrl)

0 commit comments

Comments
 (0)