[{"id":11689,"web_url":"https://patchwork.libcamera.org/comment/11689/","msgid":"<20200729081830.w7oyhcuuxhw5alel@uno.localdomain>","date":"2020-07-29T08:18:30","subject":"Re: [libcamera-devel] [PATCH v3 5/5] libcamera: camera_manager:\n\tEnforce unique camera names","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Niklas,\n\nOn Wed, Jul 29, 2020 at 01:42:25AM +0200, Niklas Söderlund wrote:\n> The camera name have always been documented that it should be unique but\n> it has never been enforced. Change this by refuse to add cameras to the\n\nby refusing\n\n> CameraManager that would create two cameras with the exact same name.\n>\n> Signed-off-by: Niklas Söderlund \n> ---\n> src/libcamera/camera_manager.cpp | 6 +++---\n> 1 file changed, 3 insertions(+), 3 deletions(-)\n>\n> diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp\n> index f60491d2c1a7500f..7d83263f1fabf5da 100644\n> --- a/src/libcamera/camera_manager.cpp\n> +++ b/src/libcamera/camera_manager.cpp\n> @@ -178,10 +178,10 @@ void CameraManager::Private::addCamera(std::shared_ptr camera,\n>\n> \tfor (std::shared_ptr c : cameras_) {\n> \t\tif (c->name() == camera->name()) {\n> -\t\t\tLOG(Camera, Warning)\n> -\t\t\t\t<< \"Registering camera with duplicate name '\"\n> +\t\t\tLOG(Camera, Error)\n> +\t\t\t\t<< \"Skip registering camera with duplicated name '\"\n> \t\t\t\t<< camera->name() << \"'\";\n> -\t\t\tbreak;\n> +\t\t\treturn;\n\nThe error is not propagated up... not from here, not from the only\ncaller PipelineHandler::registerCamera.\n\nI guess pipelines then do not have a way to know if registration was\nsuccessful or not. I don't think it's a big deal, they will notice\nwhile developing that something is wrong if a camera does not show up,\nan error code is not strictly needed.\n\nThanks\n j\n\n> \t\t}\n> \t}\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 07B41BD878\n\tfor ;\n\tWed, 29 Jul 2020 08:14:53 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 76605613C6;\n\tWed, 29 Jul 2020 10:14:52 +0200 (CEST)","from relay2-d.mail.gandi.net (relay2-d.mail.gandi.net\n\t[217.70.183.194])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 3857C605B2\n\tfor ;\n\tWed, 29 Jul 2020 10:14:51 +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 relay2-d.mail.gandi.net (Postfix) with ESMTPSA id 504F640011;\n\tWed, 29 Jul 2020 08:14:50 +0000 (UTC)"],"X-Originating-IP":"93.34.118.233","Date":"Wed, 29 Jul 2020 10:18:30 +0200","From":"Jacopo Mondi ","To":"Niklas =?utf-8?q?S=C3=B6derlund?= ","Message-ID":"<20200729081830.w7oyhcuuxhw5alel@uno.localdomain>","References":"<20200728233744.3503740-1-niklas.soderlund@ragnatech.se>\n\t<20200728234225.3505868-1-niklas.soderlund@ragnatech.se>\n\t<20200728234225.3505868-5-niklas.soderlund@ragnatech.se>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200728234225.3505868-5-niklas.soderlund@ragnatech.se>","Subject":"Re: [libcamera-devel] [PATCH v3 5/5] libcamera: camera_manager:\n\tEnforce unique camera names","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":11696,"web_url":"https://patchwork.libcamera.org/comment/11696/","msgid":"<20200729091118.GD273308@oden.dyn.berto.se>","date":"2020-07-29T09:11:18","subject":"Re: [libcamera-devel] [PATCH v3 5/5] libcamera: camera_manager:\n\tEnforce unique camera names","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 feedback.\n\nOn 2020-07-29 10:18:30 +0200, Jacopo Mondi wrote:\n> Hi Niklas,\n> \n> On Wed, Jul 29, 2020 at 01:42:25AM +0200, Niklas Söderlund wrote:\n> > The camera name have always been documented that it should be unique but\n> > it has never been enforced. Change this by refuse to add cameras to the\n> \n> by refusing\n> \n> > CameraManager that would create two cameras with the exact same name.\n> >\n> > Signed-off-by: Niklas Söderlund \n> > ---\n> > src/libcamera/camera_manager.cpp | 6 +++---\n> > 1 file changed, 3 insertions(+), 3 deletions(-)\n> >\n> > diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp\n> > index f60491d2c1a7500f..7d83263f1fabf5da 100644\n> > --- a/src/libcamera/camera_manager.cpp\n> > +++ b/src/libcamera/camera_manager.cpp\n> > @@ -178,10 +178,10 @@ void CameraManager::Private::addCamera(std::shared_ptr camera,\n> >\n> > \tfor (std::shared_ptr c : cameras_) {\n> > \t\tif (c->name() == camera->name()) {\n> > -\t\t\tLOG(Camera, Warning)\n> > -\t\t\t\t<< \"Registering camera with duplicate name '\"\n> > +\t\t\tLOG(Camera, Error)\n> > +\t\t\t\t<< \"Skip registering camera with duplicated name '\"\n> > \t\t\t\t<< camera->name() << \"'\";\n> > -\t\t\tbreak;\n> > +\t\t\treturn;\n> \n> The error is not propagated up... not from here, not from the only\n> caller PipelineHandler::registerCamera.\n> \n> I guess pipelines then do not have a way to know if registration was\n> successful or not. I don't think it's a big deal, they will notice\n> while developing that something is wrong if a camera does not show up,\n> an error code is not strictly needed.\n\nThat is true, and this should really not be happening. I originally had \nthis error as fatal but changed my mind as if by some freak of nature \nsome firmware ACPI would name their node path to \"VIMC Sensor B\" and we \nwould fatal fail if VIMC driver was loaded and I thought it better to \nerror and try to function with other cameras in the system.\n\nI'm more then willing to make this error fatal. What do you guys think?\n\n> \n> Thanks\n> j\n> \n> > \t\t}\n> > \t}\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 DB675BD86F\n\tfor ;\n\tWed, 29 Jul 2020 09:11:21 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 69AB9613C6;\n\tWed, 29 Jul 2020 11:11:21 +0200 (CEST)","from mail-lf1-x143.google.com (mail-lf1-x143.google.com\n\t[IPv6:2a00:1450:4864:20::143])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 2C966605B2\n\tfor ;\n\tWed, 29 Jul 2020 11:11:20 +0200 (CEST)","by mail-lf1-x143.google.com with SMTP id j22so6706454lfm.2\n\tfor ;\n\tWed, 29 Jul 2020 02:11:20 -0700 (PDT)","from localhost (h-209-203.A463.priv.bahnhof.se. [155.4.209.203])\n\tby smtp.gmail.com with ESMTPSA id\n\t193sm384877lfa.90.2020.07.29.02.11.18\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tWed, 29 Jul 2020 02:11:18 -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=\"P+iTglQl\"; 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=wlf3VFoRTfHHHnyhl5MMl+xIEFHPkBS09VQ4FWhY5Ic=;\n\tb=P+iTglQlcgG6UbRpSoMy4JTuFlwvzWdL/PJoyWXfOB1XeH7iKLuAgjpDDfhTZ5eGp1\n\tA4IQjasPJzLWUHfI93lcGiJEGGLnFG/6YO7OnBk0RQbnnVQWvgrNAHkKbOnjBLJ4dlbU\n\tdTaqQuaP/nWAG7ynR0aw3/ECQaKxQIPB24it0dVFCiHsKLbqeIeIvEF2/52eLLlz8QB8\n\t9EsivFWqHFEg1tZT2V6IizyX9HcwzqAmcWdxwqwOxwqv/oZQDMU5mIf47wciS3vaF1z0\n\t0C/WghtZI8rGSHswqHXCCuniTZJ/tDSa5RgLxv09PTGWsmUzUIQckwigPsSfuo8FADZb\n\tpNXA==","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=wlf3VFoRTfHHHnyhl5MMl+xIEFHPkBS09VQ4FWhY5Ic=;\n\tb=GIvVJ+gufyz8K8egApgy3OaK5chv+vQpn+/S/1SLsvIbV7jtdcqPaiaMKNWJ/3Ko4n\n\tfAEGU3kYrhNSZQrMibnhWJBGrYVu1R8H2Sw9lCTvSQpIAmo65aH+QvU/4YB8rqXAOJ90\n\trQ8nNlouW0xKePDnGm1XbTlMqzUlcqMW2FPWHpJZeRmQWiFn7zjZDGxl6nUiMImLg7a5\n\twwTSw8BtGnnlyzdkEGcNVUSD0Mieot5Ox4vhVbRLqaPGSxS+GCDF2MWQuAo8hG2uWClJ\n\t8IKIIwy6S597mx6V+7Lh+vMzGEirkS8cFU9/qCscjyfEYs5bmX5vp7sque7q+lLCpSB7\n\tYOpQ==","X-Gm-Message-State":"AOAM533Qt5e95cqGyCJStFDPvhiwoX/lFOOQBtdUBre3ucj1qEoijhsb\n\tX8ybP32KKh3PcMqDuzxwKXIN8dxeGgY=","X-Google-Smtp-Source":"ABdhPJzahJvktTlY8f4g58xCI+H8ykp6nbAvBwR44uoXFwAkUropheiXwxxEsbCkVbqmCFZO/QkEvA==","X-Received":"by 2002:a19:3803:: with SMTP id f3mr5931166lfa.179.1596013879426;\n\tWed, 29 Jul 2020 02:11:19 -0700 (PDT)","Date":"Wed, 29 Jul 2020 11:11:18 +0200","From":"Niklas =?iso-8859-1?q?S=F6derlund?= ","To":"Jacopo Mondi ","Message-ID":"<20200729091118.GD273308@oden.dyn.berto.se>","References":"<20200728233744.3503740-1-niklas.soderlund@ragnatech.se>\n\t<20200728234225.3505868-1-niklas.soderlund@ragnatech.se>\n\t<20200728234225.3505868-5-niklas.soderlund@ragnatech.se>\n\t<20200729081830.w7oyhcuuxhw5alel@uno.localdomain>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200729081830.w7oyhcuuxhw5alel@uno.localdomain>","Subject":"Re: [libcamera-devel] [PATCH v3 5/5] libcamera: camera_manager:\n\tEnforce unique camera names","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\" "}}]