[libcamera-devel,v3,1/2] libcamera: Infrastructure for digital zoom

8936 diff mbox series

These changes add a Digital Zoom control, taking a rectangle as its
argument, indicating the region of the sensor output that the
pipeline will "zoom up" to the final output size.

Additionally, we need to have a method returning the "pipelineCrop"
which gives the dimensions of the sensor output, taken by the
pipeline, and within which we can subsequently pan and zoom.

Signed-off-by: David Plowman <david.plowman@raspberrypi.com>
---
 include/libcamera/camera.h                    |  2 ++
 include/libcamera/internal/pipeline_handler.h |  4 +++
 src/libcamera/camera.cpp                      | 27 +++++++++++++++++++
 src/libcamera/control_ids.yaml                | 10 +++++++
 4 files changed, 43 insertions(+)

Hi David,
  sorry for jumping late on the topic, I managed to read the
discussions in v1 and v2 only today.

On Thu, Jul 23, 2020 at 09:43:37AM +0100, David Plowman wrote:
> These changes add a Digital Zoom control, taking a rectangle as its
> argument, indicating the region of the sensor output that the
> pipeline will "zoom up" to the final output size.
>
> Additionally, we need to have a method returning the "pipelineCrop"
> which gives the dimensions of the sensor output, taken by the
> pipeline, and within which we can subsequently pan and zoom.

This is the part I don't get. What's the intended user of this
information ? Is this meant to be provided to applications so that
they know which is the area inside which they can later pan/zoom, right ?

I'm kind of against to ad-hoc function to retrieve information about
the image processing pipeline configuration. I can easily see them
multiply and make our API awful, and each platform would need to add
its own very-specific bits, and that can't certainly go through the
main Camera class API.

If I got your use case right:

1) You need to report the pipeline crop area, which is a rectangle
   defined on the frame that is produced by the sensor.
   Using your example:

        sensorSize = 1920x1440
        pipelineCrop = 1920x1080

   It seems to me the 'pipelineCrop' is a good candidate to be a
   CameraConfiguration property, which at the moment is not yet
   implemented. Roughly speaking, we have been discussing it in the
   context of frame durations, and it's about augmenting the
   CameraConfiguration class with a set of properties that depend on
   the currently applied configuration of the enabled streams.

   Ideally we should be able to receive CameraConfiguration, validate
   it, and populate it with properties, like the minimum frame
   duration (which is a combination of durations of all streams), and
   now with this new 'pipelineCrop' which is the result of a decision
   the pipeline handler takes based on the requested output stream
   sizes.

   I think the ideal plan forward for this would be to define a new
   control/property (I would say property as it's immutable from
   applications) that is set by pipeline handlers during validate()
   when the sensor mode is selected and eventually cropped.

   Application can access it from the CameraConfiguration and decide
   where to set their pan/zoom rectangle.

   How the pipeline handler applies the cropping to the sensor
   produced frame is less relevant imo. Systems which have direct access
   to the v4l2 subdev will probably use the crop/compose rectangle
   applied on the sensor. In your case I see you use the crop target
   on the ISP video device node. Am I wrong or you could calculate
   that at validate() time, once you know all the desired output sizes and
   based on them identify the 'best' sensor mode to use ?

