[libcamera-devel,v2,2/2] libcamera: raspberrypi: Implement digital zoom
diff mbox series

Message ID 20200709091555.1617-3-david.plowman@raspberrypi.com
State Superseded
Headers show
Series
  • Digital zoom implementation
Related show

Commit Message

David Plowman July 9, 2020, 9:15 a.m. UTC 
These changes implement digital zoom for the Raspberry Pi. We detect
the digital zoom control and update the V4L2 "selection" before
starting the ISP. We also update the value in the control that we send
to the IPA, so that it has the correct value.
---
 include/libcamera/ipa/raspberrypi.h           |  1 +
 src/ipa/raspberrypi/raspberrypi.cpp           | 10 ++++
 .../pipeline/raspberrypi/raspberrypi.cpp      | 56 ++++++++++++++++++-
 3 files changed, 66 insertions(+), 1 deletion(-)

Comments

Kieran Bingham July 10, 2020, 1:40 p.m. UTC | #1 
Hi David,

I've just been chatting briefly with Laurent and he has mentioned that
this topic should be integrated with the work that Jacopo has done on
Pixel Array properties.

I haven't looked at that work yet, so I'm not (yet) in a position to
comment on that directly, but as I'd already written some generic
(libcamera) framework comments about the Rectangle class below, I
thought I should send this now...



