Skip to content

Conversation

@ngphibang
Copy link
Contributor

@ngphibang ngphibang commented Oct 2, 2024

This PR enhances the current camera fixture testcase.

  • adds some minor fixes to the current camera fixture test
  • adds a colorbar pattern check to enhance the current camera fixture test. Most of camera sensors supports generating a color bar test pattern (ov5640, ov2640, ov7670, mt9m114, etc.). This can be used to automatically check if the image test pattern generated by a camera pipeline is correct or not which is helpful to detect issues in the camera pipeline.

@josuah
Copy link
Contributor

josuah commented Oct 2, 2024

I am still new to tests and samples, and try to find the balance between the two regarding this which I stumbled upon the other day:

if a sample is performing as expected, i.e. you are testing the sample, not the subsystem it builds on top
-- https://docs.zephyrproject.org/latest/samples/sample_definition_and_criteria.html

This is the first time I read about it, so might be quoting the wrong thing: it still sounds like a very comprehensive way to test the capture sample itself.

What a nice way to automatically test almost every video system!

Copy link
Contributor

@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.

It also looks much like what the display driver sample does:
https://github.com/zephyrproject-rtos/zephyr/blob/main/samples/drivers/display/src/main.c#L35-L41

Using the same CONFIG_TEST
https://docs.zephyrproject.org/latest/kconfig.html#CONFIG_TEST

Very glad to have this color conversion code at hand! Great universal sensor text fixture!

By testing the middle of the bars, this should be plenty robust for a wide range of image sensors, and tel whether the image arrived correctly or not.

@ngphibang ngphibang changed the title Camera fixture test Enhance camera fixture test Oct 3, 2024
@ngphibang ngphibang force-pushed the camera_fixture_test branch from cce2332 to 0b36788 Compare October 3, 2024 09:02
@ngphibang
Copy link
Contributor Author

ngphibang commented Oct 3, 2024

  • Thank you for pointing me to this RFC: Stop referring to operations performed by samples as "tests" #53861. Reading through this, my understanding is that sample should not be used as a test and continuously modified to test API changes and logs in a sample should not include "test". Otherwise, it does not say we should remove the existing harness (fixture) tests linked to each sample that I see in many samples (video, display, adc, etc.) right ? If so, this PR just consolidates / enhances the current camera harness fixture test.

It also looks much like what the display driver sample does:
https://github.com/zephyrproject-rtos/zephyr/blob/main/samples/drivers/display/src/main.c#L35-L41

=> In fact, I used the CONFIG_TEST in this PR but not sure 100% if it's a correct way. Thanks for pointing out that the display sample used this with similar purpose !

@ngphibang
Copy link
Contributor Author

@hakehuang Could you help review this ?

@ngphibang ngphibang requested a review from hakehuang October 3, 2024 09:50
@josuah
Copy link
Contributor

josuah commented Oct 4, 2024

I just came upon this library part of Zephyr project, and it features color conversion functions:
https://github.com/zephyrproject-rtos/zscilib?tab=readme-ov-file#colorimetry

Not sure how fit it is for a simple colorbar test like here though. There might be a few color conversion utilities needed by various drivers. For instance, video controls that are too low-level and hardware-specific to be exposed, requiring some light math and color conversion to be exposed as CIDs. 🤔

For another PR!

@ngphibang
Copy link
Contributor Author

@josuah : thanks ! Unfortunately, I went through this zscilib but didn't find an rgb_to_cielab function so that it could be used here.

Add mimxrt1170_evk@B to test platforms.

Signed-off-by: Phi Bang Nguyen <[email protected]>
The logs that the fixture test are based on are not printed anymore due
to recent changes from "printk" to "LOG_DBG". Change the log level so
that it can work again.

Remove "Capture started" as it is not a relevant test regex. Logs are
sometimes not correctly printed in the console, for example, we can see:

DEBUG   - DEVICE: muart:~$ [mapture started

As "Capture started" is a one-time log, fixture test will then fail due
to timeout.

Also, consolidate other regex.

Signed-off-by: Phi Bang Nguyen <[email protected]>
Currently, the camera fixture test can only tell whether data are dumped
onto memory. This is to check further if the image content is correct.

The algorithm is rather simple: compare the average color of each bar
of a generated pattern with a predefined target in CIELAB76 color
space (which is nearly perceptually uniform).

Signed-off-by: Phi Bang Nguyen <[email protected]>
@ngphibang ngphibang force-pushed the camera_fixture_test branch from 0b36788 to a9afd67 Compare October 10, 2024 08:58
@ngphibang
Copy link
Contributor Author

Rebased as #76144 was merged

@ngphibang ngphibang requested review from hakehuang and josuah October 15, 2024 10:12
Copy link
Contributor

@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.

This could be a first step before hardware-in-the-loop twister tests:
https://docs.zephyrproject.org/latest/develop/test/twister.html#running-tests-on-hardware

But for now, the video test infrastructure is not there:
https://github.com/zephyrproject-rtos/zephyr/tree/main/tests/drivers/video

Just a build-all target:
https://github.com/zephyrproject-rtos/zephyr/tree/main/tests/drivers/build_all/video

Is it meant to become a ztest in tests/ once the video test infrastructure is there or is this meant to stay in samples/?

+1 as before video tests are ready, this is the only way to integrate this code.

P.S.: not sure why I thought this was the SMPTE test pattern... 😅

@ngphibang
Copy link
Contributor Author

Is it meant to become a ztest in tests/ once the video test infrastructure is there or is this meant to stay in samples/?

I am not expert in test and don't know how it will be changed in the future. But IMO, this (camera fixture test) is totally based on the code of the sample and is used to test the sample (although it can reveal problems in the camera drivers) so it should be here. It's not the special case for video as we can see in other samples, there are often / always a sample.yaml which containing test cases.

@dleach02 dleach02 merged commit e67f2b1 into zephyrproject-rtos:main Oct 18, 2024
18 checks passed
@dleach02 dleach02 deleted the camera_fixture_test branch October 18, 2024 16:46
@ngphibang ngphibang mentioned this pull request Nov 13, 2024
ngphibang added a commit to nxp-upstream/zephyr that referenced this pull request Nov 14, 2024
This is added via PRs zephyrproject-rtos#79337 and zephyrproject-rtos#79263 to automatically check if a
colorbar pattern generated by a camera pipeline is correct or not.

Signed-off-by: Phi Bang Nguyen <[email protected]>
mmahadevan108 pushed a commit that referenced this pull request Nov 14, 2024
This is added via PRs #79337 and #79263 to automatically check if a
colorbar pattern generated by a camera pipeline is correct or not.

Signed-off-by: Phi Bang Nguyen <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: Samples Samples area: Video Video subsystem

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants