video: Patch SDMA & capture sample to arbitrate height and width

[zacck]

The video capture sample does not work when using a combination of the FRDM-MCXN236 and the OV5640 camera and this is due to the sensor and the underlying interface having different defaults. This patch sets the interface resolution to that selected by the sample which in turn is set in the CONFIG.

[JarmouniA]

Please split this into 2 commits: driver changes, sample changes.
Also, explain why the driver change is needed in the commit message.

[zacck]

Please split this into 2 commits: driver changes, sample changes. Also, explain why the driver change is needed in the commit message.

Thank you, this is done.

[ngphibang]

Could you confirm again this sole commit fixes the issue? I don't have HW to test.

[zacck]

Could you confirm again this sole commit fixes the issue? I don't have HW to test.

Last I checked as stated on discord, no this does not, one needs the samples changes as well.

[zacck]

I’ll be at my lab in a couple of hours and will share the results.

[avolmat-st]

(oups, went too fast looking at previous comments, actually mine are close to some already made by @josuah)

[josuah]

Adding a DNM flag that can be removed as soon as #96986 (comment) is ok

[zacck]

Could you confirm again this sole commit fixes the issue? I don't have HW to test.

@ngphibang ok here are the results

without the samples commit

*** Booting Zephyr OS build v4.2.0-4814-gbafe2fbba202 ***
[00:00:00.125,000] <inf> main: Video device: video-sdma
[00:00:00.125,000] <inf> main: - Capabilities:
[00:00:00.125,000] <inf> main:   RGBP width [320; 320; 0] height [240; 240; 0]
[00:00:00.125,000] <inf> main: - Video format: RGBP 1280x720
[00:00:00.125,000] <err> nxp_video_sdma: Unsupported format
[00:00:00.125,000] <err> main: Unable to set format

with the samples commit

*** Booting Zephyr OS build v4.2.0-5041-gd0069f2abe68 ***
[00:00:00.125,000] <inf> main: Video device: video-sdma
[00:00:00.125,000] <inf> main: - Capabilities:
[00:00:00.125,000] <inf> main:   RGBP width [320; 320; 0] height [240; 240; 0]
[00:00:00.125,000] <inf> main: - Video format: RGBP 320x240
[00:00:00.139,000] <inf> main: - Supported frame intervals for the default format:
[00:00:00.139,000] <inf> main: - Supported controls:
[00:00:00.139,000] <inf> main:          device: ov5640@3c
[00:00:00.139,000] <inf> video_ctrls:                       Brightness 0x00980900 (int)    (flags=0x00) : min=-15 max=15 step=1 default=0 value=0
[00:00:00.139,000] <inf> video_ctrls:                         Contrast 0x00980901 (int)    (flags=0x00) : min=0 max=255 step=1 default=0 value=0
[00:00:00.139,000] <inf> video_ctrls:                       Saturation 0x00980902 (int)    (flags=0x00) : min=0 max=255 step=1 default=64 value=64
[00:00:00.139,000] <inf> video_ctrls:                              Hue 0x00980903 (int)    (flags=0x00) : min=0 max=359 step=1 default=0 value=0
[00:00:00.139,000] <inf> video_ctrls:                  Gain, Automatic 0x00980912 (bool)   (flags=0x10) : min=0 max=1 step=1 default=1 value=1
[00:00:00.139,000] <inf> video_ctrls:                  Horizontal Flip 0x00980914 (bool)   (flags=0x00) : min=0 max=1 step=1 default=0 value=0
[00:00:00.139,000] <inf> video_ctrls:                    Vertical Flip 0x00980915 (bool)   (flags=0x00) : min=0 max=1 step=1 default=0 value=0
[00:00:00.139,000] <inf> video_ctrls:             Power Line Frequency 0x00980918 (menu)   (flags=0x00) : min=0 max=3 step=1 default=1 value=1
[00:00:00.140,000] <inf> video_ctrls:                                  0: Disabled
[00:00:00.140,000] <inf> video_ctrls:                                  1: 50 Hz
[00:00:00.140,000] <inf> video_ctrls:                                  2: 60 Hz
[00:00:00.140,000] <inf> video_ctrls:                                  3: Auto
[00:00:00.140,000] <inf> video_ctrls:                    Analogue Gain 0x009e0903 (int)    (flags=0x0c) : min=0 max=1023 step=1 default=0 value=16
[00:00:00.140,000] <inf> video_ctrls:                       Pixel Rate 0x009f0902 (int64)  (flags=0x01) : min=24000000 max=96000000 step=1 default=48000000 value=24000000
[00:00:00.140,000] <inf> video_ctrls:                     Test Pattern 0x009f0903 (menu)   (flags=0x00) : min=0 max=4 step=1 default=0 value=0
[00:00:00.140,000] <inf> video_ctrls:                                  0: Disabled
[00:00:00.140,000] <inf> video_ctrls:                                  1: Color bars
[00:00:00.140,000] <inf> video_ctrls:                                  2: Color bars with rolling bar
[00:00:00.140,000] <inf> video_ctrls:                                  3: Color squares
[00:00:00.140,000] <inf> video_ctrls:                                  4: Color squares with rolling bar
[00:00:00.140,000] <inf> main: Capture started

