-
Notifications
You must be signed in to change notification settings - Fork 7.3k
drivers: video: shell: initial implementation #82393
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
Conversation
|
🤯 |
drivers/video/Kconfig.shell
Outdated
config VIDEO_SHELL | ||
bool "Video shell commands support" | ||
depends on SHELL | ||
default y |
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.
we typically don't enable shell modules by default
8af905d
to
aa38101
Compare
return -ENODEV; | ||
} | ||
|
||
if (!DEVICE_API_IS(video, dev)) { |
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.
since this depends on #82301, maybe include the associated commit in your PR for now?
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.
Hi @josuah, this looks promising. I didn't look into details yet but some of my quick ideas / observations:
I am working on another shell utility which looks like |
Thank you for your feedback!
The goal was to bring some preview as early as possible in the development: only RAM and an UART are needed.
This is the way forward! I think could not really realize it: a test utility needs to be able to stream, work on full pipelines rather than individual devices.
It does not do any interpolation, but it does skip pixels, working one a per-line basis.
-> I think
I did not know it! 👍
This PR #82393 helps early driver development: getting the first frame working, and follow it manually through a pipeline. Once a pipeline works, #82393 stops being helpful, something is missing to it! I will keep it as a draft to avoid getting it in the way of your version, and adapt it as needed, remove the parts that overlap or considered not useful. |
It's helpful. |
No confusion :) I mean I am not rushing this, and better leave unimplemented the stub commands if more comprehensive versions would come in another PR. What
|
aa38101
to
468f1f4
Compare
drivers/video/video_shell.c
Outdated
|
||
static void dcmd_video_device_name(size_t idx, struct shell_static_entry *entry) | ||
{ | ||
const struct device *dev = shell_device_lookup(idx, NULL); |
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.
you can make this smarter and only display video devices :)
zephyr/drivers/led/led_shell.c
Lines 332 to 341 in 84532b2
static bool device_is_led_and_ready(const struct device *dev) | |
{ | |
return device_is_ready(dev) && DEVICE_API_IS(led, dev); | |
} | |
static void device_name_get(size_t idx, struct shell_static_entry *entry) | |
{ | |
const struct device *dev = shell_device_filter(idx, device_is_led_and_ready); | |
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.
Nice!
uart:~$ video start
VIDEO_SW_GENERATOR video_emul_imager video_emul_rx
uart:~$ video start
Introduce a video_get_format_index() utility to help finding a caps entry out of a given format. Introduce several utilities to seek and apply frame intervals. Signed-off-by: Josuah Demangeon <me@josuah.net>
Add a new implementation of a test pattern generator, with the same architecture as real drivers: split receiver core and I2C-controlled sub-device, with changes of video format in "zephyr,emul-imager" leads to different data produced by "zephyr,emul-rx". Signed-off-by: Josuah Demangeon <me@josuah.net>
468f1f4
to
e2e76fa
Compare
Introduction of a video shell command, with subcommands to operate video APIs on a basic level, using a single buffer th be passed to a video device of choice and dumped as hexadecimal or ANSI colors. Signed-off-by: Josuah Demangeon <me@josuah.net>
e2e76fa
to
5e3db41
Compare
force-push:
|
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 will be re-written using the libpixel, proposed to Zephyr soon, so that all the color conversion present through the video tree (samples, shell, stats API and drivers, etc.) can use it. |
After a while I realize that the first proposal was way too bare-bone. After using your recommended Yavta utility, I miss it on Zephyr!
This would cover |
Dependencies:
Replaced by:
This is a draft of a video shell, a toolbox to operate video devices interactively for debug and troubleshooting purposes.
It is based on the definition of a single buffer, that is implicitly referred by all commands working on buffers.
alloc
,format
,frmimval
,enqueue/dequeue
,start/stop
families of API functions.DEVICE_API_IS(video, dev)
to avoid crashing when a non-matching API is used.native_sim
platform using the emulated video driversExample session:
For now, only previewing RGB565 is supported for the preview.
If the buffer is interpreted with the wrong pitch, so it is shown in diagonal lines (i.e. troubleshooting the pitch):
This makes a lower-level debug tap to debug video devices than UVC