drivers: video: Add format size #97197

ngphibang · 2025-10-08T13:12:39Z

Add a size field into video_format struct.
Video receiver drivers now need to set the format's size to expose to application. Application should base on the format size to allocate buffers. The caps' min/max_line_count fields (which are needed only for HWs that cannot support the whole image frame) can hence be dropped.

avolmat-st · 2025-10-08T13:28:23Z

It seems to me that currently there is a hole if a sensor provides a compressed format (ex: OV5640 which is capable, from a HW point of view, to provide JPEG data).
In such case, a receiver should not check base its calculation on the width, video_bits_per_pixel and height but rather directly on the size value provided by the source itself since, in case of compressed data there is just no way to figure out what could be size. The best entity to estimate this is the producer of the data, right ?

So, if the video_bits_per_pixel reports a non zero value, a receiver can estimate the pitch and size (as done in this PR), however if the video_bits_per_pixel is zero (as it would be in case of JPEG), then we cannot apply this calculation and need to get this from another way. (either from the sensor itself, since it could estimate it itself, knowing the quality settings etc etc, or by a rather simple estimation in each receiver, but not based on the video_bits_per_pixel).

ngphibang · 2025-10-08T13:39:17Z

It seems to me that currently there is a hole if a sensor provides a compressed format (ex: OV5640 which is capable, from a HW point of view, to provide JPEG data)...

Yes, we should distinguish uncompressed vs compressed formats (can be based on fmt->pitch or video_bits_per_pixel).
For compressed formats, the size will be an estimate for the worse case as described in the fmt->field size. But the estimation will be added later when support for compressed image in sensors are added (e.g. ov5640).

Will update.

avolmat-st · 2025-10-08T13:42:17Z

It seems to me that currently there is a hole if a sensor provides a compressed format (ex: OV5640 which is capable, from a HW point of view, to provide JPEG data)...

Yes, we should distinguish uncompressed vs compressed formats (can be based on fmt->pitch or video_bits_per_pixel). For compressed formats, the size will be an estimate for the worse case as described in the fmt->field size. Will update.

Yes. Cannot be done on the pitch value I think since pitch is related to the way it is organized into memory, which is not applicable for sensors (which do not write themselves into memory). Hence my proposal to look at video_bits_per_pixel.
It is assume that sensor would not set the pitch. For uncompressed format, sensor do not need to set fmt.size, however for compressed format they will have to set fmt.size to give a worst case frame size.

josuah

https://github.com/zephyrproject-rtos/zephyr/blob/main/drivers/video/video_shell.c
This has a print statement for logging the max count

https://github.com/zephyrproject-rtos/zephyr/blob/main/drivers/video/video_emul_rx.c
This will be needing some conversion too, useful for testing this change on CI.

This sounds like a better solution than what I proposed here IMHO #92884 (comment)

include/zephyr/drivers/video.h

drivers/video/video_mcux_smartdma.c

avolmat-st

Thanks @ngphibang. All good to me.

drivers/video/video_esp32_dvp.c

ngphibang · 2025-10-10T07:29:04Z

Dropped caps' min_line_count in the recently merged VENC driver.

ngphibang · 2025-10-10T07:41:14Z

Added 4.3 milestone as #95862 was merged which requires drivers to set format size

josuah · 2025-10-10T09:28:50Z

drivers/video/video_common.c

+		return -EINVAL;
+	}
+
+	fmt->pitch = fmt->width * video_bits_per_pixel(fmt->pixelformat) / BITS_PER_BYTE;


Good idea to also set the pitch.

avolmat-st · 2025-10-10T10:25:03Z

include/zephyr/drivers/video.h

+ * For compressed formats, it gives a rough estimate size of a complete
+ * compressed frame.
+ *
+ * @param fmt The video format


What about

@param fmt Pointer to video format struct

so that we use same wording as for other helpers.

avolmat-st · 2025-10-10T10:29:09Z

drivers/video/video_common.c

+		return -EINVAL;
+	}
+
+	fmt->pitch = fmt->width * video_bits_per_pixel(fmt->pixelformat) / BITS_PER_BYTE;


This fmt->pitch could be done within the default case since it doesn't make sense for the JPEG part for which this will always lead to 0. Thus what about setting fmt->pitch = 0 for JPEG instead ?

avolmat-st · 2025-10-10T10:32:20Z

drivers/video/video_common.c

+		fmt->size = fmt->width * fmt->height * 2;
+		return 0;
+	default:
+		if (fmt->pitch != 0) {


Doing the opposite test, aka if (fmt->pitch == 0) { return -ENOSUP; } would allow to have less { } { } and keep the expected behavior code with less indentation.

Aka something like:

switch (fmt->pixelformat) { case VIDEO_PIX_FMT_JPEG: /* Rough estimate for the worst case (quality = 100) */ fmt->pitch = 0; fmt->size = fmt->width * fmt->height * 2; break; default: /* Uncompressed format */ fmt->pitch = fmt->width * video_bits_per_pixel(fmt->pixelformat) / BITS_PER_BYTE; if (fmt->pitch == 0) { return -ENOTSUP; } fmt->size = fmt->pitch * fmt->height; break; } return 0;

Oups, noticed break were missing here ;) Updated.

