-
Notifications
You must be signed in to change notification settings - Fork 7.8k
Expose video devices to user #88182
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Expose video devices to user #88182
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,4 @@ | ||
#include <zephyr/linker/iterable_sections.h> | ||
|
||
ITERABLE_SECTION_RAM(video_device, Z_LINK_ITERABLE_SUBALIGN) | ||
ITERABLE_SECTION_RAM(video_mdev, Z_LINK_ITERABLE_SUBALIGN) |
Original file line number | Diff line number | Diff line change | ||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -16,13 +16,22 @@ struct video_device { | |||||||||||||||||||
sys_dlist_t ctrls; | ||||||||||||||||||||
}; | ||||||||||||||||||||
|
||||||||||||||||||||
struct video_mdev { | ||||||||||||||||||||
const struct device *dev; | ||||||||||||||||||||
}; | ||||||||||||||||||||
|
||||||||||||||||||||
#define VIDEO_DEVICE_DEFINE(name, device, source) \ | ||||||||||||||||||||
static STRUCT_SECTION_ITERABLE(video_device, name) = { \ | ||||||||||||||||||||
.dev = device, \ | ||||||||||||||||||||
.src_dev = source, \ | ||||||||||||||||||||
.ctrls = SYS_DLIST_STATIC_INIT(&name.ctrls), \ | ||||||||||||||||||||
} | ||||||||||||||||||||
|
||||||||||||||||||||
#define VIDEO_MDEV_DEFINE(name, device) \ | ||||||||||||||||||||
static STRUCT_SECTION_ITERABLE(video_mdev, name_##mdev) = { \ | ||||||||||||||||||||
.dev = device, \ | ||||||||||||||||||||
} | ||||||||||||||||||||
|
||||||||||||||||||||
Comment on lines
+30
to
+34
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. To reduce the boilerplate, it might be possible to combine the two declarations in one:
Suggested change
Then the user can select This also prevents to define a |
||||||||||||||||||||
struct video_device *video_find_vdev(const struct device *dev); | ||||||||||||||||||||
|
||||||||||||||||||||
#endif /* ZEPHYR_INCLUDE_DRIVERS_VIDEO_VIDEO_DEVICE_H_ */ |
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -92,22 +92,24 @@ int main(void) | |
size_t bsize; | ||
int i = 0; | ||
int err; | ||
|
||
#if DT_HAS_CHOSEN(zephyr_camera) | ||
const struct device *const video_dev = DEVICE_DT_GET(DT_CHOSEN(zephyr_camera)); | ||
|
||
if (!device_is_ready(video_dev)) { | ||
LOG_ERR("%s: video device is not ready", video_dev->name); | ||
return 0; | ||
const struct device *video_dev = NULL; | ||
const struct device *const vsg = device_get_binding(VIDEO_DEV_SW); | ||
uint8_t vdevs_num = video_get_devs_num(); | ||
bool found_vdev = false; | ||
|
||
/* Get the 1st video HW available, otherwise fallbacks to video sw generator */ | ||
josuah marked this conversation as resolved.
Show resolved
Hide resolved
|
||
for (int j = 0; j < vdevs_num; j++) { | ||
video_dev = video_get_dev(j); | ||
if (device_is_ready(video_dev) && (vdevs_num == 1 || video_dev != vsg)) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Given that |
||
found_vdev = true; | ||
break; | ||
} | ||
} | ||
#else | ||
const struct device *const video_dev = device_get_binding(VIDEO_DEV_SW); | ||
|
||
if (video_dev == NULL) { | ||
LOG_ERR("%s: video device not found or failed to initialized", VIDEO_DEV_SW); | ||
if (!found_vdev) { | ||
LOG_ERR("No video device ready!"); | ||
return 0; | ||
} | ||
#endif | ||
|
||
LOG_INF("Video device: %s", video_dev->name); | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -26,26 +26,29 @@ int main(void) | |
const struct device *video_dev; | ||
size_t bsize; | ||
int i = 0; | ||
const struct device *const vsg = device_get_binding(VIDEO_DEV_SW); | ||
uint8_t vdevs_num = video_get_devs_num(); | ||
bool found_vdev = false; | ||
|
||
display_dev = DEVICE_DT_GET(DT_CHOSEN(zephyr_display)); | ||
if (!device_is_ready(display_dev)) { | ||
LOG_ERR("Device not ready, aborting test"); | ||
return 0; | ||
} | ||
|
||
#if DT_HAS_CHOSEN(zephyr_camera) | ||
video_dev = DEVICE_DT_GET(DT_CHOSEN(zephyr_camera)); | ||
if (!device_is_ready(video_dev)) { | ||
LOG_ERR("%s device is not ready", video_dev->name); | ||
return 0; | ||
/* Get the 1st video HW available, otherwise fallbacks to video sw generator */ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It seems like this PR implements a fallback mechanism already, so is There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. vsg is also registered and listed by the video framework. So, if it is listed at first place and in case we have multiple video devices, we should continue the loop to get the 1st real hw video device, otherwise we stop the loop because we found the real hw device or we only have one video device (whatever it is sw or hw, we have no choice). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This makes sense. Great example of how to do filtering.
One How does an user control which device is at the first place? I think it is alphabetical sorting. Could there be 2 devices, with the first being the |
||
for (int j = 0; j < vdevs_num; j++) { | ||
video_dev = video_get_dev(j); | ||
if (device_is_ready(video_dev) && (vdevs_num == 1 || video_dev != vsg)) { | ||
found_vdev = true; | ||
break; | ||
} | ||
} | ||
#else | ||
video_dev = device_get_binding(VIDEO_DEV_SW); | ||
if (video_dev == NULL) { | ||
LOG_ERR("%s device not found", VIDEO_DEV_SW); | ||
|
||
if (!found_vdev) { | ||
LOG_ERR("No video device ready!"); | ||
return 0; | ||
} | ||
#endif | ||
|
||
LOG_INF("- Device name: %s", video_dev->name); | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about
video_interface
to communicate that this is the part of the video chain that an application interacts with. Hopefully reducing the jargon required to understand V4Z a little.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
An alternative is to use
STRUCT_SECTION_ITERABLE_ALTERNATE
so that defining a wrapper struct is not required. Instead, astruct device
is used, but with a dedicated section name such asvideo_mdev
orvideo_interface
:zephyr/include/zephyr/sys/iterable_sections.h
Lines 180 to 189 in c35bb0d
For instance here is how USB submits the same struct type but with different names for each USB speed:
zephyr/include/zephyr/usb/usbd.h
Lines 745 to 752 in c35bb0d
Both approach work depending on your goals, i.e. plan extra fields in the future or not.