Allow STM32 DCMIPP to work with UVC sample #94562

avolmat-st · 2025-08-15T20:30:31Z

This PR contains various area commits.

add proper caps for the dcmipp pipe, in order to have them properly retrieved by application / uvc sample
introduce a new helper in charge of applying video_set_selection (target COMPOSE) prior to video_set_format
Ensure that buffer allocated by UVC sample app are properly aligned in order to work with drivers
Simple addition of several commonly used resolution in UVC when the device is advertising range of width/height

I keep this in draft since this is still work in progress and I'd like to open the discussion about those topics.
I added the video_set_compose_format helper (probably not the best name) in order to add automatically the call to set_selection. However we might want, in the same helper or not to go even further of that and, if the device support it use crop to achieve the format requested, there could also be an option to for example keep aspect-ratio, which do the heavy lifting of checking the native resolution and perform the crop / compose in order to get a frame with the requested resolution while keeping aspect-ratio (if it has been asked to).

I used this new helper on the UVC sample as well, making it easy to select the right resolution.

When a device is advertising a range of width/height, the current UVC sample only expose the max and min resolutions (assuming they can be achieved from a memory point of view).
There would be various way to achieve computing of intermediate resolutions, computing the aspect ratio as well, depending on the step value advertised by the device the amount of intermediate resolution requested (or should I say limited).
In a trial to keep it simple, I put instead a list of commonly used resolution and have the sample app simply check if this can be achieved or not.

This PR depends on 2 other PRs, PR #93192 and PR #94081 so I merged them at the beginning so only the last 6 commits are relevant in this serie.

sonarqubecloud · 2025-08-15T20:34:13Z

Quality Gate passed

Issues
2 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

avolmat-st · 2025-10-02T20:02:33Z

Simply rebased for the time being, removing the DCMIPP Planar commit which are not already merged.

ngphibang

LGTM. Thanks

avolmat-st · 2025-10-23T13:42:26Z

Updated, simply rebase the 6 commits of this PR on top of the commits of PR #93192.
Only modification (compare to my previous push) in this PR is to check the return value in the following added code in the sample

                        ret = app_add_format(vcap->pixelformat, video_common_fmts[j].width,
                                             video_common_fmts[j].height, has_sup_fmts);
                        if (ret != 0) {
                                return ret;
                        }

josuah · 2025-10-23T14:16:08Z

~~@avolmat-st do you wish to close this current PR to centralize the review on the other PR #97425?~~
[EDIT: looks like we are reviewing them in order without skipping this PR after all]

josuah

Removed DNM as dependencies are merged or on merge-list.

LGTM for the video additions, thank you!

Waiting @jfischer-no feedback and #93192 to be merged to +1

Currently the DCMIPP driver rely on a Kconfig in order to select the right sensor resolution / format to pick. This also makes the exposure of caps easier since it can be exposed as: DUMP pipe: same caps as mentioned in Kconfig MAIN pipe: any format supported on this pipe and resolution starting at sensor selected resolution down to 64 times smaller (which is the maximum of the downscale) AUX pipe: same as MAIN except without the semi-planar and planar formats Signed-off-by: Alain Volmat <[email protected]>

Some devices allow for downscale / upscale via the set_selection compose API. When using it, it is necessary to perform a set_selection of the compose target prior to setting the format. In order to allow non-compose aware application to benefit from it, introduce a helper which take care of setting the compose prior to setting the format. Signed-off-by: Alain Volmat <[email protected]>

Simplify the code by using the video_set_compose_format helper. Signed-off-by: Alain Volmat <[email protected]>

Honor the CONFIG_VIDEO_BUFFER_POOL_ALIGN config by using the video_buffer_aligned_alloc function instead of video_buffer_alloc in order to provide properly aligned buffers to drivers. Signed-off-by: Alain Volmat <[email protected]>

Use the helper video_set_compose_format in order to allow controlling the compose. Signed-off-by: Alain Volmat <[email protected]>

Select from commonly used resolution when the video device advertise capabilities using range. Signed-off-by: Alain Volmat <[email protected]>

samples/subsys/usb/uvc/src/main.c

etienne-lms · 2025-10-23T16:10:20Z

drivers/video/video_stm32_dcmipp.c

+				      STM32_DCMIPP_MAX_PIPE_SCALE_FACTOR),			\
+	.height_max = CONFIG_VIDEO_STM32_DCMIPP_SENSOR_HEIGHT,					\
+	.width_step = pixmul, .height_step = 1,							\
+}


This closing brace should be indented by a tab IMHO, since it's part of a macro.

etienne-lms · 2025-10-23T16:15:35Z

samples/drivers/video/capture/src/main.c

