[v3,00/11] software-isp: Align crops, statistics and metadata

28472 mbox series

Hi all,

This series centers the EGL software ISP output, aligns statistics with
the visible sensor region, and reports the configured crop to applications.

The GPU projection currently places the cropped image off-center. CPU
statistics use an output-sized window at the sensor buffer's origin, so
metering can cover a different region from the one displayed. The first
four patches correct the projection and statistics window and add geometry
unit tests.

Following Kieran's request, the remaining patches report the image region
selected by CPU or EGL debayering in native sensor coordinates.
ScalerCropMaximum describes the configured region, and completed requests
include the same rectangle as ScalerCrop. The reporting builds on Kieran's
earlier patches.

Metadata reporting covers capture paths that preserve the sensor output.
Intermediate cropping or scaling requires an additional coordinate mapping.

Changes in v3:
- Add crop reporting for both software ISP backends, including CPU Bayer
  phase offsets and conversion to native sensor coordinates.
- Add unit tests for CPU crop configuration and sensor-coordinate mapping.
- Report ScalerCropMaximum and completed-request ScalerCrop in the simple
  pipeline.
- Clarify the statistics commit's opening.

Thanks,
Ray

Ray Strode (11):
  software-isp: Center the GPU image crop
  tests: Validate debayer projection geometry
  software-isp: Align statistics with the visible sensor crop
  tests: Validate the visible sensor crop
  software-isp: Expose the configured image crop
  tests: Check the configured CPU image crop
  software-isp: Map input crops to native sensor coordinates
  tests: Validate native sensor crop coordinates
  v4l2: Allow probing optional selection rectangles
  pipeline: simple: Report the software ISP's maximum crop
  pipeline: simple: Include the software ISP crop in request metadata

 .../internal/software_isp/debayer_geometry.h  |  78 ++++++++
 .../internal/software_isp/meson.build         |   1 +
 .../internal/software_isp/software_isp.h      |   1 +
 include/libcamera/internal/v4l2_subdevice.h   |   7 +-
 src/libcamera/pipeline/simple/simple.cpp      |  59 +++++-
 src/libcamera/software_isp/debayer.cpp        |  11 ++
 src/libcamera/software_isp/debayer.h          |   1 +
 src/libcamera/software_isp/debayer_cpu.h      |   4 +
 src/libcamera/software_isp/debayer_egl.cpp    |  35 +---
 src/libcamera/software_isp/debayer_egl.h      |   3 +-
 src/libcamera/software_isp/software_isp.cpp   |  14 ++
 src/libcamera/v4l2_subdevice.cpp              |  12 +-
 test/debayer-geometry.cpp                     | 170 ++++++++++++++++++
 test/meson.build                              |   2 +
 test/software_isp/debayer-crop.cpp            | 102 +++++++++++
 test/software_isp/meson.build                 |  20 +++
 16 files changed, 487 insertions(+), 33 deletions(-)
 create mode 100644 include/libcamera/internal/software_isp/debayer_geometry.h
 create mode 100644 test/debayer-geometry.cpp
 create mode 100644 test/software_isp/debayer-crop.cpp
 create mode 100644 test/software_isp/meson.build

