[{"id":27347,"web_url":"https://patchwork.libcamera.org/comment/27347/","msgid":"","date":"2023-06-15T07:23:11","subject":"Re: [libcamera-devel] [PATCH] pipeline: rpi: Do not return an error\n\tfrom pipeline config file handling","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Naush\n\nOn Wed, Jun 14, 2023 at 01:13:32PM +0100, Naushir Patuck via libcamera-devel wrote:\n> If a user provided pipeline config file is not present, or if the\n> version reported in the file is invalid, do not return with an error\n> when creating the pipeline handler. Instead, log a warning message and\n> return success with default pipeline config values used.\n>\n> This now matches the behaviour when the pipeline config file could not\n> be parsed correctly, and we revert to default values.\n>\n> Signed-off-by: Naushir Patuck \n\nI would have gone for the other way around, and make the Warning an\nError, as if someone specifies a config file, it should be a correct\none.\n\nAnyway, matter of tastes I guess, consistency is what matters here\nReviewed-by: Jacopo Mondi \n\nThanks\n j\n\n> ---\n> src/libcamera/pipeline/rpi/common/pipeline_base.cpp | 10 ++++++----\n> 1 file changed, 6 insertions(+), 4 deletions(-)\n>\n> diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> index 1e20fc2d8cb8..df7482920e75 100644\n> --- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> @@ -1093,8 +1093,9 @@ int CameraData::loadPipelineConfiguration()\n> \tFile file(filename);\n>\n> \tif (!file.open(File::OpenModeFlag::ReadOnly)) {\n> -\t\tLOG(RPI, Error) << \"Failed to open configuration file '\" << filename << \"'\";\n> -\t\treturn -EIO;\n> +\t\tLOG(RPI, Warning) << \"Failed to open configuration file '\" << filename << \"'\"\n> +\t\t\t\t << \", using defaults\";\n> +\t\treturn 0;\n> \t}\n>\n> \tLOG(RPI, Info) << \"Using configuration file '\" << filename << \"'\";\n> @@ -1107,8 +1108,9 @@ int CameraData::loadPipelineConfiguration()\n>\n> \tstd::optional ver = (*root)[\"version\"].get();\n> \tif (!ver || *ver != 1.0) {\n> -\t\tLOG(RPI, Error) << \"Unexpected configuration file version reported\";\n> -\t\treturn -EINVAL;\n> +\t\tLOG(RPI, Warning) << \"Unexpected configuration file version reported: \"\n> +\t\t\t\t << *ver;\n> +\t\treturn 0;\n> \t}\n>\n> \tconst YamlObject &phConfig = (*root)[\"pipeline_handler\"];\n> --\n> 2.34.1\n>","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 CDF9BC31E9\n\tfor ;\n\tThu, 15 Jun 2023 07:23:16 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 21887628B6;\n\tThu, 15 Jun 2023 09:23:16 +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 05D3561E45\n\tfor ;\n\tThu, 15 Jun 2023 09:23:15 +0200 (CEST)","from ideasonboard.com (unknown\n\t[IPv6:2001:b07:5d2e:52c9:72c3:346:a663:c82d])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 90477891;\n\tThu, 15 Jun 2023 09:22:43 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1686813796;\n\tbh=pR9sludCP+evF2ezUJKUXdmRaKt7VEnvciED6cNwbj4=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=YJr1lMq+tCXvcfAITPmMhgpMy5x2RNPWIRdAxZJV+93HaulX+k1/ho9QIB7BasH2a\n\tLMlUBrjHHuyX7Gf+mYxZqIKs6EH8HJosaE8EVqvs9xY4klJKWB5x4hgQ7EyrXwBWtB\n\tSUfUUBfwEl9qdrWiXPFn/ImDMVh2fEIxm8lAKfFBnPBN20nYsJhZiOb6+yq9A2l7Il\n\tA9uAAaWRTGjE8Ssx1NPXkAKCDSDnK+3oy4WLMmNghpefTUm6GCTZ4vPvPHi2IZ2WYf\n\t0mBcfUVuqR6ON/mqKT4uMq8EPOxJNXG/PtEgB7OvH4srOUnfSwU3RAzAbzQ/UFrRCA\n\tOTsfITfCfMCvA==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1686813763;\n\tbh=pR9sludCP+evF2ezUJKUXdmRaKt7VEnvciED6cNwbj4=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=udZ+MhSHUKFyhgiEX1Zoin0K38MAaK5MxtsNShzHBlJKyUROkklkLF8rJLpb/EcFu\n\trvFhNgmpldvB2ZDCOWtdLH6DoNMwgHxNoLT8MgYUep1XVjn5ax4P8pLzTTr2oWZ0Ku\n\tW6Yd5xcokagn1FGHaFUwFV/PlPENMwYCKYsCwdJw="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"udZ+MhSH\"; dkim-atps=neutral","Date":"Thu, 15 Jun 2023 09:23:11 +0200","To":"Naushir Patuck ","Message-ID":"","References":"<20230614121332.15484-1-naush@raspberrypi.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20230614121332.15484-1-naush@raspberrypi.com>","Subject":"Re: [libcamera-devel] [PATCH] pipeline: rpi: Do not return an error\n\tfrom pipeline config file handling","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","From":"Jacopo Mondi via libcamera-devel ","Reply-To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":27353,"web_url":"https://patchwork.libcamera.org/comment/27353/","msgid":"","date":"2023-06-15T11:17:36","subject":"Re: [libcamera-devel] [PATCH] pipeline: rpi: Do not return an error\n\tfrom pipeline config file handling","submitter":{"id":42,"url":"https://patchwork.libcamera.org/api/people/42/","name":"David Plowman","email":"david.plowman@raspberrypi.com"},"content":"Hi Naush\n\nThanks for the patch.\n\nOn Wed, 14 Jun 2023 at 13:13, Naushir Patuck wrote:\n>\n> If a user provided pipeline config file is not present, or if the\n> version reported in the file is invalid, do not return with an error\n> when creating the pipeline handler. Instead, log a warning message and\n> return success with default pipeline config values used.\n>\n> This now matches the behaviour when the pipeline config file could not\n> be parsed correctly, and we revert to default values.\n>\n> Signed-off-by: Naushir Patuck \n\nYes, I think I prefer this too for our users.\n\nReviewed-by: David Plowman \n\nThanks!\nDavid\n\n> ---\n> src/libcamera/pipeline/rpi/common/pipeline_base.cpp | 10 ++++++----\n> 1 file changed, 6 insertions(+), 4 deletions(-)\n>\n> diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> index 1e20fc2d8cb8..df7482920e75 100644\n> --- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> @@ -1093,8 +1093,9 @@ int CameraData::loadPipelineConfiguration()\n> File file(filename);\n>\n> if (!file.open(File::OpenModeFlag::ReadOnly)) {\n> - LOG(RPI, Error) << \"Failed to open configuration file '\" << filename << \"'\";\n> - return -EIO;\n> + LOG(RPI, Warning) << \"Failed to open configuration file '\" << filename << \"'\"\n> + << \", using defaults\";\n> + return 0;\n> }\n>\n> LOG(RPI, Info) << \"Using configuration file '\" << filename << \"'\";\n> @@ -1107,8 +1108,9 @@ int CameraData::loadPipelineConfiguration()\n>\n> std::optional ver = (*root)[\"version\"].get();\n> if (!ver || *ver != 1.0) {\n> - LOG(RPI, Error) << \"Unexpected configuration file version reported\";\n> - return -EINVAL;\n> + LOG(RPI, Warning) << \"Unexpected configuration file version reported: \"\n> + << *ver;\n> + return 0;\n> }\n>\n> const YamlObject &phConfig = (*root)[\"pipeline_handler\"];\n> --\n> 2.34.1\n>","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 0B9ACBD78E\n\tfor ;\n\tThu, 15 Jun 2023 11:17:51 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 79D2761E4F;\n\tThu, 15 Jun 2023 13:17:50 +0200 (CEST)","from mail-oa1-x2a.google.com (mail-oa1-x2a.google.com\n\t[IPv6:2001:4860:4864:20::2a])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id A24E1614FE\n\tfor ;\n\tThu, 15 Jun 2023 13:17:48 +0200 (CEST)","by mail-oa1-x2a.google.com with SMTP id\n\t586e51a60fabf-1a1b95cc10eso6101889fac.0\n\tfor ;\n\tThu, 15 Jun 2023 04:17:48 -0700 (PDT)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1686827870;\n\tbh=RW0nd9iDm6avOKWh9Qbksejd7O6SnhT2IXCnbW8N6dI=;\n\th=References:In-Reply-To:Date:To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=uKstjjIYEgv8W3VfxiWiC1LBimcWz+jNAA2YMgn+8bb/0FpyXCtR5ptCnMEROU4P4\n\tiVw9TGUELVCfPySYNCxnEov7DTc9XeFGX5VPPt0KUeqZ+9rC1MxOGAIu1TvoUozK8F\n\tTYvbtqRzb3askTdBg0K7ccTCO1Bij6TG+C5cVy2XndJVLoxdxIzxjzZrRKeCXze/Us\n\tqfVd6VI1fWVNUauRz37AoIzS1NUHG6hzZU2IBUoVcn4ofD6gqiD2HJTytNeEWAXP55\n\tUTIJXLjP6Nb7oDgI8Ka2uvHQrfx+fxabEg9dejbKJYoA1Bizeutlqn3BX2xnJBbae1\n\teeOpDd50LBJTA==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google; t=1686827867; x=1689419867;\n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:from:to:cc:subject:date:message-id:reply-to;\n\tbh=6Xk2Q/byTdrg+ik1PgIAO7MbIyPwBizypsfpY97Dt98=;\n\tb=Yf08GwdprV2TzCf0qf7L76XpOn9T+nwJDlIu9ainSyjffItNXr8FlZCTMlA/j2eilX\n\tYkFE+1e9IbPDcQPjj5G0GQz+fRwIBDTiW2yeFi3TQ9GYBppxjtIJ0tSNHig8tWAokwnX\n\t41Pk1BMq8qXPvCWDMH8hmG4WYx+py1tx7LRF/MMn8SOcudIqpeNV6lMCvMDt0kwFO4XX\n\t97SMnTm9GL6Rr+rbcAsVGQ5Q04NucIPvUrH9tTCvzC6cRfjlDtdHHZBYUfnQY5jBuUkC\n\tLg1Y7YR9pCWfDOqPts9uoHkpDO+iBgeYMlapdsEukBAfArswIqDh8EH2QjjOemyAVwCD\n\t9F8w=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=raspberrypi.com\n\theader.i=@raspberrypi.com\n\theader.b=\"Yf08Gwdp\"; dkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20221208; t=1686827867; x=1689419867;\n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:x-gm-message-state:from:to:cc:subject:date:message-id\n\t:reply-to;\n\tbh=6Xk2Q/byTdrg+ik1PgIAO7MbIyPwBizypsfpY97Dt98=;\n\tb=jbv4JwZ8O3s8Iv1I7jt/G16+xCWsZOPA+XP16NOD2GbO8xPOPw7pAsKztXRq9DWjzJ\n\tDopoeG1EZ00togF62ibHsOLl/bsALQxQT+qgDQKm/6ZPXVQLFZeMp+OPpHvbtSeEyP/D\n\tg/mB3j8+3SLNLQqHiMXsRPmQSFMyDiX/cYCHY7HQ5WRRjF8XY9Ly7jjLJZnAcpIOhdD0\n\ttSlcRDJXCxDma1XISVSnCr0luZnOD255KorP4NsPsZo2tpqzhlj5J7Y7diZrXwshj6z8\n\tkiNya6OofwpX7qIH59mMjkpbx7lJNGRTQE8opkfy786t13LHB36PDH72i/hU+jOJkdv9\n\ti5Ew==","X-Gm-Message-State":"AC+VfDw4JFkAaw5HCcR0vkNk6uQs2V79i1Y6xW3Vz1hfuFBbCvqbLlcn\n\tHLPlazDYF5OJijcMPCCGdIn+QWmT5SOfrzPxcJVuRg1NdTyRjbugNCc=","X-Google-Smtp-Source":"ACHHUZ5AGe4w/H9BFNt/ymdElQRTa+/C8x49/EdV79ysiexYF/rVbnWd3MjbYW+5R1QnZNBinZwPSBGm2bDIJUMCBJw=","X-Received":"by 2002:a05:6871:4c3:b0:19f:4dc2:428e with SMTP id\n\tn3-20020a05687104c300b0019f4dc2428emr13569847oai.14.1686827867354;\n\tThu, 15 Jun 2023 04:17:47 -0700 (PDT)","MIME-Version":"1.0","References":"<20230614121332.15484-1-naush@raspberrypi.com>","In-Reply-To":"<20230614121332.15484-1-naush@raspberrypi.com>","Date":"Thu, 15 Jun 2023 12:17:36 +0100","Message-ID":"","To":"Naushir Patuck ","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH] pipeline: rpi: Do not return an error\n\tfrom pipeline config file handling","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","From":"David Plowman via libcamera-devel ","Reply-To":"David Plowman ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]