[{"id":19146,"web_url":"https://patchwork.libcamera.org/comment/19146/","msgid":"<c17e77cd-871b-ff7b-cbac-1a71058d34bf@ideasonboard.com>","date":"2021-08-27T12:47:45","subject":"Re: [libcamera-devel] [PATCH v2 3/4] ipu3: cio2: Change sensor size\n\tselection policy","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"On 10/08/2021 08:58, Umang Jain wrote:\n> The current implementation of getSensorFormat() prioritizes sensor\n> sizes that match the output size FOV ratio.\n> \n> Modify the frame size selection procedure to prioritize resolutions\n> with the same FOV as the sensor's native one, to avoid cropping in the\n> sensor pixel array.\n> \n> For example:\n> \n> - on a Soraka device equipped with ov13858 as back sensor, with a\n>   native resolution of 4224x3136 (4:3), when requested to select the\n>   sensor output size to produce 1080p (16:9) a frame size of 2112x1188\n>   (16:9) is selected causing the ImgU configuration procedure to fail.\n>   If a resolution with the same FOV as the sensor's native size, such\n>   as 2112x1568 (4:3), is selected the pipeline works correctly.\n> > Suggested-by:: Jacopo Mondi <jacopo@jmondi.org>\n> Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>\n> Tested-by: Umang Jain <umang.jain@ideasonboard.com>\n> Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>\n> Tested-by: Jacopo Mondi <jacopo@jmondi.org>\n> ---\n>  src/libcamera/pipeline/ipu3/cio2.cpp | 8 +++++---\n>  1 file changed, 5 insertions(+), 3 deletions(-)\n> \n> diff --git a/src/libcamera/pipeline/ipu3/cio2.cpp b/src/libcamera/pipeline/ipu3/cio2.cpp\n> index 88dd364b..3c9331e3 100644\n> --- a/src/libcamera/pipeline/ipu3/cio2.cpp\n> +++ b/src/libcamera/pipeline/ipu3/cio2.cpp\n> @@ -248,8 +248,8 @@ StreamConfiguration CIO2Device::generateConfiguration(Size size) const\n>   *\n>   * - The desired \\a size shall fit in the sensor output size to avoid the need\n>   *   to up-scale.\n> - * - The sensor output size shall match the desired aspect ratio to avoid the\n> - *   need to crop the field of view.\n> + * - The aspect ratio of sensor output size shall be as close as possible to\n> + *   the sensor's native resolution field of view.\n>   * - The sensor output size shall be as small as possible to lower the required\n>   *   bandwidth.\n>   * - The desired \\a size shall be supported by one of the media bus code listed\n> @@ -273,7 +273,9 @@ V4L2SubdeviceFormat CIO2Device::getSensorFormat(const std::vector<unsigned int>\n>  {\n>  \tunsigned int desiredArea = size.width * size.height;\n>  \tunsigned int bestArea = std::numeric_limits<unsigned int>::max();\n> -\tfloat desiredRatio = static_cast<float>(size.width) / size.height;\n> +\tconst Size &resolution = sensor_->resolution();\n> +\tfloat desiredRatio = static_cast<float>(resolution.width) /\n> +\t\t\t     resolution.height;\n\n\nOk - so we always try to keep the sensor format that is close to native,\nrather than the closest to what is requested.\n\nI'm not yet sure I understand why that is better, but this patch does do\nwhat it describes, so on that aspect:\n\nReviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n\n\nBut does this mean that if I ask for a 16:9 1080p image, I'm going to\nget a larger 4k image, which would then be perceptually squashed?\n\nThe end result will still be cropped somewhere right?\n\n\n\n>  \tfloat bestRatio = std::numeric_limits<float>::max();\n>  \tSize bestSize;\n>  \tuint32_t bestCode = 0;\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 A2CF0BD87D\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 27 Aug 2021 12:47:50 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id E2E266891F;\n\tFri, 27 Aug 2021 14:47:49 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 1851860256\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 27 Aug 2021 14:47:48 +0200 (CEST)","from [192.168.0.20]\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 964E85A1;\n\tFri, 27 Aug 2021 14:47:47 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"r/1soo1g\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1630068467;\n\tbh=f679EL46TxOhUjwCe3vc0giSryT41ehE1eX0IqoJGow=;\n\th=To:References:From:Subject:Date:In-Reply-To:From;\n\tb=r/1soo1gPgGFBtkLDZrnyS6TX777UsivdLvqhUGIJt/lEp2elKSmg8dUeUyEWH70V\n\tOPKAFGCUPzTi00SO8wCDQCGSuPMYfe+uXsBy/oV6ACrvGQ4jgdrh1gekxpB5pg0F0R\n\tCQ9d5EoxiAixVRxySIQ5hPLb/gI9EEXSUgOLvRcM=","To":"Umang Jain <umang.jain@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org","References":"<20210810075854.86191-1-umang.jain@ideasonboard.com>\n\t<20210810075854.86191-4-umang.jain@ideasonboard.com>","From":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Message-ID":"<c17e77cd-871b-ff7b-cbac-1a71058d34bf@ideasonboard.com>","Date":"Fri, 27 Aug 2021 13:47:45 +0100","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101\n\tThunderbird/78.11.0","MIME-Version":"1.0","In-Reply-To":"<20210810075854.86191-4-umang.jain@ideasonboard.com>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-GB","Content-Transfer-Encoding":"8bit","Subject":"Re: [libcamera-devel] [PATCH v2 3/4] ipu3: cio2: Change sensor size\n\tselection policy","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>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":19149,"web_url":"https://patchwork.libcamera.org/comment/19149/","msgid":"<645af150-d6a7-9784-bb98-1f22dd12c286@ideasonboard.com>","date":"2021-08-27T14:07:04","subject":"Re: [libcamera-devel] [PATCH v2 3/4] ipu3: cio2: Change sensor size\n\tselection policy","submitter":{"id":86,"url":"https://patchwork.libcamera.org/api/people/86/","name":"Umang Jain","email":"umang.jain@ideasonboard.com"},"content":"Hi Kieran,\n\nOn 8/27/21 6:17 PM, Kieran Bingham wrote:\n> On 10/08/2021 08:58, Umang Jain wrote:\n>> The current implementation of getSensorFormat() prioritizes sensor\n>> sizes that match the output size FOV ratio.\n>>\n>> Modify the frame size selection procedure to prioritize resolutions\n>> with the same FOV as the sensor's native one, to avoid cropping in the\n>> sensor pixel array.\n>>\n>> For example:\n>>\n>> - on a Soraka device equipped with ov13858 as back sensor, with a\n>>    native resolution of 4224x3136 (4:3), when requested to select the\n>>    sensor output size to produce 1080p (16:9) a frame size of 2112x1188\n>>    (16:9) is selected causing the ImgU configuration procedure to fail.\n>>    If a resolution with the same FOV as the sensor's native size, such\n>>    as 2112x1568 (4:3), is selected the pipeline works correctly.\n>>> Suggested-by:: Jacopo Mondi <jacopo@jmondi.org>\n>> Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>\n>> Tested-by: Umang Jain <umang.jain@ideasonboard.com>\n>> Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>\n>> Tested-by: Jacopo Mondi <jacopo@jmondi.org>\n>> ---\n>>   src/libcamera/pipeline/ipu3/cio2.cpp | 8 +++++---\n>>   1 file changed, 5 insertions(+), 3 deletions(-)\n>>\n>> diff --git a/src/libcamera/pipeline/ipu3/cio2.cpp b/src/libcamera/pipeline/ipu3/cio2.cpp\n>> index 88dd364b..3c9331e3 100644\n>> --- a/src/libcamera/pipeline/ipu3/cio2.cpp\n>> +++ b/src/libcamera/pipeline/ipu3/cio2.cpp\n>> @@ -248,8 +248,8 @@ StreamConfiguration CIO2Device::generateConfiguration(Size size) const\n>>    *\n>>    * - The desired \\a size shall fit in the sensor output size to avoid the need\n>>    *   to up-scale.\n>> - * - The sensor output size shall match the desired aspect ratio to avoid the\n>> - *   need to crop the field of view.\n>> + * - The aspect ratio of sensor output size shall be as close as possible to\n>> + *   the sensor's native resolution field of view.\n>>    * - The sensor output size shall be as small as possible to lower the required\n>>    *   bandwidth.\n>>    * - The desired \\a size shall be supported by one of the media bus code listed\n>> @@ -273,7 +273,9 @@ V4L2SubdeviceFormat CIO2Device::getSensorFormat(const std::vector<unsigned int>\n>>   {\n>>   \tunsigned int desiredArea = size.width * size.height;\n>>   \tunsigned int bestArea = std::numeric_limits<unsigned int>::max();\n>> -\tfloat desiredRatio = static_cast<float>(size.width) / size.height;\n>> +\tconst Size &resolution = sensor_->resolution();\n>> +\tfloat desiredRatio = static_cast<float>(resolution.width) /\n>> +\t\t\t     resolution.height;\n>\n> Ok - so we always try to keep the sensor format that is close to native,\n> rather than the closest to what is requested.\n>\n> I'm not yet sure I understand why that is better, but this patch does do\n> what it describes, so on that aspect:\n>\n> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n>\n>\n> But does this mean that if I ask for a 16:9 1080p image, I'm going to\n> get a larger 4k image, which would then be perceptually squashed?\n>\n> The end result will still be cropped somewhere right?\n\n\nCropping happens at 2 places, either at the (i) \"source\" which I \nunderstand is the actual sensor (ii) in the IMGU\n\nThe end result might be same/close, i.e. what you requested, but do you \nwant your Camera sensor to run at 4k everytime 1080p is requested \n(provided it can do lower native resolutions too). No, right?\n\n>\n>\n>\n>>   \tfloat bestRatio = std::numeric_limits<float>::max();\n>>   \tSize bestSize;\n>>   \tuint32_t bestCode = 0;\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 D495FBD87D\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 27 Aug 2021 14:07:12 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 4699F6891F;\n\tFri, 27 Aug 2021 16:07:12 +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 CACF160256\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 27 Aug 2021 16:07:09 +0200 (CEST)","from [192.168.1.104] (unknown [103.251.226.167])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id CD0975A1;\n\tFri, 27 Aug 2021 16:07:08 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"brUxBxY9\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1630073229;\n\tbh=zKGzn4WuwK0odmDb0rI/9iI4veHDCQij3O30m3PQZsE=;\n\th=Subject:To:References:From:Date:In-Reply-To:From;\n\tb=brUxBxY9teXcxRWLhyaDJ8m7Nv4Ig1H51phR/nNfK7jTLJyscGSe9B2RvF05CxGwR\n\tVUgVs7G5F9Vp++AGEbb7nkqmAkBa/9eKbmhr3dP9JKr/4SAPOl9CcUmc0Y5CI4ao/a\n\trb+trHhk5x0are2i/Ukv4JeILKocVijTubGh2Sdk=","To":"Kieran Bingham <kieran.bingham@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org","References":"<20210810075854.86191-1-umang.jain@ideasonboard.com>\n\t<20210810075854.86191-4-umang.jain@ideasonboard.com>\n\t<c17e77cd-871b-ff7b-cbac-1a71058d34bf@ideasonboard.com>","From":"Umang Jain <umang.jain@ideasonboard.com>","Message-ID":"<645af150-d6a7-9784-bb98-1f22dd12c286@ideasonboard.com>","Date":"Fri, 27 Aug 2021 19:37:04 +0530","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101\n\tThunderbird/78.10.2","MIME-Version":"1.0","In-Reply-To":"<c17e77cd-871b-ff7b-cbac-1a71058d34bf@ideasonboard.com>","Content-Type":"text/plain; charset=utf-8; format=flowed","Content-Transfer-Encoding":"7bit","Content-Language":"en-US","Subject":"Re: [libcamera-devel] [PATCH v2 3/4] ipu3: cio2: Change sensor size\n\tselection policy","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>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":19150,"web_url":"https://patchwork.libcamera.org/comment/19150/","msgid":"<1e18ed42-0bdd-7dc9-a2fc-ab548d205782@ideasonboard.com>","date":"2021-08-27T14:25:31","subject":"Re: [libcamera-devel] [PATCH v2 3/4] ipu3: cio2: Change sensor size\n\tselection policy","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"On 27/08/2021 15:07, Umang Jain wrote:\n> Hi Kieran,\n>>> +    float desiredRatio = static_cast<float>(resolution.width) /\n>>> +                 resolution.height;\n>>\n>> Ok - so we always try to keep the sensor format that is close to native,\n>> rather than the closest to what is requested.\n>>\n>> I'm not yet sure I understand why that is better, but this patch does do\n>> what it describes, so on that aspect:\n>>\n>> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n>>\n>>\n>> But does this mean that if I ask for a 16:9 1080p image, I'm going to\n>> get a larger 4k image, which would then be perceptually squashed?\n>>\n>> The end result will still be cropped somewhere right?\n> \n> \n> Cropping happens at 2 places, either at the (i) \"source\" which I\n> understand is the actual sensor (ii) in the IMGU\n> \n> The end result might be same/close, i.e. what you requested, but do you\n> want your Camera sensor to run at 4k everytime 1080p is requested\n> (provided it can do lower native resolutions too). No, right?\n\n\nAs long as an image from a FoV 4:3 sensor like this:\n\n\thttps://pasteboard.co/KhPTc6U.jpg (4:3\n\nbecomes this\n\n\thttps://pasteboard.co/KhPTHQm.jpg\n\nand not this\n\n\thttps://pasteboard.co/KhPTUeO.jpg\n\nthen it's fine ;-)","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 22FC5BD87C\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 27 Aug 2021 14:25:37 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 75C4368932;\n\tFri, 27 Aug 2021 16:25:36 +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 6037660256\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 27 Aug 2021 16:25:34 +0200 (CEST)","from [192.168.0.20]\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id D121C5A1;\n\tFri, 27 Aug 2021 16:25:33 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"pqNZ5S9M\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1630074334;\n\tbh=YWRnWEC02e+Igtb2ZuRomo3QviXR7x2OjkO1mUJPR+g=;\n\th=From:Subject:To:References:Date:In-Reply-To:From;\n\tb=pqNZ5S9Mtp3POi/VU/1KboK/ikgHnr3llmapXEoNNhvb177Ivpz3lSzqbiCcfuV0/\n\tK0Oahf+hXp+bKzNsNHD/r2VI6US/gbBv1z3zL2f+p2q/mtZjbgd78wfY+MbvdksNCj\n\t0QSXsafvUbmopp9OpgfYo/kRsxmzmuJG2BsnyXmg=","From":"Kieran Bingham <kieran.bingham@ideasonboard.com>","To":"Umang Jain <umang.jain@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org","References":"<20210810075854.86191-1-umang.jain@ideasonboard.com>\n\t<20210810075854.86191-4-umang.jain@ideasonboard.com>\n\t<c17e77cd-871b-ff7b-cbac-1a71058d34bf@ideasonboard.com>\n\t<645af150-d6a7-9784-bb98-1f22dd12c286@ideasonboard.com>","Message-ID":"<1e18ed42-0bdd-7dc9-a2fc-ab548d205782@ideasonboard.com>","Date":"Fri, 27 Aug 2021 15:25:31 +0100","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101\n\tThunderbird/78.11.0","MIME-Version":"1.0","In-Reply-To":"<645af150-d6a7-9784-bb98-1f22dd12c286@ideasonboard.com>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-GB","Content-Transfer-Encoding":"8bit","Subject":"Re: [libcamera-devel] [PATCH v2 3/4] ipu3: cio2: Change sensor size\n\tselection policy","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>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":19156,"web_url":"https://patchwork.libcamera.org/comment/19156/","msgid":"<20210828110208.dyrqpqc6yjhvzpra@uno.localdomain>","date":"2021-08-28T11:02:08","subject":"Re: [libcamera-devel] [PATCH v2 3/4] ipu3: cio2: Change sensor size\n\tselection policy","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Kieran,\n\nOn Fri, Aug 27, 2021 at 01:47:45PM +0100, Kieran Bingham wrote:\n>\n> On 10/08/2021 08:58, Umang Jain wrote:\n> > The current implementation of getSensorFormat() prioritizes sensor\n> > sizes that match the output size FOV ratio.\n> >\n> > Modify the frame size selection procedure to prioritize resolutions\n> > with the same FOV as the sensor's native one, to avoid cropping in the\n> > sensor pixel array.\n> >\n> > For example:\n> >\n> > - on a Soraka device equipped with ov13858 as back sensor, with a\n> >   native resolution of 4224x3136 (4:3), when requested to select the\n> >   sensor output size to produce 1080p (16:9) a frame size of 2112x1188\n> >   (16:9) is selected causing the ImgU configuration procedure to fail.\n> >   If a resolution with the same FOV as the sensor's native size, such\n> >   as 2112x1568 (4:3), is selected the pipeline works correctly.\n> > > Suggested-by:: Jacopo Mondi <jacopo@jmondi.org>\n> > Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>\n> > Tested-by: Umang Jain <umang.jain@ideasonboard.com>\n> > Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>\n> > Tested-by: Jacopo Mondi <jacopo@jmondi.org>\n> > ---\n> >  src/libcamera/pipeline/ipu3/cio2.cpp | 8 +++++---\n> >  1 file changed, 5 insertions(+), 3 deletions(-)\n> >\n> > diff --git a/src/libcamera/pipeline/ipu3/cio2.cpp b/src/libcamera/pipeline/ipu3/cio2.cpp\n> > index 88dd364b..3c9331e3 100644\n> > --- a/src/libcamera/pipeline/ipu3/cio2.cpp\n> > +++ b/src/libcamera/pipeline/ipu3/cio2.cpp\n> > @@ -248,8 +248,8 @@ StreamConfiguration CIO2Device::generateConfiguration(Size size) const\n> >   *\n> >   * - The desired \\a size shall fit in the sensor output size to avoid the need\n> >   *   to up-scale.\n> > - * - The sensor output size shall match the desired aspect ratio to avoid the\n> > - *   need to crop the field of view.\n> > + * - The aspect ratio of sensor output size shall be as close as possible to\n> > + *   the sensor's native resolution field of view.\n> >   * - The sensor output size shall be as small as possible to lower the required\n> >   *   bandwidth.\n> >   * - The desired \\a size shall be supported by one of the media bus code listed\n> > @@ -273,7 +273,9 @@ V4L2SubdeviceFormat CIO2Device::getSensorFormat(const std::vector<unsigned int>\n> >  {\n> >  \tunsigned int desiredArea = size.width * size.height;\n> >  \tunsigned int bestArea = std::numeric_limits<unsigned int>::max();\n> > -\tfloat desiredRatio = static_cast<float>(size.width) / size.height;\n> > +\tconst Size &resolution = sensor_->resolution();\n> > +\tfloat desiredRatio = static_cast<float>(resolution.width) /\n> > +\t\t\t     resolution.height;\n>\n>\n> Ok - so we always try to keep the sensor format that is close to native,\n> rather than the closest to what is requested.\n>\n> I'm not yet sure I understand why that is better\n>\n\nNeither do I. I actually don't think it's better in a general sense.\n\nSimply, the ImgU configuration procedure has been developed using\nresolutions closer to the sensor's native one, and hand tuned when it\nwas failing, I suppose. For the sensors it has been tested with (the\nSoraka and Nautilus ones, and maybe other ChromeOS devices), providing\nthe same set of input data makes sure the configuration won't fail.\n\nWill it work with other sensors the same way ? We have no guarantees\nbut with the platforms we can test it makes the configuration\nsuccessful.\n\n\n> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n>\n>\n> But does this mean that if I ask for a 16:9 1080p image, I'm going to\n> get a larger 4k image, which would then be perceptually squashed?\n>\n> The end result will still be cropped somewhere right?\n>\n>\n>\n> >  \tfloat bestRatio = std::numeric_limits<float>::max();\n> >  \tSize bestSize;\n> >  \tuint32_t bestCode = 0;\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 20426BDC71\n\tfor <parsemail@patchwork.libcamera.org>;\n\tSat, 28 Aug 2021 11:01:23 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6FA256916A;\n\tSat, 28 Aug 2021 13:01:22 +0200 (CEST)","from relay1-d.mail.gandi.net (relay1-d.mail.gandi.net\n\t[217.70.183.193])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 2E3E561534\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSat, 28 Aug 2021 13:01:20 +0200 (CEST)","(Authenticated sender: jacopo@jmondi.org)\n\tby relay1-d.mail.gandi.net (Postfix) with ESMTPSA id 4AACF240006;\n\tSat, 28 Aug 2021 11:01:19 +0000 (UTC)"],"Date":"Sat, 28 Aug 2021 13:02:08 +0200","From":"Jacopo Mondi <jacopo@jmondi.org>","To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Message-ID":"<20210828110208.dyrqpqc6yjhvzpra@uno.localdomain>","References":"<20210810075854.86191-1-umang.jain@ideasonboard.com>\n\t<20210810075854.86191-4-umang.jain@ideasonboard.com>\n\t<c17e77cd-871b-ff7b-cbac-1a71058d34bf@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<c17e77cd-871b-ff7b-cbac-1a71058d34bf@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v2 3/4] ipu3: cio2: Change sensor size\n\tselection policy","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]