[{"id":13692,"web_url":"https://patchwork.libcamera.org/comment/13692/","msgid":"<20201112235747.GD1603296@oden.dyn.berto.se>","date":"2020-11-12T23:57:47","subject":"Re: [libcamera-devel] [PATCH] libcamera: pipeline: rkisp1: fix crop\n\tconfiguration","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Helen,\n\nThanks for your work.\n\nOn 2020-11-06 17:32:27 -0300, Helen Koike wrote:\n> Crop rectangle was not being configured on the isp output pad nor in the\n> resizer input pad, causing an unecessary crop in the image and an\n> unecessary scaling by the resizer when streaming with a higher\n> resolution then the default 800x600.\n> \n> Example:\n> cam -c 1 -C -s width=3280,height=2464\n> \n> In the pipeline:\n> sensor->isp->resizer->dma_engine\n> \n> isp output crop is set to 800x600, which limits the output format to\n> 800x600, which is propagated to the resizer input format set to 800x600,\n> and the resizer output format is set to the desired end resolution\n> 3280x2464.\n> \n> Signed-off-by: Helen Koike \n> ---\n> src/libcamera/pipeline/rkisp1/rkisp1.cpp | 16 +++++++++++++---\n> src/libcamera/pipeline/rkisp1/rkisp1_path.cpp | 7 ++++++-\n> 2 files changed, 19 insertions(+), 4 deletions(-)\n> \n> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> index c74a2e9b..10d44400 100644\n> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> @@ -698,17 +698,27 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)\n> \tif (ret < 0)\n> \t\treturn ret;\n> \n> -\tLOG(RkISP1, Debug) << \"ISP input pad configured with \" << format.toString();\n> +\tLOG(RkISP1, Debug)\n> +\t\t<< \"ISP input pad configured with \" << format.toString()\n> +\t\t<< \" crop \" << rect.toString();\n> \n> \t/* YUYV8_2X8 is required on the ISP source path pad for YUV output. */\n> \tformat.mbus_code = MEDIA_BUS_FMT_YUYV8_2X8;\n> -\tLOG(RkISP1, Debug) << \"Configuring ISP output pad with \" << format.toString();\n> +\tLOG(RkISP1, Debug)\n> +\t\t<< \"Configuring ISP output pad with \" << format.toString()\n> +\t\t<< \" crop (img stabilizer) \" << rect.toString();\n\nI understand the crop rectangle may be used to implement image \nstabilization, but it's not yet. Is there any specific reason you \nmention it here or would it make sens to s/(img stabilizer)// ?\n\nFor the patch itself,\n\nTested-by: Niklas Söderlund \n\nI will wait for us to figure out the best wording here, but I can fix it \nwhile applying once we do.\n\n> +\n> +\tret = isp_->setSelection(2, V4L2_SEL_TGT_CROP, &rect);\n> +\tif (ret < 0)\n> +\t\treturn ret;\n> \n> \tret = isp_->setFormat(2, &format);\n> \tif (ret < 0)\n> \t\treturn ret;\n> \n> -\tLOG(RkISP1, Debug) << \"ISP output pad configured with \" << format.toString();\n> +\tLOG(RkISP1, Debug)\n> +\t\t<< \"ISP output pad configured with \" << format.toString()\n> +\t\t<< \" crop (img stabilizer) \" << rect.toString();\n> \n> \tfor (const StreamConfiguration &cfg : *config) {\n> \t\tif (cfg.stream() == &data->mainPathStream_)\n> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp\n> index e98515c8..50c0747c 100644\n> --- a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp\n> +++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp\n> @@ -117,9 +117,14 @@ int RkISP1Path::configure(const StreamConfiguration &config,\n> \tif (ret < 0)\n> \t\treturn ret;\n> \n> +\tRectangle rect(0, 0, ispFormat.size);\n> +\tret = resizer_->setSelection(0, V4L2_SEL_TGT_CROP, &rect);\n> +\tif (ret < 0)\n> +\t\treturn ret;\n> +\n> \tLOG(RkISP1, Debug)\n> \t\t<< \"Configured \" << name_ << \" resizer input pad with \"\n> -\t\t<< ispFormat.toString();\n> +\t\t<< ispFormat.toString() << \" crop \" << rect.toString();\n> \n> \tispFormat.size = config.size;\n> \n> -- \n> 2.29.2\n>","headers":{"Return-Path":"","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 BBE62BE082\n\tfor ;\n\tThu, 12 Nov 2020 23:57:52 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 4CA34631AE;\n\tFri, 13 Nov 2020 00:57:52 +0100 (CET)","from mail-lf1-x144.google.com (mail-lf1-x144.google.com\n\t[IPv6:2a00:1450:4864:20::144])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id DA3EA631AD\n\tfor ;\n\tFri, 13 Nov 2020 00:57:49 +0100 (CET)","by mail-lf1-x144.google.com with SMTP id j205so11182357lfj.6\n\tfor ;\n\tThu, 12 Nov 2020 15:57:49 -0800 (PST)","from localhost (h-209-203.A463.priv.bahnhof.se. [155.4.209.203])\n\tby smtp.gmail.com with ESMTPSA id\n\tt10sm1050798lfc.258.2020.11.12.15.57.48\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tThu, 12 Nov 2020 15:57:48 -0800 (PST)"],"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=\"DOVntVpf\"; 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=t1mVeUnZyaIlpg4rJuqNvE6YveFrBO2y2FURhecnRGY=;\n\tb=DOVntVpfEaN/p+om0mmD4/zmddF4ZuUnANthlwzZcreCHWEYCQQSfD2d1aRkQQWOMW\n\t4aQ6Sme8xdkBYXvRre+kyaoHK4ttRsOM+a4YMGM7Sws/7DkWpyMK7icimhBROb5C6cKd\n\trVBLKuhLRiuaaZZK7YZHObIvgG1yWWxUb1QxiMIkUYHNsljXKGBevnBEPD53ErN1zduH\n\tMU04/uxMcY69VfIP4h5VhBy7JqlXgDD9TENPwC0ct9v+IMJ74XD4EarsHCAtSVDjMn8i\n\tqvzn+SPAd2wWVHl+89s2MfvxlkQRifUH/h58WZpzsqxC01SFt+GyI8D1aVQl/VYzyUxq\n\tmpFg==","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=t1mVeUnZyaIlpg4rJuqNvE6YveFrBO2y2FURhecnRGY=;\n\tb=G9WW80tq4Q1jMX1tuSAsbVfeuZCveN5AqeTRoAjvC/eNGHpqMHkOI/AKwaxZecGwWG\n\tWb78+ZKR2VW8tA0lRAw1poegfC9gsPwwOhIImJ/t0g85GD2EjrXHEnot86Vnu5RW8GwH\n\tJkVTvewKlWX8iO0oFbGjFq5xeFi/cAaFuPMQn1jZ//fnzhhti3hdvOVZsj5XIo8iyk/k\n\trOQ8YxhcZ++OJCyIhD15+fGspgNID8G1VavoA1BT/HY5Y3025wOtjHBj4d7hLFYeU6XH\n\tQY1wLQf/AFu9nKIFeGyuOjAO0IHWnULigZrvxQU490zDxN0srOjON6cV2qeyh9CetGpW\n\tZyOA==","X-Gm-Message-State":"AOAM533WaKdbS3/vZACMx8+Jhnw1YBOOPpxm4Rn2BZYViXykQV/Xm5T/\n\t82PkwYsZjFBU+Cat5PTs999HvlQggfKsSQ==","X-Google-Smtp-Source":"ABdhPJwwqT2RTJYR58LxcUVjWiPAtSaDbpzF6tzruxSxN3HmFKv731pBsidVq96QMLFY+1qLiHqq3Q==","X-Received":"by 2002:a19:5052:: with SMTP id z18mr744271lfj.574.1605225469246;\n\tThu, 12 Nov 2020 15:57:49 -0800 (PST)","Date":"Fri, 13 Nov 2020 00:57:47 +0100","From":"Niklas =?iso-8859-1?q?S=F6derlund?= ","To":"Helen Koike ","Message-ID":"<20201112235747.GD1603296@oden.dyn.berto.se>","References":"<20201106203227.1780845-1-helen.koike@collabora.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20201106203227.1780845-1-helen.koike@collabora.com>","Subject":"Re: [libcamera-devel] [PATCH] libcamera: pipeline: rkisp1: fix crop\n\tconfiguration","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org, kernel@collabora.com","Content-Type":"text/plain; charset=\"iso-8859-1\"","Content-Transfer-Encoding":"quoted-printable","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":13709,"web_url":"https://patchwork.libcamera.org/comment/13709/","msgid":"","date":"2020-11-13T11:53:16","subject":"Re: [libcamera-devel] [PATCH] libcamera: pipeline: rkisp1: fix crop\n\tconfiguration","submitter":{"id":20,"url":"https://patchwork.libcamera.org/api/people/20/","name":"Helen Koike","email":"helen.koike@collabora.com"},"content":"Hi Niklas,\n\nOn 11/12/20 8:57 PM, Niklas Söderlund wrote:\n> Hi Helen,\n> \n> Thanks for your work.\n> \n> On 2020-11-06 17:32:27 -0300, Helen Koike wrote:\n>> Crop rectangle was not being configured on the isp output pad nor in the\n>> resizer input pad, causing an unecessary crop in the image and an\n>> unecessary scaling by the resizer when streaming with a higher\n>> resolution then the default 800x600.\n>>\n>> Example:\n>> cam -c 1 -C -s width=3280,height=2464\n>>\n>> In the pipeline:\n>> sensor->isp->resizer->dma_engine\n>>\n>> isp output crop is set to 800x600, which limits the output format to\n>> 800x600, which is propagated to the resizer input format set to 800x600,\n>> and the resizer output format is set to the desired end resolution\n>> 3280x2464.\n>>\n>> Signed-off-by: Helen Koike \n>> ---\n>> src/libcamera/pipeline/rkisp1/rkisp1.cpp | 16 +++++++++++++---\n>> src/libcamera/pipeline/rkisp1/rkisp1_path.cpp | 7 ++++++-\n>> 2 files changed, 19 insertions(+), 4 deletions(-)\n>>\n>> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n>> index c74a2e9b..10d44400 100644\n>> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n>> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n>> @@ -698,17 +698,27 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)\n>> \tif (ret < 0)\n>> \t\treturn ret;\n>> \n>> -\tLOG(RkISP1, Debug) << \"ISP input pad configured with \" << format.toString();\n>> +\tLOG(RkISP1, Debug)\n>> +\t\t<< \"ISP input pad configured with \" << format.toString()\n>> +\t\t<< \" crop \" << rect.toString();\n>> \n>> \t/* YUYV8_2X8 is required on the ISP source path pad for YUV output. */\n>> \tformat.mbus_code = MEDIA_BUS_FMT_YUYV8_2X8;\n>> -\tLOG(RkISP1, Debug) << \"Configuring ISP output pad with \" << format.toString();\n>> +\tLOG(RkISP1, Debug)\n>> +\t\t<< \"Configuring ISP output pad with \" << format.toString()\n>> +\t\t<< \" crop (img stabilizer) \" << rect.toString();\n> \n> I understand the crop rectangle may be used to implement image \n> stabilization, but it's not yet. Is there any specific reason you \n> mention it here or would it make sens to s/(img stabilizer)// ?\n\nThere is no specific reason, just to help identifying which part of the\ntopology this configuration applies.\nWe can remove it, no problem.\n\n> \n> For the patch itself,\n> \n> Tested-by: Niklas Söderlund \n\nThanks!\n\n> \n> I will wait for us to figure out the best wording here, but I can fix it \n> while applying once we do.\n\nThanks.\nHelen\n\n> \n>> +\n>> +\tret = isp_->setSelection(2, V4L2_SEL_TGT_CROP, &rect);\n>> +\tif (ret < 0)\n>> +\t\treturn ret;\n>> \n>> \tret = isp_->setFormat(2, &format);\n>> \tif (ret < 0)\n>> \t\treturn ret;\n>> \n>> -\tLOG(RkISP1, Debug) << \"ISP output pad configured with \" << format.toString();\n>> +\tLOG(RkISP1, Debug)\n>> +\t\t<< \"ISP output pad configured with \" << format.toString()\n>> +\t\t<< \" crop (img stabilizer) \" << rect.toString();\n>> \n>> \tfor (const StreamConfiguration &cfg : *config) {\n>> \t\tif (cfg.stream() == &data->mainPathStream_)\n>> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp\n>> index e98515c8..50c0747c 100644\n>> --- a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp\n>> +++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp\n>> @@ -117,9 +117,14 @@ int RkISP1Path::configure(const StreamConfiguration &config,\n>> \tif (ret < 0)\n>> \t\treturn ret;\n>> \n>> +\tRectangle rect(0, 0, ispFormat.size);\n>> +\tret = resizer_->setSelection(0, V4L2_SEL_TGT_CROP, &rect);\n>> +\tif (ret < 0)\n>> +\t\treturn ret;\n>> +\n>> \tLOG(RkISP1, Debug)\n>> \t\t<< \"Configured \" << name_ << \" resizer input pad with \"\n>> -\t\t<< ispFormat.toString();\n>> +\t\t<< ispFormat.toString() << \" crop \" << rect.toString();\n>> \n>> \tispFormat.size = config.size;\n>> \n>> -- \n>> 2.29.2\n>>\n>","headers":{"Return-Path":"","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 ECCEBBE082\n\tfor ;\n\tFri, 13 Nov 2020 11:53:33 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8816F631B8;\n\tFri, 13 Nov 2020 12:53:33 +0100 (CET)","from bhuna.collabora.co.uk (bhuna.collabora.co.uk [46.235.227.227])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 1E5BD63161\n\tfor ;\n\tFri, 13 Nov 2020 12:53:32 +0100 (CET)","from [127.0.0.1] (localhost [127.0.0.1])\n\t(Authenticated sender: koike) with ESMTPSA id C13521F4694D"],"To":"=?utf-8?q?Niklas_S=C3=B6derlund?= ","References":"<20201106203227.1780845-1-helen.koike@collabora.com>\n\t<20201112235747.GD1603296@oden.dyn.berto.se>","From":"Helen Koike ","Message-ID":"","Date":"Fri, 13 Nov 2020 08:53:16 -0300","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101\n\tThunderbird/78.4.0","MIME-Version":"1.0","In-Reply-To":"<20201112235747.GD1603296@oden.dyn.berto.se>","Content-Language":"en-US","Subject":"Re: [libcamera-devel] [PATCH] libcamera: pipeline: rkisp1: fix crop\n\tconfiguration","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org, kernel@collabora.com","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]