[PATCH] cam: Convert RGB variations to BGR

[Pavel Machek]

Hi!

> Thank you for the patch.
> 
> On Sun, Apr 13, 2025 at 10:21:33AM +0200, Pavel Machek wrote:
> > swIsp with libcamera produces ARGB data by default, which needs
> > conversion during write. Implement it and similar conversions.
> >     
> > Signed-off-by: Pavel Machek <pavel at ucw.cz>

> > +	case libcamera::formats::BGRA8888:
> > +	case libcamera::formats::BGRX8888:
> > +		r_pos = 1;
> > +		g_pos = 2;
> > +		b_pos = 3;
> > +		bpp = 4;
> > +		break;
> > +	default:
> > +		std::cerr << "Only RGB output pixel formats are supported ("
> >  			  << config.pixelFormat << " requested)" << std::endl;
> >  		return -EINVAL;
> >  	}
> > @@ -41,14 +89,35 @@ int PPMWriter::write(const char *filename,
> >  	}
> >  
> >  	const unsigned int rowLength = config.size.width * 3;
> > -	const char *row = reinterpret_cast<const char *>(data.data());
> > -	for (unsigned int y = 0; y < config.size.height; y++, row += config.stride) {
> > -		output.write(row, rowLength);
> > +	// Output without any conversion will be slightly faster
> 
> We use C-style coments.

Can you adjust that while applying?

> > +	if (config.pixelFormat == formats::BGR888) {
> > +		const char *row = reinterpret_cast<const char *>(data.data());
> > +		for (unsigned int y = 0; y < config.size.height; y++, row += config.stride) {
> > +			output.write(row, rowLength);
> > +			if (!output) {
> > +				std::cerr << "Failed to write image data at row " << y << std::endl;
> > +				return -EIO;
> > +			}
> > +		}
> > +		return 0;
> > +	}
> > +
> > +	const unsigned int inputRowBytes = config.stride; // should be >= width * 4
> > +	// Create a temporary buffer to hold one output row.
> > +	std::vector<char> rowBuffer(rowLength);
> > +	const uint8_t *row = reinterpret_cast<const uint8_t *>(data.data());
> > +	for (unsigned int y = 0; y < config.size.height; y++, row += inputRowBytes) {
> > +		for (unsigned int x = 0; x < config.size.width; x++) {
> > +			const uint8_t *pixel = row + x * bpp;
> > +			rowBuffer[x * 3 + 0] = static_cast<char>(pixel[r_pos]);
> > +			rowBuffer[x * 3 + 1] = static_cast<char>(pixel[g_pos]);
> > +			rowBuffer[x * 3 + 2] = static_cast<char>(pixel[b_pos]);
> > +		}
> 
> I'm curious, what's the performance impact of the conversion ?

I hacked Librem 5 a lot to get libcamera working, and previous config
was not working at all, so ... I can't easily benchmark that.

So, this one is not free as it does one more copy of image
data. Compared to swIsp, it is going to be lost in the noise.

There's easy optimalization in the original case not to do writes per
row if width == stride. There are other options, but they will
complicate the code.

Best regards,
								Pavel
-- 
I don't work for Nazis and criminals, and neither should you.
Boycott Putin, Trump, and Musk!
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 195 bytes
Desc: not available
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20250419/6ed956c0/attachment.sig>