[libcamera-devel] [PATCH] pipeline: simple: Validate transform

[Jacopo Mondi]

Hi Robert,

On Thu, Apr 27, 2023 at 12:53:56PM +0200, Robert Mader via libcamera-devel wrote:
> Just like we do for other pipeline handlers already.
> This ensures we corretly pass on transforms that are not handled by the
> sensor - e.g. rotations - back to the app via the config, which is
> required on devices like the Pinephone.
>
> Signed-off-by: Robert Mader <robert.mader at collabora.com>
> Tested-by: Arnav Singh <me at arnavion.dev>
> ---
>  src/libcamera/pipeline/simple/simple.cpp | 21 ++++++++++++++-------
>  1 file changed, 14 insertions(+), 7 deletions(-)
>
> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
> index 2423ec10..abfb4c87 100644
> --- a/src/libcamera/pipeline/simple/simple.cpp
> +++ b/src/libcamera/pipeline/simple/simple.cpp
> @@ -211,7 +211,8 @@ public:
>  	int init();
>  	int setupLinks();
>  	int setupFormats(V4L2SubdeviceFormat *format,
> -			 V4L2Subdevice::Whence whence);
> +			 V4L2Subdevice::Whence whence,
> +			 Transform transform = Transform::Identity);
>  	void bufferReady(FrameBuffer *buffer);
>
>  	unsigned int streamIndex(const Stream *stream) const
> @@ -292,6 +293,7 @@ public:
>  	}
>
>  	bool needConversion() const { return needConversion_; }
> +	const Transform &combinedTransform() { return combinedTransform_; }

	const Transform &combinedTransform() const { return combinedTransform_; }

>
>  private:
>  	/*
> @@ -304,6 +306,7 @@ private:
>
>  	const SimpleCameraData::Configuration *pipeConfig_;
>  	bool needConversion_;
> +	Transform combinedTransform_;
>  };
>
>  class SimplePipelineHandler : public PipelineHandler
> @@ -664,7 +667,8 @@ int SimpleCameraData::setupLinks()
>  }
>
>  int SimpleCameraData::setupFormats(V4L2SubdeviceFormat *format,
> -				   V4L2Subdevice::Whence whence)
> +				   V4L2Subdevice::Whence whence,
> +				   Transform transform)
>  {
>  	SimplePipelineHandler *pipe = SimpleCameraData::pipe();
>  	int ret;
> @@ -673,7 +677,7 @@ int SimpleCameraData::setupFormats(V4L2SubdeviceFormat *format,
>  	 * Configure the format on the sensor output and propagate it through
>  	 * the pipeline.
>  	 */
> -	ret = sensor_->setFormat(format);
> +	ret = sensor_->setFormat(format, transform);
>  	if (ret < 0)
>  		return ret;
>
> @@ -877,15 +881,16 @@ SimpleCameraConfiguration::SimpleCameraConfiguration(Camera *camera,
>
>  CameraConfiguration::Status SimpleCameraConfiguration::validate()
>  {
> +	const CameraSensor *sensor = data_->sensor_.get();
>  	Status status = Valid;
>
>  	if (config_.empty())
>  		return Invalid;
>
> -	if (transform != Transform::Identity) {
> -		transform = Transform::Identity;
> +	Transform requestedTransform = transform;
> +	combinedTransform_ = sensor->validateTransform(&transform);
> +	if (transform != requestedTransform)
>  		status = Adjusted;
> -	}
>
>  	/* Cap the number of entries to the available streams. */
>  	if (config_.size() > data_->streams_.size()) {
> @@ -1116,7 +1121,9 @@ int SimplePipelineHandler::configure(Camera *camera, CameraConfiguration *c)
>  	format.mbus_code = pipeConfig->code;
>  	format.size = pipeConfig->sensorSize;
>
> -	ret = data->setupFormats(&format, V4L2Subdevice::ActiveFormat);
> +	ret = data->setupFormats(&format,
> +				 V4L2Subdevice::ActiveFormat,
> +				 config->combinedTransform());

Probably fits in 2 lines only

Minor nits, can be fixed when applying
Reviewed-by: Jacopo Mondi <jacopo.mondi at ideasonboard.com>

>  	if (ret < 0)
>  		return ret;
>
> --
> 2.40.0
>