[{"id":23742,"web_url":"https://patchwork.libcamera.org/comment/23742/","msgid":"<CAHW6GY+tjF7SB7+kgd_dR-DcGikZBbfTX7MSrBKS7qqe5Pgv-g@mail.gmail.com>","date":"2022-07-05T14:01:41","subject":"Re: [libcamera-devel] [PATCH v2] pipeline: raspberrypi: Fix\n\tincorrect advertising of ScalerCrop","submitter":{"id":42,"url":"https://patchwork.libcamera.org/api/people/42/","name":"David Plowman","email":"david.plowman@raspberrypi.com"},"content":"Hi Naush\n\nThanks for the new version!\n\nOn Tue, 5 Jul 2022 at 14:59, Naushir Patuck <naush@raspberrypi.com> wrote:\n>\n> The controls::ScalerCrop in the ControlInfoMap was advertised based on the ISP\n> output Rectangle. This is incorrect, it needs to be set based on the sensor\n> output Rectangle. Fix this.\n>\n> Additionally, do not use emplace to be consistent with the other controls set\n> in the ControlInfoMap.\n>\n> Fixes: 9dacde0d651d (pipeline: raspberrypi: Advertise ScalerCrop from the pipeline handler)\n> Reported-by: David Plowman <david.plowman@raspberrypi.com>\n> Signed-off-by: Naushir Patuck <naush@raspberrypi.com>\n> ---\n>  src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 5 +++--\n>  1 file changed, 3 insertions(+), 2 deletions(-)\n>\n> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> index 66a84b1dfb97..fdc24cd530c2 100644\n> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> @@ -946,8 +946,9 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config)\n>                 ctrlMap.emplace(c.first, c.second);\n>\n>         /* Add the ScalerCrop control limits based on the current mode. */\n> -       ctrlMap.emplace(&controls::ScalerCrop,\n> -                       ControlInfo(Rectangle(data->ispMinCropSize_), Rectangle(data->sensorInfo_.outputSize)));\n> +       Rectangle ispMinCrop(data->ispMinCropSize_);\n> +       ispMinCrop.scaleBy(data->sensorInfo_.analogCrop.size(), data->sensorInfo_.outputSize);\n> +       ctrlMap[&controls::ScalerCrop] = ControlInfo(ispMinCrop, Rectangle(data->sensorInfo_.analogCrop.size()));\n\nYep, I think I'm good with this now!\n\nReviewed-by: David Plowman <david.plowman@raspberrypi.com>\nTested-by: David Plowman <david.plowman@raspberrypi.com>\n\nThanks\nDavid\n\n>\n>         data->controlInfo_ = ControlInfoMap(std::move(ctrlMap), result.controlInfo.idmap());\n>\n> --\n> 2.25.1\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 2299FBD1F1\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue,  5 Jul 2022 14:01:55 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8AB8B63310;\n\tTue,  5 Jul 2022 16:01:54 +0200 (CEST)","from mail-ej1-x632.google.com (mail-ej1-x632.google.com\n\t[IPv6:2a00:1450:4864:20::632])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 2EA0961FB0\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue,  5 Jul 2022 16:01:53 +0200 (CEST)","by mail-ej1-x632.google.com with SMTP id fi2so21795503ejb.9\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 05 Jul 2022 07:01:53 -0700 (PDT)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1657029714;\n\tbh=yd1QUw5vFUMRBfGN+pM0fqPYLhzlBqQ31dGJXum7imo=;\n\th=References:In-Reply-To:Date:To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=AkWG4+17Qtcg1m1g51127x0A1nhChrkuQLC7Q/bNMQ0h9GIqe9KitIDCOeJ0JY4Sg\n\tI6T1HpT6Ty8RpPOpq5VkS8iFEBZC2mW5ZBpYgHCP53XB4ocPGJihHzhdDMtp5pFYsI\n\t1HhnK110g+c5I3NLaNATUx2SM6asmfqSmwhy+GT59qFt+G6kBVzYw1oJx3RcpJQ8jx\n\tL+rCQl92vEINeoJ/c6p6G0reM4IYVH4sMrPeex2f97V74XRo3s4MadMs0mS/LC8EY3\n\tY5NZt/ZrOPL4Z3ZsEaquJY/OUhQsXQK/nd2G0q+ErhC34dSaKLNvjYyugvvA9AaUbr\n\to1DhYbRXQEQ7A==","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=T8eL712nd3IaKdoKq6k/z14X8afF/cGXEO7ldMjvT/g=;\n\tb=BKDWVJHg+cS85iUE0y2XQfvF9g0iTOb3BGBAQn1h3Kv2DAP0o5l4HASxRIYYyGB4lR\n\t9v0oXJXMeAfKyjvf/HF8dGOGsMnWpWN4eR7afMQlAJlyPcjIZXHuDqNq10GRMgBqYhMD\n\t1JqDZAzCm0yFGBeZ7xou5WBV1j6KecNdYVbKIRrHIH1PU0u6ac3YL7E6WGZJQqUd3373\n\t9U28eSfq+WWzFQ5nKVL4EKWXDTgnNlyAJ72ydggKKSdhAG80PIIMlPEiH+zdQUV2Y/dB\n\tOkYy+9a9hgatvWd8n/kNsaC5g9z0UiSKQalDr5JEr1cLmXmYzyhp7s1KF45AmRo1ZSGG\n\tVTig=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=raspberrypi.com\n\theader.i=@raspberrypi.com\n\theader.b=\"BKDWVJHg\"; dkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=T8eL712nd3IaKdoKq6k/z14X8afF/cGXEO7ldMjvT/g=;\n\tb=ujf6oXmRLTfBtP9vDoKW2PgC52yLO5fkON21I6U5un79RoCvhxlcZo6c2aRBjSIL/E\n\tTp1Ilvb3fKcZtSSFVjf6gKIpbQEsb/du8SSYwum7WZOxxOnkyJMUSYggeMhQVHz8E7g8\n\tucssWIYZx2DJxYVzU78WUmeneNAr1FKVdZuh03GldCkFLx24UgpS1BdCYKNYnOt03cSi\n\tEfE1ePaRpU2LlMpj05NEHtUi/j5t5uoHjCwDhI7pVibeQx0LcfPZSavuvt6BOQrmib0k\n\tWgNCMT+ySb4fH3VtxDLKvszktUfapdRjSuULYw0yzm7rRFL+GIfFdOi+J4RkYCsAnnl9\n\tJOtQ==","X-Gm-Message-State":"AJIora9pZDmt468pKKVwEdu13RGIyaz94fYElSwdTS53m3/HhHjCYX5b\n\tnb1WawygyI/2+/3AT1J39ayQJ6LsKoNOmf9GKpV6e91hJ9Q=","X-Google-Smtp-Source":"AGRyM1utKr37UG9nSrn0g0fIdyHAu8cIqVSj97SFfJq+lA9VcmX/w7vre118NoqjlFE2wrDLaBWeuUyN+N6vWsU3XRY=","X-Received":"by 2002:a17:907:2d22:b0:726:34e6:52f6 with SMTP id\n\tgs34-20020a1709072d2200b0072634e652f6mr32490222ejc.371.1657029712390;\n\tTue, 05 Jul 2022 07:01:52 -0700 (PDT)","MIME-Version":"1.0","References":"<20220705135956.30444-1-naush@raspberrypi.com>","In-Reply-To":"<20220705135956.30444-1-naush@raspberrypi.com>","Date":"Tue, 5 Jul 2022 15:01:41 +0100","Message-ID":"<CAHW6GY+tjF7SB7+kgd_dR-DcGikZBbfTX7MSrBKS7qqe5Pgv-g@mail.gmail.com>","To":"Naushir Patuck <naush@raspberrypi.com>","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH v2] pipeline: raspberrypi: Fix\n\tincorrect advertising of ScalerCrop","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>","From":"David Plowman via libcamera-devel <libcamera-devel@lists.libcamera.org>","Reply-To":"David Plowman <david.plowman@raspberrypi.com>","Cc":"libcamera devel <libcamera-devel@lists.libcamera.org>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":23817,"web_url":"https://patchwork.libcamera.org/comment/23817/","msgid":"<CAEmqJPrd-QB7zXpzncYhp+tzz-0wZ5hY1q=wxg8vyct2AwXbYQ@mail.gmail.com>","date":"2022-07-11T12:02:57","subject":"Re: [libcamera-devel] [PATCH v2] pipeline: raspberrypi: Fix\n\tincorrect advertising of ScalerCrop","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hi,\n\nPing for a review please.\n\nNaush\n\nOn Tue, 5 Jul 2022 at 14:59, Naushir Patuck <naush@raspberrypi.com> wrote:\n\n> The controls::ScalerCrop in the ControlInfoMap was advertised based on the\n> ISP\n> output Rectangle. This is incorrect, it needs to be set based on the sensor\n> output Rectangle. Fix this.\n>\n> Additionally, do not use emplace to be consistent with the other controls\n> set\n> in the ControlInfoMap.\n>\n> Fixes: 9dacde0d651d (pipeline: raspberrypi: Advertise ScalerCrop from the\n> pipeline handler)\n> Reported-by: David Plowman <david.plowman@raspberrypi.com>\n> Signed-off-by: Naushir Patuck <naush@raspberrypi.com>\n> ---\n>  src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 5 +++--\n>  1 file changed, 3 insertions(+), 2 deletions(-)\n>\n> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> index 66a84b1dfb97..fdc24cd530c2 100644\n> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> @@ -946,8 +946,9 @@ int PipelineHandlerRPi::configure(Camera *camera,\n> CameraConfiguration *config)\n>                 ctrlMap.emplace(c.first, c.second);\n>\n>         /* Add the ScalerCrop control limits based on the current mode. */\n> -       ctrlMap.emplace(&controls::ScalerCrop,\n> -                       ControlInfo(Rectangle(data->ispMinCropSize_),\n> Rectangle(data->sensorInfo_.outputSize)));\n> +       Rectangle ispMinCrop(data->ispMinCropSize_);\n> +       ispMinCrop.scaleBy(data->sensorInfo_.analogCrop.size(),\n> data->sensorInfo_.outputSize);\n> +       ctrlMap[&controls::ScalerCrop] = ControlInfo(ispMinCrop,\n> Rectangle(data->sensorInfo_.analogCrop.size()));\n>\n>         data->controlInfo_ = ControlInfoMap(std::move(ctrlMap),\n> result.controlInfo.idmap());\n>\n> --\n> 2.25.1\n>\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 40A74BE173\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 11 Jul 2022 12:03:14 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 40DB76330D;\n\tMon, 11 Jul 2022 14:03:13 +0200 (CEST)","from mail-lj1-x236.google.com (mail-lj1-x236.google.com\n\t[IPv6:2a00:1450:4864:20::236])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id B70AB60401\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 11 Jul 2022 14:03:10 +0200 (CEST)","by mail-lj1-x236.google.com with SMTP id a39so5903684ljq.11\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 11 Jul 2022 05:03:10 -0700 (PDT)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1657540993;\n\tbh=OV+dq4laspaad+R0NLo+57hrHu93KEhS7m92oqI+C1o=;\n\th=References:In-Reply-To:Date:To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:\n\tFrom;\n\tb=g6c2vVYh5YhT+pgEq36Ltg0EB4so1h5BKnyF52wCQKMRJ6/xSt6v+EztACeSrRJOq\n\tEI+pT9XQxVpw9UTS+THcqGNI3jOZzogJPfipi+mqwtGrSwwsmrYsHmLrZW5n7UcrcB\n\ti47J0VJDhLEzDFhdWkXanvIbTuDiJZgCCt8nxqthwPjq30GgY4YPWRQEa9HeytvbSy\n\tkNj0APIpRjvpZdotOVCi8dsi1nlhLUoWNAFJNqvWX0jV7W0Pbf4jOSEujh43yIdp3u\n\tmH8Y96i3f1Bij6DKGbRSABSQgnCU57Ju2bK1xG3yeOl1I4u9e8eybjKF5olhO3po+0\n\t8f4Dh2pe92QhA==","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=rC+DYnKf0MoPhdKSYhmEX1Pz+lBNgj1nGI8E3AHDbw8=;\n\tb=saZQAm5BeuAaBs5K0zg76V2cixr1b73X7EIQdk0fAqzKf+GvNeSc1mw6pzaj001tfP\n\tgczdjB203szhleSdZW3fPKQ3KUwzhzJhq5HinkyBHP8VWeNIv130qmXi8Oib4WaQ6u9A\n\tJo5UWT4Hmwwu1FuipvUXBiFe39owSw9PRuR8jUwI62YeFouzOqs4agGjZDXJYeZq92CL\n\towFLDRZ55GaGSGn9lH9tnyb5Hok4f5kqvyv7spJpMBlQgHUmzxr2k+jWzW/2n7ThhKbp\n\tllBqn1dYEgh9WJvdAkUFPswORmXNaLMNNGCt/DkUnf+9PPh4VSfncAbr2cj+4aSZSgzL\n\tCYdg=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=raspberrypi.com\n\theader.i=@raspberrypi.com\n\theader.b=\"saZQAm5B\"; dkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=rC+DYnKf0MoPhdKSYhmEX1Pz+lBNgj1nGI8E3AHDbw8=;\n\tb=NLDNxQmlInVdcVidplhqLMU1lbqNHuGE68r7R0BGpWOSSYroRvALHFCupfOwfi5W6r\n\tNUlRgWh6FJu1Mv8D8RzyN65b8oNRXNYVI/027kOO0zWZ7AZzNYgBeJSKoff2PCw3TaFs\n\tiEdCfCLcpZ2ES9jLfSnjMsacmePUpLrV5SRelwTSK/h1EkWgJ11Ta+JjpIpkiwWVeM44\n\tr+jsuPp4dCjw4IcILDL1AErWlaC7Y1V371NlfUTu1qpDfAzek2IRrxQYHXhZvWzulICB\n\tFrvJb8kR+aTPDxLd6wVKKBGf/+/m3GfoZwDEiUkydqZSYR/w1k1q61vOx2tEZT1dWE1P\n\tMDmA==","X-Gm-Message-State":"AJIora+3mlmat/iMQhwqwKS8k1h7ajar8ssmFynli5MA6rwJ/536SX6t\n\tYE5ZlUBUr/dmwy3B/oTRMDtp5HDKE5p7jaT/7UEB5c9IpQBRWA==","X-Google-Smtp-Source":"AGRyM1tTVCacckE/4gpH2XQol/JYfT+Es+t/YdQnX09iqr9inkSqhZ1OGFm8Dde/Lf4zqlYm/gBLtB1fLGhQn8kPPGE=","X-Received":"by 2002:a05:651c:1a0e:b0:25b:baac:f415 with SMTP id\n\tby14-20020a05651c1a0e00b0025bbaacf415mr10099526ljb.480.1657540989654;\n\tMon, 11 Jul 2022 05:03:09 -0700 (PDT)","MIME-Version":"1.0","References":"<20220705135956.30444-1-naush@raspberrypi.com>","In-Reply-To":"<20220705135956.30444-1-naush@raspberrypi.com>","Date":"Mon, 11 Jul 2022 13:02:57 +0100","Message-ID":"<CAEmqJPrd-QB7zXpzncYhp+tzz-0wZ5hY1q=wxg8vyct2AwXbYQ@mail.gmail.com>","To":"libcamera devel <libcamera-devel@lists.libcamera.org>","Content-Type":"multipart/alternative; boundary=\"000000000000e3ae6c05e386551c\"","Subject":"Re: [libcamera-devel] [PATCH v2] pipeline: raspberrypi: Fix\n\tincorrect advertising of ScalerCrop","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>","From":"Naushir Patuck via libcamera-devel\n\t<libcamera-devel@lists.libcamera.org>","Reply-To":"Naushir Patuck <naush@raspberrypi.com>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":23826,"web_url":"https://patchwork.libcamera.org/comment/23826/","msgid":"<20220712072331.khespozodgehicri@uno.localdomain>","date":"2022-07-12T07:23:31","subject":"Re: [libcamera-devel] [PATCH v2] pipeline: raspberrypi: Fix\n\tincorrect advertising of ScalerCrop","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Naush,\n\nOn Tue, Jul 05, 2022 at 02:59:56PM +0100, Naushir Patuck via libcamera-devel wrote:\n> The controls::ScalerCrop in the ControlInfoMap was advertised based on the ISP\n> output Rectangle. This is incorrect, it needs to be set based on the sensor\n> output Rectangle. Fix this.\n\nI might have not completely woken up yet but the existing\n\n\tctrlMap.emplace(&controls::ScalerCrop,\n\t\t\tControlInfo(Rectangle(data->ispMinCropSize_), Rectangle(data->sensorInfo_.outputSize)));\n\nSeems to be based on the sensor output size, not the ISP output size\nas you mention here.\n\nWhat am I missing ?\n\n>\n> Additionally, do not use emplace to be consistent with the other controls set\n> in the ControlInfoMap.\n>\n> Fixes: 9dacde0d651d (pipeline: raspberrypi: Advertise ScalerCrop from the pipeline handler)\n> Reported-by: David Plowman <david.plowman@raspberrypi.com>\n> Signed-off-by: Naushir Patuck <naush@raspberrypi.com>\n> ---\n>  src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 5 +++--\n>  1 file changed, 3 insertions(+), 2 deletions(-)\n>\n> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> index 66a84b1dfb97..fdc24cd530c2 100644\n> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> @@ -946,8 +946,9 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config)\n>  \t\tctrlMap.emplace(c.first, c.second);\n>\n>  \t/* Add the ScalerCrop control limits based on the current mode. */\n> -\tctrlMap.emplace(&controls::ScalerCrop,\n> -\t\t\tControlInfo(Rectangle(data->ispMinCropSize_), Rectangle(data->sensorInfo_.outputSize)));\n> +\tRectangle ispMinCrop(data->ispMinCropSize_);\n> +\tispMinCrop.scaleBy(data->sensorInfo_.analogCrop.size(), data->sensorInfo_.outputSize);\n\nWhat is the purpose of scaling the minimum accepted ISP input in the\nsensor's analogue crop / sensor's output size ratio ?\n\n> +\tctrlMap[&controls::ScalerCrop] = ControlInfo(ispMinCrop, Rectangle(data->sensorInfo_.analogCrop.size()));\n\nAnd I don't get this one either, how can the maximum scaler rectangle\nbe larger than the sensor's output size ? (assumiming\nsensor->analogCrop > sensor->outputSize).\n\nI'm slightly confused :)\n>\n>  \tdata->controlInfo_ = ControlInfoMap(std::move(ctrlMap), result.controlInfo.idmap());\n>\n> --\n> 2.25.1\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 F00D3BE173\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 12 Jul 2022 07:23:35 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5728763312;\n\tTue, 12 Jul 2022 09:23:35 +0200 (CEST)","from relay5-d.mail.gandi.net (relay5-d.mail.gandi.net\n\t[IPv6:2001:4b98:dc4:8::225])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 207336330E\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 12 Jul 2022 09:23:34 +0200 (CEST)","(Authenticated sender: jacopo@jmondi.org)\n\tby mail.gandi.net (Postfix) with ESMTPSA id 633B61C000C;\n\tTue, 12 Jul 2022 07:23:32 +0000 (UTC)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1657610615;\n\tbh=PCRai/Hgp0X+1GBPBOq/KgccS02fTr1H+Jg9kRbuC7g=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=RWmQF+nt9qqn2rXVTd9QtjidbodFUby7hxnHOifX7spgh85X+EMahWsrTi2v8Svtf\n\tI7FfBFpj2FZZx/MDa+GWODOO1hqryuh95Libntb3priOYDryZeRPpvQjF+oHb85BuP\n\tCMDomuliy87nLc5NKxBkLSaa/+Zs2ZrjxSb7qTVpU0L8Z6n/9hZjnD/JY4lA/INu7H\n\t8dTjUJiTikhb7yjNa7kQYP+TzEU5TTwNgPhyQ85HFLVy0tCjvZYwWQ96geZEA2j/ml\n\tjqMnUGyuknF5KlgmPrbkSwbPCBTqhua080Y+vLYSZ5QqU2cN2T9deDW72RXXVvPXRx\n\t6C/mvH61B45gA==","Date":"Tue, 12 Jul 2022 09:23:31 +0200","To":"Naushir Patuck <naush@raspberrypi.com>","Message-ID":"<20220712072331.khespozodgehicri@uno.localdomain>","References":"<20220705135956.30444-1-naush@raspberrypi.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20220705135956.30444-1-naush@raspberrypi.com>","Subject":"Re: [libcamera-devel] [PATCH v2] pipeline: raspberrypi: Fix\n\tincorrect advertising of ScalerCrop","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>","From":"Jacopo Mondi via libcamera-devel <libcamera-devel@lists.libcamera.org>","Reply-To":"Jacopo Mondi <jacopo@jmondi.org>","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":23830,"web_url":"https://patchwork.libcamera.org/comment/23830/","msgid":"<CAEmqJPrGqRVU2sw-xrkM41Fhg7vFSaZd3w7LqpAtXSt83v4jqA@mail.gmail.com>","date":"2022-07-12T09:03:58","subject":"Re: [libcamera-devel] [PATCH v2] pipeline: raspberrypi: Fix\n\tincorrect advertising of ScalerCrop","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hi Jacopo,\n\nThis is all very confusing for me as well, so I might be wrong in my\nexplanation :-)\n\nOn Tue, 12 Jul 2022 at 08:23, Jacopo Mondi <jacopo@jmondi.org> wrote:\n\n> Hi Naush,\n>\n> On Tue, Jul 05, 2022 at 02:59:56PM +0100, Naushir Patuck via\n> libcamera-devel wrote:\n> > The controls::ScalerCrop in the ControlInfoMap was advertised based on\n> the ISP\n> > output Rectangle. This is incorrect, it needs to be set based on the\n> sensor\n> > output Rectangle. Fix this.\n>\n> I might have not completely woken up yet but the existing\n>\n>         ctrlMap.emplace(&controls::ScalerCrop,\n>                         ControlInfo(Rectangle(data->ispMinCropSize_),\n> Rectangle(data->sensorInfo_.outputSize)));\n>\n> Seems to be based on the sensor output size, not the ISP output size\n> as you mention here.\n>\n> What am I missing ?\n>\n\nispMinCropSize_ is based on the ISP's minimum output size (64x64).  This\ndoes not have\nany relation to the sensor size, so....\n\n\n>\n> >\n> > Additionally, do not use emplace to be consistent with the other\n> controls set\n> > in the ControlInfoMap.\n> >\n> > Fixes: 9dacde0d651d (pipeline: raspberrypi: Advertise ScalerCrop from\n> the pipeline handler)\n> > Reported-by: David Plowman <david.plowman@raspberrypi.com>\n> > Signed-off-by: Naushir Patuck <naush@raspberrypi.com>\n> > ---\n> >  src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 5 +++--\n> >  1 file changed, 3 insertions(+), 2 deletions(-)\n> >\n> > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > index 66a84b1dfb97..fdc24cd530c2 100644\n> > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > @@ -946,8 +946,9 @@ int PipelineHandlerRPi::configure(Camera *camera,\n> CameraConfiguration *config)\n> >               ctrlMap.emplace(c.first, c.second);\n> >\n> >       /* Add the ScalerCrop control limits based on the current mode. */\n> > -     ctrlMap.emplace(&controls::ScalerCrop,\n> > -                     ControlInfo(Rectangle(data->ispMinCropSize_),\n> Rectangle(data->sensorInfo_.outputSize)));\n> > +     Rectangle ispMinCrop(data->ispMinCropSize_);\n> > +     ispMinCrop.scaleBy(data->sensorInfo_.analogCrop.size(),\n> data->sensorInfo_.outputSize);\n>\n> What is the purpose of scaling the minimum accepted ISP input in the\n> sensor's analogue crop / sensor's output size ratio ?\n>\n\n.... this bit of code scales the ispMinCropSize_  (64x64) relative to the\nsensor input.\n\n\n>\n> > +     ctrlMap[&controls::ScalerCrop] = ControlInfo(ispMinCrop,\n> Rectangle(data->sensorInfo_.analogCrop.size()));\n>\n> And I don't get this one either, how can the maximum scaler rectangle\n> be larger than the sensor's output size ? (assumiming\n> sensor->analogCrop > sensor->outputSize).\n>\n\nIf I understand correctly, the ScalerCrop control is based on the\n\"unbinned\" resolution,\ni.e. full analogue crop readout of the sensor.  The translation to sensor\noutput scale\nhappens internally in the pipeline handler here:\nhttps://git.libcamera.org/libcamera/libcamera.git/tree/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp#n2052\n\nFor imx477, this is the result:\n\nSensor mode 4056x3040:\nScalerCrop : [(0, 0)/64x64..(0, 0)/4056x3040]\n\nSensor mode 2028x1520:\nScalerCrop : [(0, 0)/128x128..(0, 0)/4056x3040]\n\nSensor mode 2028x1080:\nScalerCrop : [(0, 0)/128x128..(0, 0)/4056x2160]\n\nSensor mode 1332x990:\nScalerCrop : [(0, 0)/128x128..(0, 0)/2664x1980]\n\nI'm slightly confused :)\n>\n\nI was as well :-) David had to patiently work though all this with me to\nget an understanding on what's going on.\n\nRegards,\nNaush\n\n\n> >\n> >       data->controlInfo_ = ControlInfoMap(std::move(ctrlMap),\n> result.controlInfo.idmap());\n> >\n> > --\n> > 2.25.1\n> >\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 6137EBE173\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 12 Jul 2022 09:04:16 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 92D0F63312;\n\tTue, 12 Jul 2022 11:04:15 +0200 (CEST)","from mail-lf1-x12a.google.com (mail-lf1-x12a.google.com\n\t[IPv6:2a00:1450:4864:20::12a])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id DBCBA6330E\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 12 Jul 2022 11:04:14 +0200 (CEST)","by mail-lf1-x12a.google.com with SMTP id r9so4836824lfp.10\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 12 Jul 2022 02:04:14 -0700 (PDT)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1657616655;\n\tbh=XlWqk/r/dpNtZBRNour9iZ3UToldTi5fB82bd35FKXY=;\n\th=References:In-Reply-To:Date:To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=zBGlmzx5Pee9scD3VN3ohuc8vdvXWsKIG1qkALStB1EQeY/Z+3cextYH1iGU62m7l\n\t/JjSqHxctkBT71NATs3sIqUeBrTv+ZUTfvxexZuh2C71fKmQ0YgvQtvyYm+o2ygtfW\n\tHv+SFNj6AvQK8uVNAn92jNlkrasspOGFxJTRE5OGZuDi7vyAvMCO4NYySot3A0sjjM\n\t9FRjtcD6cE2hqbeVVCVXm4GZe0TgxaST0rir+ngbJ/+iaFNtJit32wXf7s3Lgix60s\n\tRPcqyWOT0jRZ5tTKVAx+2i9GiboNh7zHJ80dvkcNQ8M8Jm9XdqY3ALvDzP7MkUlIy4\n\tkoB2msX//oivA==","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=o3VLu1Fle4kIPLwFkA5vYcd1Z5XQXGqK8kxGFJ2uOv0=;\n\tb=hNvMiaPKtv6flukprWJxM67W+x5+UjzE92kwXS//Lv9RODxfCbj8YxJzd6aRC4IRXf\n\t95n8Hsha5/6q/ZGHXndrMKdvfe58ZV4I3TjwIHePI6AGFt0c9yL9BZrfS/+Lb1+5abWx\n\t9EilFkFTc8dIjSeTQOeNiGrQw8vt5avLwGFcXxSZlX0Mqu8JO2JyggJuThEqtRIvgyC4\n\tzPiDI78wJQ5KmZUPWSM2nOqJa3ouJ7Vsg7PmeQ84TIExkrwKQ7UuizGk32EXoiicEe9r\n\t+Cn8R/1bPGcOzutmhEnF+AyQuBlyOLHfUHkai7g0Mea5QXSXqUWAi0Irf9l9nOgDufQi\n\tqPvQ=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=raspberrypi.com\n\theader.i=@raspberrypi.com\n\theader.b=\"hNvMiaPK\"; dkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=o3VLu1Fle4kIPLwFkA5vYcd1Z5XQXGqK8kxGFJ2uOv0=;\n\tb=6af5rw6zYTumelDoIqwVz1WaXb80jtvwYOEI+Wjdwo5IBT9lJUYdbj1ujoTQO7Milq\n\tyPAzarYf5XQyqZVvNwZguOk6F4KV1yz2mwr3HECCKmdaqpsqUTmQMDD8dXpVKZew3R5U\n\tw8eZ7IMmRxKLqn50GcxPkSI+QZVj+dy2j63Mnfcmo1Ca4zaqr3jdJdYLOFPIEq8Rz0kV\n\tgfo0QQ3bwuY0FjDaW3qkdfEk/mNf0iFsO/gZ5THmtBfXSkCEJRf8m2thZXd5/7S/xxV4\n\tRqL4jUnEcSSImHa3zwRR5b7D7kHj1+z8c6Ensor63h2HCIMqWDuLyNUrBjkDybrwd1cD\n\tbFtA==","X-Gm-Message-State":"AJIora/b7ZPjEmtOkgeAB/U/WZvTcn12nby/m4twdl7wn7AtFmr7WNJR\n\taWNFNVrwjTApjKpz8bkGzlPFmDmivJuO4RzDcuDMs4kbfVocDKXh","X-Google-Smtp-Source":"AGRyM1vDyQ/KrLVpP6veccSTTBnkME1zKFe1ZhQz1zXkxGUISxx3F4l3qwi1/4g6OU7vbBtuVv8S8zUMdsEScZ6UbF4=","X-Received":"by 2002:a05:6512:4019:b0:489:3345:c450 with SMTP id\n\tbr25-20020a056512401900b004893345c450mr13864553lfb.363.1657616654205;\n\tTue, 12 Jul 2022 02:04:14 -0700 (PDT)","MIME-Version":"1.0","References":"<20220705135956.30444-1-naush@raspberrypi.com>\n\t<20220712072331.khespozodgehicri@uno.localdomain>","In-Reply-To":"<20220712072331.khespozodgehicri@uno.localdomain>","Date":"Tue, 12 Jul 2022 10:03:58 +0100","Message-ID":"<CAEmqJPrGqRVU2sw-xrkM41Fhg7vFSaZd3w7LqpAtXSt83v4jqA@mail.gmail.com>","To":"Jacopo Mondi <jacopo@jmondi.org>","Content-Type":"multipart/alternative; boundary=\"000000000000d9181105e397f3ee\"","Subject":"Re: [libcamera-devel] [PATCH v2] pipeline: raspberrypi: Fix\n\tincorrect advertising of ScalerCrop","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>","From":"Naushir Patuck via libcamera-devel\n\t<libcamera-devel@lists.libcamera.org>","Reply-To":"Naushir Patuck <naush@raspberrypi.com>","Cc":"libcamera devel <libcamera-devel@lists.libcamera.org>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":23833,"web_url":"https://patchwork.libcamera.org/comment/23833/","msgid":"<20220712102019.mnv57bp7fbqqaaso@uno.localdomain>","date":"2022-07-12T10:20:19","subject":"Re: [libcamera-devel] [PATCH v2] pipeline: raspberrypi: Fix\n\tincorrect advertising of ScalerCrop","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"On Tue, Jul 12, 2022 at 10:03:58AM +0100, Naushir Patuck wrote:\n> Hi Jacopo,\n>\n> This is all very confusing for me as well, so I might be wrong in my\n> explanation :-)\n>\n> On Tue, 12 Jul 2022 at 08:23, Jacopo Mondi <jacopo@jmondi.org> wrote:\n>\n> > Hi Naush,\n> >\n> > On Tue, Jul 05, 2022 at 02:59:56PM +0100, Naushir Patuck via\n> > libcamera-devel wrote:\n> > > The controls::ScalerCrop in the ControlInfoMap was advertised based on\n> > the ISP\n> > > output Rectangle. This is incorrect, it needs to be set based on the\n> > sensor\n> > > output Rectangle. Fix this.\n> >\n> > I might have not completely woken up yet but the existing\n> >\n> >         ctrlMap.emplace(&controls::ScalerCrop,\n> >                         ControlInfo(Rectangle(data->ispMinCropSize_),\n> > Rectangle(data->sensorInfo_.outputSize)));\n> >\n> > Seems to be based on the sensor output size, not the ISP output size\n> > as you mention here.\n> >\n> > What am I missing ?\n> >\n>\n> ispMinCropSize_ is based on the ISP's minimum output size (64x64).  This\n> does not have\n> any relation to the sensor size, so....\n>\n>\n> >\n> > >\n> > > Additionally, do not use emplace to be consistent with the other\n> > controls set\n> > > in the ControlInfoMap.\n> > >\n> > > Fixes: 9dacde0d651d (pipeline: raspberrypi: Advertise ScalerCrop from\n> > the pipeline handler)\n> > > Reported-by: David Plowman <david.plowman@raspberrypi.com>\n> > > Signed-off-by: Naushir Patuck <naush@raspberrypi.com>\n> > > ---\n> > >  src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 5 +++--\n> > >  1 file changed, 3 insertions(+), 2 deletions(-)\n> > >\n> > > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > > index 66a84b1dfb97..fdc24cd530c2 100644\n> > > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > > @@ -946,8 +946,9 @@ int PipelineHandlerRPi::configure(Camera *camera,\n> > CameraConfiguration *config)\n> > >               ctrlMap.emplace(c.first, c.second);\n> > >\n> > >       /* Add the ScalerCrop control limits based on the current mode. */\n> > > -     ctrlMap.emplace(&controls::ScalerCrop,\n> > > -                     ControlInfo(Rectangle(data->ispMinCropSize_),\n> > Rectangle(data->sensorInfo_.outputSize)));\n> > > +     Rectangle ispMinCrop(data->ispMinCropSize_);\n> > > +     ispMinCrop.scaleBy(data->sensorInfo_.analogCrop.size(),\n> > data->sensorInfo_.outputSize);\n> >\n> > What is the purpose of scaling the minimum accepted ISP input in the\n> > sensor's analogue crop / sensor's output size ratio ?\n> >\n>\n> .... this bit of code scales the ispMinCropSize_  (64x64) relative to the\n> sensor input.\n\nI was missing that ScalerCrop is specified relatively to the analogue\ngain rectangle, so re-scaling it in the (analogue crop / sensor\noutput) ratio makes sure the sensor's mode binning/subsampling is\ntaken into account.\n\n>\n>\n> >\n> > > +     ctrlMap[&controls::ScalerCrop] = ControlInfo(ispMinCrop,\n> > Rectangle(data->sensorInfo_.analogCrop.size()));\n> >\n> > And I don't get this one either, how can the maximum scaler rectangle\n> > be larger than the sensor's output size ? (assumiming\n> > sensor->analogCrop > sensor->outputSize).\n> >\n>\n> If I understand correctly, the ScalerCrop control is based on the\n> \"unbinned\" resolution,\n> i.e. full analogue crop readout of the sensor.  The translation to sensor\n> output scale\n> happens internally in the pipeline handler here:\n> https://git.libcamera.org/libcamera/libcamera.git/tree/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp#n2052\n>\n> For imx477, this is the result:\n>\n> Sensor mode 4056x3040:\n> ScalerCrop : [(0, 0)/64x64..(0, 0)/4056x3040]\n>\n> Sensor mode 2028x1520:\n> ScalerCrop : [(0, 0)/128x128..(0, 0)/4056x3040]\n>\n> Sensor mode 2028x1080:\n> ScalerCrop : [(0, 0)/128x128..(0, 0)/4056x2160]\n>\n> Sensor mode 1332x990:\n> ScalerCrop : [(0, 0)/128x128..(0, 0)/2664x1980]\n\nWeird, the driver says this mode is 4x4 binned in one place but it's\ninstead cropped down to 2664x1980 then 2x2 binned.\n\nAnyway, these nicely match the driver's analogue crop rectangles.\n\n>\n> I'm slightly confused :)\n> >\n>\n> I was as well :-) David had to patiently work though all this with me to\n> get an understanding on what's going on.\n\nThanks for having the patience to re-explain this to me.\n\nIf I'm not mistaken then the commit message should say:\n\nThe controls::ScalerCrop in the ControlInfoMap was advertised based on\nthe ISP output Rectangle. This is incorrect, it needs to be set based\n- on the sensor output Rectangle. Fix this.\n+ on the sensor analogue crop Rectangle. Fix this.\n\nThe patch looks good and the commit message can be updated (if you\nagree with the change) when applying.\n\nReviewed-by: Jacopo Mondi <jacopo@jmondi.org>\n\nThanks\n   j\n\n>\n> Regards,\n> Naush\n>\n>\n> > >\n> > >       data->controlInfo_ = ControlInfoMap(std::move(ctrlMap),\n> > result.controlInfo.idmap());\n> > >\n> > > --\n> > > 2.25.1\n> > >\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 4E522BE173\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 12 Jul 2022 10:20:24 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 1AE0F63313;\n\tTue, 12 Jul 2022 12:20:24 +0200 (CEST)","from relay3-d.mail.gandi.net (relay3-d.mail.gandi.net\n\t[IPv6:2001:4b98:dc4:8::223])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id BD3326330B\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 12 Jul 2022 12:20:22 +0200 (CEST)","(Authenticated sender: jacopo@jmondi.org)\n\tby mail.gandi.net (Postfix) with ESMTPSA id 6E2A16000E;\n\tTue, 12 Jul 2022 10:20:21 +0000 (UTC)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1657621224;\n\tbh=higmHA3j0H3OpF3z/MeZhVXUquwuAh81us0Q1mNb4iA=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=lZOm6tGNXdRjAIp8MSRxXfgIDFbIxqlHCM32b0Q9NlMeUrkoIOAZJ8BPq7ToQunCN\n\tUOdMIWGwdkAIt59Fo8lIEurBMfUAzd5b1Cj0bJTAtFRThFUAi90EYcFuNKAv3Rc+T2\n\tPy8rLr9/YGTR7ToF8/b4VCZGdJnWUz5Jh+0MveMz2cBJwvWSvB8RNNLTSUji5sKy71\n\t3K4cS+ZstgdMGC4f6CatoI2xjXxIgQCWFrg26mHyMWWEJHR1INz7TBjv+iHT4Lq2Lq\n\tZzjweAk8DSsEzu38H38FdYXjYi7CH+M3XtJ1FBlLC4TcT9BM6h9LZFQFtsmr6JqtDh\n\tGUDqiKtUF/ZIQ==","Date":"Tue, 12 Jul 2022 12:20:19 +0200","To":"Naushir Patuck <naush@raspberrypi.com>","Message-ID":"<20220712102019.mnv57bp7fbqqaaso@uno.localdomain>","References":"<20220705135956.30444-1-naush@raspberrypi.com>\n\t<20220712072331.khespozodgehicri@uno.localdomain>\n\t<CAEmqJPrGqRVU2sw-xrkM41Fhg7vFSaZd3w7LqpAtXSt83v4jqA@mail.gmail.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<CAEmqJPrGqRVU2sw-xrkM41Fhg7vFSaZd3w7LqpAtXSt83v4jqA@mail.gmail.com>","Subject":"Re: [libcamera-devel] [PATCH v2] pipeline: raspberrypi: Fix\n\tincorrect advertising of ScalerCrop","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>","From":"Jacopo Mondi via libcamera-devel <libcamera-devel@lists.libcamera.org>","Reply-To":"Jacopo Mondi <jacopo@jmondi.org>","Cc":"libcamera devel <libcamera-devel@lists.libcamera.org>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":23834,"web_url":"https://patchwork.libcamera.org/comment/23834/","msgid":"<CAEmqJPrw=s8v-4eAX9d9sY=uHijR0dr1kpBcSjKsFu2ZbyqoTA@mail.gmail.com>","date":"2022-07-12T10:26:30","subject":"Re: [libcamera-devel] [PATCH v2] pipeline: raspberrypi: Fix\n\tincorrect advertising of ScalerCrop","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hi Jacopo,\n\nOn Tue, 12 Jul 2022 at 11:20, Jacopo Mondi <jacopo@jmondi.org> wrote:\n\n> On Tue, Jul 12, 2022 at 10:03:58AM +0100, Naushir Patuck wrote:\n> > Hi Jacopo,\n> >\n> > This is all very confusing for me as well, so I might be wrong in my\n> > explanation :-)\n> >\n> > On Tue, 12 Jul 2022 at 08:23, Jacopo Mondi <jacopo@jmondi.org> wrote:\n> >\n> > > Hi Naush,\n> > >\n> > > On Tue, Jul 05, 2022 at 02:59:56PM +0100, Naushir Patuck via\n> > > libcamera-devel wrote:\n> > > > The controls::ScalerCrop in the ControlInfoMap was advertised based\n> on\n> > > the ISP\n> > > > output Rectangle. This is incorrect, it needs to be set based on the\n> > > sensor\n> > > > output Rectangle. Fix this.\n> > >\n> > > I might have not completely woken up yet but the existing\n> > >\n> > >         ctrlMap.emplace(&controls::ScalerCrop,\n> > >                         ControlInfo(Rectangle(data->ispMinCropSize_),\n> > > Rectangle(data->sensorInfo_.outputSize)));\n> > >\n> > > Seems to be based on the sensor output size, not the ISP output size\n> > > as you mention here.\n> > >\n> > > What am I missing ?\n> > >\n> >\n> > ispMinCropSize_ is based on the ISP's minimum output size (64x64).  This\n> > does not have\n> > any relation to the sensor size, so....\n> >\n> >\n> > >\n> > > >\n> > > > Additionally, do not use emplace to be consistent with the other\n> > > controls set\n> > > > in the ControlInfoMap.\n> > > >\n> > > > Fixes: 9dacde0d651d (pipeline: raspberrypi: Advertise ScalerCrop from\n> > > the pipeline handler)\n> > > > Reported-by: David Plowman <david.plowman@raspberrypi.com>\n> > > > Signed-off-by: Naushir Patuck <naush@raspberrypi.com>\n> > > > ---\n> > > >  src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 5 +++--\n> > > >  1 file changed, 3 insertions(+), 2 deletions(-)\n> > > >\n> > > > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > > b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > > > index 66a84b1dfb97..fdc24cd530c2 100644\n> > > > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > > > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > > > @@ -946,8 +946,9 @@ int PipelineHandlerRPi::configure(Camera *camera,\n> > > CameraConfiguration *config)\n> > > >               ctrlMap.emplace(c.first, c.second);\n> > > >\n> > > >       /* Add the ScalerCrop control limits based on the current\n> mode. */\n> > > > -     ctrlMap.emplace(&controls::ScalerCrop,\n> > > > -                     ControlInfo(Rectangle(data->ispMinCropSize_),\n> > > Rectangle(data->sensorInfo_.outputSize)));\n> > > > +     Rectangle ispMinCrop(data->ispMinCropSize_);\n> > > > +     ispMinCrop.scaleBy(data->sensorInfo_.analogCrop.size(),\n> > > data->sensorInfo_.outputSize);\n> > >\n> > > What is the purpose of scaling the minimum accepted ISP input in the\n> > > sensor's analogue crop / sensor's output size ratio ?\n> > >\n> >\n> > .... this bit of code scales the ispMinCropSize_  (64x64) relative to the\n> > sensor input.\n>\n> I was missing that ScalerCrop is specified relatively to the analogue\n> gain rectangle, so re-scaling it in the (analogue crop / sensor\n> output) ratio makes sure the sensor's mode binning/subsampling is\n> taken into account.\n>\n> >\n> >\n> > >\n> > > > +     ctrlMap[&controls::ScalerCrop] = ControlInfo(ispMinCrop,\n> > > Rectangle(data->sensorInfo_.analogCrop.size()));\n> > >\n> > > And I don't get this one either, how can the maximum scaler rectangle\n> > > be larger than the sensor's output size ? (assumiming\n> > > sensor->analogCrop > sensor->outputSize).\n> > >\n> >\n> > If I understand correctly, the ScalerCrop control is based on the\n> > \"unbinned\" resolution,\n> > i.e. full analogue crop readout of the sensor.  The translation to sensor\n> > output scale\n> > happens internally in the pipeline handler here:\n> >\n> https://git.libcamera.org/libcamera/libcamera.git/tree/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp#n2052\n> >\n> > For imx477, this is the result:\n> >\n> > Sensor mode 4056x3040:\n> > ScalerCrop : [(0, 0)/64x64..(0, 0)/4056x3040]\n> >\n> > Sensor mode 2028x1520:\n> > ScalerCrop : [(0, 0)/128x128..(0, 0)/4056x3040]\n> >\n> > Sensor mode 2028x1080:\n> > ScalerCrop : [(0, 0)/128x128..(0, 0)/4056x2160]\n> >\n> > Sensor mode 1332x990:\n> > ScalerCrop : [(0, 0)/128x128..(0, 0)/2664x1980]\n>\n> Weird, the driver says this mode is 4x4 binned in one place but it's\n> instead cropped down to 2664x1980 then 2x2 binned.\n>\n> Anyway, these nicely match the driver's analogue crop rectangles.\n>\n> >\n> > I'm slightly confused :)\n> > >\n> >\n> > I was as well :-) David had to patiently work though all this with me to\n> > get an understanding on what's going on.\n>\n> Thanks for having the patience to re-explain this to me.\n>\n> If I'm not mistaken then the commit message should say:\n>\n> The controls::ScalerCrop in the ControlInfoMap was advertised based on\n> the ISP output Rectangle. This is incorrect, it needs to be set based\n> - on the sensor output Rectangle. Fix this.\n> + on the sensor analogue crop Rectangle. Fix this.\n>\n> The patch looks good and the commit message can be updated (if you\n> agree with the change) when applying.\n>\n> Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>\n>\n\nThanks!  Happy to change the commit message when applying :)\n\nRegards,\nNaush\n\n\n>\n> Thanks\n>    j\n>\n> >\n> > Regards,\n> > Naush\n> >\n> >\n> > > >\n> > > >       data->controlInfo_ = ControlInfoMap(std::move(ctrlMap),\n> > > result.controlInfo.idmap());\n> > > >\n> > > > --\n> > > > 2.25.1\n> > > >\n> > >\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 B5FD9BD1F1\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 12 Jul 2022 10:26:48 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 2495260402;\n\tTue, 12 Jul 2022 12:26:48 +0200 (CEST)","from mail-lj1-x230.google.com (mail-lj1-x230.google.com\n\t[IPv6:2a00:1450:4864:20::230])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 1D12C60402\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 12 Jul 2022 12:26:46 +0200 (CEST)","by mail-lj1-x230.google.com with SMTP id a39so9325588ljq.11\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 12 Jul 2022 03:26:46 -0700 (PDT)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1657621608;\n\tbh=/FPkREO0ZmVRKNSPcKpQyN+w3MgFjzrrcHyDYxgvz7E=;\n\th=References:In-Reply-To:Date:To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=C+3ldSq1JoHwpLDxQ/bqXn5CrUfWMIEXTiSQMSJORCEmHHI8Hu4lrJetfLL1jUt5N\n\taA4Vgqwn+EWd82BywC6Ijugk+DKC/RfiFJGN6hw+ZVAHGbTrfhMOvW8NbnRFTdejij\n\t0hOWLKJjFWE+QRD2Ivhk6GzwEgpC+GL3OGapJYPM7KgOenr0hk6f4MeRYVOK4LymwS\n\tFofTIMUP0FbFeXPpHkgZwSSg0KjJDyj092P0nSIlAbnW8lbQvhhIevD1lvN1terR1U\n\tW/CJpj0X+eFU4i2lCurglbK1D2gH2VEG5SgWRqYbCV7OHuKAandIsOTtWLjl69wRYD\n\tYaE/E56NKpJsw==","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=cx7Yo3NrBZE0968Tu6so2lJYtUjxxNXSlBTCpXpjHyA=;\n\tb=WnzsCRlc2rOzGZsFN53hVVpGF7BU+ksvo9FDUyyaWPOqp2db37EbNqQQuarZCERrgv\n\tpErGP6lg03lNt7w0SYI4Zi/X8yCTMocsQoxMjjxsCI2/GXikiBDCxK5HIVYj3G0IBZk9\n\tBICs4tqMOTm1fCBWwt70eoWwxnasJoAFRxXETQ6ebu9qL/YK5Plf8w+1ABjXYUVJZOBf\n\t23jePswrlLQGiRgbjBWzarE6dF/+/oSTt9QQeUtT8EqynAQEYksBUfUWtWe8EvaBVok6\n\tAuscvrglPC+Uy8QUQYlnNyYRyBMQlASPScBvwLLv6pm/EAz7VeRj7XchE4PICWzYBYBW\n\tjdsg=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=raspberrypi.com\n\theader.i=@raspberrypi.com\n\theader.b=\"WnzsCRlc\"; dkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=cx7Yo3NrBZE0968Tu6so2lJYtUjxxNXSlBTCpXpjHyA=;\n\tb=k2imMUKKLQFoyTOQSm6wYv8kfqKKxBxZ8jvYjxy6EXFcb8rAn24zqUj21kk6klC1tv\n\t0GR4H0Otbmt4mjiAIShJy3q9wkOChj4nyTkRe+a78YFKemSQJVZ81AgNDcESTYWJIFw2\n\t96YHG9FKH+7BhD/YWS+G89X0776E/jMeQ78EFeGJo9VQDgdrMgT9gWKT6kQh7FI5Lfw4\n\tO/waRBICX7Q5es9NRhTRRi+4L3IkRSbuDgr1UJIwB+9gquUCw0+pbMilkdLd+5d5WfOb\n\tKDIG06UnyUkxDEhzHjtaNeuj0gYf6Qnl2loNjptVID/khyGFc7UroGNJJ0b4/aQjdcM2\n\tOdPw==","X-Gm-Message-State":"AJIora9mo+OnzdJjMkPVGEklPiYJXOfy2MWEoQMXSqh+msweIgKIygnL\n\tHBicGMMdsQ+2egLQPAr3wbIhMuXi0mHbeafz3Lzhkg==","X-Google-Smtp-Source":"AGRyM1vHEWIU9iNhLLHTyEynW+c9qbUCymZ2IjnIF73wwwSa34StfSnIdD0+wFnGP5omvpTT9TWp2ggMzh7gJuXoq5g=","X-Received":"by 2002:a2e:a303:0:b0:25d:644c:9d7a with SMTP id\n\tl3-20020a2ea303000000b0025d644c9d7amr7701815lje.426.1657621605377;\n\tTue, 12 Jul 2022 03:26:45 -0700 (PDT)","MIME-Version":"1.0","References":"<20220705135956.30444-1-naush@raspberrypi.com>\n\t<20220712072331.khespozodgehicri@uno.localdomain>\n\t<CAEmqJPrGqRVU2sw-xrkM41Fhg7vFSaZd3w7LqpAtXSt83v4jqA@mail.gmail.com>\n\t<20220712102019.mnv57bp7fbqqaaso@uno.localdomain>","In-Reply-To":"<20220712102019.mnv57bp7fbqqaaso@uno.localdomain>","Date":"Tue, 12 Jul 2022 11:26:30 +0100","Message-ID":"<CAEmqJPrw=s8v-4eAX9d9sY=uHijR0dr1kpBcSjKsFu2ZbyqoTA@mail.gmail.com>","To":"Jacopo Mondi <jacopo@jmondi.org>","Content-Type":"multipart/alternative; boundary=\"000000000000f5fbbe05e3991ab1\"","Subject":"Re: [libcamera-devel] [PATCH v2] pipeline: raspberrypi: Fix\n\tincorrect advertising of ScalerCrop","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>","From":"Naushir Patuck via libcamera-devel\n\t<libcamera-devel@lists.libcamera.org>","Reply-To":"Naushir Patuck <naush@raspberrypi.com>","Cc":"libcamera devel <libcamera-devel@lists.libcamera.org>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":23873,"web_url":"https://patchwork.libcamera.org/comment/23873/","msgid":"<YtAb+38J41rw7elE@pendragon.ideasonboard.com>","date":"2022-07-14T13:36:59","subject":"Re: [libcamera-devel] [PATCH v2] pipeline: raspberrypi: Fix\n\tincorrect advertising of ScalerCrop","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hello,\n\nOn Tue, Jul 12, 2022 at 11:26:30AM +0100, Naushir Patuck via libcamera-devel wrote:\n> On Tue, 12 Jul 2022 at 11:20, Jacopo Mondi wrote:\n> > On Tue, Jul 12, 2022 at 10:03:58AM +0100, Naushir Patuck wrote:\n> > > Hi Jacopo,\n> > >\n> > > This is all very confusing for me as well, so I might be wrong in my\n> > > explanation :-)\n> > >\n> > > On Tue, 12 Jul 2022 at 08:23, Jacopo Mondi wrote:\n> > > > On Tue, Jul 05, 2022 at 02:59:56PM +0100, Naushir Patuck via libcamera-devel wrote:\n> > > > > The controls::ScalerCrop in the ControlInfoMap was advertised based on the ISP\n> > > > > output Rectangle. This is incorrect, it needs to be set based on the sensor\n> > > > > output Rectangle. Fix this.\n> > > >\n> > > > I might have not completely woken up yet but the existing\n> > > >\n> > > >         ctrlMap.emplace(&controls::ScalerCrop,\n> > > >                         ControlInfo(Rectangle(data->ispMinCropSize_),\n> > > > Rectangle(data->sensorInfo_.outputSize)));\n> > > >\n> > > > Seems to be based on the sensor output size, not the ISP output size\n> > > > as you mention here.\n> > > >\n> > > > What am I missing ?\n> > >\n> > > ispMinCropSize_ is based on the ISP's minimum output size (64x64).  This does not have\n> > > any relation to the sensor size, so....\n> > >\n> > >\n> > > > > Additionally, do not use emplace to be consistent with the other controls set\n> > > > > in the ControlInfoMap.\n> > > > >\n> > > > > Fixes: 9dacde0d651d (pipeline: raspberrypi: Advertise ScalerCrop from the pipeline handler)\n> > > > > Reported-by: David Plowman <david.plowman@raspberrypi.com>\n> > > > > Signed-off-by: Naushir Patuck <naush@raspberrypi.com>\n> > > > > ---\n> > > > >  src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 5 +++--\n> > > > >  1 file changed, 3 insertions(+), 2 deletions(-)\n> > > > >\n> > > > > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > > > > index 66a84b1dfb97..fdc24cd530c2 100644\n> > > > > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > > > > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > > > > @@ -946,8 +946,9 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config)\n> > > > >               ctrlMap.emplace(c.first, c.second);\n> > > > >\n> > > > >       /* Add the ScalerCrop control limits based on the current mode. */\n> > > > > -     ctrlMap.emplace(&controls::ScalerCrop,\n> > > > > -                     ControlInfo(Rectangle(data->ispMinCropSize_), Rectangle(data->sensorInfo_.outputSize)));\n> > > > > +     Rectangle ispMinCrop(data->ispMinCropSize_);\n> > > > > +     ispMinCrop.scaleBy(data->sensorInfo_.analogCrop.size(), data->sensorInfo_.outputSize);\n> > > >\n> > > > What is the purpose of scaling the minimum accepted ISP input in the\n> > > > sensor's analogue crop / sensor's output size ratio ?\n> > >\n> > > .... this bit of code scales the ispMinCropSize_  (64x64) relative to the\n> > > sensor input.\n> >\n> > I was missing that ScalerCrop is specified relatively to the analogue\n> > gain rectangle, so re-scaling it in the (analogue crop / sensor\n> > output) ratio makes sure the sensor's mode binning/subsampling is\n> > taken into account.\n> >\n> > > > > +     ctrlMap[&controls::ScalerCrop] = ControlInfo(ispMinCrop, Rectangle(data->sensorInfo_.analogCrop.size()));\n> > > >\n> > > > And I don't get this one either, how can the maximum scaler rectangle\n> > > > be larger than the sensor's output size ? (assumiming\n> > > > sensor->analogCrop > sensor->outputSize).\n> > >\n> > > If I understand correctly, the ScalerCrop control is based on the \"unbinned\" resolution,\n> > > i.e. full analogue crop readout of the sensor.  The translation to sensor output scale\n> > > happens internally in the pipeline handler here:\n> > >\n> > https://git.libcamera.org/libcamera/libcamera.git/tree/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp#n2052\n> > >\n> > > For imx477, this is the result:\n> > >\n> > > Sensor mode 4056x3040:\n> > > ScalerCrop : [(0, 0)/64x64..(0, 0)/4056x3040]\n> > >\n> > > Sensor mode 2028x1520:\n> > > ScalerCrop : [(0, 0)/128x128..(0, 0)/4056x3040]\n> > >\n> > > Sensor mode 2028x1080:\n> > > ScalerCrop : [(0, 0)/128x128..(0, 0)/4056x2160]\n> > >\n> > > Sensor mode 1332x990:\n> > > ScalerCrop : [(0, 0)/128x128..(0, 0)/2664x1980]\n> >\n> > Weird, the driver says this mode is 4x4 binned in one place but it's\n> > instead cropped down to 2664x1980 then 2x2 binned.\n> >\n> > Anyway, these nicely match the driver's analogue crop rectangles.\n> >\n> > > I'm slightly confused :)\n> > >\n> > > I was as well :-) David had to patiently work though all this with me to\n> > > get an understanding on what's going on.\n> >\n> > Thanks for having the patience to re-explain this to me.\n> >\n> > If I'm not mistaken then the commit message should say:\n> >\n> > The controls::ScalerCrop in the ControlInfoMap was advertised based on\n> > the ISP output Rectangle. This is incorrect, it needs to be set based\n> > - on the sensor output Rectangle. Fix this.\n> > + on the sensor analogue crop Rectangle. Fix this.\n> >\n> > The patch looks good and the commit message can be updated (if you\n> > agree with the change) when applying.\n> >\n> > Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>\n> \n> Thanks!  Happy to change the commit message when applying :)\n\nI'll handle it.\n\n> > > > >       data->controlInfo_ = ControlInfoMap(std::move(ctrlMap), result.controlInfo.idmap());\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 A9F5DBD1F1\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu, 14 Jul 2022 13:37:33 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id AD15763312;\n\tThu, 14 Jul 2022 15:37:32 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 24D466330D\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 14 Jul 2022 15:37:31 +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 78B03383;\n\tThu, 14 Jul 2022 15:37:30 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1657805852;\n\tbh=AW0dPPSQo/qfWb/zFcs8IT9RN5yUy8rYTFlXRhMijZI=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=iDEh/NlwfHYf1dmTp06Ck5oWH91fgRH+9SnIC9pNpQ2KeeXs3m3g2AmrzsgjeLPCB\n\tcmQtJ7Sw3L1CwxQ3h0m2vtjvzoYqAhUP7fPSFa/qui39uY4ZNdZ9b0/BJLUO9yT4Ux\n\tInDHnUVeVmZA3jcxTqNd602PDkGbNea1SGRBmX5rNJQ23Uu4gkgIjtJOmlxodfocBR\n\t5F+tT4+aH0IpqhBsIIa0Qr/TPGIMxQHNqWU+T1iuGFOk3z4kuVFm7lf78T8Bm5y6Hr\n\tm4QPoibKHnXqjDu5ZbEZL7cT3LL23v3kihDXYkajliKP3C9A8jXTZRkv7O88pPw0Fy\n\tPusSS5aVG1GAg==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1657805850;\n\tbh=AW0dPPSQo/qfWb/zFcs8IT9RN5yUy8rYTFlXRhMijZI=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=PBWzgAsCisiEkFklU8HcL3H5tD4wz704nZGs1t19TZ7qb3xfynLyrdeGcGFNW9CN4\n\tBoTmT/FG3pjQ0u2chHCcmcPc+B/A7eGiSHevuUUNvUJ9yuo0sXS5VuZW9CHYroqaPA\n\tODS111xV9dKObKuX9Fe34ypLKMLoccSxPzHTSIPU="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"PBWzgAsC\"; dkim-atps=neutral","Date":"Thu, 14 Jul 2022 16:36:59 +0300","To":"Naushir Patuck <naush@raspberrypi.com>","Message-ID":"<YtAb+38J41rw7elE@pendragon.ideasonboard.com>","References":"<20220705135956.30444-1-naush@raspberrypi.com>\n\t<20220712072331.khespozodgehicri@uno.localdomain>\n\t<CAEmqJPrGqRVU2sw-xrkM41Fhg7vFSaZd3w7LqpAtXSt83v4jqA@mail.gmail.com>\n\t<20220712102019.mnv57bp7fbqqaaso@uno.localdomain>\n\t<CAEmqJPrw=s8v-4eAX9d9sY=uHijR0dr1kpBcSjKsFu2ZbyqoTA@mail.gmail.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<CAEmqJPrw=s8v-4eAX9d9sY=uHijR0dr1kpBcSjKsFu2ZbyqoTA@mail.gmail.com>","Subject":"Re: [libcamera-devel] [PATCH v2] pipeline: raspberrypi: Fix\n\tincorrect advertising of ScalerCrop","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>","From":"Laurent Pinchart via libcamera-devel\n\t<libcamera-devel@lists.libcamera.org>","Reply-To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"libcamera devel <libcamera-devel@lists.libcamera.org>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]