On 09/07/2020 10:15, David Plowman wrote:
> These changes implement digital zoom for the Raspberry Pi. We detect
> the digital zoom control and update the V4L2 "selection" before
> starting the ISP. We also update the value in the control that we send
> to the IPA, so that it has the correct value.
> ---
>  include/libcamera/ipa/raspberrypi.h           |  1 +
>  src/ipa/raspberrypi/raspberrypi.cpp           | 10 ++++
>  .../pipeline/raspberrypi/raspberrypi.cpp      | 56 ++++++++++++++++++-
>  3 files changed, 66 insertions(+), 1 deletion(-)
> 
> diff --git a/include/libcamera/ipa/raspberrypi.h b/include/libcamera/ipa/raspberrypi.h
> index a18ce9a..e66402e 100644
> --- a/include/libcamera/ipa/raspberrypi.h
> +++ b/include/libcamera/ipa/raspberrypi.h
> @@ -52,6 +52,7 @@ static const ControlInfoMap RPiControls = {
>  	{ &controls::Contrast, ControlInfo(0.0f, 32.0f) },
>  	{ &controls::Saturation, ControlInfo(0.0f, 32.0f) },
>  	{ &controls::Sharpness, ControlInfo(0.0f, 16.0f, 1.0f) },
> +	{ &controls::DigitalZoom, ControlInfo(Rectangle{ 0, 0, 0, 0 }, Rectangle{ 65535, 65535, 65535, 65535 }, Rectangle{ 0, 0, 0, 0 }) },

I wonder if we should have some constructor helpers to make that easier.
   (Rectangle(0), Rectangle(65535), Rectangle(0))


Oddly, (and this is not a fault of this patch), the idea of a Rectangle
which starts at x,y = {65535,65535}, and is of size {65535,65535} seems
wrong, but of course this is jsut storage of maximum values.

I can't currently think of a better way of storing that, and of course
the control has to store the max value like that anyway, so it's not an
issue I don't think  - just a quirk we might have to be aware of.



>  };
>  
>  } /* namespace libcamera */
> diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp
> index b1f2786..3c68078 100644
> --- a/src/ipa/raspberrypi/raspberrypi.cpp
> +++ b/src/ipa/raspberrypi/raspberrypi.cpp
> @@ -658,6 +658,16 @@ void IPARPi::queueRequest(const ControlList &controls)
>  			break;
>  		}
>  
> +		case controls::DIGITAL_ZOOM: {
> +			/*
> +			 * We send the zoom info back in the metadata where the pipeline
> +			 * handler will update it to the values actually used.
> +			 */
> +			Rectangle crop = ctrl.second.get<Rectangle>();
> +			libcameraMetadata_.set(controls::DigitalZoom, crop);
> +			break;
> +		}
> +
>  		default:
>  			LOG(IPARPI, Warning)
>  				<< "Ctrl " << controls::controls.at(ctrl.first)->name()
> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> index f4966f8..55db11d 100644
> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> @@ -287,7 +287,8 @@ class RPiCameraData : public CameraData
>  public:
>  	RPiCameraData(PipelineHandler *pipe)
>  		: CameraData(pipe), sensor_(nullptr), lsTable_(nullptr),
> -		  state_(State::Stopped), dropFrame_(false), ispOutputCount_(0)
> +		  state_(State::Stopped), dropFrame_(false), ispOutputCount_(0),
> +		  ispMinSize_(0)
>  	{
>  	}
>  
> @@ -322,6 +323,14 @@ public:
>  	void handleStreamBuffer(FrameBuffer *buffer, const RPiStream *stream);
>  	void handleState();
>  
> +	void initSensorCrop(Rectangle const &crop, unsigned int ispMinSize)
> +	{
> +		/* The initial zoom region is the whole of the sensorCrop_. */
> +		sensorCrop_ = crop;
> +		zoomRect_ = Rectangle{ 0, 0, crop.width, crop.height };
> +		ispMinSize_ = ispMinSize;
> +	}
> +
>  	CameraSensor *sensor_;
>  	/* Array of Unicam and ISP device streams and associated buffers/streams. */
>  	RPiDevice<Unicam, 2> unicam_;
> @@ -358,6 +367,15 @@ private:
>  
>  	bool dropFrame_;
>  	int ispOutputCount_;
> +	/*
> +	 * sensorCrop_ is the largest region of the full sensor output that matches
> +	 * the desired aspect ratio, and therefore represents the area within
> +	 * which we can pan and zoom. zoomRect_ is the portion from within the
> +	 * sensorCrop_ that pan/zoom is currently using.
> +	 */
> +	Rectangle sensorCrop_;
> +	Rectangle zoomRect_;
> +	unsigned int ispMinSize_;
>  };
>  
>  class RPiCameraConfiguration : public CameraConfiguration
> @@ -744,6 +762,10 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config)
>  		return ret;
>  	}
>  
> +	Rectangle testCrop = { 0, 0, 1, 1 };
> +	data->isp_[Isp::Input].dev()->setSelection(V4L2_SEL_TGT_CROP, &testCrop);
> +	const unsigned int ispMinSize = testCrop.width;
> +
>  	/* Adjust aspect ratio by providing crops on the input image. */
>  	Rectangle crop = {
>  		.x = 0,
> @@ -763,8 +785,12 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config)
>  
>  	crop.x = (sensorFormat.size.width - crop.width) >> 1;
>  	crop.y = (sensorFormat.size.height - crop.height) >> 1;
> +
>  	data->isp_[Isp::Input].dev()->setSelection(V4L2_SEL_TGT_CROP, &crop);
>  
> +	sensorCrop_ = Size(crop.width, crop.height);
> +	data->initSensorCrop(crop, ispMinSize);
> +
>  	ret = configureIPA(camera);
>  	if (ret)
>  		LOG(RPI, Error) << "Failed to configure the IPA: " << ret;
> @@ -1553,6 +1579,34 @@ void RPiCameraData::tryRunPipeline()
>  	 */
>  	Request *request = requestQueue_.front();
>  
> +	if (request->controls().contains(controls::DigitalZoom)) {
> +		Rectangle rect = request->controls().get<Rectangle>(controls::DigitalZoom);
> +		/*
> +		 * If a new digital zoom value was given, check that it lies within the
> +		 * available sensorCrop_, coercing it if necessary.
> +		 */
> +		if (rect.width && rect.height) {

Laurent has very recently added a helper for the Size class as isNull.

I suspect we could add in a Rectangle.isNull() to make that statement
		if (!rect.isNull())

Of course, there could be intricacies of is a rectangle null only if the
width/height are 0, but position could be set or does the position have
to also be 0,0...



> +			zoomRect_ = rect;
> +			if (zoomRect_.width < ispMinSize_)
> +				zoomRect_.width = ispMinSize_;
> +			else if (zoomRect_.width > sensorCrop_.width)
> +				zoomRect_.width = sensorCrop_.width;
> +			if (zoomRect_.height < ispMinSize_)
> +				zoomRect_.height = ispMinSize_;
> +			else if (zoomRect_.height > sensorCrop_.height)
> +				zoomRect_.height = sensorCrop_.height;
> +			if (zoomRect_.x + zoomRect_.width > sensorCrop_.width)
> +				zoomRect_.x = sensorCrop_.width - zoomRect_.width;
> +			if (zoomRect_.y + zoomRect_.height > sensorCrop_.height)
> +				zoomRect_.y = sensorCrop_.height - zoomRect_.height;

Ayeee, and that is begging for some rectangle .clip(rect2) or
.clamp(rect2) helpers to be added to include/libcamera/geometry.h ...


> +		}
> +		Rectangle sensorRect = zoomRect_;
> +		sensorRect.x += sensorCrop_.x;
> +		sensorRect.y += sensorCrop_.y;
> +		isp_[Isp::Input].dev()->setSelection(V4L2_SEL_TGT_CROP, &sensorRect);
> +		request->controls().set(controls::DigitalZoom, zoomRect_);
> +	}
> +
>  	/*
>  	 * Process all the user controls by the IPA. Once this is complete, we
>  	 * queue the ISP output buffer listed in the request to start the HW
>
David Plowman July 21, 2020, 10:35 a.m. UTC | #2 
Hi Kieran

Thanks for the Rectangle constructors that I've seen appear in the
code, that's obviously helpful. Also the suggestion on adding a clip
function to the Rectangle class is clearly sensible - so I'll include
that in the next version of patches!

Best regards
David

On Fri, 10 Jul 2020 at 14:40, Kieran Bingham
<kieran.bingham@ideasonboard.com> wrote:
>
> Hi David,
>
> I've just been chatting briefly with Laurent and he has mentioned that
> this topic should be integrated with the work that Jacopo has done on
> Pixel Array properties.
>
> I haven't looked at that work yet, so I'm not (yet) in a position to
> comment on that directly, but as I'd already written some generic
> (libcamera) framework comments about the Rectangle class below, I
> thought I should send this now...
>
>
>
> On 09/07/2020 10:15, David Plowman wrote:
> > These changes implement digital zoom for the Raspberry Pi. We detect
> > the digital zoom control and update the V4L2 "selection" before
> > starting the ISP. We also update the value in the control that we send
> > to the IPA, so that it has the correct value.
> > ---
> >  include/libcamera/ipa/raspberrypi.h           |  1 +
> >  src/ipa/raspberrypi/raspberrypi.cpp           | 10 ++++
> >  .../pipeline/raspberrypi/raspberrypi.cpp      | 56 ++++++++++++++++++-
> >  3 files changed, 66 insertions(+), 1 deletion(-)
> >
> > diff --git a/include/libcamera/ipa/raspberrypi.h b/include/libcamera/ipa/raspberrypi.h
> > index a18ce9a..e66402e 100644
> > --- a/include/libcamera/ipa/raspberrypi.h
> > +++ b/include/libcamera/ipa/raspberrypi.h
> > @@ -52,6 +52,7 @@ static const ControlInfoMap RPiControls = {
> >       { &controls::Contrast, ControlInfo(0.0f, 32.0f) },
> >       { &controls::Saturation, ControlInfo(0.0f, 32.0f) },
> >       { &controls::Sharpness, ControlInfo(0.0f, 16.0f, 1.0f) },
> > +     { &controls::DigitalZoom, ControlInfo(Rectangle{ 0, 0, 0, 0 }, Rectangle{ 65535, 65535, 65535, 65535 }, Rectangle{ 0, 0, 0, 0 }) },
>
> I wonder if we should have some constructor helpers to make that easier.
>    (Rectangle(0), Rectangle(65535), Rectangle(0))
>
>
> Oddly, (and this is not a fault of this patch), the idea of a Rectangle
> which starts at x,y = {65535,65535}, and is of size {65535,65535} seems
> wrong, but of course this is jsut storage of maximum values.
>
> I can't currently think of a better way of storing that, and of course
> the control has to store the max value like that anyway, so it's not an
> issue I don't think  - just a quirk we might have to be aware of.
>
>
>
> >  };
> >
> >  } /* namespace libcamera */
> > diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp
> > index b1f2786..3c68078 100644
> > --- a/src/ipa/raspberrypi/raspberrypi.cpp
> > +++ b/src/ipa/raspberrypi/raspberrypi.cpp
> > @@ -658,6 +658,16 @@ void IPARPi::queueRequest(const ControlList &controls)
> >                       break;
> >               }
> >
> > +             case controls::DIGITAL_ZOOM: {
> > +                     /*
> > +                      * We send the zoom info back in the metadata where the pipeline
> > +                      * handler will update it to the values actually used.
> > +                      */
> > +                     Rectangle crop = ctrl.second.get<Rectangle>();
> > +                     libcameraMetadata_.set(controls::DigitalZoom, crop);
> > +                     break;
> > +             }
> > +
> >               default:
> >                       LOG(IPARPI, Warning)
> >                               << "Ctrl " << controls::controls.at(ctrl.first)->name()
> > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > index f4966f8..55db11d 100644
> > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > @@ -287,7 +287,8 @@ class RPiCameraData : public CameraData
> >  public:
> >       RPiCameraData(PipelineHandler *pipe)
> >               : CameraData(pipe), sensor_(nullptr), lsTable_(nullptr),
> > -               state_(State::Stopped), dropFrame_(false), ispOutputCount_(0)
> > +               state_(State::Stopped), dropFrame_(false), ispOutputCount_(0),
> > +               ispMinSize_(0)
> >       {
> >       }
> >
> > @@ -322,6 +323,14 @@ public:
> >       void handleStreamBuffer(FrameBuffer *buffer, const RPiStream *stream);
> >       void handleState();
> >
> > +     void initSensorCrop(Rectangle const &crop, unsigned int ispMinSize)
> > +     {
> > +             /* The initial zoom region is the whole of the sensorCrop_. */
> > +             sensorCrop_ = crop;
> > +             zoomRect_ = Rectangle{ 0, 0, crop.width, crop.height };
> > +             ispMinSize_ = ispMinSize;
> > +     }
> > +
> >       CameraSensor *sensor_;
> >       /* Array of Unicam and ISP device streams and associated buffers/streams. */
> >       RPiDevice<Unicam, 2> unicam_;
> > @@ -358,6 +367,15 @@ private:
> >
> >       bool dropFrame_;
> >       int ispOutputCount_;
> > +     /*
> > +      * sensorCrop_ is the largest region of the full sensor output that matches
> > +      * the desired aspect ratio, and therefore represents the area within
> > +      * which we can pan and zoom. zoomRect_ is the portion from within the
> > +      * sensorCrop_ that pan/zoom is currently using.
> > +      */
> > +     Rectangle sensorCrop_;
> > +     Rectangle zoomRect_;
> > +     unsigned int ispMinSize_;
> >  };
> >
> >  class RPiCameraConfiguration : public CameraConfiguration
> > @@ -744,6 +762,10 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config)
> >               return ret;
> >       }
> >
> > +     Rectangle testCrop = { 0, 0, 1, 1 };
> > +     data->isp_[Isp::Input].dev()->setSelection(V4L2_SEL_TGT_CROP, &testCrop);
> > +     const unsigned int ispMinSize = testCrop.width;
> > +
> >       /* Adjust aspect ratio by providing crops on the input image. */
> >       Rectangle crop = {
> >               .x = 0,
> > @@ -763,8 +785,12 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config)
> >
> >       crop.x = (sensorFormat.size.width - crop.width) >> 1;
> >       crop.y = (sensorFormat.size.height - crop.height) >> 1;
> > +
> >       data->isp_[Isp::Input].dev()->setSelection(V4L2_SEL_TGT_CROP, &crop);
> >
> > +     sensorCrop_ = Size(crop.width, crop.height);
> > +     data->initSensorCrop(crop, ispMinSize);
> > +
> >       ret = configureIPA(camera);
> >       if (ret)
> >               LOG(RPI, Error) << "Failed to configure the IPA: " << ret;
> > @@ -1553,6 +1579,34 @@ void RPiCameraData::tryRunPipeline()
> >        */
> >       Request *request = requestQueue_.front();
> >
> > +     if (request->controls().contains(controls::DigitalZoom)) {
> > +             Rectangle rect = request->controls().get<Rectangle>(controls::DigitalZoom);
> > +             /*
> > +              * If a new digital zoom value was given, check that it lies within the
> > +              * available sensorCrop_, coercing it if necessary.
> > +              */
> > +             if (rect.width && rect.height) {
>
> Laurent has very recently added a helper for the Size class as isNull.
>
> I suspect we could add in a Rectangle.isNull() to make that statement
>                 if (!rect.isNull())
>
> Of course, there could be intricacies of is a rectangle null only if the
> width/height are 0, but position could be set or does the position have
> to also be 0,0...
>
>
>
> > +                     zoomRect_ = rect;
> > +                     if (zoomRect_.width < ispMinSize_)
> > +                             zoomRect_.width = ispMinSize_;
> > +                     else if (zoomRect_.width > sensorCrop_.width)
> > +                             zoomRect_.width = sensorCrop_.width;
> > +                     if (zoomRect_.height < ispMinSize_)
> > +                             zoomRect_.height = ispMinSize_;
> > +                     else if (zoomRect_.height > sensorCrop_.height)
> > +                             zoomRect_.height = sensorCrop_.height;
> > +                     if (zoomRect_.x + zoomRect_.width > sensorCrop_.width)
> > +                             zoomRect_.x = sensorCrop_.width - zoomRect_.width;
> > +                     if (zoomRect_.y + zoomRect_.height > sensorCrop_.height)
> > +                             zoomRect_.y = sensorCrop_.height - zoomRect_.height;
>
> Ayeee, and that is begging for some rectangle .clip(rect2) or
> .clamp(rect2) helpers to be added to include/libcamera/geometry.h ...
>
>
> > +             }
> > +             Rectangle sensorRect = zoomRect_;
> > +             sensorRect.x += sensorCrop_.x;
> > +             sensorRect.y += sensorCrop_.y;
> > +             isp_[Isp::Input].dev()->setSelection(V4L2_SEL_TGT_CROP, &sensorRect);
> > +             request->controls().set(controls::DigitalZoom, zoomRect_);
> > +     }
> > +
> >       /*
> >        * Process all the user controls by the IPA. Once this is complete, we
> >        * queue the ISP output buffer listed in the request to start the HW
> >
>
> --
> Regards
> --
> Kieran

Patch
diff mbox series 

diff --git a/include/libcamera/ipa/raspberrypi.h b/include/libcamera/ipa/raspberrypi.h
index a18ce9a..e66402e 100644
--- a/include/libcamera/ipa/raspberrypi.h
+++ b/include/libcamera/ipa/raspberrypi.h
@@ -52,6 +52,7 @@  static const ControlInfoMap RPiControls = {
 	{ &controls::Contrast, ControlInfo(0.0f, 32.0f) },
 	{ &controls::Saturation, ControlInfo(0.0f, 32.0f) },
 	{ &controls::Sharpness, ControlInfo(0.0f, 16.0f, 1.0f) },
+	{ &controls::DigitalZoom, ControlInfo(Rectangle{ 0, 0, 0, 0 }, Rectangle{ 65535, 65535, 65535, 65535 }, Rectangle{ 0, 0, 0, 0 }) },
 };
 
 } /* namespace libcamera */
diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp
index b1f2786..3c68078 100644
--- a/src/ipa/raspberrypi/raspberrypi.cpp
+++ b/src/ipa/raspberrypi/raspberrypi.cpp
@@ -658,6 +658,16 @@  void IPARPi::queueRequest(const ControlList &controls)
 			break;
 		}
 
+		case controls::DIGITAL_ZOOM: {
+			/*
+			 * We send the zoom info back in the metadata where the pipeline
+			 * handler will update it to the values actually used.
+			 */
+			Rectangle crop = ctrl.second.get<Rectangle>();
+			libcameraMetadata_.set(controls::DigitalZoom, crop);
+			break;
+		}
+
 		default:
 			LOG(IPARPI, Warning)
 				<< "Ctrl " << controls::controls.at(ctrl.first)->name()
diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
index f4966f8..55db11d 100644
--- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
+++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
@@ -287,7 +287,8 @@  class RPiCameraData : public CameraData
 public:
 	RPiCameraData(PipelineHandler *pipe)
 		: CameraData(pipe), sensor_(nullptr), lsTable_(nullptr),
-		  state_(State::Stopped), dropFrame_(false), ispOutputCount_(0)
+		  state_(State::Stopped), dropFrame_(false), ispOutputCount_(0),
+		  ispMinSize_(0)
 	{
 	}
 