cc @josuah @avolmat-st

let me know if I should add back the samples commit

[avolmat-st]

cc @josuah @avolmat-st

let me know if I should add back the samples commit

Do you mean the 2 proposals I mentioned above in the driver ? Well if that's easy and you have possibility to do that, why not ;)
Those are not blocking remarks so that is up to you.

[zacck]

cc @josuah @avolmat-st
let me know if I should add back the samples commit

Do you mean the 2 proposals I mentioned above in the driver ? Well if that's easy and you have possibility to do that, why not ;) Those are not blocking remarks so that is up to you.

Oh no no, your comments I will add but there is a commit that we are unclear on it looks like below

/frdm_mcxn236.conf
index 0f4576e59d1..ad449bf981a 100644
--- a/samples/drivers/video/capture/boards/frdm_mcxn236.conf
+++ b/samples/drivers/video/capture/boards/frdm_mcxn236.conf
@@ -1 +1,3 @@
 CONFIG_VIDEO_BUFFER_POOL_SZ_MAX=40000
+CONFIG_VIDEO_FRAME_WIDTH=320
+CONFIG_VIDEO_FRAME_HEIGHT=240

I am positive this is needed but Phi thinks otherwise and I do trust he knows why but I cant seem to make it work without this.

[zacck]

@avolmat-st @ngphibang @josuah I would appreciate another review round I think I applied all the required changes.

[ngphibang]

Although the 2nd commit is not really needed for the capture sample, configuring the frame width & height is still OK to ensure that if, for some reason, the sample application does not get the format before setting the format, it's still working.

BTW, could you change the commits title, to something like :

drivers: video: mcux_sdma: Reconfigure the source when getting format
Reconfigure the sensor format with the only supported format of the sdma when getting format

samples: video: capture: Configure frame width and heght for mcxn236

Thanks !

[zacck]

Although the 2nd commit is not really needed for the capture sample, configuring the frame width & height is still OK to ensure that if, for some reason, the sample application does not get the format before setting the format, it's still working.

BTW, could you change the commits title, to something like :

drivers: video: mcux_sdma: Reconfigure the source when getting format
Reconfigure the sensor format with the only supported format of the sdma when getting format

samples: video: capture: Configure frame width and heght for mcxn236

Thanks !

Can do!

[sonarqubecloud]

Quality Gate passed

Issues
0 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

[ngphibang]

The 2nd commit message needs a bit rewording but not a big problem to me. Thanks.

[josuah]

Thanks for the commits/testing/bugfix!
This will help working with more sensors.

[josuah]

Some light commit message rewording could be:

**With** SDMA as the video interface on this board**, it** requires
**the frame size** to be set for the sample to work

[ngphibang]

Removed DNM label as it seems @josuah has approved but forgot to do it.

[github-actions]

Hi @zacck!
Congratulations on getting your very first Zephyr pull request merged 🎉🥳. This is a fantastic achievement, and we're thrilled to have you as part of our community!

To celebrate this milestone and showcase your contribution, we'd love to award you the Zephyr Technical Contributor badge. If you're interested, please claim your badge by filling out this form: Claim Your Zephyr Badge.

Thank you for your valuable input, and we look forward to seeing more of your contributions in the future! 🪁