[PATCH] cam: Convert RGB variations to BGR

[Milan Zamazal]

Hi Pavel,

Pavel Machek <pavel at ucw.cz> writes:

> Hi!
>
>> > > > 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>
>
>> > > > -	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 that's the only change required, yes.
>
> Thanks!
>
>> > > > +	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.
>> 
>> I wonder if it would make sense for the soft ISP to support other RGB
>> formats. CC'ing Milan and Hans to get their opinion.
>
> From looking at the code, soft ISP seemed to be able to do other stuff
> than ARGB, but for some reason ARGB is selected by default, and I
> guess I can get better performance by forcing right format.

Software ISP debayering supports multiple output formats and `cam' can
request a particular output format.  For example,

  cam -c1 -C4 -s pixelformat=BGR888 -Ffoo#.ppm

outputs the right thing.  Example of requesting an output in a different
format:

  cam -c1 -C4 -s pixelformat=ARGB8888 -Ffoo#.raw

It works; is it a problem in your application?

I don't mind enhancing the PPM writer but maybe you don't need the
conversion there in the first place.

> Conversion will still be useful for when hardware produces some other
> permutation than BGR.
>
> Best regards,
> 								Pavel