From 80f83f74d5db83a6cda65ad60f1673d1b90d7323 Mon Sep 17 00:00:00 2001 From: Josuah Demangeon Date: Wed, 12 Feb 2025 00:12:05 +0100 Subject: [PATCH 1/2] drivers: video: introduce an API for collecting statistics Introduce an abstraction layer handling the diversity of ways hardware have to report statistics. This allows to take advantage of the various channel average or histograms present on some hardware, that skip the need to manually compute statistics. Fixes #85457 Signed-off-by: Josuah Demangeon --- include/zephyr/drivers/video.h | 113 ++++++++++++++++++++++++++++++++- 1 file changed, 112 insertions(+), 1 deletion(-) diff --git a/include/zephyr/drivers/video.h b/include/zephyr/drivers/video.h index 67212110ebcd..abcde2ee748f 100644 --- a/include/zephyr/drivers/video.h +++ b/include/zephyr/drivers/video.h @@ -25,7 +25,6 @@ #include #include #include - #include #ifdef __cplusplus @@ -205,6 +204,71 @@ struct video_frmival_enum { }; }; +/** Custom statistics allowing applications to implement their own format. */ +#define VIDEO_STATS_CUSTOM BIT(0) + +/** Channel statistics for the Y (luma) channel. */ +#define VIDEO_STATS_CHANNELS_Y BIT(1) + +/** Channel statistics for the R, G, B channels. */ +#define VIDEO_STATS_CHANNELS_RGB BIT(2) + +/** Bitmap to ask for all statistics compatible with @struct video_stats_channels */ +#define VIDEO_STATS_CHANNELS (VIDEO_STATS_CHANNELS_RGB | VIDEO_STATS_CHANNELS_Y) + +/** Statistics in the form of an histogram with the Y (luma) channel present. */ +#define VIDEO_STATS_HISTOGRAM_Y BIT(3) + +/** Statistics in the form of an histogram with the R, G, B channels present. */ +#define VIDEO_STATS_HISTOGRAM_RGB BIT(4) + +/** Bitmap to ask for one of the statistics compatible with @struct video_stats_histogram */ +#define VIDEO_STATS_HISTOGRAM (VIDEO_STATS_HISTOGRAM_RGB | VIDEO_STATS_HISTOGRAM_Y) + +/** + * @brief Statistics base type, present as the first field of the other types. + * + * This type is to be casted to one of the other "video_..._stats" types. This permits to define + * new custom types in application and still use the upstream video API. + */ +struct video_stats { + /** Bitmak that describes the type of the stats filled */ + uint16_t flags; + /** Frame counter to know if a frame elapsed since the last call. Quickly overflowing. */ + uint16_t frame_counter; +}; + +/** + * @brief Per-channel average values. + * + * Each field represent 8-bit integer values that corresponds to the average value of a channel. + */ +struct video_stats_channels { + /** Base structure with fields common to all types of statistics. */ + struct video_stats base; + /** The luma channel average. */ + uint8_t y; + /** RGB24-formatted averages. */ + uint8_t rgb[3]; +}; + +/** + * @brief Statistics about the video image color content. + * + * Used by software algorithms to control the color balance such as White Balance (AWB), + * Black Level Correction (BLC), or control sensors such as Exposure/Gain Control (AEC/AGC). + */ +struct video_stats_histogram { + /** Base structure with fields common to all types of statistics. */ + struct video_stats base; + /** The histogram content for the Y or the R, G, B channels as defined by @c base.flags. */ + uint16_t *buckets; + /** Total number of values in @c buckets. */ + size_t num_buckets; + /** Total number of values added to the historam. */ + uint32_t num_values; +}; + /** * @brief video_endpoint_id enum * @@ -344,6 +408,19 @@ typedef int (*video_api_get_caps_t)(const struct device *dev, enum video_endpoin typedef int (*video_api_set_signal_t)(const struct device *dev, enum video_endpoint_id ep, struct k_poll_signal *signal); +/** + * @typedef video_api_get_stats_t + * @brief Register/Unregister poll signal for buffer events. + * + * See video_set_signal() for argument descriptions. + * @param dev Pointer to the device structure. + * @param ep Endpoint ID. + * @param stats Pointer to the statistics structure, which must have enough storage for the + type of stats requested by the @p stats flags. + */ +typedef int (*video_api_get_stats_t)(const struct device *dev, enum video_endpoint_id ep, + struct video_stats *stats); + __subsystem struct video_driver_api { /* mandatory callbacks */ video_api_set_format_t set_format; @@ -360,6 +437,7 @@ __subsystem struct video_driver_api { video_api_set_frmival_t set_frmival; video_api_get_frmival_t get_frmival; video_api_enum_frmival_t enum_frmival; + video_api_get_stats_t get_stats; }; /** @@ -496,6 +574,39 @@ static inline int video_enum_frmival(const struct device *dev, enum video_endpoi return api->enum_frmival(dev, ep, fie); } +/** + * @brief Get image statistics out of the video devices. + * + * This permits to implement algorithms reacting to statistics about the images + * collected by the hadware. For instance, in order to implement an image signal + * processor with auto-controls (AEC, AGC, AWB...). + * + * The driver will read the @p stats flags fields to learn about what the caller wants for + * statistics, and update them with the statistics effectively added. + * + * All memory buffers of @p stats are to be provided by the caller, and the driver will update + * the @c size field with the size effectively filled with statistics data. + * + * @param dev Pointer to the device structure that collects the statistics. + * @param ep Endpoint ID from which collect the statistics. + * @param stats Pointer to a video statistic structure filled by this device. + * + * @retval 0 If successful. + * @retval -ENOSYS If API is not implemented. + * @return other error number otherwise. + */ +static inline int video_get_stats(const struct device *dev, enum video_endpoint_id ep, + struct video_stats *stats) +{ + const struct video_driver_api *api = (const struct video_driver_api *)dev->api; + + if (api->get_stats == NULL) { + return -ENOSYS; + } + + return api->get_stats(dev, ep, stats); +} + /** * @brief Enqueue a video buffer. * From 0598b54ef0881c9b5a4c3dd448c0b82443655e81 Mon Sep 17 00:00:00 2001 From: Josuah Demangeon Date: Sun, 9 Mar 2025 00:43:21 +0000 Subject: [PATCH 2/2] drivers: video: emul_rx: add basic support for video_set_stats() Add support for the new API video_set_stats() to the emulated video RX driver, and use it to implement simple API tests. The data returned is arbitrary for the sake of testing the API itself. Signed-off-by: Josuah Demangeon --- drivers/video/video_emul_rx.c | 20 ++++++++++++++++ tests/drivers/video/api/src/video_emul.c | 29 ++++++++++++++++++++++++ 2 files changed, 49 insertions(+) diff --git a/drivers/video/video_emul_rx.c b/drivers/video/video_emul_rx.c index 7b7d1f580a72..94ab9fc5ca43 100644 --- a/drivers/video/video_emul_rx.c +++ b/drivers/video/video_emul_rx.c @@ -117,6 +117,25 @@ static int emul_rx_get_caps(const struct device *dev, enum video_endpoint_id ep, return video_get_caps(cfg->source_dev, VIDEO_EP_OUT, caps); } +static int emul_rx_get_stats(const struct device *dev, enum video_endpoint_id ep, + struct video_stats *stats) +{ + struct video_stats_channels *chan = (void *)stats; + + if ((stats->flags & VIDEO_STATS_CHANNELS_Y) == 0) { + return -ENOTSUP; + } + + /* Fake data for the sake of demonstrating and testing the APIs */ + chan->y = 0x7f; + stats->frame_counter = k_cycle_get_32() / 1024; + + /* Let the caller know what type of statistics is collected */ + stats->flags = VIDEO_STATS_CHANNELS_Y; + + return 0; +} + static int emul_rx_set_stream(const struct device *dev, bool enable) { const struct emul_rx_config *cfg = dev->config; @@ -238,6 +257,7 @@ static DEVICE_API(video, emul_rx_driver_api) = { .set_format = emul_rx_set_fmt, .get_format = emul_rx_get_fmt, .get_caps = emul_rx_get_caps, + .get_stats = emul_rx_get_stats, .set_stream = emul_rx_set_stream, .enqueue = emul_rx_enqueue, .dequeue = emul_rx_dequeue, diff --git a/tests/drivers/video/api/src/video_emul.c b/tests/drivers/video/api/src/video_emul.c index cd58bbf0506b..f6435496fad3 100644 --- a/tests/drivers/video/api/src/video_emul.c +++ b/tests/drivers/video/api/src/video_emul.c @@ -185,4 +185,33 @@ ZTEST(video_common, test_video_vbuf) video_buffer_release(vbuf); } +ZTEST(video_emul, test_video_stats) +{ + struct video_stats_channels chan = { + .base.flags = VIDEO_STATS_CHANNELS, + }; + + zexpect_ok(video_get_stats(rx_dev, VIDEO_EP_OUT, &chan.base), + "statistics collection should succeed for the emulated device"); + + zexpect_equal(chan.base.flags & VIDEO_STATS_HISTOGRAM, 0, "histogram was not requested"); + + zexpect_not_equal(chan.base.flags & VIDEO_STATS_CHANNELS, 0, + "this emulated device is known to support channel averages."); + + if (chan.base.flags & VIDEO_STATS_CHANNELS_Y) { + zexpect_not_equal(chan.y, 0x00, "Test data likely not completely black."); + zexpect_not_equal(chan.y, 0xff, "Test data likely not completely white."); + } + + if (chan.base.flags & VIDEO_STATS_CHANNELS_RGB) { + zexpect_not_equal(chan.rgb[0], 0x00, "Red channel likely not completely 0x00."); + zexpect_not_equal(chan.rgb[0], 0xff, "Red channel likely not completely 0xff."); + zexpect_not_equal(chan.rgb[1], 0x00, "Green channel likely not completely 0x00."); + zexpect_not_equal(chan.rgb[1], 0xff, "Green channel likely not completely 0xff."); + zexpect_not_equal(chan.rgb[2], 0x00, "Blue channel likely not completely 0x00."); + zexpect_not_equal(chan.rgb[2], 0xff, "Blue channel likely not completely 0xff."); + } +} + ZTEST_SUITE(video_emul, NULL, NULL, NULL, NULL, NULL);