[PATCH/RFC,18/32] libcamera: camera_sensor: Expose the Bayer order
diff mbox series

Message ID 20240301212121.9072-19-laurent.pinchart@ideasonboard.com
State RFC
Headers show
Series
  • libcamera: Support the upstream Unicam driver
Related show

Commit Message

Laurent Pinchart March 1, 2024, 9:21 p.m. UTC
Pipeline handlers may need to know the Bayer order produced by the
sensor when a Transform is applied (horizontal or vertical flip). This
is currently implemented manually in the Raspberry Pi pipeline handler.
Move the implementation to the CameraSensor class to make it usable in
other pipeline handlers.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 include/libcamera/internal/camera_sensor.h    |  4 +-
 .../pipeline/rpi/common/pipeline_base.cpp     | 54 +++----------------
 .../pipeline/rpi/common/pipeline_base.h       |  6 +--
 src/libcamera/sensor/camera_sensor.cpp        | 37 ++++++++++++-
 4 files changed, 45 insertions(+), 56 deletions(-)

Comments

Jacopo Mondi March 4, 2024, 5:50 p.m. UTC | #1
And this can be fast-tracked as well imho

Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>

On Fri, Mar 01, 2024 at 11:21:07PM +0200, Laurent Pinchart wrote:
> Pipeline handlers may need to know the Bayer order produced by the
> sensor when a Transform is applied (horizontal or vertical flip). This
> is currently implemented manually in the Raspberry Pi pipeline handler.
> Move the implementation to the CameraSensor class to make it usable in
> other pipeline handlers.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  include/libcamera/internal/camera_sensor.h    |  4 +-
>  .../pipeline/rpi/common/pipeline_base.cpp     | 54 +++----------------
>  .../pipeline/rpi/common/pipeline_base.h       |  6 +--
>  src/libcamera/sensor/camera_sensor.cpp        | 37 ++++++++++++-
>  4 files changed, 45 insertions(+), 56 deletions(-)
>
> diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h
> index 750d6d729cac..d05f48ebeebe 100644
> --- a/include/libcamera/internal/camera_sensor.h
> +++ b/include/libcamera/internal/camera_sensor.h
> @@ -22,12 +22,12 @@
>
>  #include <libcamera/ipa/core_ipa_interface.h>
>
> +#include "libcamera/internal/bayer_format.h"
>  #include "libcamera/internal/formats.h"
>  #include "libcamera/internal/v4l2_subdevice.h"
>
>  namespace libcamera {
>
> -class BayerFormat;
>  class CameraLens;
>  class MediaEntity;
>  class SensorConfiguration;
> @@ -69,6 +69,7 @@ public:
>  	const ControlList &properties() const { return properties_; }
>  	int sensorInfo(IPACameraSensorInfo *info) const;
>  	Transform computeTransform(Orientation *orientation) const;
> +	BayerFormat::Order bayerOrder(Transform t) const;
>
>  	const ControlInfoMap &controls() const;
>  	ControlList getControls(const std::vector<uint32_t> &ids);
> @@ -114,6 +115,7 @@ private:
>  	Rectangle activeArea_;
>  	const BayerFormat *bayerFormat_;
>  	bool supportFlips_;
> +	bool flipsAlterBayerOrder_;
>  	Orientation mountingOrientation_;
>
>  	ControlList properties_;
> diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
> index 9449c3dc458c..7e420b3f90a4 100644
> --- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
> +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
> @@ -235,24 +235,16 @@ CameraConfiguration::Status RPiCameraConfiguration::validate()
>  	for (auto &raw : rawStreams_) {
>  		StreamConfiguration *rawStream = raw.cfg;
>
> -		/* Adjust the RAW stream to match the computed sensor format. */
> -		BayerFormat sensorBayer = BayerFormat::fromMbusCode(sensorFormat_.code);
> -
>  		/*
> -		 * Some sensors change their Bayer order when they are h-flipped
> -		 * or v-flipped, according to the transform. If this one does, we
> -		 * must advertise the transformed Bayer order in the raw stream.
> -		 * Note how we must fetch the "native" (i.e. untransformed) Bayer
> -		 * order, because the sensor may currently be flipped!
> +		 * Some sensors change their Bayer order when they are
> +		 * h-flipped or v-flipped, according to the transform. Adjust
> +		 * the RAW stream to match the computed sensor format by
> +		 * applying the sensor Bayer order resulting from the transform
> +		 * to the user request.
>  		 */
> -		if (data_->flipsAlterBayerOrder_) {
> -			sensorBayer.order = data_->nativeBayerOrder_;
> -			sensorBayer = sensorBayer.transform(combinedTransform_);
> -		}
>
> -		/* Apply the sensor adjusted Bayer order to the user request. */
>  		BayerFormat cfgBayer = BayerFormat::fromPixelFormat(rawStream->pixelFormat);
> -		cfgBayer.order = sensorBayer.order;
> +		cfgBayer.order = data_->sensor_->bayerOrder(combinedTransform_);
>
>  		if (rawStream->pixelFormat != cfgBayer.toPixelFormat()) {
>  			rawStream->pixelFormat = cfgBayer.toPixelFormat();
> @@ -840,40 +832,6 @@ int PipelineHandlerBase::registerCamera(std::unique_ptr<RPi::CameraData> &camera
>  	 */
>  	data->properties_.set(properties::ScalerCropMaximum, Rectangle{});
>
> -	/*
> -	 * We cache two things about the sensor in relation to transforms
> -	 * (meaning horizontal and vertical flips): if they affect the Bayer
> -	 * ordering, and what the "native" Bayer order is, when no transforms
> -	 * are applied.
> -	 *
> -	 * If flips are supported verify if they affect the Bayer ordering
> -	 * and what the "native" Bayer order is, when no transforms are
> -	 * applied.
> -	 *
> -	 * We note that the sensor's cached list of supported formats is
> -	 * already in the "native" order, with any flips having been undone.
> -	 */
> -	const V4L2Subdevice *sensor = data->sensor_->device();
> -	const struct v4l2_query_ext_ctrl *hflipCtrl = sensor->controlInfo(V4L2_CID_HFLIP);
> -	if (hflipCtrl) {
> -		/* We assume it will support vflips too... */
> -		data->flipsAlterBayerOrder_ = hflipCtrl->flags & V4L2_CTRL_FLAG_MODIFY_LAYOUT;
> -	}
> -
> -	/* Look for a valid Bayer format. */
> -	BayerFormat bayerFormat;
> -	for (const auto &iter : data->sensorFormats_) {
> -		bayerFormat = BayerFormat::fromMbusCode(iter.first);
> -		if (bayerFormat.isValid())
> -			break;
> -	}
> -
> -	if (!bayerFormat.isValid()) {
> -		LOG(RPI, Error) << "No Bayer format found";
> -		return -EINVAL;
> -	}
> -	data->nativeBayerOrder_ = bayerFormat.order;
> -
>  	ret = platformRegister(cameraData, frontend, backend);
>  	if (ret)
>  		return ret;
> diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.h b/src/libcamera/pipeline/rpi/common/pipeline_base.h
> index 267eef1102f1..0608bbe5f0c7 100644
> --- a/src/libcamera/pipeline/rpi/common/pipeline_base.h
> +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.h
> @@ -48,7 +48,7 @@ class CameraData : public Camera::Private
>  public:
>  	CameraData(PipelineHandler *pipe)
>  		: Camera::Private(pipe), state_(State::Stopped),
> -		  flipsAlterBayerOrder_(false), dropFrameCount_(0), buffersAllocated_(false),
> +		  dropFrameCount_(0), buffersAllocated_(false),
>  		  ispOutputCount_(0), ispOutputTotal_(0)
>  	{
>  	}
> @@ -131,10 +131,6 @@ public:
>
>  	std::queue<Request *> requestQueue_;
>
> -	/* Store the "native" Bayer order (that is, with no transforms applied). */
> -	bool flipsAlterBayerOrder_;
> -	BayerFormat::Order nativeBayerOrder_;
> -
>  	/* For handling digital zoom. */
>  	IPACameraSensorInfo sensorInfo_;
>  	Rectangle ispCrop_; /* crop in ISP (camera mode) pixels */
> diff --git a/src/libcamera/sensor/camera_sensor.cpp b/src/libcamera/sensor/camera_sensor.cpp
> index 402025566544..5c4f35324055 100644
> --- a/src/libcamera/sensor/camera_sensor.cpp
> +++ b/src/libcamera/sensor/camera_sensor.cpp
> @@ -58,7 +58,7 @@ LOG_DEFINE_CATEGORY(CameraSensor)
>  CameraSensor::CameraSensor(const MediaEntity *entity)
>  	: entity_(entity), pad_(UINT_MAX), staticProps_(nullptr),
>  	  bayerFormat_(nullptr), supportFlips_(false),
> -	  properties_(properties::properties)
> +	  flipsAlterBayerOrder_(false), properties_(properties::properties)
>  {
>  }
>
> @@ -271,9 +271,14 @@ int CameraSensor::validateSensorDriver()
>  	const struct v4l2_query_ext_ctrl *hflipInfo = subdev_->controlInfo(V4L2_CID_HFLIP);
>  	const struct v4l2_query_ext_ctrl *vflipInfo = subdev_->controlInfo(V4L2_CID_VFLIP);
>  	if (hflipInfo && !(hflipInfo->flags & V4L2_CTRL_FLAG_READ_ONLY) &&
> -	    vflipInfo && !(vflipInfo->flags & V4L2_CTRL_FLAG_READ_ONLY))
> +	    vflipInfo && !(vflipInfo->flags & V4L2_CTRL_FLAG_READ_ONLY)) {
>  		supportFlips_ = true;
>
> +		if (hflipInfo->flags & V4L2_CTRL_FLAG_MODIFY_LAYOUT ||
> +		    vflipInfo->flags & V4L2_CTRL_FLAG_MODIFY_LAYOUT)
> +			flipsAlterBayerOrder_ = true;
> +	}
> +
>  	if (!supportFlips_)
>  		LOG(CameraSensor, Debug)
>  			<< "Camera sensor does not support horizontal/vertical flip";
> @@ -1041,6 +1046,34 @@ Transform CameraSensor::computeTransform(Orientation *orientation) const
>  	return transform;
>  }
>
> +/**
> + * \brief Compute the Bayer order that results from the given Transform
> + * \param[in] t The Transform to apply to the sensor
> + *
> + * Some sensors change their Bayer order when they are h-flipped or v-flipped.
> + * This function computes and returns the Bayer order that would result from the
> + * given transform applied to the sensor.
> + *
> + * This function is valid only when the sensor produces raw Bayer formats.
> + *
> + * \return The Bayer order produced by the sensor when the Transform is applied
> + */
> +BayerFormat::Order CameraSensor::bayerOrder(Transform t) const
> +{
> +	/* Return a defined by meaningless value for non-Bayer sensors. */
> +	if (!bayerFormat_)
> +		return BayerFormat::Order::BGGR;
> +
> +	if (!flipsAlterBayerOrder_)
> +		return bayerFormat_->order;
> +
> +	/*
> +	 * Apply the transform to the native (i.e. untransformed) Bayer order,
> +	 * using the rest of the Bayer format supplied by the caller.
> +	 */
> +	return bayerFormat_->transform(t).order;
> +}
> +
>  /**
>   * \brief Retrieve the supported V4L2 controls and their information
>   *
> --
> Regards,
>
> Laurent Pinchart
>
Laurent Pinchart March 4, 2024, 10:56 p.m. UTC | #2
On Mon, Mar 04, 2024 at 06:50:02PM +0100, Jacopo Mondi wrote:
> And this can be fast-tracked as well imho

How about "[PATCH/RFC 14/32] libcamera: camera_sensor: Move related
classes to subdirectory" ? Patches 15/32 to 18/32 depend on it. I could
rebase, but...

You have also reviewed the refactoring of V4L2Subdevice (01/32 to
09/32), and acked most of it. I'd like to merge that too if possible.
03/32, 04/32, 07/32 and 09/32 are missing your Rb.

Does anyone else want to review all those patches too ?

> Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> 
> On Fri, Mar 01, 2024 at 11:21:07PM +0200, Laurent Pinchart wrote:
> > Pipeline handlers may need to know the Bayer order produced by the
> > sensor when a Transform is applied (horizontal or vertical flip). This
> > is currently implemented manually in the Raspberry Pi pipeline handler.
> > Move the implementation to the CameraSensor class to make it usable in
> > other pipeline handlers.
> >
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> >  include/libcamera/internal/camera_sensor.h    |  4 +-
> >  .../pipeline/rpi/common/pipeline_base.cpp     | 54 +++----------------
> >  .../pipeline/rpi/common/pipeline_base.h       |  6 +--
> >  src/libcamera/sensor/camera_sensor.cpp        | 37 ++++++++++++-
> >  4 files changed, 45 insertions(+), 56 deletions(-)
> >
> > diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h
> > index 750d6d729cac..d05f48ebeebe 100644
> > --- a/include/libcamera/internal/camera_sensor.h
> > +++ b/include/libcamera/internal/camera_sensor.h
> > @@ -22,12 +22,12 @@
> >
> >  #include <libcamera/ipa/core_ipa_interface.h>
> >
> > +#include "libcamera/internal/bayer_format.h"
> >  #include "libcamera/internal/formats.h"
> >  #include "libcamera/internal/v4l2_subdevice.h"
> >
> >  namespace libcamera {
> >
> > -class BayerFormat;
> >  class CameraLens;
> >  class MediaEntity;
> >  class SensorConfiguration;
> > @@ -69,6 +69,7 @@ public:
> >  	const ControlList &properties() const { return properties_; }
> >  	int sensorInfo(IPACameraSensorInfo *info) const;
> >  	Transform computeTransform(Orientation *orientation) const;
> > +	BayerFormat::Order bayerOrder(Transform t) const;
> >
> >  	const ControlInfoMap &controls() const;
> >  	ControlList getControls(const std::vector<uint32_t> &ids);
> > @@ -114,6 +115,7 @@ private:
> >  	Rectangle activeArea_;
> >  	const BayerFormat *bayerFormat_;
> >  	bool supportFlips_;
> > +	bool flipsAlterBayerOrder_;
> >  	Orientation mountingOrientation_;
> >
> >  	ControlList properties_;
> > diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
> > index 9449c3dc458c..7e420b3f90a4 100644
> > --- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
> > +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
> > @@ -235,24 +235,16 @@ CameraConfiguration::Status RPiCameraConfiguration::validate()
> >  	for (auto &raw : rawStreams_) {
> >  		StreamConfiguration *rawStream = raw.cfg;
> >
> > -		/* Adjust the RAW stream to match the computed sensor format. */
> > -		BayerFormat sensorBayer = BayerFormat::fromMbusCode(sensorFormat_.code);
> > -
> >  		/*
> > -		 * Some sensors change their Bayer order when they are h-flipped
> > -		 * or v-flipped, according to the transform. If this one does, we
> > -		 * must advertise the transformed Bayer order in the raw stream.
> > -		 * Note how we must fetch the "native" (i.e. untransformed) Bayer
> > -		 * order, because the sensor may currently be flipped!
> > +		 * Some sensors change their Bayer order when they are
> > +		 * h-flipped or v-flipped, according to the transform. Adjust
> > +		 * the RAW stream to match the computed sensor format by
> > +		 * applying the sensor Bayer order resulting from the transform
> > +		 * to the user request.
> >  		 */
> > -		if (data_->flipsAlterBayerOrder_) {
> > -			sensorBayer.order = data_->nativeBayerOrder_;
> > -			sensorBayer = sensorBayer.transform(combinedTransform_);
> > -		}
> >
> > -		/* Apply the sensor adjusted Bayer order to the user request. */
> >  		BayerFormat cfgBayer = BayerFormat::fromPixelFormat(rawStream->pixelFormat);
> > -		cfgBayer.order = sensorBayer.order;
> > +		cfgBayer.order = data_->sensor_->bayerOrder(combinedTransform_);
> >
> >  		if (rawStream->pixelFormat != cfgBayer.toPixelFormat()) {
> >  			rawStream->pixelFormat = cfgBayer.toPixelFormat();
> > @@ -840,40 +832,6 @@ int PipelineHandlerBase::registerCamera(std::unique_ptr<RPi::CameraData> &camera
> >  	 */
> >  	data->properties_.set(properties::ScalerCropMaximum, Rectangle{});
> >
> > -	/*
> > -	 * We cache two things about the sensor in relation to transforms
> > -	 * (meaning horizontal and vertical flips): if they affect the Bayer
> > -	 * ordering, and what the "native" Bayer order is, when no transforms
> > -	 * are applied.
> > -	 *
> > -	 * If flips are supported verify if they affect the Bayer ordering
> > -	 * and what the "native" Bayer order is, when no transforms are
> > -	 * applied.
> > -	 *
> > -	 * We note that the sensor's cached list of supported formats is
> > -	 * already in the "native" order, with any flips having been undone.
> > -	 */
> > -	const V4L2Subdevice *sensor = data->sensor_->device();
> > -	const struct v4l2_query_ext_ctrl *hflipCtrl = sensor->controlInfo(V4L2_CID_HFLIP);
> > -	if (hflipCtrl) {
> > -		/* We assume it will support vflips too... */
> > -		data->flipsAlterBayerOrder_ = hflipCtrl->flags & V4L2_CTRL_FLAG_MODIFY_LAYOUT;
> > -	}
> > -
> > -	/* Look for a valid Bayer format. */
> > -	BayerFormat bayerFormat;
> > -	for (const auto &iter : data->sensorFormats_) {
> > -		bayerFormat = BayerFormat::fromMbusCode(iter.first);
> > -		if (bayerFormat.isValid())
> > -			break;
> > -	}
> > -
> > -	if (!bayerFormat.isValid()) {
> > -		LOG(RPI, Error) << "No Bayer format found";
> > -		return -EINVAL;
> > -	}
> > -	data->nativeBayerOrder_ = bayerFormat.order;
> > -
> >  	ret = platformRegister(cameraData, frontend, backend);
> >  	if (ret)
> >  		return ret;
> > diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.h b/src/libcamera/pipeline/rpi/common/pipeline_base.h
> > index 267eef1102f1..0608bbe5f0c7 100644
> > --- a/src/libcamera/pipeline/rpi/common/pipeline_base.h
> > +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.h
> > @@ -48,7 +48,7 @@ class CameraData : public Camera::Private
> >  public:
> >  	CameraData(PipelineHandler *pipe)
> >  		: Camera::Private(pipe), state_(State::Stopped),
> > -		  flipsAlterBayerOrder_(false), dropFrameCount_(0), buffersAllocated_(false),
> > +		  dropFrameCount_(0), buffersAllocated_(false),
> >  		  ispOutputCount_(0), ispOutputTotal_(0)
> >  	{
> >  	}
> > @@ -131,10 +131,6 @@ public:
> >
> >  	std::queue<Request *> requestQueue_;
> >
> > -	/* Store the "native" Bayer order (that is, with no transforms applied). */
> > -	bool flipsAlterBayerOrder_;
> > -	BayerFormat::Order nativeBayerOrder_;
> > -
> >  	/* For handling digital zoom. */
> >  	IPACameraSensorInfo sensorInfo_;
> >  	Rectangle ispCrop_; /* crop in ISP (camera mode) pixels */
> > diff --git a/src/libcamera/sensor/camera_sensor.cpp b/src/libcamera/sensor/camera_sensor.cpp
> > index 402025566544..5c4f35324055 100644
> > --- a/src/libcamera/sensor/camera_sensor.cpp
> > +++ b/src/libcamera/sensor/camera_sensor.cpp
> > @@ -58,7 +58,7 @@ LOG_DEFINE_CATEGORY(CameraSensor)
> >  CameraSensor::CameraSensor(const MediaEntity *entity)
> >  	: entity_(entity), pad_(UINT_MAX), staticProps_(nullptr),
> >  	  bayerFormat_(nullptr), supportFlips_(false),
> > -	  properties_(properties::properties)
> > +	  flipsAlterBayerOrder_(false), properties_(properties::properties)
> >  {
> >  }
> >
> > @@ -271,9 +271,14 @@ int CameraSensor::validateSensorDriver()
> >  	const struct v4l2_query_ext_ctrl *hflipInfo = subdev_->controlInfo(V4L2_CID_HFLIP);
> >  	const struct v4l2_query_ext_ctrl *vflipInfo = subdev_->controlInfo(V4L2_CID_VFLIP);
> >  	if (hflipInfo && !(hflipInfo->flags & V4L2_CTRL_FLAG_READ_ONLY) &&
> > -	    vflipInfo && !(vflipInfo->flags & V4L2_CTRL_FLAG_READ_ONLY))
> > +	    vflipInfo && !(vflipInfo->flags & V4L2_CTRL_FLAG_READ_ONLY)) {
> >  		supportFlips_ = true;
> >
> > +		if (hflipInfo->flags & V4L2_CTRL_FLAG_MODIFY_LAYOUT ||
> > +		    vflipInfo->flags & V4L2_CTRL_FLAG_MODIFY_LAYOUT)
> > +			flipsAlterBayerOrder_ = true;
> > +	}
> > +
> >  	if (!supportFlips_)
> >  		LOG(CameraSensor, Debug)
> >  			<< "Camera sensor does not support horizontal/vertical flip";
> > @@ -1041,6 +1046,34 @@ Transform CameraSensor::computeTransform(Orientation *orientation) const
> >  	return transform;
> >  }
> >
> > +/**
> > + * \brief Compute the Bayer order that results from the given Transform
> > + * \param[in] t The Transform to apply to the sensor
> > + *
> > + * Some sensors change their Bayer order when they are h-flipped or v-flipped.
> > + * This function computes and returns the Bayer order that would result from the
> > + * given transform applied to the sensor.
> > + *
> > + * This function is valid only when the sensor produces raw Bayer formats.
> > + *
> > + * \return The Bayer order produced by the sensor when the Transform is applied
> > + */
> > +BayerFormat::Order CameraSensor::bayerOrder(Transform t) const
> > +{
> > +	/* Return a defined by meaningless value for non-Bayer sensors. */
> > +	if (!bayerFormat_)
> > +		return BayerFormat::Order::BGGR;
> > +
> > +	if (!flipsAlterBayerOrder_)
> > +		return bayerFormat_->order;
> > +
> > +	/*
> > +	 * Apply the transform to the native (i.e. untransformed) Bayer order,
> > +	 * using the rest of the Bayer format supplied by the caller.
> > +	 */
> > +	return bayerFormat_->transform(t).order;
> > +}
> > +
> >  /**
> >   * \brief Retrieve the supported V4L2 controls and their information
> >   *
Daniel Scally March 5, 2024, 1:56 p.m. UTC | #3
Hi Laurent - thanks for the patch; this will handily solve a problem I have

On 01/03/2024 21:21, Laurent Pinchart wrote:
> Pipeline handlers may need to know the Bayer order produced by the
> sensor when a Transform is applied (horizontal or vertical flip). This
> is currently implemented manually in the Raspberry Pi pipeline handler.
> Move the implementation to the CameraSensor class to make it usable in
> other pipeline handlers.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>


Reviewed-by: Daniel Scally <dan.scally@ideasonboard.com>

> ---
>   include/libcamera/internal/camera_sensor.h    |  4 +-
>   .../pipeline/rpi/common/pipeline_base.cpp     | 54 +++----------------
>   .../pipeline/rpi/common/pipeline_base.h       |  6 +--
>   src/libcamera/sensor/camera_sensor.cpp        | 37 ++++++++++++-
>   4 files changed, 45 insertions(+), 56 deletions(-)
>
> diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h
> index 750d6d729cac..d05f48ebeebe 100644
> --- a/include/libcamera/internal/camera_sensor.h
> +++ b/include/libcamera/internal/camera_sensor.h
> @@ -22,12 +22,12 @@
>   
>   #include <libcamera/ipa/core_ipa_interface.h>
>   
> +#include "libcamera/internal/bayer_format.h"
>   #include "libcamera/internal/formats.h"
>   #include "libcamera/internal/v4l2_subdevice.h"
>   
>   namespace libcamera {
>   
> -class BayerFormat;
>   class CameraLens;
>   class MediaEntity;
>   class SensorConfiguration;
> @@ -69,6 +69,7 @@ public:
>   	const ControlList &properties() const { return properties_; }
>   	int sensorInfo(IPACameraSensorInfo *info) const;
>   	Transform computeTransform(Orientation *orientation) const;
> +	BayerFormat::Order bayerOrder(Transform t) const;
>   
>   	const ControlInfoMap &controls() const;
>   	ControlList getControls(const std::vector<uint32_t> &ids);
> @@ -114,6 +115,7 @@ private:
>   	Rectangle activeArea_;
>   	const BayerFormat *bayerFormat_;
>   	bool supportFlips_;
> +	bool flipsAlterBayerOrder_;
>   	Orientation mountingOrientation_;
>   
>   	ControlList properties_;
> diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
> index 9449c3dc458c..7e420b3f90a4 100644
> --- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
> +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
> @@ -235,24 +235,16 @@ CameraConfiguration::Status RPiCameraConfiguration::validate()
>   	for (auto &raw : rawStreams_) {
>   		StreamConfiguration *rawStream = raw.cfg;
>   
> -		/* Adjust the RAW stream to match the computed sensor format. */
> -		BayerFormat sensorBayer = BayerFormat::fromMbusCode(sensorFormat_.code);
> -
>   		/*
> -		 * Some sensors change their Bayer order when they are h-flipped
> -		 * or v-flipped, according to the transform. If this one does, we
> -		 * must advertise the transformed Bayer order in the raw stream.
> -		 * Note how we must fetch the "native" (i.e. untransformed) Bayer
> -		 * order, because the sensor may currently be flipped!
> +		 * Some sensors change their Bayer order when they are
> +		 * h-flipped or v-flipped, according to the transform. Adjust
> +		 * the RAW stream to match the computed sensor format by
> +		 * applying the sensor Bayer order resulting from the transform
> +		 * to the user request.
>   		 */
> -		if (data_->flipsAlterBayerOrder_) {
> -			sensorBayer.order = data_->nativeBayerOrder_;
> -			sensorBayer = sensorBayer.transform(combinedTransform_);
> -		}
>   
> -		/* Apply the sensor adjusted Bayer order to the user request. */
>   		BayerFormat cfgBayer = BayerFormat::fromPixelFormat(rawStream->pixelFormat);
> -		cfgBayer.order = sensorBayer.order;
> +		cfgBayer.order = data_->sensor_->bayerOrder(combinedTransform_);
>   
>   		if (rawStream->pixelFormat != cfgBayer.toPixelFormat()) {
>   			rawStream->pixelFormat = cfgBayer.toPixelFormat();
> @@ -840,40 +832,6 @@ int PipelineHandlerBase::registerCamera(std::unique_ptr<RPi::CameraData> &camera
>   	 */
>   	data->properties_.set(properties::ScalerCropMaximum, Rectangle{});
>   
> -	/*
> -	 * We cache two things about the sensor in relation to transforms
> -	 * (meaning horizontal and vertical flips): if they affect the Bayer
> -	 * ordering, and what the "native" Bayer order is, when no transforms
> -	 * are applied.
> -	 *
> -	 * If flips are supported verify if they affect the Bayer ordering
> -	 * and what the "native" Bayer order is, when no transforms are
> -	 * applied.
> -	 *
> -	 * We note that the sensor's cached list of supported formats is
> -	 * already in the "native" order, with any flips having been undone.
> -	 */
> -	const V4L2Subdevice *sensor = data->sensor_->device();
> -	const struct v4l2_query_ext_ctrl *hflipCtrl = sensor->controlInfo(V4L2_CID_HFLIP);
> -	if (hflipCtrl) {
> -		/* We assume it will support vflips too... */
> -		data->flipsAlterBayerOrder_ = hflipCtrl->flags & V4L2_CTRL_FLAG_MODIFY_LAYOUT;
> -	}
> -
> -	/* Look for a valid Bayer format. */
> -	BayerFormat bayerFormat;
> -	for (const auto &iter : data->sensorFormats_) {
> -		bayerFormat = BayerFormat::fromMbusCode(iter.first);
> -		if (bayerFormat.isValid())
> -			break;
> -	}
> -
> -	if (!bayerFormat.isValid()) {
> -		LOG(RPI, Error) << "No Bayer format found";
> -		return -EINVAL;
> -	}
> -	data->nativeBayerOrder_ = bayerFormat.order;
> -
>   	ret = platformRegister(cameraData, frontend, backend);
>   	if (ret)
>   		return ret;
> diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.h b/src/libcamera/pipeline/rpi/common/pipeline_base.h
> index 267eef1102f1..0608bbe5f0c7 100644
> --- a/src/libcamera/pipeline/rpi/common/pipeline_base.h
> +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.h
> @@ -48,7 +48,7 @@ class CameraData : public Camera::Private
>   public:
>   	CameraData(PipelineHandler *pipe)
>   		: Camera::Private(pipe), state_(State::Stopped),
> -		  flipsAlterBayerOrder_(false), dropFrameCount_(0), buffersAllocated_(false),
> +		  dropFrameCount_(0), buffersAllocated_(false),
>   		  ispOutputCount_(0), ispOutputTotal_(0)
>   	{
>   	}
> @@ -131,10 +131,6 @@ public:
>   
>   	std::queue<Request *> requestQueue_;
>   
> -	/* Store the "native" Bayer order (that is, with no transforms applied). */
> -	bool flipsAlterBayerOrder_;
> -	BayerFormat::Order nativeBayerOrder_;
> -
>   	/* For handling digital zoom. */
>   	IPACameraSensorInfo sensorInfo_;
>   	Rectangle ispCrop_; /* crop in ISP (camera mode) pixels */
> diff --git a/src/libcamera/sensor/camera_sensor.cpp b/src/libcamera/sensor/camera_sensor.cpp
> index 402025566544..5c4f35324055 100644
> --- a/src/libcamera/sensor/camera_sensor.cpp
> +++ b/src/libcamera/sensor/camera_sensor.cpp
> @@ -58,7 +58,7 @@ LOG_DEFINE_CATEGORY(CameraSensor)
>   CameraSensor::CameraSensor(const MediaEntity *entity)
>   	: entity_(entity), pad_(UINT_MAX), staticProps_(nullptr),
>   	  bayerFormat_(nullptr), supportFlips_(false),
> -	  properties_(properties::properties)
> +	  flipsAlterBayerOrder_(false), properties_(properties::properties)
>   {
>   }
>   
> @@ -271,9 +271,14 @@ int CameraSensor::validateSensorDriver()
>   	const struct v4l2_query_ext_ctrl *hflipInfo = subdev_->controlInfo(V4L2_CID_HFLIP);
>   	const struct v4l2_query_ext_ctrl *vflipInfo = subdev_->controlInfo(V4L2_CID_VFLIP);
>   	if (hflipInfo && !(hflipInfo->flags & V4L2_CTRL_FLAG_READ_ONLY) &&
> -	    vflipInfo && !(vflipInfo->flags & V4L2_CTRL_FLAG_READ_ONLY))
> +	    vflipInfo && !(vflipInfo->flags & V4L2_CTRL_FLAG_READ_ONLY)) {
>   		supportFlips_ = true;
>   
> +		if (hflipInfo->flags & V4L2_CTRL_FLAG_MODIFY_LAYOUT ||
> +		    vflipInfo->flags & V4L2_CTRL_FLAG_MODIFY_LAYOUT)
> +			flipsAlterBayerOrder_ = true;
> +	}
> +
>   	if (!supportFlips_)
>   		LOG(CameraSensor, Debug)
>   			<< "Camera sensor does not support horizontal/vertical flip";
> @@ -1041,6 +1046,34 @@ Transform CameraSensor::computeTransform(Orientation *orientation) const
>   	return transform;
>   }
>   
> +/**
> + * \brief Compute the Bayer order that results from the given Transform
> + * \param[in] t The Transform to apply to the sensor
> + *
> + * Some sensors change their Bayer order when they are h-flipped or v-flipped.
> + * This function computes and returns the Bayer order that would result from the
> + * given transform applied to the sensor.
> + *
> + * This function is valid only when the sensor produces raw Bayer formats.
> + *
> + * \return The Bayer order produced by the sensor when the Transform is applied
> + */
> +BayerFormat::Order CameraSensor::bayerOrder(Transform t) const
> +{
> +	/* Return a defined by meaningless value for non-Bayer sensors. */
> +	if (!bayerFormat_)
> +		return BayerFormat::Order::BGGR;
> +
> +	if (!flipsAlterBayerOrder_)
> +		return bayerFormat_->order;
> +
> +	/*
> +	 * Apply the transform to the native (i.e. untransformed) Bayer order,
> +	 * using the rest of the Bayer format supplied by the caller.
> +	 */
> +	return bayerFormat_->transform(t).order;
> +}
> +
>   /**
>    * \brief Retrieve the supported V4L2 controls and their information
>    *

Patch
diff mbox series

diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h
index 750d6d729cac..d05f48ebeebe 100644
--- a/include/libcamera/internal/camera_sensor.h
+++ b/include/libcamera/internal/camera_sensor.h
@@ -22,12 +22,12 @@ 
 
 #include <libcamera/ipa/core_ipa_interface.h>
 
+#include "libcamera/internal/bayer_format.h"
 #include "libcamera/internal/formats.h"
 #include "libcamera/internal/v4l2_subdevice.h"
 
 namespace libcamera {
 
-class BayerFormat;
 class CameraLens;
 class MediaEntity;
 class SensorConfiguration;
@@ -69,6 +69,7 @@  public:
 	const ControlList &properties() const { return properties_; }
 	int sensorInfo(IPACameraSensorInfo *info) const;
 	Transform computeTransform(Orientation *orientation) const;
+	BayerFormat::Order bayerOrder(Transform t) const;
 
 	const ControlInfoMap &controls() const;
 	ControlList getControls(const std::vector<uint32_t> &ids);
@@ -114,6 +115,7 @@  private:
 	Rectangle activeArea_;
 	const BayerFormat *bayerFormat_;
 	bool supportFlips_;
+	bool flipsAlterBayerOrder_;
 	Orientation mountingOrientation_;
 
 	ControlList properties_;
diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
index 9449c3dc458c..7e420b3f90a4 100644
--- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
+++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
@@ -235,24 +235,16 @@  CameraConfiguration::Status RPiCameraConfiguration::validate()
 	for (auto &raw : rawStreams_) {
 		StreamConfiguration *rawStream = raw.cfg;
 
-		/* Adjust the RAW stream to match the computed sensor format. */
-		BayerFormat sensorBayer = BayerFormat::fromMbusCode(sensorFormat_.code);
-
 		/*
-		 * Some sensors change their Bayer order when they are h-flipped
-		 * or v-flipped, according to the transform. If this one does, we
-		 * must advertise the transformed Bayer order in the raw stream.
-		 * Note how we must fetch the "native" (i.e. untransformed) Bayer
-		 * order, because the sensor may currently be flipped!
+		 * Some sensors change their Bayer order when they are
+		 * h-flipped or v-flipped, according to the transform. Adjust
+		 * the RAW stream to match the computed sensor format by
+		 * applying the sensor Bayer order resulting from the transform
+		 * to the user request.
 		 */
-		if (data_->flipsAlterBayerOrder_) {
-			sensorBayer.order = data_->nativeBayerOrder_;
-			sensorBayer = sensorBayer.transform(combinedTransform_);
-		}
 
-		/* Apply the sensor adjusted Bayer order to the user request. */
 		BayerFormat cfgBayer = BayerFormat::fromPixelFormat(rawStream->pixelFormat);
-		cfgBayer.order = sensorBayer.order;
+		cfgBayer.order = data_->sensor_->bayerOrder(combinedTransform_);
 
 		if (rawStream->pixelFormat != cfgBayer.toPixelFormat()) {
 			rawStream->pixelFormat = cfgBayer.toPixelFormat();
@@ -840,40 +832,6 @@  int PipelineHandlerBase::registerCamera(std::unique_ptr<RPi::CameraData> &camera
 	 */
 	data->properties_.set(properties::ScalerCropMaximum, Rectangle{});
 
-	/*
-	 * We cache two things about the sensor in relation to transforms
-	 * (meaning horizontal and vertical flips): if they affect the Bayer
-	 * ordering, and what the "native" Bayer order is, when no transforms
-	 * are applied.
-	 *
-	 * If flips are supported verify if they affect the Bayer ordering
-	 * and what the "native" Bayer order is, when no transforms are
-	 * applied.
-	 *
-	 * We note that the sensor's cached list of supported formats is
-	 * already in the "native" order, with any flips having been undone.
-	 */
-	const V4L2Subdevice *sensor = data->sensor_->device();
-	const struct v4l2_query_ext_ctrl *hflipCtrl = sensor->controlInfo(V4L2_CID_HFLIP);
-	if (hflipCtrl) {
-		/* We assume it will support vflips too... */
-		data->flipsAlterBayerOrder_ = hflipCtrl->flags & V4L2_CTRL_FLAG_MODIFY_LAYOUT;
-	}
-
-	/* Look for a valid Bayer format. */
-	BayerFormat bayerFormat;
-	for (const auto &iter : data->sensorFormats_) {
-		bayerFormat = BayerFormat::fromMbusCode(iter.first);
-		if (bayerFormat.isValid())
-			break;
-	}
-
-	if (!bayerFormat.isValid()) {
-		LOG(RPI, Error) << "No Bayer format found";
-		return -EINVAL;
-	}
-	data->nativeBayerOrder_ = bayerFormat.order;
-
 	ret = platformRegister(cameraData, frontend, backend);
 	if (ret)
 		return ret;
diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.h b/src/libcamera/pipeline/rpi/common/pipeline_base.h
index 267eef1102f1..0608bbe5f0c7 100644
--- a/src/libcamera/pipeline/rpi/common/pipeline_base.h
+++ b/src/libcamera/pipeline/rpi/common/pipeline_base.h
@@ -48,7 +48,7 @@  class CameraData : public Camera::Private
 public:
 	CameraData(PipelineHandler *pipe)
 		: Camera::Private(pipe), state_(State::Stopped),
-		  flipsAlterBayerOrder_(false), dropFrameCount_(0), buffersAllocated_(false),
+		  dropFrameCount_(0), buffersAllocated_(false),
 		  ispOutputCount_(0), ispOutputTotal_(0)
 	{
 	}
@@ -131,10 +131,6 @@  public:
 
 	std::queue<Request *> requestQueue_;
 
-	/* Store the "native" Bayer order (that is, with no transforms applied). */
-	bool flipsAlterBayerOrder_;
-	BayerFormat::Order nativeBayerOrder_;
-
 	/* For handling digital zoom. */
 	IPACameraSensorInfo sensorInfo_;
 	Rectangle ispCrop_; /* crop in ISP (camera mode) pixels */
diff --git a/src/libcamera/sensor/camera_sensor.cpp b/src/libcamera/sensor/camera_sensor.cpp
index 402025566544..5c4f35324055 100644
--- a/src/libcamera/sensor/camera_sensor.cpp
+++ b/src/libcamera/sensor/camera_sensor.cpp
@@ -58,7 +58,7 @@  LOG_DEFINE_CATEGORY(CameraSensor)
 CameraSensor::CameraSensor(const MediaEntity *entity)
 	: entity_(entity), pad_(UINT_MAX), staticProps_(nullptr),
 	  bayerFormat_(nullptr), supportFlips_(false),
-	  properties_(properties::properties)
+	  flipsAlterBayerOrder_(false), properties_(properties::properties)
 {
 }
 
@@ -271,9 +271,14 @@  int CameraSensor::validateSensorDriver()
 	const struct v4l2_query_ext_ctrl *hflipInfo = subdev_->controlInfo(V4L2_CID_HFLIP);
 	const struct v4l2_query_ext_ctrl *vflipInfo = subdev_->controlInfo(V4L2_CID_VFLIP);
 	if (hflipInfo && !(hflipInfo->flags & V4L2_CTRL_FLAG_READ_ONLY) &&
-	    vflipInfo && !(vflipInfo->flags & V4L2_CTRL_FLAG_READ_ONLY))
+	    vflipInfo && !(vflipInfo->flags & V4L2_CTRL_FLAG_READ_ONLY)) {
 		supportFlips_ = true;
 
+		if (hflipInfo->flags & V4L2_CTRL_FLAG_MODIFY_LAYOUT ||
+		    vflipInfo->flags & V4L2_CTRL_FLAG_MODIFY_LAYOUT)
+			flipsAlterBayerOrder_ = true;
+	}
+
 	if (!supportFlips_)
 		LOG(CameraSensor, Debug)
 			<< "Camera sensor does not support horizontal/vertical flip";
@@ -1041,6 +1046,34 @@  Transform CameraSensor::computeTransform(Orientation *orientation) const
 	return transform;
 }
 
+/**
+ * \brief Compute the Bayer order that results from the given Transform
+ * \param[in] t The Transform to apply to the sensor
+ *
+ * Some sensors change their Bayer order when they are h-flipped or v-flipped.
+ * This function computes and returns the Bayer order that would result from the
+ * given transform applied to the sensor.
+ *
+ * This function is valid only when the sensor produces raw Bayer formats.
+ *
+ * \return The Bayer order produced by the sensor when the Transform is applied
+ */
+BayerFormat::Order CameraSensor::bayerOrder(Transform t) const
+{
+	/* Return a defined by meaningless value for non-Bayer sensors. */
+	if (!bayerFormat_)
+		return BayerFormat::Order::BGGR;
+
+	if (!flipsAlterBayerOrder_)
+		return bayerFormat_->order;
+
+	/*
+	 * Apply the transform to the native (i.e. untransformed) Bayer order,
+	 * using the rest of the Bayer format supplied by the caller.
+	 */
+	return bayerFormat_->transform(t).order;
+}
+
 /**
  * \brief Retrieve the supported V4L2 controls and their information
  *