Range-diff against v2:
 1:  adb62393 !  1:  21af6599 software-isp: Center the GPU image crop
    @@ include/libcamera/internal/software_isp/debayer_geometry.h (new)
     + * the input. All sizes must be nonzero, with nativeSize contained in inputSize.
     + */
     +inline std::array<float, 16> debayerProjection(const Size &inputSize,
    -+					     const Size &nativeSize,
    -+					     const Size &outputSize)
    ++					       const Size &nativeSize,
    ++					       const Size &outputSize)
     +{
     +	float scale = std::max(static_cast<float>(outputSize.width) / nativeSize.width,
     +			       static_cast<float>(outputSize.height) / nativeSize.height);
    @@ include/libcamera/internal/software_isp/debayer_geometry.h (new)
     +	float transX = static_cast<float>(outputSize.width) / inputSize.width - 1.0f;
     +	float transY = static_cast<float>(outputSize.height) / inputSize.height - 1.0f;
     +
    ++	/* clang-format off */
     +	return {
     +		scale, 0, 0, 0,
     +		0, scale, 0, 0,
     +		0, 0, 1, 0,
     +		transX, transY, 0, 1,
     +	};
    ++	/* clang-format on */
     +}
     +
     +} /* namespace libcamera */
 2:  d9b43b04 !  2:  fe4f2eb7 tests: Validate debayer projection geometry
    @@ test/debayer-geometry.cpp (new)
     +				{ { left, top }, { 0, 0 } },
     +				{ { right, top }, { static_cast<int>(entry.output.width), 0 } },
     +				{ { left, bottom }, { 0, static_cast<int>(entry.output.height) } },
    -+				{ { right, bottom }, { static_cast<int>(entry.output.width),
    -+						      static_cast<int>(entry.output.height) } },
    ++				{ { right, bottom }, { static_cast<int>(entry.output.width), static_cast<int>(entry.output.height) } },
     +				{ Rectangle(entry.input).center(), Rectangle(entry.output).center() },
     +			};
     +
    @@ test/debayer-geometry.cpp (new)
     +
     +		return TestPass;
     +	}
    -+
     +};
     +
     +TEST_REGISTER(DebayerGeometryTest)
 -:  -------- >  3:  ccab8fc9 software-isp: Align statistics with the visible sensor crop
 -:  -------- >  4:  63e0f994 tests: Validate the visible sensor crop
 3:  22025624 !  5:  1a620d5d software-isp: Meter the visible sensor crop
    @@ Metadata
     Author: Ray Strode <rstrode@redhat.com>
     
      ## Commit message ##
    -    software-isp: Meter the visible sensor crop
    +    software-isp: Expose the configured image crop
     
    -    The GPU debayer path draws a centered sensor crop into the requested output
    -    size, while CPU statistics read the original input buffer.
    +    This commit makes the image region selected by the debayer available to the
    +    pipeline handler, so ScalerCrop metadata can describe the processed output.
     
    -    The statistics window still follows the output dimensions, so metering
    -    covers a different part of the sensor when output sizes or aspect ratios
    -    change.
    +    The CPU and EGL backends crop input differently. CPU debayering keeps an
    +    output-sized window whose origin depends on the Bayer pattern and output
    +    channel order, while EGL scales a centered crop to fill the output. The
    +    pipeline currently cannot retrieve the region chosen by either backend.
     
    -    This commit derives the visible crop in sensor coordinates and centers the
    -    statistics window on that region.
    +    Each backend returns its actual configured image crop in input-buffer
    +    coordinates. The EGL crop is retained before statistics alignment, so the
    +    returned rectangle describes the image rather than its metering samples.
     
         Signed-off-by: Ray Strode <rstrode@redhat.com>
     
    - ## include/libcamera/internal/software_isp/debayer_geometry.h ##
    -@@ include/libcamera/internal/software_isp/debayer_geometry.h: inline std::array<float, 16> debayerProjection(const Size &inputSize,
    - 	};
    + ## include/libcamera/internal/software_isp/software_isp.h ##
    +@@ include/libcamera/internal/software_isp/software_isp.h: public:
    + 		      const std::vector<std::reference_wrapper<const StreamConfiguration>> &outputCfgs,
    + 		      const ipa::softisp::IPAConfigInfo &configInfo,
    + 		      ControlInfoMap *ipaControls);
    ++	Rectangle inputCrop() const;
    + 
    + 	int exportBuffers(const Stream *stream, unsigned int count,
    + 			  std::vector<std::unique_ptr<FrameBuffer>> *buffers);
    +
    + ## src/libcamera/software_isp/debayer.cpp ##
    +@@ src/libcamera/software_isp/debayer.cpp: Debayer::~Debayer()
    + {
      }
      
    -+/* Return the visible sensor rectangle, before statistics Bayer alignment. */
    -+inline Rectangle debayerCrop(const Size &inputSize, const Size &nativeSize,
    -+			     const Size &outputSize)
    -+{
    -+	const Size visibleSize = nativeSize.boundedToAspectRatio(outputSize);
    -+	return visibleSize.centeredTo(Rectangle(inputSize).center());
    -+}
    ++/**
    ++ * \fn Rectangle Debayer::inputCrop() const
    ++ * \brief Retrieve the input region visible in the configured output
    ++ *
    ++ * The rectangle is in input-buffer coordinates and excludes any additional
    ++ * alignment performed for statistics sampling. It is valid after a successful
    ++ * configure() call.
    ++ *
    ++ * \return The configured image crop
    ++ */
     +
    - } /* namespace libcamera */
    + /**
    +  * \fn int Debayer::configure()
    +  * \brief Configure the debayer object according to the passed in parameters
    +
    + ## src/libcamera/software_isp/debayer.h ##
    +@@ src/libcamera/software_isp/debayer.h: public:
    + 	virtual void stop() {}
    + 
    + 	virtual SizeRange sizes(PixelFormat inputFormat, const Size &inputSize) = 0;
    ++	virtual Rectangle inputCrop() const = 0;
    + 
    + 	virtual const SharedFD &getStatsFD() = 0;
    + 
    +
    + ## src/libcamera/software_isp/debayer_cpu.h ##
    +@@ src/libcamera/software_isp/debayer_cpu.h: public:
    + 	int start() override;
    + 	void stop() override;
    + 	SizeRange sizes(PixelFormat inputFormat, const Size &inputSize) override;
    ++	Rectangle inputCrop() const override
    ++	{
    ++		return window_.translatedBy({ static_cast<int>(xShift_), 0 });
    ++	}
    + 	const SharedFD &getStatsFD() override { return stats_->getStatsFD(); }
    + 
    + private:
     
      ## src/libcamera/software_isp/debayer_egl.cpp ##
     @@ src/libcamera/software_isp/debayer_egl.cpp: int DebayerEGL::configure(const StreamConfiguration &inputCfg,
    - 	outputSize_ = outputCfg.size;
    - 	nativeOutputSize_ = outSizeRange.max;
    - 
    --	window_.x = ((inputCfg.size.width - outputCfg.size.width) / 2) &
    --		    ~(inputConfig_.patternSize.width - 1);
    --	window_.y = ((inputCfg.size.height - outputCfg.size.height) / 2) &
    --		    ~(inputConfig_.patternSize.height - 1);
    --	window_.width = outputCfg.size.width;
    --	window_.height = outputCfg.size.height;
    --
    - 	/*
    --	 * Don't pass x,y from window_ since process() already adjusts for it.
    --	 * But crop the window to 2/3 of its width and height for speedup.
    -+	 * The GPU scales the visible sensor crop to outputSize_. Statistics
    -+	 * consume the original input buffer, so use sensor coordinates here.
    + 	 * The GPU scales the visible sensor crop to outputSize_. Statistics
    + 	 * consume the original input buffer, so use sensor coordinates here.
      	 */
    --	stats_->setWindow(Rectangle(window_.size()));
    -+	stats_->setWindow(debayerCrop(inputCfg.size, nativeOutputSize_, outputSize_));
    +-	stats_->setWindow(debayerCrop(inputCfg.size, nativeOutputSize_, outputSize_));
    ++	inputCrop_ = debayerCrop(inputCfg.size, nativeOutputSize_, outputSize_);
    ++	stats_->setWindow(inputCrop_);
      
      	inputBufferCount_ = inputCfg.bufferCount;
      	outputBufferCount_ = outputCfg.bufferCount;
     
      ## src/libcamera/software_isp/debayer_egl.h ##
    +@@ src/libcamera/software_isp/debayer_egl.h: public:
    + 	const SharedFD &getStatsFD() override { return stats_->getStatsFD(); }
    + 
    + 	SizeRange sizes(PixelFormat inputFormat, const Size &inputSize) override;
    ++	Rectangle inputCrop() const override { return inputCrop_; }
    + 
    + private:
    + 	static int getInputConfig(PixelFormat inputFormat, DebayerInputConfig &config);
     @@ src/libcamera/software_isp/debayer_egl.h: private:
      	GLint contrastExpUniformDataIn_;
      
      	Size nativeOutputSize_;
    --	Rectangle window_;
    ++	Rectangle inputCrop_;
      	std::unique_ptr<SwStatsCpu> stats_;
      	eGL egl_;
      	uint32_t width_;
    +
    + ## src/libcamera/software_isp/software_isp.cpp ##
    +@@ src/libcamera/software_isp/software_isp.cpp: int SoftwareIsp::configure(const StreamConfiguration &inputCfg,
    + 	return 0;
    + }
    + 
    ++/**
    ++ * \brief Retrieve the input region used to produce the configured output
    ++ *
    ++ * The rectangle is expressed in input-buffer pixels, before any Bayer
    ++ * alignment applied by the statistics engine. It is valid after configure()
    ++ * succeeds and describes the selected CPU or EGL debayer backend.
    ++ *
    ++ * \return The configured input crop
    ++ */
    ++Rectangle SoftwareIsp::inputCrop() const
    ++{
    ++	return debayer_->inputCrop();
    ++}
    ++
    + /**
    +  * \brief Export the buffers from the Software ISP
    +  * \param[in] stream Output stream exporting the buffers
 4:  a070f61a !  6:  7068afcf tests: Validate the visible sensor crop
    @@ Metadata
     Author: Ray Strode <rstrode@redhat.com>
     
      ## Commit message ##
    -    tests: Validate the visible sensor crop
    +    tests: Check the configured CPU image crop
     
    -    The GPU debayer path selects its statistics window from the visible sensor
    -    crop. The geometry unit test checks the projection used for rendering.
    +    This commit checks the crop returned by CPU debayering for supported Bayer
    +    input formats and both RGB channel orders.
     
    -    The project lacks a direct check that the sensor crop remains centered and
    -    follows the output aspect ratio without shrinking to the output resolution.
    +    Unpacked kernels may shift the first rendered pixel by one column depending
    +    on the Bayer pattern and output channel order. The reported image region
    +    must include that shift, while the specialized packed kernels retain their
    +    unshifted origin.
     
    -    This commit extends the geometry unit test with expected sensor rectangles
    -    for native, downscaled, wider, narrower, and portrait outputs. It also
    -    checks integer rounding that leaves an odd top coordinate before Bayer
    -    alignment.
    +    The configuration-only cases cover all four standard Bayer orders at 8, 10
    +    and 12 bits, including packed RAW10 and RAW12. Reusing the debayer across
    +    configurations also checks that changing formats clears a previous phase
    +    shift.
     
         Signed-off-by: Ray Strode <rstrode@redhat.com>
     
    - ## test/debayer-geometry.cpp ##
    + ## test/meson.build ##
    +@@ test/meson.build: subdir('media_device')
    + subdir('process')
    + subdir('py')
    + subdir('serialization')
    ++subdir('software_isp')
    + subdir('stream')
    + subdir('v4l2_compat')
    + subdir('v4l2_subdevice')
    +
    + ## test/software_isp/debayer-crop.cpp (new) ##
     @@
    - /*
    -  * Copyright (C) 2026, Red Hat Inc.
    -  *
    -- * Debayer projection tests
    -+ * Debayer projection and sensor crop tests
    -  */
    - 
    - #include <array>
    -@@ test/debayer-geometry.cpp: class DebayerGeometryTest : public Test
    - protected:
    - 	int run() override
    - 	{
    --		if (testProjection())
    -+		if (testProjection() || testCrop())
    - 			return TestFail;
    - 
    - 		return TestPass;
    -@@ test/debayer-geometry.cpp: private:
    - 		return TestPass;
    - 	}
    - 
    -+	int testCrop()
    ++/* SPDX-License-Identifier: GPL-2.0-or-later */
    ++/*
    ++ * Copyright (C) 2026, Red Hat Inc.
    ++ *
    ++ * CPU debayer crop configuration tests
    ++ */
    ++
    ++#include <functional>
    ++#include <iostream>
    ++#include <memory>
    ++#include <tuple>
    ++#include <utility>
    ++#include <vector>
    ++
    ++#include <libcamera/camera_manager.h>
    ++#include <libcamera/formats.h>
    ++#include <libcamera/stream.h>
    ++
    ++#include "libcamera/internal/formats.h"
    ++#include "libcamera/internal/software_isp/swstats_cpu.h"
    ++
    ++#include "debayer_cpu.h"
    ++#include "test.h"
    ++
    ++using namespace libcamera;
    ++
    ++class DebayerCropTest : public Test
    ++{
    ++protected:
    ++	int run() override
     +	{
    ++		CameraManager manager;
    ++		auto stats = std::make_unique<SwStatsCpu>(manager);
    ++		if (!stats->isValid())
    ++			return TestFail;
    ++
    ++		DebayerCpu debayer(std::move(stats), manager);
     +		static const struct {
    -+			Size input;
    -+			Size native;
    -+			Size output;
    -+			Rectangle expected;
    ++			PixelFormat input;
    ++			int rgbShift;
    ++			int bgrShift;
     +		} cases[] = {
    -+			{ { 1920, 1080 }, { 1920, 1080 }, { 1920, 1080 }, { 0, 0, 1920, 1080 } },
    -+			{ { 1928, 1092 }, { 1920, 1092 }, { 1920, 1092 }, { 4, 0, 1920, 1092 } },
    -+			/* Output resolution changes must not shrink the sensor window. */
    -+			{ { 1928, 1092 }, { 1920, 1092 }, { 1920, 1080 }, { 4, 6, 1920, 1080 } },
    -+			{ { 1928, 1092 }, { 1920, 1092 }, { 1280, 720 }, { 4, 6, 1920, 1080 } },
    -+			{ { 1928, 1092 }, { 1920, 1092 }, { 640, 480 }, { 236, 0, 1456, 1092 } },
    -+			{ { 1928, 1092 }, { 1920, 1092 }, { 512, 768 }, { 600, 0, 728, 1092 } },
    -+			{ { 1928, 1092 }, { 1920, 1092 }, { 1280, 640 }, { 4, 66, 1920, 960 } },
    -+			/* A smaller Bayer border leaves an odd top coordinate. */
    -+			{ { 1928, 1092 }, { 1924, 1092 }, { 1280, 720 }, { 2, 5, 1924, 1082 } },
    ++			{ formats::SBGGR8, 0, 1 },
    ++			{ formats::SGBRG8, 1, 0 },
    ++			{ formats::SGRBG8, 0, 1 },
    ++			{ formats::SRGGB8, 1, 0 },
    ++			{ formats::SBGGR10, 0, 1 },
    ++			{ formats::SGBRG10, 1, 0 },
    ++			{ formats::SGRBG10, 0, 1 },
    ++			{ formats::SRGGB10, 1, 0 },
    ++			{ formats::SBGGR12, 0, 1 },
    ++			{ formats::SGBRG12, 1, 0 },
    ++			{ formats::SGRBG12, 0, 1 },
    ++			{ formats::SRGGB12, 1, 0 },
    ++			/* Packed kernels use each Bayer order without a phase shift. */
    ++			{ formats::SBGGR10_CSI2P, 0, 0 },
    ++			{ formats::SGBRG10_CSI2P, 0, 0 },
    ++			{ formats::SGRBG10_CSI2P, 0, 0 },
    ++			{ formats::SRGGB10_CSI2P, 0, 0 },
    ++			{ formats::SBGGR12_CSI2P, 0, 0 },
    ++			{ formats::SGBRG12_CSI2P, 0, 0 },
    ++			{ formats::SGRBG12_CSI2P, 0, 0 },
    ++			{ formats::SRGGB12_CSI2P, 0, 0 },
     +		};
     +
     +		for (const auto &entry : cases) {
    -+			Rectangle crop = debayerCrop(entry.input, entry.native, entry.output);
    -+			if (crop != entry.expected) {
    -+				std::cerr << "Crop " << entry.input << " -> " << entry.output
    -+					  << " is " << crop << ", expected " << entry.expected << std::endl;
    -+				return TestFail;
    ++			StreamConfiguration input;
    ++			input.pixelFormat = entry.input;
    ++			input.size = { 1928, 1092 };
    ++			input.stride = PixelFormatInfo::info(entry.input).stride(input.size.width, 0);
    ++
    ++			for (PixelFormat format : { formats::RGB888, formats::BGR888,
    ++						    formats::XRGB8888, formats::XBGR8888 }) {
    ++				StreamConfiguration output;
    ++				output.pixelFormat = format;
    ++				output.size = { 1280, 720 };
    ++				std::tie(output.stride, output.frameSize) =
    ++					debayer.strideAndFrameSize(format, output.size);
    ++				const std::vector<std::reference_wrapper<const StreamConfiguration>> outputs{ output };
    ++				if (debayer.configure(input, outputs, false)) {
    ++					std::cerr << "Unable to configure " << entry.input << " -> " << format
    ++						  << std::endl;
    ++					return TestFail;
    ++				}
    ++
    ++				int shift = (format == formats::RGB888 || format == formats::XRGB8888)
    ++						    ? entry.rgbShift
    ++						    : entry.bgrShift;
    ++				Rectangle expected(324 + shift, 186, 1280, 720);
    ++				if (debayer.inputCrop() != expected) {
    ++					std::cerr << "Crop for " << entry.input << " -> " << format << " is "
    ++						  << debayer.inputCrop() << ", expected " << expected << std::endl;
    ++					return TestFail;
    ++				}
     +			}
     +		}
     +
     +		return TestPass;
     +	}
    - };
    - 
    - TEST_REGISTER(DebayerGeometryTest)
    ++};
    ++
    ++TEST_REGISTER(DebayerCropTest)
    +
    + ## test/software_isp/meson.build (new) ##
    +@@
    ++# SPDX-License-Identifier: CC0-1.0
    ++
    ++if not softisp_enabled
    ++    subdir_done()
    ++endif
    ++
    ++software_isp_tests = [
    ++    {'name': 'debayer-crop', 'sources': ['debayer-crop.cpp']},
    ++]
    ++
    ++foreach test : software_isp_tests
    ++    exe = executable(test['name'], test['sources'],
    ++                     dependencies : libcamera_private,
    ++                     implicit_include_directories : false,
    ++                     link_with : test_libraries,
    ++                     include_directories : [test_includes_internal,
    ++                                            '../../src/libcamera/software_isp/'])
    ++
    ++    test(test['name'], exe, suite : 'software_isp')
    ++endforeach
 -:  -------- >  7:  4ae3bd7c software-isp: Map input crops to native sensor coordinates
 -:  -------- >  8:  285bae8c tests: Validate native sensor crop coordinates
 -:  -------- >  9:  fb0df758 v4l2: Allow probing optional selection rectangles
 -:  -------- > 10:  3578634e pipeline: simple: Report the software ISP's maximum crop
 -:  -------- > 11:  37078b31 pipeline: simple: Include the software ISP crop in request metadata

base-commit: a92285e9c8f8d5aba24c6f72f49efc73074002e1