Skip to content
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.

Sign up for GitHub

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

Jump to bottom

drivers: video: Add VIDEO_BUF_STARTED to poll signals #78499

Closed
wants to merge 1 commit into from
Closed

drivers: video: Add VIDEO_BUF_STARTED to poll signals #78499

wants to merge 1 commit into from

Conversation

epc-ake
Copy link
Contributor

@epc-ake epc-ake commented Sep 16, 2024

Adding VIDEO_BUF_STARTED result to enum video_signal_result.

VIDEO_BUF_STARTED: Gets triggered at the start of buffer processing (e.g. start of capturing data)

Full disclosure:
I'm developing a sensor driver for a TOF image sensor and I need to change some settings on the driver after every frame.
The Idea would be to produce the VIDEO_BUF_STARTED in the video driver using the vsync or SOF interrupt and pulling for it in the sensor driver using a meta-IRQ thread.

@epc-ake
drivers: video: Add VIDEO_BUF_STARTED to poll signals
1000738 
Add VIDEO_BUF_STARTED event to video_signal_result.
VIDEO_BUF_STARTED should be triggered once processing
of the buffer has started (e.g. start_of_transmission/reception)

Signed-off-by: Armin Kessler <[email protected]>
@zephyrbot zephyrbot added the area: Video Video subsystem label Sep 16, 2024
@epc-ake epc-ake force-pushed the feature/signal_buffer_events branch from b52aec8 to 1000738 Compare September 16, 2024 16:28
@zephyrbot zephyrbot added the size: XS A PR changing only a single line of code label Sep 16, 2024
josuah
josuah previously approved these changes Sep 16, 2024
View reviewed changes
Copy link
Collaborator

@josuah josuah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A very light addition to an API that enables a new use-case.

@josuah josuah added the Trivial Changes that can be reviewed by anyone, i.e. doc changes, minor build system tweaks, etc. label Sep 16, 2024
@zephyrbot zephyrbot added the platform: ESP32 Espressif ESP32 label Sep 17, 2024
uLipe
uLipe requested changes Sep 17, 2024
View reviewed changes
drivers/video/Kconfig.esp32_dvp Outdated Show resolved Hide resolved
drivers/video/video_esp32_dvp.c Outdated Show resolved Hide resolved
@loicpoulain
Copy link
Collaborator

IMO the naming this VIDEO_BUF_STARTED is a bit misleading, wouldn't' make it sense to name it VIDEO_VSYNC or more generically VIDEO_START_OF_FRAME?

@epc-ake
Copy link
Contributor Author

epc-ake commented Sep 17, 2024

IMO the naming this VIDEO_BUF_STARTED is a bit misleading, wouldn't' make it sense to name it VIDEO_VSYNC or more generically VIDEO_START_OF_FRAME?

I had VIDEO_BUF_SOF and VIDEO_BUF_EOF first but then realized that VIDEO_BUF_EOF is the same as VIDEO_BUF_DONE.
I felt like VIDEO_BUF_STARTED fits more to the existing entries. But I can rename it to VIDEO_BUF_START_OF_FRAME if you really prefer.
I wouldn't call it VIDEO_VSYNC since other drivers might not have a vsync signal at the start of capturing image data. (e.g. mipi interfaces).

@epc-ake epc-ake changed the title drivers: video: Add VIDEO_BUF_STARTED to poll signals drivers: video: esp32s3: Add signals for vsync and image received Sep 17, 2024
@epc-ake epc-ake force-pushed the feature/signal_buffer_events branch from 8518b6b to 1000738 Compare September 17, 2024 13:34
@epc-ake epc-ake changed the title drivers: video: esp32s3: Add signals for vsync and image received drivers: video: Add VIDEO_BUF_STARTED to poll signals Sep 17, 2024
@epc-ake
Copy link
Contributor Author

epc-ake commented Sep 17, 2024

Sorry, I messed up and pushed to the wrong branch while I intended to make a new PR...
If you're interested, #78566 shows my intention for the new signal result on the esp32s3 video driver.

@uLipe uLipe mentioned this pull request Sep 17, 2024
Merged
@loicpoulain
Copy link
Collaborator

@epc-ake it still not entirely clear from your diagram where is the allowed window for changing the 'settings', and why it can not be done before/after enqueuing/dequeuing the buffer?

If I try to understand a bit better, you need a specific setting/control for each frame you're going to queue?

@epc-ake
Copy link
Contributor Author

epc-ake commented Sep 18, 2024 •
edited
Loading

If I try to understand a bit better, you need a specific setting/control for each frame you're going to queue?

Yes, I need to change a setting on my image sensor for every frame.
So during the time the image sensor is transmitting the captured image, I setup the needed settings for the next image.
And sure, this has to be done before the previous image has been transmitted completely.

@epc-ake
Copy link
Contributor Author

epc-ake commented Sep 19, 2024

I close this PR again since adding a new video_signal_result seems not to be the right way to achieve what I need.

@epc-ake epc-ake closed this Sep 19, 2024
@epc-ake epc-ake deleted the feature/signal_buffer_events branch September 19, 2024 08:52
@josuah josuah mentioned this pull request Nov 3, 2024
Draft
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@josuah josuah josuah left review comments

@uLipe uLipe uLipe requested changes

@loicpoulain loicpoulain Awaiting requested review from loicpoulain

@LucasTambor LucasTambor Awaiting requested review from LucasTambor

@marekmatej marekmatej Awaiting requested review from marekmatej

@raffarost raffarost Awaiting requested review from raffarost

@sylvioalves sylvioalves Awaiting requested review from sylvioalves

@wmrsouza wmrsouza Awaiting requested review from wmrsouza

Assignees

@sylvioalves sylvioalves

Labels
area: Video Video subsystem platform: ESP32 Espressif ESP32 size: XS A PR changing only a single line of code Trivial Changes that can be reviewed by anyone, i.e. doc changes, minor build system tweaks, etc.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@epc-ake @loicpoulain @uLipe @josuah @sylvioalves @zephyrbot