[PATCH v3 15/16] libcamera: debayer_cpu: Add BGR888 output support

[Milan Zamazal]

Hans de Goede <hdegoede at redhat.com> writes:

> BGR888 is RGB888 with the red and blue pixels swapped, adjust
> the debayering to swap the red and blue pixels in the bayer pattern
> to add support for writing formats::BGR888.

Hmm, a simple and efficient trick, but indeed confusing and error-prone.  After
looking at the patch and browsing the sources for quite a while, I /think/ it
/probably/ works as expected.

Can anything be done reasonably to reduce the risk of breaking this with
possible future changes to stats/debayering?

> Tested-by: Bryan O'Donoghue <bryan.odonoghue at linaro.org> # sc8280xp Lenovo x13s
> Tested-by: Pavel Machek <pavel at ucw.cz>
> Signed-off-by: Hans de Goede <hdegoede at redhat.com>
> ---
>  src/libcamera/software_isp/debayer_cpu.cpp | 42 +++++++++++++++++++---
>  src/libcamera/software_isp/debayer_cpu.h   |  1 +
>  2 files changed, 38 insertions(+), 5 deletions(-)
>
> diff --git a/src/libcamera/software_isp/debayer_cpu.cpp b/src/libcamera/software_isp/debayer_cpu.cpp
> index 22ae99cd..215ca615 100644
> --- a/src/libcamera/software_isp/debayer_cpu.cpp
> +++ b/src/libcamera/software_isp/debayer_cpu.cpp
> @@ -261,7 +261,7 @@ int DebayerCpu::getInputConfig(PixelFormat inputFormat, DebayerInputConfig &conf
>  		config.bpp = (bayerFormat.bitDepth + 7) & ~7;
>  		config.patternSize.width = 2;
>  		config.patternSize.height = 2;
> -		config.outputFormats = std::vector<PixelFormat>({ formats::RGB888 });
> +		config.outputFormats = std::vector<PixelFormat>({ formats::RGB888, formats::BGR888 });
>  		return 0;
>  	}
>  
> @@ -271,7 +271,7 @@ int DebayerCpu::getInputConfig(PixelFormat inputFormat, DebayerInputConfig &conf
>  		config.bpp = 10;
>  		config.patternSize.width = 4; /* 5 bytes per *4* pixels */
>  		config.patternSize.height = 2;
> -		config.outputFormats = std::vector<PixelFormat>({ formats::RGB888 });
> +		config.outputFormats = std::vector<PixelFormat>({ formats::RGB888, formats::BGR888 });
>  		return 0;
>  	}
>  
> @@ -282,7 +282,7 @@ int DebayerCpu::getInputConfig(PixelFormat inputFormat, DebayerInputConfig &conf
>  
>  int DebayerCpu::getOutputConfig(PixelFormat outputFormat, DebayerOutputConfig &config)
>  {
> -	if (outputFormat == formats::RGB888) {
> +	if (outputFormat == formats::RGB888 || outputFormat == formats::BGR888) {
>  		config.bpp = 24;
>  		return 0;
>  	}
> @@ -318,13 +318,41 @@ int DebayerCpu::setupStandardBayerOrder(BayerFormat::Order order)
>  	return 0;
>  }
>  
> -/* TODO: this ignores outputFormat since there is only 1 supported outputFormat for now */
> -int DebayerCpu::setDebayerFunctions(PixelFormat inputFormat, [[maybe_unused]] PixelFormat outputFormat)
> +int DebayerCpu::setDebayerFunctions(PixelFormat inputFormat, PixelFormat outputFormat)
>  {
>  	BayerFormat bayerFormat =
>  		BayerFormat::fromPixelFormat(inputFormat);
>  
>  	xShift_ = 0;
> +	swapRedBlueGains_ = false;
> +
> +	switch (outputFormat) {
> +	case formats::RGB888:
> +		break;
> +	case formats::BGR888:
> +		/* Swap R and B in bayer order to generate BGR888 instead of RGB888 */
> +		swapRedBlueGains_ = true;
> +
> +		switch (bayerFormat.order) {
> +		case BayerFormat::BGGR:
> +			bayerFormat.order = BayerFormat::RGGB;
> +			break;
> +		case BayerFormat::GBRG:
> +			bayerFormat.order = BayerFormat::GRBG;
> +			break;
> +		case BayerFormat::GRBG:
> +			bayerFormat.order = BayerFormat::GBRG;
> +			break;
> +		case BayerFormat::RGGB:
> +			bayerFormat.order = BayerFormat::BGGR;
> +			break;
> +		default:
> +			goto invalid_fmt;
> +		}
> +		break;
> +	default:
> +		goto invalid_fmt;
> +	}
>  
>  	if ((bayerFormat.bitDepth == 8 || bayerFormat.bitDepth == 10 || bayerFormat.bitDepth == 12) &&
>  	    bayerFormat.packing == BayerFormat::Packing::None &&
> @@ -371,6 +399,7 @@ int DebayerCpu::setDebayerFunctions(PixelFormat inputFormat, [[maybe_unused]] Pi
>  		}
>  	}
>  
> +invalid_fmt:
>  	LOG(Debayer, Error) << "Unsupported input output format combination";
>  	return -EINVAL;
>  }
> @@ -654,6 +683,9 @@ void DebayerCpu::process(FrameBuffer *input, FrameBuffer *output, DebayerParams
>  		gamma_correction_ = params.gamma;
>  	}
>  
> +	if (swapRedBlueGains_)
> +		std::swap(params.gainR, params.gainB);
> +
>  	for (unsigned int i = 0; i < kRGBLookupSize; i++) {
>  		constexpr unsigned int div =
>  			kRGBLookupSize * DebayerParams::kGain10 / kGammaLookupSize;
> diff --git a/src/libcamera/software_isp/debayer_cpu.h b/src/libcamera/software_isp/debayer_cpu.h
> index 58c842ce..33e8b4b8 100644
> --- a/src/libcamera/software_isp/debayer_cpu.h
> +++ b/src/libcamera/software_isp/debayer_cpu.h
> @@ -145,6 +145,7 @@ private:
>  	unsigned int lineBufferIndex_;
>  	unsigned int xShift_; /* Offset of 0/1 applied to window_.x */
>  	bool enableInputMemcpy_;
> +	bool swapRedBlueGains_;
>  	float gamma_correction_;
>  	unsigned int measuredFrames_;
>  	int64_t frameProcessTime_;