2) Application, once they know the 'pipelineCrop' (I'm ok with that
   term btw, as we have analogCrop and we will have a digitalCrop for
   the sensor's processing stages, so pipelineCrop feels right) can
   set their pan/zoom rectangle with a control, as the one you have
   defined below. Now, what do you think the reference of that rectangle
   should be ? I mean, should the 'pipelineCrop' always be in position
   (0,0) and the 'digitalZoom' be applied on top of it ? I would say
   so, but then we lose the information on which part of the image
   the pipeline decided to crop, and I'm not sure it is relevant or
   not.

   The other possibility is to report the 'pipelineCrop' in respect to
   the full active pixel array size, and ask application to provide a
   'digitalZoom' rectangle with the same reference. Is it worth the
   additional complication in you opinion?

   Third option is to report the sensor frame size AND the pipeline crop
   defined in respect to it, and let application provide a digitalZoom
   rectangle defined in respect to the pipeline crop. This would look
   like (numbers are random):
        sensorFrameSize = {0, 0, 1920, 1440}
        pipelineCrop = { 0, 150, 1920, 1080}
        digitalZoom = {300, 300, 1280, 720}

   This would allow application to know that the sensor frame is
   1920x1400, the pipeline selected a region on 1920x1080 by applying
   an offset of 150 pixels in the vertical direction and the desired
   digital zoom is then applied on this last rectangle (resulting in a
   effective (300,450) dislocation from the full sensor frame).

I won't bikeshed on names too much, just leave as a reference that
Android reports the same property we are defining as pipelineCrop as
'scalera.cropRegion' as for them 'scaler' is used to report properties
of the ISP processing pipeline.

I'm sorry for the wall of text, I wish we had a blackboard :)

Thanks
  j

>
> Signed-off-by: David Plowman <david.plowman@raspberrypi.com>
> ---
>  include/libcamera/camera.h                    |  2 ++
>  include/libcamera/internal/pipeline_handler.h |  4 +++
>  src/libcamera/camera.cpp                      | 27 +++++++++++++++++++
>  src/libcamera/control_ids.yaml                | 10 +++++++
>  4 files changed, 43 insertions(+)
>
> diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h
> index 4d1a4a9..6819b8e 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 &getPipelineCrop() 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..5bfe890 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 &getPipelineCrop() const { return pipelineCrop_; }
> +
>  protected:
>  	void registerCamera(std::shared_ptr<Camera> camera,
>  			    std::unique_ptr<CameraData> data);
> @@ -100,6 +102,8 @@ protected:
>
>  	CameraManager *manager_;
>
> +	Size pipelineCrop_;
> +
>  private:
>  	void mediaDeviceDisconnected(MediaDevice *media);
>  	virtual void disconnect();
> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp
> index 69a1b44..f8b8ec6 100644
> --- a/src/libcamera/camera.cpp
> +++ b/src/libcamera/camera.cpp
> @@ -793,6 +793,33 @@ int Camera::configure(CameraConfiguration *config)
>  	return 0;
>  }
>
> +/**
> + * \brief Return the size of the sensor image being used by the pipeline
> + * to create the output.
> + *
> + * This method returns the size, in pixels, of the raw image read from the
> + * sensor and which is used by the pipeline to form the output image(s)
> + * (rescaling if necessary). 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 might
> + * 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 used by the pipeline.
> + */
> +
> +Size const &Camera::getPipelineCrop() const
> +{
> +	return p_->pipe_->getPipelineCrop();
> +}
> +
>  /**
>   * \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 988b501..5a099d5 100644
> --- a/src/libcamera/control_ids.yaml
> +++ b/src/libcamera/control_ids.yaml
> @@ -262,4 +262,14 @@ controls:
>          In this respect, it is not necessarily aimed at providing a way to
>          implement a focus algorithm by the application, rather an indication of
>          how in-focus a frame is.
> +
> +  - 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.
>  ...
> --
> 2.20.1
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel

Hi Jacopo, everyone

On Thu, 30 Jul 2020 at 17:34, Jacopo Mondi <jacopo@jmondi.org> wrote:
>
> Hi David,
>   sorry for jumping late on the topic, I managed to read the
> discussions in v1 and v2 only today.

Please don't apologise, I'm very glad that you've picked this up. This
is actually one of the "big ticket" items that libcamera is currently
lacking compared to our existing stack, so I was going to give the
discussion a "nudge" quite soon!

>
> On Thu, Jul 23, 2020 at 09:43:37AM +0100, David Plowman wrote:
> > These changes add a Digital Zoom control, taking a rectangle as its
> > argument, indicating the region of the sensor output that the
> > pipeline will "zoom up" to the final output size.
> >
> > Additionally, we need to have a method returning the "pipelineCrop"
> > which gives the dimensions of the sensor output, taken by the
> > pipeline, and within which we can subsequently pan and zoom.
>
> This is the part I don't get. What's the intended user of this
> information ? Is this meant to be provided to applications so that
> they know which is the area inside which they can later pan/zoom, right ?

Correct. I'm imagining a user might say "I want to see the image at 2x
zoom". If we know the unzoomed image is being created by (as per one
of my examples somewhere) 2028x1140 pixels from the sensor, then the
application would send a "digital zoom" of offset: (507,285) size:
1014x570,

I think the feeling was that we liked having control right down to the
pixel level, which in turn means the the numbers 2028x1140 need to be
supplied to the application from somewhere.

>
> I'm kind of against to ad-hoc function to retrieve information about
> the image processing pipeline configuration. I can easily see them
> multiply and make our API awful, and each platform would need to add
> its own very-specific bits, and that can't certainly go through the
> main Camera class API.
>
> If I got your use case right:
>
> 1) You need to report the pipeline crop area, which is a rectangle
>    defined on the frame that is produced by the sensor.
>    Using your example:
>
>         sensorSize = 1920x1440
>         pipelineCrop = 1920x1080
>
>    It seems to me the 'pipelineCrop' is a good candidate to be a
>    CameraConfiguration property, which at the moment is not yet
>    implemented. Roughly speaking, we have been discussing it in the
>    context of frame durations, and it's about augmenting the
>    CameraConfiguration class with a set of properties that depend on
>    the currently applied configuration of the enabled streams.
>
>    Ideally we should be able to receive CameraConfiguration, validate
>    it, and populate it with properties, like the minimum frame
>    duration (which is a combination of durations of all streams), and
>    now with this new 'pipelineCrop' which is the result of a decision
>    the pipeline handler takes based on the requested output stream
>    sizes.
>
>    I think the ideal plan forward for this would be to define a new
>    control/property (I would say property as it's immutable from
>    applications) that is set by pipeline handlers during validate()
>    when the sensor mode is selected and eventually cropped.

Absolutely. The problem with this has never been how to calculate the
"pipelineCrop", or who calculates it (that's the pipeline handler),
but how to signal this value to the application. When can I have this
feature, please? :)

>
>    Application can access it from the CameraConfiguration and decide
>    where to set their pan/zoom rectangle.
>
>    How the pipeline handler applies the cropping to the sensor
>    produced frame is less relevant imo. Systems which have direct access
>    to the v4l2 subdev will probably use the crop/compose rectangle
>    applied on the sensor. In your case I see you use the crop target
>    on the ISP video device node. Am I wrong or you could calculate
>    that at validate() time, once you know all the desired output sizes and
>    based on them identify the 'best' sensor mode to use ?

We update the selection on the ISP device node. I would expect this
approach to be more common than doing it on the sensor itself (you
don't lose statistics for the rest of the frame), but it's of course
up to the pipeline handlers.

>
> 2) Application, once they know the 'pipelineCrop' (I'm ok with that
>    term btw, as we have analogCrop and we will have a digitalCrop for
>    the sensor's processing stages, so pipelineCrop feels right) can
>    set their pan/zoom rectangle with a control, as the one you have
>    defined below. Now, what do you think the reference of that rectangle
>    should be ? I mean, should the 'pipelineCrop' always be in position
>    (0,0) and the 'digitalZoom' be applied on top of it ? I would say
>    so, but then we lose the information on which part of the image
>    the pipeline decided to crop, and I'm not sure it is relevant or
>    not.

So I'm in favour of giving applications only the size of the pipeline
crop, nothing else. It's all you need to implement digital zoom, and
panning within the original image, and it makes life very
straightforward for everyone.

I agree there are more complex scenarios you could imagine. Maybe you
let applications pan/zoom within the whole of the sensor area, so
you'd be able to pan to parts of the image that you can't see in the
"default unzoomed" version. I mean, there's nothing "wrong" in this,
it just feels more complicated and a bit unexpected to me. I also
think this would delegate most of the aspect ratio calculations to the
applications, and I suspect they'd get themselves confused and
actually implement the simpler scheme anyway... But yes, there is a
decision to be made here.

>
>    The other possibility is to report the 'pipelineCrop' in respect to
>    the full active pixel array size, and ask application to provide a
>    'digitalZoom' rectangle with the same reference. Is it worth the
>    additional complication in you opinion?

Sorry, answered that one above!

>
>    Third option is to report the sensor frame size AND the pipeline crop
>    defined in respect to it, and let application provide a digitalZoom
>    rectangle defined in respect to the pipeline crop. This would look
>    like (numbers are random):
>         sensorFrameSize = {0, 0, 1920, 1440}
>         pipelineCrop = { 0, 150, 1920, 1080}
>         digitalZoom = {300, 300, 1280, 720}
>
>    This would allow application to know that the sensor frame is
>    1920x1400, the pipeline selected a region on 1920x1080 by applying
>    an offset of 150 pixels in the vertical direction and the desired
>    digital zoom is then applied on this last rectangle (resulting in a
>    effective (300,450) dislocation from the full sensor frame).
>
> I won't bikeshed on names too much, just leave as a reference that
> Android reports the same property we are defining as pipelineCrop as
> 'scalera.cropRegion' as for them 'scaler' is used to report properties
> of the ISP processing pipeline.

I could go with "scalerCrop" too!

Best regards
David

>
> I'm sorry for the wall of text, I wish we had a blackboard :)
>
> Thanks
>   j
>
> >
> > Signed-off-by: David Plowman <david.plowman@raspberrypi.com>
> > ---
> >  include/libcamera/camera.h                    |  2 ++
> >  include/libcamera/internal/pipeline_handler.h |  4 +++
> >  src/libcamera/camera.cpp                      | 27 +++++++++++++++++++
> >  src/libcamera/control_ids.yaml                | 10 +++++++
> >  4 files changed, 43 insertions(+)
> >
> > diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h
> > index 4d1a4a9..6819b8e 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 &getPipelineCrop() 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..5bfe890 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 &getPipelineCrop() const { return pipelineCrop_; }
> > +
> >  protected:
> >       void registerCamera(std::shared_ptr<Camera> camera,
> >                           std::unique_ptr<CameraData> data);
> > @@ -100,6 +102,8 @@ protected:
> >
> >       CameraManager *manager_;
> >
> > +     Size pipelineCrop_;
> > +
> >  private:
> >       void mediaDeviceDisconnected(MediaDevice *media);
> >       virtual void disconnect();
> > diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp
> > index 69a1b44..f8b8ec6 100644
> > --- a/src/libcamera/camera.cpp
> > +++ b/src/libcamera/camera.cpp
> > @@ -793,6 +793,33 @@ int Camera::configure(CameraConfiguration *config)
> >       return 0;
> >  }
> >
> > +/**
> > + * \brief Return the size of the sensor image being used by the pipeline
> > + * to create the output.
> > + *
> > + * This method returns the size, in pixels, of the raw image read from the
> > + * sensor and which is used by the pipeline to form the output image(s)
> > + * (rescaling if necessary). 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 might
> > + * 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 used by the pipeline.
> > + */
> > +
> > +Size const &Camera::getPipelineCrop() const
> > +{
> > +     return p_->pipe_->getPipelineCrop();
> > +}
> > +
> >  /**
> >   * \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 988b501..5a099d5 100644
> > --- a/src/libcamera/control_ids.yaml
> > +++ b/src/libcamera/control_ids.yaml
> > @@ -262,4 +262,14 @@ controls:
> >          In this respect, it is not necessarily aimed at providing a way to
> >          implement a focus algorithm by the application, rather an indication of
> >          how in-focus a frame is.
> > +
> > +  - 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.
> >  ...
> > --
> > 2.20.1
> >
> > _______________________________________________
> > libcamera-devel mailing list
> > libcamera-devel@lists.libcamera.org
> > https://lists.libcamera.org/listinfo/libcamera-devel

Hi David,

On Fri, Jul 31, 2020 at 10:02:44AM +0100, David Plowman wrote:
> Hi Jacopo, everyone
>
> On Thu, 30 Jul 2020 at 17:34, Jacopo Mondi <jacopo@jmondi.org> wrote:
> >
> > Hi David,
> >   sorry for jumping late on the topic, I managed to read the
> > discussions in v1 and v2 only today.
>
> Please don't apologise, I'm very glad that you've picked this up. This
> is actually one of the "big ticket" items that libcamera is currently
> lacking compared to our existing stack, so I was going to give the
> discussion a "nudge" quite soon!
>
> >
> > On Thu, Jul 23, 2020 at 09:43:37AM +0100, David Plowman wrote:
> > > These changes add a Digital Zoom control, taking a rectangle as its
> > > argument, indicating the region of the sensor output that the
> > > pipeline will "zoom up" to the final output size.
> > >
> > > Additionally, we need to have a method returning the "pipelineCrop"
> > > which gives the dimensions of the sensor output, taken by the
> > > pipeline, and within which we can subsequently pan and zoom.
> >
> > This is the part I don't get. What's the intended user of this
> > information ? Is this meant to be provided to applications so that
> > they know which is the area inside which they can later pan/zoom, right ?
>
> Correct. I'm imagining a user might say "I want to see the image at 2x
> zoom". If we know the unzoomed image is being created by (as per one
> of my examples somewhere) 2028x1140 pixels from the sensor, then the
> application would send a "digital zoom" of offset: (507,285) size:
> 1014x570,
>
> I think the feeling was that we liked having control right down to the
> pixel level, which in turn means the the numbers 2028x1140 need to be
> supplied to the application from somewhere.
>

Just for the sake of discussion, a 'digitalZoomFactor' (or whatever
other name) control that takes a, in example, 2x value alongside with
a more precies 'digitalZoomRectangle' that behaves like you have
described, would make sense ?

> >
> > I'm kind of against to ad-hoc function to retrieve information about
> > the image processing pipeline configuration. I can easily see them
> > multiply and make our API awful, and each platform would need to add
> > its own very-specific bits, and that can't certainly go through the
> > main Camera class API.
> >
> > If I got your use case right:
> >
> > 1) You need to report the pipeline crop area, which is a rectangle
> >    defined on the frame that is produced by the sensor.
> >    Using your example:
> >
> >         sensorSize = 1920x1440
> >         pipelineCrop = 1920x1080
> >
> >    It seems to me the 'pipelineCrop' is a good candidate to be a
> >    CameraConfiguration property, which at the moment is not yet
> >    implemented. Roughly speaking, we have been discussing it in the
> >    context of frame durations, and it's about augmenting the
> >    CameraConfiguration class with a set of properties that depend on
> >    the currently applied configuration of the enabled streams.
> >
> >    Ideally we should be able to receive CameraConfiguration, validate
> >    it, and populate it with properties, like the minimum frame
> >    duration (which is a combination of durations of all streams), and
> >    now with this new 'pipelineCrop' which is the result of a decision
> >    the pipeline handler takes based on the requested output stream
> >    sizes.
> >
> >    I think the ideal plan forward for this would be to define a new
> >    control/property (I would say property as it's immutable from
> >    applications) that is set by pipeline handlers during validate()
> >    when the sensor mode is selected and eventually cropped.
>
> Absolutely. The problem with this has never been how to calculate the
> "pipelineCrop", or who calculates it (that's the pipeline handler),
> but how to signal this value to the application. When can I have this
> feature, please? :)
>

I think we need to resume discussions on frame durations calculation,
or either decide that the proposed direction for 'pipelineCrop' is the
right one and:
1) Define the property
2) Add a ControlList of properties to CameraConfiguration
3) Instrument the pipeline handler to report it in validate()

It seems pretty straightforward to me, but I might be missing pieces.
Laurent, do you have opinions here ?

> >
> >    Application can access it from the CameraConfiguration and decide
> >    where to set their pan/zoom rectangle.
> >
> >    How the pipeline handler applies the cropping to the sensor
> >    produced frame is less relevant imo. Systems which have direct access
> >    to the v4l2 subdev will probably use the crop/compose rectangle
> >    applied on the sensor. In your case I see you use the crop target
> >    on the ISP video device node. Am I wrong or you could calculate
> >    that at validate() time, once you know all the desired output sizes and
> >    based on them identify the 'best' sensor mode to use ?
>
> We update the selection on the ISP device node. I would expect this
> approach to be more common than doing it on the sensor itself (you
> don't lose statistics for the rest of the frame), but it's of course
> up to the pipeline handlers.
>

You are right, this will mostly always come from ISP (sub)device node,
not from the sensor.

> >
> > 2) Application, once they know the 'pipelineCrop' (I'm ok with that
> >    term btw, as we have analogCrop and we will have a digitalCrop for
> >    the sensor's processing stages, so pipelineCrop feels right) can
> >    set their pan/zoom rectangle with a control, as the one you have
> >    defined below. Now, what do you think the reference of that rectangle
> >    should be ? I mean, should the 'pipelineCrop' always be in position
> >    (0,0) and the 'digitalZoom' be applied on top of it ? I would say
> >    so, but then we lose the information on which part of the image
> >    the pipeline decided to crop, and I'm not sure it is relevant or
> >    not.
>
> So I'm in favour of giving applications only the size of the pipeline
> crop, nothing else. It's all you need to implement digital zoom, and
> panning within the original image, and it makes life very
> straightforward for everyone.
>
> I agree there are more complex scenarios you could imagine. Maybe you
> let applications pan/zoom within the whole of the sensor area, so
> you'd be able to pan to parts of the image that you can't see in the
> "default unzoomed" version. I mean, there's nothing "wrong" in this,
> it just feels more complicated and a bit unexpected to me. I also
> think this would delegate most of the aspect ratio calculations to the
> applications, and I suspect they'd get themselves confused and
> actually implement the simpler scheme anyway... But yes, there is a
> decision to be made here.
>
> >
> >    The other possibility is to report the 'pipelineCrop' in respect to
> >    the full active pixel array size, and ask application to provide a
> >    'digitalZoom' rectangle with the same reference. Is it worth the
> >    additional complication in you opinion?
>
> Sorry, answered that one above!
>
> >
> >    Third option is to report the sensor frame size AND the pipeline crop
> >    defined in respect to it, and let application provide a digitalZoom
> >    rectangle defined in respect to the pipeline crop. This would look
> >    like (numbers are random):
> >         sensorFrameSize = {0, 0, 1920, 1440}
> >         pipelineCrop = { 0, 150, 1920, 1080}
> >         digitalZoom = {300, 300, 1280, 720}
> >
> >    This would allow application to know that the sensor frame is
> >    1920x1400, the pipeline selected a region on 1920x1080 by applying
> >    an offset of 150 pixels in the vertical direction and the desired
> >    digital zoom is then applied on this last rectangle (resulting in a
> >    effective (300,450) dislocation from the full sensor frame).
> >
> > I won't bikeshed on names too much, just leave as a reference that
> > Android reports the same property we are defining as pipelineCrop as
> > 'scalera.cropRegion' as for them 'scaler' is used to report properties
> > of the ISP processing pipeline.
>
> I could go with "scalerCrop" too!

Just for reference, I paste here the Android control documentation,
mostly to wrap our heads around the fact they decided to implement it
using the full pixel array as reference, but the documentation later
throws the sensor scaling in the mix, so I don't fully get it.

Reading it through might help spotting details I might have missed.

------------------------------------------------------------------------------
android.scaler.cropRegion

The desired region of the sensor to read out for this capture.

Pixel coordinates relative to android.sensor.info.activeArraySize or
android.sensor.info.preCorrectionActiveArraySize depending on
distortion correction capability and mode

This control can be used to implement digital zoom.

For devices not supporting android.distortionCorrection.mode control,
the coordinate system always follows that of
android.sensor.info.activeArraySize, with (0, 0) being the top-left
pixel of the active array.

For devices supporting android.distortionCorrection.mode control, the
coordinate system depends on the mode being set.When the distortion
correction mode is OFF, the coordinate system follows
android.sensor.info.preCorrectionActiveArraySize, with (0, 0) being
the top-left pixel of the pre-correction active array.When the
distortion correction mode is not OFF, the coordinate system follows
android.sensor.info.activeArraySize, with (0, 0) being the top-left
pixel of the active array.

Output streams use this rectangle to produce their output,cropping to
a smaller region if necessary to maintain the stream's aspect ratio,
then scaling the sensor input to match the output's configured
resolution.

The crop region is applied after the RAW to other color space (e.g.
YUV) conversion. Since raw streams (e.g. RAW16) don't have the
conversion stage, they are not croppable. The crop region will be
ignored by raw streams.

For non-raw streams, any additional per-stream cropping will be done
to maximize the final pixel area of the stream.

For example, if the crop region is set to a 4:3 aspect ratio, then 4:3
streams will use the exact crop region. 16:9 streams will further crop
vertically (letterbox).

Conversely, if the crop region is set to a 16:9, then 4:3 outputs will
crop horizontally (pillarbox), and 16:9 streams will match exactly.
These additional crops will be centered within the crop region.

If the coordinate system is android.sensor.info.activeArraySize, the
width and height of the crop region cannot be set to be smaller than
floor( activeArraySize.width / android.scaler.availableMaxDigitalZoom
) and floor( activeArraySize.height /
android.scaler.availableMaxDigitalZoom ), respectively.

If the coordinate system is
android.sensor.info.preCorrectionActiveArraySize, the width and height
of the crop region cannot be set to be smaller than floor(
preCorrectionActiveArraySize.width /
android.scaler.availableMaxDigitalZoom ) and floor(
preCorrectionActiveArraySize.height /
android.scaler.availableMaxDigitalZoom ),respectively.

The camera device may adjust the crop region to account for rounding
and other hardware requirements; the final crop region used will be
included in the output capture result.

HAL Implementation Details: The output streams must maintain square
pixels at all times, no matter what the relative aspect ratios of the
crop region and the stream are. Negative values for corner are allowed
for raw output if full pixel array is larger than active pixel array.
Width and height may be rounded to nearest larger supportable width,
especially for raw output, where only a few fixed scales may be
possible.

For a set of output streams configured, if the sensor output is
cropped to a smaller size than pre-correction active array size, the
HAL need follow below cropping rules:

The HAL need handle the cropRegion as if the sensor crop size is the
effective pre-correction active array size. More specifically, the HAL
must transform the request cropRegion from
android.sensor.info.preCorrectionActiveArraySize to the sensor cropped
pixel area size in this way:

Translate the requested cropRegion w.r.t., the left top corner of the
sensor cropped pixel area by (tx, ty),where ty = sensorCrop.top *
(sensorCrop.height / preCorrectionActiveArraySize.height) and tx =
sensorCrop.left * (sensorCrop.width /
preCorrectionActiveArraySize.width).The (sensorCrop.top,
sensorCrop.left) is the coordinate based off the
android.sensor.info.activeArraySize.  Scale the width and height of
requested cropRegion with scaling factor of
sensorCrop.width/preCorrectionActiveArraySize.width and
sensorCrop.height/preCorrectionActiveArraySize.height
respectively.Once this new cropRegion is calculated, the HAL must use
this region to crop the image with regard to the sensor crop size
(effective pre-correction active array size). The HAL still need
follow the general cropping rule for this new cropRegion and effective
pre-correction active array size.  The HAL must report the cropRegion
with regard to android.sensor.info.preCorrectionActiveArraySize.The
HAL need convert the new cropRegion generated above w.r.t., full
pre-correction active array size. The reported cropRegion may be
slightly different with the requested cropRegion since the HAL may
adjust the crop region to account for rounding, conversion error, or
other hardware limitations.
------------------------------------------------------------------------------

Thanks
  j


>
> Best regards
> David
>
> >
> > I'm sorry for the wall of text, I wish we had a blackboard :)
> >
> > Thanks
> >   j
> >
> > >
> > > Signed-off-by: David Plowman <david.plowman@raspberrypi.com>
> > > ---
> > >  include/libcamera/camera.h                    |  2 ++
> > >  include/libcamera/internal/pipeline_handler.h |  4 +++
> > >  src/libcamera/camera.cpp                      | 27 +++++++++++++++++++
> > >  src/libcamera/control_ids.yaml                | 10 +++++++
> > >  4 files changed, 43 insertions(+)
> > >
> > > diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h
> > > index 4d1a4a9..6819b8e 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 &getPipelineCrop() 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..5bfe890 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 &getPipelineCrop() const { return pipelineCrop_; }
> > > +
> > >  protected:
> > >       void registerCamera(std::shared_ptr<Camera> camera,
> > >                           std::unique_ptr<CameraData> data);
> > > @@ -100,6 +102,8 @@ protected:
> > >
> > >       CameraManager *manager_;
> > >
> > > +     Size pipelineCrop_;
> > > +
> > >  private:
> > >       void mediaDeviceDisconnected(MediaDevice *media);
> > >       virtual void disconnect();
> > > diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp
> > > index 69a1b44..f8b8ec6 100644
> > > --- a/src/libcamera/camera.cpp
> > > +++ b/src/libcamera/camera.cpp
> > > @@ -793,6 +793,33 @@ int Camera::configure(CameraConfiguration *config)
> > >       return 0;
> > >  }
> > >
> > > +/**
> > > + * \brief Return the size of the sensor image being used by the pipeline
> > > + * to create the output.
> > > + *
> > > + * This method returns the size, in pixels, of the raw image read from the
> > > + * sensor and which is used by the pipeline to form the output image(s)
> > > + * (rescaling if necessary). 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 might
> > > + * 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 used by the pipeline.
> > > + */
> > > +
> > > +Size const &Camera::getPipelineCrop() const
> > > +{
> > > +     return p_->pipe_->getPipelineCrop();
> > > +}
> > > +
> > >  /**
> > >   * \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 988b501..5a099d5 100644
> > > --- a/src/libcamera/control_ids.yaml
> > > +++ b/src/libcamera/control_ids.yaml
> > > @@ -262,4 +262,14 @@ controls:
> > >          In this respect, it is not necessarily aimed at providing a way to
> > >          implement a focus algorithm by the application, rather an indication of
> > >          how in-focus a frame is.
> > > +
> > > +  - 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.
> > >  ...
> > > --
> > > 2.20.1
> > >
> > > _______________________________________________
> > > libcamera-devel mailing list
> > > libcamera-devel@lists.libcamera.org
> > > https://lists.libcamera.org/listinfo/libcamera-devel

Hi Jacopo

Thanks for all the information, especially good to know what Android wants...

On Sat, 1 Aug 2020 at 10:26, Jacopo Mondi <jacopo@jmondi.org> wrote:
>
> Hi David,
>
> On Fri, Jul 31, 2020 at 10:02:44AM +0100, David Plowman wrote:
> > Hi Jacopo, everyone
> >
> > On Thu, 30 Jul 2020 at 17:34, Jacopo Mondi <jacopo@jmondi.org> wrote:
> > >
> > > Hi David,
> > >   sorry for jumping late on the topic, I managed to read the
> > > discussions in v1 and v2 only today.
> >
> > Please don't apologise, I'm very glad that you've picked this up. This
> > is actually one of the "big ticket" items that libcamera is currently
> > lacking compared to our existing stack, so I was going to give the
> > discussion a "nudge" quite soon!
> >
> > >
> > > On Thu, Jul 23, 2020 at 09:43:37AM +0100, David Plowman wrote:
> > > > These changes add a Digital Zoom control, taking a rectangle as its
> > > > argument, indicating the region of the sensor output that the
> > > > pipeline will "zoom up" to the final output size.
> > > >
> > > > Additionally, we need to have a method returning the "pipelineCrop"
> > > > which gives the dimensions of the sensor output, taken by the
> > > > pipeline, and within which we can subsequently pan and zoom.
> > >
> > > This is the part I don't get. What's the intended user of this
> > > information ? Is this meant to be provided to applications so that
> > > they know which is the area inside which they can later pan/zoom, right ?
> >
> > Correct. I'm imagining a user might say "I want to see the image at 2x
> > zoom". If we know the unzoomed image is being created by (as per one
> > of my examples somewhere) 2028x1140 pixels from the sensor, then the
> > application would send a "digital zoom" of offset: (507,285) size:
> > 1014x570,
> >
> > I think the feeling was that we liked having control right down to the
> > pixel level, which in turn means the the numbers 2028x1140 need to be
> > supplied to the application from somewhere.
> >
>
> Just for the sake of discussion, a 'digitalZoomFactor' (or whatever
> other name) control that takes a, in example, 2x value alongside with
> a more precies 'digitalZoomRectangle' that behaves like you have
> described, would make sense ?

Yes possibly, though I slightly get the feeling we'd be making more
work for ourselves... another control, pipeline handlers would all be
expected to do it, does it mean more metadata being returned... You
also have to define what it would do if someone has already panned
away from the centre etc. Maybe adding some kind of helper function
would be a good idea here? (e.g. give me the Rectangle I need for this
zoom factor, and then there's only one control).

>
> > >
> > > I'm kind of against to ad-hoc function to retrieve information about
> > > the image processing pipeline configuration. I can easily see them
> > > multiply and make our API awful, and each platform would need to add
> > > its own very-specific bits, and that can't certainly go through the
> > > main Camera class API.
> > >
> > > If I got your use case right:
> > >
> > > 1) You need to report the pipeline crop area, which is a rectangle
> > >    defined on the frame that is produced by the sensor.
> > >    Using your example:
> > >
> > >         sensorSize = 1920x1440
> > >         pipelineCrop = 1920x1080
> > >
> > >    It seems to me the 'pipelineCrop' is a good candidate to be a
> > >    CameraConfiguration property, which at the moment is not yet
> > >    implemented. Roughly speaking, we have been discussing it in the
> > >    context of frame durations, and it's about augmenting the
> > >    CameraConfiguration class with a set of properties that depend on
> > >    the currently applied configuration of the enabled streams.
> > >
> > >    Ideally we should be able to receive CameraConfiguration, validate
> > >    it, and populate it with properties, like the minimum frame
> > >    duration (which is a combination of durations of all streams), and
> > >    now with this new 'pipelineCrop' which is the result of a decision
> > >    the pipeline handler takes based on the requested output stream
> > >    sizes.
> > >
> > >    I think the ideal plan forward for this would be to define a new
> > >    control/property (I would say property as it's immutable from
> > >    applications) that is set by pipeline handlers during validate()
> > >    when the sensor mode is selected and eventually cropped.
> >
> > Absolutely. The problem with this has never been how to calculate the
> > "pipelineCrop", or who calculates it (that's the pipeline handler),
> > but how to signal this value to the application. When can I have this
> > feature, please? :)
> >
>
> I think we need to resume discussions on frame durations calculation,
> or either decide that the proposed direction for 'pipelineCrop' is the
> right one and:
> 1) Define the property
> 2) Add a ControlList of properties to CameraConfiguration
> 3) Instrument the pipeline handler to report it in validate()
>
> It seems pretty straightforward to me, but I might be missing pieces.
> Laurent, do you have opinions here ?
>
> > >
> > >    Application can access it from the CameraConfiguration and decide
> > >    where to set their pan/zoom rectangle.
> > >
> > >    How the pipeline handler applies the cropping to the sensor
> > >    produced frame is less relevant imo. Systems which have direct access
> > >    to the v4l2 subdev will probably use the crop/compose rectangle
> > >    applied on the sensor. In your case I see you use the crop target
> > >    on the ISP video device node. Am I wrong or you could calculate
> > >    that at validate() time, once you know all the desired output sizes and
> > >    based on them identify the 'best' sensor mode to use ?
> >
> > We update the selection on the ISP device node. I would expect this
> > approach to be more common than doing it on the sensor itself (you
> > don't lose statistics for the rest of the frame), but it's of course
> > up to the pipeline handlers.
> >
>
> You are right, this will mostly always come from ISP (sub)device node,
> not from the sensor.
>
> > >
> > > 2) Application, once they know the 'pipelineCrop' (I'm ok with that
> > >    term btw, as we have analogCrop and we will have a digitalCrop for
> > >    the sensor's processing stages, so pipelineCrop feels right) can
> > >    set their pan/zoom rectangle with a control, as the one you have
> > >    defined below. Now, what do you think the reference of that rectangle
> > >    should be ? I mean, should the 'pipelineCrop' always be in position
> > >    (0,0) and the 'digitalZoom' be applied on top of it ? I would say
> > >    so, but then we lose the information on which part of the image
> > >    the pipeline decided to crop, and I'm not sure it is relevant or
> > >    not.
> >
> > So I'm in favour of giving applications only the size of the pipeline
> > crop, nothing else. It's all you need to implement digital zoom, and
> > panning within the original image, and it makes life very
> > straightforward for everyone.
> >
> > I agree there are more complex scenarios you could imagine. Maybe you
> > let applications pan/zoom within the whole of the sensor area, so
> > you'd be able to pan to parts of the image that you can't see in the
> > "default unzoomed" version. I mean, there's nothing "wrong" in this,
> > it just feels more complicated and a bit unexpected to me. I also
> > think this would delegate most of the aspect ratio calculations to the
> > applications, and I suspect they'd get themselves confused and
> > actually implement the simpler scheme anyway... But yes, there is a
> > decision to be made here.
> >
> > >
> > >    The other possibility is to report the 'pipelineCrop' in respect to
> > >    the full active pixel array size, and ask application to provide a
> > >    'digitalZoom' rectangle with the same reference. Is it worth the
> > >    additional complication in you opinion?
> >
> > Sorry, answered that one above!
> >
> > >
> > >    Third option is to report the sensor frame size AND the pipeline crop
> > >    defined in respect to it, and let application provide a digitalZoom
> > >    rectangle defined in respect to the pipeline crop. This would look
> > >    like (numbers are random):
> > >         sensorFrameSize = {0, 0, 1920, 1440}
> > >         pipelineCrop = { 0, 150, 1920, 1080}
> > >         digitalZoom = {300, 300, 1280, 720}
> > >
> > >    This would allow application to know that the sensor frame is
> > >    1920x1400, the pipeline selected a region on 1920x1080 by applying
> > >    an offset of 150 pixels in the vertical direction and the desired
> > >    digital zoom is then applied on this last rectangle (resulting in a
> > >    effective (300,450) dislocation from the full sensor frame).
> > >
> > > I won't bikeshed on names too much, just leave as a reference that
> > > Android reports the same property we are defining as pipelineCrop as
> > > 'scalera.cropRegion' as for them 'scaler' is used to report properties
> > > of the ISP processing pipeline.
> >
> > I could go with "scalerCrop" too!
>
> Just for reference, I paste here the Android control documentation,
> mostly to wrap our heads around the fact they decided to implement it
> using the full pixel array as reference, but the documentation later
> throws the sensor scaling in the mix, so I don't fully get it.
>
> Reading it through might help spotting details I might have missed.

This is indeed different. They're letting you pan anywhere that the
sensor will let you, and then they're sorting out the aspect ratio
issues for you, it seems.

Panning anywhere that the sensor lets you is rather going away from
what I was proposing. It means they'll get the behaviour that I
thought was a bit weird (zooming in more will reveal previously
invisible parts of the image). But they take care of the aspect ratio
calculations for the user, though that presumably means you couldn't
ask for a strange aspect ratio even if you wanted to.

Perhaps the answer is like the one I gave earlier. We should remain as
flexible as possible (weird behaviours and all) and provide helper
functions to make it easy for applications to implement the behaviours
they want. So that would mean:

1. The "pipeline crop" disappears and we use the total image size that
is available from the sensor. That is, the size of the sensor output
in actual pixels, taking account of any scaling/binning/cropping
performed *within* the sensor. I think this might already be available
somewhere? (If the pipeline handler doesn't want to use the full
sensor area for some reason, then the pipeline handler can re-scale
internally, though you might not have quite the same pixel-level
control as you thought)

2. We take an arbitrary rectangle supplied by the application and
simply use it to specify the pan/zoom without altering it. The extra
munging that Android would do can be performed in the
libcamera/Android interface layer.

3. Then there's a helper function taking that sensor output size from
1, the desired pan/zoom parameters (2), and called
getMeTheSensibleZoomRectanglePlease (OK, we might change that name).

How does that sound?
David

>
> ------------------------------------------------------------------------------
> android.scaler.cropRegion
>
> The desired region of the sensor to read out for this capture.
>
> Pixel coordinates relative to android.sensor.info.activeArraySize or
> android.sensor.info.preCorrectionActiveArraySize depending on
> distortion correction capability and mode
>
> This control can be used to implement digital zoom.
>
> For devices not supporting android.distortionCorrection.mode control,
> the coordinate system always follows that of
> android.sensor.info.activeArraySize, with (0, 0) being the top-left
> pixel of the active array.
>
> For devices supporting android.distortionCorrection.mode control, the
> coordinate system depends on the mode being set.When the distortion
> correction mode is OFF, the coordinate system follows
> android.sensor.info.preCorrectionActiveArraySize, with (0, 0) being
> the top-left pixel of the pre-correction active array.When the
> distortion correction mode is not OFF, the coordinate system follows
> android.sensor.info.activeArraySize, with (0, 0) being the top-left
> pixel of the active array.
>
> Output streams use this rectangle to produce their output,cropping to
> a smaller region if necessary to maintain the stream's aspect ratio,
> then scaling the sensor input to match the output's configured
> resolution.
>
> The crop region is applied after the RAW to other color space (e.g.
> YUV) conversion. Since raw streams (e.g. RAW16) don't have the
> conversion stage, they are not croppable. The crop region will be
> ignored by raw streams.
>
> For non-raw streams, any additional per-stream cropping will be done
> to maximize the final pixel area of the stream.
>
> For example, if the crop region is set to a 4:3 aspect ratio, then 4:3
> streams will use the exact crop region. 16:9 streams will further crop
> vertically (letterbox).
>
> Conversely, if the crop region is set to a 16:9, then 4:3 outputs will
> crop horizontally (pillarbox), and 16:9 streams will match exactly.
> These additional crops will be centered within the crop region.
>
> If the coordinate system is android.sensor.info.activeArraySize, the
> width and height of the crop region cannot be set to be smaller than
> floor( activeArraySize.width / android.scaler.availableMaxDigitalZoom
> ) and floor( activeArraySize.height /
> android.scaler.availableMaxDigitalZoom ), respectively.
>
> If the coordinate system is
> android.sensor.info.preCorrectionActiveArraySize, the width and height
> of the crop region cannot be set to be smaller than floor(
> preCorrectionActiveArraySize.width /
> android.scaler.availableMaxDigitalZoom ) and floor(
> preCorrectionActiveArraySize.height /
> android.scaler.availableMaxDigitalZoom ),respectively.
>
> The camera device may adjust the crop region to account for rounding
> and other hardware requirements; the final crop region used will be
> included in the output capture result.
>
> HAL Implementation Details: The output streams must maintain square
> pixels at all times, no matter what the relative aspect ratios of the
> crop region and the stream are. Negative values for corner are allowed
> for raw output if full pixel array is larger than active pixel array.
> Width and height may be rounded to nearest larger supportable width,
> especially for raw output, where only a few fixed scales may be
> possible.
>
> For a set of output streams configured, if the sensor output is
> cropped to a smaller size than pre-correction active array size, the
> HAL need follow below cropping rules:
>
> The HAL need handle the cropRegion as if the sensor crop size is the
> effective pre-correction active array size. More specifically, the HAL
> must transform the request cropRegion from
> android.sensor.info.preCorrectionActiveArraySize to the sensor cropped
> pixel area size in this way:
>
> Translate the requested cropRegion w.r.t., the left top corner of the
> sensor cropped pixel area by (tx, ty),where ty = sensorCrop.top *
> (sensorCrop.height / preCorrectionActiveArraySize.height) and tx =
> sensorCrop.left * (sensorCrop.width /
> preCorrectionActiveArraySize.width).The (sensorCrop.top,
> sensorCrop.left) is the coordinate based off the
> android.sensor.info.activeArraySize.  Scale the width and height of
> requested cropRegion with scaling factor of
> sensorCrop.width/preCorrectionActiveArraySize.width and
> sensorCrop.height/preCorrectionActiveArraySize.height
> respectively.Once this new cropRegion is calculated, the HAL must use
> this region to crop the image with regard to the sensor crop size
> (effective pre-correction active array size). The HAL still need
> follow the general cropping rule for this new cropRegion and effective
> pre-correction active array size.  The HAL must report the cropRegion
> with regard to android.sensor.info.preCorrectionActiveArraySize.The
> HAL need convert the new cropRegion generated above w.r.t., full
> pre-correction active array size. The reported cropRegion may be
> slightly different with the requested cropRegion since the HAL may
> adjust the crop region to account for rounding, conversion error, or
> other hardware limitations.
> ------------------------------------------------------------------------------
>
> Thanks
>   j
>
>
> >
> > Best regards
> > David
> >
> > >
> > > I'm sorry for the wall of text, I wish we had a blackboard :)
> > >
> > > Thanks
> > >   j
> > >
> > > >
> > > > Signed-off-by: David Plowman <david.plowman@raspberrypi.com>
> > > > ---
> > > >  include/libcamera/camera.h                    |  2 ++
> > > >  include/libcamera/internal/pipeline_handler.h |  4 +++
> > > >  src/libcamera/camera.cpp                      | 27 +++++++++++++++++++
> > > >  src/libcamera/control_ids.yaml                | 10 +++++++
> > > >  4 files changed, 43 insertions(+)
> > > >
> > > > diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h
> > > > index 4d1a4a9..6819b8e 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 &getPipelineCrop() 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..5bfe890 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 &getPipelineCrop() const { return pipelineCrop_; }
> > > > +
> > > >  protected:
> > > >       void registerCamera(std::shared_ptr<Camera> camera,
> > > >                           std::unique_ptr<CameraData> data);
> > > > @@ -100,6 +102,8 @@ protected:
> > > >
> > > >       CameraManager *manager_;
> > > >
> > > > +     Size pipelineCrop_;
> > > > +
> > > >  private:
> > > >       void mediaDeviceDisconnected(MediaDevice *media);
> > > >       virtual void disconnect();
> > > > diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp
> > > > index 69a1b44..f8b8ec6 100644
> > > > --- a/src/libcamera/camera.cpp
> > > > +++ b/src/libcamera/camera.cpp
> > > > @@ -793,6 +793,33 @@ int Camera::configure(CameraConfiguration *config)
> > > >       return 0;
> > > >  }
> > > >
> > > > +/**
> > > > + * \brief Return the size of the sensor image being used by the pipeline
> > > > + * to create the output.
> > > > + *
> > > > + * This method returns the size, in pixels, of the raw image read from the
> > > > + * sensor and which is used by the pipeline to form the output image(s)
> > > > + * (rescaling if necessary). 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 might
> > > > + * 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 used by the pipeline.
> > > > + */
> > > > +
> > > > +Size const &Camera::getPipelineCrop() const
> > > > +{
> > > > +     return p_->pipe_->getPipelineCrop();
> > > > +}
> > > > +
> > > >  /**
> > > >   * \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 988b501..5a099d5 100644
> > > > --- a/src/libcamera/control_ids.yaml
> > > > +++ b/src/libcamera/control_ids.yaml
> > > > @@ -262,4 +262,14 @@ controls:
> > > >          In this respect, it is not necessarily aimed at providing a way to
> > > >          implement a focus algorithm by the application, rather an indication of
> > > >          how in-focus a frame is.
> > > > +
> > > > +  - 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.
> > > >  ...
> > > > --
> > > > 2.20.1
> > > >
> > > > _______________________________________________
> > > > libcamera-devel mailing list
> > > > libcamera-devel@lists.libcamera.org
> > > > https://lists.libcamera.org/listinfo/libcamera-devel