avolmat-st · 2025-10-10T10:35:52Z

drivers/video/video_emul_rx.c

 	}

 	/* Cache the format selected locally to use it for getting the size of the buffer  */
 	fmt->pitch = fmt->width * video_bits_per_pixel(fmt->pixelformat) / BITS_PER_BYTE;


To be replaced by video_estimate_fmt_size call ?

avolmat-st · 2025-10-10T10:36:21Z

drivers/video/video_mcux_csi.c

 		break;
 	}

 	fmt->pitch = fmt->width * video_bits_per_pixel(fmt->pixelformat) / BITS_PER_BYTE;


To be replaced by video_estimate_fmt_size ?

avolmat-st · 2025-10-10T10:37:46Z

drivers/video/video_renesas_ra_ceu.c

 		return ret;
 	}

 	fmt->pitch = fmt->width * video_bits_per_pixel(fmt->pixelformat) / BITS_PER_BYTE;


Use video_estimate_fmt_size ?

avolmat-st · 2025-10-10T10:37:59Z

drivers/video/video_renesas_ra_ceu.c

 		return -EIO;
 	}

 	fmt->pitch = fmt->width * video_bits_per_pixel(fmt->pixelformat) / BITS_PER_BYTE;


Use video_estimate_fmt_size ?

avolmat-st · 2025-10-10T10:38:44Z

drivers/video/video_sw_generator.c

 		return ret;
 	}

 	fmt->pitch = fmt->width * video_bits_per_pixel(fmt->pixelformat) / BITS_PER_BYTE;


Use video_estimate_fmt_size ?

avolmat-st · 2025-10-10T10:39:22Z

drivers/video/video_sw_generator.c


 	*fmt = data->fmt;

+	fmt->pitch = fmt->width * video_bits_per_pixel(fmt->pixelformat) / BITS_PER_BYTE;


Use video_estimate_fmt_size ?

include/zephyr/drivers/video.h

avolmat-st · 2025-10-10T13:26:27Z

drivers/video/video_common.c

+		/* Rough estimate for the worst case (quality = 100) */
+		fmt->pitch = 0;
+		fmt->size = fmt->width * fmt->height * 2;
+		return 0;


return 0 at the end of the function seems better to me since currently a rapid look is showing that nothing is returned.

It returns for all cases and the compiler does not complain. I think if we return at the end, we need to do "break;" and this will take more lines, just that.

Yeah I wasn't pointing that a return was missing, I am only pointing that this feel as if there were a return missing at the end since this is a non void function which finish with

} }

I'd feel better to have in such case only one return.

avolmat-st · 2025-10-10T13:29:50Z

drivers/video/video_emul_rx.c


 	/* Cache the format selected locally to use it for getting the size of the buffer  */
-	fmt->pitch = fmt->width * video_bits_per_pixel(fmt->pixelformat) / BITS_PER_BYTE;
+	ret = video_estimate_fmt_size(fmt);


Ok for this part. However I think similar one is missing in the init function of this driver since doing a get_fmt without a set first will lead just read copy the fmt structure initialized during the init, into which only pitch is set. Hence video_estimate_fmt_size can simply be called during the _init function instead of the existing fmt->pitch set.

Yes, it lacks a call in init(), will add it, but we still need to call video_estimate_fmt_size() in each set_format() because the call in init() is only for the default format.

Yes agree. I just used this part to comment since I couldn't point the _init section of course ;)

avolmat-st · 2025-10-10T13:34:12Z

drivers/video/video_mcux_csi.c

 	}

-	fmt->pitch = fmt->width * video_bits_per_pixel(fmt->pixelformat) / BITS_PER_BYTE;
+	video_estimate_fmt_size(fmt);


Ok. However it seems to me that video_estimate_fmt_size call is missing the in get_fmt function, in case of CONFIG_VIDEO_MCUX_MIPI_CSI2RX is not defined. In such situation result of the video_get_format from the source device is returned straight.

Add a helper to estimate format size and pitch. Signed-off-by: Phi Bang Nguyen <[email protected]>

Receiver drivers now need to set the format's size to expose it to the application. Application should base on the format size to allocate buffers. The caps' min/max_line_count (which are needed only for HWs that cannot support the whole image frame) can hence be dropped. Signed-off-by: Phi Bang Nguyen <[email protected]>

The mipid02 is just a bridge driver which does not deal with memory so should not expose caps' min_vbuf_count. Signed-off-by: Phi Bang Nguyen <[email protected]>

Mention some dropped fields in the video_caps structure and the buffer allocation size for application. Signed-off-by: Phi Bang Nguyen <[email protected]>

Mention a new video_estimate_fmt_size helper. Signed-off-by: Phi Bang Nguyen <[email protected]>

avolmat-st

LGTM. Thanks a lot.

sonarqubecloud · 2025-10-10T15:13:56Z

