From 7557dd56631da3675e57d582644728ba1a295a01 Mon Sep 17 00:00:00 2001 From: Josuah Demangeon Date: Tue, 18 Feb 2025 22:26:12 +0000 Subject: [PATCH 01/12] drivers: video: sw_generator: preserve full prefix for internal functions In order to help debugging through GDB and other error messages and debug tools, convert the __xxx prefix to video_sw_generator_xxx full prefix. To help keep function names short, use slightly shorter sufixes. Signed-off-by: Josuah Demangeon --- drivers/video/video_sw_generator.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/drivers/video/video_sw_generator.c b/drivers/video/video_sw_generator.c index 40ba10011bea..42b5c3982f5c 100644 --- a/drivers/video/video_sw_generator.c +++ b/drivers/video/video_sw_generator.c @@ -115,7 +115,8 @@ uint16_t rgb565_colorbar_value[] = {0x0000, 0x001F, 0xF800, 0xF81F, 0x07E0, 0x07 uint32_t xrgb32_colorbar_value[] = {0xFF000000, 0xFF0000FF, 0xFFFF0000, 0xFFFF00FF, 0xFF00FF00, 0xFF00FFFF, 0xFFFFFF00, 0xFFFFFFFF}; -static void __fill_buffer_colorbar(struct video_sw_generator_data *data, struct video_buffer *vbuf) +static void video_sw_generator_fill_colorbar(struct video_sw_generator_data *data, + struct video_buffer *vbuf) { int bw = data->fmt.width / 8; int h, w, i = 0; @@ -140,7 +141,7 @@ static void __fill_buffer_colorbar(struct video_sw_generator_data *data, struct vbuf->line_offset = 0; } -static void __buffer_work(struct k_work *work) +static void video_sw_generator_worker(struct k_work *work) { struct k_work_delayable *dwork = k_work_delayable_from_work(work); struct video_sw_generator_data *data; @@ -157,7 +158,7 @@ static void __buffer_work(struct k_work *work) switch (data->pattern) { case VIDEO_PATTERN_COLOR_BAR: - __fill_buffer_colorbar(data, vbuf); + video_sw_generator_fill_colorbar(data, vbuf); break; } @@ -333,7 +334,7 @@ static int video_sw_generator_init(const struct device *dev) data->dev = dev; k_fifo_init(&data->fifo_in); k_fifo_init(&data->fifo_out); - k_work_init_delayable(&data->buf_work, __buffer_work); + k_work_init_delayable(&data->buf_work, video_sw_generator_worker); return video_sw_generator_init_controls(dev); } From 7eea58d7d719fdee1a8dc53f0bf7eb82bdda9a9d Mon Sep 17 00:00:00 2001 From: Josuah Demangeon Date: Tue, 18 Feb 2025 22:28:56 +0000 Subject: [PATCH 02/12] drivers: video: sw_generator: refactor: flatten arrays Help with maintainance and possibly readability by using a more regular layout for various tables of numbers. This adds a comma on the last element to help with formatters like clang-format. Signed-off-by: Josuah Demangeon --- drivers/video/video_sw_generator.c | 49 +++++++++++++++++------------- 1 file changed, 28 insertions(+), 21 deletions(-) diff --git a/drivers/video/video_sw_generator.c b/drivers/video/video_sw_generator.c index 42b5c3982f5c..f9de0f54fb66 100644 --- a/drivers/video/video_sw_generator.c +++ b/drivers/video/video_sw_generator.c @@ -44,24 +44,27 @@ struct video_sw_generator_data { uint32_t frame_rate; }; -static const struct video_format_cap fmts[] = {{ - .pixelformat = VIDEO_PIX_FMT_RGB565, - .width_min = 64, - .width_max = 1920, - .height_min = 64, - .height_max = 1080, - .width_step = 1, - .height_step = 1, - }, { - .pixelformat = VIDEO_PIX_FMT_XRGB32, - .width_min = 64, - .width_max = 1920, - .height_min = 64, - .height_max = 1080, - .width_step = 1, - .height_step = 1, - }, - {0}}; +static const struct video_format_cap fmts[] = { + { + .pixelformat = VIDEO_PIX_FMT_RGB565, + .width_min = 64, + .width_max = 1920, + .height_min = 64, + .height_max = 1080, + .width_step = 1, + .height_step = 1, + }, + { + .pixelformat = VIDEO_PIX_FMT_XRGB32, + .width_min = 64, + .width_max = 1920, + .height_min = 64, + .height_max = 1080, + .width_step = 1, + .height_step = 1, + }, + {0}, +}; static int video_sw_generator_set_fmt(const struct device *dev, struct video_format *fmt) { @@ -110,10 +113,14 @@ static int video_sw_generator_set_stream(const struct device *dev, bool enable, } /* Black, Blue, Red, Purple, Green, Aqua, Yellow, White */ -uint16_t rgb565_colorbar_value[] = {0x0000, 0x001F, 0xF800, 0xF81F, 0x07E0, 0x07FF, 0xFFE0, 0xFFFF}; +uint16_t rgb565_colorbar_value[] = { + 0x0000, 0x001F, 0xF800, 0xF81F, 0x07E0, 0x07FF, 0xFFE0, 0xFFFF, +}; -uint32_t xrgb32_colorbar_value[] = {0xFF000000, 0xFF0000FF, 0xFFFF0000, 0xFFFF00FF, - 0xFF00FF00, 0xFF00FFFF, 0xFFFFFF00, 0xFFFFFFFF}; +uint32_t xrgb32_colorbar_value[] = { + 0xFF000000, 0xFF0000FF, 0xFFFF0000, 0xFFFF00FF, + 0xFF00FF00, 0xFF00FFFF, 0xFFFFFF00, 0xFFFFFFFF, +}; static void video_sw_generator_fill_colorbar(struct video_sw_generator_data *data, struct video_buffer *vbuf) From 606ecd70c5f24d03d618f6f5941971f1a17573b7 Mon Sep 17 00:00:00 2001 From: Josuah Demangeon Date: Fri, 18 Apr 2025 18:35:14 +0000 Subject: [PATCH 03/12] drivers: video: sw_generator: use video_format_caps_index() Use the video_format_caps_index() function to check if a format matches any entry of the format caps. Signed-off-by: Josuah Demangeon --- drivers/video/video_sw_generator.c | 34 +++++++++++------------------- 1 file changed, 12 insertions(+), 22 deletions(-) diff --git a/drivers/video/video_sw_generator.c b/drivers/video/video_sw_generator.c index f9de0f54fb66..4931a69906aa 100644 --- a/drivers/video/video_sw_generator.c +++ b/drivers/video/video_sw_generator.c @@ -10,6 +10,7 @@ #include #include #include +#include #include "video_ctrls.h" #include "video_device.h" @@ -69,23 +70,16 @@ static const struct video_format_cap fmts[] = { static int video_sw_generator_set_fmt(const struct device *dev, struct video_format *fmt) { struct video_sw_generator_data *data = dev->data; - int i = 0; + size_t idx; + int ret; - for (i = 0; i < ARRAY_SIZE(fmts); ++i) { - if (fmt->pixelformat == fmts[i].pixelformat && fmt->width >= fmts[i].width_min && - fmt->width <= fmts[i].width_max && fmt->height >= fmts[i].height_min && - fmt->height <= fmts[i].height_max) { - break; - } - } - - if (i == ARRAY_SIZE(fmts)) { + ret = video_format_caps_index(fmts, fmt, &idx); + if (ret < 0) { LOG_ERR("Unsupported pixel format or resolution"); - return -ENOTSUP; + return ret; } data->fmt = *fmt; - return 0; } @@ -277,17 +271,13 @@ static int video_sw_generator_get_frmival(const struct device *dev, struct video static int video_sw_generator_enum_frmival(const struct device *dev, struct video_frmival_enum *fie) { - int i = 0; + size_t idx; + int ret; - while (fmts[i].pixelformat && (fmts[i].pixelformat != fie->format->pixelformat)) { - i++; - } - - if ((i == ARRAY_SIZE(fmts)) || (fie->format->width > fmts[i].width_max) || - (fie->format->width < fmts[i].width_min) || - (fie->format->height > fmts[i].height_max) || - (fie->format->height < fmts[i].height_min)) { - return -EINVAL; + ret = video_format_caps_index(fmts, fie->format, &idx); + if (ret < 0) { + LOG_ERR("Unsupported pixel format or resolution"); + return ret; } fie->type = VIDEO_FRMIVAL_TYPE_STEPWISE; From f80515e0861099956ba224f44f32a8d59447fc1f Mon Sep 17 00:00:00 2001 From: Josuah Demangeon Date: Tue, 18 Feb 2025 22:39:38 +0000 Subject: [PATCH 04/12] drivers: video: sw_generator: fix video_sw_generator_enum_frmival() Return an error on fie.index >= 1 as there is only one framerate entry per pixelformat, this prevents an endless loop. Signed-off-by: Josuah Demangeon --- drivers/video/video_sw_generator.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/drivers/video/video_sw_generator.c b/drivers/video/video_sw_generator.c index 4931a69906aa..f7a28a89f407 100644 --- a/drivers/video/video_sw_generator.c +++ b/drivers/video/video_sw_generator.c @@ -274,6 +274,10 @@ static int video_sw_generator_enum_frmival(const struct device *dev, struct vide size_t idx; int ret; + if (fie->index >= 1) { + return -ERANGE; + } + ret = video_format_caps_index(fmts, fie->format, &idx); if (ret < 0) { LOG_ERR("Unsupported pixel format or resolution"); From a481c8c93beab3dac0cd8846bd3c676619b40b21 Mon Sep 17 00:00:00 2001 From: Josuah Demangeon Date: Tue, 18 Feb 2025 22:45:59 +0000 Subject: [PATCH 05/12] drivers: video: sw_generator: modify video_sw_generator_fill_colorbar() Add a check for the array size to avoid overwriting unrelated memory when the buffer is too small for the full format. It first check if there is enough buffer for one line, and fill it programmatically. Then, it will try to duplicate that line over the entire buffer, in the limit of the room available. Signed-off-by: Josuah Demangeon --- drivers/video/video_sw_generator.c | 52 +++++++++++++++++++++--------- 1 file changed, 37 insertions(+), 15 deletions(-) diff --git a/drivers/video/video_sw_generator.c b/drivers/video/video_sw_generator.c index f7a28a89f407..f483ece13c8e 100644 --- a/drivers/video/video_sw_generator.c +++ b/drivers/video/video_sw_generator.c @@ -11,6 +11,7 @@ #include #include #include +#include #include "video_ctrls.h" #include "video_device.h" @@ -116,29 +117,50 @@ uint32_t xrgb32_colorbar_value[] = { 0xFF00FF00, 0xFF00FFFF, 0xFFFFFF00, 0xFFFFFFFF, }; +static inline int video_sw_generator_get_color_idx(uint16_t w, uint16_t width, bool hflip) +{ + /* If hflip is on, start from the right instead */ + w = (hflip) ? (width - w - 1) : (w); + + /* Downscale from w/width to #/8 */ + return 8 * w / width; +} + static void video_sw_generator_fill_colorbar(struct video_sw_generator_data *data, struct video_buffer *vbuf) { int bw = data->fmt.width / 8; - int h, w, i = 0; - - for (h = 0; h < data->fmt.height; h++) { - for (w = 0; w < data->fmt.width; w++) { - int color_idx = data->ctrls.hflip.val ? 7 - w / bw : w / bw; - if (data->fmt.pixelformat == VIDEO_PIX_FMT_RGB565) { - uint16_t *pixel = (uint16_t *)&vbuf->buffer[i]; - *pixel = rgb565_colorbar_value[color_idx]; - i += 2; - } else if (data->fmt.pixelformat == VIDEO_PIX_FMT_XRGB32) { - uint32_t *pixel = (uint32_t *)&vbuf->buffer[i]; - *pixel = xrgb32_colorbar_value[color_idx]; - i += 4; - } + struct video_format *fmt = &data->fmt; + size_t pitch = fmt->width * video_bits_per_pixel(fmt->pixelformat) / BITS_PER_BYTE; + bool hflip = data->ctrls.hflip.val; + + vbuf->bytesused = 0; + + /* Generate the first line of data */ + for (int w = 0, i = 0; w < data->fmt.width; w++) { + int color_idx = video_sw_generator_get_color_idx(w, width, hflip); + + if (data->fmt.pixelformat == VIDEO_PIX_FMT_RGB565) { + uint16_t *pixel = (uint16_t *)&vbuf->buffer[i]; + *pixel = sys_cpu_to_le16(rgb565_colorbar_value[color_idx]); + } else if (data->fmt.pixelformat == VIDEO_PIX_FMT_XRGB32) { + uint32_t *pixel = (uint32_t *)&vbuf->buffer[i]; + *pixel = sys_cpu_to_le32(xrgb32_colorbar_value[color_idx]); } + i += video_bits_per_pixel(data->fmt.pixelformat) / BITS_PER_BYTE; + } + + /* Duplicate the first line all over the buffer */ + for (int h = 1; h < data->fmt.height; h++) { + if (vbuf->size < vbuf->bytesused + pitch) { + break; + } + + memcpy(vbuf->buffer + h * pitch, vbuf->buffer, pitch); + vbuf->bytesused += pitch; } vbuf->timestamp = k_uptime_get_32(); - vbuf->bytesused = i; vbuf->line_offset = 0; } From b5ee6ee289fdaf7636947542da6ea7852dd258df Mon Sep 17 00:00:00 2001 From: Josuah Demangeon Date: Wed, 19 Feb 2025 23:11:36 +0000 Subject: [PATCH 06/12] drivers: video: sw_generator: add support for bayer and YUV formats This refactors the pattern generator functions to also offer bayer formats as input. 4 variant of bayer formats are proposed. The pixel packing is also now split from the color selection: only a single RGB and single YUV array used by all the pattern generators. Signed-off-by: Josuah Demangeon --- drivers/video/video_sw_generator.c | 195 ++++++++++++++++++++++------- 1 file changed, 149 insertions(+), 46 deletions(-) diff --git a/drivers/video/video_sw_generator.c b/drivers/video/video_sw_generator.c index f483ece13c8e..4167220ccff5 100644 --- a/drivers/video/video_sw_generator.c +++ b/drivers/video/video_sw_generator.c @@ -46,25 +46,25 @@ struct video_sw_generator_data { uint32_t frame_rate; }; +#define VIDEO_SW_GENERATOR_FORMAT_CAP(pixfmt) \ + { \ + .pixelformat = pixfmt, \ + .width_min = 64, \ + .width_max = 1920, \ + .height_min = 64, \ + .height_max = 1080, \ + .width_step = 1, \ + .height_step = 1, \ + } + static const struct video_format_cap fmts[] = { - { - .pixelformat = VIDEO_PIX_FMT_RGB565, - .width_min = 64, - .width_max = 1920, - .height_min = 64, - .height_max = 1080, - .width_step = 1, - .height_step = 1, - }, - { - .pixelformat = VIDEO_PIX_FMT_XRGB32, - .width_min = 64, - .width_max = 1920, - .height_min = 64, - .height_max = 1080, - .width_step = 1, - .height_step = 1, - }, + VIDEO_SW_GENERATOR_FORMAT_CAP(VIDEO_PIX_FMT_YUYV), + VIDEO_SW_GENERATOR_FORMAT_CAP(VIDEO_PIX_FMT_RGB565), + VIDEO_SW_GENERATOR_FORMAT_CAP(VIDEO_PIX_FMT_XRGB32), + VIDEO_SW_GENERATOR_FORMAT_CAP(VIDEO_PIX_FMT_SRGGB8), + VIDEO_SW_GENERATOR_FORMAT_CAP(VIDEO_PIX_FMT_SGRBG8), + VIDEO_SW_GENERATOR_FORMAT_CAP(VIDEO_PIX_FMT_SBGGR8), + VIDEO_SW_GENERATOR_FORMAT_CAP(VIDEO_PIX_FMT_SGBRG8), {0}, }; @@ -107,14 +107,21 @@ static int video_sw_generator_set_stream(const struct device *dev, bool enable, return 0; } -/* Black, Blue, Red, Purple, Green, Aqua, Yellow, White */ -uint16_t rgb565_colorbar_value[] = { - 0x0000, 0x001F, 0xF800, 0xF81F, 0x07E0, 0x07FF, 0xFFE0, 0xFFFF, +static const uint8_t pattern_rggb_idx[] = {0, 1, 1, 2}; +static const uint8_t pattern_bggr_idx[] = {2, 1, 1, 0}; +static const uint8_t pattern_gbrg_idx[] = {1, 2, 0, 1}; +static const uint8_t pattern_grbg_idx[] = {1, 0, 2, 1}; + +/* White, Yellow, Cyan, Green, Magenta, Red, Blue, Black */ + +static const uint16_t pattern_8bars_yuv_bt709[8][3] = { + {0xFE, 0x80, 0x7F}, {0xEC, 0x00, 0x8B}, {0xC8, 0x9D, 0x00}, {0xB6, 0x1D, 0x0C}, + {0x48, 0xE2, 0xF3}, {0x36, 0x62, 0xFF}, {0x12, 0xFF, 0x74}, {0x00, 0x80, 0x80}, }; -uint32_t xrgb32_colorbar_value[] = { - 0xFF000000, 0xFF0000FF, 0xFFFF0000, 0xFFFF00FF, - 0xFF00FF00, 0xFF00FFFF, 0xFFFFFF00, 0xFFFFFFFF, +static const uint16_t pattern_8bars_rgb[8][3] = { + {0xFF, 0xFF, 0xFF}, {0xFF, 0xFF, 0x00}, {0x00, 0xFF, 0xFF}, {0x00, 0xFF, 0x00}, + {0xFF, 0x00, 0xFF}, {0xFF, 0x00, 0x00}, {0x00, 0x00, 0xFF}, {0x00, 0x00, 0x00}, }; static inline int video_sw_generator_get_color_idx(uint16_t w, uint16_t width, bool hflip) @@ -126,42 +133,138 @@ static inline int video_sw_generator_get_color_idx(uint16_t w, uint16_t width, b return 8 * w / width; } -static void video_sw_generator_fill_colorbar(struct video_sw_generator_data *data, - struct video_buffer *vbuf) +static uint16_t video_sw_generator_fill_yuyv(uint8_t *buffer, uint16_t width, bool hflip) +{ + if (width % 2 != 0) { + LOG_ERR("YUYV pixels always go by pairs"); + return 0; + } + + for (size_t w = 0; w < width; w += 2) { + int color_idx = video_sw_generator_get_color_idx(w, width, hflip); + + buffer[w * 2 + 0] = pattern_8bars_yuv_bt709[color_idx][0]; + buffer[w * 2 + 1] = pattern_8bars_yuv_bt709[color_idx][1]; + buffer[w * 2 + 2] = pattern_8bars_yuv_bt709[color_idx][0]; + buffer[w * 2 + 3] = pattern_8bars_yuv_bt709[color_idx][2]; + } + return 1; +} + +static uint16_t video_sw_generator_fill_xrgb32(uint8_t *buffer, uint16_t width, bool hflip) +{ + for (size_t w = 0; w < width; w++) { + int color_idx = video_sw_generator_get_color_idx(w, width, hflip); + + buffer[w * 4 + 0] = 0xff; + buffer[w * 4 + 1] = pattern_8bars_rgb[color_idx][0]; + buffer[w * 4 + 2] = pattern_8bars_rgb[color_idx][1]; + buffer[w * 4 + 3] = pattern_8bars_rgb[color_idx][2]; + } + return 1; +} + +static uint16_t video_sw_generator_fill_rgb565(uint8_t *buffer, uint16_t width, bool hflip) +{ + for (size_t w = 0; w < width; w++) { + int color_idx = video_sw_generator_get_color_idx(w, width, hflip); + uint8_t r = pattern_8bars_rgb[color_idx][0] >> (8 - 5); + uint8_t g = pattern_8bars_rgb[color_idx][1] >> (8 - 6); + uint8_t b = pattern_8bars_rgb[color_idx][2] >> (8 - 5); + + ((uint16_t *)buffer)[w] = sys_cpu_to_le16((r << 11) | (g << 6) | (b << 0)); + } + return 1; +} + +static uint16_t video_sw_generator_fill_bayer8(uint8_t *buffer, uint16_t width, bool hflip, + const uint8_t *bayer_idx) +{ + uint8_t *row0 = buffer + 0; + uint8_t *row1 = buffer + width; + + if (width % 2 != 0) { + LOG_ERR("Bayer pixels always go by pairs (vertically and horizontally)"); + return 0; + } + + for (size_t w = 0; w < width; w += 2) { + int color_idx = video_sw_generator_get_color_idx(w, width, hflip); + + row0[w + 0] = pattern_8bars_rgb[color_idx][bayer_idx[0]]; + row0[w + 1] = pattern_8bars_rgb[color_idx][bayer_idx[1]]; + row1[w + 0] = pattern_8bars_rgb[color_idx][bayer_idx[2]]; + row1[w + 1] = pattern_8bars_rgb[color_idx][bayer_idx[3]]; + } + return 2; +} + +static int video_sw_generator_fill(const struct device *const dev, struct video_buffer *vbuf) { - int bw = data->fmt.width / 8; + struct video_sw_generator_data *data = dev->data; struct video_format *fmt = &data->fmt; size_t pitch = fmt->width * video_bits_per_pixel(fmt->pixelformat) / BITS_PER_BYTE; bool hflip = data->ctrls.hflip.val; + uint16_t lines = 0; - vbuf->bytesused = 0; + if (vbuf->size < pitch * 2) { + LOG_ERR("At least 2 lines needed for bayer formats support"); + return -EINVAL; + } - /* Generate the first line of data */ - for (int w = 0, i = 0; w < data->fmt.width; w++) { - int color_idx = video_sw_generator_get_color_idx(w, width, hflip); + /* Fill the first row of the emulated framebuffer */ + switch (data->fmt.pixelformat) { + case VIDEO_PIX_FMT_YUYV: + lines = video_sw_generator_fill_yuyv(vbuf->buffer, fmt->width, hflip); + break; + case VIDEO_PIX_FMT_XRGB32: + lines = video_sw_generator_fill_xrgb32(vbuf->buffer, fmt->width, hflip); + break; + case VIDEO_PIX_FMT_RGB565: + lines = video_sw_generator_fill_rgb565(vbuf->buffer, fmt->width, hflip); + break; + case VIDEO_PIX_FMT_SRGGB8: + lines = video_sw_generator_fill_bayer8(vbuf->buffer, fmt->width, hflip, + pattern_rggb_idx); + break; + case VIDEO_PIX_FMT_SGBRG8: + lines = video_sw_generator_fill_bayer8(vbuf->buffer, fmt->width, hflip, + pattern_gbrg_idx); + break; + case VIDEO_PIX_FMT_SBGGR8: + lines = video_sw_generator_fill_bayer8(vbuf->buffer, fmt->width, hflip, + pattern_bggr_idx); + break; + case VIDEO_PIX_FMT_SGRBG8: + lines = video_sw_generator_fill_bayer8(vbuf->buffer, fmt->width, hflip, + pattern_grbg_idx); + break; + default: + CODE_UNREACHABLE; + break; + } - if (data->fmt.pixelformat == VIDEO_PIX_FMT_RGB565) { - uint16_t *pixel = (uint16_t *)&vbuf->buffer[i]; - *pixel = sys_cpu_to_le16(rgb565_colorbar_value[color_idx]); - } else if (data->fmt.pixelformat == VIDEO_PIX_FMT_XRGB32) { - uint32_t *pixel = (uint32_t *)&vbuf->buffer[i]; - *pixel = sys_cpu_to_le32(xrgb32_colorbar_value[color_idx]); - } - i += video_bits_per_pixel(data->fmt.pixelformat) / BITS_PER_BYTE; + if (lines == 0) { + return -EINVAL; } - /* Duplicate the first line all over the buffer */ - for (int h = 1; h < data->fmt.height; h++) { - if (vbuf->size < vbuf->bytesused + pitch) { + /* How much was filled in so far */ + vbuf->bytesused = data->fmt.pitch * lines; + + /* Duplicate the first line(s) all over the buffer */ + for (int h = lines; h < data->fmt.height; h += lines) { + if (vbuf->size < vbuf->bytesused + pitch * lines) { + LOG_WRN("Generation stopped early: buffer too small"); break; } - - memcpy(vbuf->buffer + h * pitch, vbuf->buffer, pitch); - vbuf->bytesused += pitch; + memcpy(vbuf->buffer + h * pitch, vbuf->buffer, pitch * lines); + vbuf->bytesused += pitch * lines; } vbuf->timestamp = k_uptime_get_32(); vbuf->line_offset = 0; + + return 0; } static void video_sw_generator_worker(struct k_work *work) @@ -181,7 +284,7 @@ static void video_sw_generator_worker(struct k_work *work) switch (data->pattern) { case VIDEO_PATTERN_COLOR_BAR: - video_sw_generator_fill_colorbar(data, vbuf); + video_sw_generator_fill(data->dev, vbuf); break; } From 43f9e329408d55d1ccae3c32e2889768cca13038 Mon Sep 17 00:00:00 2001 From: Josuah Demangeon Date: Thu, 20 Feb 2025 23:06:42 +0000 Subject: [PATCH 07/12] drivers: video: sw_generator: introduce the RGB24 test pattern This complements the 32-bit RGB (XRGB32) test pattern with an equivalent 24-bit RGB (RGB24) implementation. Signed-off-by: Josuah Demangeon --- drivers/video/video_sw_generator.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/drivers/video/video_sw_generator.c b/drivers/video/video_sw_generator.c index 4167220ccff5..c59f16f5b219 100644 --- a/drivers/video/video_sw_generator.c +++ b/drivers/video/video_sw_generator.c @@ -58,6 +58,7 @@ struct video_sw_generator_data { } static const struct video_format_cap fmts[] = { + VIDEO_SW_GENERATOR_FORMAT_CAP(VIDEO_PIX_FMT_RGB24), VIDEO_SW_GENERATOR_FORMAT_CAP(VIDEO_PIX_FMT_YUYV), VIDEO_SW_GENERATOR_FORMAT_CAP(VIDEO_PIX_FMT_RGB565), VIDEO_SW_GENERATOR_FORMAT_CAP(VIDEO_PIX_FMT_XRGB32), @@ -164,6 +165,18 @@ static uint16_t video_sw_generator_fill_xrgb32(uint8_t *buffer, uint16_t width, return 1; } +static uint16_t video_sw_generator_fill_rgb24(uint8_t *buffer, uint16_t width, bool hflip) +{ + for (size_t w = 0; w < width; w++) { + int color_idx = video_sw_generator_get_color_idx(w, width, hflip); + + buffer[w * 3 + 0] = pattern_8bars_rgb[color_idx][0]; + buffer[w * 3 + 1] = pattern_8bars_rgb[color_idx][1]; + buffer[w * 3 + 2] = pattern_8bars_rgb[color_idx][2]; + } + return 1; +} + static uint16_t video_sw_generator_fill_rgb565(uint8_t *buffer, uint16_t width, bool hflip) { for (size_t w = 0; w < width; w++) { @@ -220,6 +233,9 @@ static int video_sw_generator_fill(const struct device *const dev, struct video_ case VIDEO_PIX_FMT_XRGB32: lines = video_sw_generator_fill_xrgb32(vbuf->buffer, fmt->width, hflip); break; + case VIDEO_PIX_FMT_RGB24: + lines = video_sw_generator_fill_rgb24(vbuf->buffer, fmt->width, hflip); + break; case VIDEO_PIX_FMT_RGB565: lines = video_sw_generator_fill_rgb565(vbuf->buffer, fmt->width, hflip); break; From 0391d8b221990e9afa319b45d59352511f5e7398 Mon Sep 17 00:00:00 2001 From: Josuah Demangeon Date: Sun, 23 Feb 2025 12:50:17 +0000 Subject: [PATCH 08/12] drivers: video: sw_generator: make k_work_sync a local variable The documentation of k_work_cancel_delayable_sync() states that the input k_work_sync parameter needs to be valid until the function call returns, so there is no need to preserve the state across successive calls. Now that there is a single work-related field in the struct, rename it to simply "work". Signed-off-by: Josuah Demangeon --- drivers/video/video_sw_generator.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/drivers/video/video_sw_generator.c b/drivers/video/video_sw_generator.c index c59f16f5b219..51fa0c6cf54b 100644 --- a/drivers/video/video_sw_generator.c +++ b/drivers/video/video_sw_generator.c @@ -39,8 +39,7 @@ struct video_sw_generator_data { struct video_format fmt; struct k_fifo fifo_in; struct k_fifo fifo_out; - struct k_work_delayable buf_work; - struct k_work_sync work_sync; + struct k_work_delayable work; int pattern; struct k_poll_signal *sig; uint32_t frame_rate; @@ -98,11 +97,12 @@ static int video_sw_generator_set_stream(const struct device *dev, bool enable, enum video_buf_type type) { struct video_sw_generator_data *data = dev->data; + struct k_work_sync work_sync = {0}; if (enable) { - k_work_schedule(&data->buf_work, K_MSEC(1000 / data->frame_rate)); + k_work_schedule(&data->work, K_MSEC(1000 / data->frame_rate)); } else { - k_work_cancel_delayable_sync(&data->buf_work, &data->work_sync); + k_work_cancel_delayable_sync(&data->work, &work_sync); } return 0; @@ -289,9 +289,9 @@ static void video_sw_generator_worker(struct k_work *work) struct video_sw_generator_data *data; struct video_buffer *vbuf; - data = CONTAINER_OF(dwork, struct video_sw_generator_data, buf_work); + data = CONTAINER_OF(dwork, struct video_sw_generator_data, work); - k_work_reschedule(&data->buf_work, K_MSEC(1000 / data->frame_rate)); + k_work_reschedule(&data->work, K_MSEC(1000 / data->frame_rate)); vbuf = k_fifo_get(&data->fifo_in, K_NO_WAIT); if (vbuf == NULL) { @@ -476,7 +476,7 @@ static int video_sw_generator_init(const struct device *dev) data->dev = dev; k_fifo_init(&data->fifo_in); k_fifo_init(&data->fifo_out); - k_work_init_delayable(&data->buf_work, video_sw_generator_worker); + k_work_init_delayable(&data->work, video_sw_generator_worker); return video_sw_generator_init_controls(dev); } From e9ce587983e722ed3667df271375270f36b77eef Mon Sep 17 00:00:00 2001 From: Josuah Demangeon Date: Mon, 28 Apr 2025 00:20:35 +0000 Subject: [PATCH 09/12] drivers: video: sw_generator: fix div by zero on very low framerate When the FPS value stored in data->frame_rate is zero, a division by zero occurs. Fix it by clamping the frame rate between min and max values. Signed-off-by: Josuah Demangeon --- drivers/video/video_sw_generator.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/video/video_sw_generator.c b/drivers/video/video_sw_generator.c index 51fa0c6cf54b..319fe518b573 100644 --- a/drivers/video/video_sw_generator.c +++ b/drivers/video/video_sw_generator.c @@ -28,6 +28,7 @@ LOG_MODULE_REGISTER(video_sw_generator, CONFIG_VIDEO_LOG_LEVEL); * format. 60 fps is therefore chosen as a common value in practice. */ #define MAX_FRAME_RATE 60 +#define MIN_FRAME_RATE 1 struct sw_ctrls { struct video_ctrl hflip; @@ -387,13 +388,12 @@ static int video_sw_generator_set_frmival(const struct device *dev, struct video { struct video_sw_generator_data *data = dev->data; - if (frmival->denominator && frmival->numerator) { - data->frame_rate = MIN(DIV_ROUND_CLOSEST(frmival->denominator, frmival->numerator), - MAX_FRAME_RATE); - } else { + if (frmival->denominator == 0 || frmival->numerator == 0) { return -EINVAL; } + data->frame_rate = CLAMP(DIV_ROUND_CLOSEST(frmival->denominator, frmival->numerator), + MIN_FRAME_RATE, MAX_FRAME_RATE); frmival->numerator = 1; frmival->denominator = data->frame_rate; From dabaae9921a2b492c78686da6e1814435aa0e7bb Mon Sep 17 00:00:00 2001 From: Josuah Demangeon Date: Sun, 18 May 2025 14:33:58 +0000 Subject: [PATCH 10/12] drivers: video: sw_generator: sort header alphabetically Sort alphabetically the header added in the previous few commits as well as the original implementation. Signed-off-by: Josuah Demangeon --- drivers/video/video_sw_generator.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/video/video_sw_generator.c b/drivers/video/video_sw_generator.c index 319fe518b573..46fceafa5116 100644 --- a/drivers/video/video_sw_generator.c +++ b/drivers/video/video_sw_generator.c @@ -6,12 +6,12 @@ #define DT_DRV_COMPAT zephyr_sw_generator -#include -#include #include +#include +#include #include -#include #include +#include #include "video_ctrls.h" #include "video_device.h" From 55eb71f687fcf606077bff263e2f6b853024c79d Mon Sep 17 00:00:00 2001 From: Josuah Demangeon Date: Sun, 18 May 2025 15:22:34 +0000 Subject: [PATCH 11/12] samples: drivers: video: set an initial video buffer Provide an initial video buffer in the first call to video_dequeue(). It is used by the driver for choosing between the input or output queue. Applied to capture, capture_to_lvgl and tcpserversink samples. Signed-off-by: Josuah Demangeon --- samples/drivers/video/capture/src/main.c | 3 ++- samples/drivers/video/capture_to_lvgl/src/main.c | 3 ++- samples/drivers/video/tcpserversink/src/main.c | 3 ++- 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/samples/drivers/video/capture/src/main.c b/samples/drivers/video/capture/src/main.c index f5afea592542..f2f42181bf12 100644 --- a/samples/drivers/video/capture/src/main.c +++ b/samples/drivers/video/capture/src/main.c @@ -90,7 +90,8 @@ static inline void video_display_frame(const struct device *const display_dev, int main(void) { - struct video_buffer *buffers[CONFIG_VIDEO_BUFFER_POOL_NUM_MAX], *vbuf; + struct video_buffer *buffers[CONFIG_VIDEO_BUFFER_POOL_NUM_MAX]; + struct video_buffer *vbuf = &(struct video_buffer){}; struct video_format fmt; struct video_caps caps; struct video_frmival frmival; diff --git a/samples/drivers/video/capture_to_lvgl/src/main.c b/samples/drivers/video/capture_to_lvgl/src/main.c index 81883577ad8f..c5f37b69b726 100644 --- a/samples/drivers/video/capture_to_lvgl/src/main.c +++ b/samples/drivers/video/capture_to_lvgl/src/main.c @@ -19,7 +19,8 @@ LOG_MODULE_REGISTER(main); int main(void) { - struct video_buffer *buffers[2], *vbuf; + struct video_buffer *buffers[2]; + struct video_buffer *vbuf = &(struct video_buffer){}; const struct device *display_dev; struct video_format fmt; struct video_caps caps; diff --git a/samples/drivers/video/tcpserversink/src/main.c b/samples/drivers/video/tcpserversink/src/main.c index 85d321bf8e7a..05c190013ad0 100644 --- a/samples/drivers/video/tcpserversink/src/main.c +++ b/samples/drivers/video/tcpserversink/src/main.c @@ -38,7 +38,8 @@ int main(void) { struct sockaddr_in addr, client_addr; socklen_t client_addr_len = sizeof(client_addr); - struct video_buffer *buffers[2], *vbuf; + struct video_buffer *buffers[2]; + struct video_buffer *vbuf = &(struct video_buffer){}; int i, ret, sock, client; struct video_format fmt; struct video_caps caps; From 0850ee812730e36006965e1ea8c2c136d8fbe0dd Mon Sep 17 00:00:00 2001 From: Josuah Demangeon Date: Sun, 18 May 2025 15:26:33 +0000 Subject: [PATCH 12/12] samples: drivers: video: capture: logging improvements Remove trailing spaces at the end of log string. Move arguments on the next line for readability. Align arguments to first line. Signed-off-by: Josuah Demangeon --- samples/drivers/video/capture/src/main.c | 28 ++++++++++++------------ 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/samples/drivers/video/capture/src/main.c b/samples/drivers/video/capture/src/main.c index f2f42181bf12..eba4e4f8f959 100644 --- a/samples/drivers/video/capture/src/main.c +++ b/samples/drivers/video/capture/src/main.c @@ -37,10 +37,10 @@ static inline int display_setup(const struct device *const display_dev, const ui LOG_INF("- Capabilities:"); LOG_INF(" x_resolution = %u, y_resolution = %u, supported_pixel_formats = %u" - " current_pixel_format = %u, current_orientation = %u", - capabilities.x_resolution, capabilities.y_resolution, - capabilities.supported_pixel_formats, capabilities.current_pixel_format, - capabilities.current_orientation); + " current_pixel_format = %u, current_orientation = %u", + capabilities.x_resolution, capabilities.y_resolution, + capabilities.supported_pixel_formats, capabilities.current_pixel_format, + capabilities.current_orientation); /* Set display pixel format to match the one in use by the camera */ switch (pixfmt) { @@ -132,9 +132,9 @@ int main(void) const struct video_format_cap *fcap = &caps.format_caps[i]; /* fourcc to string */ LOG_INF(" %s width [%u; %u; %u] height [%u; %u; %u]", - VIDEO_FOURCC_TO_STR(fcap->pixelformat), - fcap->width_min, fcap->width_max, fcap->width_step, - fcap->height_min, fcap->height_max, fcap->height_step); + VIDEO_FOURCC_TO_STR(fcap->pixelformat), + fcap->width_min, fcap->width_max, fcap->width_step, + fcap->height_min, fcap->height_max, fcap->height_step); i++; } @@ -168,7 +168,7 @@ int main(void) if (!video_get_frmival(video_dev, &frmival)) { LOG_INF("- Default frame rate : %f fps", - 1.0 * frmival.denominator / frmival.numerator); + 1.0 * frmival.denominator / frmival.numerator); } LOG_INF("- Supported frame intervals for the default format:"); @@ -176,12 +176,12 @@ int main(void) fie.format = &fmt; while (video_enum_frmival(video_dev, &fie) == 0) { if (fie.type == VIDEO_FRMIVAL_TYPE_DISCRETE) { - LOG_INF(" %u/%u ", fie.discrete.numerator, fie.discrete.denominator); + LOG_INF(" %u/%u", fie.discrete.numerator, fie.discrete.denominator); } else { LOG_INF(" [min = %u/%u; max = %u/%u; step = %u/%u]", - fie.stepwise.min.numerator, fie.stepwise.min.denominator, - fie.stepwise.max.numerator, fie.stepwise.max.denominator, - fie.stepwise.step.numerator, fie.stepwise.step.denominator); + fie.stepwise.min.numerator, fie.stepwise.min.denominator, + fie.stepwise.max.numerator, fie.stepwise.max.denominator, + fie.stepwise.step.numerator, fie.stepwise.step.denominator); } fie.index++; } @@ -263,8 +263,8 @@ int main(void) return 0; } - LOG_DBG("Got frame %u! size: %u; timestamp %u ms", frame++, vbuf->bytesused, - vbuf->timestamp); + LOG_DBG("Got frame %u! size: %u; timestamp %u ms", + frame++, vbuf->bytesused, vbuf->timestamp); #ifdef CONFIG_TEST if (is_colorbar_ok(vbuf->buffer, fmt)) {