[{"id":13814,"web_url":"https://patchwork.libcamera.org/comment/13814/","msgid":"<96ba7671-14ef-f33a-6110-6f171e480a33@collabora.com>","date":"2020-11-20T13:40:43","subject":"Re: [libcamera-devel] [PATCH v2 0/1]\n\tpipeline-rkisp1-Fix-sensor-ISP-format-mismatch","submitter":{"id":20,"url":"https://patchwork.libcamera.org/api/people/20/","name":"Helen Koike","email":"helen.koike@collabora.com"},"content":"Hi Sebastian,\n\nThanks for your patch.\n\nOn 11/20/20 9:45 AM, Sebastian Fricke wrote:\n> This patch fixes a mismatch of image formats during the pipeline\n> creation of the RkISP1. The mismatch happens because the current code\n> does not check if the configured format exceeded the maximum viable\n> resolution of the ISP.\n> This patch fixes the issue by first setting the ISP, checking if the\n> given resolution was taken and searching for another viable format that\n> is lower or equal to the ISP maximum.\n\nThis description could be in the commit message, so we can save in the\nhistory the reasons for the change.\n\n> \n> ---\n> \n> Changelog:\n> \n> Changes since v1:\n> \n> * Change snake_case variable names to camelCase\n> * Use the request comment style\n> * Correct the scope of the newly implemented variables\n> * Correct the subject of the debug log for the ISP format configuration\n> * Update the comment above the ISP format configuration\n> * Check if the original format is not equal to the configured ISP format\n>   instead of checking if it is greater, this denies a false positive\n>   where the height exceeds the maximum while the width is smaller.\n>   If the configured format does not exceed the maximum resolution of the\n>   ISP, it will stay untouched so the inequality always means that we\n>   have to reconfigure the format.\n> * Adjust the comparison of the ISP format size with the available sensor\n>   formats, to detect a false-positive were the width is smaller while\n>   the height exceeds the maximum\n> * Use the standard function `max`\n> \n> ---\n> \n> The following tests were all able to create a working camera pipeline:\n> 1. Without stream configuration\n> 2. With a normal stream configuration\n> 3. With a stream configuration that exceeds the maximum of the ISP\n> 4. With a very small resolution stream configuration\n> 5. With a configuration that is closer to the upper than to the lower\n> resolution\n> \n> ----\n> \n> 1. LIBCAMERA_LOG_LEVELS=0 cam -c 1 --capture=3\n> \n> [8:05:49.668381732] [9973] DEBUG Camera camera.cpp:771 streams configuration: (0) 1920x1920-NV12\n> [8:05:49.669108269] [9973]  INFO Camera camera.cpp:830 configuring streams: (0) 1920x1920-NV12\n> ...\n> [8:05:49.670965005] [9974] DEBUG RkISP1 rkisp1.cpp:687 Configuring sensor with 4224x3136-SBGGR10_1X10\n> [8:05:49.671126004] [9974] DEBUG RkISP1 rkisp1.cpp:694 ISP configured with 4032x3024-SBGGR10_1X10\n> [8:05:49.671233045] [9974]  INFO RkISP1 rkisp1.cpp:697 Configured resolution is greater than the maximum resolution for the ISP, trying to re-configure to a smaller valid sensor format\n> [8:05:49.671378294] [9974] DEBUG RkISP1 rkisp1.cpp:716 ISP re-configured with 2112x1568-SBGGR10_1X10\n> [8:05:49.671510710] [9974] DEBUG RkISP1 rkisp1.cpp:724 Sensor configured with 2112x1568-SBGGR10_1X10\n> [8:05:49.671676667] [9974] DEBUG RkISP1 rkisp1.cpp:731 ISP input pad configured with 2112x1568-SBGGR10_1X10\n\nThere are two prints here for the same ISP pad, please see my comments on 1/1.\n\n> [8:05:49.671787791] [9974] DEBUG RkISP1 rkisp1.cpp:735 Configuring ISP output pad with 2112x1568-YUYV8_2X8\n> [8:05:49.671894540] [9974] DEBUG RkISP1 rkisp1.cpp:741 ISP output pad configured with 800x600-YUYV8_2X8\n> [8:05:49.672107747] [9974] DEBUG RkISP1 rkisp1_path.cpp:120 Configured main resizer input pad with 800x600-YUYV8_2X8\n> [8:05:49.672226163] [9974] DEBUG RkISP1 rkisp1_path.cpp:126 Configuring main resizer output pad with 1920x1920-YUYV8_2X8\n> [8:05:49.672341662] [9974] DEBUG RkISP1 rkisp1_path.cpp:144 Configured main resizer output pad with 1920x1920-YUYV8_1_5X8\n> \n> 2. LIBCAMERA_LOG_LEVELS=0 cam -c 1 --capture=3 -s width=900,height=600,pixelformat=NV12,role=video\n> \n> [8:07:05.725799205] [9977] DEBUG Camera camera.cpp:771 streams configuration: (0) 1920x1920-NV12\n> [8:07:05.726242535] [9977]  INFO Camera camera.cpp:830 configuring streams: (0) 900x600-NV12\n> ...\n> [8:07:05.727718025] [9978] DEBUG RkISP1 rkisp1.cpp:687 Configuring sensor with 2112x1568-SBGGR10_1X10\n> [8:07:05.727873482] [9978] DEBUG RkISP1 rkisp1.cpp:694 ISP configured with 2112x1568-SBGGR10_1X10\n> [8:07:05.728056356] [9978] DEBUG RkISP1 rkisp1.cpp:724 Sensor configured with 2112x1568-SBGGR10_1X10\n> [8:07:05.728233104] [9978] DEBUG RkISP1 rkisp1.cpp:731 ISP input pad configured with 2112x1568-SBGGR10_1X10\n> [8:07:05.728340437] [9978] DEBUG RkISP1 rkisp1.cpp:735 Configuring ISP output pad with 2112x1568-YUYV8_2X8\n> [8:07:05.728448353] [9978] DEBUG RkISP1 rkisp1.cpp:741 ISP output pad configured with 800x600-YUYV8_2X8\n> [8:07:05.728567352] [9978] DEBUG RkISP1 rkisp1_path.cpp:120 Configured main resizer input pad with 800x600-YUYV8_2X8\n> [8:07:05.728673809] [9978] DEBUG RkISP1 rkisp1_path.cpp:126 Configuring main resizer output pad with 900x600-YUYV8_2X8\n> [8:07:05.728785517] [9978] DEBUG RkISP1 rkisp1_path.cpp:144 Configured main resizer output pad with 900x600-YUYV8_1_5X8\n> \n> 3. LIBCAMERA_LOG_LEVELS=0 cam -c 1 --capture=3 -s width=4500,height=3500,pixelformat=NV12,role=video\n> \n> [8:08:02.372820979] [9981] DEBUG Camera camera.cpp:771 streams configuration: (0) 1920x1920-NV12\n> [8:08:02.373280350] [9981] DEBUG RkISP1 rkisp1_path.cpp:100 Adjusting format from 4500x3500-NV12 to 4416x3312-NV12\n> [8:08:02.373534098] [9981] DEBUG RkISP1 rkisp1_path.cpp:100 Adjusting format from 4500x3500-NV12 to 1920x1920-NV12\n> [8:08:02.373767722] [9981] DEBUG RkISP1 rkisp1_path.cpp:100 Adjusting format from 4500x3500-NV12 to 4416x3312-NV12\n> [8:08:02.374010095] [9981] DEBUG CameraSensor camera_sensor.cpp:408 'ov13850 1-0010': No supported format or size found\n> Camera configuration adjusted\n> [8:08:02.374449634] [9981] DEBUG CameraSensor camera_sensor.cpp:408 'ov13850 1-0010': No supported format or size found\n> [8:08:02.374591091] [9981]  INFO Camera camera.cpp:830 configuring streams: (0) 4416x3312-NV12\n> ...\n> [8:08:02.376301704] [9982] DEBUG RkISP1 rkisp1.cpp:687 Configuring sensor with 4224x3136-0x0000\n> [8:08:02.376469994] [9982] DEBUG RkISP1 rkisp1.cpp:694 ISP configured with 4032x3024-SRGGB10_1X10\n> [8:08:02.376576160] [9982]  INFO RkISP1 rkisp1.cpp:697 Configured resolution is greater than the maximum resolution for the ISP, trying to re-configure to a smaller valid sensor format\n> [8:08:02.376710909] [9982] DEBUG RkISP1 rkisp1.cpp:716 ISP re-configured with 2112x1568-SBGGR10_1X10\n> [8:08:02.376839241] [9982] DEBUG RkISP1 rkisp1.cpp:724 Sensor configured with 2112x1568-SBGGR10_1X10\n> [8:08:02.377001115] [9982] DEBUG RkISP1 rkisp1.cpp:731 ISP input pad configured with 2112x1568-SBGGR10_1X10\n> [8:08:02.377104364] [9982] DEBUG RkISP1 rkisp1.cpp:735 Configuring ISP output pad with 2112x1568-YUYV8_2X8\n> [8:08:02.377208780] [9982] DEBUG RkISP1 rkisp1.cpp:741 ISP output pad configured with 800x600-YUYV8_2X8\n> [8:08:02.377324863] [9982] DEBUG RkISP1 rkisp1_path.cpp:120 Configured main resizer input pad with 800x600-YUYV8_2X8\n> [8:08:02.377425779] [9982] DEBUG RkISP1 rkisp1_path.cpp:126 Configuring main resizer output pad with 4416x3312-YUYV8_2X8\n> [8:08:02.377533695] [9982] DEBUG RkISP1 rkisp1_path.cpp:144 Configured main resizer output pad with 4416x3312-YUYV8_1_5X8\n> \n> 4. LIBCAMERA_LOG_LEVELS=0 cam -c 1 --capture=3 -s width=40,height=30,pixelformat=NV12,role=video\n> \n> [8:10:41.140561164] [9989] DEBUG Camera camera.cpp:771 streams configuration: (0) 1920x1920-NV12\n> [8:10:41.141348367] [9989]  INFO Camera camera.cpp:830 configuring streams: (0) 40x30-NV12\n> ...\n> [8:10:41.143016688] [9990] DEBUG RkISP1 rkisp1.cpp:687 Configuring sensor with 2112x1568-SBGGR10_1X10\n> [8:10:41.143161062] [9990] DEBUG RkISP1 rkisp1.cpp:694 ISP configured with 2112x1568-SBGGR10_1X10\n> [8:10:41.143283561] [9990] DEBUG RkISP1 rkisp1.cpp:724 Sensor configured with 2112x1568-SBGGR10_1X10\n> [8:10:41.143442810] [9990] DEBUG RkISP1 rkisp1.cpp:731 ISP input pad configured with 2112x1568-SBGGR10_1X10\n> [8:10:41.143544893] [9990] DEBUG RkISP1 rkisp1.cpp:735 Configuring ISP output pad with 2112x1568-YUYV8_2X8\n> [8:10:41.143644934] [9990] DEBUG RkISP1 rkisp1.cpp:741 ISP output pad configured with 800x600-YUYV8_2X8\n> [8:10:41.143756058] [9990] DEBUG RkISP1 rkisp1_path.cpp:120 Configured main resizer input pad with 800x600-YUYV8_2X8\n> [8:10:41.143854640] [9990] DEBUG RkISP1 rkisp1_path.cpp:126 Configuring main resizer output pad with 40x30-YUYV8_2X8\n> [8:10:41.143960223] [9990] DEBUG RkISP1 rkisp1_path.cpp:144 Configured main resizer output pad with 40x30-YUYV8_1_5X8\n> \n> 5. LIBCAMERA_LOG_LEVELS=0 cam -c 1 --capture=3 -s width=3450,height=2456,pixelformat=NV12,role=video\n> \n> [8:11:40.321895811] [9992] DEBUG Camera camera.cpp:771 streams configuration: (0) 1920x1920-NV12\n> [8:11:40.322697014] [9992]  INFO Camera camera.cpp:830 configuring streams: (0) 3450x2456-NV12\n> ...\n> [8:11:40.324431543] [9993] DEBUG RkISP1 rkisp1.cpp:687 Configuring sensor with 4224x3136-SBGGR10_1X10\n> [8:11:40.324579125] [9993] DEBUG RkISP1 rkisp1.cpp:694 ISP configured with 4032x3024-SBGGR10_1X10\n> [8:11:40.324680041] [9993]  INFO RkISP1 rkisp1.cpp:697 Configured resolution is greater than the maximum resolution for the ISP, trying to re-configure to a smaller valid sensor format\n> [8:11:40.324815082] [9993] DEBUG RkISP1 rkisp1.cpp:716 ISP re-configured with 2112x1568-SBGGR10_1X10\n> [8:11:40.324946039] [9993] DEBUG RkISP1 rkisp1.cpp:724 Sensor configured with 2112x1568-SBGGR10_1X10\n> [8:11:40.325111121] [9993] DEBUG RkISP1 rkisp1.cpp:731 ISP input pad configured with 2112x1568-SBGGR10_1X10\n> [8:11:40.325216121] [9993] DEBUG RkISP1 rkisp1.cpp:735 Configuring ISP output pad with 2112x1568-YUYV8_2X8\n> [8:11:40.325323453] [9993] DEBUG RkISP1 rkisp1.cpp:741 ISP output pad configured with 800x600-YUYV8_2X8\n> [8:11:40.325441286] [9993] DEBUG RkISP1 rkisp1_path.cpp:120 Configured main resizer input pad with 800x600-YUYV8_2X8\n> [8:11:40.325547451] [9993] DEBUG RkISP1 rkisp1_path.cpp:126 Configuring main resizer output pad with 3450x2456-YUYV8_2X8\n> [8:11:40.325660909] [9993] DEBUG RkISP1 rkisp1_path.cpp:144 Configured main resizer output pad with 3450x2456-YUYV8_1_5X8\n> \n\nJust a tip, when sending a single patch, I usually don't\nbother sending a cover letter, I would put all this information\nafter the 3 dashes \"---\" in the patch (what is written after those\n3 dashes will be ignored when git am file.patch)\n\nBut maybe this is just my style, but I wanted to let you know\nyou have this option :)\n\nRegards,\nHelen\n\n\n> -----\n> \n> Sebastian Fricke (1):\n>   pipeline: rkisp1: Fix sensor-ISP format mismatch\n> \n>  src/libcamera/pipeline/rkisp1/rkisp1.cpp | 46 ++++++++++++++++++++----\n>  1 file changed, 40 insertions(+), 6 deletions(-)\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 619BBBE176\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 20 Nov 2020 13:40:51 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id ECE92625AC;\n\tFri, 20 Nov 2020 14:40:50 +0100 (CET)","from bhuna.collabora.co.uk (bhuna.collabora.co.uk\n\t[IPv6:2a00:1098:0:82:1000:25:2eeb:e3e3])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id CBCF06220D\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 20 Nov 2020 14:40:49 +0100 (CET)","from [127.0.0.1] (localhost [127.0.0.1])\n\t(Authenticated sender: koike) with ESMTPSA id 5F8A61F45F31"],"To":"Sebastian Fricke <sebastian.fricke.linux@gmail.com>,\n\tlibcamera-devel@lists.libcamera.org","References":"<20201120124503.22718-1-sebastian.fricke.linux@gmail.com>","From":"Helen Koike <helen.koike@collabora.com>","Message-ID":"<96ba7671-14ef-f33a-6110-6f171e480a33@collabora.com>","Date":"Fri, 20 Nov 2020 10:40:43 -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":"<20201120124503.22718-1-sebastian.fricke.linux@gmail.com>","Content-Language":"en-US","Subject":"Re: [libcamera-devel] [PATCH v2 0/1]\n\tpipeline-rkisp1-Fix-sensor-ISP-format-mismatch","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>","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]