[{"id":12800,"web_url":"https://patchwork.libcamera.org/comment/12800/","msgid":"<20200928104422.k5imrdjq73gl6cqa@uno.localdomain>","date":"2020-09-28T10:44:22","subject":"Re: [libcamera-devel] [PATCH v2 6/6] cam: Add command line option\n\tto test IspCrop control","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi David,\n\nOn Fri, Sep 25, 2020 at 09:51:27AM +0100, David Plowman wrote:\n> This allows the user to request digital zoom by adding, for example,\n> \"-z 0.25,0.25,0.5,0.5\" which would give a 2x zoom, still centred in\n> the middle of the image.\n>\n> Signed-off-by: David Plowman <david.plowman@raspberrypi.com>\n> ---\n>  src/cam/capture.cpp | 25 +++++++++++++++++++++++--\n>  src/cam/capture.h   |  2 +-\n>  src/cam/main.cpp    |  3 +++\n>  src/cam/main.h      |  1 +\n>  4 files changed, 28 insertions(+), 3 deletions(-)\n>\n> diff --git a/src/cam/capture.cpp b/src/cam/capture.cpp\n> index 5510c00..3b50c59 100644\n> --- a/src/cam/capture.cpp\n> +++ b/src/cam/capture.cpp\n> @@ -10,6 +10,9 @@\n>  #include <limits.h>\n>  #include <sstream>\n>\n> +#include <libcamera/control_ids.h>\n> +#include <libcamera/property_ids.h>\n> +\n>  #include \"capture.h\"\n>  #include \"main.h\"\n>\n> @@ -58,7 +61,7 @@ int Capture::run(const OptionsParser::Options &options)\n>\n>  \tFrameBufferAllocator *allocator = new FrameBufferAllocator(camera_);\n>\n> -\tret = capture(allocator);\n> +\tret = capture(allocator, options);\n>\n>  \tif (options.isSet(OptFile)) {\n>  \t\tdelete writer_;\n> @@ -70,7 +73,7 @@ int Capture::run(const OptionsParser::Options &options)\n>  \treturn ret;\n>  }\n>\n> -int Capture::capture(FrameBufferAllocator *allocator)\n> +int Capture::capture(FrameBufferAllocator *allocator, const OptionsParser::Options &options)\n>  {\n>  \tint ret;\n>\n> @@ -120,6 +123,24 @@ int Capture::capture(FrameBufferAllocator *allocator)\n>  \t\trequests.push_back(request);\n>  \t}\n>\n> +\t/* Set up digital zoom if it was requested on the command line. */\n> +\tif (options.isSet(OptZoom)) {\n\nI would have preferred to keep option parsing where the other options\nparsing happens, but this is the first control we set in Request, so I\nguess this it's fine to do it here.\n\n> +\t\tstd::string zoom = options[OptZoom].toString();\n> +\t\tfloat x, y, width, height;\n> +\n> +\t\tif (sscanf(zoom.c_str(), \"%f,%f,%f,%f\", &x, &y, &width, &height) == 4) {\n> +\t\t\tSize sensorArea = camera_->properties().get(properties::SensorOutputSize);\n> +\t\t\tRectangle crop(x * sensorArea.width,\n> +\t\t\t\t       y * sensorArea.height,\n> +\t\t\t\t       width * sensorArea.width,\n> +\t\t\t\t       height * sensorArea.height);\n> +\n> +\t\t\trequests.front()->controls().set(controls::IspCrop, crop);\n> +\t\t} else {\n> +\t\t\tstd::cout << \"Invalid zoom values - ignoring\" << std::endl;\n> +\t\t}\n> +\t}\n> +\n>  \tret = camera_->start();\n>  \tif (ret) {\n>  \t\tstd::cout << \"Failed to start capture\" << std::endl;\n> diff --git a/src/cam/capture.h b/src/cam/capture.h\n> index 0aebdac..5280616 100644\n> --- a/src/cam/capture.h\n> +++ b/src/cam/capture.h\n> @@ -29,7 +29,7 @@ public:\n>\n>  \tint run(const OptionsParser::Options &options);\n>  private:\n> -\tint capture(libcamera::FrameBufferAllocator *allocator);\n> +\tint capture(libcamera::FrameBufferAllocator *allocator, const OptionsParser::Options &options);\n>\n>  \tvoid requestComplete(libcamera::Request *request);\n>\n> diff --git a/src/cam/main.cpp b/src/cam/main.cpp\n> index 244720b..8f326d9 100644\n> --- a/src/cam/main.cpp\n> +++ b/src/cam/main.cpp\n> @@ -201,6 +201,9 @@ int CamApp::parseOptions(int argc, char *argv[])\n>  \tparser.addOption(OptStrictFormats, OptionNone,\n>  \t\t\t \"Do not allow requested stream format(s) to be adjusted\",\n>  \t\t\t \"strict-formats\");\n> +\tparser.addOption(OptZoom, OptionString,\n> +\t\t\t \"Specify digital zoom by supplying IspCrop control as x,y,width,height e.g. 0.25,0.25,0.5,0.5\",\n\nCould we specify the values are percentages ? Maybe it's a standard\nthing, but for the uneducated like me is it worth specifying it ?\n\n\t\t\t \"Zoom on the image portion described by the x,y,width,height rectangle (in %) e.g. 0.25,0.25,0.5,0.5\",\n\nOr something better..\n\nReviewed-by: Jacopo Mondi <jacopo@jmondi.org>\n\nThanks\n  j\n\n\n> +\t\t\t \"zoom\", ArgumentRequired, \"zoom\");\n>\n>  \toptions_ = parser.parse(argc, argv);\n>  \tif (!options_.valid())\n> diff --git a/src/cam/main.h b/src/cam/main.h\n> index ea8dfd3..a1fd899 100644\n> --- a/src/cam/main.h\n> +++ b/src/cam/main.h\n> @@ -17,6 +17,7 @@ enum {\n>  \tOptListProperties = 'p',\n>  \tOptMonitor = 'm',\n>  \tOptStream = 's',\n> +\tOptZoom = 'z',\n>  \tOptListControls = 256,\n>  \tOptStrictFormats = 257,\n>  };\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","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 19C0AC3B5C\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 28 Sep 2020 10:40:28 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id AF75460BD7;\n\tMon, 28 Sep 2020 12:40:27 +0200 (CEST)","from relay6-d.mail.gandi.net (relay6-d.mail.gandi.net\n\t[217.70.183.198])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id CEBC460364\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 28 Sep 2020 12:40:26 +0200 (CEST)","from uno.localdomain (93-34-118-233.ip49.fastwebnet.it\n\t[93.34.118.233]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay6-d.mail.gandi.net (Postfix) with ESMTPSA id 597FDC000A;\n\tMon, 28 Sep 2020 10:40:25 +0000 (UTC)"],"X-Originating-IP":"93.34.118.233","Date":"Mon, 28 Sep 2020 12:44:22 +0200","From":"Jacopo Mondi <jacopo@jmondi.org>","To":"David Plowman <david.plowman@raspberrypi.com>","Message-ID":"<20200928104422.k5imrdjq73gl6cqa@uno.localdomain>","References":"<20200925085127.17214-1-david.plowman@raspberrypi.com>\n\t<20200925085127.17214-7-david.plowman@raspberrypi.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200925085127.17214-7-david.plowman@raspberrypi.com>","Subject":"Re: [libcamera-devel] [PATCH v2 6/6] cam: Add command line option\n\tto test IspCrop control","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":12802,"web_url":"https://patchwork.libcamera.org/comment/12802/","msgid":"<20200928110735.GC2219@oden.dyn.berto.se>","date":"2020-09-28T11:07:35","subject":"Re: [libcamera-devel] [PATCH v2 6/6] cam: Add command line option\n\tto test IspCrop control","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi David,\n\nThanks for your patch.\n\nOn 2020-09-25 09:51:27 +0100, David Plowman wrote:\n> This allows the user to request digital zoom by adding, for example,\n> \"-z 0.25,0.25,0.5,0.5\" which would give a 2x zoom, still centred in\n> the middle of the image.\n> \n> Signed-off-by: David Plowman <david.plowman@raspberrypi.com>\n> ---\n>  src/cam/capture.cpp | 25 +++++++++++++++++++++++--\n>  src/cam/capture.h   |  2 +-\n>  src/cam/main.cpp    |  3 +++\n>  src/cam/main.h      |  1 +\n>  4 files changed, 28 insertions(+), 3 deletions(-)\n> \n> diff --git a/src/cam/capture.cpp b/src/cam/capture.cpp\n> index 5510c00..3b50c59 100644\n> --- a/src/cam/capture.cpp\n> +++ b/src/cam/capture.cpp\n> @@ -10,6 +10,9 @@\n>  #include <limits.h>\n>  #include <sstream>\n>  \n> +#include <libcamera/control_ids.h>\n> +#include <libcamera/property_ids.h>\n> +\n>  #include \"capture.h\"\n>  #include \"main.h\"\n>  \n> @@ -58,7 +61,7 @@ int Capture::run(const OptionsParser::Options &options)\n>  \n>  \tFrameBufferAllocator *allocator = new FrameBufferAllocator(camera_);\n>  \n> -\tret = capture(allocator);\n> +\tret = capture(allocator, options);\n>  \n>  \tif (options.isSet(OptFile)) {\n>  \t\tdelete writer_;\n> @@ -70,7 +73,7 @@ int Capture::run(const OptionsParser::Options &options)\n>  \treturn ret;\n>  }\n>  \n> -int Capture::capture(FrameBufferAllocator *allocator)\n> +int Capture::capture(FrameBufferAllocator *allocator, const OptionsParser::Options &options)\n>  {\n>  \tint ret;\n>  \n> @@ -120,6 +123,24 @@ int Capture::capture(FrameBufferAllocator *allocator)\n>  \t\trequests.push_back(request);\n>  \t}\n>  \n> +\t/* Set up digital zoom if it was requested on the command line. */\n> +\tif (options.isSet(OptZoom)) {\n> +\t\tstd::string zoom = options[OptZoom].toString();\n> +\t\tfloat x, y, width, height;\n> +\n> +\t\tif (sscanf(zoom.c_str(), \"%f,%f,%f,%f\", &x, &y, &width, &height) == 4) {\n\nThis is the wrong place to parse the input from the user. If the zoom \noptions are global something similar like StreamKeyValueParser \n(ZoomKeyValueParser?) should be created.\n\n> +\t\t\tSize sensorArea = camera_->properties().get(properties::SensorOutputSize);\n> +\t\t\tRectangle crop(x * sensorArea.width,\n> +\t\t\t\t       y * sensorArea.height,\n> +\t\t\t\t       width * sensorArea.width,\n> +\t\t\t\t       height * sensorArea.height);\n\n\n> +\n> +\t\t\trequests.front()->controls().set(controls::IspCrop, crop);\n\nDo we wish to expose the ISP in the public API? If we do is the ISP \n(from an application point of view) the correct level to operate on?  \nWould not setting this on a stream level cover more hardware?\n\nI'm thinking of an ISP that can apply different crop parameters to two \n(or more) source pads from a single sink pad. Having a control that sets \na global crop on the ISP would that not limit such hardware or am I \nmissing something?\n\n> +\t\t} else {\n> +\t\t\tstd::cout << \"Invalid zoom values - ignoring\" << std::endl;\n> +\t\t}\n> +\t}\n> +\n>  \tret = camera_->start();\n>  \tif (ret) {\n>  \t\tstd::cout << \"Failed to start capture\" << std::endl;\n> diff --git a/src/cam/capture.h b/src/cam/capture.h\n> index 0aebdac..5280616 100644\n> --- a/src/cam/capture.h\n> +++ b/src/cam/capture.h\n> @@ -29,7 +29,7 @@ public:\n>  \n>  \tint run(const OptionsParser::Options &options);\n>  private:\n> -\tint capture(libcamera::FrameBufferAllocator *allocator);\n> +\tint capture(libcamera::FrameBufferAllocator *allocator, const OptionsParser::Options &options);\n>  \n>  \tvoid requestComplete(libcamera::Request *request);\n>  \n> diff --git a/src/cam/main.cpp b/src/cam/main.cpp\n> index 244720b..8f326d9 100644\n> --- a/src/cam/main.cpp\n> +++ b/src/cam/main.cpp\n> @@ -201,6 +201,9 @@ int CamApp::parseOptions(int argc, char *argv[])\n>  \tparser.addOption(OptStrictFormats, OptionNone,\n>  \t\t\t \"Do not allow requested stream format(s) to be adjusted\",\n>  \t\t\t \"strict-formats\");\n> +\tparser.addOption(OptZoom, OptionString,\n> +\t\t\t \"Specify digital zoom by supplying IspCrop control as x,y,width,height e.g. 0.25,0.25,0.5,0.5\",\n> +\t\t\t \"zoom\", ArgumentRequired, \"zoom\");\n>  \n>  \toptions_ = parser.parse(argc, argv);\n>  \tif (!options_.valid())\n> diff --git a/src/cam/main.h b/src/cam/main.h\n> index ea8dfd3..a1fd899 100644\n> --- a/src/cam/main.h\n> +++ b/src/cam/main.h\n> @@ -17,6 +17,7 @@ enum {\n>  \tOptListProperties = 'p',\n>  \tOptMonitor = 'm',\n>  \tOptStream = 's',\n> +\tOptZoom = 'z',\n>  \tOptListControls = 256,\n>  \tOptStrictFormats = 257,\n>  };\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","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 EA9ABC3B5C\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 28 Sep 2020 11:07:39 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7EB9060366;\n\tMon, 28 Sep 2020 13:07:39 +0200 (CEST)","from mail-lf1-x12e.google.com (mail-lf1-x12e.google.com\n\t[IPv6:2a00:1450:4864:20::12e])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id BC60F60364\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 28 Sep 2020 13:07:37 +0200 (CEST)","by mail-lf1-x12e.google.com with SMTP id y17so750851lfa.8\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 28 Sep 2020 04:07:37 -0700 (PDT)","from localhost (h-209-203.A463.priv.bahnhof.se. [155.4.209.203])\n\tby smtp.gmail.com with ESMTPSA id n8sm63829lji.1.2020.09.28.04.07.36\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tMon, 28 Sep 2020 04:07:36 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=ragnatech-se.20150623.gappssmtp.com\n\theader.i=@ragnatech-se.20150623.gappssmtp.com\n\theader.b=\"KPVIGajG\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ragnatech-se.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:content-transfer-encoding:in-reply-to;\n\tbh=/EDsJqhFbGjxNh6AgpjMyVI8/BUGIuxkiAZPkYOcRE0=;\n\tb=KPVIGajG3mRSDI2i8KBHK84YA+Y7AFQdvsoyUxho1/NVolsSf9H0lGWESIjsDMAWwd\n\tVO+wSmFUibbZ/W6pljUUKj4WsOoHUDHuEvyLBG2p3pqQivmL0PZCaq90mLDjKNrRWClj\n\tPM6m8nCjxkL+T0QdPwI2J8SNzNZeNjjOtyJMnC/SqDCXnvqQvdRY/LWjZPPxAaxJ6DTm\n\txANo1+pdDY4NmubuAbRgU6kiYbzE7KCdJQMPQQO52MYQH2pazYlZmZQA1a5KeI7NEj7T\n\tM/HMwB3l+HHxP40NtSwk/2sHMFXOGKtvLOoR+z5i4Sbb2oRt8Lhzl+aJtsolQRL6GFLM\n\tmw7g==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:content-transfer-encoding\n\t:in-reply-to;\n\tbh=/EDsJqhFbGjxNh6AgpjMyVI8/BUGIuxkiAZPkYOcRE0=;\n\tb=JSMCQ+6mU4tQ+ePm7so29+X5GEKBk0RUQ0AnaoLtAyaJ+h2vQUJ4/0S+Y+vQLTnX6D\n\tCRHWsIndd3mcIJ2v3Pu1E8giZUnkzROYmjbMGE52gSpLVhlAbg3G3NC1J6bXbl/l4iDT\n\tP4/Ef8Lv1WcFrqME3livHP5A7MerlgvBkKY0+yTEiCWhe3mdezz0twlhAsl7GErTaGTH\n\tbshJJeSQZTccmavmp6oP8SSx3oY7aIpCjwqBbb2oHGZ/gPuFRokRWthys1fNXQniwfD0\n\tELqdoJxwwe3OM9ctwEoyptWfW/0iwlY9MowTVugAkZr/z1aj+vBys7TLSNL++vAsAWQg\n\tRFrw==","X-Gm-Message-State":"AOAM533UF0NB70UNv+m5WZlvPvDI8bg8fp8bTy+qnhXIIectjGdg8Ai5\n\thmOBAgPhe+cv723AXiEIrSJIOw==","X-Google-Smtp-Source":"ABdhPJwQoOO7P1aFrsVfKCfjR3ZUgYxsUBwX+YHISzANHo9TnxXmK7FKM03frg1U0fSlKoaRtrdlzA==","X-Received":"by 2002:a19:348:: with SMTP id 69mr339649lfd.308.1601291256914; \n\tMon, 28 Sep 2020 04:07:36 -0700 (PDT)","Date":"Mon, 28 Sep 2020 13:07:35 +0200","From":"Niklas =?iso-8859-1?q?S=F6derlund?= <niklas.soderlund@ragnatech.se>","To":"David Plowman <david.plowman@raspberrypi.com>","Message-ID":"<20200928110735.GC2219@oden.dyn.berto.se>","References":"<20200925085127.17214-1-david.plowman@raspberrypi.com>\n\t<20200925085127.17214-7-david.plowman@raspberrypi.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200925085127.17214-7-david.plowman@raspberrypi.com>","Subject":"Re: [libcamera-devel] [PATCH v2 6/6] cam: Add command line option\n\tto test IspCrop control","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=\"iso-8859-1\"","Content-Transfer-Encoding":"quoted-printable","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":12803,"web_url":"https://patchwork.libcamera.org/comment/12803/","msgid":"<CAHW6GYKtiWMp=1DRo=MbGy7wsVCzHP79CNDHkBS87AZ+rSCQpw@mail.gmail.com>","date":"2020-09-28T11:13:23","subject":"Re: [libcamera-devel] [PATCH v2 6/6] cam: Add command line option\n\tto test IspCrop control","submitter":{"id":42,"url":"https://patchwork.libcamera.org/api/people/42/","name":"David Plowman","email":"david.plowman@raspberrypi.com"},"content":"Hi Jacopo\n\nThanks for the comments. Just one small clarification...\n\nOn Mon, 28 Sep 2020 at 11:40, Jacopo Mondi <jacopo@jmondi.org> wrote:\n>\n> Hi David,\n>\n> On Fri, Sep 25, 2020 at 09:51:27AM +0100, David Plowman wrote:\n> > This allows the user to request digital zoom by adding, for example,\n> > \"-z 0.25,0.25,0.5,0.5\" which would give a 2x zoom, still centred in\n> > the middle of the image.\n> >\n> > Signed-off-by: David Plowman <david.plowman@raspberrypi.com>\n> > ---\n> >  src/cam/capture.cpp | 25 +++++++++++++++++++++++--\n> >  src/cam/capture.h   |  2 +-\n> >  src/cam/main.cpp    |  3 +++\n> >  src/cam/main.h      |  1 +\n> >  4 files changed, 28 insertions(+), 3 deletions(-)\n> >\n> > diff --git a/src/cam/capture.cpp b/src/cam/capture.cpp\n> > index 5510c00..3b50c59 100644\n> > --- a/src/cam/capture.cpp\n> > +++ b/src/cam/capture.cpp\n> > @@ -10,6 +10,9 @@\n> >  #include <limits.h>\n> >  #include <sstream>\n> >\n> > +#include <libcamera/control_ids.h>\n> > +#include <libcamera/property_ids.h>\n> > +\n> >  #include \"capture.h\"\n> >  #include \"main.h\"\n> >\n> > @@ -58,7 +61,7 @@ int Capture::run(const OptionsParser::Options &options)\n> >\n> >       FrameBufferAllocator *allocator = new FrameBufferAllocator(camera_);\n> >\n> > -     ret = capture(allocator);\n> > +     ret = capture(allocator, options);\n> >\n> >       if (options.isSet(OptFile)) {\n> >               delete writer_;\n> > @@ -70,7 +73,7 @@ int Capture::run(const OptionsParser::Options &options)\n> >       return ret;\n> >  }\n> >\n> > -int Capture::capture(FrameBufferAllocator *allocator)\n> > +int Capture::capture(FrameBufferAllocator *allocator, const OptionsParser::Options &options)\n> >  {\n> >       int ret;\n> >\n> > @@ -120,6 +123,24 @@ int Capture::capture(FrameBufferAllocator *allocator)\n> >               requests.push_back(request);\n> >       }\n> >\n> > +     /* Set up digital zoom if it was requested on the command line. */\n> > +     if (options.isSet(OptZoom)) {\n>\n> I would have preferred to keep option parsing where the other options\n> parsing happens, but this is the first control we set in Request, so I\n> guess this it's fine to do it here.\n>\n> > +             std::string zoom = options[OptZoom].toString();\n> > +             float x, y, width, height;\n> > +\n> > +             if (sscanf(zoom.c_str(), \"%f,%f,%f,%f\", &x, &y, &width, &height) == 4) {\n> > +                     Size sensorArea = camera_->properties().get(properties::SensorOutputSize);\n> > +                     Rectangle crop(x * sensorArea.width,\n> > +                                    y * sensorArea.height,\n> > +                                    width * sensorArea.width,\n> > +                                    height * sensorArea.height);\n> > +\n> > +                     requests.front()->controls().set(controls::IspCrop, crop);\n> > +             } else {\n> > +                     std::cout << \"Invalid zoom values - ignoring\" << std::endl;\n> > +             }\n> > +     }\n> > +\n> >       ret = camera_->start();\n> >       if (ret) {\n> >               std::cout << \"Failed to start capture\" << std::endl;\n> > diff --git a/src/cam/capture.h b/src/cam/capture.h\n> > index 0aebdac..5280616 100644\n> > --- a/src/cam/capture.h\n> > +++ b/src/cam/capture.h\n> > @@ -29,7 +29,7 @@ public:\n> >\n> >       int run(const OptionsParser::Options &options);\n> >  private:\n> > -     int capture(libcamera::FrameBufferAllocator *allocator);\n> > +     int capture(libcamera::FrameBufferAllocator *allocator, const OptionsParser::Options &options);\n> >\n> >       void requestComplete(libcamera::Request *request);\n> >\n> > diff --git a/src/cam/main.cpp b/src/cam/main.cpp\n> > index 244720b..8f326d9 100644\n> > --- a/src/cam/main.cpp\n> > +++ b/src/cam/main.cpp\n> > @@ -201,6 +201,9 @@ int CamApp::parseOptions(int argc, char *argv[])\n> >       parser.addOption(OptStrictFormats, OptionNone,\n> >                        \"Do not allow requested stream format(s) to be adjusted\",\n> >                        \"strict-formats\");\n> > +     parser.addOption(OptZoom, OptionString,\n> > +                      \"Specify digital zoom by supplying IspCrop control as x,y,width,height e.g. 0.25,0.25,0.5,0.5\",\n>\n> Could we specify the values are percentages ? Maybe it's a standard\n> thing, but for the uneducated like me is it worth specifying it ?\n>\n>                          \"Zoom on the image portion described by the x,y,width,height rectangle (in %) e.g. 0.25,0.25,0.5,0.5\",\n>\n> Or something better..\n\nI think I'd expect a percentage to be out of one hundred, so can we go\nwith something like:\n\n\"Zoom on the image portion described by the x,y,width,height rectangle\n(fraction of the maximum FOV) e.g. 0.25,0.25,0.5,0.5\"\n\n(percentages would be OK too, but I think there'd be a small code\nchange where we divide everything by 100)\n\nAre we OK to make this change while applying or would another patch be better?\n\nThanks\nDavid\n\n>\n> Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>\n>\n> Thanks\n>   j\n>\n>\n> > +                      \"zoom\", ArgumentRequired, \"zoom\");\n> >\n> >       options_ = parser.parse(argc, argv);\n> >       if (!options_.valid())\n> > diff --git a/src/cam/main.h b/src/cam/main.h\n> > index ea8dfd3..a1fd899 100644\n> > --- a/src/cam/main.h\n> > +++ b/src/cam/main.h\n> > @@ -17,6 +17,7 @@ enum {\n> >       OptListProperties = 'p',\n> >       OptMonitor = 'm',\n> >       OptStream = 's',\n> > +     OptZoom = 'z',\n> >       OptListControls = 256,\n> >       OptStrictFormats = 257,\n> >  };\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","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 E8E41C3B5B\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 28 Sep 2020 11:13:37 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 4E3E260BD7;\n\tMon, 28 Sep 2020 13:13:37 +0200 (CEST)","from mail-ot1-x331.google.com (mail-ot1-x331.google.com\n\t[IPv6:2607:f8b0:4864:20::331])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id C1B1360364\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 28 Sep 2020 13:13:35 +0200 (CEST)","by mail-ot1-x331.google.com with SMTP id m12so531354otr.0\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 28 Sep 2020 04:13:35 -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=\"mb+tLODs\"; 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=rFxwFKDSRzBgDhUP+cu6o/1SQ6/i7vyGvB+TIaJ8VPg=;\n\tb=mb+tLODsRCH/g6Lx4yxGVW6A5VeK6msThjma/BF9SwjHuBe9vaPQvzBFJmp3803/PF\n\tvDjG6dFhvAWpIOJOYoXiynjoLL6uVKWokV4ES4pORsx61iO9HgnmAjtBRAG4we6VkHie\n\tHzw/LV/0GvTn35jYK9th7rods0icow4Pze9/05SXsw9r9IxBbBQD3onq36E7fc11CRtt\n\tKJbLdUqRBkHMnRjile8ur9zJmAKv+wB8Q+P8ZlaT/r0wRXjyqBAw3wh4bEQaa1zYTWtf\n\tvUHvqm1O9AreC3ziwb1KXSQtnijbue6kHTbzhJ7itee4eB8CY+VaJNXttFcJNULPbPge\n\tU+uQ==","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=rFxwFKDSRzBgDhUP+cu6o/1SQ6/i7vyGvB+TIaJ8VPg=;\n\tb=tAoWhyTglwAI2kWq6C5GkcOzAKaeYC8PWwXcE8foig0zySLBNbe1K140Jx3cXd6kL+\n\t8ubgMpbPwy6Sms2je8LLuC3uoIKceL/+9Wlz+CxuDIfFq9+aKcYj4ZYx7xWFw6kh1U7L\n\tWFxU/NLyTTFSdTfwVBBWXvzlHbsqfPUCbqgrufUEyhPKShmRdl+x5Xldh25Hn4I9zXPr\n\ta4MFhV0rw99XloD9l4qLrQM+GI+hOqqDeQS7+ML47oMPFqUnWYqeHUsvKbLfOWxg7hkF\n\tgEZTk8z+LmnjYfppcYK8k1zTNCk+4Fjc1gk10nC3PGNnLNe740J3hDrp2fS+Cm+Bnqc9\n\t9BlQ==","X-Gm-Message-State":"AOAM533Jo8gOk7T20maHQzl2xm1zI/XdrHS/owKuhlvpNDtedfOrs0Wc\n\tC0T/DryTIZvYnrf9aWXNwTWSZ6fgVkBHnJtWd222RVlc5Qs+7w==","X-Google-Smtp-Source":"ABdhPJy6HwakYy5o8Y644T7MK3DtYyqkEaU78fqiT6ljysWY4d+6xozy7GtyxjfBL72zOWkEgeax0BFZot8E2kKK2/U=","X-Received":"by 2002:a9d:4c0a:: with SMTP id l10mr584060otf.166.1601291614407;\n\tMon, 28 Sep 2020 04:13:34 -0700 (PDT)","MIME-Version":"1.0","References":"<20200925085127.17214-1-david.plowman@raspberrypi.com>\n\t<20200925085127.17214-7-david.plowman@raspberrypi.com>\n\t<20200928104422.k5imrdjq73gl6cqa@uno.localdomain>","In-Reply-To":"<20200928104422.k5imrdjq73gl6cqa@uno.localdomain>","From":"David Plowman <david.plowman@raspberrypi.com>","Date":"Mon, 28 Sep 2020 12:13:23 +0100","Message-ID":"<CAHW6GYKtiWMp=1DRo=MbGy7wsVCzHP79CNDHkBS87AZ+rSCQpw@mail.gmail.com>","To":"Jacopo Mondi <jacopo@jmondi.org>","Subject":"Re: [libcamera-devel] [PATCH v2 6/6] cam: Add command line option\n\tto test IspCrop control","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":12804,"web_url":"https://patchwork.libcamera.org/comment/12804/","msgid":"<20200928111937.vj6bf5hfk3tnjekf@uno.localdomain>","date":"2020-09-28T11:19:37","subject":"Re: [libcamera-devel] [PATCH v2 6/6] cam: Add command line option\n\tto test IspCrop control","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"HI David,\n\nOn Mon, Sep 28, 2020 at 12:13:23PM +0100, David Plowman wrote:\n> Hi Jacopo\n>\n> Thanks for the comments. Just one small clarification...\n>\n> On Mon, 28 Sep 2020 at 11:40, Jacopo Mondi <jacopo@jmondi.org> wrote:\n> >\n> > Hi David,\n> >\n> > On Fri, Sep 25, 2020 at 09:51:27AM +0100, David Plowman wrote:\n> > > This allows the user to request digital zoom by adding, for example,\n> > > \"-z 0.25,0.25,0.5,0.5\" which would give a 2x zoom, still centred in\n> > > the middle of the image.\n> > >\n> > > Signed-off-by: David Plowman <david.plowman@raspberrypi.com>\n> > > ---\n> > >  src/cam/capture.cpp | 25 +++++++++++++++++++++++--\n> > >  src/cam/capture.h   |  2 +-\n> > >  src/cam/main.cpp    |  3 +++\n> > >  src/cam/main.h      |  1 +\n> > >  4 files changed, 28 insertions(+), 3 deletions(-)\n> > >\n> > > diff --git a/src/cam/capture.cpp b/src/cam/capture.cpp\n> > > index 5510c00..3b50c59 100644\n> > > --- a/src/cam/capture.cpp\n> > > +++ b/src/cam/capture.cpp\n> > > @@ -10,6 +10,9 @@\n> > >  #include <limits.h>\n> > >  #include <sstream>\n> > >\n> > > +#include <libcamera/control_ids.h>\n> > > +#include <libcamera/property_ids.h>\n> > > +\n> > >  #include \"capture.h\"\n> > >  #include \"main.h\"\n> > >\n> > > @@ -58,7 +61,7 @@ int Capture::run(const OptionsParser::Options &options)\n> > >\n> > >       FrameBufferAllocator *allocator = new FrameBufferAllocator(camera_);\n> > >\n> > > -     ret = capture(allocator);\n> > > +     ret = capture(allocator, options);\n> > >\n> > >       if (options.isSet(OptFile)) {\n> > >               delete writer_;\n> > > @@ -70,7 +73,7 @@ int Capture::run(const OptionsParser::Options &options)\n> > >       return ret;\n> > >  }\n> > >\n> > > -int Capture::capture(FrameBufferAllocator *allocator)\n> > > +int Capture::capture(FrameBufferAllocator *allocator, const OptionsParser::Options &options)\n> > >  {\n> > >       int ret;\n> > >\n> > > @@ -120,6 +123,24 @@ int Capture::capture(FrameBufferAllocator *allocator)\n> > >               requests.push_back(request);\n> > >       }\n> > >\n> > > +     /* Set up digital zoom if it was requested on the command line. */\n> > > +     if (options.isSet(OptZoom)) {\n> >\n> > I would have preferred to keep option parsing where the other options\n> > parsing happens, but this is the first control we set in Request, so I\n> > guess this it's fine to do it here.\n> >\n> > > +             std::string zoom = options[OptZoom].toString();\n> > > +             float x, y, width, height;\n> > > +\n> > > +             if (sscanf(zoom.c_str(), \"%f,%f,%f,%f\", &x, &y, &width, &height) == 4) {\n> > > +                     Size sensorArea = camera_->properties().get(properties::SensorOutputSize);\n> > > +                     Rectangle crop(x * sensorArea.width,\n> > > +                                    y * sensorArea.height,\n> > > +                                    width * sensorArea.width,\n> > > +                                    height * sensorArea.height);\n> > > +\n> > > +                     requests.front()->controls().set(controls::IspCrop, crop);\n> > > +             } else {\n> > > +                     std::cout << \"Invalid zoom values - ignoring\" << std::endl;\n> > > +             }\n> > > +     }\n> > > +\n> > >       ret = camera_->start();\n> > >       if (ret) {\n> > >               std::cout << \"Failed to start capture\" << std::endl;\n> > > diff --git a/src/cam/capture.h b/src/cam/capture.h\n> > > index 0aebdac..5280616 100644\n> > > --- a/src/cam/capture.h\n> > > +++ b/src/cam/capture.h\n> > > @@ -29,7 +29,7 @@ public:\n> > >\n> > >       int run(const OptionsParser::Options &options);\n> > >  private:\n> > > -     int capture(libcamera::FrameBufferAllocator *allocator);\n> > > +     int capture(libcamera::FrameBufferAllocator *allocator, const OptionsParser::Options &options);\n> > >\n> > >       void requestComplete(libcamera::Request *request);\n> > >\n> > > diff --git a/src/cam/main.cpp b/src/cam/main.cpp\n> > > index 244720b..8f326d9 100644\n> > > --- a/src/cam/main.cpp\n> > > +++ b/src/cam/main.cpp\n> > > @@ -201,6 +201,9 @@ int CamApp::parseOptions(int argc, char *argv[])\n> > >       parser.addOption(OptStrictFormats, OptionNone,\n> > >                        \"Do not allow requested stream format(s) to be adjusted\",\n> > >                        \"strict-formats\");\n> > > +     parser.addOption(OptZoom, OptionString,\n> > > +                      \"Specify digital zoom by supplying IspCrop control as x,y,width,height e.g. 0.25,0.25,0.5,0.5\",\n> >\n> > Could we specify the values are percentages ? Maybe it's a standard\n> > thing, but for the uneducated like me is it worth specifying it ?\n> >\n> >                          \"Zoom on the image portion described by the x,y,width,height rectangle (in %) e.g. 0.25,0.25,0.5,0.5\",\n> >\n> > Or something better..\n>\n> I think I'd expect a percentage to be out of one hundred, so can we go\n> with something like:\n>\n> \"Zoom on the image portion described by the x,y,width,height rectangle\n> (fraction of the maximum FOV) e.g. 0.25,0.25,0.5,0.5\"\n>\n> (percentages would be OK too, but I think there'd be a small code\n> change where we divide everything by 100)\n\nYou are right, I should have wrote \"fraction\"\n>\n> Are we OK to make this change while applying or would another patch be better?\n>\n\nA new patch for this change only is not required imho.\n\nThanks\n  j\n\n> Thanks\n> David\n>\n> >\n> > Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>\n> >\n> > Thanks\n> >   j\n> >\n> >\n> > > +                      \"zoom\", ArgumentRequired, \"zoom\");\n> > >\n> > >       options_ = parser.parse(argc, argv);\n> > >       if (!options_.valid())\n> > > diff --git a/src/cam/main.h b/src/cam/main.h\n> > > index ea8dfd3..a1fd899 100644\n> > > --- a/src/cam/main.h\n> > > +++ b/src/cam/main.h\n> > > @@ -17,6 +17,7 @@ enum {\n> > >       OptListProperties = 'p',\n> > >       OptMonitor = 'm',\n> > >       OptStream = 's',\n> > > +     OptZoom = 'z',\n> > >       OptListControls = 256,\n> > >       OptStrictFormats = 257,\n> > >  };\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","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 66AD3C3B5C\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 28 Sep 2020 11:15:45 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id E0CDB60CE7;\n\tMon, 28 Sep 2020 13:15:44 +0200 (CEST)","from relay10.mail.gandi.net (relay10.mail.gandi.net\n\t[217.70.178.230])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id EB15C60364\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 28 Sep 2020 13:15:42 +0200 (CEST)","from uno.localdomain (93-34-118-233.ip49.fastwebnet.it\n\t[93.34.118.233]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay10.mail.gandi.net (Postfix) with ESMTPSA id 72164240003;\n\tMon, 28 Sep 2020 11:15:41 +0000 (UTC)"],"Date":"Mon, 28 Sep 2020 13:19:37 +0200","From":"Jacopo Mondi <jacopo@jmondi.org>","To":"David Plowman <david.plowman@raspberrypi.com>","Message-ID":"<20200928111937.vj6bf5hfk3tnjekf@uno.localdomain>","References":"<20200925085127.17214-1-david.plowman@raspberrypi.com>\n\t<20200925085127.17214-7-david.plowman@raspberrypi.com>\n\t<20200928104422.k5imrdjq73gl6cqa@uno.localdomain>\n\t<CAHW6GYKtiWMp=1DRo=MbGy7wsVCzHP79CNDHkBS87AZ+rSCQpw@mail.gmail.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<CAHW6GYKtiWMp=1DRo=MbGy7wsVCzHP79CNDHkBS87AZ+rSCQpw@mail.gmail.com>","Subject":"Re: [libcamera-devel] [PATCH v2 6/6] cam: Add command line option\n\tto test IspCrop control","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":13159,"web_url":"https://patchwork.libcamera.org/comment/13159/","msgid":"<20201011004121.GB23159@pendragon.ideasonboard.com>","date":"2020-10-11T00:41:21","subject":"Re: [libcamera-devel] [PATCH v2 6/6] cam: Add command line option\n\tto test IspCrop control","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Niklas,\n\nOn Mon, Sep 28, 2020 at 01:07:35PM +0200, Niklas Söderlund wrote:\n> On 2020-09-25 09:51:27 +0100, David Plowman wrote:\n> > This allows the user to request digital zoom by adding, for example,\n> > \"-z 0.25,0.25,0.5,0.5\" which would give a 2x zoom, still centred in\n> > the middle of the image.\n> > \n> > Signed-off-by: David Plowman <david.plowman@raspberrypi.com>\n> > ---\n> >  src/cam/capture.cpp | 25 +++++++++++++++++++++++--\n> >  src/cam/capture.h   |  2 +-\n> >  src/cam/main.cpp    |  3 +++\n> >  src/cam/main.h      |  1 +\n> >  4 files changed, 28 insertions(+), 3 deletions(-)\n> > \n> > diff --git a/src/cam/capture.cpp b/src/cam/capture.cpp\n> > index 5510c00..3b50c59 100644\n> > --- a/src/cam/capture.cpp\n> > +++ b/src/cam/capture.cpp\n> > @@ -10,6 +10,9 @@\n> >  #include <limits.h>\n> >  #include <sstream>\n> >  \n> > +#include <libcamera/control_ids.h>\n> > +#include <libcamera/property_ids.h>\n> > +\n> >  #include \"capture.h\"\n> >  #include \"main.h\"\n> >  \n> > @@ -58,7 +61,7 @@ int Capture::run(const OptionsParser::Options &options)\n> >  \n> >  \tFrameBufferAllocator *allocator = new FrameBufferAllocator(camera_);\n> >  \n> > -\tret = capture(allocator);\n> > +\tret = capture(allocator, options);\n> >  \n> >  \tif (options.isSet(OptFile)) {\n> >  \t\tdelete writer_;\n> > @@ -70,7 +73,7 @@ int Capture::run(const OptionsParser::Options &options)\n> >  \treturn ret;\n> >  }\n> >  \n> > -int Capture::capture(FrameBufferAllocator *allocator)\n> > +int Capture::capture(FrameBufferAllocator *allocator, const OptionsParser::Options &options)\n> >  {\n> >  \tint ret;\n> >  \n> > @@ -120,6 +123,24 @@ int Capture::capture(FrameBufferAllocator *allocator)\n> >  \t\trequests.push_back(request);\n> >  \t}\n> >  \n> > +\t/* Set up digital zoom if it was requested on the command line. */\n> > +\tif (options.isSet(OptZoom)) {\n> > +\t\tstd::string zoom = options[OptZoom].toString();\n> > +\t\tfloat x, y, width, height;\n> > +\n> > +\t\tif (sscanf(zoom.c_str(), \"%f,%f,%f,%f\", &x, &y, &width, &height) == 4) {\n> \n> This is the wrong place to parse the input from the user. If the zoom \n> options are global something similar like StreamKeyValueParser \n> (ZoomKeyValueParser?) should be created.\n> \n> > +\t\t\tSize sensorArea = camera_->properties().get(properties::SensorOutputSize);\n> > +\t\t\tRectangle crop(x * sensorArea.width,\n> > +\t\t\t\t       y * sensorArea.height,\n> > +\t\t\t\t       width * sensorArea.width,\n> > +\t\t\t\t       height * sensorArea.height);\n> \n> \n> > +\n> > +\t\t\trequests.front()->controls().set(controls::IspCrop, crop);\n> \n> Do we wish to expose the ISP in the public API? If we do is the ISP \n> (from an application point of view) the correct level to operate on?  \n> Would not setting this on a stream level cover more hardware?\n> \n> I'm thinking of an ISP that can apply different crop parameters to two \n> (or more) source pads from a single sink pad. Having a control that sets \n> a global crop on the ISP would that not limit such hardware or am I \n> missing something?\n\nThis is a very good question, and a very fundamental design decision we\nneed to make. I've started thinking that we need to define and document\nan explicit abstract pipeline model for cameras, striking the right\nbalance between flexibility and adaptability to different hardware\narchitectures, and ease of implementation for pipeline handlers and of\nuse for applications.\n\nIn this context, David mentioned in a previous version of this series\nthat, while different streams could indeed have different zoom levels,\nuse cases for this feature were not immediately apparent. We could thus\nleave this out of our pipeline model. If we ever want to support this\nfeature in the future, I think we will be able to do so with the help of\nper-stream controls in requests, using the same IspCrop control, so it\nappears we would have a way forward.\n\n> > +\t\t} else {\n> > +\t\t\tstd::cout << \"Invalid zoom values - ignoring\" << std::endl;\n> > +\t\t}\n> > +\t}\n> > +\n> >  \tret = camera_->start();\n> >  \tif (ret) {\n> >  \t\tstd::cout << \"Failed to start capture\" << std::endl;\n> > diff --git a/src/cam/capture.h b/src/cam/capture.h\n> > index 0aebdac..5280616 100644\n> > --- a/src/cam/capture.h\n> > +++ b/src/cam/capture.h\n> > @@ -29,7 +29,7 @@ public:\n> >  \n> >  \tint run(const OptionsParser::Options &options);\n> >  private:\n> > -\tint capture(libcamera::FrameBufferAllocator *allocator);\n> > +\tint capture(libcamera::FrameBufferAllocator *allocator, const OptionsParser::Options &options);\n> >  \n> >  \tvoid requestComplete(libcamera::Request *request);\n> >  \n> > diff --git a/src/cam/main.cpp b/src/cam/main.cpp\n> > index 244720b..8f326d9 100644\n> > --- a/src/cam/main.cpp\n> > +++ b/src/cam/main.cpp\n> > @@ -201,6 +201,9 @@ int CamApp::parseOptions(int argc, char *argv[])\n> >  \tparser.addOption(OptStrictFormats, OptionNone,\n> >  \t\t\t \"Do not allow requested stream format(s) to be adjusted\",\n> >  \t\t\t \"strict-formats\");\n> > +\tparser.addOption(OptZoom, OptionString,\n> > +\t\t\t \"Specify digital zoom by supplying IspCrop control as x,y,width,height e.g. 0.25,0.25,0.5,0.5\",\n> > +\t\t\t \"zoom\", ArgumentRequired, \"zoom\");\n> >  \n> >  \toptions_ = parser.parse(argc, argv);\n> >  \tif (!options_.valid())\n> > diff --git a/src/cam/main.h b/src/cam/main.h\n> > index ea8dfd3..a1fd899 100644\n> > --- a/src/cam/main.h\n> > +++ b/src/cam/main.h\n> > @@ -17,6 +17,7 @@ enum {\n> >  \tOptListProperties = 'p',\n> >  \tOptMonitor = 'm',\n> >  \tOptStream = 's',\n> > +\tOptZoom = 'z',\n> >  \tOptListControls = 256,\n> >  \tOptStrictFormats = 257,\n> >  };","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 CBB69BEEDF\n\tfor <parsemail@patchwork.libcamera.org>;\n\tSun, 11 Oct 2020 00:42:07 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 697FB60358;\n\tSun, 11 Oct 2020 02:42:07 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 7B49760358\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSun, 11 Oct 2020 02:42:06 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id EBABB528;\n\tSun, 11 Oct 2020 02:42:05 +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=\"dDDvni8a\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1602376926;\n\tbh=ZTtJ7a7RsiR4o7HPH/uMH9v16Jyv92jorjw6SOxu/ho=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=dDDvni8a5pBCjBmHa3Vzz41w+foaHVzIle9+/FWf7d3Sj1nbTgQGBleiwWz0+azfO\n\tG00qxgXs0sWvLjHjvKL/fZoreV8pZDEQEbAjYN8c2NNqGewdhCXj1jnLx3Vc8z+zyC\n\tpuj6sXe8VydUuDHTPpzC767o6wQ46VsbO6oRJ7vM=","Date":"Sun, 11 Oct 2020 03:41:21 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Niklas =?utf-8?q?S=C3=B6derlund?= <niklas.soderlund@ragnatech.se>","Message-ID":"<20201011004121.GB23159@pendragon.ideasonboard.com>","References":"<20200925085127.17214-1-david.plowman@raspberrypi.com>\n\t<20200925085127.17214-7-david.plowman@raspberrypi.com>\n\t<20200928110735.GC2219@oden.dyn.berto.se>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200928110735.GC2219@oden.dyn.berto.se>","Subject":"Re: [libcamera-devel] [PATCH v2 6/6] cam: Add command line option\n\tto test IspCrop control","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=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]