[{"id":15755,"web_url":"https://patchwork.libcamera.org/comment/15755/","msgid":"<YFNPE2SynJLMnGxK@pendragon.ideasonboard.com>","date":"2021-03-18T13:01:07","subject":"Re: [libcamera-devel] [PATCH 1/2] libcamera: camera_sensor: Do not\n\tregister Location if not available","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nThank you for the patch.\n\nOn Thu, Mar 18, 2021 at 01:55:20PM +0100, Jacopo Mondi wrote:\n> Do not register the Location property if not available from the firmware\n> interface instead of defaulting it to External.\n\nThis looks good, but I think we need a patch before this one to make the\nlocation property optional in the cam application.\n\n> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>\n\nReviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n\n> ---\n>  src/libcamera/camera_sensor.cpp | 6 ++----\n>  1 file changed, 2 insertions(+), 4 deletions(-)\n> \n> diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp\n> index 27f82071151e..f7ed91d990f7 100644\n> --- a/src/libcamera/camera_sensor.cpp\n> +++ b/src/libcamera/camera_sensor.cpp\n> @@ -468,12 +468,10 @@ int CameraSensor::initProperties()\n>  \t\t\tpropertyValue = properties::CameraLocationBack;\n>  \t\t\tbreak;\n>  \t\t}\n> +\t\tproperties_.set(properties::Location, propertyValue);\n>  \t} else {\n> -\t\tLOG(CameraSensor, Warning)\n> -\t\t\t<< \"Failed to retrieve the camera location, setting to External\";\n> -\t\tpropertyValue = properties::CameraLocationExternal;\n> +\t\tLOG(CameraSensor, Warning) << \"Failed to retrieve the camera location\";\n>  \t}\n> -\tproperties_.set(properties::Location, propertyValue);\n>  \n>  \tconst auto &rotationControl = controls.find(V4L2_CID_CAMERA_SENSOR_ROTATION);\n>  \tif (rotationControl != controls.end()) {","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 EFCEFBD80C\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu, 18 Mar 2021 13:01:47 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 73980602E3;\n\tThu, 18 Mar 2021 14:01:47 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id F1FFE602DB\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 18 Mar 2021 14:01:45 +0100 (CET)","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 635F6899;\n\tThu, 18 Mar 2021 14:01:45 +0100 (CET)"],"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=\"OrKGKuOm\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1616072505;\n\tbh=CKB0XPlkNCW4GOHab7QSfY128jyV0085/uHEdVCGStg=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=OrKGKuOmJifBH085SJ03GRRh0rxMUhECQto4m4wFaJq8IU9icM0w+M8flHqMoMHgD\n\tvLSb8R050h9ccpIJbR2rkaYFi3EZSNORCiTotitFsuts6ky/z+EvpiNOvkuz/pPHCa\n\t48xIhGL9C7ZZXLxOz9BTLuwq3rbMYTe3G9yeoN/8=","Date":"Thu, 18 Mar 2021 15:01:07 +0200","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Jacopo Mondi <jacopo@jmondi.org>","Message-ID":"<YFNPE2SynJLMnGxK@pendragon.ideasonboard.com>","References":"<20210318125521.43278-1-jacopo@jmondi.org>\n\t<20210318125521.43278-2-jacopo@jmondi.org>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20210318125521.43278-2-jacopo@jmondi.org>","Subject":"Re: [libcamera-devel] [PATCH 1/2] libcamera: camera_sensor: Do not\n\tregister Location if not available","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=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":15756,"web_url":"https://patchwork.libcamera.org/comment/15756/","msgid":"<20210318134109.mn4isqlbtbazy7dk@uno.localdomain>","date":"2021-03-18T13:41:09","subject":"Re: [libcamera-devel] [PATCH 1/2] libcamera: camera_sensor: Do not\n\tregister Location if not available","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Laurent,\n\nOn Thu, Mar 18, 2021 at 03:01:07PM +0200, Laurent Pinchart wrote:\n> Hi Jacopo,\n>\n> Thank you for the patch.\n>\n> On Thu, Mar 18, 2021 at 01:55:20PM +0100, Jacopo Mondi wrote:\n> > Do not register the Location property if not available from the firmware\n> > interface instead of defaulting it to External.\n>\n> This looks good, but I think we need a patch before this one to make the\n> location property optional in the cam application.\n\nI'm so concerned about CTS I've ignored our own test app :/\nHow should it look like, as now Location is used to build the camera\nname. Simply remove any reference to the camera position ?\n\nThanks\n  j\n\n>\n> > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>\n>\n> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n>\n> > ---\n> >  src/libcamera/camera_sensor.cpp | 6 ++----\n> >  1 file changed, 2 insertions(+), 4 deletions(-)\n> >\n> > diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp\n> > index 27f82071151e..f7ed91d990f7 100644\n> > --- a/src/libcamera/camera_sensor.cpp\n> > +++ b/src/libcamera/camera_sensor.cpp\n> > @@ -468,12 +468,10 @@ int CameraSensor::initProperties()\n> >  \t\t\tpropertyValue = properties::CameraLocationBack;\n> >  \t\t\tbreak;\n> >  \t\t}\n> > +\t\tproperties_.set(properties::Location, propertyValue);\n> >  \t} else {\n> > -\t\tLOG(CameraSensor, Warning)\n> > -\t\t\t<< \"Failed to retrieve the camera location, setting to External\";\n> > -\t\tpropertyValue = properties::CameraLocationExternal;\n> > +\t\tLOG(CameraSensor, Warning) << \"Failed to retrieve the camera location\";\n> >  \t}\n> > -\tproperties_.set(properties::Location, propertyValue);\n> >\n> >  \tconst auto &rotationControl = controls.find(V4L2_CID_CAMERA_SENSOR_ROTATION);\n> >  \tif (rotationControl != controls.end()) {\n>\n> --\n> Regards,\n>\n> Laurent Pinchart","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 41721C32E1\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu, 18 Mar 2021 13:40:40 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A1ACE68D5E;\n\tThu, 18 Mar 2021 14:40:39 +0100 (CET)","from relay9-d.mail.gandi.net (relay9-d.mail.gandi.net\n\t[217.70.183.199])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 589C9602DB\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 18 Mar 2021 14:40:38 +0100 (CET)","from uno.localdomain (host-79-22-58-175.retail.telecomitalia.it\n\t[79.22.58.175]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay9-d.mail.gandi.net (Postfix) with ESMTPSA id A16D4FF802;\n\tThu, 18 Mar 2021 13:40:37 +0000 (UTC)"],"X-Originating-IP":"79.22.58.175","Date":"Thu, 18 Mar 2021 14:41:09 +0100","From":"Jacopo Mondi <jacopo@jmondi.org>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Message-ID":"<20210318134109.mn4isqlbtbazy7dk@uno.localdomain>","References":"<20210318125521.43278-1-jacopo@jmondi.org>\n\t<20210318125521.43278-2-jacopo@jmondi.org>\n\t<YFNPE2SynJLMnGxK@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<YFNPE2SynJLMnGxK@pendragon.ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH 1/2] libcamera: camera_sensor: Do not\n\tregister Location if not available","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=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":15757,"web_url":"https://patchwork.libcamera.org/comment/15757/","msgid":"<YFNaJ8mBUGX6n848@pendragon.ideasonboard.com>","date":"2021-03-18T13:48:23","subject":"Re: [libcamera-devel] [PATCH 1/2] libcamera: camera_sensor: Do not\n\tregister Location if not available","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nOn Thu, Mar 18, 2021 at 02:41:09PM +0100, Jacopo Mondi wrote:\n> On Thu, Mar 18, 2021 at 03:01:07PM +0200, Laurent Pinchart wrote:\n> > On Thu, Mar 18, 2021 at 01:55:20PM +0100, Jacopo Mondi wrote:\n> > > Do not register the Location property if not available from the firmware\n> > > interface instead of defaulting it to External.\n> >\n> > This looks good, but I think we need a patch before this one to make the\n> > location property optional in the cam application.\n> \n> I'm so concerned about CTS I've ignored our own test app :/\n> How should it look like, as now Location is used to build the camera\n> name. Simply remove any reference to the camera position ?\n\nI'd make it optional, keeping it in the name if it's present, removing\nit otherwise.\n\n> > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>\n> >\n> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> >\n> > > ---\n> > >  src/libcamera/camera_sensor.cpp | 6 ++----\n> > >  1 file changed, 2 insertions(+), 4 deletions(-)\n> > >\n> > > diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp\n> > > index 27f82071151e..f7ed91d990f7 100644\n> > > --- a/src/libcamera/camera_sensor.cpp\n> > > +++ b/src/libcamera/camera_sensor.cpp\n> > > @@ -468,12 +468,10 @@ int CameraSensor::initProperties()\n> > >  \t\t\tpropertyValue = properties::CameraLocationBack;\n> > >  \t\t\tbreak;\n> > >  \t\t}\n> > > +\t\tproperties_.set(properties::Location, propertyValue);\n> > >  \t} else {\n> > > -\t\tLOG(CameraSensor, Warning)\n> > > -\t\t\t<< \"Failed to retrieve the camera location, setting to External\";\n> > > -\t\tpropertyValue = properties::CameraLocationExternal;\n> > > +\t\tLOG(CameraSensor, Warning) << \"Failed to retrieve the camera location\";\n> > >  \t}\n> > > -\tproperties_.set(properties::Location, propertyValue);\n> > >\n> > >  \tconst auto &rotationControl = controls.find(V4L2_CID_CAMERA_SENSOR_ROTATION);\n> > >  \tif (rotationControl != controls.end()) {","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 5E9BFBD80C\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu, 18 Mar 2021 13:49:04 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 9BB10602E3;\n\tThu, 18 Mar 2021 14:49:03 +0100 (CET)","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 47AD8602DB\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 18 Mar 2021 14:49:02 +0100 (CET)","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 9CA29899;\n\tThu, 18 Mar 2021 14:49:01 +0100 (CET)"],"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=\"iVSHsCqP\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1616075341;\n\tbh=P+uAdwLnSmG+L/zjIsgL/buPUVSJJJkxCcUmACA/wQw=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=iVSHsCqP1BVIqrrXLJei6L1RuXgTCV7MwW+Hn/Arqy1LE8Kh7qckcu390mgoIPD/Q\n\trUJrlGl2PqfRu4ckS4q9xXxCWWubUTFWPUJXvwi2qDPecfjLf6F+MgkK9mxcAAv9TQ\n\t3OW0CGNPOClaFr6epTU3zwYut98HXFlY+tguCJRs=","Date":"Thu, 18 Mar 2021 15:48:23 +0200","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Jacopo Mondi <jacopo@jmondi.org>","Message-ID":"<YFNaJ8mBUGX6n848@pendragon.ideasonboard.com>","References":"<20210318125521.43278-1-jacopo@jmondi.org>\n\t<20210318125521.43278-2-jacopo@jmondi.org>\n\t<YFNPE2SynJLMnGxK@pendragon.ideasonboard.com>\n\t<20210318134109.mn4isqlbtbazy7dk@uno.localdomain>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20210318134109.mn4isqlbtbazy7dk@uno.localdomain>","Subject":"Re: [libcamera-devel] [PATCH 1/2] libcamera: camera_sensor: Do not\n\tregister Location if not available","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=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]