video: Improve control framework#96393
video: Improve control framework#96393ngphibang wants to merge 6 commits intozephyrproject-rtos:mainfrom
Conversation
Currently, control initialization is done in an unconsistent way, i.e. drivers need to pass some field values to video_init_ctrl() but still needs to set some other fields directly. Refactor video_init_ctrl() to make it more consistent, i.e. initializing all kinds of control (standard, menu, integer menu, custom) is now done by setting directly the video_ctrl's fields and using the same API. Signed-off-by: Phi Bang Nguyen <phibang.nguyen@nxp.com>
Add VIDEO_CTRL_FLAG_EXECUTE_ON_WRITE flag. Signed-off-by: Phi Bang Nguyen <phibang.nguyen@nxp.com>
Add VIDEO_CTRL_TYPE_BUTTON to support command-like controls. Signed-off-by: Phi Bang Nguyen <phibang.nguyen@nxp.com>
Add a name field to the video_ctrl struct to allow naming custom controls. Signed-off-by: Phi Bang Nguyen <phibang.nguyen@nxp.com> Signed-off-by: Armin Kessler <ake@espros.com>
The video_init_ctrl() function signature has been changed. Signed-off-by: Phi Bang Nguyen <phibang.nguyen@nxp.com>
Drop video_init_menu_ctrl() and video_init_int_menu_ctrl() Signed-off-by: Phi Bang Nguyen <phibang.nguyen@nxp.com>
zephyrbot
added
area: Video
zephyrbot
requested review from
avolmat-st,
cfriedt,
jhedberg,
josuah,
kartben and
loicpoulain
|
josuah
left a comment
josuah
left a comment
There was a problem hiding this comment.
This makes video control declaration quite trivial/intuitive, and still offer to change the definition at runtime before calling video_init_ctrl().
+1 from me for the new organization.
| for (uint8_t i = 0; i < ARRAY_SIZE(ctrls); i++) { | ||
| ret = video_init_ctrl(ctrls[i], dev); | ||
| if (ret) { | ||
| return ret; | ||
| } | ||
| } |
There was a problem hiding this comment.
It seems possible to do ctrls = (struct video_ctrl *)&data->ctrls and then loop like this:
for (uint8_t i = 0; i < sizeof(data->ctrls) / sizeof(*ctrls); i++) {
ret = video_init_ctrl(ctrls[i], dev);
if (ret) {
return ret;
}
}
|
In the next round, maybe I will change the function name |
|
|
||
| int video_init_int_menu_ctrl(struct video_ctrl *ctrl, const struct device *dev, uint32_t id, | ||
| uint8_t def, const int64_t menu[], size_t menu_len); | ||
| int video_init_ctrl(struct video_ctrl *ctrl, const struct device *const dev); |
There was a problem hiding this comment.
Would it be useful to add a size_t sz parameter that behaves like video_cluster_ctrl()?
This way, driver only have to make a single call:
return video_init_ctrl(&data->ctrls, sizeof(data->ctrls) / sizeof(struct video_ctrl), dev);
|
This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time. |
|
|
This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time. |
3 participants
Changes are for now applied on ov5640 and gc2145.
TODO: Apply changes on other camera sensor drivers