[{"id":11287,"web_url":"https://patchwork.libcamera.org/comment/11287/","msgid":"<20200709134654.GO3875643@oden.dyn.berto.se>","date":"2020-07-09T13:46:54","subject":"Re: [libcamera-devel] [PATCH v2 12/20] libcamera: ipu3: Always use\n\tthe maximum frame size","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Jacopo,\n\nThanks for your work.\n\nOn 2020-07-09 10:41:20 +0200, Jacopo Mondi wrote:\n> The requirement of having the ImgU output height 32 pixels smaller\n> than the input frame produced by the CIO2 makes it complicated to\n> re-adjust the sensor produced size after the alignement has been\n> applied.\n> \n> To simplify the procedure, always ask for the full frame size from the\n> CIO2 unit.\n> \n> Signed-off-by: Jacopo Mondi \n> ---\n> src/libcamera/pipeline/ipu3/ipu3.cpp | 12 ++++--------\n> 1 file changed, 4 insertions(+), 8 deletions(-)\n> \n> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> index d07f1a7b5ae8..feabffe641e1 100644\n> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> @@ -156,8 +156,6 @@ CameraConfiguration::Status IPU3CameraConfiguration::validate()\n> \tunsigned int rawCount = 0;\n> \tunsigned int outCount = 0;\n> \tSize yuvSize;\n> -\tSize size;\n> -\n> \tfor (const StreamConfiguration &cfg : config_) {\n> \t\tconst PixelFormatInfo &info =\n> \t\t\tPixelFormatInfo::info(cfg.pixelFormat);\n> @@ -174,11 +172,6 @@ CameraConfiguration::Status IPU3CameraConfiguration::validate()\n> \t\t\tif (cfg.size > yuvSize)\n> \t\t\t\tyuvSize = cfg.size;\n> \t\t}\n> -\n> -\t\tif (cfg.size.width > size.width)\n> -\t\t\tsize.width = cfg.size.width;\n> -\t\tif (cfg.size.height > size.height)\n> -\t\t\tsize.height = cfg.size.height;\n> \t}\n> \tif (rawCount > 1 || outCount > 2) {\n> \t\tLOG(IPU3, Error)\n> @@ -189,10 +182,13 @@ CameraConfiguration::Status IPU3CameraConfiguration::validate()\n> \t}\n> \n> \t/* Generate raw configuration from CIO2. */\n> -\tcio2Configuration_ = data_->cio2_.generateConfiguration(size);\n> +\tSize sensorSize = data_->cio2_.sensor()->resolution();\n> +\tcio2Configuration_ = data_->cio2_.generateConfiguration(sensorSize);\n\nCould this not impact framerate for the CIO2 if we always request the \nlargest sensor size? Would it be possible instead to increase the size \npassed to CIO2Device::generateConfiguration() by 32?\n\n> \tif (!cio2Configuration_.pixelFormat.isValid())\n> \t\treturn Invalid;\n> \n> +\tLOG(IPU3, Debug) << \"CIO2 configuration: \" << cio2Configuration_.toString();\n> +\n> \t/*\n> \t * Adjust the configurations if needed and assign streams while\n> \t * iterating them.\n> -- \n> 2.27.0\n> \n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","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 E00EBBD792\n\tfor ;\n\tThu, 9 Jul 2020 13:46:57 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 78BA8611B2;\n\tThu, 9 Jul 2020 15:46:57 +0200 (CEST)","from mail-lj1-x242.google.com (mail-lj1-x242.google.com\n\t[IPv6:2a00:1450:4864:20::242])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 3B13D611B2\n\tfor ;\n\tThu, 9 Jul 2020 15:46:56 +0200 (CEST)","by mail-lj1-x242.google.com with SMTP id f5so2431072ljj.10\n\tfor ;\n\tThu, 09 Jul 2020 06:46:56 -0700 (PDT)","from localhost (h-209-203.A463.priv.bahnhof.se. [155.4.209.203])\n\tby smtp.gmail.com with ESMTPSA id\n\tg2sm1069804ljj.90.2020.07.09.06.46.54\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tThu, 09 Jul 2020 06:46:54 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=ragnatech-se.20150623.gappssmtp.com\n\theader.i=@ragnatech-se.20150623.gappssmtp.com\n\theader.b=\"v3cH0gOH\"; 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=0+BfeBQqSR+GJt7wih07KRLefF/bEn9boEHGkXbJXKo=;\n\tb=v3cH0gOHTTAUSeC4ic0NSjPAXwJYvMH5Q+8vZp+oR+FZhpLo3XqFQmHFJx/Li8XzjS\n\tKkCSdjqeD2Z1WF8wenQTnYRwZtArSp8urRmxRaa106rrOxDaa1fpNcL5yDJ4hVXHP2Ll\n\tVxOOp2H91XedDxFNQdkuY64njnAYpwTVuSn4PQklgq3v0Hy4A8NXLccxEobhB7TKWLfo\n\tJf3t52H2S2T+VA+dbk7wYTdBOH67ou033hD980qsIfcVs0LGHaO4azojpkP5T6IoaBUQ\n\tEjHYYJmThBRuP4XBcxMARcJUQu6HtPPpubMB6+ny02Jmi6G4MSf89fkPGB2AQ+q7Dfpq\n\thaKA==","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=0+BfeBQqSR+GJt7wih07KRLefF/bEn9boEHGkXbJXKo=;\n\tb=abntbuPbBsuWx6FBzcfQbfV1gL4iyFzQcZkIwiOkTiyK2c6fMff1sngoBMJSvoEANp\n\tPoQalLWTrH+pV0H4crb5lnTMNCic94ej2eKT/m8K/rgHD6BPR5xXPPoUY//d9hUdlLfD\n\twYU0zhkt0Yrdps/yRGpb5r3G17YCnzM+/0fDbLz5wR75XIPkr9MOZDsaD+ox6Pw2cgln\n\tpwz74vI5Hf9QNnY5gaHWmo/lSAAVMCK943SHYFwo33g8Ijvdt4ba5k68zt5YCE4AFjlG\n\t3dd0JTlk23EtA0A6TKqY711qIWA9cUrO96BkGUVc+pUbco8aib5LlGWZkCGtxfK/DZgs\n\t1QjQ==","X-Gm-Message-State":"AOAM530RxFZZiyMgnFUiErWbdypxD52+ULj6Dc25mDHTbfUyKRXMDSiy\n\toyA/dIJFc9XuUico0riso3aIt4g4CjM=","X-Google-Smtp-Source":"ABdhPJxDc+I9saT8jDBarfXFEoCzl0n6XUPKEX4ZtvDXOzagtOO63vE0UZ0nX2aYtaNV1pmP6dtDNw==","X-Received":"by 2002:a2e:a413:: with SMTP id\n\tp19mr26607564ljn.145.1594302415573; \n\tThu, 09 Jul 2020 06:46:55 -0700 (PDT)","Date":"Thu, 9 Jul 2020 15:46:54 +0200","From":"Niklas =?iso-8859-1?q?S=F6derlund?= ","To":"Jacopo Mondi ","Message-ID":"<20200709134654.GO3875643@oden.dyn.berto.se>","References":"<20200709084128.5316-1-jacopo@jmondi.org>\n\t<20200709084128.5316-13-jacopo@jmondi.org>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200709084128.5316-13-jacopo@jmondi.org>","Subject":"Re: [libcamera-devel] [PATCH v2 12/20] libcamera: ipu3: Always use\n\tthe maximum frame size","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","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":11310,"web_url":"https://patchwork.libcamera.org/comment/11310/","msgid":"<20200710073853.476lehorlkfmnicr@uno.localdomain>","date":"2020-07-10T07:38:53","subject":"Re: [libcamera-devel] [PATCH v2 12/20] libcamera: ipu3: Always use\n\tthe maximum frame size","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Niklas,\n\nOn Thu, Jul 09, 2020 at 03:46:54PM +0200, Niklas Söderlund wrote:\n> Hi Jacopo,\n>\n> Thanks for your work.\n>\n> On 2020-07-09 10:41:20 +0200, Jacopo Mondi wrote:\n> > The requirement of having the ImgU output height 32 pixels smaller\n> > than the input frame produced by the CIO2 makes it complicated to\n> > re-adjust the sensor produced size after the alignement has been\n> > applied.\n> >\n> > To simplify the procedure, always ask for the full frame size from the\n> > CIO2 unit.\n> >\n> > Signed-off-by: Jacopo Mondi \n> > ---\n> > src/libcamera/pipeline/ipu3/ipu3.cpp | 12 ++++--------\n> > 1 file changed, 4 insertions(+), 8 deletions(-)\n> >\n> > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > index d07f1a7b5ae8..feabffe641e1 100644\n> > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > @@ -156,8 +156,6 @@ CameraConfiguration::Status IPU3CameraConfiguration::validate()\n> > \tunsigned int rawCount = 0;\n> > \tunsigned int outCount = 0;\n> > \tSize yuvSize;\n> > -\tSize size;\n> > -\n> > \tfor (const StreamConfiguration &cfg : config_) {\n> > \t\tconst PixelFormatInfo &info =\n> > \t\t\tPixelFormatInfo::info(cfg.pixelFormat);\n> > @@ -174,11 +172,6 @@ CameraConfiguration::Status IPU3CameraConfiguration::validate()\n> > \t\t\tif (cfg.size > yuvSize)\n> > \t\t\t\tyuvSize = cfg.size;\n> > \t\t}\n> > -\n> > -\t\tif (cfg.size.width > size.width)\n> > -\t\t\tsize.width = cfg.size.width;\n> > -\t\tif (cfg.size.height > size.height)\n> > -\t\t\tsize.height = cfg.size.height;\n> > \t}\n> > \tif (rawCount > 1 || outCount > 2) {\n> > \t\tLOG(IPU3, Error)\n> > @@ -189,10 +182,13 @@ CameraConfiguration::Status IPU3CameraConfiguration::validate()\n> > \t}\n> >\n> > \t/* Generate raw configuration from CIO2. */\n> > -\tcio2Configuration_ = data_->cio2_.generateConfiguration(size);\n> > +\tSize sensorSize = data_->cio2_.sensor()->resolution();\n> > +\tcio2Configuration_ = data_->cio2_.generateConfiguration(sensorSize);\n>\n> Could this not impact framerate for the CIO2 if we always request the\n> largest sensor size? Would it be possible instead to increase the size\n> passed to CIO2Device::generateConfiguration() by 32?\n>\n\nYes it does, and this is probably just wrong, as it ignores the\nrequested raw capture size.\n\nFun fact which actually lead me to add this change: if I keep the\nprevious version where the sensor is asked for the smaller as possible\nresolution that supports all stream (+32 as you here suggest) the IPU3\npipeline parameter calculations fail on Soraka for some output\nresolutions.\n\nIn example: calculating the configuration for a single stream in\n1280x720 with a frame resolution of 2112x1188 (which is the smaller\nov13858 resolution that is large enough) fails: the tool is not able\nto find any suitable configuration that satisfied the pipeline\nconstraints:\n\n$ python pipe_config.py input=2112x1188 main=1280x720\nIndexError: list index out of range\n\nIf I use the full frame size, everything is unicorns&rainbows again\n\n$ python pipe_config.py input=4224x3136 main=1280x720\n-------- The selected pipe configuration: --------------\noutput_res_if:4220x2600\noutput_res_bds:1688x1040\noutput_res_gdc:1280x720\nFOV Horizontal:0.757576, FOV vertical:0.573980\n\nWhy ? I have no idea, and I think we should stop waiting for Intel to\nactually care about this platform anymore, so I don't expect they\ncould help us debugging the tool.\n\nThe re-implementation I made performs the same calculations as the\nscript, hence reducing the frame size makes a lot of configurations\nfail.\n\nIf you look at\nhttps://chromium.googlesource.com/chromiumos/overlays/board-overlays/+/master/baseboard-poppy/media-libs/cros-camera-hal-configs-poppy/files/gcss/graph_settings_ov5670.xml\nhttps://chromium.googlesource.com/chromiumos/overlays/board-overlays/+/master/baseboard-poppy/media-libs/cros-camera-hal-configs-poppy/files/gcss/graph_settings_ov13858.xml\n\nThe sensor is always asked for the full frame, so I suspect that's how\nthe system is intenteded to work (even if, I wonder, what happens if I\nconnect a sensor that can only produce the above failing 2112x1188\nresolution ?)\n\nI -fear- for IPU3 we should always go full frame size to make the pipe\nconfig happy, or there's maybe a requirement to respect some aspect\nratio I still haven't been able to figure out. In any case, this would\nhinder the ability to capture raw+YUV, as the raw frame resolution in\nthat case is selected from the application, and it might prevent the\npipeline from being able to calculate a configuration.\n\nNot nice, I know.\n\nIf that's how thing have to work, a software scaler for raw frames\nwould nee to be plugged to the pipeline to down-scale raw frames to\nthe desired resolution. Or we restrict RAW streams to the full size\nonly. What do you think ?\n\n\n> > \tif (!cio2Configuration_.pixelFormat.isValid())\n> > \t\treturn Invalid;\n> >\n> > +\tLOG(IPU3, Debug) << \"CIO2 configuration: \" << cio2Configuration_.toString();\n> > +\n> > \t/*\n> > \t * Adjust the configurations if needed and assign streams while\n> > \t * iterating them.\n> > --\n> > 2.27.0\n> >\n> > _______________________________________________\n> > libcamera-devel mailing list\n> > libcamera-devel@lists.libcamera.org\n> > https://lists.libcamera.org/listinfo/libcamera-devel\n>\n> --\n> Regards,\n> Niklas Söderlund","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 C4D4CBD792\n\tfor ;\n\tFri, 10 Jul 2020 07:35:21 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3BCAF613A9;\n\tFri, 10 Jul 2020 09:35:21 +0200 (CEST)","from relay9-d.mail.gandi.net (relay9-d.mail.gandi.net\n\t[217.70.183.199])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id DF31861253\n\tfor ;\n\tFri, 10 Jul 2020 09:35:19 +0200 (CEST)","from uno.localdomain (host-95-245-128-189.retail.telecomitalia.it\n\t[95.245.128.189]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay9-d.mail.gandi.net (Postfix) with ESMTPSA id D0BA7FF807;\n\tFri, 10 Jul 2020 07:35:18 +0000 (UTC)"],"X-Originating-IP":"95.245.128.189","Date":"Fri, 10 Jul 2020 09:38:53 +0200","From":"Jacopo Mondi ","To":"Niklas =?utf-8?q?S=C3=B6derlund?= ","Message-ID":"<20200710073853.476lehorlkfmnicr@uno.localdomain>","References":"<20200709084128.5316-1-jacopo@jmondi.org>\n\t<20200709084128.5316-13-jacopo@jmondi.org>\n\t<20200709134654.GO3875643@oden.dyn.berto.se>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200709134654.GO3875643@oden.dyn.berto.se>","Subject":"Re: [libcamera-devel] [PATCH v2 12/20] libcamera: ipu3: Always use\n\tthe maximum frame size","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","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":11322,"web_url":"https://patchwork.libcamera.org/comment/11322/","msgid":"<20200710120740.GN5964@pendragon.ideasonboard.com>","date":"2020-07-10T12:07:40","subject":"Re: [libcamera-devel] [PATCH v2 12/20] libcamera: ipu3: Always use\n\tthe maximum frame size","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nThank you for the patch.\n\nOn Fri, Jul 10, 2020 at 09:38:53AM +0200, Jacopo Mondi wrote:\n> On Thu, Jul 09, 2020 at 03:46:54PM +0200, Niklas Söderlund wrote:\n> > On 2020-07-09 10:41:20 +0200, Jacopo Mondi wrote:\n> > > The requirement of having the ImgU output height 32 pixels smaller\n> > > than the input frame produced by the CIO2 makes it complicated to\n> > > re-adjust the sensor produced size after the alignement has been\n> > > applied.\n> > >\n> > > To simplify the procedure, always ask for the full frame size from the\n> > > CIO2 unit.\n> > >\n> > > Signed-off-by: Jacopo Mondi \n> > > ---\n> > > src/libcamera/pipeline/ipu3/ipu3.cpp | 12 ++++--------\n> > > 1 file changed, 4 insertions(+), 8 deletions(-)\n> > >\n> > > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > > index d07f1a7b5ae8..feabffe641e1 100644\n> > > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > > @@ -156,8 +156,6 @@ CameraConfiguration::Status IPU3CameraConfiguration::validate()\n> > > \tunsigned int rawCount = 0;\n> > > \tunsigned int outCount = 0;\n> > > \tSize yuvSize;\n> > > -\tSize size;\n> > > -\n> > > \tfor (const StreamConfiguration &cfg : config_) {\n> > > \t\tconst PixelFormatInfo &info =\n> > > \t\t\tPixelFormatInfo::info(cfg.pixelFormat);\n> > > @@ -174,11 +172,6 @@ CameraConfiguration::Status IPU3CameraConfiguration::validate()\n> > > \t\t\tif (cfg.size > yuvSize)\n> > > \t\t\t\tyuvSize = cfg.size;\n> > > \t\t}\n> > > -\n> > > -\t\tif (cfg.size.width > size.width)\n> > > -\t\t\tsize.width = cfg.size.width;\n> > > -\t\tif (cfg.size.height > size.height)\n> > > -\t\t\tsize.height = cfg.size.height;\n> > > \t}\n> > > \tif (rawCount > 1 || outCount > 2) {\n> > > \t\tLOG(IPU3, Error)\n> > > @@ -189,10 +182,13 @@ CameraConfiguration::Status IPU3CameraConfiguration::validate()\n> > > \t}\n> > >\n> > > \t/* Generate raw configuration from CIO2. */\n> > > -\tcio2Configuration_ = data_->cio2_.generateConfiguration(size);\n> > > +\tSize sensorSize = data_->cio2_.sensor()->resolution();\n> > > +\tcio2Configuration_ = data_->cio2_.generateConfiguration(sensorSize);\n> >\n> > Could this not impact framerate for the CIO2 if we always request the\n> > largest sensor size? Would it be possible instead to increase the size\n> > passed to CIO2Device::generateConfiguration() by 32?\n> \n> Yes it does, and this is probably just wrong, as it ignores the\n> requested raw capture size.\n> \n> Fun fact which actually lead me to add this change: if I keep the\n> previous version where the sensor is asked for the smaller as possible\n> resolution that supports all stream (+32 as you here suggest) the IPU3\n> pipeline parameter calculations fail on Soraka for some output\n> resolutions.\n> \n> In example: calculating the configuration for a single stream in\n> 1280x720 with a frame resolution of 2112x1188 (which is the smaller\n> ov13858 resolution that is large enough) fails: the tool is not able\n> to find any suitable configuration that satisfied the pipeline\n> constraints:\n> \n> $ python pipe_config.py input=2112x1188 main=1280x720\n> IndexError: list index out of range\n> \n> If I use the full frame size, everything is unicorns&rainbows again\n> \n> $ python pipe_config.py input=4224x3136 main=1280x720\n> -------- The selected pipe configuration: --------------\n> output_res_if:4220x2600\n> output_res_bds:1688x1040\n> output_res_gdc:1280x720\n> FOV Horizontal:0.757576, FOV vertical:0.573980\n> \n> Why ? I have no idea, and I think we should stop waiting for Intel to\n> actually care about this platform anymore, so I don't expect they\n> could help us debugging the tool.\n\nWhat's the alternative, to you want to debug it yourself ? :-) If there\nare issue, please report them, and you can CC Bingbu and Tomasz.\n\n> The re-implementation I made performs the same calculations as the\n> script, hence reducing the frame size makes a lot of configurations\n> fail.\n> \n> If you look at\n> https://chromium.googlesource.com/chromiumos/overlays/board-overlays/+/master/baseboard-poppy/media-libs/cros-camera-hal-configs-poppy/files/gcss/graph_settings_ov5670.xml\n> https://chromium.googlesource.com/chromiumos/overlays/board-overlays/+/master/baseboard-poppy/media-libs/cros-camera-hal-configs-poppy/files/gcss/graph_settings_ov13858.xml\n> \n> The sensor is always asked for the full frame,\n\nIs it ? For the ov5670 I see three configurations using 1296x972, and\nfor the ov13858 I see nine configurations with smaller sensor\nresolutions.\n\n> so I suspect that's how\n> the system is intenteded to work (even if, I wonder, what happens if I\n> connect a sensor that can only produce the above failing 2112x1188\n> resolution ?)\n> \n> I -fear- for IPU3 we should always go full frame size to make the pipe\n> config happy, or there's maybe a requirement to respect some aspect\n> ratio I still haven't been able to figure out. In any case, this would\n> hinder the ability to capture raw+YUV, as the raw frame resolution in\n> that case is selected from the application, and it might prevent the\n> pipeline from being able to calculate a configuration.\n> \n> Not nice, I know.\n> \n> If that's how thing have to work, a software scaler for raw frames\n> would nee to be plugged to the pipeline to down-scale raw frames to\n> the desired resolution. Or we restrict RAW streams to the full size\n> only. What do you think ?\n\nNo software scaler, no. If a configuration isn't supported by the\nhardware we can obviously reject it. For instance if the application\nrequests raw in 640x480 and wants YUV in a larger resolution, as the\nImgU can't downscale, we can't accept that, and we can go for a higher\nraw resolution or a small YUV resolution. There's nothing new under the\nsun here, and I'm OK with temporarily restricting the raw frames to the\nfull sensor resolution if it helps moving forward, but let's keep in\nmind we may have to fix that at some point in the future, so we\nshouldn't hardcode too much.\n\n> > > \tif (!cio2Configuration_.pixelFormat.isValid())\n> > > \t\treturn Invalid;\n> > >\n> > > +\tLOG(IPU3, Debug) << \"CIO2 configuration: \" << cio2Configuration_.toString();\n> > > +\n> > > \t/*\n> > > \t * Adjust the configurations if needed and assign streams while\n> > > \t * iterating them.","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 4FA55BD792\n\tfor ;\n\tFri, 10 Jul 2020 12:07:49 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id DEF2A611BA;\n\tFri, 10 Jul 2020 14:07:48 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 25D3E6118A\n\tfor ;\n\tFri, 10 Jul 2020 14:07:47 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 98D6A2C0;\n\tFri, 10 Jul 2020 14:07:46 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"iQC/ru/k\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1594382866;\n\tbh=c2XLKyFNX+/POdILlqr5AMoXaFgm6JOvr4ziDHcNkeE=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=iQC/ru/kgOqDgLZXecw2iVgb7dL1mNI3HEGbwStVEJ6UBMaYlr9ODclkk4mcL5/2u\n\tJDVqesTKBUPntPw+7z/mOssI7owpL8Vr5oUTcrTUDrEdWmT18+ZtKJKQNpMsjlN9oL\n\tqEOrBXzkbaPDA/DWvfm9XV/Wmtd26iLi9vBdNthg=","Date":"Fri, 10 Jul 2020 15:07:40 +0300","From":"Laurent Pinchart ","To":"Jacopo Mondi ","Message-ID":"<20200710120740.GN5964@pendragon.ideasonboard.com>","References":"<20200709084128.5316-1-jacopo@jmondi.org>\n\t<20200709084128.5316-13-jacopo@jmondi.org>\n\t<20200709134654.GO3875643@oden.dyn.berto.se>\n\t<20200710073853.476lehorlkfmnicr@uno.localdomain>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200710073853.476lehorlkfmnicr@uno.localdomain>","Subject":"Re: [libcamera-devel] [PATCH v2 12/20] libcamera: ipu3: Always use\n\tthe maximum frame size","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","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":11329,"web_url":"https://patchwork.libcamera.org/comment/11329/","msgid":"<20200710123538.qwlysoj2ggowpgdp@uno.localdomain>","date":"2020-07-10T12:35:38","subject":"Re: [libcamera-devel] [PATCH v2 12/20] libcamera: ipu3: Always use\n\tthe maximum frame size","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Laurent,\n\nOn Fri, Jul 10, 2020 at 03:07:40PM +0300, Laurent Pinchart wrote:\n> Hi Jacopo,\n>\n> Thank you for the patch.\n>\n> On Fri, Jul 10, 2020 at 09:38:53AM +0200, Jacopo Mondi wrote:\n> > On Thu, Jul 09, 2020 at 03:46:54PM +0200, Niklas Söderlund wrote:\n> > > On 2020-07-09 10:41:20 +0200, Jacopo Mondi wrote:\n> > > > The requirement of having the ImgU output height 32 pixels smaller\n> > > > than the input frame produced by the CIO2 makes it complicated to\n> > > > re-adjust the sensor produced size after the alignement has been\n> > > > applied.\n> > > >\n> > > > To simplify the procedure, always ask for the full frame size from the\n> > > > CIO2 unit.\n> > > >\n> > > > Signed-off-by: Jacopo Mondi \n> > > > ---\n> > > > src/libcamera/pipeline/ipu3/ipu3.cpp | 12 ++++--------\n> > > > 1 file changed, 4 insertions(+), 8 deletions(-)\n> > > >\n> > > > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > > > index d07f1a7b5ae8..feabffe641e1 100644\n> > > > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > > > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > > > @@ -156,8 +156,6 @@ CameraConfiguration::Status IPU3CameraConfiguration::validate()\n> > > > \tunsigned int rawCount = 0;\n> > > > \tunsigned int outCount = 0;\n> > > > \tSize yuvSize;\n> > > > -\tSize size;\n> > > > -\n> > > > \tfor (const StreamConfiguration &cfg : config_) {\n> > > > \t\tconst PixelFormatInfo &info =\n> > > > \t\t\tPixelFormatInfo::info(cfg.pixelFormat);\n> > > > @@ -174,11 +172,6 @@ CameraConfiguration::Status IPU3CameraConfiguration::validate()\n> > > > \t\t\tif (cfg.size > yuvSize)\n> > > > \t\t\t\tyuvSize = cfg.size;\n> > > > \t\t}\n> > > > -\n> > > > -\t\tif (cfg.size.width > size.width)\n> > > > -\t\t\tsize.width = cfg.size.width;\n> > > > -\t\tif (cfg.size.height > size.height)\n> > > > -\t\t\tsize.height = cfg.size.height;\n> > > > \t}\n> > > > \tif (rawCount > 1 || outCount > 2) {\n> > > > \t\tLOG(IPU3, Error)\n> > > > @@ -189,10 +182,13 @@ CameraConfiguration::Status IPU3CameraConfiguration::validate()\n> > > > \t}\n> > > >\n> > > > \t/* Generate raw configuration from CIO2. */\n> > > > -\tcio2Configuration_ = data_->cio2_.generateConfiguration(size);\n> > > > +\tSize sensorSize = data_->cio2_.sensor()->resolution();\n> > > > +\tcio2Configuration_ = data_->cio2_.generateConfiguration(sensorSize);\n> > >\n> > > Could this not impact framerate for the CIO2 if we always request the\n> > > largest sensor size? Would it be possible instead to increase the size\n> > > passed to CIO2Device::generateConfiguration() by 32?\n> >\n> > Yes it does, and this is probably just wrong, as it ignores the\n> > requested raw capture size.\n> >\n> > Fun fact which actually lead me to add this change: if I keep the\n> > previous version where the sensor is asked for the smaller as possible\n> > resolution that supports all stream (+32 as you here suggest) the IPU3\n> > pipeline parameter calculations fail on Soraka for some output\n> > resolutions.\n> >\n> > In example: calculating the configuration for a single stream in\n> > 1280x720 with a frame resolution of 2112x1188 (which is the smaller\n> > ov13858 resolution that is large enough) fails: the tool is not able\n> > to find any suitable configuration that satisfied the pipeline\n> > constraints:\n> >\n> > $ python pipe_config.py input=2112x1188 main=1280x720\n> > IndexError: list index out of range\n> >\n> > If I use the full frame size, everything is unicorns&rainbows again\n> >\n> > $ python pipe_config.py input=4224x3136 main=1280x720\n> > -------- The selected pipe configuration: --------------\n> > output_res_if:4220x2600\n> > output_res_bds:1688x1040\n> > output_res_gdc:1280x720\n> > FOV Horizontal:0.757576, FOV vertical:0.573980\n> >\n> > Why ? I have no idea, and I think we should stop waiting for Intel to\n> > actually care about this platform anymore, so I don't expect they\n> > could help us debugging the tool.\n>\n> What's the alternative, to you want to debug it yourself ? :-) If there\n> are issue, please report them, and you can CC Bingbu and Tomasz.\n>\n\nThe alternative is to live with the risk of the procedure not working\nfor all resolutions. Which is anyway better thant what we have today\n\n> > The re-implementation I made performs the same calculations as the\n> > script, hence reducing the frame size makes a lot of configurations\n> > fail.\n> >\n> > If you look at\n> > https://chromium.googlesource.com/chromiumos/overlays/board-overlays/+/master/baseboard-poppy/media-libs/cros-camera-hal-configs-poppy/files/gcss/graph_settings_ov5670.xml\n> > https://chromium.googlesource.com/chromiumos/overlays/board-overlays/+/master/baseboard-poppy/media-libs/cros-camera-hal-configs-poppy/files/gcss/graph_settings_ov13858.xml\n> >\n> > The sensor is always asked for the full frame,\n>\n> Is it ? For the ov5670 I see three configurations using 1296x972, and\n> for the ov13858 I see nine configurations with smaller sensor\n> resolutions.\n\nThanks for checking, I grepped on the xml file, but visual inspection\nrevealed that I was not doing it right\n\n \n\nNow I see this too\n\n>\n> > so I suspect that's how\n> > the system is intenteded to work (even if, I wonder, what happens if I\n> > connect a sensor that can only produce the above failing 2112x1188\n> > resolution ?)\n> >\n> > I -fear- for IPU3 we should always go full frame size to make the pipe\n> > config happy, or there's maybe a requirement to respect some aspect\n> > ratio I still haven't been able to figure out. In any case, this would\n> > hinder the ability to capture raw+YUV, as the raw frame resolution in\n> > that case is selected from the application, and it might prevent the\n> > pipeline from being able to calculate a configuration.\n> >\n> > Not nice, I know.\n> >\n> > If that's how thing have to work, a software scaler for raw frames\n> > would nee to be plugged to the pipeline to down-scale raw frames to\n> > the desired resolution. Or we restrict RAW streams to the full size\n> > only. What do you think ?\n>\n> No software scaler, no. If a configuration isn't supported by the\n> hardware we can obviously reject it. For instance if the application\n> requests raw in 640x480 and wants YUV in a larger resolution, as the\n> ImgU can't downscale, we can't accept that, and we can go for a higher\n> raw resolution or a small YUV resolution. There's nothing new under the\n> sun here, and I'm OK with temporarily restricting the raw frames to the\n> full sensor resolution if it helps moving forward, but let's keep in\n> mind we may have to fix that at some point in the future, so we\n> shouldn't hardcode too much.\n\nI see. I'll try to rework this with this in mind.\n\nThanks\n j\n\n>\n> > > > \tif (!cio2Configuration_.pixelFormat.isValid())\n> > > > \t\treturn Invalid;\n> > > >\n> > > > +\tLOG(IPU3, Debug) << \"CIO2 configuration: \" << cio2Configuration_.toString();\n> > > > +\n> > > > \t/*\n> > > > \t * Adjust the configurations if needed and assign streams while\n> > > > \t * iterating them.\n>\n> --\n> Regards,\n>\n> Laurent Pinchart","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 75824BD790\n\tfor ;\n\tFri, 10 Jul 2020 12:32:06 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 11819613CB;\n\tFri, 10 Jul 2020 14:32:06 +0200 (CEST)","from relay11.mail.gandi.net (relay11.mail.gandi.net\n\t[217.70.178.231])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 7F72C611BA\n\tfor ;\n\tFri, 10 Jul 2020 14:32:05 +0200 (CEST)","from uno.localdomain (host-95-245-128-189.retail.telecomitalia.it\n\t[95.245.128.189]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay11.mail.gandi.net (Postfix) with ESMTPSA id 2A7FF100008;\n\tFri, 10 Jul 2020 12:32:03 +0000 (UTC)"],"Date":"Fri, 10 Jul 2020 14:35:38 +0200","From":"Jacopo Mondi ","To":"Laurent Pinchart ","Message-ID":"<20200710123538.qwlysoj2ggowpgdp@uno.localdomain>","References":"<20200709084128.5316-1-jacopo@jmondi.org>\n\t<20200709084128.5316-13-jacopo@jmondi.org>\n\t<20200709134654.GO3875643@oden.dyn.berto.se>\n\t<20200710073853.476lehorlkfmnicr@uno.localdomain>\n\t<20200710120740.GN5964@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200710120740.GN5964@pendragon.ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v2 12/20] libcamera: ipu3: Always use\n\tthe maximum frame size","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","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]