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

19496 diff mbox series

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.

Tested-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> # sc8280xp Lenovo x13s
Tested-by: Pavel Machek <pavel@ucw.cz>
Signed-off-by: Hans de Goede <hdegoede@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(-)

Hans de Goede <hdegoede@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@linaro.org> # sc8280xp Lenovo x13s
> Tested-by: Pavel Machek <pavel@ucw.cz>
> Signed-off-by: Hans de Goede <hdegoede@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_;