[{"id":12401,"web_url":"https://patchwork.libcamera.org/comment/12401/","msgid":"<20200910102546.GB4095624@oden.dyn.berto.se>","date":"2020-09-10T10:25:46","subject":"Re: [libcamera-devel] [RFC] libcamera: ipu3: Always use sensor full\n\tframe 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-09-09 15:10:14 +0200, Jacopo Mondi wrote:\n> When calculating the pipeline configuration for the IPU3 platform,\n> libcamera tries to be smart and select the smaller sensor frame\n> resolution larger enough to accommodate the stream sizes\n> requested by the application.\n> \n> While this seems to make a lot of sense, in practice optimizing the\n> selected sensor resolution makes the pipeline configuration calculation\n> process fail in multiple occasions, or results in stalls in the capture\n> process.\n> \n> As a trivial example, capturing with cam with the following command\n> line result in a stall:\n> $ cam -swidth=1280,height=720 -swidth=640,height=480 -c1 -C\n> \n> Likewise, the Android HAL supported format enumeration fails in\n> reporting smaller resolutions as supported, when used with the OV5670\n> sensor.\n> \n> 320x240:\n> DEBUG IPU3 ipu3.cpp:192 CIO2 configuration: 648x486-SGRBG10_IPU3\n> ERROR IPU3 imgu.cpp:408 Failed to calculate pipe configuration\n> ERROR IPU3 ipu3.cpp:299 Failed to calculate pipe configuration: unsupported resolutions.\n> \n> 640x480:\n> DEBUG IPU3 ipu3.cpp:192 CIO2 configuration: 648x486-SGRBG10_IPU3\n> ERROR IPU3 imgu.cpp:408 Failed to calculate pipe configuration\n> ERROR IPU3 ipu3.cpp:299 Failed to calculate pipe configuration: unsupported resolutions.\n> \n> Furthermore the reference .xml files used for the IPU3 camera\n> configuration on the ChromeOS platform restrict the number of sensor\n> resolution to be used for the OV5670 sensor to 2 from the 6 supported by\n> the driver [1].\n> \n> The selection criteria of the correct CIO2 mode are not specified, and\n> for the time being, always use the sensor maximum resolution at the\n> expense of frame rate and bus bandwidth allows the pipeline to successfully\n> support smaller modes for the OV5670 sensor and solves pipeline stalls\n> when capturing with both sensors.\n> \n> [1] See the <sensor_modes> enumeration in:\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> \n> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>\n\nI love how the IPU3 configuration depends on a xml file ;-) Given that \nour current configuration depends on this and assumes the larger \nresolution I think this is the right move.\n\nReviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n\n> ---\n>  src/libcamera/pipeline/ipu3/ipu3.cpp | 9 +--------\n>  1 file changed, 1 insertion(+), 8 deletions(-)\n> \n> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> index 2d881fe28f98..488e9fff299e 100644\n> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> @@ -155,14 +155,12 @@ CameraConfiguration::Status IPU3CameraConfiguration::validate()\n>  \tunsigned int rawCount = 0;\n>  \tunsigned int yuvCount = 0;\n>  \tSize maxYuvSize;\n> -\tSize maxRawSize;\n>  \n>  \tfor (const StreamConfiguration &cfg : config_) {\n>  \t\tconst PixelFormatInfo &info = PixelFormatInfo::info(cfg.pixelFormat);\n>  \n>  \t\tif (info.colourEncoding == PixelFormatInfo::ColourEncodingRAW) {\n>  \t\t\trawCount++;\n> -\t\t\tmaxRawSize.expandTo(cfg.size);\n>  \t\t} else {\n>  \t\t\tyuvCount++;\n>  \t\t\tmaxYuvSize.expandTo(cfg.size);\n> @@ -174,18 +172,13 @@ CameraConfiguration::Status IPU3CameraConfiguration::validate()\n>  \t\treturn Invalid;\n>  \t}\n>  \n> -\tif (maxRawSize.isNull())\n> -\t\tmaxRawSize = maxYuvSize.alignedUpTo(IMGU_OUTPUT_WIDTH_MARGIN,\n> -\t\t\t\t\t\t    IMGU_OUTPUT_HEIGHT_MARGIN)\n> -\t\t\t\t       .boundedTo(data_->cio2_.sensor()->resolution());\n> -\n>  \t/*\n>  \t * Generate raw configuration from CIO2.\n>  \t *\n>  \t * The output YUV streams will be limited in size to the maximum\n>  \t * frame size requested for the RAW stream.\n>  \t */\n> -\tcio2Configuration_ = data_->cio2_.generateConfiguration(maxRawSize);\n> +\tcio2Configuration_ = data_->cio2_.generateConfiguration({});\n>  \tif (!cio2Configuration_.pixelFormat.isValid())\n>  \t\treturn Invalid;\n>  \n> -- \n> 2.28.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":"<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 08879C3B5B\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu, 10 Sep 2020 10:25:50 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 9367A62D04;\n\tThu, 10 Sep 2020 12:25:49 +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 AF5C462C43\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 10 Sep 2020 12:25:48 +0200 (CEST)","by mail-lj1-x230.google.com with SMTP id u4so7423131ljd.10\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 10 Sep 2020 03:25:48 -0700 (PDT)","from localhost (h-209-203.A463.priv.bahnhof.se. [155.4.209.203])\n\tby smtp.gmail.com with ESMTPSA id\n\tt5sm1450468ljg.111.2020.09.10.03.25.47\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tThu, 10 Sep 2020 03:25:47 -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=\"wIpJniu5\"; 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=SJE4mgVsNcOBoe6udccuXk5A9XTV2DCyRecRMlNWVS4=;\n\tb=wIpJniu52sBVKl1VJ+2JksGZJEZPgK63CKoqEcGjPzynL4HAVkxK8HHfr9/yIAM/lZ\n\t/1j9cmLLL6/CprkDId3ceZnrvm3hoBDmdMcaixdk/cuQMvWUnkd5NT+R/SZfbbczLiHG\n\tN+ppWRg3Wod/NXmIpuV18FYxwMuqt0lfhKwbvoysZJi+SrOElZIoX9Ls3Buci9H7H3ib\n\tF+dAu5kWRYSTh4g6URb4WHAxJUGrbV79aFATznQbOjEkZ9vXrqL1043+UXeDIqkurFcJ\n\tqKoIS4aVeKyucg5pR+J0OGK+RWXmYKDqh7GPbNoNBVSFkldcNoO0/30qsEIq09KiJa0m\n\tLY/Q==","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=SJE4mgVsNcOBoe6udccuXk5A9XTV2DCyRecRMlNWVS4=;\n\tb=oVOQXtvKSTLGK3qj873oAEiC7rg4Xh6G7e0h41dnZJOijDn+X2he5fa4Jn4z4eX2Yj\n\t6ZrjsLfwdc2YxekhLmyBPood0hx3L4t22/MxxICarN/vFa4HfnM5B0KilxnwxYSRonHN\n\twdkn6YNpdbwL66xyalDGlaYrZT2biSrLJ/oEGHMovsrFIZ4q+0Y7Cl2SPW8id12osjne\n\tI/hQYU6rUKXSenVC0FcxhpTHWuLeD4Vji5fQAdPg6IBuXgdIWP1azpSxtV0iJAqgkPBh\n\tzHAiooI/XelSqmh3zQ+ezPIxkgXPPnAAhB8p/hruL5vYb1oAtgYX5j9SyjV83fXOxln7\n\tT3rA==","X-Gm-Message-State":"AOAM530yBC75XXBG3Zx6v0fJdOcFOWSMnduE0/h646xKyn+TS1a3hV29\n\txeqFVI7OZoj2ByvcIqshwG2sE5nGw7DIKA==","X-Google-Smtp-Source":"ABdhPJytUAomoq9iQmRx6VOCqxy0l7+iq5M6r/Fru1gR7cVJQJJD8j/mImaVvLZxU0c9Kq7UHCBGSw==","X-Received":"by 2002:a2e:5357:: with SMTP id\n\tt23mr3946518ljd.394.1599733547766; \n\tThu, 10 Sep 2020 03:25:47 -0700 (PDT)","Date":"Thu, 10 Sep 2020 12:25:46 +0200","From":"Niklas =?iso-8859-1?q?S=F6derlund?= <niklas.soderlund@ragnatech.se>","To":"Jacopo Mondi <jacopo@jmondi.org>","Message-ID":"<20200910102546.GB4095624@oden.dyn.berto.se>","References":"<20200909131014.115092-1-jacopo@jmondi.org>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200909131014.115092-1-jacopo@jmondi.org>","Subject":"Re: [libcamera-devel] [RFC] libcamera: ipu3: Always use sensor full\n\tframe size","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","Content-Type":"text/plain; charset=\"iso-8859-1\"","Content-Transfer-Encoding":"quoted-printable","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":12460,"web_url":"https://patchwork.libcamera.org/comment/12460/","msgid":"<CAAFQd5CB7XKge9QfKr6cH3VnF7MXdZBiOdhTk+wmLCvQOw7p+w@mail.gmail.com>","date":"2020-09-11T18:48:52","subject":"Re: [libcamera-devel] [RFC] libcamera: ipu3: Always use sensor full\n\tframe size","submitter":{"id":9,"url":"https://patchwork.libcamera.org/api/people/9/","name":"Tomasz Figa","email":"tfiga@chromium.org"},"content":"On Thu, Sep 10, 2020 at 12:25 PM Niklas Söderlund\n<niklas.soderlund@ragnatech.se> wrote:\n>\n> Hi Jacopo,\n>\n> Thanks for your work.\n>\n> On 2020-09-09 15:10:14 +0200, Jacopo Mondi wrote:\n> > When calculating the pipeline configuration for the IPU3 platform,\n> > libcamera tries to be smart and select the smaller sensor frame\n> > resolution larger enough to accommodate the stream sizes\n> > requested by the application.\n> >\n> > While this seems to make a lot of sense, in practice optimizing the\n> > selected sensor resolution makes the pipeline configuration calculation\n> > process fail in multiple occasions, or results in stalls in the capture\n> > process.\n> >\n> > As a trivial example, capturing with cam with the following command\n> > line result in a stall:\n> > $ cam -swidth=1280,height=720 -swidth=640,height=480 -c1 -C\n> >\n> > Likewise, the Android HAL supported format enumeration fails in\n> > reporting smaller resolutions as supported, when used with the OV5670\n> > sensor.\n> >\n> > 320x240:\n> > DEBUG IPU3 ipu3.cpp:192 CIO2 configuration: 648x486-SGRBG10_IPU3\n> > ERROR IPU3 imgu.cpp:408 Failed to calculate pipe configuration\n> > ERROR IPU3 ipu3.cpp:299 Failed to calculate pipe configuration: unsupported resolutions.\n> >\n> > 640x480:\n> > DEBUG IPU3 ipu3.cpp:192 CIO2 configuration: 648x486-SGRBG10_IPU3\n> > ERROR IPU3 imgu.cpp:408 Failed to calculate pipe configuration\n> > ERROR IPU3 ipu3.cpp:299 Failed to calculate pipe configuration: unsupported resolutions.\n> >\n> > Furthermore the reference .xml files used for the IPU3 camera\n> > configuration on the ChromeOS platform restrict the number of sensor\n> > resolution to be used for the OV5670 sensor to 2 from the 6 supported by\n> > the driver [1].\n> >\n> > The selection criteria of the correct CIO2 mode are not specified, and\n> > for the time being, always use the sensor maximum resolution at the\n> > expense of frame rate and bus bandwidth allows the pipeline to successfully\n> > support smaller modes for the OV5670 sensor and solves pipeline stalls\n> > when capturing with both sensors.\n> >\n> > [1] See the <sensor_modes> enumeration in:\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> >\n> > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>\n>\n> I love how the IPU3 configuration depends on a xml file ;-)\n\nHowever it sounds, I believe some static configuration is unavoidable,\nespecially for software which is expected to be used in production and\nmaintain stable operating conditions. Linux has a lot of configuration\nas well, either compile-time or runtime (sysfs, proc) for the same\nreason.\n\nAs we noticed in another thread, about Android stream mapping, there\nare multiple configurations existing to achieve the same final\nparameters requested, even though the end result might not be exactly\nthe same. For example, scaling in one part of the pipeline or another\ncould have different effect on quality, with the choice between one or\nanother depending on business decisions of a particular integrator.\n\nSo as much as we want to avoid configuration files, I think that we\nactually have to start thinking of how to make it possible to use such\nconfiguration if one wants.\n\nBest regards,\nTomasz\n\n> Given that\n> our current configuration depends on this and assumes the larger\n> resolution I think this is the right move.\n>\n> Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n>\n> > ---\n> >  src/libcamera/pipeline/ipu3/ipu3.cpp | 9 +--------\n> >  1 file changed, 1 insertion(+), 8 deletions(-)\n> >\n> > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > index 2d881fe28f98..488e9fff299e 100644\n> > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > @@ -155,14 +155,12 @@ CameraConfiguration::Status IPU3CameraConfiguration::validate()\n> >       unsigned int rawCount = 0;\n> >       unsigned int yuvCount = 0;\n> >       Size maxYuvSize;\n> > -     Size maxRawSize;\n> >\n> >       for (const StreamConfiguration &cfg : config_) {\n> >               const PixelFormatInfo &info = PixelFormatInfo::info(cfg.pixelFormat);\n> >\n> >               if (info.colourEncoding == PixelFormatInfo::ColourEncodingRAW) {\n> >                       rawCount++;\n> > -                     maxRawSize.expandTo(cfg.size);\n> >               } else {\n> >                       yuvCount++;\n> >                       maxYuvSize.expandTo(cfg.size);\n> > @@ -174,18 +172,13 @@ CameraConfiguration::Status IPU3CameraConfiguration::validate()\n> >               return Invalid;\n> >       }\n> >\n> > -     if (maxRawSize.isNull())\n> > -             maxRawSize = maxYuvSize.alignedUpTo(IMGU_OUTPUT_WIDTH_MARGIN,\n> > -                                                 IMGU_OUTPUT_HEIGHT_MARGIN)\n> > -                                    .boundedTo(data_->cio2_.sensor()->resolution());\n> > -\n> >       /*\n> >        * Generate raw configuration from CIO2.\n> >        *\n> >        * The output YUV streams will be limited in size to the maximum\n> >        * frame size requested for the RAW stream.\n> >        */\n> > -     cio2Configuration_ = data_->cio2_.generateConfiguration(maxRawSize);\n> > +     cio2Configuration_ = data_->cio2_.generateConfiguration({});\n> >       if (!cio2Configuration_.pixelFormat.isValid())\n> >               return Invalid;\n> >\n> > --\n> > 2.28.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\n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","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 2F27BC3B5B\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 11 Sep 2020 18:49:29 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5C64D62D86;\n\tFri, 11 Sep 2020 20:49:28 +0200 (CEST)","from mail-ej1-x633.google.com (mail-ej1-x633.google.com\n\t[IPv6:2a00:1450:4864:20::633])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id D128860534\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 11 Sep 2020 20:49:26 +0200 (CEST)","by mail-ej1-x633.google.com with SMTP id q13so15057311ejo.9\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 11 Sep 2020 11:49:26 -0700 (PDT)","from mail-wm1-f52.google.com (mail-wm1-f52.google.com.\n\t[209.85.128.52]) by smtp.gmail.com with ESMTPSA id\n\ty9sm2331134edo.37.2020.09.11.11.49.25\n\tfor <libcamera-devel@lists.libcamera.org>\n\t(version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128);\n\tFri, 11 Sep 2020 11:49:25 -0700 (PDT)","by mail-wm1-f52.google.com with SMTP id l9so5704983wme.3\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 11 Sep 2020 11:49:25 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=chromium.org header.i=@chromium.org\n\theader.b=\"byTmNI/B\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org;\n\ts=google; \n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc:content-transfer-encoding;\n\tbh=kLcm8vbvCDLW1vq0CVo738udIRn9E2XqXhgiYqgoUK0=;\n\tb=byTmNI/BLFxGZxxQp4CSu5v8herrlPcuCOcefMEAKRW45nHuwrfnn82O532H3R4sdj\n\teFYxPnWRgcLBS8Fq5ytsZjCqcWjVr3pvRKsLdE5i+SolIJgBStmZinFevmUQFNdhqTP+\n\tJ60gHR/nXF/Oq5HWCpdptlZXmOvrYts+7it/E=","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc:content-transfer-encoding;\n\tbh=kLcm8vbvCDLW1vq0CVo738udIRn9E2XqXhgiYqgoUK0=;\n\tb=OOFgGq7OJizjnqlQv9JVpdoU3iYcmcgB9JyuuPYnQqHQ59f5KBAB2hhNwCajfcX+BN\n\tmLOIH+0Kcwm0S55gGuKXCIxZMxXdFxk/fGZlCM/nn8oAvtzDLTm4S7RVKgBwMSxD5FZQ\n\tVKKrYSjkYdeq5VQklLZ/4Lu1gHwkNfcF50jPgg5J01v+uhJSZvYdYtTpeGvVwHdXrg+B\n\tZDIew6G3GKWAPpGFWuOXvjels4CtJvnnM9gGjVAwC24FaDKFUtkArjYeuUCQ23+/KhnK\n\tHYTrIDFospOnw878moOLGhjOI1yHrmmfgAvLpIeXua9sUDlG6zrgOZe9I01B/lfef9bs\n\t09cA==","X-Gm-Message-State":"AOAM5331pn8MD0Z5yoTu1r2QOoNH8LeexXo3i2r0PKV32hZV6CSMXpox\n\tRkuTRxS8xagj/TQTgJIscLxS+qXTEByGsQ==","X-Google-Smtp-Source":"ABdhPJyB0NoPkHTwS5sa3bwEGsM38szEyy3NXyN9fLD3QCAPZxMzVuWTK9OcPz+mg0M/7+kSQaG13A==","X-Received":["by 2002:a17:906:780f:: with SMTP id\n\tu15mr3522767ejm.259.1599850165774; \n\tFri, 11 Sep 2020 11:49:25 -0700 (PDT)","by 2002:a7b:c24b:: with SMTP id\n\tb11mr3703931wmj.134.1599850164606; \n\tFri, 11 Sep 2020 11:49:24 -0700 (PDT)"],"MIME-Version":"1.0","References":"<20200909131014.115092-1-jacopo@jmondi.org>\n\t<20200910102546.GB4095624@oden.dyn.berto.se>","In-Reply-To":"<20200910102546.GB4095624@oden.dyn.berto.se>","From":"Tomasz Figa <tfiga@chromium.org>","Date":"Fri, 11 Sep 2020 20:48:52 +0200","X-Gmail-Original-Message-ID":"<CAAFQd5CB7XKge9QfKr6cH3VnF7MXdZBiOdhTk+wmLCvQOw7p+w@mail.gmail.com>","Message-ID":"<CAAFQd5CB7XKge9QfKr6cH3VnF7MXdZBiOdhTk+wmLCvQOw7p+w@mail.gmail.com>","To":"=?utf-8?q?Niklas_S=C3=B6derlund?= <niklas.soderlund@ragnatech.se>","Subject":"Re: [libcamera-devel] [RFC] libcamera: ipu3: Always use sensor full\n\tframe size","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 <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\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":12559,"web_url":"https://patchwork.libcamera.org/comment/12559/","msgid":"<20200917104655.pcjvstgid7ttl4p3@uno.localdomain>","date":"2020-09-17T10:46:55","subject":"Re: [libcamera-devel] [RFC] libcamera: ipu3: Always use sensor full\n\tframe size","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Tomasz,\n        +Laurent\n\nOn Fri, Sep 11, 2020 at 08:48:52PM +0200, Tomasz Figa wrote:\n> On Thu, Sep 10, 2020 at 12:25 PM Niklas Söderlund\n> <niklas.soderlund@ragnatech.se> wrote:\n> >\n> > Hi Jacopo,\n> >\n> > Thanks for your work.\n> >\n> > On 2020-09-09 15:10:14 +0200, Jacopo Mondi wrote:\n> > > When calculating the pipeline configuration for the IPU3 platform,\n> > > libcamera tries to be smart and select the smaller sensor frame\n> > > resolution larger enough to accommodate the stream sizes\n> > > requested by the application.\n> > >\n> > > While this seems to make a lot of sense, in practice optimizing the\n> > > selected sensor resolution makes the pipeline configuration calculation\n> > > process fail in multiple occasions, or results in stalls in the capture\n> > > process.\n> > >\n> > > As a trivial example, capturing with cam with the following command\n> > > line result in a stall:\n> > > $ cam -swidth=1280,height=720 -swidth=640,height=480 -c1 -C\n> > >\n> > > Likewise, the Android HAL supported format enumeration fails in\n> > > reporting smaller resolutions as supported, when used with the OV5670\n> > > sensor.\n> > >\n> > > 320x240:\n> > > DEBUG IPU3 ipu3.cpp:192 CIO2 configuration: 648x486-SGRBG10_IPU3\n> > > ERROR IPU3 imgu.cpp:408 Failed to calculate pipe configuration\n> > > ERROR IPU3 ipu3.cpp:299 Failed to calculate pipe configuration: unsupported resolutions.\n> > >\n> > > 640x480:\n> > > DEBUG IPU3 ipu3.cpp:192 CIO2 configuration: 648x486-SGRBG10_IPU3\n> > > ERROR IPU3 imgu.cpp:408 Failed to calculate pipe configuration\n> > > ERROR IPU3 ipu3.cpp:299 Failed to calculate pipe configuration: unsupported resolutions.\n> > >\n> > > Furthermore the reference .xml files used for the IPU3 camera\n> > > configuration on the ChromeOS platform restrict the number of sensor\n> > > resolution to be used for the OV5670 sensor to 2 from the 6 supported by\n> > > the driver [1].\n> > >\n> > > The selection criteria of the correct CIO2 mode are not specified, and\n> > > for the time being, always use the sensor maximum resolution at the\n> > > expense of frame rate and bus bandwidth allows the pipeline to successfully\n> > > support smaller modes for the OV5670 sensor and solves pipeline stalls\n> > > when capturing with both sensors.\n> > >\n> > > [1] See the <sensor_modes> enumeration in:\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> > >\n> > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>\n> >\n> > I love how the IPU3 configuration depends on a xml file ;-)\n>\n> However it sounds, I believe some static configuration is unavoidable,\n> especially for software which is expected to be used in production and\n> maintain stable operating conditions. Linux has a lot of configuration\n> as well, either compile-time or runtime (sysfs, proc) for the same\n> reason.\n>\n> As we noticed in another thread, about Android stream mapping, there\n> are multiple configurations existing to achieve the same final\n> parameters requested, even though the end result might not be exactly\n> the same. For example, scaling in one part of the pipeline or another\n> could have different effect on quality, with the choice between one or\n> another depending on business decisions of a particular integrator.\n>\n> So as much as we want to avoid configuration files, I think that we\n> actually have to start thinking of how to make it possible to use such\n> configuration if one wants.\n\nI, sadly, agree.\n\nIn the meantime for this platform, I'll push this patch (maybe in a\nslighter different form).\n\nThanks\n   j\n\n>\n> Best regards,\n> Tomasz\n>\n> > Given that\n> > our current configuration depends on this and assumes the larger\n> > resolution I think this is the right move.\n> >\n> > Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n> >\n> > > ---\n> > >  src/libcamera/pipeline/ipu3/ipu3.cpp | 9 +--------\n> > >  1 file changed, 1 insertion(+), 8 deletions(-)\n> > >\n> > > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > > index 2d881fe28f98..488e9fff299e 100644\n> > > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > > @@ -155,14 +155,12 @@ CameraConfiguration::Status IPU3CameraConfiguration::validate()\n> > >       unsigned int rawCount = 0;\n> > >       unsigned int yuvCount = 0;\n> > >       Size maxYuvSize;\n> > > -     Size maxRawSize;\n> > >\n> > >       for (const StreamConfiguration &cfg : config_) {\n> > >               const PixelFormatInfo &info = PixelFormatInfo::info(cfg.pixelFormat);\n> > >\n> > >               if (info.colourEncoding == PixelFormatInfo::ColourEncodingRAW) {\n> > >                       rawCount++;\n> > > -                     maxRawSize.expandTo(cfg.size);\n> > >               } else {\n> > >                       yuvCount++;\n> > >                       maxYuvSize.expandTo(cfg.size);\n> > > @@ -174,18 +172,13 @@ CameraConfiguration::Status IPU3CameraConfiguration::validate()\n> > >               return Invalid;\n> > >       }\n> > >\n> > > -     if (maxRawSize.isNull())\n> > > -             maxRawSize = maxYuvSize.alignedUpTo(IMGU_OUTPUT_WIDTH_MARGIN,\n> > > -                                                 IMGU_OUTPUT_HEIGHT_MARGIN)\n> > > -                                    .boundedTo(data_->cio2_.sensor()->resolution());\n> > > -\n> > >       /*\n> > >        * Generate raw configuration from CIO2.\n> > >        *\n> > >        * The output YUV streams will be limited in size to the maximum\n> > >        * frame size requested for the RAW stream.\n> > >        */\n> > > -     cio2Configuration_ = data_->cio2_.generateConfiguration(maxRawSize);\n> > > +     cio2Configuration_ = data_->cio2_.generateConfiguration({});\n> > >       if (!cio2Configuration_.pixelFormat.isValid())\n> > >               return Invalid;\n> > >\n> > > --\n> > > 2.28.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\n> > _______________________________________________\n> > libcamera-devel mailing list\n> > libcamera-devel@lists.libcamera.org\n> > https://lists.libcamera.org/listinfo/libcamera-devel","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 E634FC3B5B\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu, 17 Sep 2020 10:43:06 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 4DE9262F5B;\n\tThu, 17 Sep 2020 12:43:06 +0200 (CEST)","from relay7-d.mail.gandi.net (relay7-d.mail.gandi.net\n\t[217.70.183.200])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 4ADA86036A\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 17 Sep 2020 12:43:05 +0200 (CEST)","from uno.localdomain (2-224-242-101.ip172.fastwebnet.it\n\t[2.224.242.101]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay7-d.mail.gandi.net (Postfix) with ESMTPSA id 0B52A20007;\n\tThu, 17 Sep 2020 10:43:03 +0000 (UTC)"],"X-Originating-IP":"2.224.242.101","Date":"Thu, 17 Sep 2020 12:46:55 +0200","From":"Jacopo Mondi <jacopo@jmondi.org>","To":"Tomasz Figa <tfiga@chromium.org>,\n\tLaurent Pinchart <laurent.pinchart@ideasonboard.com>","Message-ID":"<20200917104655.pcjvstgid7ttl4p3@uno.localdomain>","References":"<20200909131014.115092-1-jacopo@jmondi.org>\n\t<20200910102546.GB4095624@oden.dyn.berto.se>\n\t<CAAFQd5CB7XKge9QfKr6cH3VnF7MXdZBiOdhTk+wmLCvQOw7p+w@mail.gmail.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<CAAFQd5CB7XKge9QfKr6cH3VnF7MXdZBiOdhTk+wmLCvQOw7p+w@mail.gmail.com>","Subject":"Re: [libcamera-devel] [RFC] libcamera: ipu3: Always use sensor full\n\tframe size","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 <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\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":12560,"web_url":"https://patchwork.libcamera.org/comment/12560/","msgid":"<CAAFQd5CqNmcLVHa8pYX=Y=zor_U1_-X5SAc772ix-9sM1D044A@mail.gmail.com>","date":"2020-09-17T11:00:53","subject":"Re: [libcamera-devel] [RFC] libcamera: ipu3: Always use sensor full\n\tframe size","submitter":{"id":9,"url":"https://patchwork.libcamera.org/api/people/9/","name":"Tomasz Figa","email":"tfiga@chromium.org"},"content":"On Thu, Sep 17, 2020 at 12:43 PM Jacopo Mondi <jacopo@jmondi.org> wrote:\n>\n> Hi Tomasz,\n>         +Laurent\n>\n> On Fri, Sep 11, 2020 at 08:48:52PM +0200, Tomasz Figa wrote:\n> > On Thu, Sep 10, 2020 at 12:25 PM Niklas Söderlund\n> > <niklas.soderlund@ragnatech.se> wrote:\n> > >\n> > > Hi Jacopo,\n> > >\n> > > Thanks for your work.\n> > >\n> > > On 2020-09-09 15:10:14 +0200, Jacopo Mondi wrote:\n> > > > When calculating the pipeline configuration for the IPU3 platform,\n> > > > libcamera tries to be smart and select the smaller sensor frame\n> > > > resolution larger enough to accommodate the stream sizes\n> > > > requested by the application.\n> > > >\n> > > > While this seems to make a lot of sense, in practice optimizing the\n> > > > selected sensor resolution makes the pipeline configuration calculation\n> > > > process fail in multiple occasions, or results in stalls in the capture\n> > > > process.\n> > > >\n> > > > As a trivial example, capturing with cam with the following command\n> > > > line result in a stall:\n> > > > $ cam -swidth=1280,height=720 -swidth=640,height=480 -c1 -C\n> > > >\n> > > > Likewise, the Android HAL supported format enumeration fails in\n> > > > reporting smaller resolutions as supported, when used with the OV5670\n> > > > sensor.\n> > > >\n> > > > 320x240:\n> > > > DEBUG IPU3 ipu3.cpp:192 CIO2 configuration: 648x486-SGRBG10_IPU3\n> > > > ERROR IPU3 imgu.cpp:408 Failed to calculate pipe configuration\n> > > > ERROR IPU3 ipu3.cpp:299 Failed to calculate pipe configuration: unsupported resolutions.\n> > > >\n> > > > 640x480:\n> > > > DEBUG IPU3 ipu3.cpp:192 CIO2 configuration: 648x486-SGRBG10_IPU3\n> > > > ERROR IPU3 imgu.cpp:408 Failed to calculate pipe configuration\n> > > > ERROR IPU3 ipu3.cpp:299 Failed to calculate pipe configuration: unsupported resolutions.\n> > > >\n> > > > Furthermore the reference .xml files used for the IPU3 camera\n> > > > configuration on the ChromeOS platform restrict the number of sensor\n> > > > resolution to be used for the OV5670 sensor to 2 from the 6 supported by\n> > > > the driver [1].\n> > > >\n> > > > The selection criteria of the correct CIO2 mode are not specified, and\n> > > > for the time being, always use the sensor maximum resolution at the\n> > > > expense of frame rate and bus bandwidth allows the pipeline to successfully\n> > > > support smaller modes for the OV5670 sensor and solves pipeline stalls\n> > > > when capturing with both sensors.\n> > > >\n> > > > [1] See the <sensor_modes> enumeration in:\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> > > >\n> > > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>\n> > >\n> > > I love how the IPU3 configuration depends on a xml file ;-)\n> >\n> > However it sounds, I believe some static configuration is unavoidable,\n> > especially for software which is expected to be used in production and\n> > maintain stable operating conditions. Linux has a lot of configuration\n> > as well, either compile-time or runtime (sysfs, proc) for the same\n> > reason.\n> >\n> > As we noticed in another thread, about Android stream mapping, there\n> > are multiple configurations existing to achieve the same final\n> > parameters requested, even though the end result might not be exactly\n> > the same. For example, scaling in one part of the pipeline or another\n> > could have different effect on quality, with the choice between one or\n> > another depending on business decisions of a particular integrator.\n> >\n> > So as much as we want to avoid configuration files, I think that we\n> > actually have to start thinking of how to make it possible to use such\n> > configuration if one wants.\n>\n> I, sadly, agree.\n>\n> In the meantime for this platform, I'll push this patch (maybe in a\n> slighter different form).\n\nYeah, the patch itself makes sense, given that the other modes are\ncurrently broken.\n\nHowever, the binned mode is important for power and performance\nreasons in use cases like video conferencing, so we should look at\nbringing some of the modes back as a relatively high priority task.\n\n>\n> Thanks\n>    j\n>\n> >\n> > Best regards,\n> > Tomasz\n> >\n> > > Given that\n> > > our current configuration depends on this and assumes the larger\n> > > resolution I think this is the right move.\n> > >\n> > > Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n> > >\n> > > > ---\n> > > >  src/libcamera/pipeline/ipu3/ipu3.cpp | 9 +--------\n> > > >  1 file changed, 1 insertion(+), 8 deletions(-)\n> > > >\n> > > > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > > > index 2d881fe28f98..488e9fff299e 100644\n> > > > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > > > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > > > @@ -155,14 +155,12 @@ CameraConfiguration::Status IPU3CameraConfiguration::validate()\n> > > >       unsigned int rawCount = 0;\n> > > >       unsigned int yuvCount = 0;\n> > > >       Size maxYuvSize;\n> > > > -     Size maxRawSize;\n> > > >\n> > > >       for (const StreamConfiguration &cfg : config_) {\n> > > >               const PixelFormatInfo &info = PixelFormatInfo::info(cfg.pixelFormat);\n> > > >\n> > > >               if (info.colourEncoding == PixelFormatInfo::ColourEncodingRAW) {\n> > > >                       rawCount++;\n> > > > -                     maxRawSize.expandTo(cfg.size);\n> > > >               } else {\n> > > >                       yuvCount++;\n> > > >                       maxYuvSize.expandTo(cfg.size);\n> > > > @@ -174,18 +172,13 @@ CameraConfiguration::Status IPU3CameraConfiguration::validate()\n> > > >               return Invalid;\n> > > >       }\n> > > >\n> > > > -     if (maxRawSize.isNull())\n> > > > -             maxRawSize = maxYuvSize.alignedUpTo(IMGU_OUTPUT_WIDTH_MARGIN,\n> > > > -                                                 IMGU_OUTPUT_HEIGHT_MARGIN)\n> > > > -                                    .boundedTo(data_->cio2_.sensor()->resolution());\n> > > > -\n> > > >       /*\n> > > >        * Generate raw configuration from CIO2.\n> > > >        *\n> > > >        * The output YUV streams will be limited in size to the maximum\n> > > >        * frame size requested for the RAW stream.\n> > > >        */\n> > > > -     cio2Configuration_ = data_->cio2_.generateConfiguration(maxRawSize);\n> > > > +     cio2Configuration_ = data_->cio2_.generateConfiguration({});\n> > > >       if (!cio2Configuration_.pixelFormat.isValid())\n> > > >               return Invalid;\n> > > >\n> > > > --\n> > > > 2.28.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\n> > > _______________________________________________\n> > > libcamera-devel mailing list\n> > > libcamera-devel@lists.libcamera.org\n> > > https://lists.libcamera.org/listinfo/libcamera-devel","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 A658BBF01C\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu, 17 Sep 2020 11:01:18 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 43EF46036A;\n\tThu, 17 Sep 2020 13:01:18 +0200 (CEST)","from mail-ed1-x536.google.com (mail-ed1-x536.google.com\n\t[IPv6:2a00:1450:4864:20::536])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id A0DD26036A\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 17 Sep 2020 13:01:16 +0200 (CEST)","by mail-ed1-x536.google.com with SMTP id c8so2064595edv.5\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 17 Sep 2020 04:01:16 -0700 (PDT)","from mail-wr1-f48.google.com (mail-wr1-f48.google.com.\n\t[209.85.221.48]) by smtp.gmail.com with ESMTPSA id\n\tbc18sm15938202edb.66.2020.09.17.04.01.14\n\tfor <libcamera-devel@lists.libcamera.org>\n\t(version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128);\n\tThu, 17 Sep 2020 04:01:15 -0700 (PDT)","by mail-wr1-f48.google.com with SMTP id m6so1600614wrn.0\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 17 Sep 2020 04:01:14 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=chromium.org header.i=@chromium.org\n\theader.b=\"k4ZSKx7+\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org;\n\ts=google; \n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc:content-transfer-encoding;\n\tbh=mwZwb7hbuBzHkOsaoJA+KTqPlZfuIi0gkMIKTC62TsM=;\n\tb=k4ZSKx7+0vK5YXEbMkCsk78J1owGiZJeXmwk/73TR7s7NAxUD9+6aSaaQC15xCsPCZ\n\tUgA2uXS5mARuRj68tnzi6xNT99yOugAz0mTUFcHx5Qu++s2XUkPmxEGm7vkAFy6fSppa\n\tAs0rtQo47te6IWNAENs26LlGNaHd4+Kz4wI/4=","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc:content-transfer-encoding;\n\tbh=mwZwb7hbuBzHkOsaoJA+KTqPlZfuIi0gkMIKTC62TsM=;\n\tb=Ss0d5hShz15cTiWPygKRv1CX5jIy3S8WdCHeCRDG9l8NaKf2EpvauqltVu+1K2g2Ck\n\tfZFZwF6eXgRWKn4tyiR8smSmtnD0mm40cWOlj7YBUYCSp5vpezeOc1hB6zLaQKrGRYFO\n\tb7rZyAJ2KkkswT6w+B6CHhdNdGJ9bRFX2Jy5x8IiP2r6bh6QfbHyCg2wCiymVvP8pszh\n\tURhZhcSzoSkEzBDpS/IN9DdIw/hKozgTQEnPcgm/N1pbwsWdI5n9Aea7qgI0FejdRe8C\n\t/T+tq2VJeleL6YSk7GgTRiP1+xIGR6dWMpczJoz2aO/Mzke72uE+Ddu1yxdafxPWq4MZ\n\tg99Q==","X-Gm-Message-State":"AOAM532BO1cmUrYhDyGE6qTqnE+IxfylLZiUvV/eW2+ID/l3oLUJu19g\n\tO626NBcgcKXnWo2WPgw/2vIxI4VDM/rIYQ==","X-Google-Smtp-Source":"ABdhPJz5te0SwD5pQfpYJwrFHv4NoFy/KDwmbPjMNelCAi8sRaDUWy72bMwDEApRdzOeJlCqyKJyOg==","X-Received":["by 2002:aa7:dd8d:: with SMTP id\n\tg13mr32978351edv.324.1600340475750; \n\tThu, 17 Sep 2020 04:01:15 -0700 (PDT)","by 2002:adf:a29a:: with SMTP id\n\ts26mr27365303wra.197.1600340474378; \n\tThu, 17 Sep 2020 04:01:14 -0700 (PDT)"],"MIME-Version":"1.0","References":"<20200909131014.115092-1-jacopo@jmondi.org>\n\t<20200910102546.GB4095624@oden.dyn.berto.se>\n\t<CAAFQd5CB7XKge9QfKr6cH3VnF7MXdZBiOdhTk+wmLCvQOw7p+w@mail.gmail.com>\n\t<20200917104655.pcjvstgid7ttl4p3@uno.localdomain>","In-Reply-To":"<20200917104655.pcjvstgid7ttl4p3@uno.localdomain>","From":"Tomasz Figa <tfiga@chromium.org>","Date":"Thu, 17 Sep 2020 13:00:53 +0200","X-Gmail-Original-Message-ID":"<CAAFQd5CqNmcLVHa8pYX=Y=zor_U1_-X5SAc772ix-9sM1D044A@mail.gmail.com>","Message-ID":"<CAAFQd5CqNmcLVHa8pYX=Y=zor_U1_-X5SAc772ix-9sM1D044A@mail.gmail.com>","To":"Jacopo Mondi <jacopo@jmondi.org>","Subject":"Re: [libcamera-devel] [RFC] libcamera: ipu3: Always use sensor full\n\tframe size","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 <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\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":12563,"web_url":"https://patchwork.libcamera.org/comment/12563/","msgid":"<20200918024913.GF589@pendragon.ideasonboard.com>","date":"2020-09-18T02:49:13","subject":"Re: [libcamera-devel] [RFC] libcamera: ipu3: Always use sensor full\n\tframe size","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Tomasz,\n\nOn Fri, Sep 11, 2020 at 08:48:52PM +0200, Tomasz Figa wrote:\n> On Thu, Sep 10, 2020 at 12:25 PM Niklas Söderlund wrote:\n> > On 2020-09-09 15:10:14 +0200, Jacopo Mondi wrote:\n> > > When calculating the pipeline configuration for the IPU3 platform,\n> > > libcamera tries to be smart and select the smaller sensor frame\n> > > resolution larger enough to accommodate the stream sizes\n> > > requested by the application.\n> > >\n> > > While this seems to make a lot of sense, in practice optimizing the\n> > > selected sensor resolution makes the pipeline configuration calculation\n> > > process fail in multiple occasions, or results in stalls in the capture\n> > > process.\n> > >\n> > > As a trivial example, capturing with cam with the following command\n> > > line result in a stall:\n> > > $ cam -swidth=1280,height=720 -swidth=640,height=480 -c1 -C\n> > >\n> > > Likewise, the Android HAL supported format enumeration fails in\n> > > reporting smaller resolutions as supported, when used with the OV5670\n> > > sensor.\n> > >\n> > > 320x240:\n> > > DEBUG IPU3 ipu3.cpp:192 CIO2 configuration: 648x486-SGRBG10_IPU3\n> > > ERROR IPU3 imgu.cpp:408 Failed to calculate pipe configuration\n> > > ERROR IPU3 ipu3.cpp:299 Failed to calculate pipe configuration: unsupported resolutions.\n> > >\n> > > 640x480:\n> > > DEBUG IPU3 ipu3.cpp:192 CIO2 configuration: 648x486-SGRBG10_IPU3\n> > > ERROR IPU3 imgu.cpp:408 Failed to calculate pipe configuration\n> > > ERROR IPU3 ipu3.cpp:299 Failed to calculate pipe configuration: unsupported resolutions.\n> > >\n> > > Furthermore the reference .xml files used for the IPU3 camera\n> > > configuration on the ChromeOS platform restrict the number of sensor\n> > > resolution to be used for the OV5670 sensor to 2 from the 6 supported by\n> > > the driver [1].\n> > >\n> > > The selection criteria of the correct CIO2 mode are not specified, and\n> > > for the time being, always use the sensor maximum resolution at the\n> > > expense of frame rate and bus bandwidth allows the pipeline to successfully\n> > > support smaller modes for the OV5670 sensor and solves pipeline stalls\n> > > when capturing with both sensors.\n> > >\n> > > [1] See the <sensor_modes> enumeration in:\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> > >\n> > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>\n> >\n> > I love how the IPU3 configuration depends on a xml file ;-)\n> \n> However it sounds, I believe some static configuration is unavoidable,\n> especially for software which is expected to be used in production and\n> maintain stable operating conditions. Linux has a lot of configuration\n> as well, either compile-time or runtime (sysfs, proc) for the same\n> reason.\n> \n> As we noticed in another thread, about Android stream mapping, there\n> are multiple configurations existing to achieve the same final\n> parameters requested, even though the end result might not be exactly\n> the same. For example, scaling in one part of the pipeline or another\n> could have different effect on quality, with the choice between one or\n> another depending on business decisions of a particular integrator.\n> \n> So as much as we want to avoid configuration files, I think that we\n> actually have to start thinking of how to make it possible to use such\n> configuration if one wants.\n\nRegardless of how we will allow overriding pipeline configuration as\npart of the integration process, we need a heuristic that works\nreasonably well. This patch, which I think is fine as a short term\nworkaround, outlines shortcomings in the configuration of the ImgU. As\nJacopo figured out, either we're not using the python code provided by\nIntel correctly, or the code fails in some cases. This is something that\nhas to be sorted out before the ImgU driver can get out of staging.\n\n> > Given that\n> > our current configuration depends on this and assumes the larger\n> > resolution I think this is the right move.\n> >\n> > Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n> >\n> > > ---\n> > >  src/libcamera/pipeline/ipu3/ipu3.cpp | 9 +--------\n> > >  1 file changed, 1 insertion(+), 8 deletions(-)\n> > >\n> > > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > > index 2d881fe28f98..488e9fff299e 100644\n> > > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > > @@ -155,14 +155,12 @@ CameraConfiguration::Status IPU3CameraConfiguration::validate()\n> > >       unsigned int rawCount = 0;\n> > >       unsigned int yuvCount = 0;\n> > >       Size maxYuvSize;\n> > > -     Size maxRawSize;\n> > >\n> > >       for (const StreamConfiguration &cfg : config_) {\n> > >               const PixelFormatInfo &info = PixelFormatInfo::info(cfg.pixelFormat);\n> > >\n> > >               if (info.colourEncoding == PixelFormatInfo::ColourEncodingRAW) {\n> > >                       rawCount++;\n> > > -                     maxRawSize.expandTo(cfg.size);\n> > >               } else {\n> > >                       yuvCount++;\n> > >                       maxYuvSize.expandTo(cfg.size);\n> > > @@ -174,18 +172,13 @@ CameraConfiguration::Status IPU3CameraConfiguration::validate()\n> > >               return Invalid;\n> > >       }\n> > >\n> > > -     if (maxRawSize.isNull())\n> > > -             maxRawSize = maxYuvSize.alignedUpTo(IMGU_OUTPUT_WIDTH_MARGIN,\n> > > -                                                 IMGU_OUTPUT_HEIGHT_MARGIN)\n> > > -                                    .boundedTo(data_->cio2_.sensor()->resolution());\n> > > -\n> > >       /*\n> > >        * Generate raw configuration from CIO2.\n> > >        *\n> > >        * The output YUV streams will be limited in size to the maximum\n> > >        * frame size requested for the RAW stream.\n> > >        */\n> > > -     cio2Configuration_ = data_->cio2_.generateConfiguration(maxRawSize);\n> > > +     cio2Configuration_ = data_->cio2_.generateConfiguration({});\n> > >       if (!cio2Configuration_.pixelFormat.isValid())\n> > >               return Invalid;\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 DB760BF01C\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 18 Sep 2020 02:49:46 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 336D262FAE;\n\tFri, 18 Sep 2020 04:49:46 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 4CABD60380\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 18 Sep 2020 04:49:44 +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 BBB412D7;\n\tFri, 18 Sep 2020 04:49:43 +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=\"qvtvvonU\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1600397383;\n\tbh=LFsHh0/08tsqksRLbwjXNo6ZR8uPetSC/arMD0HSISs=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=qvtvvonUdF1hy3I6tads4v0y7JB08fSVEQ1EcNdKZyPo9uINkqCl/Sx20RSRGy0fm\n\ttv4yAazjjWTblRfFzKX0+tiWgZd8RbooHF6Pr7xR2M00qr2m8SYkYW5VzyyfLQjpbJ\n\tp+hJSY/4EoQp2FIpX7kGqS6Vj639SITf59vnvWS0=","Date":"Fri, 18 Sep 2020 05:49:13 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Tomasz Figa <tfiga@chromium.org>","Message-ID":"<20200918024913.GF589@pendragon.ideasonboard.com>","References":"<20200909131014.115092-1-jacopo@jmondi.org>\n\t<20200910102546.GB4095624@oden.dyn.berto.se>\n\t<CAAFQd5CB7XKge9QfKr6cH3VnF7MXdZBiOdhTk+wmLCvQOw7p+w@mail.gmail.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<CAAFQd5CB7XKge9QfKr6cH3VnF7MXdZBiOdhTk+wmLCvQOw7p+w@mail.gmail.com>","Subject":"Re: [libcamera-devel] [RFC] libcamera: ipu3: Always use sensor full\n\tframe size","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 <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\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":12566,"web_url":"https://patchwork.libcamera.org/comment/12566/","msgid":"<20200918100551.2a7da2qhijlnpm62@uno.localdomain>","date":"2020-09-18T10:05:51","subject":"Re: [libcamera-devel] [RFC] libcamera: ipu3: Always use sensor full\n\tframe 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, Sep 18, 2020 at 05:49:13AM +0300, Laurent Pinchart wrote:\n> Hi Tomasz,\n>\n> On Fri, Sep 11, 2020 at 08:48:52PM +0200, Tomasz Figa wrote:\n> > On Thu, Sep 10, 2020 at 12:25 PM Niklas Söderlund wrote:\n> > > On 2020-09-09 15:10:14 +0200, Jacopo Mondi wrote:\n> > > > When calculating the pipeline configuration for the IPU3 platform,\n> > > > libcamera tries to be smart and select the smaller sensor frame\n> > > > resolution larger enough to accommodate the stream sizes\n> > > > requested by the application.\n> > > >\n> > > > While this seems to make a lot of sense, in practice optimizing the\n> > > > selected sensor resolution makes the pipeline configuration calculation\n> > > > process fail in multiple occasions, or results in stalls in the capture\n> > > > process.\n> > > >\n> > > > As a trivial example, capturing with cam with the following command\n> > > > line result in a stall:\n> > > > $ cam -swidth=1280,height=720 -swidth=640,height=480 -c1 -C\n> > > >\n> > > > Likewise, the Android HAL supported format enumeration fails in\n> > > > reporting smaller resolutions as supported, when used with the OV5670\n> > > > sensor.\n> > > >\n> > > > 320x240:\n> > > > DEBUG IPU3 ipu3.cpp:192 CIO2 configuration: 648x486-SGRBG10_IPU3\n> > > > ERROR IPU3 imgu.cpp:408 Failed to calculate pipe configuration\n> > > > ERROR IPU3 ipu3.cpp:299 Failed to calculate pipe configuration: unsupported resolutions.\n> > > >\n> > > > 640x480:\n> > > > DEBUG IPU3 ipu3.cpp:192 CIO2 configuration: 648x486-SGRBG10_IPU3\n> > > > ERROR IPU3 imgu.cpp:408 Failed to calculate pipe configuration\n> > > > ERROR IPU3 ipu3.cpp:299 Failed to calculate pipe configuration: unsupported resolutions.\n> > > >\n> > > > Furthermore the reference .xml files used for the IPU3 camera\n> > > > configuration on the ChromeOS platform restrict the number of sensor\n> > > > resolution to be used for the OV5670 sensor to 2 from the 6 supported by\n> > > > the driver [1].\n> > > >\n> > > > The selection criteria of the correct CIO2 mode are not specified, and\n> > > > for the time being, always use the sensor maximum resolution at the\n> > > > expense of frame rate and bus bandwidth allows the pipeline to successfully\n> > > > support smaller modes for the OV5670 sensor and solves pipeline stalls\n> > > > when capturing with both sensors.\n> > > >\n> > > > [1] See the <sensor_modes> enumeration in:\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> > > >\n> > > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>\n> > >\n> > > I love how the IPU3 configuration depends on a xml file ;-)\n> >\n> > However it sounds, I believe some static configuration is unavoidable,\n> > especially for software which is expected to be used in production and\n> > maintain stable operating conditions. Linux has a lot of configuration\n> > as well, either compile-time or runtime (sysfs, proc) for the same\n> > reason.\n> >\n> > As we noticed in another thread, about Android stream mapping, there\n> > are multiple configurations existing to achieve the same final\n> > parameters requested, even though the end result might not be exactly\n> > the same. For example, scaling in one part of the pipeline or another\n> > could have different effect on quality, with the choice between one or\n> > another depending on business decisions of a particular integrator.\n> >\n> > So as much as we want to avoid configuration files, I think that we\n> > actually have to start thinking of how to make it possible to use such\n> > configuration if one wants.\n>\n> Regardless of how we will allow overriding pipeline configuration as\n> part of the integration process, we need a heuristic that works\n> reasonably well. This patch, which I think is fine as a short term\n> workaround, outlines shortcomings in the configuration of the ImgU. As\n> Jacopo figured out, either we're not using the python code provided by\n> Intel correctly, or the code fails in some cases. This is something that\n> has to be sorted out before the ImgU driver can get out of staging.\n>\n\nFor the records:\n\n1) This issue is not much releated to the python tool configuration,\n(which expects input sizes from CIO2 to be pre-calculated) but more\nabout the sensor size selection criteria which are not specified\n\n2) The python script gives different results than the ones recorded in\nthe xml file and I've opened an un-resolved issue on github in June\nfor this:\nhttps://github.com/intel/intel-ipu3-pipecfg/issues/1\n\n3) Of course there can be glitches in our pipeline configuration\nroutine implementation, but the issue of stalling the pipeline when\nusing certain resolution seems not related to the ImgU configuration\nparameters calculation, or at least, our implementation gives the same\nresults as the python script, but using those parameters result in a\npipeline stall.\n\nIn example, as reported by the commit message, this command results in\na stall during capture operations (no frames are produced at the ImgU\noutputs):\n$ cam -swidth=1280,height=720 -swidth=640,height=480 -c1 -C\n\nIf I inspect our pipeline configuration routine results I get:\n\nDEBUG IPU3 imgu.cpp:386 Calculating pipe configuration for:\nDEBUG IPU3 imgu.cpp:387 input: 2112x1188\nDEBUG IPU3 imgu.cpp:388 main: 1280x720\nDEBUG IPU3 imgu.cpp:389 vf: 640x480\nDEBUG IPU3 imgu.cpp:425 Computed pipe configuration:\nDEBUG IPU3 imgu.cpp:426 IF: 2072x1192\nDEBUG IPU3 imgu.cpp:427 BDS: 1792x1027\nDEBUG IPU3 imgu.cpp:428 GDC: 1280x960\n\nThe same parameters fed to the python script give the same results for\nthe IF, BDS and GDC rectangles:\n\n$ python3 pipe_config.py input=2112x1188 main=1280x720 vf=640x480\n-------- The selected pipe configuration: --------------\noutput_res_if:2072x1192\noutput_res_bds:1792x1027\noutput_res_gdc:1280x960\n\nBut, as reported, this does not work. If I use the sensor full frame size as\nCIO2 input (4224x3136 for OV13858) I get frames and the pipeline works\nas expected\n\nDEBUG IPU3 imgu.cpp:386 Calculating pipe configuration for:\nDEBUG IPU3 imgu.cpp:387 input: 4224x3136\nDEBUG IPU3 imgu.cpp:388 main: 1280x720\nDEBUG IPU3 imgu.cpp:389 vf: 640x480\nDEBUG IPU3 imgu.cpp:425 Computed pipe configuration:\nDEBUG IPU3 imgu.cpp:426 IF: 4220x3120\nDEBUG IPU3 imgu.cpp:427 BDS: 1688x1248\nDEBUG IPU3 imgu.cpp:428 GDC: 1280x960\n\nWhich again matches the python script results\n\n python3 pipe_config.py input=4224x3136 main=1280x720 vf=640x480\n-------- The selected pipe configuration: --------------\noutput_res_if:4220x3120\noutput_res_bds:1688x1248\noutput_res_gdc:1280x960","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 D1704BF01C\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 18 Sep 2020 10:02:02 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6789362FAF;\n\tFri, 18 Sep 2020 12:02:02 +0200 (CEST)","from relay6-d.mail.gandi.net (relay6-d.mail.gandi.net\n\t[217.70.183.198])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id DC59862E65\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 18 Sep 2020 12:02:00 +0200 (CEST)","from uno.localdomain (93-34-118-233.ip49.fastwebnet.it\n\t[93.34.118.233]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay6-d.mail.gandi.net (Postfix) with ESMTPSA id 2E60EC01D1;\n\tFri, 18 Sep 2020 10:01:59 +0000 (UTC)"],"X-Originating-IP":"93.34.118.233","Date":"Fri, 18 Sep 2020 12:05:51 +0200","From":"Jacopo Mondi <jacopo@jmondi.org>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Message-ID":"<20200918100551.2a7da2qhijlnpm62@uno.localdomain>","References":"<20200909131014.115092-1-jacopo@jmondi.org>\n\t<20200910102546.GB4095624@oden.dyn.berto.se>\n\t<CAAFQd5CB7XKge9QfKr6cH3VnF7MXdZBiOdhTk+wmLCvQOw7p+w@mail.gmail.com>\n\t<20200918024913.GF589@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200918024913.GF589@pendragon.ideasonboard.com>","Subject":"Re: [libcamera-devel] [RFC] libcamera: ipu3: Always use sensor full\n\tframe size","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 <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\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":12575,"web_url":"https://patchwork.libcamera.org/comment/12575/","msgid":"<20200918140542.GD28436@pendragon.ideasonboard.com>","date":"2020-09-18T14:05:42","subject":"Re: [libcamera-devel] [RFC] libcamera: ipu3: Always use sensor full\n\tframe size","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nOn Fri, Sep 18, 2020 at 12:05:51PM +0200, Jacopo Mondi wrote:\n> On Fri, Sep 18, 2020 at 05:49:13AM +0300, Laurent Pinchart wrote:\n> > On Fri, Sep 11, 2020 at 08:48:52PM +0200, Tomasz Figa wrote:\n> > > On Thu, Sep 10, 2020 at 12:25 PM Niklas Söderlund wrote:\n> > > > On 2020-09-09 15:10:14 +0200, Jacopo Mondi wrote:\n> > > > > When calculating the pipeline configuration for the IPU3 platform,\n> > > > > libcamera tries to be smart and select the smaller sensor frame\n> > > > > resolution larger enough to accommodate the stream sizes\n> > > > > requested by the application.\n> > > > >\n> > > > > While this seems to make a lot of sense, in practice optimizing the\n> > > > > selected sensor resolution makes the pipeline configuration calculation\n> > > > > process fail in multiple occasions, or results in stalls in the capture\n> > > > > process.\n> > > > >\n> > > > > As a trivial example, capturing with cam with the following command\n> > > > > line result in a stall:\n> > > > > $ cam -swidth=1280,height=720 -swidth=640,height=480 -c1 -C\n> > > > >\n> > > > > Likewise, the Android HAL supported format enumeration fails in\n> > > > > reporting smaller resolutions as supported, when used with the OV5670\n> > > > > sensor.\n> > > > >\n> > > > > 320x240:\n> > > > > DEBUG IPU3 ipu3.cpp:192 CIO2 configuration: 648x486-SGRBG10_IPU3\n> > > > > ERROR IPU3 imgu.cpp:408 Failed to calculate pipe configuration\n> > > > > ERROR IPU3 ipu3.cpp:299 Failed to calculate pipe configuration: unsupported resolutions.\n> > > > >\n> > > > > 640x480:\n> > > > > DEBUG IPU3 ipu3.cpp:192 CIO2 configuration: 648x486-SGRBG10_IPU3\n> > > > > ERROR IPU3 imgu.cpp:408 Failed to calculate pipe configuration\n> > > > > ERROR IPU3 ipu3.cpp:299 Failed to calculate pipe configuration: unsupported resolutions.\n> > > > >\n> > > > > Furthermore the reference .xml files used for the IPU3 camera\n> > > > > configuration on the ChromeOS platform restrict the number of sensor\n> > > > > resolution to be used for the OV5670 sensor to 2 from the 6 supported by\n> > > > > the driver [1].\n> > > > >\n> > > > > The selection criteria of the correct CIO2 mode are not specified, and\n> > > > > for the time being, always use the sensor maximum resolution at the\n> > > > > expense of frame rate and bus bandwidth allows the pipeline to successfully\n> > > > > support smaller modes for the OV5670 sensor and solves pipeline stalls\n> > > > > when capturing with both sensors.\n> > > > >\n> > > > > [1] See the <sensor_modes> enumeration in:\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> > > > >\n> > > > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>\n> > > >\n> > > > I love how the IPU3 configuration depends on a xml file ;-)\n> > >\n> > > However it sounds, I believe some static configuration is unavoidable,\n> > > especially for software which is expected to be used in production and\n> > > maintain stable operating conditions. Linux has a lot of configuration\n> > > as well, either compile-time or runtime (sysfs, proc) for the same\n> > > reason.\n> > >\n> > > As we noticed in another thread, about Android stream mapping, there\n> > > are multiple configurations existing to achieve the same final\n> > > parameters requested, even though the end result might not be exactly\n> > > the same. For example, scaling in one part of the pipeline or another\n> > > could have different effect on quality, with the choice between one or\n> > > another depending on business decisions of a particular integrator.\n> > >\n> > > So as much as we want to avoid configuration files, I think that we\n> > > actually have to start thinking of how to make it possible to use such\n> > > configuration if one wants.\n> >\n> > Regardless of how we will allow overriding pipeline configuration as\n> > part of the integration process, we need a heuristic that works\n> > reasonably well. This patch, which I think is fine as a short term\n> > workaround, outlines shortcomings in the configuration of the ImgU. As\n> > Jacopo figured out, either we're not using the python code provided by\n> > Intel correctly, or the code fails in some cases. This is something that\n> > has to be sorted out before the ImgU driver can get out of staging.\n> \n> For the records:\n\nThanks for the summary, that will be helpful when we'll get back to this\nin the future, and try to recall what the problem was.\n\n> 1) This issue is not much releated to the python tool configuration,\n> (which expects input sizes from CIO2 to be pre-calculated) but more\n> about the sensor size selection criteria which are not specified\n> \n> 2) The python script gives different results than the ones recorded in\n> the xml file and I've opened an un-resolved issue on github in June\n> for this:\n> https://github.com/intel/intel-ipu3-pipecfg/issues/1\n> \n> 3) Of course there can be glitches in our pipeline configuration\n> routine implementation, but the issue of stalling the pipeline when\n> using certain resolution seems not related to the ImgU configuration\n> parameters calculation, or at least, our implementation gives the same\n> results as the python script, but using those parameters result in a\n> pipeline stall.\n> \n> In example, as reported by the commit message, this command results in\n> a stall during capture operations (no frames are produced at the ImgU\n> outputs):\n> $ cam -swidth=1280,height=720 -swidth=640,height=480 -c1 -C\n> \n> If I inspect our pipeline configuration routine results I get:\n> \n> DEBUG IPU3 imgu.cpp:386 Calculating pipe configuration for:\n> DEBUG IPU3 imgu.cpp:387 input: 2112x1188\n> DEBUG IPU3 imgu.cpp:388 main: 1280x720\n> DEBUG IPU3 imgu.cpp:389 vf: 640x480\n> DEBUG IPU3 imgu.cpp:425 Computed pipe configuration:\n> DEBUG IPU3 imgu.cpp:426 IF: 2072x1192\n> DEBUG IPU3 imgu.cpp:427 BDS: 1792x1027\n> DEBUG IPU3 imgu.cpp:428 GDC: 1280x960\n> \n> The same parameters fed to the python script give the same results for\n> the IF, BDS and GDC rectangles:\n> \n> $ python3 pipe_config.py input=2112x1188 main=1280x720 vf=640x480\n> -------- The selected pipe configuration: --------------\n> output_res_if:2072x1192\n> output_res_bds:1792x1027\n> output_res_gdc:1280x960\n> \n> But, as reported, this does not work. If I use the sensor full frame size as\n> CIO2 input (4224x3136 for OV13858) I get frames and the pipeline works\n> as expected\n> \n> DEBUG IPU3 imgu.cpp:386 Calculating pipe configuration for:\n> DEBUG IPU3 imgu.cpp:387 input: 4224x3136\n> DEBUG IPU3 imgu.cpp:388 main: 1280x720\n> DEBUG IPU3 imgu.cpp:389 vf: 640x480\n> DEBUG IPU3 imgu.cpp:425 Computed pipe configuration:\n> DEBUG IPU3 imgu.cpp:426 IF: 4220x3120\n> DEBUG IPU3 imgu.cpp:427 BDS: 1688x1248\n> DEBUG IPU3 imgu.cpp:428 GDC: 1280x960\n> \n> Which again matches the python script results\n> \n>  python3 pipe_config.py input=4224x3136 main=1280x720 vf=640x480\n> -------- The selected pipe configuration: --------------\n> output_res_if:4220x3120\n> output_res_bds:1688x1248\n> output_res_gdc:1280x960\n\nI'd consider this as a driver bug, as the driver should reject invalid\nconfigurations, not stall. Of course, when the driver will be fixed, we\nstill won't have a functional system, because the configuration we\ncalculate for the ImgU is invalid :-)","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 ED9E6C3B5B\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 18 Sep 2020 14:06:15 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6A5C562FB5;\n\tFri, 18 Sep 2020 16:06:15 +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 996EC62F4F\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 18 Sep 2020 16:06:14 +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 113212D7;\n\tFri, 18 Sep 2020 16:06:14 +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=\"XVy15Hk3\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1600437974;\n\tbh=YT/RWKj+OoQKlmOPjg1yyaZXIZDym86H3MCf8f9/Puo=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=XVy15Hk336i+9Ab0z1x+Px48OjgIzvy1HcBawpn8UN+fs062lym+M9T8EUO6DwY4i\n\tJIVgabq9EDmSKPFmvjKCWYBoj0Om9UJ8V/oIkzBWJRHUIi9Wc7JdgaHEeAvyAqDJ8D\n\t9be3QHGgKEamkh+W+6kNiXTW6s89mFsJ1R63C4pk=","Date":"Fri, 18 Sep 2020 17:05:42 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Jacopo Mondi <jacopo@jmondi.org>","Message-ID":"<20200918140542.GD28436@pendragon.ideasonboard.com>","References":"<20200909131014.115092-1-jacopo@jmondi.org>\n\t<20200910102546.GB4095624@oden.dyn.berto.se>\n\t<CAAFQd5CB7XKge9QfKr6cH3VnF7MXdZBiOdhTk+wmLCvQOw7p+w@mail.gmail.com>\n\t<20200918024913.GF589@pendragon.ideasonboard.com>\n\t<20200918100551.2a7da2qhijlnpm62@uno.localdomain>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200918100551.2a7da2qhijlnpm62@uno.localdomain>","Subject":"Re: [libcamera-devel] [RFC] libcamera: ipu3: Always use sensor full\n\tframe size","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 <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\" <libcamera-devel-bounces@lists.libcamera.org>"}}]