[{"id":5315,"web_url":"https://patchwork.libcamera.org/comment/5315/","msgid":"<20200622035424.GI25355@pendragon.ideasonboard.com>","date":"2020-06-22T03:54:24","subject":"Re: [libcamera-devel] [PATCH v2 1/2] libcamera: raspberrypi: allow\n\tSwitchMode method to return camera settings","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi David,\n\nThank you for the patch.\n\nOn Thu, Jun 18, 2020 at 12:12:35PM +0100, David Plowman wrote:\n> This commit adds a Metadata parameter to the SwitchMode method\n> enabling it to return camera and other settings to the caller\n> (usually the configure method, just after the camera mode has been\n> selected).\n> \n> In future this will allow the Raspberry Pi IPAs to take those settings\n> (such as exposure and analogue gain) and program them directly into\n> the camera or ISP before the camera is even started.\n> \n> Signed-off-by: David Plowman \n\nReviewed-by: Laurent Pinchart \n\n> ---\n> src/ipa/raspberrypi/controller/algorithm.cpp | 3 ++-\n> src/ipa/raspberrypi/controller/algorithm.hpp | 2 +-\n> src/ipa/raspberrypi/controller/controller.cpp | 4 ++--\n> src/ipa/raspberrypi/controller/controller.hpp | 2 +-\n> src/ipa/raspberrypi/controller/rpi/alsc.cpp | 4 +++-\n> src/ipa/raspberrypi/controller/rpi/alsc.hpp | 2 +-\n> src/ipa/raspberrypi/controller/rpi/noise.cpp | 4 +++-\n> src/ipa/raspberrypi/controller/rpi/noise.hpp | 2 +-\n> src/ipa/raspberrypi/controller/rpi/sharpen.cpp | 4 +++-\n> src/ipa/raspberrypi/controller/rpi/sharpen.hpp | 2 +-\n> src/ipa/raspberrypi/raspberrypi.cpp | 3 ++-\n> 11 files changed, 20 insertions(+), 12 deletions(-)\n> \n> diff --git a/src/ipa/raspberrypi/controller/algorithm.cpp b/src/ipa/raspberrypi/controller/algorithm.cpp\n> index 9bd3df8..55cb201 100644\n> --- a/src/ipa/raspberrypi/controller/algorithm.cpp\n> +++ b/src/ipa/raspberrypi/controller/algorithm.cpp\n> @@ -16,9 +16,10 @@ void Algorithm::Read(boost::property_tree::ptree const ¶ms)\n> \n> void Algorithm::Initialise() {}\n> \n> -void Algorithm::SwitchMode(CameraMode const &camera_mode)\n> +void Algorithm::SwitchMode(CameraMode const &camera_mode, Metadata *metadata)\n> {\n> \t(void)camera_mode;\n> +\t(void)metadata;\n> }\n> \n> void Algorithm::Prepare(Metadata *image_metadata)\n> diff --git a/src/ipa/raspberrypi/controller/algorithm.hpp b/src/ipa/raspberrypi/controller/algorithm.hpp\n> index b82c184..187c50c 100644\n> --- a/src/ipa/raspberrypi/controller/algorithm.hpp\n> +++ b/src/ipa/raspberrypi/controller/algorithm.hpp\n> @@ -37,7 +37,7 @@ public:\n> \tvirtual void Resume() { paused_ = false; }\n> \tvirtual void Read(boost::property_tree::ptree const ¶ms);\n> \tvirtual void Initialise();\n> -\tvirtual void SwitchMode(CameraMode const &camera_mode);\n> +\tvirtual void SwitchMode(CameraMode const &camera_mode, Metadata *metadata);\n> \tvirtual void Prepare(Metadata *image_metadata);\n> \tvirtual void Process(StatisticsPtr &stats, Metadata *image_metadata);\n> \tMetadata &GetGlobalMetadata() const\n> diff --git a/src/ipa/raspberrypi/controller/controller.cpp b/src/ipa/raspberrypi/controller/controller.cpp\n> index 20dd4c7..7c4b04f 100644\n> --- a/src/ipa/raspberrypi/controller/controller.cpp\n> +++ b/src/ipa/raspberrypi/controller/controller.cpp\n> @@ -56,11 +56,11 @@ void Controller::Initialise()\n> \tRPI_LOG(\"Controller finished\");\n> }\n> \n> -void Controller::SwitchMode(CameraMode const &camera_mode)\n> +void Controller::SwitchMode(CameraMode const &camera_mode, Metadata *metadata)\n> {\n> \tRPI_LOG(\"Controller starting\");\n> \tfor (auto &algo : algorithms_)\n> -\t\talgo->SwitchMode(camera_mode);\n> +\t\talgo->SwitchMode(camera_mode, metadata);\n> \tswitch_mode_called_ = true;\n> \tRPI_LOG(\"Controller finished\");\n> }\n> diff --git a/src/ipa/raspberrypi/controller/controller.hpp b/src/ipa/raspberrypi/controller/controller.hpp\n> index d685386..6ba9412 100644\n> --- a/src/ipa/raspberrypi/controller/controller.hpp\n> +++ b/src/ipa/raspberrypi/controller/controller.hpp\n> @@ -39,7 +39,7 @@ public:\n> \tAlgorithm *CreateAlgorithm(char const *name);\n> \tvoid Read(char const *filename);\n> \tvoid Initialise();\n> -\tvoid SwitchMode(CameraMode const &camera_mode);\n> +\tvoid SwitchMode(CameraMode const &camera_mode, Metadata *metadata);\n> \tvoid Prepare(Metadata *image_metadata);\n> \tvoid Process(StatisticsPtr stats, Metadata *image_metadata);\n> \tMetadata &GetGlobalMetadata();\n> diff --git a/src/ipa/raspberrypi/controller/rpi/alsc.cpp b/src/ipa/raspberrypi/controller/rpi/alsc.cpp\n> index 821a0ca..76e2f04 100644\n> --- a/src/ipa/raspberrypi/controller/rpi/alsc.cpp\n> +++ b/src/ipa/raspberrypi/controller/rpi/alsc.cpp\n> @@ -173,8 +173,10 @@ void Alsc::Initialise()\n> \t\tlambda_r_[i] = lambda_b_[i] = 1.0;\n> }\n> \n> -void Alsc::SwitchMode(CameraMode const &camera_mode)\n> +void Alsc::SwitchMode(CameraMode const &camera_mode, Metadata *metadata)\n> {\n> +\t(void)metadata;\n> +\n> \t// There's a bit of a question what we should do if the \"crop\" of the\n> \t// camera mode has changed. Any calculation currently in flight would\n> \t// not be useful to the new mode, so arguably we should abort it, and\n> diff --git a/src/ipa/raspberrypi/controller/rpi/alsc.hpp b/src/ipa/raspberrypi/controller/rpi/alsc.hpp\n> index c8ed3d2..3806257 100644\n> --- a/src/ipa/raspberrypi/controller/rpi/alsc.hpp\n> +++ b/src/ipa/raspberrypi/controller/rpi/alsc.hpp\n> @@ -50,7 +50,7 @@ public:\n> \t~Alsc();\n> \tchar const *Name() const override;\n> \tvoid Initialise() override;\n> -\tvoid SwitchMode(CameraMode const &camera_mode) override;\n> +\tvoid SwitchMode(CameraMode const &camera_mode, Metadata *metadata) override;\n> \tvoid Read(boost::property_tree::ptree const ¶ms) override;\n> \tvoid Prepare(Metadata *image_metadata) override;\n> \tvoid Process(StatisticsPtr &stats, Metadata *image_metadata) override;\n> diff --git a/src/ipa/raspberrypi/controller/rpi/noise.cpp b/src/ipa/raspberrypi/controller/rpi/noise.cpp\n> index 2209d79..2cafde3 100644\n> --- a/src/ipa/raspberrypi/controller/rpi/noise.cpp\n> +++ b/src/ipa/raspberrypi/controller/rpi/noise.cpp\n> @@ -27,8 +27,10 @@ char const *Noise::Name() const\n> \treturn NAME;\n> }\n> \n> -void Noise::SwitchMode(CameraMode const &camera_mode)\n> +void Noise::SwitchMode(CameraMode const &camera_mode, Metadata *metadata)\n> {\n> +\t(void)metadata;\n> +\n> \t// For example, we would expect a 2x2 binned mode to have a \"noise\n> \t// factor\" of sqrt(2x2) = 2. (can't be less than one, right?)\n> \tmode_factor_ = std::max(1.0, camera_mode.noise_factor);\n> diff --git a/src/ipa/raspberrypi/controller/rpi/noise.hpp b/src/ipa/raspberrypi/controller/rpi/noise.hpp\n> index 51d46a3..25bf188 100644\n> --- a/src/ipa/raspberrypi/controller/rpi/noise.hpp\n> +++ b/src/ipa/raspberrypi/controller/rpi/noise.hpp\n> @@ -18,7 +18,7 @@ class Noise : public Algorithm\n> public:\n> \tNoise(Controller *controller);\n> \tchar const *Name() const override;\n> -\tvoid SwitchMode(CameraMode const &camera_mode) override;\n> +\tvoid SwitchMode(CameraMode const &camera_mode, Metadata *metadata) override;\n> \tvoid Read(boost::property_tree::ptree const ¶ms) override;\n> \tvoid Prepare(Metadata *image_metadata) override;\n> \n> diff --git a/src/ipa/raspberrypi/controller/rpi/sharpen.cpp b/src/ipa/raspberrypi/controller/rpi/sharpen.cpp\n> index 1f07bb6..086952f 100644\n> --- a/src/ipa/raspberrypi/controller/rpi/sharpen.cpp\n> +++ b/src/ipa/raspberrypi/controller/rpi/sharpen.cpp\n> @@ -26,8 +26,10 @@ char const *Sharpen::Name() const\n> \treturn NAME;\n> }\n> \n> -void Sharpen::SwitchMode(CameraMode const &camera_mode)\n> +void Sharpen::SwitchMode(CameraMode const &camera_mode, Metadata *metadata)\n> {\n> +\t(void)metadata;\n> +\n> \t// can't be less than one, right?\n> \tmode_factor_ = std::max(1.0, camera_mode.noise_factor);\n> }\n> diff --git a/src/ipa/raspberrypi/controller/rpi/sharpen.hpp b/src/ipa/raspberrypi/controller/rpi/sharpen.hpp\n> index 3b0d680..f871aa6 100644\n> --- a/src/ipa/raspberrypi/controller/rpi/sharpen.hpp\n> +++ b/src/ipa/raspberrypi/controller/rpi/sharpen.hpp\n> @@ -18,7 +18,7 @@ class Sharpen : public Algorithm\n> public:\n> \tSharpen(Controller *controller);\n> \tchar const *Name() const override;\n> -\tvoid SwitchMode(CameraMode const &camera_mode) override;\n> +\tvoid SwitchMode(CameraMode const &camera_mode, Metadata *metadata) override;\n> \tvoid Read(boost::property_tree::ptree const ¶ms) override;\n> \tvoid Prepare(Metadata *image_metadata) override;\n> \n> diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp\n> index 9669f21..d6fd3df 100644\n> --- a/src/ipa/raspberrypi/raspberrypi.cpp\n> +++ b/src/ipa/raspberrypi/raspberrypi.cpp\n> @@ -267,7 +267,8 @@ void IPARPi::configure(const CameraSensorInfo &sensorInfo,\n> \t\tqueueFrameAction.emit(0, op);\n> \t}\n> \n> -\tcontroller_.SwitchMode(mode_);\n> +\tRPi::Metadata metadata;\n> +\tcontroller_.SwitchMode(mode_, &metadata);\n> \n> \tlastMode_ = mode_;\n> }","headers":{"Return-Path":"","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id BFFB2609A3\n\tfor ;\n\tMon, 22 Jun 2020 05:54:49 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 313DE30D;\n\tMon, 22 Jun 2020 05:54:49 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"ILYROqnS\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1592798089;\n\tbh=tjtRsWnoTMeso0WI+Qg/Ir6gQqxQzoyrpDvtZQIsKr8=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=ILYROqnS67hZNmxxmBAsQm2ya0Y2EBhEuWqDWBjtLAsvmnMFItM1ZJKJ6WShPIu1J\n\tqS9WwqbsoHyw+lAGKxbldLN3zbGl5Vv1kxlv43NI/XTdMxDZ4fGtsZsYD0rm+I2L4/\n\ti74deznL45mGfM4Hm/I8CvA6qiGP1H22WKLsMmDY=","Date":"Mon, 22 Jun 2020 06:54:24 +0300","From":"Laurent Pinchart ","To":"David Plowman ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20200622035424.GI25355@pendragon.ideasonboard.com>","References":"<20200618111236.26897-1-david.plowman@raspberrypi.com>\n\t<20200618111236.26897-2-david.plowman@raspberrypi.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20200618111236.26897-2-david.plowman@raspberrypi.com>","Subject":"Re: [libcamera-devel] [PATCH v2 1/2] libcamera: raspberrypi: allow\n\tSwitchMode method to return camera settings","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","X-List-Received-Date":"Mon, 22 Jun 2020 03:54:50 -0000"}}]