Re: [raw2rgbpnm,PATCH] Add support for all RAW memory layouts

["Niklas Söderlund" <niklas.soderlund@ragnatech.se>]

Hi Laurent,

On 2026-01-22 11:20:04 +0200, Laurent Pinchart wrote:
> Hi Niklas,
> 
> I know this has been merged already, but it only came to my attention
> now.
> 
> On Sun, May 11, 2025 at 05:46:59PM +0200, Niklas Söderlund wrote:
> > Convert all supported RAW input images to GRBG memory layout before
> > feeding it to the RAW to RGB conversion algorithms. This way all layouts
> > can produced good colors in the output image.
> 
> I don't think the result will be accurate. It does produce better
> colours, but all liens and columns in the output image end up being
> swapped in groups of two. I would have kept printing a warning message.

Thinking about it now, yes having kept or updated the warning message to 
make the user aware of the result not being pixel perfect would likely 
have been a good idea.

> 
> > Limit the conversion to input images that have an even width and height
> > to make the conversion easier.
> > 
> > Images with and uneven width or hight can sill be processed, but will
> > be processed without first being converted to GRBG layout. Such input
> > images will result, just as before this change, in output images with
> > bad colors. The warning message about this have been preserved.
> > 
> > Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
> > ---
> >  raw2rgbpnm.c | 98 ++++++++++++++++++++++++++++++++++++++++++++++++++--
> >  1 file changed, 95 insertions(+), 3 deletions(-)
> > 
> > diff --git a/raw2rgbpnm.c b/raw2rgbpnm.c
> > index 5838bc7347e9..b834add4ea6c 100644
> > --- a/raw2rgbpnm.c
> > +++ b/raw2rgbpnm.c
> > @@ -186,6 +186,97 @@ static unsigned char *read_raw_data(char *filename, int width, int height,
> >  	return b;
> >  }
> >  
> > +static int raw_layout_to_grbg(const struct format_info *info, unsigned char *src,
> > +			      int src_width, int src_height, unsigned int src_stride)
> > +{
> > +	int swap_line = 0;
> > +	int swap_gbrg = 0;
> > +	int dst_x, dst_y;
> > +
> > +	switch (info->fmt) {
> > +	case V4L2_PIX_FMT_SBGGR16:
> > +	case V4L2_PIX_FMT_SBGGR14:
> > +	case V4L2_PIX_FMT_SBGGR12:
> > +	case V4L2_PIX_FMT_SBGGR10:
> > +	case V4L2_PIX_FMT_SBGGR8:
> > +		swap_line = 1; /* BGGR -> GBRG */
> > +		swap_gbrg = 1; /* GBRG -> GRBG */
> > +		break;
> > +	case V4L2_PIX_FMT_SGBRG16:
> > +	case V4L2_PIX_FMT_SGBRG14:
> > +	case V4L2_PIX_FMT_SGBRG12:
> > +	case V4L2_PIX_FMT_SGBRG10:
> > +	case V4L2_PIX_FMT_SGBRG8:
> > +		swap_gbrg = 1; /* GBRG -> GRBG */
> > +		break;
> > +	case V4L2_PIX_FMT_SRGGB16:
> > +	case V4L2_PIX_FMT_SRGGB14:
> > +	case V4L2_PIX_FMT_SRGGB12:
> > +	case V4L2_PIX_FMT_SRGGB10:
> > +	case V4L2_PIX_FMT_SRGGB8:
> > +		swap_line = 1; /* RGGB -> GRBG */
> > +		break;
> > +	case V4L2_PIX_FMT_SGRBG16:
> > +	case V4L2_PIX_FMT_SGRBG14:
> > +	case V4L2_PIX_FMT_SGRBG12:
> > +	case V4L2_PIX_FMT_SGRBG10:
> > +	case V4L2_PIX_FMT_SGRBG8:
> > +		/* No conversion needed. */
> > +		break;
> > +	default:
> > +		return -1;
> > +	}
> > +
> > +	if (src_width % 2 != 0 || src_height % 2 != 0) {
> > +		printf("WARNING: bayer layout conversion not supported for uneven sized images -> expect bad colors\n");
> > +		return 0;
> > +	}
> > +
> > +	if (swap_line) {
> > +		/* Swap pixel pairs on each line AB -> BA. */
> > +		for (dst_y=0; dst_y<src_height; dst_y++) {
> > +			for (dst_x=0; dst_x<src_width; dst_x += 2) {
> > +				if (info->bpp == 8) {
> > +					unsigned char *p1 = &(src[src_stride*dst_y+(dst_x+0)]);
> > +					unsigned char *p2 = &(src[src_stride*dst_y+(dst_x+1)]);
> > +					unsigned char tmp = *p1;
> > +					*p1 = *p2;
> > +					*p2 = tmp;
> > +				} else {
> > +					unsigned short *p1 = (unsigned short *)&(src[src_stride*dst_y+(dst_x+0)*2]);
> > +					unsigned short *p2 = (unsigned short *)&(src[src_stride*dst_y+(dst_x+1)*2]);
> > +					unsigned short tmp = *p1;
> > +					*p1 = *p2;
> > +					*p2 = tmp;
> > +				}
> > +			}
> > +		}
> > +	}
> > +
> > +	if (swap_gbrg) {
> > +		/* Swap R and B components, format is always GBRG at this point. */
> > +		for (dst_y=0; dst_y<src_height; dst_y += 2) {
> > +			for (dst_x=0; dst_x<src_width; dst_x += 2) {
> > +				if (info->bpp == 8) {
> > +					unsigned char *p1 = &(src[src_stride*(dst_y+0)+(dst_x+1)]);
> > +					unsigned char *p2 = &(src[src_stride*(dst_y+1)+(dst_x+0)]);
> > +					unsigned char tmp = *p1;
> > +					*p1 = *p2;
> > +					*p2 = tmp;
> > +				} else {
> > +					unsigned short *p1 = (unsigned short *)&(src[src_stride*(dst_y+0)+(dst_x+1)*2]);
> > +					unsigned short *p2 = (unsigned short *)&(src[src_stride*(dst_y+1)+(dst_x+0)*2]);
> > +					unsigned short tmp = *p1;
> > +					*p1 = *p2;
> > +					*p2 = tmp;
> > +				}
> > +			}
> > +		}
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> >  static void raw_to_rgb(const struct format_info *info,
> >  		       unsigned char *src, int src_width, int src_height, unsigned char *rgb)
> >  {
> > @@ -480,7 +571,8 @@ static void raw_to_rgb(const struct format_info *info,
> >  	case V4L2_PIX_FMT_SBGGR10:
> >  	case V4L2_PIX_FMT_SGBRG10:
> >  	case V4L2_PIX_FMT_SRGGB10:
> > -		printf("WARNING: bayer phase not supported -> expect bad colors\n");
> > +		if (raw_layout_to_grbg(info, src, src_width, src_height, src_stride))
> > +			error("Can't convert RAW layout to GRBG");
> >  		/* fallthrough */
> >  	case V4L2_PIX_FMT_SGRBG16:
> >  	case V4L2_PIX_FMT_SGRBG14:
> > @@ -523,10 +615,10 @@ static void raw_to_rgb(const struct format_info *info,
> >  	case V4L2_PIX_FMT_SBGGR8:
> >  	case V4L2_PIX_FMT_SGBRG8:
> >  	case V4L2_PIX_FMT_SRGGB8:
> > -		printf("WARNING: bayer phase not supported -> expect bad colors\n");
> > +		if (raw_layout_to_grbg(info, src, src_width, src_height, src_stride))
> > +			error("Can't convert RAW layout to GRBG");
> >  		/* fallthrough */
> >  	case V4L2_PIX_FMT_SGRBG8:
> > -		/* FIXME: only SGRBG8 handled properly: color phase is ignored. */
> >  		buf = malloc(src_width * src_height * 3);
> >  		if (buf==NULL) error("out of memory");
> >  		qc_imag_bay2rgb8(src, src_stride, buf, src_width*3, src_width, src_height, 3);
> 
> -- 
> Regards,
> 
> Laurent Pinchart

-- 
Kind Regards,
Niklas Söderlund