[{"id":12038,"web_url":"https://patchwork.libcamera.org/comment/12038/","msgid":"<20200819021352.GA20152@pendragon.ideasonboard.com>","date":"2020-08-19T02:13:52","subject":"Re: [libcamera-devel] [PATCH v2 4/5] libcamera: raspberrypi: Plumb\n\tuser transform through to IPA","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, Aug 06, 2020 at 05:36:38PM +0100, David Plowman wrote:\n> This commit plumbs the user transform from the Raspberry Pi pipeline\n> handler through to the IPA. Once in the IPA we add it to the\n> CameraMode description, so that it becomes automatically available to\n> all the individual control algorithms.\n> \n> The IPA configure method has to be reordered just a little so as to\n> fill in the transform in the camera mode before calling SwitchMode.\n\nMaybe a comment somewhere that explains the assumption that the\ntransformation is applied on the sensor directly, not on the ISP, would\nbe useful for readers. Apart from that, this looks good to me.\n\n> ---\n> src/ipa/raspberrypi/controller/camera_mode.h | 4 ++\n> src/ipa/raspberrypi/raspberrypi.cpp | 48 +++++++++++--------\n> .../pipeline/raspberrypi/raspberrypi.cpp | 5 +-\n> 3 files changed, 35 insertions(+), 22 deletions(-)\n> \n> diff --git a/src/ipa/raspberrypi/controller/camera_mode.h b/src/ipa/raspberrypi/controller/camera_mode.h\n> index 875bab3..920f11b 100644\n> --- a/src/ipa/raspberrypi/controller/camera_mode.h\n> +++ b/src/ipa/raspberrypi/controller/camera_mode.h\n> @@ -6,6 +6,8 @@\n> */\n> #pragma once\n> \n> +#include \n> +\n> // Description of a \"camera mode\", holding enough information for control\n> // algorithms to adapt their behaviour to the different modes of the camera,\n> // including binning, scaling, cropping etc.\n> @@ -33,6 +35,8 @@ struct CameraMode {\n> \tdouble noise_factor;\n> \t// line time in nanoseconds\n> \tdouble line_length;\n> +\t// any camera transform *not* reflected already in the camera tuning\n> +\tlibcamera::Transform transform;\n> };\n> \n> #ifdef __cplusplus\n> diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp\n> index 3747208..0b36d57 100644\n> --- a/src/ipa/raspberrypi/raspberrypi.cpp\n> +++ b/src/ipa/raspberrypi/raspberrypi.cpp\n> @@ -232,6 +232,33 @@ void IPARPi::configure(const CameraSensorInfo &sensorInfo,\n> \t/* Re-assemble camera mode using the sensor info. */\n> \tsetMode(sensorInfo);\n> \n> +\t/*\n> +\t * The ipaConfig.data always gives us the user transform first. Note that\n> +\t * this will always make the LS table pointer (if present) element 1.\n> +\t */\n> +\tmode_.transform = static_cast(ipaConfig.data[0]);\n> +\n> +\t/* Store the lens shading table pointer and handle if available. */\n> +\tif (ipaConfig.operation & RPI_IPA_CONFIG_LS_TABLE) {\n> +\t\t/* Remove any previous table, if there was one. */\n> +\t\tif (lsTable_) {\n> +\t\t\tmunmap(lsTable_, MAX_LS_GRID_SIZE);\n> +\t\t\tlsTable_ = nullptr;\n> +\t\t}\n> +\n> +\t\t/* Map the LS table buffer into user space (now element 1). */\n> +\t\tlsTableHandle_ = FileDescriptor(ipaConfig.data[1]);\n> +\t\tif (lsTableHandle_.isValid()) {\n> +\t\t\tlsTable_ = mmap(nullptr, MAX_LS_GRID_SIZE, PROT_READ | PROT_WRITE,\n> +\t\t\t\t\tMAP_SHARED, lsTableHandle_.fd(), 0);\n> +\n> +\t\t\tif (lsTable_ == MAP_FAILED) {\n> +\t\t\t\tLOG(IPARPI, Error) << \"dmaHeap mmap failure for LS table.\";\n> +\t\t\t\tlsTable_ = nullptr;\n> +\t\t\t}\n> +\t\t}\n> +\t}\n> +\n> \t/* Pass the camera mode to the CamHelper to setup algorithms. */\n> \thelper_->SetCameraMode(mode_);\n> \n> @@ -280,27 +307,6 @@ void IPARPi::configure(const CameraSensorInfo &sensorInfo,\n> \t}\n> \n> \tlastMode_ = mode_;\n> -\n> -\t/* Store the lens shading table pointer and handle if available. */\n> -\tif (ipaConfig.operation & RPI_IPA_CONFIG_LS_TABLE) {\n> -\t\t/* Remove any previous table, if there was one. */\n> -\t\tif (lsTable_) {\n> -\t\t\tmunmap(lsTable_, MAX_LS_GRID_SIZE);\n> -\t\t\tlsTable_ = nullptr;\n> -\t\t}\n> -\n> -\t\t/* Map the LS table buffer into user space. */\n> -\t\tlsTableHandle_ = FileDescriptor(ipaConfig.data[0]);\n> -\t\tif (lsTableHandle_.isValid()) {\n> -\t\t\tlsTable_ = mmap(nullptr, MAX_LS_GRID_SIZE, PROT_READ | PROT_WRITE,\n> -\t\t\t\t\tMAP_SHARED, lsTableHandle_.fd(), 0);\n> -\n> -\t\t\tif (lsTable_ == MAP_FAILED) {\n> -\t\t\t\tLOG(IPARPI, Error) << \"dmaHeap mmap failure for LS table.\";\n> -\t\t\t\tlsTable_ = nullptr;\n> -\t\t\t}\n> -\t\t}\n> -\t}\n> }\n> \n> void IPARPi::mapBuffers(const std::vector &buffers)\n> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> index 9d183e3..2fdf79f 100644\n> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> @@ -1141,6 +1141,9 @@ int RPiCameraData::configureIPA()\n> \tentityControls.emplace(0, unicam_[Unicam::Image].dev()->controls());\n> \tentityControls.emplace(1, isp_[Isp::Input].dev()->controls());\n> \n> +\t/* Always send the user transform to the IPA. */\n> +\tipaConfig.data = { static_cast(transform_) };\n> +\n> \t/* Allocate the lens shading table via dmaHeap and pass to the IPA. */\n> \tif (!lsTable_.isValid()) {\n> \t\tlsTable_ = dmaHeap_.alloc(\"ls_grid\", MAX_LS_GRID_SIZE);\n> @@ -1149,7 +1152,7 @@ int RPiCameraData::configureIPA()\n> \n> \t\t/* Allow the IPA to mmap the LS table via the file descriptor. */\n> \t\tipaConfig.operation = RPI_IPA_CONFIG_LS_TABLE;\n> -\t\tipaConfig.data = { static_cast(lsTable_.fd()) };\n> +\t\tipaConfig.data.push_back(static_cast(lsTable_.fd()));\n> \t}\n> \n> \tCameraSensorInfo sensorInfo = {};","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 D8E9CBE173\n\tfor ;\n\tWed, 19 Aug 2020 02:14:11 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6A49B61E0E;\n\tWed, 19 Aug 2020 04:14: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 90AA060384\n\tfor ;\n\tWed, 19 Aug 2020 04:14:10 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 20A8929E;\n\tWed, 19 Aug 2020 04:14:10 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"J37emOsq\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1597803250;\n\tbh=1FW6dOZMUGfab+g4+VnHgin5CF+gwo8MNf9gvAW4cug=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=J37emOsqT0675dgYoEGhx6Amp9CAUTjO4u8WdA4eGsVT4yfMOetaJkWVOlHzc3DGS\n\tIve91TErQ7e173ubVKQe3R6DgMvWyHRPHdIsgvDtCMJarno9uFl3bY2L7xYyDULYnA\n\tF30OTKM0YnN1lBHj3NLufpPPnxF5FbrXMdyoXt68=","Date":"Wed, 19 Aug 2020 05:13:52 +0300","From":"Laurent Pinchart ","To":"David Plowman ","Message-ID":"<20200819021352.GA20152@pendragon.ideasonboard.com>","References":"<20200806163639.12971-1-david.plowman@raspberrypi.com>\n\t<20200806163639.12971-5-david.plowman@raspberrypi.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200806163639.12971-5-david.plowman@raspberrypi.com>","Subject":"Re: [libcamera-devel] [PATCH v2 4/5] libcamera: raspberrypi: Plumb\n\tuser transform through to IPA","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]