[{"id":27299,"web_url":"https://patchwork.libcamera.org/comment/27299/","msgid":"<168613694925.2889415.11201906332109191974@Monstersaurus>","date":"2023-06-07T11:22:29","subject":"Re: [libcamera-devel] [PATCH v1 3/3] ipa: rpi: agc: Gracefully\n\thandle missing agc modes","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Naushir Patuck via libcamera-devel (2023-06-07 11:00:54)\n> If a metering/exposure/constraint mode is not listed in the sensor\n> tuning file, and a control for the missing mode is set on the agc, we\n> terminate the application with a fatal log message.\n> \n> Instead of this fatal termination, log a warning message and switch to\n> the appropriate default mode so that the application continues running.\n\nI think this will improve the user experience better, at least it goes\nfrom \"Oh ... the program crashed\" to \"Oh the program didn't give me the\nimage settings I expected\" ...\n\nAs long as the actually applied configuration is reported in the\nmetadata, I think that's fine and reasonable - and the warnings here\nwill help someone when they investigate the 'why'.\n\n\n\n> \n> Reported-on: https://github.com/raspberrypi/libcamera/issues/59\n> Reported-on: https://github.com/ayufan/camera-streamer/issues/67\n> Signed-off-by: Naushir Patuck \n> ---\n> src/ipa/rpi/controller/rpi/agc.cpp | 30 +++++++++++++++++++++---------\n> 1 file changed, 21 insertions(+), 9 deletions(-)\n> \n> diff --git a/src/ipa/rpi/controller/rpi/agc.cpp b/src/ipa/rpi/controller/rpi/agc.cpp\n> index b611157af1f0..1b05d478818e 100644\n> --- a/src/ipa/rpi/controller/rpi/agc.cpp\n> +++ b/src/ipa/rpi/controller/rpi/agc.cpp\n> @@ -540,24 +540,36 @@ void Agc::housekeepConfig()\n> */\n> if (meteringModeName_ != status_.meteringMode) {\n> auto it = config_.meteringModes.find(meteringModeName_);\n> - if (it == config_.meteringModes.end())\n> - LOG(RPiAgc, Fatal) << \"No metering mode \" << meteringModeName_;\n> - meteringMode_ = &it->second;\n> + if (it == config_.meteringModes.end()) {\n> + LOG(RPiAgc, Warning) << \"No metering mode \" << meteringModeName_\n> + << \", defaulting to \" << config_.defaultMeteringMode;\n\nTaking the default here seems like a good fallback. I also wonder if we\nshould instead take the settings from uncalibrated.json. But that may be\nmore tricky to get the data in - and would still need a fallback in case\n*that* couldn't be found!.\n\nSo this seems like a good step forwards to me.\n\n\nReviewed-by: Kieran Bingham \n\n> + meteringModeName_ = config_.defaultMeteringMode;\n> + meteringMode_ = &config_.meteringModes[meteringModeName_];\n> + } else\n> + meteringMode_ = &it->second;\n> status_.meteringMode = meteringModeName_;\n> }\n> if (exposureModeName_ != status_.exposureMode) {\n> auto it = config_.exposureModes.find(exposureModeName_);\n> - if (it == config_.exposureModes.end())\n> - LOG(RPiAgc, Fatal) << \"No exposure profile \" << exposureModeName_;\n> - exposureMode_ = &it->second;\n> + if (it == config_.exposureModes.end()) {\n> + LOG(RPiAgc, Warning) << \"No exposure profile \" << exposureModeName_\n> + << \", defaulting to \" << config_.defaultExposureMode;\n> + exposureModeName_ = config_.defaultExposureMode;\n> + exposureMode_ = &config_.exposureModes[exposureModeName_];\n> + } else\n> + exposureMode_ = &it->second;\n> status_.exposureMode = exposureModeName_;\n> }\n> if (constraintModeName_ != status_.constraintMode) {\n> auto it =\n> config_.constraintModes.find(constraintModeName_);\n> - if (it == config_.constraintModes.end())\n> - LOG(RPiAgc, Fatal) << \"No constraint list \" << constraintModeName_;\n> - constraintMode_ = &it->second;\n> + if (it == config_.constraintModes.end()) {\n> + LOG(RPiAgc, Warning) << \"No constraint list \" << constraintModeName_\n> + << \", defaulting to \" << config_.defaultConstraintMode;\n> + constraintModeName_ = config_.defaultConstraintMode;\n> + constraintMode_ = &config_.constraintModes[constraintModeName_];\n> + } else\n> + constraintMode_ = &it->second;\n> status_.constraintMode = constraintModeName_;\n> }\n> LOG(RPiAgc, Debug) << \"exposureMode \"\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 DF98DC31E9\n\tfor ;\n\tWed, 7 Jun 2023 11:22:34 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5F13362890;\n\tWed, 7 Jun 2023 13:22:34 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 1ED7762886\n\tfor ;\n\tWed, 7 Jun 2023 13:22:33 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(aztw-30-b2-v4wan-166917-cust845.vm26.cable.virginm.net\n\t[82.37.23.78])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 1231A2B6;\n\tWed, 7 Jun 2023 13:22:07 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1686136954;\n\tbh=nCg5VbPPUO7viH1FFn+ECphl2OHVWbtwN5iOSfaS60Y=;\n\th=In-Reply-To:References:To:Date:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:\n\tFrom;\n\tb=pdGz9qsjf62Eovcsm2Tlfey8e9152FFQhmdHfaQU7iYEKCAkGOGvAgpl4pcCOrvw/\n\tVlBpY+4+h35+uJglKDg6lOMzWlx6WyhnMlkzruWv6dzabuwOoOOUb0MJrjSoE0IO4f\n\t5keUiDoIRt1zSiLkq5tvOEEsCsPEsm0rfCSt7+XCAXTiNWh5CY5udppFbC3Y8Yq0hO\n\tx8BgYhyC0de9uuSi39KiyW+HuU9GVkHfOzDXKBpF+07VPdQlwiLCrnJnQ5v9s6fvMZ\n\tddOrh2BsnqN6C3/l3Z8CFlvzvwltXQF38pAmXi6bmDoTqEgJ577rBXq8C2Fbc7bAVE\n\tL2KQ5N5Twq/uQ==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1686136927;\n\tbh=nCg5VbPPUO7viH1FFn+ECphl2OHVWbtwN5iOSfaS60Y=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=n643xxbmZZA7NLCDHzwIeh/QMrsgRp3JjkbYmPnd9frvyS4j2dmqHxUW4oGqjA5VN\n\tZs3P6Farw2InjQbzV1jBfciMURJLeK6Nl3zwqrEmxRWdm5xmk/ATbAPzZr7GfroPpA\n\tF/VIWMWRDmRRjvXcada5GZd35xeXeWwG+OGBzKtI="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"n643xxbm\"; dkim-atps=neutral","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20230607100054.4576-4-naush@raspberrypi.com>","References":"<20230607100054.4576-1-naush@raspberrypi.com>\n\t<20230607100054.4576-4-naush@raspberrypi.com>","To":"Naushir Patuck ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Wed, 07 Jun 2023 12:22:29 +0100","Message-ID":"<168613694925.2889415.11201906332109191974@Monstersaurus>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH v1 3/3] ipa: rpi: agc: Gracefully\n\thandle missing agc modes","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":"Kieran Bingham via libcamera-devel\n\t","Reply-To":"Kieran Bingham ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":27305,"web_url":"https://patchwork.libcamera.org/comment/27305/","msgid":"<20230607151059.GE22127@pendragon.ideasonboard.com>","date":"2023-06-07T15:10:59","subject":"Re: [libcamera-devel] [PATCH v1 3/3] ipa: rpi: agc: Gracefully\n\thandle missing agc modes","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Naush,\n\nThank you for the patch.\n\nOn Wed, Jun 07, 2023 at 11:00:54AM +0100, Naushir Patuck via libcamera-devel wrote:\n> If a metering/exposure/constraint mode is not listed in the sensor\n> tuning file, and a control for the missing mode is set on the agc, we\n> terminate the application with a fatal log message.\n> \n> Instead of this fatal termination, log a warning message and switch to\n> the appropriate default mode so that the application continues running.\n> \n> Reported-on: https://github.com/raspberrypi/libcamera/issues/59\n> Reported-on: https://github.com/ayufan/camera-streamer/issues/67\n\nWe haven't used \"Reported-on\" before, and grepping in the whole Linux\nkernel history there's a single occurrence. The tags used in the kernel\nwith github issues links are\n\n 1 Ref.\n 1 regression\n 1 Reported-on\n 1 see\n 1 URL\n 2 Issue\n 2 link\n 2 Ref\n 3 Buglink\n 3 Related\n 3 Reported-by\n 3 Resolves\n 5 Bug\n 5 References\n 6 Fixes\n 7 Reference\n 17 Addresses-KSPP-ID\n 43 See\n 89 Closes\n 158 BugLink\n 1426 Link\n\nWe don't use BugLink and have used Link once only to refer to a mailing\nlist entry, while we use Bug for bugzilla entries. I would vote for Bug\nor Link in this case.\n\n> Signed-off-by: Naushir Patuck \n> ---\n> src/ipa/rpi/controller/rpi/agc.cpp | 30 +++++++++++++++++++++---------\n> 1 file changed, 21 insertions(+), 9 deletions(-)\n> \n> diff --git a/src/ipa/rpi/controller/rpi/agc.cpp b/src/ipa/rpi/controller/rpi/agc.cpp\n> index b611157af1f0..1b05d478818e 100644\n> --- a/src/ipa/rpi/controller/rpi/agc.cpp\n> +++ b/src/ipa/rpi/controller/rpi/agc.cpp\n> @@ -540,24 +540,36 @@ void Agc::housekeepConfig()\n> \t */\n> \tif (meteringModeName_ != status_.meteringMode) {\n> \t\tauto it = config_.meteringModes.find(meteringModeName_);\n> -\t\tif (it == config_.meteringModes.end())\n> -\t\t\tLOG(RPiAgc, Fatal) << \"No metering mode \" << meteringModeName_;\n> -\t\tmeteringMode_ = &it->second;\n> +\t\tif (it == config_.meteringModes.end()) {\n> +\t\t\tLOG(RPiAgc, Warning) << \"No metering mode \" << meteringModeName_\n> +\t\t\t\t\t << \", defaulting to \" << config_.defaultMeteringMode;\n> +\t\t\tmeteringModeName_ = config_.defaultMeteringMode;\n> +\t\t\tmeteringMode_ = &config_.meteringModes[meteringModeName_];\n> +\t\t} else\n> +\t\t\tmeteringMode_ = &it->second;\n\nPlease use curly braces here and below. This can be updated when\napplying the patch, no need to resubmit.\n\nReviewed-by: Laurent Pinchart \n\n> \t\tstatus_.meteringMode = meteringModeName_;\n> \t}\n> \tif (exposureModeName_ != status_.exposureMode) {\n> \t\tauto it = config_.exposureModes.find(exposureModeName_);\n> -\t\tif (it == config_.exposureModes.end())\n> -\t\t\tLOG(RPiAgc, Fatal) << \"No exposure profile \" << exposureModeName_;\n> -\t\texposureMode_ = &it->second;\n> +\t\tif (it == config_.exposureModes.end()) {\n> +\t\t\tLOG(RPiAgc, Warning) << \"No exposure profile \" << exposureModeName_\n> +\t\t\t\t\t << \", defaulting to \" << config_.defaultExposureMode;\n> +\t\t\texposureModeName_ = config_.defaultExposureMode;\n> +\t\t\texposureMode_ = &config_.exposureModes[exposureModeName_];\n> +\t\t} else\n> +\t\t\texposureMode_ = &it->second;\n> \t\tstatus_.exposureMode = exposureModeName_;\n> \t}\n> \tif (constraintModeName_ != status_.constraintMode) {\n> \t\tauto it =\n> \t\t\tconfig_.constraintModes.find(constraintModeName_);\n> -\t\tif (it == config_.constraintModes.end())\n> -\t\t\tLOG(RPiAgc, Fatal) << \"No constraint list \" << constraintModeName_;\n> -\t\tconstraintMode_ = &it->second;\n> +\t\tif (it == config_.constraintModes.end()) {\n> +\t\t\tLOG(RPiAgc, Warning) << \"No constraint list \" << constraintModeName_\n> +\t\t\t\t\t << \", defaulting to \" << config_.defaultConstraintMode;\n> +\t\t\tconstraintModeName_ = config_.defaultConstraintMode;\n> +\t\t\tconstraintMode_ = &config_.constraintModes[constraintModeName_];\n> +\t\t} else\n> +\t\t\tconstraintMode_ = &it->second;\n> \t\tstatus_.constraintMode = constraintModeName_;\n> \t}\n> \tLOG(RPiAgc, Debug) << \"exposureMode \"","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 06CF6C31E9\n\tfor ;\n\tWed, 7 Jun 2023 15:11:06 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 70B8F62890;\n\tWed, 7 Jun 2023 17:11:05 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 1DFA962886\n\tfor ;\n\tWed, 7 Jun 2023 17:11:03 +0200 (CEST)","from pendragon.ideasonboard.com (om126233170111.36.openmobile.ne.jp\n\t[126.233.170.111])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 245382B6;\n\tWed, 7 Jun 2023 17:10:35 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1686150665;\n\tbh=LCy/fPIcwCXyvrQfZzTsi0fHS1oCVUpEei3DKln8m5g=;\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=V1PWy94RBXs4ZnmPkFhSyYnEwSB4cgszmD2voeD60u557kpgzSm5/OcTfKjD99qE6\n\tBL2DAdtbjOCJxem4ietLGUsuNSYo6mvH34UPgs8lfFd6hAGdDz8ML1jiq0Gu4y3Inr\n\tQgN6ap1up+JiCgP2lErJRmbxojrrLepES6/a9gzfk80aaa3/xCv5G3xgVvHJe8ip2c\n\ttaqEWmXsmjYH+LDfEGSpxz1CWnACY4oHY3vD7vNB+X4+9offzWpbnTEY3PbavtUkq4\n\tvVgeLiAh1JyWyLZj6C+4E/xemppVuBDd3fNImOy8A+yu9NyYxvWtq1qSRiggukMhNV\n\tTKpp03sTaWtoQ==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1686150637;\n\tbh=LCy/fPIcwCXyvrQfZzTsi0fHS1oCVUpEei3DKln8m5g=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=AjlyeW+gGARDI3MyWZTuMLPOOQVs0NqVr0obx7HYT46hL3wv3YIzJMHc8Ph/Ait2f\n\t8wJ7f8LPrb0n1jnQEzX0eoDYd9/N+6zR23uU+LyIXsNZxMWG7bevOIHgwBa7AxQjtC\n\tgv9nj3XR9uHawvwohYcPT83WA9qXl4conuqwoUuU="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"AjlyeW+g\"; dkim-atps=neutral","Date":"Wed, 7 Jun 2023 18:10:59 +0300","To":"Naushir Patuck ","Message-ID":"<20230607151059.GE22127@pendragon.ideasonboard.com>","References":"<20230607100054.4576-1-naush@raspberrypi.com>\n\t<20230607100054.4576-4-naush@raspberrypi.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20230607100054.4576-4-naush@raspberrypi.com>","Subject":"Re: [libcamera-devel] [PATCH v1 3/3] ipa: rpi: agc: Gracefully\n\thandle missing agc modes","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":"Laurent Pinchart via libcamera-devel\n\t","Reply-To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":27309,"web_url":"https://patchwork.libcamera.org/comment/27309/","msgid":"<168616781019.1159722.3914381123768662632@Monstersaurus>","date":"2023-06-07T19:56:50","subject":"Re: [libcamera-devel] [PATCH v1 3/3] ipa: rpi: agc: Gracefully\n\thandle missing agc modes","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Laurent Pinchart via libcamera-devel (2023-06-07 16:10:59)\n> Hi Naush,\n> \n> Thank you for the patch.\n> \n> On Wed, Jun 07, 2023 at 11:00:54AM +0100, Naushir Patuck via libcamera-devel wrote:\n> > If a metering/exposure/constraint mode is not listed in the sensor\n> > tuning file, and a control for the missing mode is set on the agc, we\n> > terminate the application with a fatal log message.\n> > \n> > Instead of this fatal termination, log a warning message and switch to\n> > the appropriate default mode so that the application continues running.\n> > \n> > Reported-on: https://github.com/raspberrypi/libcamera/issues/59\n> > Reported-on: https://github.com/ayufan/camera-streamer/issues/67\n> \n> We haven't used \"Reported-on\" before, and grepping in the whole Linux\n\nErr ... we have:\n\n2a261d911f50 (\"pipeline: raspberrypi: Iterate over all Unicam instances in match()\")\n\nOther references we've used for github issues include:\n\n Reported-on: https://github.com/raspberrypi/libcamera/issues/59\n Reported-on: https://github.com/ayufan/camera-streamer/issues/67\n Reported-on: https://github.com/raspberrypi/libcamera/issues/44\n Bug: https://github.com/raspberrypi/libcamera/issues/29\n Reported-by: https://github.com/kralo\n Bug: https://github.com/raspberrypi/libcamera-apps/issues/217\n Bug: https://github.com/raspberrypi/libcamera-apps/issues/236\n Bug: https://github.com/raspberrypi/libcamera-apps/issues/238\n Bug: https://github.com/raspberrypi/libcamera-apps/issues/240\n See https://github.com/PyCQA/pycodestyle/issues/466\n\nI think Bug: is the most appropriate still, so I'll use that.\n\n\n> kernel history there's a single occurrence. The tags used in the kernel\n> with github issues links are\n> \n> 1 Ref.\n> 1 regression\n> 1 Reported-on\n> 1 see\n> 1 URL\n> 2 Issue\n> 2 link\n> 2 Ref\n> 3 Buglink\n> 3 Related\n> 3 Reported-by\n> 3 Resolves\n> 5 Bug\n> 5 References\n> 6 Fixes\n> 7 Reference\n> 17 Addresses-KSPP-ID\n> 43 See\n> 89 Closes\n> 158 BugLink\n> 1426 Link\n> \n> We don't use BugLink and have used Link once only to refer to a mailing\n> list entry, while we use Bug for bugzilla entries. I would vote for Bug\n> or Link in this case.\n> \n> > Signed-off-by: Naushir Patuck \n> > ---\n> > src/ipa/rpi/controller/rpi/agc.cpp | 30 +++++++++++++++++++++---------\n> > 1 file changed, 21 insertions(+), 9 deletions(-)\n> > \n> > diff --git a/src/ipa/rpi/controller/rpi/agc.cpp b/src/ipa/rpi/controller/rpi/agc.cpp\n> > index b611157af1f0..1b05d478818e 100644\n> > --- a/src/ipa/rpi/controller/rpi/agc.cpp\n> > +++ b/src/ipa/rpi/controller/rpi/agc.cpp\n> > @@ -540,24 +540,36 @@ void Agc::housekeepConfig()\n> > */\n> > if (meteringModeName_ != status_.meteringMode) {\n> > auto it = config_.meteringModes.find(meteringModeName_);\n> > - if (it == config_.meteringModes.end())\n> > - LOG(RPiAgc, Fatal) << \"No metering mode \" << meteringModeName_;\n> > - meteringMode_ = &it->second;\n> > + if (it == config_.meteringModes.end()) {\n> > + LOG(RPiAgc, Warning) << \"No metering mode \" << meteringModeName_\n> > + << \", defaulting to \" << config_.defaultMeteringMode;\n> > + meteringModeName_ = config_.defaultMeteringMode;\n> > + meteringMode_ = &config_.meteringModes[meteringModeName_];\n> > + } else\n> > + meteringMode_ = &it->second;\n> \n> Please use curly braces here and below. This can be updated when\n> applying the patch, no need to resubmit.\n\nUpdated, and running through the integration tests.\n\n--\nKieran\n\n\n> \n> Reviewed-by: Laurent Pinchart \n> \n> > status_.meteringMode = meteringModeName_;\n> > }\n> > if (exposureModeName_ != status_.exposureMode) {\n> > auto it = config_.exposureModes.find(exposureModeName_);\n> > - if (it == config_.exposureModes.end())\n> > - LOG(RPiAgc, Fatal) << \"No exposure profile \" << exposureModeName_;\n> > - exposureMode_ = &it->second;\n> > + if (it == config_.exposureModes.end()) {\n> > + LOG(RPiAgc, Warning) << \"No exposure profile \" << exposureModeName_\n> > + << \", defaulting to \" << config_.defaultExposureMode;\n> > + exposureModeName_ = config_.defaultExposureMode;\n> > + exposureMode_ = &config_.exposureModes[exposureModeName_];\n> > + } else\n> > + exposureMode_ = &it->second;\n> > status_.exposureMode = exposureModeName_;\n> > }\n> > if (constraintModeName_ != status_.constraintMode) {\n> > auto it =\n> > config_.constraintModes.find(constraintModeName_);\n> > - if (it == config_.constraintModes.end())\n> > - LOG(RPiAgc, Fatal) << \"No constraint list \" << constraintModeName_;\n> > - constraintMode_ = &it->second;\n> > + if (it == config_.constraintModes.end()) {\n> > + LOG(RPiAgc, Warning) << \"No constraint list \" << constraintModeName_\n> > + << \", defaulting to \" << config_.defaultConstraintMode;\n> > + constraintModeName_ = config_.defaultConstraintMode;\n> > + constraintMode_ = &config_.constraintModes[constraintModeName_];\n> > + } else\n> > + constraintMode_ = &it->second;\n> > status_.constraintMode = constraintModeName_;\n> > }\n> > LOG(RPiAgc, Debug) << \"exposureMode \"\n> \n> -- \n> Regards,\n> \n> Laurent Pinchart","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 8C698C31E9\n\tfor ;\n\tWed, 7 Jun 2023 19:56:54 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 0CFB360A16;\n\tWed, 7 Jun 2023 21:56:54 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 4AB6C609FF\n\tfor ;\n\tWed, 7 Jun 2023 21:56:53 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(aztw-30-b2-v4wan-166917-cust845.vm26.cable.virginm.net\n\t[82.37.23.78])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id E494074C;\n\tWed, 7 Jun 2023 21:56:26 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1686167814;\n\tbh=P+WFZXDwttyCjAiEAsd8Pg0zu17i3qiS3MWpxg8d7Jk=;\n\th=In-Reply-To:References:To:Date:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=IT2cOJV/b1Sp1DnBOTEimICKl3ptA45C1fpjdhkdxuch61kx9dBF2vD9f+Rgycr54\n\tH2tg6qlIFASxw+TMQxIdgkav9AeCrbBnESG3TrxC6PMfo6MCeHC6ItSOsBti/dcLvG\n\tIV3NqoDltgnBXzhwrU8CWiE3SfnhAQyrduXkOkAAV2BfKLbULpZDxmAmUGVVUbOZaV\n\t/Gw2pw0zf55iQDoZpkJbq0+22g1BZJ3KBDF2BQV17JrUO917RkWsPJnRQ2k6OLGQX6\n\tvo6Tc4pieCy+aWd+3nEo5bAQwA2yf9wcnOGBrA1feKG0FEmCkkE2d8Vb8DuhOW91xQ\n\tBfwqscAxr9pOQ==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1686167787;\n\tbh=P+WFZXDwttyCjAiEAsd8Pg0zu17i3qiS3MWpxg8d7Jk=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=UZaIwaW4l9NIyAT+LDp7YzlSukm73Xy6889Y7DULoKkSD3pKvBc7gKsROBtU/h2sM\n\tM4e69eeZXkcWVxhwDJboB+YAdHdt+Wu0M9aHiSNnVKTME0tw5fTlHvhkS5OtvD6prH\n\tGSBJ/LgQ6xZkMWyvbHb8obzYNYpS1B3SkWoa8NbU="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"UZaIwaW4\"; dkim-atps=neutral","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20230607151059.GE22127@pendragon.ideasonboard.com>","References":"<20230607100054.4576-1-naush@raspberrypi.com>\n\t<20230607100054.4576-4-naush@raspberrypi.com>\n\t<20230607151059.GE22127@pendragon.ideasonboard.com>","To":"Laurent Pinchart ,\n\tLaurent Pinchart via libcamera-devel\n\t, \n\tNaushir Patuck ","Date":"Wed, 07 Jun 2023 20:56:50 +0100","Message-ID":"<168616781019.1159722.3914381123768662632@Monstersaurus>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH v1 3/3] ipa: rpi: agc: Gracefully\n\thandle missing agc modes","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":"Kieran Bingham via libcamera-devel\n\t","Reply-To":"Kieran Bingham ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":27315,"web_url":"https://patchwork.libcamera.org/comment/27315/","msgid":"","date":"2023-06-12T08:54:35","subject":"Re: [libcamera-devel] [PATCH v1 3/3] ipa: rpi: agc: Gracefully\n\thandle missing agc modes","submitter":{"id":42,"url":"https://patchwork.libcamera.org/api/people/42/","name":"David Plowman","email":"david.plowman@raspberrypi.com"},"content":"Hi everyone\n\nI think this is a good idea. The only question I have is whether we\nmight simply print a warning and do nothing, rather than change to the\ndefault mode.\n\nI think this is a slightly less surprising behaviour - if you make a\nmistake it has no effect, rather than change you to some possibly\nunrelated mode that you might even have to undo again. It's also a\ngeneral rule that could be applied to all controls: ask for something\nthat can't be done and it tells you and does nothing.\n\nThoughts?\n\nThanks!\nDavid\n\nOn Wed, 7 Jun 2023 at 20:56, Kieran Bingham via libcamera-devel\n wrote:\n>\n> Quoting Laurent Pinchart via libcamera-devel (2023-06-07 16:10:59)\n> > Hi Naush,\n> >\n> > Thank you for the patch.\n> >\n> > On Wed, Jun 07, 2023 at 11:00:54AM +0100, Naushir Patuck via libcamera-devel wrote:\n> > > If a metering/exposure/constraint mode is not listed in the sensor\n> > > tuning file, and a control for the missing mode is set on the agc, we\n> > > terminate the application with a fatal log message.\n> > >\n> > > Instead of this fatal termination, log a warning message and switch to\n> > > the appropriate default mode so that the application continues running.\n> > >\n> > > Reported-on: https://github.com/raspberrypi/libcamera/issues/59\n> > > Reported-on: https://github.com/ayufan/camera-streamer/issues/67\n> >\n> > We haven't used \"Reported-on\" before, and grepping in the whole Linux\n>\n> Err ... we have:\n>\n> 2a261d911f50 (\"pipeline: raspberrypi: Iterate over all Unicam instances in match()\")\n>\n> Other references we've used for github issues include:\n>\n> Reported-on: https://github.com/raspberrypi/libcamera/issues/59\n> Reported-on: https://github.com/ayufan/camera-streamer/issues/67\n> Reported-on: https://github.com/raspberrypi/libcamera/issues/44\n> Bug: https://github.com/raspberrypi/libcamera/issues/29\n> Reported-by: https://github.com/kralo\n> Bug: https://github.com/raspberrypi/libcamera-apps/issues/217\n> Bug: https://github.com/raspberrypi/libcamera-apps/issues/236\n> Bug: https://github.com/raspberrypi/libcamera-apps/issues/238\n> Bug: https://github.com/raspberrypi/libcamera-apps/issues/240\n> See https://github.com/PyCQA/pycodestyle/issues/466\n>\n> I think Bug: is the most appropriate still, so I'll use that.\n>\n>\n> > kernel history there's a single occurrence. The tags used in the kernel\n> > with github issues links are\n> >\n> > 1 Ref.\n> > 1 regression\n> > 1 Reported-on\n> > 1 see\n> > 1 URL\n> > 2 Issue\n> > 2 link\n> > 2 Ref\n> > 3 Buglink\n> > 3 Related\n> > 3 Reported-by\n> > 3 Resolves\n> > 5 Bug\n> > 5 References\n> > 6 Fixes\n> > 7 Reference\n> > 17 Addresses-KSPP-ID\n> > 43 See\n> > 89 Closes\n> > 158 BugLink\n> > 1426 Link\n> >\n> > We don't use BugLink and have used Link once only to refer to a mailing\n> > list entry, while we use Bug for bugzilla entries. I would vote for Bug\n> > or Link in this case.\n> >\n> > > Signed-off-by: Naushir Patuck \n> > > ---\n> > > src/ipa/rpi/controller/rpi/agc.cpp | 30 +++++++++++++++++++++---------\n> > > 1 file changed, 21 insertions(+), 9 deletions(-)\n> > >\n> > > diff --git a/src/ipa/rpi/controller/rpi/agc.cpp b/src/ipa/rpi/controller/rpi/agc.cpp\n> > > index b611157af1f0..1b05d478818e 100644\n> > > --- a/src/ipa/rpi/controller/rpi/agc.cpp\n> > > +++ b/src/ipa/rpi/controller/rpi/agc.cpp\n> > > @@ -540,24 +540,36 @@ void Agc::housekeepConfig()\n> > > */\n> > > if (meteringModeName_ != status_.meteringMode) {\n> > > auto it = config_.meteringModes.find(meteringModeName_);\n> > > - if (it == config_.meteringModes.end())\n> > > - LOG(RPiAgc, Fatal) << \"No metering mode \" << meteringModeName_;\n> > > - meteringMode_ = &it->second;\n> > > + if (it == config_.meteringModes.end()) {\n> > > + LOG(RPiAgc, Warning) << \"No metering mode \" << meteringModeName_\n> > > + << \", defaulting to \" << config_.defaultMeteringMode;\n> > > + meteringModeName_ = config_.defaultMeteringMode;\n> > > + meteringMode_ = &config_.meteringModes[meteringModeName_];\n> > > + } else\n> > > + meteringMode_ = &it->second;\n> >\n> > Please use curly braces here and below. This can be updated when\n> > applying the patch, no need to resubmit.\n>\n> Updated, and running through the integration tests.\n>\n> --\n> Kieran\n>\n>\n> >\n> > Reviewed-by: Laurent Pinchart \n> >\n> > > status_.meteringMode = meteringModeName_;\n> > > }\n> > > if (exposureModeName_ != status_.exposureMode) {\n> > > auto it = config_.exposureModes.find(exposureModeName_);\n> > > - if (it == config_.exposureModes.end())\n> > > - LOG(RPiAgc, Fatal) << \"No exposure profile \" << exposureModeName_;\n> > > - exposureMode_ = &it->second;\n> > > + if (it == config_.exposureModes.end()) {\n> > > + LOG(RPiAgc, Warning) << \"No exposure profile \" << exposureModeName_\n> > > + << \", defaulting to \" << config_.defaultExposureMode;\n> > > + exposureModeName_ = config_.defaultExposureMode;\n> > > + exposureMode_ = &config_.exposureModes[exposureModeName_];\n> > > + } else\n> > > + exposureMode_ = &it->second;\n> > > status_.exposureMode = exposureModeName_;\n> > > }\n> > > if (constraintModeName_ != status_.constraintMode) {\n> > > auto it =\n> > > config_.constraintModes.find(constraintModeName_);\n> > > - if (it == config_.constraintModes.end())\n> > > - LOG(RPiAgc, Fatal) << \"No constraint list \" << constraintModeName_;\n> > > - constraintMode_ = &it->second;\n> > > + if (it == config_.constraintModes.end()) {\n> > > + LOG(RPiAgc, Warning) << \"No constraint list \" << constraintModeName_\n> > > + << \", defaulting to \" << config_.defaultConstraintMode;\n> > > + constraintModeName_ = config_.defaultConstraintMode;\n> > > + constraintMode_ = &config_.constraintModes[constraintModeName_];\n> > > + } else\n> > > + constraintMode_ = &it->second;\n> > > status_.constraintMode = constraintModeName_;\n> > > }\n> > > LOG(RPiAgc, Debug) << \"exposureMode \"\n> >\n> > --\n> > Regards,\n> >\n> > Laurent Pinchart","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 ECF1EC31E9\n\tfor ;\n\tMon, 12 Jun 2023 08:54:50 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 2E02E61E49;\n\tMon, 12 Jun 2023 10:54:50 +0200 (CEST)","from mail-oa1-x2c.google.com (mail-oa1-x2c.google.com\n\t[IPv6:2001:4860:4864:20::2c])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 5699261E48\n\tfor ;\n\tMon, 12 Jun 2023 10:54:47 +0200 (CEST)","by mail-oa1-x2c.google.com with SMTP id\n\t586e51a60fabf-1a5229df1f2so2274621fac.3\n\tfor ;\n\tMon, 12 Jun 2023 01:54:47 -0700 (PDT)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1686560090;\n\tbh=CoDSaa5uc/yxqhYlt3rXqMUCZVkMpxnqeYT6W7dear4=;\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=rczoDZ0mLBz97lXoB6M0Fk84zCa/d43xeQ97FFxfYRHzV+YZ01D57jlZpaRDJl373\n\t6e+KdTRC4f3v6yAWd75H7DXRaK9pKKPrTJESyCvG4e9g/1OK+2cFuGwhZPr2E93M1L\n\t4QOiQBum0tApPgZ9kQPHtF5FhakByD3qrGTGLhTa6ozCxyxZp5pMmb1IXGJ6CaQ+ac\n\t0VAUg3tpoh9YcTiX/7ynj4fwlZyy8uGDDwK1KsvynuIq/noDBoOQw5C8/BhxfG7jIM\n\tHF4JbeYBrrbbJZeMfU10BgWltSoOaePYjS2g2q8OF+oVOzcoJlmw8vc96YX9j5n8h4\n\twyxNEdjLsA/og==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google; t=1686560086; x=1689152086;\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=PyJqW3aPGMxS4bYYst3p7H35TCmnWmDhlm7mzkq6Pkc=;\n\tb=Y9C5WyYJHjlnMOpw2SrPBL53wHZwT5/itMUkfs6XN8Z+N8kcgpHRFPNW/0JkqkmInN\n\tjlR1k516620ild65PrYJFwBXhPBsbjc1Dty0XVnmkvpQud2se+NS9eWGYJHCoHXD5Qjh\n\ttuQljFfBHuEcn1hefEfS2Y/F1K0n5/B/Esbd0It8DIlsVvX5++9Azy5ni99R7oCpAkM/\n\tC5Gm1aUa3iWIVhBRMz6qEKfsihMjB9ytdPDIYJ3Dbv4nT7rDo4D0i350lOEWFRR9wZ38\n\tsptir4GP4XT2xA2Ach4LTqufvmocMe0XYaYAtIt226M2Q9cVZOPhiIRVHCEdH+944CRX\n\tu+YA=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=raspberrypi.com\n\theader.i=@raspberrypi.com\n\theader.b=\"Y9C5WyYJ\"; dkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20221208; t=1686560086; x=1689152086;\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=PyJqW3aPGMxS4bYYst3p7H35TCmnWmDhlm7mzkq6Pkc=;\n\tb=CoE9aD/6pPJ6NLH7NU0KqTdO4oMQsA7lXPTC/npy00Q3GhawpBooatB1w2qLUJBPaR\n\t/ytlL2C5iqlt6vl3OcGvHFC3n4sLHVTDaZRpj3Xj8e+wB6DpNNU+YtP9lEG6bXtMx0om\n\tQUxyn3Kespra7QS7cl3Oo1aP9FYHbZ/pVbycrtln8i2DsobUFSni7SjFzJiHzM2N4Wd6\n\tPEPkdifm0AdJTX5O17jyfn2FoX363UztQaepwMt4lSmxhXZUIpUNEy8Zj5JUbYhYOqWa\n\tsb78Th/Y91vf3Vm/v2ornxyZNQTfYrd8EffyqD2d9gbGmtvTb6ObP6OsADgrcor7jenp\n\t3sAw==","X-Gm-Message-State":"AC+VfDx9zzPRLh0cbXeC9ZgRsmKbS1/DduWqBDm92p4MpW3qX5+3/qGY\n\tZZ2IC6ZSIDeHDpx0nTyY9gjbDv0vJhZnM3JS5JzrAw==","X-Google-Smtp-Source":"ACHHUZ7yLI10712GjH7wXujtM4aUUDvbrncbSgFKaJ2bRvEtAP3kgVldbqxzg+P7eRLA0Xg8wRyKk2CJlSpyzadopn0=","X-Received":"by 2002:a05:6870:42c6:b0:1a3:74d:cdc7 with SMTP id\n\tz6-20020a05687042c600b001a3074dcdc7mr5568098oah.5.1686560085957;\n\tMon, 12 Jun 2023 01:54:45 -0700 (PDT)","MIME-Version":"1.0","References":"<20230607100054.4576-1-naush@raspberrypi.com>\n\t<20230607100054.4576-4-naush@raspberrypi.com>\n\t<20230607151059.GE22127@pendragon.ideasonboard.com>\n\t<168616781019.1159722.3914381123768662632@Monstersaurus>","In-Reply-To":"<168616781019.1159722.3914381123768662632@Monstersaurus>","Date":"Mon, 12 Jun 2023 09:54:35 +0100","Message-ID":"","To":"Kieran Bingham ","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH v1 3/3] ipa: rpi: agc: Gracefully\n\thandle missing agc modes","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":"Laurent Pinchart via libcamera-devel\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":27317,"web_url":"https://patchwork.libcamera.org/comment/27317/","msgid":"","date":"2023-06-12T09:15:42","subject":"Re: [libcamera-devel] [PATCH v1 3/3] ipa: rpi: agc: Gracefully\n\thandle missing agc modes","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hi David,\n\nOn Mon, 12 Jun 2023 at 09:54, David Plowman\n wrote:\n>\n> Hi everyone\n>\n> I think this is a good idea. The only question I have is whether we\n> might simply print a warning and do nothing, rather than change to the\n> default mode.\n>\n> I think this is a slightly less surprising behaviour - if you make a\n> mistake it has no effect, rather than change you to some possibly\n> unrelated mode that you might even have to undo again. It's also a\n> general rule that could be applied to all controls: ask for something\n> that can't be done and it tells you and does nothing.\n>\n> Thoughts?\n\nI agree, it's slightly less surprising if we don't change the mode (instead of\nchanging to default) on the error condition. This also matches the behaviour of\nvarious other control handling that we have in the IPA.\n\nI'll make the change and post it shortly.\n\nRegards,\nNaush\n\n>\n> Thanks!\n> David\n>\n> On Wed, 7 Jun 2023 at 20:56, Kieran Bingham via libcamera-devel\n> wrote:\n> >\n> > Quoting Laurent Pinchart via libcamera-devel (2023-06-07 16:10:59)\n> > > Hi Naush,\n> > >\n> > > Thank you for the patch.\n> > >\n> > > On Wed, Jun 07, 2023 at 11:00:54AM +0100, Naushir Patuck via libcamera-devel wrote:\n> > > > If a metering/exposure/constraint mode is not listed in the sensor\n> > > > tuning file, and a control for the missing mode is set on the agc, we\n> > > > terminate the application with a fatal log message.\n> > > >\n> > > > Instead of this fatal termination, log a warning message and switch to\n> > > > the appropriate default mode so that the application continues running.\n> > > >\n> > > > Reported-on: https://github.com/raspberrypi/libcamera/issues/59\n> > > > Reported-on: https://github.com/ayufan/camera-streamer/issues/67\n> > >\n> > > We haven't used \"Reported-on\" before, and grepping in the whole Linux\n> >\n> > Err ... we have:\n> >\n> > 2a261d911f50 (\"pipeline: raspberrypi: Iterate over all Unicam instances in match()\")\n> >\n> > Other references we've used for github issues include:\n> >\n> > Reported-on: https://github.com/raspberrypi/libcamera/issues/59\n> > Reported-on: https://github.com/ayufan/camera-streamer/issues/67\n> > Reported-on: https://github.com/raspberrypi/libcamera/issues/44\n> > Bug: https://github.com/raspberrypi/libcamera/issues/29\n> > Reported-by: https://github.com/kralo\n> > Bug: https://github.com/raspberrypi/libcamera-apps/issues/217\n> > Bug: https://github.com/raspberrypi/libcamera-apps/issues/236\n> > Bug: https://github.com/raspberrypi/libcamera-apps/issues/238\n> > Bug: https://github.com/raspberrypi/libcamera-apps/issues/240\n> > See https://github.com/PyCQA/pycodestyle/issues/466\n> >\n> > I think Bug: is the most appropriate still, so I'll use that.\n> >\n> >\n> > > kernel history there's a single occurrence. The tags used in the kernel\n> > > with github issues links are\n> > >\n> > > 1 Ref.\n> > > 1 regression\n> > > 1 Reported-on\n> > > 1 see\n> > > 1 URL\n> > > 2 Issue\n> > > 2 link\n> > > 2 Ref\n> > > 3 Buglink\n> > > 3 Related\n> > > 3 Reported-by\n> > > 3 Resolves\n> > > 5 Bug\n> > > 5 References\n> > > 6 Fixes\n> > > 7 Reference\n> > > 17 Addresses-KSPP-ID\n> > > 43 See\n> > > 89 Closes\n> > > 158 BugLink\n> > > 1426 Link\n> > >\n> > > We don't use BugLink and have used Link once only to refer to a mailing\n> > > list entry, while we use Bug for bugzilla entries. I would vote for Bug\n> > > or Link in this case.\n> > >\n> > > > Signed-off-by: Naushir Patuck \n> > > > ---\n> > > > src/ipa/rpi/controller/rpi/agc.cpp | 30 +++++++++++++++++++++---------\n> > > > 1 file changed, 21 insertions(+), 9 deletions(-)\n> > > >\n> > > > diff --git a/src/ipa/rpi/controller/rpi/agc.cpp b/src/ipa/rpi/controller/rpi/agc.cpp\n> > > > index b611157af1f0..1b05d478818e 100644\n> > > > --- a/src/ipa/rpi/controller/rpi/agc.cpp\n> > > > +++ b/src/ipa/rpi/controller/rpi/agc.cpp\n> > > > @@ -540,24 +540,36 @@ void Agc::housekeepConfig()\n> > > > */\n> > > > if (meteringModeName_ != status_.meteringMode) {\n> > > > auto it = config_.meteringModes.find(meteringModeName_);\n> > > > - if (it == config_.meteringModes.end())\n> > > > - LOG(RPiAgc, Fatal) << \"No metering mode \" << meteringModeName_;\n> > > > - meteringMode_ = &it->second;\n> > > > + if (it == config_.meteringModes.end()) {\n> > > > + LOG(RPiAgc, Warning) << \"No metering mode \" << meteringModeName_\n> > > > + << \", defaulting to \" << config_.defaultMeteringMode;\n> > > > + meteringModeName_ = config_.defaultMeteringMode;\n> > > > + meteringMode_ = &config_.meteringModes[meteringModeName_];\n> > > > + } else\n> > > > + meteringMode_ = &it->second;\n> > >\n> > > Please use curly braces here and below. This can be updated when\n> > > applying the patch, no need to resubmit.\n> >\n> > Updated, and running through the integration tests.\n> >\n> > --\n> > Kieran\n> >\n> >\n> > >\n> > > Reviewed-by: Laurent Pinchart \n> > >\n> > > > status_.meteringMode = meteringModeName_;\n> > > > }\n> > > > if (exposureModeName_ != status_.exposureMode) {\n> > > > auto it = config_.exposureModes.find(exposureModeName_);\n> > > > - if (it == config_.exposureModes.end())\n> > > > - LOG(RPiAgc, Fatal) << \"No exposure profile \" << exposureModeName_;\n> > > > - exposureMode_ = &it->second;\n> > > > + if (it == config_.exposureModes.end()) {\n> > > > + LOG(RPiAgc, Warning) << \"No exposure profile \" << exposureModeName_\n> > > > + << \", defaulting to \" << config_.defaultExposureMode;\n> > > > + exposureModeName_ = config_.defaultExposureMode;\n> > > > + exposureMode_ = &config_.exposureModes[exposureModeName_];\n> > > > + } else\n> > > > + exposureMode_ = &it->second;\n> > > > status_.exposureMode = exposureModeName_;\n> > > > }\n> > > > if (constraintModeName_ != status_.constraintMode) {\n> > > > auto it =\n> > > > config_.constraintModes.find(constraintModeName_);\n> > > > - if (it == config_.constraintModes.end())\n> > > > - LOG(RPiAgc, Fatal) << \"No constraint list \" << constraintModeName_;\n> > > > - constraintMode_ = &it->second;\n> > > > + if (it == config_.constraintModes.end()) {\n> > > > + LOG(RPiAgc, Warning) << \"No constraint list \" << constraintModeName_\n> > > > + << \", defaulting to \" << config_.defaultConstraintMode;\n> > > > + constraintModeName_ = config_.defaultConstraintMode;\n> > > > + constraintMode_ = &config_.constraintModes[constraintModeName_];\n> > > > + } else\n> > > > + constraintMode_ = &it->second;\n> > > > status_.constraintMode = constraintModeName_;\n> > > > }\n> > > > LOG(RPiAgc, Debug) << \"exposureMode \"\n> > >\n> > > --\n> > > Regards,\n> > >\n> > > Laurent Pinchart","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 B39E2C31E9\n\tfor ;\n\tMon, 12 Jun 2023 09:16:01 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id EBB5A61E49;\n\tMon, 12 Jun 2023 11:16:00 +0200 (CEST)","from mail-yw1-x1130.google.com (mail-yw1-x1130.google.com\n\t[IPv6:2607:f8b0:4864:20::1130])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 2999361E48\n\tfor ;\n\tMon, 12 Jun 2023 11:15:59 +0200 (CEST)","by mail-yw1-x1130.google.com with SMTP id\n\t00721157ae682-56d2ac0d990so11067457b3.0\n\tfor ;\n\tMon, 12 Jun 2023 02:15:59 -0700 (PDT)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1686561361;\n\tbh=8WMsXzWqAf31DB3SSzQoUDp+JwzCYw+gC+1Ilbc/5s8=;\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=ZKbglMq8fEPCmlVUl3U6Dh9wftfrnK6VlTKtyaNLlnZAAGVo7+vnj591pAewi6ih9\n\t2odnnd+6lDFIARkiR9qFm9U68epuJ3s5jboNnpgRaFJ81sys0o0ImZZiJpioXlft+X\n\t2lh+/ZiTkFwOiqhO8c5MrdJqZg53cQml8E2wut/nfMkQMi+8Qsu+2Kp49G8MbyjKW+\n\t5O9q/fFwAnO5cTKtvm4qyjTI5O6gUvzT2vMUd2M9CdGH4Yol0SlSHYyQK/agXVVR87\n\tuXIfzl40YbRj5TuEKKFACe0IRBXwLUq5rTCtDhcK2S6ZFGYaOA3OIqF1CbMsY80X1I\n\teq6ONd2atneZA==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google; t=1686561358; x=1689153358;\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=9OaHDbaWjMd2jBY2PKFUjpQkC/+1ND5pRADx6IVKibk=;\n\tb=MxjI5iFrEfQAc0M+RhhlVmGBTDcgq0xQryet8Y6dg0s9yqok1aghRbR9HWltolTbhi\n\t3rBaC7dfCT1v3eWNWM+dt3+z8x6SkJq4k5jluJ5tLt8WR4iVhGPnm28astw68fpIW3de\n\tB3JIKLnDSXR+B1kB0pJfqYZqNpam4Iivnm8gn7hnsM9ojatGs2btD+eCDvVjLNKk/48n\n\tKtoldp/E4gNfe7EfKRA2vx6fKeD+pE9HeReZX0RnIR2ljlnl8y6fiXy4eaehFTH4Vq/v\n\tdHUIcj+IpQaA2yHqpVgLRz0ETkn6Q2Ao9TR1mU6spY99JoPbrcuTuU+lcb0LJ/AdgVUa\n\tciBA=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=raspberrypi.com\n\theader.i=@raspberrypi.com\n\theader.b=\"MxjI5iFr\"; dkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20221208; t=1686561358; x=1689153358;\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=9OaHDbaWjMd2jBY2PKFUjpQkC/+1ND5pRADx6IVKibk=;\n\tb=P1t2NoU+XNZ4S+RwQ+EUmLMu9pc8FRCECOl+/B8i1ughjQj5sA6C6sTbT4au0eQ/i/\n\tZaJMJ563IjDwkXZXuA9lZ+SBN8+r2dVJCnYFBm/S95tegegvyB27s2mhHnIDnNwSuGI3\n\tx4kuXd08OOVnprLf/by4idZycwqED4FdFuAeDJeP+kn5wpcPMcf0+9n1eEsmlWr0PlcV\n\tx44f54JatSwufqvG4PxxE8PmD5C23PUrW4TtQFcOOUd6529UAt5X6PN/hgQiysfSs8gV\n\t3qdFgxZ1ClfvD+BhQP45rQ7kKqWwrEUIPHqFgWdi2EJPwVHU9aXrVkybbyDNFjJ9LRIv\n\tW2NA==","X-Gm-Message-State":"AC+VfDy+izdmwy+PBREitYLJQV0TBk5fuPv5XQnV/oToFJEvYfAejboX\n\tPg2lJE8GgFjIbF1lCSKEjkl+aDgQS4UKcuo9n80U6w==","X-Google-Smtp-Source":"ACHHUZ4geqHwQkvFprAWHQgJzWHq7S9w7XT3Y5sZfm08veefbiwP/fpxeS7EK2JGEQTvXW8koYzRctqnnSjLjy3NdxQ=","X-Received":"by 2002:a0d:db41:0:b0:56d:2d82:63de with SMTP id\n\td62-20020a0ddb41000000b0056d2d8263demr2165650ywe.3.1686561357851;\n\tMon, 12 Jun 2023 02:15:57 -0700 (PDT)","MIME-Version":"1.0","References":"<20230607100054.4576-1-naush@raspberrypi.com>\n\t<20230607100054.4576-4-naush@raspberrypi.com>\n\t<20230607151059.GE22127@pendragon.ideasonboard.com>\n\t<168616781019.1159722.3914381123768662632@Monstersaurus>\n\t","In-Reply-To":"","Date":"Mon, 12 Jun 2023 10:15:42 +0100","Message-ID":"","To":"David Plowman ","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH v1 3/3] ipa: rpi: agc: Gracefully\n\thandle missing agc modes","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":"Naushir Patuck via libcamera-devel\n\t","Reply-To":"Naushir Patuck ","Cc":"Laurent Pinchart via libcamera-devel\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":27325,"web_url":"https://patchwork.libcamera.org/comment/27325/","msgid":"<20230612224034.GC17519@pendragon.ideasonboard.com>","date":"2023-06-12T22:40:34","subject":"Re: [libcamera-devel] [PATCH v1 3/3] ipa: rpi: agc: Gracefully\n\thandle missing agc modes","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Wed, Jun 07, 2023 at 08:56:50PM +0100, Kieran Bingham wrote:\n> Quoting Laurent Pinchart via libcamera-devel (2023-06-07 16:10:59)\n> > Hi Naush,\n> > \n> > Thank you for the patch.\n> > \n> > On Wed, Jun 07, 2023 at 11:00:54AM +0100, Naushir Patuck via libcamera-devel wrote:\n> > > If a metering/exposure/constraint mode is not listed in the sensor\n> > > tuning file, and a control for the missing mode is set on the agc, we\n> > > terminate the application with a fatal log message.\n> > > \n> > > Instead of this fatal termination, log a warning message and switch to\n> > > the appropriate default mode so that the application continues running.\n> > > \n> > > Reported-on: https://github.com/raspberrypi/libcamera/issues/59\n> > > Reported-on: https://github.com/ayufan/camera-streamer/issues/67\n> > \n> > We haven't used \"Reported-on\" before, and grepping in the whole Linux\n> \n> Err ... we have:\n> \n> 2a261d911f50 (\"pipeline: raspberrypi: Iterate over all Unicam instances in match()\")\n\nOops, indeed :-/\n\n> Other references we've used for github issues include:\n> \n> Reported-on: https://github.com/raspberrypi/libcamera/issues/59\n> Reported-on: https://github.com/ayufan/camera-streamer/issues/67\n> Reported-on: https://github.com/raspberrypi/libcamera/issues/44\n> Bug: https://github.com/raspberrypi/libcamera/issues/29\n> Reported-by: https://github.com/kralo\n> Bug: https://github.com/raspberrypi/libcamera-apps/issues/217\n> Bug: https://github.com/raspberrypi/libcamera-apps/issues/236\n> Bug: https://github.com/raspberrypi/libcamera-apps/issues/238\n> Bug: https://github.com/raspberrypi/libcamera-apps/issues/240\n> See https://github.com/PyCQA/pycodestyle/issues/466\n> \n> I think Bug: is the most appropriate still, so I'll use that.\n\nIn order to avoid manual handling if similar issues in the future, I've\nimplemented a git commit message trailers checker for checkstyle.py.\nI'll post patches shortly.\n\n> > kernel history there's a single occurrence. The tags used in the kernel\n> > with github issues links are\n> > \n> > 1 Ref.\n> > 1 regression\n> > 1 Reported-on\n> > 1 see\n> > 1 URL\n> > 2 Issue\n> > 2 link\n> > 2 Ref\n> > 3 Buglink\n> > 3 Related\n> > 3 Reported-by\n> > 3 Resolves\n> > 5 Bug\n> > 5 References\n> > 6 Fixes\n> > 7 Reference\n> > 17 Addresses-KSPP-ID\n> > 43 See\n> > 89 Closes\n> > 158 BugLink\n> > 1426 Link\n> > \n> > We don't use BugLink and have used Link once only to refer to a mailing\n> > list entry, while we use Bug for bugzilla entries. I would vote for Bug\n> > or Link in this case.\n> > \n> > > Signed-off-by: Naushir Patuck \n> > > ---\n> > > src/ipa/rpi/controller/rpi/agc.cpp | 30 +++++++++++++++++++++---------\n> > > 1 file changed, 21 insertions(+), 9 deletions(-)\n> > > \n> > > diff --git a/src/ipa/rpi/controller/rpi/agc.cpp b/src/ipa/rpi/controller/rpi/agc.cpp\n> > > index b611157af1f0..1b05d478818e 100644\n> > > --- a/src/ipa/rpi/controller/rpi/agc.cpp\n> > > +++ b/src/ipa/rpi/controller/rpi/agc.cpp\n> > > @@ -540,24 +540,36 @@ void Agc::housekeepConfig()\n> > > */\n> > > if (meteringModeName_ != status_.meteringMode) {\n> > > auto it = config_.meteringModes.find(meteringModeName_);\n> > > - if (it == config_.meteringModes.end())\n> > > - LOG(RPiAgc, Fatal) << \"No metering mode \" << meteringModeName_;\n> > > - meteringMode_ = &it->second;\n> > > + if (it == config_.meteringModes.end()) {\n> > > + LOG(RPiAgc, Warning) << \"No metering mode \" << meteringModeName_\n> > > + << \", defaulting to \" << config_.defaultMeteringMode;\n> > > + meteringModeName_ = config_.defaultMeteringMode;\n> > > + meteringMode_ = &config_.meteringModes[meteringModeName_];\n> > > + } else\n> > > + meteringMode_ = &it->second;\n> > \n> > Please use curly braces here and below. This can be updated when\n> > applying the patch, no need to resubmit.\n> \n> Updated, and running through the integration tests.\n> \n> > Reviewed-by: Laurent Pinchart \n> > \n> > > status_.meteringMode = meteringModeName_;\n> > > }\n> > > if (exposureModeName_ != status_.exposureMode) {\n> > > auto it = config_.exposureModes.find(exposureModeName_);\n> > > - if (it == config_.exposureModes.end())\n> > > - LOG(RPiAgc, Fatal) << \"No exposure profile \" << exposureModeName_;\n> > > - exposureMode_ = &it->second;\n> > > + if (it == config_.exposureModes.end()) {\n> > > + LOG(RPiAgc, Warning) << \"No exposure profile \" << exposureModeName_\n> > > + << \", defaulting to \" << config_.defaultExposureMode;\n> > > + exposureModeName_ = config_.defaultExposureMode;\n> > > + exposureMode_ = &config_.exposureModes[exposureModeName_];\n> > > + } else\n> > > + exposureMode_ = &it->second;\n> > > status_.exposureMode = exposureModeName_;\n> > > }\n> > > if (constraintModeName_ != status_.constraintMode) {\n> > > auto it =\n> > > config_.constraintModes.find(constraintModeName_);\n> > > - if (it == config_.constraintModes.end())\n> > > - LOG(RPiAgc, Fatal) << \"No constraint list \" << constraintModeName_;\n> > > - constraintMode_ = &it->second;\n> > > + if (it == config_.constraintModes.end()) {\n> > > + LOG(RPiAgc, Warning) << \"No constraint list \" << constraintModeName_\n> > > + << \", defaulting to \" << config_.defaultConstraintMode;\n> > > + constraintModeName_ = config_.defaultConstraintMode;\n> > > + constraintMode_ = &config_.constraintModes[constraintModeName_];\n> > > + } else\n> > > + constraintMode_ = &it->second;\n> > > status_.constraintMode = constraintModeName_;\n> > > }\n> > > LOG(RPiAgc, Debug) << \"exposureMode \"","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 221BEC3220\n\tfor ;\n\tMon, 12 Jun 2023 22:40:38 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 63A4261E50;\n\tTue, 13 Jun 2023 00:40:37 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 5A99A6020C\n\tfor ;\n\tTue, 13 Jun 2023 00:40:35 +0200 (CEST)","from pendragon.ideasonboard.com (213-243-189-158.bb.dnainternet.fi\n\t[213.243.189.158])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 27535BC;\n\tTue, 13 Jun 2023 00:40:05 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1686609637;\n\tbh=+3t/cB/XZAUnPUQj/nsgKmjUoApca7PFJGROv4fw9Lw=;\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=vnG36r1B4KYMr84kqIh9TfgqYkOiQsgDbnvAtlXiZhGMIoAOfpZTb1DGMG6W5vkyD\n\teLAyYKX3zJ/F0sq8R2X5pDAvkm/5jN3X+qn1oPQyrTYo1dit3KnrKW9reEQL848k6T\n\tiQRQJl5zC90ZsRS6mCpa4IcunP482/2EjvqL+b9IBPP2JCpCFV7i4Mm7XP/mtiLKc2\n\teTkMCVneDHqXMkCAWv+c9TudL5q2nWlQL4X1uBx6SaPm1KopvzsoI5Dpeqx9NA3Q3P\n\t9XqYpqgaryDczq/VR1ms+uYM+5E+sqevBNH57f4zMa9FiTpXxMU4Gy55DTpCSpcBvI\n\tG6al+km2wJI0A==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1686609605;\n\tbh=+3t/cB/XZAUnPUQj/nsgKmjUoApca7PFJGROv4fw9Lw=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=sKUMFxGG3NUv4xsVvL91cvGmZNwy5fB2pdSXm9B3TXduu8VaPUEv8nDxniaJV77Bs\n\t5FND2FKYPlyIGFfsmHDez0GmUB5Qbfe7Yz1Cpn2cigzkqtXvhaCXrkPNjoOAxgmJcB\n\tLrlxuai7dM0lEagGxiAnBSIQv3KoWYUYvbYX+rFY="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"sKUMFxGG\"; dkim-atps=neutral","Date":"Tue, 13 Jun 2023 01:40:34 +0300","To":"Kieran Bingham ","Message-ID":"<20230612224034.GC17519@pendragon.ideasonboard.com>","References":"<20230607100054.4576-1-naush@raspberrypi.com>\n\t<20230607100054.4576-4-naush@raspberrypi.com>\n\t<20230607151059.GE22127@pendragon.ideasonboard.com>\n\t<168616781019.1159722.3914381123768662632@Monstersaurus>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<168616781019.1159722.3914381123768662632@Monstersaurus>","Subject":"Re: [libcamera-devel] [PATCH v1 3/3] ipa: rpi: agc: Gracefully\n\thandle missing agc modes","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":"Laurent Pinchart via libcamera-devel\n\t","Reply-To":"Laurent Pinchart ","Cc":"Laurent Pinchart via libcamera-devel\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":27326,"web_url":"https://patchwork.libcamera.org/comment/27326/","msgid":"<20230612224810.GE28480@pendragon.ideasonboard.com>","date":"2023-06-12T22:48:10","subject":"Re: [libcamera-devel] [PATCH v1 3/3] ipa: rpi: agc: Gracefully\n\thandle missing agc modes","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hello,\n\nOn Mon, Jun 12, 2023 at 10:15:42AM +0100, Naushir Patuck wrote:\n> On Mon, 12 Jun 2023 at 09:54, David Plowman wrote:\n> >\n> > Hi everyone\n> >\n> > I think this is a good idea. The only question I have is whether we\n> > might simply print a warning and do nothing, rather than change to the\n> > default mode.\n> >\n> > I think this is a slightly less surprising behaviour - if you make a\n> > mistake it has no effect, rather than change you to some possibly\n> > unrelated mode that you might even have to undo again. It's also a\n> > general rule that could be applied to all controls: ask for something\n> > that can't be done and it tells you and does nothing.\n> >\n> > Thoughts?\n> \n> I agree, it's slightly less surprising if we don't change the mode (instead of\n> changing to default) on the error condition. This also matches the behaviour of\n> various other control handling that we have in the IPA.\n\nSounds good to me too. Thanks David for the suggestion.\n\n> I'll make the change and post it shortly.\n> \n> > On Wed, 7 Jun 2023 at 20:56, Kieran Bingham via libcamera-devel wrote:\n> > > Quoting Laurent Pinchart via libcamera-devel (2023-06-07 16:10:59)\n> > > > Hi Naush,\n> > > >\n> > > > Thank you for the patch.\n> > > >\n> > > > On Wed, Jun 07, 2023 at 11:00:54AM +0100, Naushir Patuck via libcamera-devel wrote:\n> > > > > If a metering/exposure/constraint mode is not listed in the sensor\n> > > > > tuning file, and a control for the missing mode is set on the agc, we\n> > > > > terminate the application with a fatal log message.\n> > > > >\n> > > > > Instead of this fatal termination, log a warning message and switch to\n> > > > > the appropriate default mode so that the application continues running.\n> > > > >\n> > > > > Reported-on: https://github.com/raspberrypi/libcamera/issues/59\n> > > > > Reported-on: https://github.com/ayufan/camera-streamer/issues/67\n> > > >\n> > > > We haven't used \"Reported-on\" before, and grepping in the whole Linux\n> > >\n> > > Err ... we have:\n> > >\n> > > 2a261d911f50 (\"pipeline: raspberrypi: Iterate over all Unicam instances in match()\")\n> > >\n> > > Other references we've used for github issues include:\n> > >\n> > > Reported-on: https://github.com/raspberrypi/libcamera/issues/59\n> > > Reported-on: https://github.com/ayufan/camera-streamer/issues/67\n> > > Reported-on: https://github.com/raspberrypi/libcamera/issues/44\n> > > Bug: https://github.com/raspberrypi/libcamera/issues/29\n> > > Reported-by: https://github.com/kralo\n> > > Bug: https://github.com/raspberrypi/libcamera-apps/issues/217\n> > > Bug: https://github.com/raspberrypi/libcamera-apps/issues/236\n> > > Bug: https://github.com/raspberrypi/libcamera-apps/issues/238\n> > > Bug: https://github.com/raspberrypi/libcamera-apps/issues/240\n> > > See https://github.com/PyCQA/pycodestyle/issues/466\n> > >\n> > > I think Bug: is the most appropriate still, so I'll use that.\n> > >\n> > >\n> > > > kernel history there's a single occurrence. The tags used in the kernel\n> > > > with github issues links are\n> > > >\n> > > > 1 Ref.\n> > > > 1 regression\n> > > > 1 Reported-on\n> > > > 1 see\n> > > > 1 URL\n> > > > 2 Issue\n> > > > 2 link\n> > > > 2 Ref\n> > > > 3 Buglink\n> > > > 3 Related\n> > > > 3 Reported-by\n> > > > 3 Resolves\n> > > > 5 Bug\n> > > > 5 References\n> > > > 6 Fixes\n> > > > 7 Reference\n> > > > 17 Addresses-KSPP-ID\n> > > > 43 See\n> > > > 89 Closes\n> > > > 158 BugLink\n> > > > 1426 Link\n> > > >\n> > > > We don't use BugLink and have used Link once only to refer to a mailing\n> > > > list entry, while we use Bug for bugzilla entries. I would vote for Bug\n> > > > or Link in this case.\n> > > >\n> > > > > Signed-off-by: Naushir Patuck \n> > > > > ---\n> > > > > src/ipa/rpi/controller/rpi/agc.cpp | 30 +++++++++++++++++++++---------\n> > > > > 1 file changed, 21 insertions(+), 9 deletions(-)\n> > > > >\n> > > > > diff --git a/src/ipa/rpi/controller/rpi/agc.cpp b/src/ipa/rpi/controller/rpi/agc.cpp\n> > > > > index b611157af1f0..1b05d478818e 100644\n> > > > > --- a/src/ipa/rpi/controller/rpi/agc.cpp\n> > > > > +++ b/src/ipa/rpi/controller/rpi/agc.cpp\n> > > > > @@ -540,24 +540,36 @@ void Agc::housekeepConfig()\n> > > > > */\n> > > > > if (meteringModeName_ != status_.meteringMode) {\n> > > > > auto it = config_.meteringModes.find(meteringModeName_);\n> > > > > - if (it == config_.meteringModes.end())\n> > > > > - LOG(RPiAgc, Fatal) << \"No metering mode \" << meteringModeName_;\n> > > > > - meteringMode_ = &it->second;\n> > > > > + if (it == config_.meteringModes.end()) {\n> > > > > + LOG(RPiAgc, Warning) << \"No metering mode \" << meteringModeName_\n> > > > > + << \", defaulting to \" << config_.defaultMeteringMode;\n> > > > > + meteringModeName_ = config_.defaultMeteringMode;\n> > > > > + meteringMode_ = &config_.meteringModes[meteringModeName_];\n> > > > > + } else\n> > > > > + meteringMode_ = &it->second;\n> > > >\n> > > > Please use curly braces here and below. This can be updated when\n> > > > applying the patch, no need to resubmit.\n> > >\n> > > Updated, and running through the integration tests.\n> > >\n> > > > Reviewed-by: Laurent Pinchart \n> > > >\n> > > > > status_.meteringMode = meteringModeName_;\n> > > > > }\n> > > > > if (exposureModeName_ != status_.exposureMode) {\n> > > > > auto it = config_.exposureModes.find(exposureModeName_);\n> > > > > - if (it == config_.exposureModes.end())\n> > > > > - LOG(RPiAgc, Fatal) << \"No exposure profile \" << exposureModeName_;\n> > > > > - exposureMode_ = &it->second;\n> > > > > + if (it == config_.exposureModes.end()) {\n> > > > > + LOG(RPiAgc, Warning) << \"No exposure profile \" << exposureModeName_\n> > > > > + << \", defaulting to \" << config_.defaultExposureMode;\n> > > > > + exposureModeName_ = config_.defaultExposureMode;\n> > > > > + exposureMode_ = &config_.exposureModes[exposureModeName_];\n> > > > > + } else\n> > > > > + exposureMode_ = &it->second;\n> > > > > status_.exposureMode = exposureModeName_;\n> > > > > }\n> > > > > if (constraintModeName_ != status_.constraintMode) {\n> > > > > auto it =\n> > > > > config_.constraintModes.find(constraintModeName_);\n> > > > > - if (it == config_.constraintModes.end())\n> > > > > - LOG(RPiAgc, Fatal) << \"No constraint list \" << constraintModeName_;\n> > > > > - constraintMode_ = &it->second;\n> > > > > + if (it == config_.constraintModes.end()) {\n> > > > > + LOG(RPiAgc, Warning) << \"No constraint list \" << constraintModeName_\n> > > > > + << \", defaulting to \" << config_.defaultConstraintMode;\n> > > > > + constraintModeName_ = config_.defaultConstraintMode;\n> > > > > + constraintMode_ = &config_.constraintModes[constraintModeName_];\n> > > > > + } else\n> > > > > + constraintMode_ = &it->second;\n> > > > > status_.constraintMode = constraintModeName_;\n> > > > > }\n> > > > > LOG(RPiAgc, Debug) << \"exposureMode \"","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 1627CC31E9\n\tfor ;\n\tMon, 12 Jun 2023 22:48:12 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D6E9561E55;\n\tTue, 13 Jun 2023 00:48:11 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id C376061E50\n\tfor ;\n\tTue, 13 Jun 2023 00:48:10 +0200 (CEST)","from pendragon.ideasonboard.com (213-243-189-158.bb.dnainternet.fi\n\t[213.243.189.158])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 02AC9C7F;\n\tTue, 13 Jun 2023 00:47:40 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1686610091;\n\tbh=j6iVrb7Ht7ULJ8Efi6mthem8yFX3HsXj04720h6UpYk=;\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=xyPLLFzy+Y+heIaXVb4En+HU+4x2jyW/LpZn3WSPqjvenTzBDSHOEJLpkaJnOqy2r\n\tA9A32H0aDs8l0LwtOMZOF8daotz1lO2SWy0N9EaTsG3CJijXJ7QRO7wcns6J8lcBR3\n\tHRT2UXqWTPhF3PyKZTONGBWCoDJ9pDCeH5slgfS7GhM9kixmgTaOHCkxgw9paOYUbh\n\tK3VynyRClI8nUBCi3shsiAGRB0KIrBeoTpIBlpGvUU+y8qSZcUit8FJLtt/xBgKtol\n\t5A69JzjTG5J3oSt4gaUavpuoLYRaxEHrLEe7zbV3Sajn6lGFIZnROKzotIjCuxr85g\n\tyPzcRkQ9DfwFA==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1686610061;\n\tbh=j6iVrb7Ht7ULJ8Efi6mthem8yFX3HsXj04720h6UpYk=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=UbrCzv3d5acvUu5ff6xyT85qmIxebrqH50X4erItQR4nNnwMYOMQ9DSvXd3StI892\n\td/cvHG9WyKfEOYZ1GrfKq9SRSBfOFXcZH0sTOjsMJP20ocRlteZQ2Zr2at8CkJi6Br\n\tGq+vALjmUV/e5ClOoVRtksMx6vzqzSrLEEVUALbI="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"UbrCzv3d\"; dkim-atps=neutral","Date":"Tue, 13 Jun 2023 01:48:10 +0300","To":"Naushir Patuck ","Message-ID":"<20230612224810.GE28480@pendragon.ideasonboard.com>","References":"<20230607100054.4576-1-naush@raspberrypi.com>\n\t<20230607100054.4576-4-naush@raspberrypi.com>\n\t<20230607151059.GE22127@pendragon.ideasonboard.com>\n\t<168616781019.1159722.3914381123768662632@Monstersaurus>\n\t\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH v1 3/3] ipa: rpi: agc: Gracefully\n\thandle missing agc modes","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":"Laurent Pinchart via libcamera-devel\n\t","Reply-To":"Laurent Pinchart ","Cc":"Laurent Pinchart via libcamera-devel\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]