[{"id":33029,"web_url":"https://patchwork.libcamera.org/comment/33029/","msgid":"<20250112201329.GH7733@pendragon.ideasonboard.com>","date":"2025-01-12T20:13:29","subject":"Re: [PATCH v7 10/12] libcamera: camera: Pre-process AeEnable control","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Paul,\n\nThank you for the patch.\n\nOn Fri, Jan 10, 2025 at 05:57:35PM -0600, Paul Elder wrote:\n> Handle the AeEnable under the hood in the Camera class, such that\n> AeEnable activates ExposureTimeMode and AnalogueGain together. This\n> allows applications the convenience of setting auto/manual mode of all\n> of the AE-related controls, as well as protecting applications against a\n> nasty behavior change if an aperture control is added in the future.\n> This also moves common handling code out of the IPA.\n> \n> While we also want to inject AeEnable in Camera::controls() so that IPAs\n> don't have to report it, it is technically difficult at the moment as\n> ControlInfoMaps are not easily modifiable.\n> \n> Signed-off-by: Paul Elder \n\nReviewed-by: Laurent Pinchart \n\n> ---\n> Changes in v7:\n> - check that the camera supports the ae sub-controls before setting them\n> when handling AeEnable\n> \n> New in v6\n> ---\n> src/libcamera/camera.cpp | 20 ++++++++++++++++++++\n> 1 file changed, 20 insertions(+)\n> \n> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\n> index 69a7ee535..56c585199 100644\n> --- a/src/libcamera/camera.cpp\n> +++ b/src/libcamera/camera.cpp\n> @@ -19,6 +19,7 @@\n> #include \n> \n> #include \n> +#include \n> #include \n> #include \n> #include \n> @@ -1325,6 +1326,25 @@ int Camera::queueRequest(Request *request)\n> \t\t}\n> \t}\n> \n> +\t/* Pre-process AeEnable. */\n> +\tControlList &controls = request->controls();\n> +\tconst auto &aeEnable = controls.get(controls::AeEnable);\n> +\tif (aeEnable) {\n> +\t\tif (_d()->controlInfo_.count(controls::AnalogueGainMode.id()) &&\n> +\t\t !controls.contains(controls::AnalogueGainMode.id())) {\n> +\t\t\tcontrols.set(controls::AnalogueGainMode,\n> +\t\t\t\t *aeEnable ? controls::AnalogueGainModeAuto\n> +\t\t\t\t\t : controls::AnalogueGainModeManual);\n> +\t\t}\n> +\n> +\t\tif (_d()->controlInfo_.count(controls::ExposureTimeMode.id()) &&\n> +\t\t !controls.contains(controls::ExposureTimeMode.id())) {\n> +\t\t\tcontrols.set(controls::ExposureTimeMode,\n> +\t\t\t\t *aeEnable ? controls::ExposureTimeModeAuto\n> +\t\t\t\t\t : controls::ExposureTimeModeManual);\n> +\t\t}\n> +\t}\n> +\n> \td->pipe_->invokeMethod(&PipelineHandler::queueRequest,\n> \t\t\t ConnectionTypeQueued, request);\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 14F27C326C\n\tfor ;\n\tSun, 12 Jan 2025 20:13:37 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 59506684EA;\n\tSun, 12 Jan 2025 21:13:36 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id F055C61882\n\tfor ;\n\tSun, 12 Jan 2025 21:13:33 +0100 (CET)","from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi\n\t[81.175.209.231])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 797531112;\n\tSun, 12 Jan 2025 21:12:37 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"jqlF5XQv\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1736712758;\n\tbh=+ql3lJOCOAcB6KSZH43oSMUo1OT2KF3d35KS+R089Hs=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=jqlF5XQvlB7tL6IboMnSfl2+ukW+WnQs3CXnvvFv4LSZZeuEyr+RtSvZ8/ZUCshFV\n\t/KtlPwCLaNiQ91pyzyCyroXOdYTK4iCYutFtN+oE9G4d+EySRAbnFXIYqmp4KrTRlr\n\tlVWRCBd+nUwZJVWAC8Vvjx9xEbxITH2TLXwPprbA=","Date":"Sun, 12 Jan 2025 22:13:29 +0200","From":"Laurent Pinchart ","To":"Paul Elder ","Cc":"libcamera-devel@lists.libcamera.org, stefan.klug@ideasonboard.com","Subject":"Re: [PATCH v7 10/12] libcamera: camera: Pre-process AeEnable control","Message-ID":"<20250112201329.GH7733@pendragon.ideasonboard.com>","References":"<20250110235737.1524733-1-paul.elder@ideasonboard.com>\n\t<20250110235737.1524733-11-paul.elder@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20250110235737.1524733-11-paul.elder@ideasonboard.com>","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":33038,"web_url":"https://patchwork.libcamera.org/comment/33038/","msgid":"","date":"2025-01-13T11:03:18","subject":"Re: [PATCH v7 10/12] libcamera: camera: Pre-process AeEnable control","submitter":{"id":184,"url":"https://patchwork.libcamera.org/api/people/184/","name":"Stefan Klug","email":"stefan.klug@ideasonboard.com"},"content":"Hi Paul,\n\nThank you for the patch.\n\nOn Fri, Jan 10, 2025 at 05:57:35PM -0600, Paul Elder wrote:\n> Handle the AeEnable under the hood in the Camera class, such that\n> AeEnable activates ExposureTimeMode and AnalogueGain together. This\n> allows applications the convenience of setting auto/manual mode of all\n> of the AE-related controls, as well as protecting applications against a\n> nasty behavior change if an aperture control is added in the future.\n> This also moves common handling code out of the IPA.\n> \n> While we also want to inject AeEnable in Camera::controls() so that IPAs\n> don't have to report it, it is technically difficult at the moment as\n> ControlInfoMaps are not easily modifiable.\n> \n> Signed-off-by: Paul Elder \n> \n> ---\n> Changes in v7:\n> - check that the camera supports the ae sub-controls before setting them\n> when handling AeEnable\n> \n> New in v6\n> ---\n> src/libcamera/camera.cpp | 20 ++++++++++++++++++++\n> 1 file changed, 20 insertions(+)\n> \n> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\n> index 69a7ee535..56c585199 100644\n> --- a/src/libcamera/camera.cpp\n> +++ b/src/libcamera/camera.cpp\n> @@ -19,6 +19,7 @@\n> #include \n> \n> #include \n> +#include \n> #include \n> #include \n> #include \n> @@ -1325,6 +1326,25 @@ int Camera::queueRequest(Request *request)\n> \t\t}\n> \t}\n> \n> +\t/* Pre-process AeEnable. */\n> +\tControlList &controls = request->controls();\n> +\tconst auto &aeEnable = controls.get(controls::AeEnable);\n> +\tif (aeEnable) {\n> +\t\tif (_d()->controlInfo_.count(controls::AnalogueGainMode.id()) &&\n> +\t\t !controls.contains(controls::AnalogueGainMode.id())) {\n> +\t\t\tcontrols.set(controls::AnalogueGainMode,\n> +\t\t\t\t *aeEnable ? controls::AnalogueGainModeAuto\n> +\t\t\t\t\t : controls::AnalogueGainModeManual);\n> +\t\t}\n> +\n> +\t\tif (_d()->controlInfo_.count(controls::ExposureTimeMode.id()) &&\n> +\t\t !controls.contains(controls::ExposureTimeMode.id())) {\n> +\t\t\tcontrols.set(controls::ExposureTimeMode,\n> +\t\t\t\t *aeEnable ? controls::ExposureTimeModeAuto\n> +\t\t\t\t\t : controls::ExposureTimeModeManual);\n> +\t\t}\n> +\t}\n> +\n\nIt feels a bit bad to modify the controls list that was filled by the\nuser. But there is no way around it atm. So\n\nReviewed-by: Stefan Klug \n\nCheers,\nStefan\n\n> \td->pipe_->invokeMethod(&PipelineHandler::queueRequest,\n> \t\t\t ConnectionTypeQueued, request);\n> \n> -- \n> 2.39.2\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 DABC2C32F6\n\tfor ;\n\tMon, 13 Jan 2025 11:03:22 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 1301368503;\n\tMon, 13 Jan 2025 12:03:22 +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 AE69F607D6\n\tfor ;\n\tMon, 13 Jan 2025 12:03:20 +0100 (CET)","from ideasonboard.com (unknown\n\t[IPv6:2a00:6020:448c:6c00:3c15:8eea:8742:2e5a])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 29C8563F;\n\tMon, 13 Jan 2025 12:02:24 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"M+aHlcUQ\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1736766144;\n\tbh=1miG+nZYurAmj5DCANnFOOnA61NIHybPimuavl/PgFk=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=M+aHlcUQGvF6HHNM4fIpW3jnq4ZEydllf3OnGWDA9Giwf9WXlr9zExnJQkJevTjUO\n\tiUhH9A6a+6rSwiSmjHUCdFvl+K5uCBiDFbhcOwEaLv9XtHWznDAdAk5tcEnEdHDFzn\n\tn1kUpYHzgmbXO6hQ97vn5XbM4DV4uQZ1rmI60XMQ=","Date":"Mon, 13 Jan 2025 12:03:18 +0100","From":"Stefan Klug ","To":"Paul Elder ","Cc":"libcamera-devel@lists.libcamera.org, laurent.pinchart@ideasonboard.com","Subject":"Re: [PATCH v7 10/12] libcamera: camera: Pre-process AeEnable control","Message-ID":"","References":"<20250110235737.1524733-1-paul.elder@ideasonboard.com>\n\t<20250110235737.1524733-11-paul.elder@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20250110235737.1524733-11-paul.elder@ideasonboard.com>","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]