[libcamera-devel,v5,0/5] Digital zoom

10213 mbox series

Hi everyone

Thanks for the reviews and comments, latest version of digital zoom
patches attached. I reordered the first couple because the ScalerCrop
control seems more fundamental now, so it seemed slightly better
before the ScalerCropMaximum one. Don't think it matters much.

The main differences are:

1 and 2. These are now the control then the property, as described.

3. Initialisation of the ScalerCropMaximum property is now in the RPi
pipeline handler, and to zero values (isNull is our friend).

4. Geometry helpers - a bit more renaming, as usual. I found I had to
keep some of the casts because widths and heights tend to be unsigned
ints (not just ints). There's a Rectangle::topLeft() function and you
now translate by a Point (I'll live with my discomfort on that one).

That Rectangle scaling function is now described as a "non-uniform
scaling", I think that's something familiar to most people. I agree it
was confusing before (I'd described it by the way I used it, rather
than what it was), but I think a non-unifrom scaling is clearer than a
homothety (actually I had some difficulties with that too).

5. RPi implementation. I fill in the ScalerCrop in the metadata
directly now, when the metadata returns from the IPA (which just
ignores it).

I thought I'd get this lot out there first. In the meantime I'll work
on some geometry tests and include them either in the next round or as
a separate patch.

Thanks and best regards
David

David Plowman (5):
  libcamera: Add ScalerCrop control
  libcamera: Add SensorCropMaximum property
  libcamera: raspberrypi: Initialise the SensorCropMaximum property
  libcamera: Add geometry helper functions
  libcamera: pipeline: raspberrypi: Implementation of digital zoom

 include/libcamera/geometry.h                  |  52 +++
 include/libcamera/ipa/raspberrypi.h           |   1 +
 src/ipa/raspberrypi/raspberrypi.cpp           |   5 +
 src/libcamera/control_ids.yaml                |  13 +
 src/libcamera/geometry.cpp                    | 307 ++++++++++++++++++
 .../pipeline/raspberrypi/raspberrypi.cpp      |  89 ++++-
 src/libcamera/property_ids.yaml               |  14 +
 7 files changed, 465 insertions(+), 16 deletions(-)

Hi David,

On Fri, Oct 23, 2020 at 11:21:54AM +0100, David Plowman wrote:
> Hi everyone
> 
> Thanks for the reviews and comments, latest version of digital zoom
> patches attached. I reordered the first couple because the ScalerCrop
> control seems more fundamental now, so it seemed slightly better
> before the ScalerCropMaximum one. Don't think it matters much.
> 
> The main differences are:
> 
> 1 and 2. These are now the control then the property, as described.
> 
> 3. Initialisation of the ScalerCropMaximum property is now in the RPi
> pipeline handler, and to zero values (isNull is our friend).
> 
> 4. Geometry helpers - a bit more renaming, as usual. I found I had to
> keep some of the casts because widths and heights tend to be unsigned
> ints (not just ints). There's a Rectangle::topLeft() function and you
> now translate by a Point (I'll live with my discomfort on that one).
> 
> That Rectangle scaling function is now described as a "non-uniform
> scaling", I think that's something familiar to most people. I agree it
> was confusing before (I'd described it by the way I used it, rather
> than what it was), but I think a non-unifrom scaling is clearer than a
> homothety (actually I had some difficulties with that too).
> 
> 5. RPi implementation. I fill in the ScalerCrop in the metadata
> directly now, when the metadata returns from the IPA (which just
> ignores it).

All this looks really good to me ! I've reviewed all patches, there are
a fair number of comments, but they're getting really minor. I think the
next iteration could be the final one.

> I thought I'd get this lot out there first. In the meantime I'll work
> on some geometry tests and include them either in the next round or as
> a separate patch.

Thank you for that. The tests should be split to a separate patch, but
if it can be included in the series, that would be best.

> David Plowman (5):
>   libcamera: Add ScalerCrop control
>   libcamera: Add SensorCropMaximum property
>   libcamera: raspberrypi: Initialise the SensorCropMaximum property
>   libcamera: Add geometry helper functions
>   libcamera: pipeline: raspberrypi: Implementation of digital zoom
> 
>  include/libcamera/geometry.h                  |  52 +++
>  include/libcamera/ipa/raspberrypi.h           |   1 +
>  src/ipa/raspberrypi/raspberrypi.cpp           |   5 +
>  src/libcamera/control_ids.yaml                |  13 +
>  src/libcamera/geometry.cpp                    | 307 ++++++++++++++++++
>  .../pipeline/raspberrypi/raspberrypi.cpp      |  89 ++++-
>  src/libcamera/property_ids.yaml               |  14 +
>  7 files changed, 465 insertions(+), 16 deletions(-)