Re: [raw2rgbpnm PATCH v4 08/11] Add --stride (-S) option for setting stride

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

On Tue, Mar 10, 2026 at 10:46:14AM +0200, Sakari Ailus wrote:
> Being able to set stride explicitly is useful in some cases.

Definitely.

> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> ---
>  raw2rgbpnm.c | 24 +++++++++++++++---------
>  1 file changed, 15 insertions(+), 9 deletions(-)
> 
> diff --git a/raw2rgbpnm.c b/raw2rgbpnm.c
> index 0e0a5b1e63e9..9ec9f06de052 100644
> --- a/raw2rgbpnm.c
> +++ b/raw2rgbpnm.c
> @@ -139,11 +139,11 @@ static const struct format_info *get_format_info(__u32 f)
>  }
>  
>  /* Read and return image data at given width, height and format information. */
> -static unsigned char *read_raw_data(char *filename, int width, int height,
> +static unsigned char *read_raw_data(char *filename, unsigned int width,
> +				    unsigned int height, unsigned int line_length,
>  				    const struct format_info *info)
>  {
>  	/* Get file size */
> -	unsigned int line_length;
>  	unsigned int padding = 0;
>  	unsigned char *b = NULL;
>  	unsigned int i;
> @@ -174,9 +174,9 @@ static unsigned char *read_raw_data(char *filename, int width, int height,
>  	}
>  
>  	/* Read data */
> -	b = xalloc((width*height*bpp+7)/8);
> +	b = xalloc(line_length * height);
>  	if (padding == 0) {
> -		r = fread(b, (width*height*bpp+7)/8, 1, f);
> +		r = fread(b, line_length * height, 1, f);
>  		if (r != 1)
>  			error("fread");
>  	} else {
> @@ -327,10 +327,11 @@ static int raw_layout_to_grbg(const struct format_info *info, unsigned char *src
>  }
>  
>  static void raw_to_rgb(const struct format_info *info,
> -		       unsigned char *src, int src_width, int src_height, unsigned char *rgb)
> +		       unsigned char *src, int src_width, int src_height,
> +		       unsigned int stride, unsigned char *rgb)
>  {
>  	unsigned char *tmp_src = NULL;
> -	unsigned int src_stride = src_width * info->bpp / 8;
> +	unsigned int src_stride = stride ?: src_width * info->bpp / 8;

I assume the rest of the code already validates the stride value to
avoid buffer overflows. Have you checked that it does so in a way that
avoid integer overflows ? If so,

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

>  	unsigned int rgb_stride = src_width * 3;
>  	unsigned char *src_luma, *src_chroma;
>  	unsigned char *src_cb, *src_cr;
> @@ -844,6 +845,7 @@ static struct option options[] = {
>  	{ "help", no_argument, NULL, 'h', },
>  	{ "high-bits", no_argument, NULL, 'g', },
>  	{ "size", required_argument, NULL, 's', },
> +	{ "stride", required_argument, NULL, 'S', },
>  	{ "swap-rb", no_argument, NULL, 'w', },
>  	{ 0 },
>  };
> @@ -855,13 +857,14 @@ int main(int argc, char *argv[])
>  	char *file_in = NULL, *file_out = NULL;
>  	int format = V4L2_PIX_FMT_UYVY;
>  	const struct format_info *info;
> +	unsigned int stride = 0;
>  	int r;
>  	char *algorithm_name = NULL;
>  	int height = -1;
>  	int width = -1;
>  
>  	for (;;) {
> -		int c = getopt_long(argc, argv, "a:b:f:ghs:w", options, NULL);
> +		int c = getopt_long(argc, argv, "a:b:f:ghs:S:w", options, NULL);
>  		if (c==-1) break;
>  		switch (c) {
>  		case 'a':
> @@ -920,6 +923,9 @@ int main(int argc, char *argv[])
>  				exit(0);
>  			}
>  			break;
> +		case 'S':
> +			stride = strtoul(optarg, NULL, 10);
> +			break;
>  		case 'w':
>  			swaprb = 1;
>  			break;
> @@ -943,12 +949,12 @@ int main(int argc, char *argv[])
>  	}
>  
>  	/* Read, convert, and save image */
> -	src = read_raw_data(file_in, width, height, info);
> +	src = read_raw_data(file_in, width, height, stride, info);
>  	printf("Image size: %ix%i, bytes per pixel: %i, format: %s\n",
>  	       width, height, info->bpp, info->name);
>  	dst = xalloc(width*height*3);
>  
> -	raw_to_rgb(info, src, width, height, dst);
> +	raw_to_rgb(info, src, width, height, stride, dst);
>  	printf("Writing to file `%s'...\n", file_out);
>  	f = fopen(file_out, "wb");
>  	if (!f) error("file open failed");

-- 
Regards,

Laurent Pinchart