Quality Gate passed

Issues
1 New issue
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

avolmat-st · 2025-10-10T15:38:40Z

Regarding the new issue reported, we might want to add more compressed format in the future in this generic helper so I propose to not take it into account.

zephyrbot added platform: Renesas RA Renesas Electronics Corporation, RA platform: ESP32 Espressif ESP32 area: Samples Samples platform: STM32 ST Micro STM32 area: Video Video subsystem labels Oct 8, 2025

ngphibang mentioned this pull request Oct 8, 2025

video: introduction of STM32 VENC driver for H264 hardware video compression #92884

Merged

zephyrbot added platform: NXP NXP area: USB Universal Serial Bus labels Oct 8, 2025

zephyrbot requested review from FRASTM, GeorgeCGV, avolmat-st, dbaluta, decsny, djiatsaf-st, dleach02, erwango, gautierg-st, josuah, loicpoulain, manuargue, marwaiehm-st, mathieuchopstm and mmahadevan108 October 8, 2025 13:14

zephyrbot assigned josuah and ngphibang Oct 8, 2025

decsny removed their request for review October 8, 2025 13:36

josuah reviewed Oct 8, 2025

View reviewed changes

include/zephyr/drivers/video.h Outdated Show resolved Hide resolved

drivers/video/video_mcux_smartdma.c Show resolved Hide resolved

ngphibang force-pushed the add_fmt_size branch from 8c52f1e to c4b4648 Compare October 9, 2025 09:51

avolmat-st previously approved these changes Oct 9, 2025

View reviewed changes

ngphibang dismissed avolmat-st’s stale review via bb65040 October 9, 2025 14:04

ngphibang force-pushed the add_fmt_size branch from c4b4648 to bb65040 Compare October 9, 2025 14:04

avolmat-st reviewed Oct 9, 2025

View reviewed changes

drivers/video/video_esp32_dvp.c Outdated Show resolved Hide resolved

ngphibang force-pushed the add_fmt_size branch 3 times, most recently from 73736a2 to ffea025 Compare October 10, 2025 07:27

ngphibang added this to the v4.3.0 milestone Oct 10, 2025

josuah requested a review from avolmat-st October 10, 2025 09:25

josuah previously approved these changes Oct 10, 2025

View reviewed changes

avolmat-st requested changes Oct 10, 2025

View reviewed changes

ngphibang dismissed josuah’s stale review via 3b29e74 October 10, 2025 13:17

ngphibang force-pushed the add_fmt_size branch 2 times, most recently from 3b29e74 to da0ba92 Compare October 10, 2025 13:22

avolmat-st requested changes Oct 10, 2025

View reviewed changes

drivers: video: Add helper to estimate format size

3a0bb4a

Add a helper to estimate format size and pitch. Signed-off-by: Phi Bang Nguyen <[email protected]>

ngphibang force-pushed the add_fmt_size branch from da0ba92 to a32e42b Compare October 10, 2025 14:37

ngphibang added 4 commits October 10, 2025 16:48

drivers: video: st_mipid02: Drop caps' min_vbuf_count

7aa63f7

The mipid02 is just a bridge driver which does not deal with memory so should not expose caps' min_vbuf_count. Signed-off-by: Phi Bang Nguyen <[email protected]>

doc: migration-guide-4.3: Mention some changes in video structures

c3511b5

Mention some dropped fields in the video_caps structure and the buffer allocation size for application. Signed-off-by: Phi Bang Nguyen <[email protected]>

doc: release-note-4.3: Add video format estimation helper

ab8ea7b

Mention a new video_estimate_fmt_size helper. Signed-off-by: Phi Bang Nguyen <[email protected]>

ngphibang force-pushed the add_fmt_size branch from a32e42b to ab8ea7b Compare October 10, 2025 14:48

avolmat-st approved these changes Oct 10, 2025

View reviewed changes

josuah approved these changes Oct 10, 2025

View reviewed changes

cfriedt merged commit c1a27a1 into zephyrproject-rtos:main Oct 10, 2025
26 checks passed


		*fmt = data->fmt;

		fmt->pitch = fmt->width * video_bits_per_pixel(fmt->pixelformat) / BITS_PER_BYTE;

drivers: video: Add format size #97197

drivers: video: Add format size #97197

Uh oh!

Conversation

ngphibang commented Oct 8, 2025

Uh oh!

avolmat-st commented Oct 8, 2025

Uh oh!

ngphibang commented Oct 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

avolmat-st commented Oct 8, 2025

Uh oh!

josuah left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

avolmat-st left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ngphibang commented Oct 10, 2025

Uh oh!

ngphibang commented Oct 10, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

avolmat-st Oct 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

avolmat-st Oct 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

avolmat-st left a comment

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud bot commented Oct 10, 2025

Quality Gate passed

Uh oh!

avolmat-st commented Oct 10, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

ngphibang commented Oct 8, 2025 •

edited

Loading

avolmat-st Oct 10, 2025 •

edited

Loading

avolmat-st Oct 10, 2025 •

edited

Loading