[{"id":11112,"web_url":"https://patchwork.libcamera.org/comment/11112/","msgid":"<CAEmqJPozH-jYPz8HYmGG5rwmre=sduM_MZRiu7DqLDwbYBs-yA@mail.gmail.com>","date":"2020-07-03T07:02:07","subject":"Re: [libcamera-devel] [PATCH 2/2] libcamera: raspberrypi: Implement\n\tdigital zoom","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hi David,\n\nThanks for the patch.\n\nOn Thu, 2 Jul 2020 at 11:53, David Plowman\n<david.plowman@raspberrypi.com> wrote:\n>\n> These changes implement digital zoom for the Raspberry Pi. We detect\n> the digital zoom control and update the V4L2 \"selection\" before\n> starting the ISP.\n>\n> Libcamera metadata is always filled in by the IPAs, so we intercept it\n> when it comes back to the pipeline handler and update the digital zoom\n> region with the rectangle that we actually used.\n> ---\n>  include/libcamera/ipa/raspberrypi.h           |  1 +\n>  src/ipa/raspberrypi/raspberrypi.cpp           | 10 ++++\n>  .../pipeline/raspberrypi/raspberrypi.cpp      | 59 +++++++++++++++++++\n>  3 files changed, 70 insertions(+)\n>\n> diff --git a/include/libcamera/ipa/raspberrypi.h b/include/libcamera/ipa/raspberrypi.h\n> index a18ce9a..e66402e 100644\n> --- a/include/libcamera/ipa/raspberrypi.h\n> +++ b/include/libcamera/ipa/raspberrypi.h\n> @@ -52,6 +52,7 @@ static const ControlInfoMap RPiControls = {\n>         { &controls::Contrast, ControlInfo(0.0f, 32.0f) },\n>         { &controls::Saturation, ControlInfo(0.0f, 32.0f) },\n>         { &controls::Sharpness, ControlInfo(0.0f, 16.0f, 1.0f) },\n> +       { &controls::DigitalZoom, ControlInfo(Rectangle{ 0, 0, 0, 0 }, Rectangle{ 65535, 65535, 65535, 65535 }, Rectangle{ 0, 0, 0, 0 }) },\n>  };\n>\n>  } /* namespace libcamera */\n> diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp\n> index bc89ab5..b4e42c1 100644\n> --- a/src/ipa/raspberrypi/raspberrypi.cpp\n> +++ b/src/ipa/raspberrypi/raspberrypi.cpp\n> @@ -645,6 +645,16 @@ void IPARPi::queueRequest(const ControlList &controls)\n>                         break;\n>                 }\n>\n> +               case controls::DIGITAL_ZOOM: {\n> +                       /*\n> +                        * We send the zoom info back in the metadata where the pipeline\n> +                        * handler will update it to the values actually used.\n> +                        */\n> +                       Rectangle crop = ctrl.second.get<Rectangle>();\n> +                       libcameraMetadata_.set(controls::DigitalZoom, crop);\n> +                       break;\n> +               }\n> +\n>                 default:\n>                         LOG(IPARPI, Warning)\n>                                 << \"Ctrl \" << controls::controls.at(ctrl.first)->name()\n> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> index 9d887b7..25cbe13 100644\n> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> @@ -37,6 +37,8 @@ namespace libcamera {\n>\n>  LOG_DEFINE_CATEGORY(RPI)\n>\n> +static constexpr int ISP_MIN_SIZE = 64;\n\nI'm wondering if we actually need this (see below comments).\n\n> +\n>  using V4L2PixFmtMap = std::map<V4L2PixelFormat, std::vector<SizeRange>>;\n>\n>  namespace {\n> @@ -322,6 +324,13 @@ public:\n>         void handleStreamBuffer(FrameBuffer *buffer, const RPiStream *stream);\n>         void handleState();\n>\n> +       void initSensorCrop(Rectangle const &crop)\n> +       {\n> +               /* The initial zoom region is the whole of the sensorCrop_. */\n> +               sensorCrop_ = crop;\n> +               zoomRect_ = Rectangle{ 0, 0, crop.width, crop.height };\n> +       }\n> +\n>         CameraSensor *sensor_;\n>         /* Array of Unicam and ISP device streams and associated buffers/streams. */\n>         RPiDevice<Unicam, 2> unicam_;\n> @@ -358,6 +367,14 @@ private:\n>\n>         bool dropFrame_;\n>         int ispOutputCount_;\n> +       /*\n> +        * sensorCrop_ is the largest region of the full sensor output that matches\n> +        * the desired aspect ratio, and therefore represents the area within\n> +        * which we can pan and zoom. zoomRect_ is the portion from within the\n> +        * sensorCrop_ that pan/zoom is currently using.\n> +        */\n> +       Rectangle sensorCrop_;\n> +       Rectangle zoomRect_;\n>  };\n>\n>  class RPiCameraConfiguration : public CameraConfiguration\n> @@ -763,6 +780,14 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config)\n>\n>         crop.x = (sensorFormat.size.width - crop.width) >> 1;\n>         crop.y = (sensorFormat.size.height - crop.height) >> 1;\n> +\n> +       if (crop.width < ISP_MIN_SIZE || crop.height < ISP_MIN_SIZE) {\n> +               LOG(RPI, Error) << \"Crop from sensor \" << crop.width << \"x\" << crop.height << \" is too small\";\n> +               return -1;\n> +       }\n\nThe isp driver does validate (and adjust if necessary) the crop window\nin the call to setSelection().  So you probably don't need this code\nhere.\n\n> +       sensorCrop_ = Size(crop.width, crop.height);\n> +       data->initSensorCrop(crop);\n> +\n\nIf this block is moved after the setSelection() call, then the crop\nstructure should have been adjusted by the driver, and we can use it\ndirectly in initSensorCrop().\n\n>         data->isp_[Isp::Input].dev()->setSelection(V4L2_SEL_TGT_CROP, &crop);\n>\n>         ret = configureIPA(camera);\n> @@ -1226,6 +1251,13 @@ void RPiCameraData::queueFrameAction(unsigned int frame, const IPAOperationData\n>                 handleStreamBuffer(buffer, &isp_[Isp::Stats]);\n>                 /* Fill the Request metadata buffer with what the IPA has provided */\n>                 requestQueue_.front()->metadata() = std::move(action.controls[0]);\n> +               ControlList &metadata = requestQueue_.front()->metadata();\n> +               /*\n> +                * The IPA sends the zoom metadata back to us, but we're the ones who\n> +                * actually know what we used, so must fill it in correctly.\n> +                */\n> +               if (metadata.contains(controls::DigitalZoom))\n> +                       metadata.set(controls::DigitalZoom, zoomRect_);\n\nPerhaps it would be better if we were to give the IPA the crop\nrectangle corrected for aspect ratio as well as user digital zoom (in\ntryRunPipeline())?  This way, if the IPA were to ever want the true\ncrop used, it has it.  In that case, there is no need for the above\nmetadata munging either, the IPA will return back the true crop used.\n\n>                 state_ = State::IpaComplete;\n>                 break;\n>         }\n> @@ -1548,6 +1580,33 @@ void RPiCameraData::tryRunPipeline()\n>          */\n>         Request *request = requestQueue_.front();\n>\n> +       if (request->controls().contains(controls::DigitalZoom)) {\n> +               Rectangle rect = request->controls().get<Rectangle>(controls::DigitalZoom);\n> +               /*\n> +                * If a new digital zoom value was given, check that it lies within the\n> +                * available sensorCrop_, coercing it if necessary.\n> +                */\n> +               if (rect.width && rect.height) {\n> +                       zoomRect_ = rect;\n> +                       if (zoomRect_.width < ISP_MIN_SIZE)\n> +                               zoomRect_.width = ISP_MIN_SIZE;\n> +                       else if (zoomRect_.width > sensorCrop_.width)\n> +                               zoomRect_.width = sensorCrop_.width;\n> +                       if (zoomRect_.height < ISP_MIN_SIZE)\n> +                               zoomRect_.height = ISP_MIN_SIZE;\n> +                       else if (zoomRect_.height > sensorCrop_.height)\n> +                               zoomRect_.height = sensorCrop_.height;\n> +                       if (zoomRect_.x + zoomRect_.width > sensorCrop_.width)\n> +                               zoomRect_.x = sensorCrop_.width - zoomRect_.width;\n> +                       if (zoomRect_.y + zoomRect_.height > sensorCrop_.height)\n> +                               zoomRect_.y = sensorCrop_.height - zoomRect_.height;\n> +               }\n\nAgain, this validation should already happen in the ISP driver.\n\n> +               Rectangle sensorRect = zoomRect_;\n> +               sensorRect.x += sensorCrop_.x;\n> +               sensorRect.y += sensorCrop_.y;\n> +               isp_[Isp::Input].dev()->setSelection(V4L2_SEL_TGT_CROP, &sensorRect);\n> +       }\n> +\n>         /*\n>          * Process all the user controls by the IPA. Once this is complete, we\n>          * queue the ISP output buffer listed in the request to start the HW\n> --\n> 2.20.1\n>\n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel\n\nRegards,\nNaush","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 6A4B2BE905\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri,  3 Jul 2020 07:02:27 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D41E660C58;\n\tFri,  3 Jul 2020 09:02:26 +0200 (CEST)","from mail-lf1-x141.google.com (mail-lf1-x141.google.com\n\t[IPv6:2a00:1450:4864:20::141])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 1BD44603AD\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri,  3 Jul 2020 09:02:24 +0200 (CEST)","by mail-lf1-x141.google.com with SMTP id s16so12410766lfp.12\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 03 Jul 2020 00:02:24 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"cvIn81u3\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=avBINyWPijbQ9ILTq1V+gM5bYkKZICZbRMvJ08t4NFg=;\n\tb=cvIn81u3sTVLWIW3eKMRRkcOVtIFm+jmJJnzM57CREaCnMtVRNPAgbdAJpktDrXJcO\n\t6KP4hNraRY28XWhx2Jo9HQnm4thGPHJpWZnfs3JXT2R/VPFcD6bH1r4f1+WmGtLOLn7+\n\tDCmfGK/p5E4CvzwoqtIjHNie1mlEWLppqnsxM8CTJf8vUQBikH2XNunc4V73tqiHl7Ev\n\tMOlRNhrGc6uY+b84WAFNQdmcZvjKTLy1Cgsg6Y9NVy/Wxt+RwMgNObRPA7Szr/tEmY62\n\t8rpC8WcN3Dr3+P4vPqDt8veTvfMAqLpJPgociL5m8U/Kc0VYbSIP+y5bATn1y1Yi+tmL\n\tFXwQ==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=avBINyWPijbQ9ILTq1V+gM5bYkKZICZbRMvJ08t4NFg=;\n\tb=o7bYjHwKtCE1keyHL5M/laPd9pp+Tr+O8v27SquR566ayPWYW3nlnisSw4b+/g2Tqo\n\tYsjSFNnmK/KxA8y/5SLHbBzhw60JCA+TVaa7uR0BqIASSEsvcEX6Byb9ABhAiEm/ovFl\n\tflv+hQdpGQQUm4kbAmMVyP/43wDQfVHc1xRiuR9BA7k0UVcNf+mtV6Vn+7YgGL7kZRnZ\n\tgNCyhPgjpt/d+U5kN59BdVRKPr6gl+NzqLoBML4iucAVC0ogNT0SDNXvyQTsphOwNgUm\n\tWaQJMph9k2slKcRl9HoGQzZvNekk/UTwHa4RnY1yzujlOj7Pm+VaI7lEaHKMIQamVE5R\n\ttZzw==","X-Gm-Message-State":"AOAM530BnYZlElqMLzHtxJGIVNQqm9kEE45DAwARFEDUZYEvyGU/zmlg\n\tIQU09euCPA7VW5vjwBHjC/pOGtWjw3s5CRrYtjO8Nw==","X-Google-Smtp-Source":"ABdhPJw5EIaqz7swRmLM3Izh3Y7aPZ695Hzke4NwEZOy6DAPWvJzoU4qjHDusrH2bIGzKDEKrsW0cndMSfO8CEUlcPU=","X-Received":"by 2002:a19:4285:: with SMTP id\n\tp127mr19856454lfa.74.1593759743901; \n\tFri, 03 Jul 2020 00:02:23 -0700 (PDT)","MIME-Version":"1.0","References":"<20200702105337.31161-1-david.plowman@raspberrypi.com>\n\t<20200702105337.31161-3-david.plowman@raspberrypi.com>","In-Reply-To":"<20200702105337.31161-3-david.plowman@raspberrypi.com>","From":"Naushir Patuck <naush@raspberrypi.com>","Date":"Fri, 3 Jul 2020 08:02:07 +0100","Message-ID":"<CAEmqJPozH-jYPz8HYmGG5rwmre=sduM_MZRiu7DqLDwbYBs-yA@mail.gmail.com>","To":"David Plowman <david.plowman@raspberrypi.com>","Subject":"Re: [libcamera-devel] [PATCH 2/2] libcamera: raspberrypi: Implement\n\tdigital zoom","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":11143,"web_url":"https://patchwork.libcamera.org/comment/11143/","msgid":"<20200703104824.GE5963@pendragon.ideasonboard.com>","date":"2020-07-03T10:48:24","subject":"Re: [libcamera-devel] [PATCH 2/2] libcamera: raspberrypi: Implement\n\tdigital zoom","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi David and Naush,\n\nThank you for the patch.\n\nOn Fri, Jul 03, 2020 at 08:02:07AM +0100, Naushir Patuck wrote:\n> On Thu, 2 Jul 2020 at 11:53, David Plowman wrote:\n> >\n> > These changes implement digital zoom for the Raspberry Pi. We detect\n> > the digital zoom control and update the V4L2 \"selection\" before\n> > starting the ISP.\n> >\n> > Libcamera metadata is always filled in by the IPAs, so we intercept it\n> > when it comes back to the pipeline handler and update the digital zoom\n> > region with the rectangle that we actually used.\n> > ---\n> >  include/libcamera/ipa/raspberrypi.h           |  1 +\n> >  src/ipa/raspberrypi/raspberrypi.cpp           | 10 ++++\n> >  .../pipeline/raspberrypi/raspberrypi.cpp      | 59 +++++++++++++++++++\n> >  3 files changed, 70 insertions(+)\n> >\n> > diff --git a/include/libcamera/ipa/raspberrypi.h b/include/libcamera/ipa/raspberrypi.h\n> > index a18ce9a..e66402e 100644\n> > --- a/include/libcamera/ipa/raspberrypi.h\n> > +++ b/include/libcamera/ipa/raspberrypi.h\n> > @@ -52,6 +52,7 @@ static const ControlInfoMap RPiControls = {\n> >         { &controls::Contrast, ControlInfo(0.0f, 32.0f) },\n> >         { &controls::Saturation, ControlInfo(0.0f, 32.0f) },\n> >         { &controls::Sharpness, ControlInfo(0.0f, 16.0f, 1.0f) },\n> > +       { &controls::DigitalZoom, ControlInfo(Rectangle{ 0, 0, 0, 0 }, Rectangle{ 65535, 65535, 65535, 65535 }, Rectangle{ 0, 0, 0, 0 }) },\n> >  };\n> >\n> >  } /* namespace libcamera */\n> > diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp\n> > index bc89ab5..b4e42c1 100644\n> > --- a/src/ipa/raspberrypi/raspberrypi.cpp\n> > +++ b/src/ipa/raspberrypi/raspberrypi.cpp\n> > @@ -645,6 +645,16 @@ void IPARPi::queueRequest(const ControlList &controls)\n> >                         break;\n> >                 }\n> >\n> > +               case controls::DIGITAL_ZOOM: {\n> > +                       /*\n> > +                        * We send the zoom info back in the metadata where the pipeline\n> > +                        * handler will update it to the values actually used.\n> > +                        */\n> > +                       Rectangle crop = ctrl.second.get<Rectangle>();\n> > +                       libcameraMetadata_.set(controls::DigitalZoom, crop);\n> > +                       break;\n> > +               }\n> > +\n\nDoes this need to go through the IPA, if all the IPA does it to set it\nin metadata without using or modifying the value ?\n\n> >                 default:\n> >                         LOG(IPARPI, Warning)\n> >                                 << \"Ctrl \" << controls::controls.at(ctrl.first)->name()\n> > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > index 9d887b7..25cbe13 100644\n> > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > @@ -37,6 +37,8 @@ namespace libcamera {\n> >\n> >  LOG_DEFINE_CATEGORY(RPI)\n> >\n> > +static constexpr int ISP_MIN_SIZE = 64;\n> \n> I'm wondering if we actually need this (see below comments).\n> \n> > +\n> >  using V4L2PixFmtMap = std::map<V4L2PixelFormat, std::vector<SizeRange>>;\n> >\n> >  namespace {\n> > @@ -322,6 +324,13 @@ public:\n> >         void handleStreamBuffer(FrameBuffer *buffer, const RPiStream *stream);\n> >         void handleState();\n> >\n> > +       void initSensorCrop(Rectangle const &crop)\n> > +       {\n> > +               /* The initial zoom region is the whole of the sensorCrop_. */\n> > +               sensorCrop_ = crop;\n> > +               zoomRect_ = Rectangle{ 0, 0, crop.width, crop.height };\n> > +       }\n> > +\n> >         CameraSensor *sensor_;\n> >         /* Array of Unicam and ISP device streams and associated buffers/streams. */\n> >         RPiDevice<Unicam, 2> unicam_;\n> > @@ -358,6 +367,14 @@ private:\n> >\n> >         bool dropFrame_;\n> >         int ispOutputCount_;\n> > +       /*\n> > +        * sensorCrop_ is the largest region of the full sensor output that matches\n> > +        * the desired aspect ratio, and therefore represents the area within\n> > +        * which we can pan and zoom. zoomRect_ is the portion from within the\n> > +        * sensorCrop_ that pan/zoom is currently using.\n> > +        */\n> > +       Rectangle sensorCrop_;\n> > +       Rectangle zoomRect_;\n> >  };\n> >\n> >  class RPiCameraConfiguration : public CameraConfiguration\n> > @@ -763,6 +780,14 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config)\n> >\n> >         crop.x = (sensorFormat.size.width - crop.width) >> 1;\n> >         crop.y = (sensorFormat.size.height - crop.height) >> 1;\n> > +\n> > +       if (crop.width < ISP_MIN_SIZE || crop.height < ISP_MIN_SIZE) {\n> > +               LOG(RPI, Error) << \"Crop from sensor \" << crop.width << \"x\" << crop.height << \" is too small\";\n> > +               return -1;\n> > +       }\n> \n> The isp driver does validate (and adjust if necessary) the crop window\n> in the call to setSelection().  So you probably don't need this code\n> here.\n> \n> > +       sensorCrop_ = Size(crop.width, crop.height);\n> > +       data->initSensorCrop(crop);\n> > +\n> \n> If this block is moved after the setSelection() call, then the crop\n> structure should have been adjusted by the driver, and we can use it\n> directly in initSensorCrop().\n> \n> >         data->isp_[Isp::Input].dev()->setSelection(V4L2_SEL_TGT_CROP, &crop);\n> >\n> >         ret = configureIPA(camera);\n> > @@ -1226,6 +1251,13 @@ void RPiCameraData::queueFrameAction(unsigned int frame, const IPAOperationData\n> >                 handleStreamBuffer(buffer, &isp_[Isp::Stats]);\n> >                 /* Fill the Request metadata buffer with what the IPA has provided */\n> >                 requestQueue_.front()->metadata() = std::move(action.controls[0]);\n> > +               ControlList &metadata = requestQueue_.front()->metadata();\n> > +               /*\n> > +                * The IPA sends the zoom metadata back to us, but we're the ones who\n> > +                * actually know what we used, so must fill it in correctly.\n> > +                */\n> > +               if (metadata.contains(controls::DigitalZoom))\n> > +                       metadata.set(controls::DigitalZoom, zoomRect_);\n> \n> Perhaps it would be better if we were to give the IPA the crop\n> rectangle corrected for aspect ratio as well as user digital zoom (in\n> tryRunPipeline())?  This way, if the IPA were to ever want the true\n> crop used, it has it.  In that case, there is no need for the above\n> metadata munging either, the IPA will return back the true crop used.\n> \n> >                 state_ = State::IpaComplete;\n> >                 break;\n> >         }\n> > @@ -1548,6 +1580,33 @@ void RPiCameraData::tryRunPipeline()\n> >          */\n> >         Request *request = requestQueue_.front();\n> >\n> > +       if (request->controls().contains(controls::DigitalZoom)) {\n> > +               Rectangle rect = request->controls().get<Rectangle>(controls::DigitalZoom);\n> > +               /*\n> > +                * If a new digital zoom value was given, check that it lies within the\n> > +                * available sensorCrop_, coercing it if necessary.\n> > +                */\n> > +               if (rect.width && rect.height) {\n> > +                       zoomRect_ = rect;\n> > +                       if (zoomRect_.width < ISP_MIN_SIZE)\n> > +                               zoomRect_.width = ISP_MIN_SIZE;\n> > +                       else if (zoomRect_.width > sensorCrop_.width)\n> > +                               zoomRect_.width = sensorCrop_.width;\n> > +                       if (zoomRect_.height < ISP_MIN_SIZE)\n> > +                               zoomRect_.height = ISP_MIN_SIZE;\n> > +                       else if (zoomRect_.height > sensorCrop_.height)\n> > +                               zoomRect_.height = sensorCrop_.height;\n> > +                       if (zoomRect_.x + zoomRect_.width > sensorCrop_.width)\n> > +                               zoomRect_.x = sensorCrop_.width - zoomRect_.width;\n> > +                       if (zoomRect_.y + zoomRect_.height > sensorCrop_.height)\n> > +                               zoomRect_.y = sensorCrop_.height - zoomRect_.height;\n> > +               }\n> \n> Again, this validation should already happen in the ISP driver.\n> \n> > +               Rectangle sensorRect = zoomRect_;\n> > +               sensorRect.x += sensorCrop_.x;\n> > +               sensorRect.y += sensorCrop_.y;\n> > +               isp_[Isp::Input].dev()->setSelection(V4L2_SEL_TGT_CROP, &sensorRect);\n> > +       }\n> > +\n> >         /*\n> >          * Process all the user controls by the IPA. Once this is complete, we\n> >          * queue the ISP output buffer listed in the request to start the HW","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id AA705BE905\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri,  3 Jul 2020 10:48:30 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 39DE260C59;\n\tFri,  3 Jul 2020 12:48:30 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 6C6A2603AE\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri,  3 Jul 2020 12:48:28 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id E2C2551B;\n\tFri,  3 Jul 2020 12:48:27 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"KkIRVWMf\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1593773308;\n\tbh=+TNzUpgwYB3DN3/9yLyjYJNfhFpdQGyokvUxFxF/1tc=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=KkIRVWMfjLda5VUZPoHBnaNXwoME65i2EvQWr9O+IE/24wC0IiftyzgmB51Vx7QnD\n\th2LJiJk+h5imRGRGZ04j9ZkuzxkV5NzyB2gbPwELwkLbAmrprdGRNirjJxdRX/yatZ\n\tKhvK4A/ncPotngSpgEczrMR024t0O2uoeuFs+DrM=","Date":"Fri, 3 Jul 2020 13:48:24 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"David Plowman <david.plowman@raspberrypi.com>","Message-ID":"<20200703104824.GE5963@pendragon.ideasonboard.com>","References":"<20200702105337.31161-1-david.plowman@raspberrypi.com>\n\t<20200702105337.31161-3-david.plowman@raspberrypi.com>\n\t<CAEmqJPozH-jYPz8HYmGG5rwmre=sduM_MZRiu7DqLDwbYBs-yA@mail.gmail.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<CAEmqJPozH-jYPz8HYmGG5rwmre=sduM_MZRiu7DqLDwbYBs-yA@mail.gmail.com>","Subject":"Re: [libcamera-devel] [PATCH 2/2] libcamera: raspberrypi: Implement\n\tdigital zoom","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]