From 43aedb6242f5c6375aeb52a36a40a7dfffdb8205 Mon Sep 17 00:00:00 2001 From: Josuah Demangeon Date: Sun, 6 Oct 2024 18:18:13 +0000 Subject: [PATCH 1/4] drivers: video: improve video formats documentation Introduce a new text documentation of the video formats, describing every bit, gives the position of the most significant bit, outline how it is cut in bytes, and show the pixel segmentation. Extra care is taken for the RGB565 which requies paying attention to the bit endianess as well as the byte endianess. Signed-off-by: Josuah Demangeon --- include/zephyr/drivers/video.h | 112 ++++++++++++++++++++++++++------- 1 file changed, 91 insertions(+), 21 deletions(-) diff --git a/include/zephyr/drivers/video.h b/include/zephyr/drivers/video.h index 959083a411a3d0..a3aa05bc051ec3 100644 --- a/include/zephyr/drivers/video.h +++ b/include/zephyr/drivers/video.h @@ -818,22 +818,51 @@ void video_closest_frmival(const struct device *dev, enum video_endpoint_id ep, /** * @defgroup video_pixel_formats Video pixel formats + * The @c | characters separate the pixels, and spaces separate the bytes. + * The uppercase letter represents the most significant bit. + * The lowercase letters represent the rest of the bits. * @{ */ /** - * @name Bayer formats + * @name Bayer formats (R, G, B channels). + * + * The full color information is spread over multiple pixels. + * * @{ */ -/** BGGR8 pixel format */ -#define VIDEO_PIX_FMT_BGGR8 video_fourcc('B', 'G', 'G', 'R') /* 8 BGBG.. GRGR.. */ -/** GBRG8 pixel format */ -#define VIDEO_PIX_FMT_GBRG8 video_fourcc('G', 'B', 'R', 'G') /* 8 GBGB.. RGRG.. */ -/** GRBG8 pixel format */ -#define VIDEO_PIX_FMT_GRBG8 video_fourcc('G', 'R', 'B', 'G') /* 8 GRGR.. BGBG.. */ -/** RGGB8 pixel format */ -#define VIDEO_PIX_FMT_RGGB8 video_fourcc('R', 'G', 'G', 'B') /* 8 RGRG.. GBGB.. */ +/** + * @verbatim + * | Bbbbbbbb | Gggggggg | Bbbbbbbb | Gggggggg | Bbbbbbbb | Gggggggg | ... + * | Gggggggg | Rrrrrrrr | Gggggggg | Rrrrrrrr | Gggggggg | Rrrrrrrr | ... + * @endverbatim + */ +#define VIDEO_PIX_FMT_BGGR8 VIDEO_FOURCC('B', 'A', '8', '1') + +/** + * @verbatim + * | Gggggggg | Bbbbbbbb | Gggggggg | Bbbbbbbb | Gggggggg | Bbbbbbbb | ... + * | Rrrrrrrr | Gggggggg | Rrrrrrrr | Gggggggg | Rrrrrrrr | Gggggggg | ... + * @endverbatim + */ +#define VIDEO_PIX_FMT_GBRG8 VIDEO_FOURCC('G', 'B', 'R', 'G') + +/** + * @verbatim + * | Gggggggg | Rrrrrrrr | Gggggggg | Rrrrrrrr | Gggggggg | Rrrrrrrr | ... + * | Bbbbbbbb | Gggggggg | Bbbbbbbb | Gggggggg | Bbbbbbbb | Gggggggg | ... + * @endverbatim + */ +#define VIDEO_PIX_FMT_GRBG8 VIDEO_FOURCC('G', 'R', 'B', 'G') + +/** + * @verbatim + * | Rrrrrrrr | Gggggggg | Rrrrrrrr | Gggggggg | Rrrrrrrr | Gggggggg | ... + * | Gggggggg | Bbbbbbbb | Gggggggg | Bbbbbbbb | Gggggggg | Bbbbbbbb | ... + * @endverbatim + */ +#define VIDEO_PIX_FMT_RGGB8 VIDEO_FOURCC('R', 'G', 'G', 'B') /** * @} @@ -841,14 +870,40 @@ void video_closest_frmival(const struct device *dev, enum video_endpoint_id ep, /** * @name RGB formats + * Per-color (R, G, B) channels. * @{ */ -/** RGB565 pixel format */ -#define VIDEO_PIX_FMT_RGB565 video_fourcc('R', 'G', 'B', 'P') /* 16 RGB-5-6-5 */ +/** + * 5 red bits [15:11], 6 green bits [10:5], 5 blue bits [4:0]. + * This 16-bit integer is then packed in big endian format over two bytes: + * + * @verbatim + * 15.....8 7......0 + * | RrrrrGgg gggBbbbb | ... + * @endverbatim + */ +#define VIDEO_PIX_FMT_RGB565X VIDEO_FOURCC('R', 'G', 'B', 'R') + +/** + * 5 red bits [15:11], 6 green bits [10:5], 5 blue bits [4:0]. + * This 16-bit integer is then packed in little endian format over two bytes: + * + * @verbatim + * 7......0 15.....8 + * | gggBbbbb RrrrrGgg | ... + * @endverbatim + */ +#define VIDEO_PIX_FMT_RGB565 VIDEO_FOURCC('R', 'G', 'B', 'P') -/** XRGB32 pixel format */ -#define VIDEO_PIX_FMT_XRGB32 video_fourcc('B', 'X', '2', '4') /* 32 XRGB-8-8-8-8 */ +/** + * The first byte is empty (X) for each pixel. + * + * @verbatim + * | Xxxxxxxx Rrrrrrrr Gggggggg Bbbbbbbb | ... + * @endverbatim + */ +#define VIDEO_PIX_FMT_XRGB32 VIDEO_FOURCC('B', 'X', '2', '4') /** * @} @@ -856,27 +911,42 @@ void video_closest_frmival(const struct device *dev, enum video_endpoint_id ep, /** * @name YUV formats + * Luminance (Y) and chrominance (U, V) channels. * @{ */ -/** YUYV pixel format */ -#define VIDEO_PIX_FMT_YUYV video_fourcc('Y', 'U', 'Y', 'V') /* 16 Y0-Cb0 Y1-Cr0 */ - -/** XYUV32 pixel format */ -#define VIDEO_PIX_FMT_XYUV32 video_fourcc('X', 'Y', 'U', 'V') /* 32 XYUV-8-8-8-8 */ +/** + * There is either a missing channel per pixel, U or V. + * The value is to be averaged over 2 pixels to get the value of individual pixel. + * + * @verbatim + * | Yyyyyyyy Uuuuuuuu | Yyyyyyyy Vvvvvvvv | ... + * @endverbatim + */ +#define VIDEO_PIX_FMT_YUYV VIDEO_FOURCC('Y', 'U', 'Y', 'V') /** + * The first byte is empty (X) for each pixel. * + * @verbatim + * | Xxxxxxxx Yyyyyyyy Uuuuuuuu Vvvvvvvv | ... + * @endverbatim + */ +#define VIDEO_PIX_FMT_XYUV32 VIDEO_FOURCC('X', 'Y', 'U', 'V') + +/** * @} */ /** - * @name JPEG formats + * @name Compressed formats * @{ */ -/** JPEG pixel format */ -#define VIDEO_PIX_FMT_JPEG video_fourcc('J', 'P', 'E', 'G') /* 8 JPEG */ +/** + * Both JPEG (single frame) and Motion-JPEG (MJPEG, multiple JPEG frames concatenated) + */ +#define VIDEO_PIX_FMT_JPEG VIDEO_FOURCC('J', 'P', 'E', 'G') /** * @} From 5729c7bd6eff6dee984553bd52858b572aa5a9d2 Mon Sep 17 00:00:00 2001 From: Josuah Demangeon Date: Sun, 6 Oct 2024 18:19:44 +0000 Subject: [PATCH 2/4] drivers: video: Make VIDEO_FOURCC() a documented API Rename video_fourcc() to VIDEO_FOURCC(), differing from the Linux implementation, but more compliant with the coding style. Also introduce a VIDEO_FOURCC_FROM_STR() to generate a FOURCC format code out of a 4-characters string. Signed-off-by: Josuah Demangeon --- include/zephyr/drivers/video.h | 17 +++++++++++++++-- samples/drivers/video/capture/src/main.c | 4 +--- 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/include/zephyr/drivers/video.h b/include/zephyr/drivers/video.h index a3aa05bc051ec3..6eac2336bf4b61 100644 --- a/include/zephyr/drivers/video.h +++ b/include/zephyr/drivers/video.h @@ -812,10 +812,23 @@ void video_closest_frmival_stepwise(const struct video_frmival_stepwise *stepwis void video_closest_frmival(const struct device *dev, enum video_endpoint_id ep, struct video_frmival_enum *match); -/* fourcc - four-character-code */ -#define video_fourcc(a, b, c, d) \ +/** + * @brief Four-character-code uniquely identifying the pixel format + */ +#define VIDEO_FOURCC(a, b, c, d) \ ((uint32_t)(a) | ((uint32_t)(b) << 8) | ((uint32_t)(c) << 16) | ((uint32_t)(d) << 24)) +/** + * @brief Convert a four-character-string to a four-character-code + * + * Convert a string literal or variable into a four-character-code + * as defined by @ref VIDEO_FOURCC. + * + * @param str String to be converted + * @return Four-character-code. + */ +#define VIDEO_FOURCC_FROM_STR(str) VIDEO_FOURCC((str)[0], (str)[1], (str)[2], (str)[3]) + /** * @defgroup video_pixel_formats Video pixel formats * The @c | characters separate the pixels, and spaces separate the bytes. diff --git a/samples/drivers/video/capture/src/main.c b/samples/drivers/video/capture/src/main.c index 10af7ef7d9ea6a..59f200b94d8d0c 100644 --- a/samples/drivers/video/capture/src/main.c +++ b/samples/drivers/video/capture/src/main.c @@ -144,9 +144,7 @@ int main(void) #endif if (strcmp(CONFIG_VIDEO_PIXEL_FORMAT, "")) { - fmt.pixelformat = - video_fourcc(CONFIG_VIDEO_PIXEL_FORMAT[0], CONFIG_VIDEO_PIXEL_FORMAT[1], - CONFIG_VIDEO_PIXEL_FORMAT[2], CONFIG_VIDEO_PIXEL_FORMAT[3]); + fmt.pixelformat = VIDEO_FOURCC_FROM_STR(CONFIG_VIDEO_PIXEL_FORMAT); } LOG_INF("- Video format: %c%c%c%c %ux%u", (char)fmt.pixelformat, From 93687b2bbe0cf0e9a5117f0c9b8c6626ee47d6fb Mon Sep 17 00:00:00 2001 From: Josuah Demangeon Date: Mon, 21 Oct 2024 10:34:26 +0000 Subject: [PATCH 3/4] drivers: video: introduce video_bits_per_pixel(pixelformat) helper This was present in the form of video_pix_fmt_bpp() inside ST and NXP drivers, and was returning the number of bytes, which does not allow support for 10-bits, 4-bits or non-byte aligned video formats. The helper leverages the VIDEO_PIX_FMT_*_BITS macros. Signed-off-by: Josuah Demangeon --- drivers/video/video_mcux_csi.c | 4 +-- drivers/video/video_mcux_mipi_csi2rx.c | 4 +-- drivers/video/video_stm32_dcmi.c | 2 +- include/zephyr/drivers/video.h | 40 ++++++++++++++------------ 4 files changed, 27 insertions(+), 23 deletions(-) diff --git a/drivers/video/video_mcux_csi.c b/drivers/video/video_mcux_csi.c index 101549969d1524..4fe1dcd7fa145c 100644 --- a/drivers/video/video_mcux_csi.c +++ b/drivers/video/video_mcux_csi.c @@ -123,7 +123,7 @@ static inline void video_pix_fmt_convert(struct video_format *fmt, bool isGetFmt break; } - fmt->pitch = fmt->width * video_pix_fmt_bpp(fmt->pixelformat); + fmt->pitch = fmt->width * video_bits_per_pixel(fmt->pixelformat) / 8; } #endif @@ -132,7 +132,7 @@ static int video_mcux_csi_set_fmt(const struct device *dev, enum video_endpoint_ { const struct video_mcux_csi_config *config = dev->config; struct video_mcux_csi_data *data = dev->data; - unsigned int bpp = video_pix_fmt_bpp(fmt->pixelformat); + unsigned int bpp = video_bits_per_pixel(fmt->pixelformat) / 8; status_t ret; struct video_format format = *fmt; diff --git a/drivers/video/video_mcux_mipi_csi2rx.c b/drivers/video/video_mcux_mipi_csi2rx.c index efb96637c52fad..fe0909da7f2618 100644 --- a/drivers/video/video_mcux_mipi_csi2rx.c +++ b/drivers/video/video_mcux_mipi_csi2rx.c @@ -73,7 +73,7 @@ static int mipi_csi2rx_update_settings(const struct device *dev, enum video_endp return -ENOTSUP; } - bpp = video_pix_fmt_bpp(fmt.pixelformat) * 8; + bpp = video_bits_per_pixel(fmt.pixelformat); sensor_byte_clk = sensor_pixel_rate * bpp / drv_data->csi2rxConfig.laneNum / 8; ret = clock_control_get_rate(drv_data->clock_dev, drv_data->clock_root, &root_clk_rate); @@ -224,7 +224,7 @@ static int mipi_csi2rx_get_frmival(const struct device *dev, enum video_endpoint static uint64_t mipi_csi2rx_cal_frame_size(const struct video_format *fmt) { - return fmt->height * fmt->width * video_pix_fmt_bpp(fmt->pixelformat) * 8; + return fmt->height * fmt->width * video_bits_per_pixel(fmt->pixelformat); } static uint64_t mipi_csi2rx_estimate_pixel_rate(const struct video_frmival *cur_fmival, diff --git a/drivers/video/video_stm32_dcmi.c b/drivers/video/video_stm32_dcmi.c index 599d1a5d5a1bdc..13941555220401 100644 --- a/drivers/video/video_stm32_dcmi.c +++ b/drivers/video/video_stm32_dcmi.c @@ -198,7 +198,7 @@ static int video_stm32_dcmi_set_fmt(const struct device *dev, { const struct video_stm32_dcmi_config *config = dev->config; struct video_stm32_dcmi_data *data = dev->data; - unsigned int bpp = video_pix_fmt_bpp(fmt->pixelformat); + unsigned int bpp = video_bits_per_pixel(fmt->pixelformat) / 8; if (bpp == 0 || (ep != VIDEO_EP_OUT && ep != VIDEO_EP_ALL)) { return -EINVAL; diff --git a/include/zephyr/drivers/video.h b/include/zephyr/drivers/video.h index 6eac2336bf4b61..0351755e39fbfa 100644 --- a/include/zephyr/drivers/video.h +++ b/include/zephyr/drivers/video.h @@ -812,6 +812,14 @@ void video_closest_frmival_stepwise(const struct video_frmival_stepwise *stepwis void video_closest_frmival(const struct device *dev, enum video_endpoint_id ep, struct video_frmival_enum *match); +/** + * @defgroup video_pixel_formats Video pixel formats + * The @c | characters separate the pixels, and spaces separate the bytes. + * The uppercase letter represents the most significant bit. + * The lowercase letters represent the rest of the bits. + * @{ + */ + /** * @brief Four-character-code uniquely identifying the pixel format */ @@ -829,14 +837,6 @@ void video_closest_frmival(const struct device *dev, enum video_endpoint_id ep, */ #define VIDEO_FOURCC_FROM_STR(str) VIDEO_FOURCC((str)[0], (str)[1], (str)[2], (str)[3]) -/** - * @defgroup video_pixel_formats Video pixel formats - * The @c | characters separate the pixels, and spaces separate the bytes. - * The uppercase letter represents the most significant bit. - * The lowercase letters represent the rest of the bits. - * @{ - */ - /** * @name Bayer formats (R, G, B channels). * @@ -966,33 +966,37 @@ void video_closest_frmival(const struct device *dev, enum video_endpoint_id ep, */ /** - * @} - */ - -/** - * @brief Get number of bytes per pixel of a pixel format + * @brief Get number of bits per pixel of a pixel format + * + * @param pixfmt FourCC pixel format value (@ref video_pixel_formats). * - * @param pixfmt FourCC pixel format value (\ref video_pixel_formats). + * @retval 0 if the format is unhandled or if it is variable number of bits + * @retval bit size of one pixel for this format */ -static inline unsigned int video_pix_fmt_bpp(uint32_t pixfmt) +static inline unsigned int video_bits_per_pixel(uint32_t pixfmt) { switch (pixfmt) { case VIDEO_PIX_FMT_BGGR8: case VIDEO_PIX_FMT_GBRG8: case VIDEO_PIX_FMT_GRBG8: case VIDEO_PIX_FMT_RGGB8: - return 1; + return 8; case VIDEO_PIX_FMT_RGB565: case VIDEO_PIX_FMT_YUYV: - return 2; + return 16; case VIDEO_PIX_FMT_XRGB32: case VIDEO_PIX_FMT_XYUV32: - return 4; + return 32; default: + /* Variable number of bits per pixel or unknown format */ return 0; } } +/** + * @} + */ + #ifdef __cplusplus } #endif From e8ad2e75314f57ac9bc23ac9fccee50f9038d5f8 Mon Sep 17 00:00:00 2001 From: Josuah Demangeon Date: Fri, 10 Jan 2025 22:43:05 +0000 Subject: [PATCH 4/4] doc: releases: 4.1: announce switch to video_bits_per_pixel() The previous commit introduces video_bits_per_pixel() and converts all invocation relying on video_pix_fmt_bpp(), 3rd-party users will need to do the same on their code, and this is not a 100% drop-in API, so better document it on the release notes. Signed-off-by: Josuah Demangeon --- doc/releases/migration-guide-4.1.rst | 5 +++++ doc/releases/release-notes-4.1.rst | 3 +++ 2 files changed, 8 insertions(+) diff --git a/doc/releases/migration-guide-4.1.rst b/doc/releases/migration-guide-4.1.rst index b2860fbc9cf543..8447f216da007c 100644 --- a/doc/releases/migration-guide-4.1.rst +++ b/doc/releases/migration-guide-4.1.rst @@ -228,6 +228,11 @@ Video The new ``video-controls.h`` source now contains description of each control ID to help disambiguating. +* The ``video_pix_fmt_bpp()`` function was returning a byte count, this got replaced by + ``video_bits_per_pixel()`` which return a bit count. For instance, invocations such as + ``pitch = width * video_pix_fmt_bpp(pixfmt)`` needs to be replaced by an equivalent + ``pitch = width * video_bits_per_pixel(pixfmt) / 8``. + Watchdog ======== diff --git a/doc/releases/release-notes-4.1.rst b/doc/releases/release-notes-4.1.rst index b47361a380331b..803ab2bcc98d0d 100644 --- a/doc/releases/release-notes-4.1.rst +++ b/doc/releases/release-notes-4.1.rst @@ -240,6 +240,9 @@ Drivers and Sensors * Changed :file:`include/zephyr/drivers/video-controls.h` to have control IDs (CIDs) matching those present in the Linux kernel. + * Changed ``video_pix_fmt_bpp()`` returning the byte count and only supports 8-bit depth, + into ``video_bits_per_pixel()`` returning the bit count and supports any color depth. + * Watchdog * Wi-Fi