[libcamera-devel,1/2] libcamera: Implement digital zoom

8553 diff mbox series

These changes add a Digital Zoom control, tacking a rectangle as its
argument, indicating the region of the sensor output that will be
zoomed up to the final output size.

Additionally, we need have a method returning the "sensorCrop" which
gives the dimensions of the sensor output within which we can pan and
zoom.
---
 include/libcamera/camera.h                    |  2 ++
 include/libcamera/internal/pipeline_handler.h |  4 +++
 src/libcamera/camera.cpp                      | 26 +++++++++++++++++++
 src/libcamera/control_ids.yaml                | 10 +++++++
 4 files changed, 42 insertions(+)

Hi David,

Thank you for the patch.

On Thu, Jul 02, 2020 at 11:53:36AM +0100, David Plowman wrote:
> These changes add a Digital Zoom control, tacking a rectangle as its
> argument, indicating the region of the sensor output that will be
> zoomed up to the final output size.
> 
> Additionally, we need have a method returning the "sensorCrop" which
> gives the dimensions of the sensor output within which we can pan and
> zoom.

This is a bit of a ad-hoc function. Jacopo has submitted "[PATCH v6]
libcamera: properties: Define pixel array properties" some time ago that
aims at reporting the same information, and more, through properties. It
hasn't been merged yet as we're still trying to make sure all the
details are correct. Could you maybe review the properties in that
patch, to see if they would suit your need, and if you think they
describe the necessary information about the sensor in a good way ?