@@ -322,6 +323,14 @@  public:
 	void handleStreamBuffer(FrameBuffer *buffer, const RPiStream *stream);
 	void handleState();
 
+	void initSensorCrop(Rectangle const &crop, unsigned int ispMinSize)
+	{
+		/* The initial zoom region is the whole of the sensorCrop_. */
+		sensorCrop_ = crop;
+		zoomRect_ = Rectangle{ 0, 0, crop.width, crop.height };
+		ispMinSize_ = ispMinSize;
+	}
+
 	CameraSensor *sensor_;
 	/* Array of Unicam and ISP device streams and associated buffers/streams. */
 	RPiDevice<Unicam, 2> unicam_;
@@ -358,6 +367,15 @@  private:
 
 	bool dropFrame_;
 	int ispOutputCount_;
+	/*
+	 * sensorCrop_ is the largest region of the full sensor output that matches
+	 * the desired aspect ratio, and therefore represents the area within
+	 * which we can pan and zoom. zoomRect_ is the portion from within the
+	 * sensorCrop_ that pan/zoom is currently using.
+	 */
+	Rectangle sensorCrop_;
+	Rectangle zoomRect_;
+	unsigned int ispMinSize_;
 };
 
 class RPiCameraConfiguration : public CameraConfiguration
@@ -744,6 +762,10 @@  int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config)
 		return ret;
 	}
 
