[PATCH v4 08/20] libcamera: converter: Add function to query crop bounds

[Jacopo Mondi]

Hi Stefan

On Mon, Dec 16, 2024 at 04:40:48PM +0100, Stefan Klug wrote:
> The inputCropBounds_ member of the V4L2M2MConverter::Stream class is
> only initialized after a V4L2M2MConverter::configure() call, when the
> streams are initialized.
>
> However, the converter has crop limits that do not depend on the
> configured Streams, and which are currently not accessible from the
> class interface.

However you seem to store the absolute limts computed at class
creation in inputCropBounds_ which gets updated at each configure()
time.

>
> Add a new inputCropBounds() function to the V4L2M2MConverter class
> that allows to retrieve the converter crop limits before any stream
> is configured.

But in this way, after a configuration, calling

        inputCropBounds()
        inputCropBounds(stream)

return the same data.

Or have I missed something ?
When we discussed it I know I suggested to split the two functions
apart, but my thinking was about being able to return the default crop
rectangle despite the currently configured format. Sorry if I wasn't
clear. Or do you actually prefer this version ?

>
> This is particularly useful for pipelines to initialize controls and
> properties and to implement validation before the Camera gets
> configured.
>
> Signed-off-by: Stefan Klug <stefan.klug at ideasonboard.com>
>
> ---
>
> Changes in v4:
> - Split off from libcamera: converter_v4l2_m2m: Improve crop bounds
>   support
> - Added separate inputCropBounds() function to the dewarper class
>
> Fuxup inputCrop()
> ---
>  include/libcamera/internal/converter.h         |  1 +
>  .../internal/converter/converter_v4l2_m2m.h    |  2 ++
>  src/libcamera/converter.cpp                    | 13 +++++++++++++
>  src/libcamera/converter/converter_v4l2_m2m.cpp | 18 ++++++++++++------
>  4 files changed, 28 insertions(+), 6 deletions(-)
>
> diff --git a/include/libcamera/internal/converter.h b/include/libcamera/internal/converter.h
> index ffbb6f345cd5..04187a2a96e8 100644
> --- a/include/libcamera/internal/converter.h
> +++ b/include/libcamera/internal/converter.h
> @@ -66,6 +66,7 @@ public:
>  				 const std::map<const Stream *, FrameBuffer *> &outputs) = 0;
>
>  	virtual int setInputCrop(const Stream *stream, Rectangle *rect) = 0;
> +	virtual std::pair<Rectangle, Rectangle> inputCropBounds() = 0;
>  	virtual std::pair<Rectangle, Rectangle> inputCropBounds(const Stream *stream) = 0;
>
>  	Signal<FrameBuffer *> inputBufferReady;
> diff --git a/include/libcamera/internal/converter/converter_v4l2_m2m.h b/include/libcamera/internal/converter/converter_v4l2_m2m.h
> index 9b8e43ff0b91..402a803959b9 100644
> --- a/include/libcamera/internal/converter/converter_v4l2_m2m.h
> +++ b/include/libcamera/internal/converter/converter_v4l2_m2m.h
> @@ -60,6 +60,7 @@ public:
>  			 const std::map<const Stream *, FrameBuffer *> &outputs) override;
>
>  	int setInputCrop(const Stream *stream, Rectangle *rect) override;
> +	std::pair<Rectangle, Rectangle> inputCropBounds() override { return inputCropBounds_; }
>  	std::pair<Rectangle, Rectangle> inputCropBounds(const Stream *stream) override;
>
>  private:
> @@ -106,6 +107,7 @@ private:
>
>  	std::map<const Stream *, std::unique_ptr<V4L2M2MStream>> streams_;
>  	std::map<FrameBuffer *, unsigned int> queue_;
> +	std::pair<Rectangle, Rectangle> inputCropBounds_;
>  };
>
>  } /* namespace libcamera */
> diff --git a/src/libcamera/converter.cpp b/src/libcamera/converter.cpp
> index 3a3f84344c5e..73c02fdcf4bb 100644
> --- a/src/libcamera/converter.cpp
> +++ b/src/libcamera/converter.cpp
> @@ -185,6 +185,16 @@ Converter::~Converter()
>
>  /**
>   * \fn Converter::inputCropBounds()
> + * \brief Retrieve the crop bounds of the converter
> + *
> + * Retrieve the minimum and maximum crop bounds of the converter. This can be
> + * used to query the crop bounds before configuring a stream.
> + *
> + * \return A pair containing the minimum and maximum crop bound in that order
> + */
> +
> +/**
> + * \fn Converter::inputCropBounds(const Stream *stream)
>   * \brief Retrieve the crop bounds for \a stream
>   * \param[in] stream The output stream
>   *
> @@ -195,6 +205,9 @@ Converter::~Converter()
>   * this function should be called after the \a stream has been configured using
>   * configure().
>   *
> + * When called with an unconfigured \a stream, this function returns a pair of
> + * null rectangles.
> + *
>   * \return A pair containing the minimum and maximum crop bound in that order
>   */
>
> diff --git a/src/libcamera/converter/converter_v4l2_m2m.cpp b/src/libcamera/converter/converter_v4l2_m2m.cpp
> index 8c341fe199f6..342aa32dab52 100644
> --- a/src/libcamera/converter/converter_v4l2_m2m.cpp
> +++ b/src/libcamera/converter/converter_v4l2_m2m.cpp
> @@ -273,10 +273,9 @@ V4L2M2MConverter::V4L2M2MConverter(MediaDevice *media)
>  		return;
>  	}
>
> -	Rectangle minCrop;
> -	Rectangle maxCrop;
> -	ret = getCropBounds(m2m_->output(), minCrop, maxCrop);
> -	if (!ret && minCrop != maxCrop) {
> +	ret = getCropBounds(m2m_->output(), inputCropBounds_.first,
> +			    inputCropBounds_.second);
> +	if (!ret && inputCropBounds_.first != inputCropBounds_.second) {
>  		features_ |= Feature::InputCrop;
>
>  		LOG(Converter, Info)
> @@ -469,14 +468,21 @@ int V4L2M2MConverter::setInputCrop(const Stream *stream, Rectangle *rect)
>  }
>
>  /**
> - * \copydoc libcamera::Converter::inputCropBounds
> + * \fn libcamera::V4L2M2MConverter::inputCropBounds()
> + * \copydoc libcamera::Converter::inputCropBounds()
> + */
> +
> +/**
> + * \copydoc libcamera::Converter::inputCropBounds(const Stream *stream)
>   */
>  std::pair<Rectangle, Rectangle>
>  V4L2M2MConverter::inputCropBounds(const Stream *stream)
>  {
>  	auto iter = streams_.find(stream);
> -	if (iter == streams_.end())
> +	if (iter == streams_.end()) {
> +		LOG(Converter, Error) << "Invalid output stream";
>  		return {};
> +	}
>
>  	return iter->second->inputCropBounds();
>  }
> --
> 2.43.0
>