[{"id":15125,"web_url":"https://patchwork.libcamera.org/comment/15125/","msgid":"","date":"2021-02-12T08:10:34","subject":"Re: [libcamera-devel] [PATCH v2 2/4] libcamera: camera_sensor: Set\n\tdefault sensor location to Unknown","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hello Paul,\n\nThanks for your work.\n\nOn 2021-02-12 14:48:14 +0900, Paul Elder wrote:\n> Instead of choosing some arbitrary location for the sensor when its\n> location is unknown, set it explicitly to unknown.\n> \n> Signed-off-by: Paul Elder \n> Reviewed-by: Kieran Bingham \n> Reviewed-by: Laurent Pinchart \n> \n> ---\n> No change in v2\n> ---\n> src/libcamera/camera_sensor.cpp | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n> \n> diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp\n> index c9e8d49b..474055ba 100644\n> --- a/src/libcamera/camera_sensor.cpp\n> +++ b/src/libcamera/camera_sensor.cpp\n> @@ -446,7 +446,7 @@ int CameraSensor::initProperties()\n> \t\t\tbreak;\n> \t\t}\n> \t} else {\n> -\t\tpropertyValue = properties::CameraLocationExternal;\n> +\t\tpropertyValue = properties::CameraLocationUnknown;\n\nI'm still unconvinced about the use-case for unknown :-)\n\nTo me it looks like we are pushing the problem of an incomplete firmware \ndescription on to applications. How should an application handle a \nexternal camera vs an unknown one?\n\nI would like to understand the advantage of adding a new location here \nother then just to pass a CTS test that as far as I understands it fails \nas the device it's running on have incomplete firmware. Is that not a \nvalid fail of CTS?\n\n> \t}\n> \tproperties_.set(properties::Location, propertyValue);\n> \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 0BDCCBD160\n\tfor ;\n\tFri, 12 Feb 2021 08:10:39 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7786463765;\n\tFri, 12 Feb 2021 09:10:38 +0100 (CET)","from mail-lj1-x235.google.com (mail-lj1-x235.google.com\n\t[IPv6:2a00:1450:4864:20::235])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 2D480613F3\n\tfor ;\n\tFri, 12 Feb 2021 09:10:37 +0100 (CET)","by mail-lj1-x235.google.com with SMTP id u4so10547166ljh.6\n\tfor ;\n\tFri, 12 Feb 2021 00:10:37 -0800 (PST)","from localhost (h-209-203.A463.priv.bahnhof.se. [155.4.209.203])\n\tby smtp.gmail.com with ESMTPSA id\n\ty27sm898573lfg.273.2021.02.12.00.10.35\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tFri, 12 Feb 2021 00:10:35 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=ragnatech-se.20150623.gappssmtp.com\n\theader.i=@ragnatech-se.20150623.gappssmtp.com\n\theader.b=\"ypCiCe2n\"; 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=6fmzFVG5qAmb8vj3unpkbm8TCa8MfJZiT7BNnMuurJE=;\n\tb=ypCiCe2nTLu7txGvHcraT0a/cvSVmnLFCIxogW04POUKiPYKiKJM0JfUTcjIeiVHx2\n\t1UbRrRn3kk2ruxJ3mhV7dEaQRfq7R3moM23XVNuZFQAAGnEch1QUNseOe5lZ4tY/TSoZ\n\t1CY9wgQMS8k8UkOz/BxOYFfTBF0jbpjQ1VbyF6Bhi8nsdfqn4PDMRUx5jbu+wCgnowPo\n\tzZJY765xfCAMsvUdZvC5HTf3fKiwt7fnvA9aiLd9Nzw5WAaYmYMdEw8FYRyTlJfEmVp1\n\t5qAbRgvRHmdc+mMfE7s6oBnh6Wm8u0sBBzsHtHp+SPX1phEeDEBukx6mhqwf1YFbF3Bp\n\tJHBg==","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=6fmzFVG5qAmb8vj3unpkbm8TCa8MfJZiT7BNnMuurJE=;\n\tb=pEnjul1DkemL05/ksjdXw6QSGaQuwFVIwgzd03CE8YSoEjjozgw81GxB6sCHmDC1HG\n\tsguvrYOBMcyoR74M/YDKMvnnXwc2mPd4ikWcmayu1ibhWRiMzKm/vCUdn5HwhSASwXds\n\ttYKnFeM6UCgRD5IQaPLO5Nsvm40QjLPywrApcVvqGk6qZ/dm5+cVv/+AAVtoAyNMk9QN\n\tg4XW+EfArcnbpRr9rBfapSve5ZdXcR7iR+sd/ANbGmYpAmZdCj6JVkcQU45lMJa1XhTo\n\tC4AawfBGzd7gTSYNObGMSyk1Q6O0Oo3NV1W+a1/0TZdMWZICTzR/pFptKOU8vN0DvBHU\n\t/Y+w==","X-Gm-Message-State":"AOAM5306ZysmGSXIozubDMSrGJ2fd4jSW5oLAiw+W+HwNzEAeL5prjBY\n\tvhMlwAcOFu49ST5RplP96NetMg==","X-Google-Smtp-Source":"ABdhPJxgpoKtegHiXPDNl0MDaY3Sli7CsJfPUyK+JePaU83j99IzOjBaLFKI8WpUMu9qftoSlfBqwA==","X-Received":"by 2002:a2e:8654:: with SMTP id\n\ti20mr1049673ljj.362.1613117436344; \n\tFri, 12 Feb 2021 00:10:36 -0800 (PST)","Date":"Fri, 12 Feb 2021 09:10:34 +0100","From":"Niklas =?iso-8859-1?q?S=F6derlund?= ","To":"Paul Elder ","Message-ID":"","References":"<20210212054816.53561-1-paul.elder@ideasonboard.com>\n\t<20210212054816.53561-3-paul.elder@ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20210212054816.53561-3-paul.elder@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v2 2/4] libcamera: camera_sensor: Set\n\tdefault sensor location to Unknown","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":15127,"web_url":"https://patchwork.libcamera.org/comment/15127/","msgid":"<20210212085445.7rby2vq2xon2edmy@uno.localdomain>","date":"2021-02-12T08:54:45","subject":"Re: [libcamera-devel] [PATCH v2 2/4] libcamera: camera_sensor: Set\n\tdefault sensor location to Unknown","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Niklas,\n\nOn Fri, Feb 12, 2021 at 09:10:34AM +0100, Niklas Söderlund wrote:\n> Hello Paul,\n>\n> Thanks for your work.\n>\n> On 2021-02-12 14:48:14 +0900, Paul Elder wrote:\n> > Instead of choosing some arbitrary location for the sensor when its\n> > location is unknown, set it explicitly to unknown.\n> >\n> > Signed-off-by: Paul Elder \n> > Reviewed-by: Kieran Bingham \n> > Reviewed-by: Laurent Pinchart \n> >\n> > ---\n> > No change in v2\n> > ---\n> > src/libcamera/camera_sensor.cpp | 2 +-\n> > 1 file changed, 1 insertion(+), 1 deletion(-)\n> >\n> > diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp\n> > index c9e8d49b..474055ba 100644\n> > --- a/src/libcamera/camera_sensor.cpp\n> > +++ b/src/libcamera/camera_sensor.cpp\n> > @@ -446,7 +446,7 @@ int CameraSensor::initProperties()\n> > \t\t\tbreak;\n> > \t\t}\n> > \t} else {\n> > -\t\tpropertyValue = properties::CameraLocationExternal;\n> > +\t\tpropertyValue = properties::CameraLocationUnknown;\n>\n> I'm still unconvinced about the use-case for unknown :-)\n>\n> To me it looks like we are pushing the problem of an incomplete firmware\n> description on to applications. How should an application handle a\n> external camera vs an unknown one?\n>\n> I would like to understand the advantage of adding a new location here\n> other then just to pass a CTS test that as far as I understands it fails\n> as the device it's running on have incomplete firmware. Is that not a\n> valid fail of CTS?\n>\n\nYour concerns are valid, but is it any better to default to 'Front\" if\nthe information is not available in the firmware. Bear in mind that\nlocation is also used to construct the unique camera name, and\ninstead of having two cameras arbitrary assigned to the front location\nisn't it better to just report that the location isn't known ?\n\nI this it's fair for libcamera to not assume any arbitrary defaults\nand have firmware fixed :)\n\n> > \t}\n> > \tproperties_.set(properties::Location, propertyValue);\n> >\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\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 AA568BD162\n\tfor ;\n\tFri, 12 Feb 2021 08:54:24 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8692E63768;\n\tFri, 12 Feb 2021 09:54:24 +0100 (CET)","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 1AAF263760\n\tfor ;\n\tFri, 12 Feb 2021 09:54:22 +0100 (CET)","from uno.localdomain (93-34-118-233.ip49.fastwebnet.it\n\t[93.34.118.233]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay7-d.mail.gandi.net (Postfix) with ESMTPSA id 780CC20006;\n\tFri, 12 Feb 2021 08:54:21 +0000 (UTC)"],"X-Originating-IP":"93.34.118.233","Date":"Fri, 12 Feb 2021 09:54:45 +0100","From":"Jacopo Mondi ","To":"Niklas =?utf-8?q?S=C3=B6derlund?= ","Message-ID":"<20210212085445.7rby2vq2xon2edmy@uno.localdomain>","References":"<20210212054816.53561-1-paul.elder@ideasonboard.com>\n\t<20210212054816.53561-3-paul.elder@ideasonboard.com>\n\t","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH v2 2/4] libcamera: camera_sensor: Set\n\tdefault sensor location to Unknown","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":15128,"web_url":"https://patchwork.libcamera.org/comment/15128/","msgid":"","date":"2021-02-12T09:07:58","subject":"Re: [libcamera-devel] [PATCH v2 2/4] libcamera: camera_sensor: Set\n\tdefault sensor location to Unknown","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Jacopo,\n\nOn 2021-02-12 09:54:45 +0100, Jacopo Mondi wrote:\n> Hi Niklas,\n> \n> On Fri, Feb 12, 2021 at 09:10:34AM +0100, Niklas Söderlund wrote:\n> > Hello Paul,\n> >\n> > Thanks for your work.\n> >\n> > On 2021-02-12 14:48:14 +0900, Paul Elder wrote:\n> > > Instead of choosing some arbitrary location for the sensor when its\n> > > location is unknown, set it explicitly to unknown.\n> > >\n> > > Signed-off-by: Paul Elder \n> > > Reviewed-by: Kieran Bingham \n> > > Reviewed-by: Laurent Pinchart \n> > >\n> > > ---\n> > > No change in v2\n> > > ---\n> > > src/libcamera/camera_sensor.cpp | 2 +-\n> > > 1 file changed, 1 insertion(+), 1 deletion(-)\n> > >\n> > > diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp\n> > > index c9e8d49b..474055ba 100644\n> > > --- a/src/libcamera/camera_sensor.cpp\n> > > +++ b/src/libcamera/camera_sensor.cpp\n> > > @@ -446,7 +446,7 @@ int CameraSensor::initProperties()\n> > > \t\t\tbreak;\n> > > \t\t}\n> > > \t} else {\n> > > -\t\tpropertyValue = properties::CameraLocationExternal;\n> > > +\t\tpropertyValue = properties::CameraLocationUnknown;\n> >\n> > I'm still unconvinced about the use-case for unknown :-)\n> >\n> > To me it looks like we are pushing the problem of an incomplete firmware\n> > description on to applications. How should an application handle a\n> > external camera vs an unknown one?\n> >\n> > I would like to understand the advantage of adding a new location here\n> > other then just to pass a CTS test that as far as I understands it fails\n> > as the device it's running on have incomplete firmware. Is that not a\n> > valid fail of CTS?\n> >\n> \n> Your concerns are valid, but is it any better to default to 'Front\" if\n> the information is not available in the firmware. Bear in mind that\n> location is also used to construct the unique camera name, and\n> instead of having two cameras arbitrary assigned to the front location\n> isn't it better to just report that the location isn't known ?\n\nI think it's better to default to something useful and LOG(Error) that \nwe default. I would default as we do to external. That CTS fails due to \nthis is just a consequence of incomplete camera integration.\n\n> \n> I this it's fair for libcamera to not assume any arbitrary defaults\n> and have firmware fixed :)\n\nSo we should refuse to instantiate cameras who don't report locations, I \nlike it ;-)\n\n> \n> > > \t}\n> > > \tproperties_.set(properties::Location, propertyValue);\n> > >\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\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 348CEBD160\n\tfor ;\n\tFri, 12 Feb 2021 09:08:02 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B6F3763767;\n\tFri, 12 Feb 2021 10:08:01 +0100 (CET)","from mail-lf1-x129.google.com (mail-lf1-x129.google.com\n\t[IPv6:2a00:1450:4864:20::129])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 3C36663762\n\tfor ;\n\tFri, 12 Feb 2021 10:08:00 +0100 (CET)","by mail-lf1-x129.google.com with SMTP id f1so12107308lfu.3\n\tfor ;\n\tFri, 12 Feb 2021 01:08:00 -0800 (PST)","from localhost (h-209-203.A463.priv.bahnhof.se. [155.4.209.203])\n\tby smtp.gmail.com with ESMTPSA id\n\tf21sm912021lfk.244.2021.02.12.01.07.58\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tFri, 12 Feb 2021 01:07:59 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=ragnatech-se.20150623.gappssmtp.com\n\theader.i=@ragnatech-se.20150623.gappssmtp.com\n\theader.b=\"SPSR9frS\"; 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=wEcruEIR7K4BZOese6IWHcrjTgrkf7w3B2cwGHTveEk=;\n\tb=SPSR9frSSsVw2W/9h0Tj2qg4ImUkc928n2r/tugY3Jd9cm3GKkoIxoFIs+wEIw9B7u\n\tskZajy8tEgQWd+T+aE7jRXif+0Pv6PCRgyytvrXC/8PsNxsKSTNXs71X7Cu+KvyKESdY\n\thxP2jna5BYy3x7YUqlN0ghDZTSqeE5sjEReCDQfTBakkTNnecf4hXBYdJtTF+KDlxGvE\n\tYLlBO0B9M/6mYtvIMEB9E3yU0O7AVVXwuWYtaIqpbyXUo93baOJtHG8aL9s9ySYAiqQi\n\tHAink47uxoVZ7wdXDcsYpjXPMtvHMkfx78uDQrvNtuFO5ATEqMvU2o2ll3xPyCjghaxl\n\tU6Wg==","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=wEcruEIR7K4BZOese6IWHcrjTgrkf7w3B2cwGHTveEk=;\n\tb=hGK2pbWOLvpMR2BAhQWrP2Wh8MChIVxOCgAL4xbELzovpRAf9DtOdjHSAnYQ5pAEO9\n\td6BRwXcpukYSSGSTaEz3V2PL2gmdWgCV+nqRSjnBU/r9I4jAyoHgKqNutizSsYmM6zwn\n\tqFTOymfPTzzlS+0cBaz3tpm8x7Vmz+5kA7tUeCvL6VgEXhz3G74lms8lf+EP5a+BL+D6\n\tfa0vX1BDDWSCEna4auzqKFssUuuC7gk/fXq+wJRdhbN/RHbPZMkhO7knPldLAPCdBlYb\n\txFsYG0UwFth7xvskjLOKsyJ9r4bmcLqzd6Hl/KfFViZ16iwlRqVF4o6zIuHPe5KKoBgY\n\tXXhA==","X-Gm-Message-State":"AOAM531Y70YTHYBW5WdsH8uWLPynaO+4f7bJY2D12mX+h49+kKCCStmj\n\t0T4j3hpd3PJ7/N3v76E0vrlL/Q==","X-Google-Smtp-Source":"ABdhPJy2CWnfFJZXpi4bKJSu4iGa96gRmLkH7Z5od4qzUWiXJZ0a1q7oJFXFVhkkLUJsv+pnH0qZ8Q==","X-Received":"by 2002:a05:6512:3607:: with SMTP id\n\tf7mr1143199lfs.550.1613120879644; \n\tFri, 12 Feb 2021 01:07:59 -0800 (PST)","Date":"Fri, 12 Feb 2021 10:07:58 +0100","From":"Niklas =?iso-8859-1?q?S=F6derlund?= ","To":"Jacopo Mondi ","Message-ID":"","References":"<20210212054816.53561-1-paul.elder@ideasonboard.com>\n\t<20210212054816.53561-3-paul.elder@ideasonboard.com>\n\t\n\t<20210212085445.7rby2vq2xon2edmy@uno.localdomain>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20210212085445.7rby2vq2xon2edmy@uno.localdomain>","Subject":"Re: [libcamera-devel] [PATCH v2 2/4] libcamera: camera_sensor: Set\n\tdefault sensor location to Unknown","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":15130,"web_url":"https://patchwork.libcamera.org/comment/15130/","msgid":"<20210212092319.q3w4jrj5vxseef44@uno.localdomain>","date":"2021-02-12T09:23:19","subject":"Re: [libcamera-devel] [PATCH v2 2/4] libcamera: camera_sensor: Set\n\tdefault sensor location to Unknown","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Niklas,\n\nOn Fri, Feb 12, 2021 at 10:07:58AM +0100, Niklas Söderlund wrote:\n> Hi Jacopo,\n>\n> On 2021-02-12 09:54:45 +0100, Jacopo Mondi wrote:\n> > Hi Niklas,\n> >\n> > On Fri, Feb 12, 2021 at 09:10:34AM +0100, Niklas Söderlund wrote:\n> > > Hello Paul,\n> > >\n> > > Thanks for your work.\n> > >\n> > > On 2021-02-12 14:48:14 +0900, Paul Elder wrote:\n> > > > Instead of choosing some arbitrary location for the sensor when its\n> > > > location is unknown, set it explicitly to unknown.\n> > > >\n> > > > Signed-off-by: Paul Elder \n> > > > Reviewed-by: Kieran Bingham \n> > > > Reviewed-by: Laurent Pinchart \n> > > >\n> > > > ---\n> > > > No change in v2\n> > > > ---\n> > > > src/libcamera/camera_sensor.cpp | 2 +-\n> > > > 1 file changed, 1 insertion(+), 1 deletion(-)\n> > > >\n> > > > diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp\n> > > > index c9e8d49b..474055ba 100644\n> > > > --- a/src/libcamera/camera_sensor.cpp\n> > > > +++ b/src/libcamera/camera_sensor.cpp\n> > > > @@ -446,7 +446,7 @@ int CameraSensor::initProperties()\n> > > > \t\t\tbreak;\n> > > > \t\t}\n> > > > \t} else {\n> > > > -\t\tpropertyValue = properties::CameraLocationExternal;\n> > > > +\t\tpropertyValue = properties::CameraLocationUnknown;\n> > >\n> > > I'm still unconvinced about the use-case for unknown :-)\n> > >\n> > > To me it looks like we are pushing the problem of an incomplete firmware\n> > > description on to applications. How should an application handle a\n> > > external camera vs an unknown one?\n> > >\n> > > I would like to understand the advantage of adding a new location here\n> > > other then just to pass a CTS test that as far as I understands it fails\n> > > as the device it's running on have incomplete firmware. Is that not a\n> > > valid fail of CTS?\n> > >\n> >\n> > Your concerns are valid, but is it any better to default to 'Front\" if\n> > the information is not available in the firmware. Bear in mind that\n> > location is also used to construct the unique camera name, and\n> > instead of having two cameras arbitrary assigned to the front location\n> > isn't it better to just report that the location isn't known ?\n>\n> I think it's better to default to something useful and LOG(Error) that\n> we default. I would default as we do to external. That CTS fails due to\n> this is just a consequence of incomplete camera integration.\n>\n\nI'm sorry I don't agree here. Defaulting to something reasonable is an\noperation that happens in many places in libcamera, but assuming\nFRONT/EXTERNAL is a totally arbitrary choice that might provide\napplication a false sense of security.\n\nWhat if an application wants to open a camera that is reported as\nfront and is actually installed on the back ? As we have seen a\ncamera being 'external' as implications for Android as I assume it\ncould have for other applications that might decide to rule out\nexternal pluggable cameras.\n\n> >\n> > I this it's fair for libcamera to not assume any arbitrary defaults\n> > and have firmware fixed :)\n>\n> So we should refuse to instantiate cameras who don't report locations, I\n> like it ;-)\n>\n\nI wish :)\n\nNo, but reported \"I don't know and cannot tell\" will require\napplication and system integrator to find that information out\nsomehow. If integrators can and want change the acpi tables or DTS\nthey should do so, otherwise I expect downstreams will hardcode those\ninformation somehow.\n\nBut again, as we don't have any criteria to chose what to default to,\nI guess it's better to just report we don't know and cannot retrieve\nthat information.\n\n> >\n> > > > \t}\n> > > > \tproperties_.set(properties::Location, propertyValue);\n> > > >\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\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 DC514BD162\n\tfor ;\n\tFri, 12 Feb 2021 09:22:55 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B5EEB63768;\n\tFri, 12 Feb 2021 10:22:55 +0100 (CET)","from relay12.mail.gandi.net (relay12.mail.gandi.net\n\t[217.70.178.232])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 2652463762\n\tfor ;\n\tFri, 12 Feb 2021 10:22:55 +0100 (CET)","from uno.localdomain (93-34-118-233.ip49.fastwebnet.it\n\t[93.34.118.233]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay12.mail.gandi.net (Postfix) with ESMTPSA id 86C91200004;\n\tFri, 12 Feb 2021 09:22:54 +0000 (UTC)"],"Date":"Fri, 12 Feb 2021 10:23:19 +0100","From":"Jacopo Mondi ","To":"Niklas =?utf-8?q?S=C3=B6derlund?= ","Message-ID":"<20210212092319.q3w4jrj5vxseef44@uno.localdomain>","References":"<20210212054816.53561-1-paul.elder@ideasonboard.com>\n\t<20210212054816.53561-3-paul.elder@ideasonboard.com>\n\t\n\t<20210212085445.7rby2vq2xon2edmy@uno.localdomain>\n\t","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH v2 2/4] libcamera: camera_sensor: Set\n\tdefault sensor location to Unknown","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":15132,"web_url":"https://patchwork.libcamera.org/comment/15132/","msgid":"","date":"2021-02-12T09:36:44","subject":"Re: [libcamera-devel] [PATCH v2 2/4] libcamera: camera_sensor: Set\n\tdefault sensor location to Unknown","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hello Jacopo,\n\nOn 2021-02-12 10:23:19 +0100, Jacopo Mondi wrote:\n> Hi Niklas,\n> \n> On Fri, Feb 12, 2021 at 10:07:58AM +0100, Niklas Söderlund wrote:\n> > Hi Jacopo,\n> >\n> > On 2021-02-12 09:54:45 +0100, Jacopo Mondi wrote:\n> > > Hi Niklas,\n> > >\n> > > On Fri, Feb 12, 2021 at 09:10:34AM +0100, Niklas Söderlund wrote:\n> > > > Hello Paul,\n> > > >\n> > > > Thanks for your work.\n> > > >\n> > > > On 2021-02-12 14:48:14 +0900, Paul Elder wrote:\n> > > > > Instead of choosing some arbitrary location for the sensor when its\n> > > > > location is unknown, set it explicitly to unknown.\n> > > > >\n> > > > > Signed-off-by: Paul Elder \n> > > > > Reviewed-by: Kieran Bingham \n> > > > > Reviewed-by: Laurent Pinchart \n> > > > >\n> > > > > ---\n> > > > > No change in v2\n> > > > > ---\n> > > > > src/libcamera/camera_sensor.cpp | 2 +-\n> > > > > 1 file changed, 1 insertion(+), 1 deletion(-)\n> > > > >\n> > > > > diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp\n> > > > > index c9e8d49b..474055ba 100644\n> > > > > --- a/src/libcamera/camera_sensor.cpp\n> > > > > +++ b/src/libcamera/camera_sensor.cpp\n> > > > > @@ -446,7 +446,7 @@ int CameraSensor::initProperties()\n> > > > > \t\t\tbreak;\n> > > > > \t\t}\n> > > > > \t} else {\n> > > > > -\t\tpropertyValue = properties::CameraLocationExternal;\n> > > > > +\t\tpropertyValue = properties::CameraLocationUnknown;\n> > > >\n> > > > I'm still unconvinced about the use-case for unknown :-)\n> > > >\n> > > > To me it looks like we are pushing the problem of an incomplete firmware\n> > > > description on to applications. How should an application handle a\n> > > > external camera vs an unknown one?\n> > > >\n> > > > I would like to understand the advantage of adding a new location here\n> > > > other then just to pass a CTS test that as far as I understands it fails\n> > > > as the device it's running on have incomplete firmware. Is that not a\n> > > > valid fail of CTS?\n> > > >\n> > >\n> > > Your concerns are valid, but is it any better to default to 'Front\" if\n> > > the information is not available in the firmware. Bear in mind that\n> > > location is also used to construct the unique camera name, and\n> > > instead of having two cameras arbitrary assigned to the front location\n> > > isn't it better to just report that the location isn't known ?\n> >\n> > I think it's better to default to something useful and LOG(Error) that\n> > we default. I would default as we do to external. That CTS fails due to\n> > this is just a consequence of incomplete camera integration.\n> >\n> \n> I'm sorry I don't agree here. Defaulting to something reasonable is an\n> operation that happens in many places in libcamera, but assuming\n> FRONT/EXTERNAL is a totally arbitrary choice that might provide\n> application a false sense of security.\n> \n> What if an application wants to open a camera that is reported as\n> front and is actually installed on the back ? As we have seen a\n> camera being 'external' as implications for Android as I assume it\n> could have for other applications that might decide to rule out\n> external pluggable cameras.\n> \n> > >\n> > > I this it's fair for libcamera to not assume any arbitrary defaults\n> > > and have firmware fixed :)\n> >\n> > So we should refuse to instantiate cameras who don't report locations, I\n> > like it ;-)\n> >\n> \n> I wish :)\n> \n> No, but reported \"I don't know and cannot tell\" will require\n> application and system integrator to find that information out\n> somehow. If integrators can and want change the acpi tables or DTS\n> they should do so, otherwise I expect downstreams will hardcode those\n> information somehow.\n> \n> But again, as we don't have any criteria to chose what to default to,\n> I guess it's better to just report we don't know and cannot retrieve\n> that information.\n\nI'm sorry, I still don't see a use-case here other then to push \nincomplete firmware or integration configuration onto applications and \nget CTS to pass using IMHO a hack. But I seems to be in minority so I \nwill let it go and move onto implementation :-)\n\nIf we don't know the location and are fine to pushing the problem to \napplications, is it not then nicer to make Location an optional property \nand simply not set it if we don't know the location?\n\n> \n> > >\n> > > > > \t}\n> > > > > \tproperties_.set(properties::Location, propertyValue);\n> > > > >\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\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 C39EEBD162\n\tfor ;\n\tFri, 12 Feb 2021 09:36:47 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5B26063762;\n\tFri, 12 Feb 2021 10:36:47 +0100 (CET)","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 EE2BF63762\n\tfor ;\n\tFri, 12 Feb 2021 10:36:45 +0100 (CET)","by mail-lj1-x230.google.com with SMTP id a17so10820690ljq.2\n\tfor ;\n\tFri, 12 Feb 2021 01:36:45 -0800 (PST)","from localhost (h-209-203.A463.priv.bahnhof.se. [155.4.209.203])\n\tby smtp.gmail.com with ESMTPSA id\n\tw21sm912685lfu.132.2021.02.12.01.36.44\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tFri, 12 Feb 2021 01:36:44 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=ragnatech-se.20150623.gappssmtp.com\n\theader.i=@ragnatech-se.20150623.gappssmtp.com\n\theader.b=\"ewmtigtX\"; 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=wiu3dDHh8KJGDl4pUbgQ762DVPNTBxiKlhLLNw+PGnQ=;\n\tb=ewmtigtXaYzK5osNjhCF2eEbIgKpu559s06oUI8WS494cpbzAtHVhrdKJSgq3Gi6OG\n\t1f+j947Gs9dMJOw7l9a8n7s++4A6sYRqpwFbWkTGe9ePlN3VwmjFhBzVjsr3gfYR07Ex\n\tGSvd6HWDKJV7/xbKhnUYjpXSHBJSAkSJvb4M0h2emhEbzW3yrnDrFUcrybdoIar88mDL\n\tuuVoZbeItV/DbtqptbS48K3F4w/mhROn0HIXnzFwszR2dSa7YLa1hN/o/JHofa2NhqeJ\n\tJZKgxuD+UMQcrhnValbd5LC49pqorXCo2xz3ivb2i9ye9q3nBgIXNIBuy9YBcZ1OeFG3\n\t7/+g==","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=wiu3dDHh8KJGDl4pUbgQ762DVPNTBxiKlhLLNw+PGnQ=;\n\tb=nlI9J3MPU2kWDklX6uOJZypAgTh5m3UCwpMZlKLWfDPqzezkE8sq9wJA5dNsxJe98m\n\t56qr7j8BhFIy65+ybOt1FebfmMpF4EHE8s4YNNLj7e/hyYQdv0Wouww2Ejgs1V8MeM3p\n\tAQW2gzIr1x4Qyc7jbaRedNd8iM4MMCpkUdBmd8O4mxUNr1gLUq/T67lu5W010H3Pd2c1\n\tJNKKbwu7453xm2dkSzXQbXN3sjIslScktzPX7GE56ydwXoqSKzZilfeEkMD88xNFblke\n\t3Et6dzimHN32+aSvOSEai3eCdF5hy+vAkF/tdZ6aAufHyanORDLLdfZWA8LvUZpAAZQj\n\tm/Ww==","X-Gm-Message-State":"AOAM533jrlGlJf8yvrGEFtvHthC4ebk3mIACzuTfHBypBkkvF4aDzUUH\n\tiW85z9VJyB4N9iIhHH0EZ5Bi7sDG4lUstQ==","X-Google-Smtp-Source":"ABdhPJz/QBX2Xu/+4o9+nuC/UGPJrXwVqTue8jHU/SQwNOy57vrPG6Yh6IYsp/Vh2XxZDaUNJKF3fw==","X-Received":"by 2002:a2e:b0f3:: with SMTP id\n\th19mr1150333ljl.233.1613122605344; \n\tFri, 12 Feb 2021 01:36:45 -0800 (PST)","Date":"Fri, 12 Feb 2021 10:36:44 +0100","From":"Niklas =?iso-8859-1?q?S=F6derlund?= ","To":"Jacopo Mondi ","Message-ID":"","References":"<20210212054816.53561-1-paul.elder@ideasonboard.com>\n\t<20210212054816.53561-3-paul.elder@ideasonboard.com>\n\t\n\t<20210212085445.7rby2vq2xon2edmy@uno.localdomain>\n\t\n\t<20210212092319.q3w4jrj5vxseef44@uno.localdomain>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20210212092319.q3w4jrj5vxseef44@uno.localdomain>","Subject":"Re: [libcamera-devel] [PATCH v2 2/4] libcamera: camera_sensor: Set\n\tdefault sensor location to Unknown","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\" "}}]