> ---
>  include/libcamera/camera.h                    |  2 ++
>  include/libcamera/internal/pipeline_handler.h |  4 +++
>  src/libcamera/camera.cpp                      | 26 +++++++++++++++++++
>  src/libcamera/control_ids.yaml                | 10 +++++++
>  4 files changed, 42 insertions(+)
> 
> diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h
> index 9c0e58f..d57b606 100644
> --- a/include/libcamera/camera.h
> +++ b/include/libcamera/camera.h
> @@ -92,6 +92,8 @@ public:
>  	std::unique_ptr<CameraConfiguration> generateConfiguration(const StreamRoles &roles);
>  	int configure(CameraConfiguration *config);
>  
> +	Size const &getSensorCrop() const;
> +
>  	Request *createRequest(uint64_t cookie = 0);
>  	int queueRequest(Request *request);
>  
> diff --git a/include/libcamera/internal/pipeline_handler.h b/include/libcamera/internal/pipeline_handler.h
> index 22e629a..37e069a 100644
> --- a/include/libcamera/internal/pipeline_handler.h
> +++ b/include/libcamera/internal/pipeline_handler.h
> @@ -89,6 +89,8 @@ public:
>  
>  	const char *name() const { return name_; }
>  
> +	Size const &getSensorCrop() const { return sensorCrop_; }
> +
>  protected:
>  	void registerCamera(std::shared_ptr<Camera> camera,
>  			    std::unique_ptr<CameraData> data);
> @@ -100,6 +102,8 @@ protected:
>  
>  	CameraManager *manager_;
>  
> +	Size sensorCrop_;
> +
>  private:
>  	void mediaDeviceDisconnected(MediaDevice *media);
>  	virtual void disconnect();
> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp
> index 69a1b44..6614c93 100644
> --- a/src/libcamera/camera.cpp
> +++ b/src/libcamera/camera.cpp
> @@ -793,6 +793,32 @@ int Camera::configure(CameraConfiguration *config)
>  	return 0;
>  }
>  
> +/**
> + * \brief Return the size of the sensor image being used to form the output
> + *
> + * This method returns the size, in pixels, of the raw image read from the
> + * sensor and which is used to form the output image(s). Note that these
> + * values take account of any cropping performed on the sensor output so
> + * as to produce the correct aspect ratio. It would normally be necessary
> + * to retrieve these values in order to calculate correct parameters for
> + * digital zoom.
> + *
> + * Example: a sensor mode may produce a 1920x1440 output image. But if an
> + * application has requested a 16:9 image, the values returned here would
> + * be 1920x1080 - the largest portion of the sensor output that provides
> + * the correct aspect ratio.
> + *
> + * \context This function is \threadsafe. It will only return valid
> + * (non-zero) values when the camera has been configured.
> + *
> + * \return The dimensions of the sensor image in use.
> + */
> +
> +Size const &Camera::getSensorCrop() const
> +{
> +	return p_->pipe_->getSensorCrop();
> +}
> +
>  /**
>   * \brief Create a request object for the camera
>   * \param[in] cookie Opaque cookie for application use
> diff --git a/src/libcamera/control_ids.yaml b/src/libcamera/control_ids.yaml
> index 8c3e4c7..ac72e2a 100644
> --- a/src/libcamera/control_ids.yaml
> +++ b/src/libcamera/control_ids.yaml
> @@ -251,4 +251,14 @@ controls:
>          higher than anyone could reasonably want. Negative values are
>          not allowed. Note also that sharpening is not applied to raw
>          streams.
> +
> +  - DigitalZoom:
> +      type: Rectangle
> +      description: |
> +        Sets the portion of the full sensor image, in pixels, that will be
> +        used for digital zoom. That is, this part of the sensor output will
> +        be scaled up to make the full size output image (and everything else
> +        discarded). To obtain the "full sensor image" that is available, the
> +        method Camera::getOutputCrop() should be called once the camera is
> +        configured. An application may pan and zoom within this rectangle.
>  ...

Hi Laurent

Thanks for pointing this out.

On Fri, 3 Jul 2020 at 11:43, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> Hi David,
>
> Thank you for the patch.
>
> On Thu, Jul 02, 2020 at 11:53:36AM +0100, David Plowman wrote:
> > These changes add a Digital Zoom control, tacking a rectangle as its
> > argument, indicating the region of the sensor output that will be
> > zoomed up to the final output size.
> >
> > Additionally, we need have a method returning the "sensorCrop" which
> > gives the dimensions of the sensor output within which we can pan and
> > zoom.
>
> This is a bit of a ad-hoc function. Jacopo has submitted "[PATCH v6]
> libcamera: properties: Define pixel array properties" some time ago that
> aims at reporting the same information, and more, through properties. It
> hasn't been merged yet as we're still trying to make sure all the
> details are correct. Could you maybe review the properties in that
> patch, to see if they would suit your need, and if you think they
> describe the necessary information about the sensor in a good way ?

Yes, this is a bit awkward. The pixel array properties define things
that are only a property of the pixel array, but here we have
something that depends on the pixel array, the camera modes defined
from it, and some choices made by the pipeline hander. To recap an
earlier example (from the imx477).

The pixel array actually has 4056x3040 usable pixels, and let's
suppose we're asking for 1080p output.

We've defined a 2x2 binned 2028x1520 camera mode, and our pipeline
handler should choose this. It then has to pick a 16:9 rectangle to
match the output aspect ratio, and this represents the "unzoomed"
field of view.

It might choose the largest field of view that it can, therefore a
2028x1141 rectangle taken from offset (0,190) in the sensor's
2028x1520 output.

Or, if we're feeling mathematically anxious, a pipeline handler might
prefer 1:1 pixels, and therefore give us a 1920x1080 rectangle, from
offset (54,220).

In the first case, the "sensorCrop" returns 2028x1141, and in the
second, 1920x1080.

It seems to me this number is the culmination of quite a few different
properties and decisions, and only the pipeline handler can work it
out - I can't see that a fixed set of "pixel array properties" would
be enough. But I agree, a "less arbitrary" way to signal this would be
nicer.

Plan B was of course always to use ratios, rather than pixels. In this
scheme you simply don't need this function, though you don't get
"precise pixel-level control" if that's what you want.

Indeed, suggestions would be appreciated...

Best regards
David

>
> > ---
> >  include/libcamera/camera.h                    |  2 ++
> >  include/libcamera/internal/pipeline_handler.h |  4 +++
> >  src/libcamera/camera.cpp                      | 26 +++++++++++++++++++
> >  src/libcamera/control_ids.yaml                | 10 +++++++
> >  4 files changed, 42 insertions(+)
> >
> > diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h
> > index 9c0e58f..d57b606 100644
> > --- a/include/libcamera/camera.h
> > +++ b/include/libcamera/camera.h
> > @@ -92,6 +92,8 @@ public:
> >       std::unique_ptr<CameraConfiguration> generateConfiguration(const StreamRoles &roles);
> >       int configure(CameraConfiguration *config);
> >
> > +     Size const &getSensorCrop() const;
> > +
> >       Request *createRequest(uint64_t cookie = 0);
> >       int queueRequest(Request *request);
> >
> > diff --git a/include/libcamera/internal/pipeline_handler.h b/include/libcamera/internal/pipeline_handler.h
> > index 22e629a..37e069a 100644
> > --- a/include/libcamera/internal/pipeline_handler.h
> > +++ b/include/libcamera/internal/pipeline_handler.h
> > @@ -89,6 +89,8 @@ public:
> >
> >       const char *name() const { return name_; }
> >
> > +     Size const &getSensorCrop() const { return sensorCrop_; }
> > +
> >  protected:
> >       void registerCamera(std::shared_ptr<Camera> camera,
> >                           std::unique_ptr<CameraData> data);
> > @@ -100,6 +102,8 @@ protected:
> >
> >       CameraManager *manager_;
> >
> > +     Size sensorCrop_;
> > +
> >  private:
> >       void mediaDeviceDisconnected(MediaDevice *media);
> >       virtual void disconnect();
> > diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp
> > index 69a1b44..6614c93 100644
> > --- a/src/libcamera/camera.cpp
> > +++ b/src/libcamera/camera.cpp
> > @@ -793,6 +793,32 @@ int Camera::configure(CameraConfiguration *config)
> >       return 0;
> >  }
> >
> > +/**
> > + * \brief Return the size of the sensor image being used to form the output
> > + *
> > + * This method returns the size, in pixels, of the raw image read from the
> > + * sensor and which is used to form the output image(s). Note that these
> > + * values take account of any cropping performed on the sensor output so
> > + * as to produce the correct aspect ratio. It would normally be necessary
> > + * to retrieve these values in order to calculate correct parameters for
> > + * digital zoom.
> > + *
> > + * Example: a sensor mode may produce a 1920x1440 output image. But if an
> > + * application has requested a 16:9 image, the values returned here would
> > + * be 1920x1080 - the largest portion of the sensor output that provides
> > + * the correct aspect ratio.
> > + *
> > + * \context This function is \threadsafe. It will only return valid
> > + * (non-zero) values when the camera has been configured.
> > + *
> > + * \return The dimensions of the sensor image in use.
> > + */
> > +
> > +Size const &Camera::getSensorCrop() const
> > +{
> > +     return p_->pipe_->getSensorCrop();
> > +}
> > +
> >  /**
> >   * \brief Create a request object for the camera
> >   * \param[in] cookie Opaque cookie for application use
> > diff --git a/src/libcamera/control_ids.yaml b/src/libcamera/control_ids.yaml
> > index 8c3e4c7..ac72e2a 100644
> > --- a/src/libcamera/control_ids.yaml
> > +++ b/src/libcamera/control_ids.yaml
> > @@ -251,4 +251,14 @@ controls:
> >          higher than anyone could reasonably want. Negative values are
> >          not allowed. Note also that sharpening is not applied to raw
> >          streams.
> > +
> > +  - DigitalZoom:
> > +      type: Rectangle
> > +      description: |
> > +        Sets the portion of the full sensor image, in pixels, that will be
> > +        used for digital zoom. That is, this part of the sensor output will
> > +        be scaled up to make the full size output image (and everything else
> > +        discarded). To obtain the "full sensor image" that is available, the
> > +        method Camera::getOutputCrop() should be called once the camera is
> > +        configured. An application may pan and zoom within this rectangle.
> >  ...
>
> --
> Regards,
>
> Laurent Pinchart