-	sel.rect.width = CONFIG_VIDEO_SOURCE_CROP_WIDTH;
-	sel.rect.height = CONFIG_VIDEO_SOURCE_CROP_HEIGHT;
-	if (video_set_selection(video_dev, &sel)) {
+	if (video_set_selection(video_dev, &crop_sel)) {


Prefer

if (video_set_selection(video_dev, &crop_sel) < 0) {

That is nonsense, even would introduce undefined behavior and -1 from my side.

What do you mean by an undefined behavior?
If you prefer:

if (video_set_selection(video_dev, &crop_sel) != 0) {

I'm fine too.

That is completely different proposal, at least for USB that would be preferable, but there is no difference between if (video_set_selection(video_dev, &crop_sel) != 0) { and if (video_set_selection(video_dev, &crop_sel)) {.

My mistake. I misised we're here in a demo sample.

etienne-lms · 2025-10-23T16:17:12Z

samples/drivers/video/capture/src/main.c

 		VIDEO_FOURCC_TO_STR(fmt.pixelformat), fmt.width, fmt.height);

-	if (video_set_format(video_dev, &fmt)) {
+	if (video_set_compose_format(video_dev, &fmt)) {


Ditto:

if (video_set_compose_format(video_dev, &fmt) < 0) {

Same as #94562 (comment)

sonarqubecloud · 2025-10-23T16:34:52Z

Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

etienne-lms · 2025-10-23T16:43:56Z

By the way, my review comments are non blocking (hence I did not send a "Request change" notif).
They can be addressed later, as cleanup.
I see it's approved by video gurus so fine as-is.

josuah · 2025-10-23T19:54:48Z

@etienne-lms

The previous style from the samples is from long ago and new PRs tried to be locally consistent (discussion at #95862 (comment) for instance).

Importing the video sample code as-is implies a PR to go fix that (#96072). But why not importing the sample code using the current Zephyr coding style directly. Thanks for pointing it out!

The large amount of code in samples is meant to be refactored into identical helpers for all samples in the meantime that libMP is proposed [video, pdf].

avolmat-st requested review from josuah and ngphibang August 15, 2025 20:32

avolmat-st force-pushed the pr-dcmipp-caps-uvc-range branch from fcd6732 to 371ef0e Compare October 2, 2025 20:01

avolmat-st force-pushed the pr-dcmipp-caps-uvc-range branch from 371ef0e to 99218f4 Compare October 11, 2025 13:07

avolmat-st marked this pull request as ready for review October 11, 2025 13:44

zephyrbot added Release Notes To be mentioned in the release notes platform: STM32 ST Micro STM32 area: Samples Samples area: USB Universal Serial Bus area: Video Video subsystem labels Oct 11, 2025

zephyrbot requested review from FRASTM, GeorgeCGV, cfriedt, djiatsaf-st, erwango, etienne-lms, gautierg-st, jfischer-no, jhedberg, loicpoulain, marwaiehm-st, mathieuchopstm and tmon-nordic October 11, 2025 13:46

zephyrbot assigned jfischer-no Oct 11, 2025

avolmat-st mentioned this pull request Oct 12, 2025

UVC Video encoder (zephyr,videoenc) + H264 support + STM32 STM32N6 conf files #97425

Merged

avolmat-st added this to the 4.3 milestone Oct 13, 2025

erwango assigned josuah and unassigned jfischer-no Oct 14, 2025

ngphibang previously approved these changes Oct 23, 2025

View reviewed changes

JarmouniA previously approved these changes Oct 23, 2025

View reviewed changes

avolmat-st dismissed stale reviews from JarmouniA and ngphibang via 5120e27 October 23, 2025 13:39

avolmat-st force-pushed the pr-dcmipp-caps-uvc-range branch from b0f77aa to 5120e27 Compare October 23, 2025 13:39

zephyrbot requested a review from ngphibang October 23, 2025 13:41

josuah removed the DNM This PR should not be merged (Do Not Merge) label Oct 23, 2025

josuah reviewed Oct 23, 2025

View reviewed changes

Alain Volmat added 6 commits October 23, 2025 17:57

samples: video: capture: use video_set_compose_format helper

49458fc

Simplify the code by using the video_set_compose_format helper. Signed-off-by: Alain Volmat <[email protected]>

samples: usb: uvc: use video_set_compose_format

a5971e1

Use the helper video_set_compose_format in order to allow controlling the compose. Signed-off-by: Alain Volmat <[email protected]>

samples: usb: uvc: select applicable resolutions from range

d2e3f66

Select from commonly used resolution when the video device advertise capabilities using range. Signed-off-by: Alain Volmat <[email protected]>

jfischer-no previously approved these changes Oct 23, 2025

View reviewed changes

avolmat-st dismissed jfischer-no’s stale review via d2e3f66 October 23, 2025 16:08

avolmat-st force-pushed the pr-dcmipp-caps-uvc-range branch from 5120e27 to d2e3f66 Compare October 23, 2025 16:08

zephyrbot requested review from jfischer-no and josuah October 23, 2025 16:09

josuah approved these changes Oct 23, 2025

View reviewed changes

jfischer-no approved these changes Oct 23, 2025

View reviewed changes

etienne-lms reviewed Oct 23, 2025

View reviewed changes

cfriedt merged commit 66b8139 into zephyrproject-rtos:main Oct 23, 2025
26 checks passed

avolmat-st mentioned this pull request Oct 24, 2025

Fix DCMIPP video driver compilation on STM32MP13 #98211

Open

josuah mentioned this pull request Oct 25, 2025

UVC support: USB Video Class to turn Arduino's into webcams arduino/ArduinoCore-zephyr#153

Closed

Allow STM32 DCMIPP to work with UVC sample #94562

Allow STM32 DCMIPP to work with UVC sample #94562

Conversation

avolmat-st commented Aug 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sonarqubecloud bot commented Aug 15, 2025

Quality Gate passed

Uh oh!

avolmat-st commented Oct 2, 2025

Uh oh!

ngphibang left a comment

Choose a reason for hiding this comment

Uh oh!

avolmat-st commented Oct 23, 2025

Uh oh!

josuah commented Oct 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

josuah left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud bot commented Oct 23, 2025

Quality Gate passed

Uh oh!

etienne-lms commented Oct 23, 2025

Uh oh!

josuah commented Oct 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

10 participants

avolmat-st commented Aug 15, 2025 •

edited

Loading

josuah commented Oct 23, 2025 •

edited

Loading

josuah commented Oct 23, 2025 •

edited

Loading