+	Rectangle testCrop = { 0, 0, 1, 1 };
+	data->isp_[Isp::Input].dev()->setSelection(V4L2_SEL_TGT_CROP, &testCrop);
+	const unsigned int ispMinSize = testCrop.width;
+
 	/* Adjust aspect ratio by providing crops on the input image. */
 	Rectangle crop = {
 		.x = 0,
@@ -763,8 +785,12 @@  int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config)
 
 	crop.x = (sensorFormat.size.width - crop.width) >> 1;
 	crop.y = (sensorFormat.size.height - crop.height) >> 1;
+
 	data->isp_[Isp::Input].dev()->setSelection(V4L2_SEL_TGT_CROP, &crop);
 
+	sensorCrop_ = Size(crop.width, crop.height);
+	data->initSensorCrop(crop, ispMinSize);
+
 	ret = configureIPA(camera);
 	if (ret)
 		LOG(RPI, Error) << "Failed to configure the IPA: " << ret;
@@ -1553,6 +1579,34 @@  void RPiCameraData::tryRunPipeline()
 	 */
 	Request *request = requestQueue_.front();
 
+	if (request->controls().contains(controls::DigitalZoom)) {
+		Rectangle rect = request->controls().get<Rectangle>(controls::DigitalZoom);
+		/*
+		 * If a new digital zoom value was given, check that it lies within the
+		 * available sensorCrop_, coercing it if necessary.
+		 */
+		if (rect.width && rect.height) {
+			zoomRect_ = rect;
+			if (zoomRect_.width < ispMinSize_)
+				zoomRect_.width = ispMinSize_;
+			else if (zoomRect_.width > sensorCrop_.width)
+				zoomRect_.width = sensorCrop_.width;
+			if (zoomRect_.height < ispMinSize_)
+				zoomRect_.height = ispMinSize_;
+			else if (zoomRect_.height > sensorCrop_.height)
+				zoomRect_.height = sensorCrop_.height;
+			if (zoomRect_.x + zoomRect_.width > sensorCrop_.width)
+				zoomRect_.x = sensorCrop_.width - zoomRect_.width;
+			if (zoomRect_.y + zoomRect_.height > sensorCrop_.height)
+				zoomRect_.y = sensorCrop_.height - zoomRect_.height;
+		}
+		Rectangle sensorRect = zoomRect_;
+		sensorRect.x += sensorCrop_.x;
+		sensorRect.y += sensorCrop_.y;
+		isp_[Isp::Input].dev()->setSelection(V4L2_SEL_TGT_CROP, &sensorRect);
+		request->controls().set(controls::DigitalZoom, zoomRect_);
+	}
+
 	/*
 	 * Process all the user controls by the IPA. Once this is complete, we
 	 * queue the ISP output buffer listed in the request to start the HW