[{"id":17150,"web_url":"https://patchwork.libcamera.org/comment/17150/","msgid":"","date":"2021-05-24T00:56:18","subject":"Re: [libcamera-devel] [PATCH v3 3/6] ipa: ipc: Rename\n\tCameraSensorInfo to IPACameraSensorInfo","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Umang,\n\nThank you for the patch.\n\nOn Fri, May 21, 2021 at 06:58:20PM +0530, Umang Jain wrote:\n> This matches the naming convention for IPA IPC.\n> \n> Signed-off-by: Umang Jain \n> Acked-by: Paul Elder \n> Reviewed-by: Jean-Michel Hautbois \n> Reviewed-by: Kieran Bingham \n> ---\n> Documentation/guides/ipa.rst | 8 ++--\n> include/libcamera/internal/camera_sensor.h | 2 +-\n> include/libcamera/ipa/core.mojom | 2 +-\n> include/libcamera/ipa/raspberrypi.mojom | 2 +-\n> include/libcamera/ipa/rkisp1.mojom | 2 +-\n> src/ipa/ipu3/ipu3_agc.cpp | 2 +-\n> src/ipa/raspberrypi/raspberrypi.cpp | 8 ++--\n> src/ipa/rkisp1/rkisp1.cpp | 6 +--\n> src/libcamera/camera_sensor.cpp | 6 +--\n> src/libcamera/ipa/core_ipa_interface.cpp | 38 +++++++++----------\n> src/libcamera/pipeline/ipu3/ipu3.cpp | 4 +-\n> .../pipeline/raspberrypi/raspberrypi.cpp | 4 +-\n> src/libcamera/pipeline/rkisp1/rkisp1.cpp | 2 +-\n> 13 files changed, 43 insertions(+), 43 deletions(-)\n> \n> diff --git a/Documentation/guides/ipa.rst b/Documentation/guides/ipa.rst\n> index cbffbd9a..6d460e13 100644\n> --- a/Documentation/guides/ipa.rst\n> +++ b/Documentation/guides/ipa.rst\n> @@ -74,7 +74,7 @@ serialization, any custom data containers must be defined with the mojo IDL.\n> The following list of libcamera objects are supported in the interface\n> definition, and may be used as function parameter types or struct field types:\n> \n> -- libcamera.CameraSensorInfo\n> +- libcamera.IPACameraSensorInfo\n\nAlphabetically sorted please :-)\n\nReviewed-by: Laurent Pinchart \n\n> - libcamera.ControlInfoMap\n> - libcamera.ControlList\n> - libcamera.FileDescriptor\n> @@ -208,7 +208,7 @@ The following is an example of a main interface definition:\n> start() => (int32 ret);\n> stop();\n> \n> - configure(libcamera.CameraSensorInfo sensorInfo,\n> + configure(libcamera.IPACameraSensorInfo sensorInfo,\n> map streamConfig,\n> map entityControls,\n> ConfigInput ipaConfig)\n> @@ -470,7 +470,7 @@ definition:\n> \n> .. code-block:: none\n> \n> - configure(libcamera.CameraSensorInfo sensorInfo,\n> + configure(libcamera.IPACameraSensorInfo sensorInfo,\n> uint32 exampleNumber,\n> map streamConfig,\n> map entityControls,\n> @@ -481,7 +481,7 @@ We will need to implement a function with the following function signature:\n> \n> .. code-block:: C++\n> \n> - int configure(const CameraSensorInfo &sensorInfo,\n> + int configure(const IPACameraSensorInfo &sensorInfo,\n> uint32_t exampleNumber,\n> const std::map &streamConfig,\n> const std::map &entityControls,\n> diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h\n> index 0905ebfa..cf6c1c1e 100644\n> --- a/include/libcamera/internal/camera_sensor.h\n> +++ b/include/libcamera/internal/camera_sensor.h\n> @@ -51,7 +51,7 @@ public:\n> \tV4L2Subdevice *device() { return subdev_.get(); }\n> \n> \tconst ControlList &properties() const { return properties_; }\n> -\tint sensorInfo(CameraSensorInfo *info) const;\n> +\tint sensorInfo(IPACameraSensorInfo *info) const;\n> \n> \tvoid updateControlInfo();\n> \n> diff --git a/include/libcamera/ipa/core.mojom b/include/libcamera/ipa/core.mojom\n> index b95b3dc4..34a799c2 100644\n> --- a/include/libcamera/ipa/core.mojom\n> +++ b/include/libcamera/ipa/core.mojom\n> @@ -78,7 +78,7 @@ module libcamera;\n> \tuint32 height;\n> };\n> \n> -struct CameraSensorInfo {\n> +struct IPACameraSensorInfo {\n> \tstring model;\n> \n> \tuint32 bitsPerPixel;\n> diff --git a/include/libcamera/ipa/raspberrypi.mojom b/include/libcamera/ipa/raspberrypi.mojom\n> index 770e3049..db1f689f 100644\n> --- a/include/libcamera/ipa/raspberrypi.mojom\n> +++ b/include/libcamera/ipa/raspberrypi.mojom\n> @@ -68,7 +68,7 @@ interface IPARPiInterface {\n> \t * The \\a ipaConfig and \\a controls parameters carry data passed by the\n> \t * pipeline handler to the IPA and back.\n> \t */\n> -\tconfigure(libcamera.CameraSensorInfo sensorInfo,\n> +\tconfigure(libcamera.IPACameraSensorInfo sensorInfo,\n> \t\t map streamConfig,\n> \t\t map entityControls,\n> \t\t IPAConfig ipaConfig)\n> diff --git a/include/libcamera/ipa/rkisp1.mojom b/include/libcamera/ipa/rkisp1.mojom\n> index da6646df..55fa43be 100644\n> --- a/include/libcamera/ipa/rkisp1.mojom\n> +++ b/include/libcamera/ipa/rkisp1.mojom\n> @@ -34,7 +34,7 @@ interface IPARkISP1Interface {\n> \tstart() => (int32 ret);\n> \tstop();\n> \n> -\tconfigure(libcamera.CameraSensorInfo sensorInfo,\n> +\tconfigure(libcamera.IPACameraSensorInfo sensorInfo,\n> \t\t map streamConfig,\n> \t\t map entityControls)\n> \t\t=> (int32 ret);\n> diff --git a/src/ipa/ipu3/ipu3_agc.cpp b/src/ipa/ipu3/ipu3_agc.cpp\n> index ca54d89a..8bae423f 100644\n> --- a/src/ipa/ipu3/ipu3_agc.cpp\n> +++ b/src/ipa/ipu3/ipu3_agc.cpp\n> @@ -39,7 +39,7 @@ static constexpr uint32_t kMaxGain = kMaxISO / 100;\n> static constexpr uint32_t kMinExposure = 1;\n> static constexpr uint32_t kMaxExposure = 1976;\n> \n> -/* \\todo those should be get from CameraSensorInfo ! */\n> +/* \\todo those should be got from IPACameraSensorInfo ! */\n> /* line duration in microseconds */\n> static constexpr double kLineDuration = 16.8;\n> static constexpr double kMaxExposureTime = kMaxExposure * kLineDuration;\n> diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp\n> index 87774500..e5bb8159 100644\n> --- a/src/ipa/raspberrypi/raspberrypi.cpp\n> +++ b/src/ipa/raspberrypi/raspberrypi.cpp\n> @@ -89,7 +89,7 @@ public:\n> \tvoid start(const ControlList &controls, ipa::RPi::StartConfig *startConfig) override;\n> \tvoid stop() override {}\n> \n> -\tint configure(const CameraSensorInfo &sensorInfo,\n> +\tint configure(const IPACameraSensorInfo &sensorInfo,\n> \t\t const std::map &streamConfig,\n> \t\t const std::map &entityControls,\n> \t\t const ipa::RPi::IPAConfig &data,\n> @@ -101,7 +101,7 @@ public:\n> \tvoid signalIspPrepare(const ipa::RPi::ISPConfig &data) override;\n> \n> private:\n> -\tvoid setMode(const CameraSensorInfo &sensorInfo);\n> +\tvoid setMode(const IPACameraSensorInfo &sensorInfo);\n> \tbool validateSensorControls();\n> \tbool validateIspControls();\n> \tvoid queueRequest(const ControlList &controls);\n> @@ -278,7 +278,7 @@ void IPARPi::start(const ControlList &controls, ipa::RPi::StartConfig *startConf\n> \tlastRunTimestamp_ = 0;\n> }\n> \n> -void IPARPi::setMode(const CameraSensorInfo &sensorInfo)\n> +void IPARPi::setMode(const IPACameraSensorInfo &sensorInfo)\n> {\n> \tmode_.bitdepth = sensorInfo.bitsPerPixel;\n> \tmode_.width = sensorInfo.outputSize.width;\n> @@ -323,7 +323,7 @@ void IPARPi::setMode(const CameraSensorInfo &sensorInfo)\n> \tmode_.max_frame_length = sensorInfo.maxFrameLength;\n> }\n> \n> -int IPARPi::configure(const CameraSensorInfo &sensorInfo,\n> +int IPARPi::configure(const IPACameraSensorInfo &sensorInfo,\n> \t\t [[maybe_unused]] const std::map &streamConfig,\n> \t\t const std::map &entityControls,\n> \t\t const ipa::RPi::IPAConfig &ipaConfig,\n> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\n> index 6d45673c..b47ea324 100644\n> --- a/src/ipa/rkisp1/rkisp1.cpp\n> +++ b/src/ipa/rkisp1/rkisp1.cpp\n> @@ -37,7 +37,7 @@ public:\n> \tint start() override;\n> \tvoid stop() override {}\n> \n> -\tint configure(const CameraSensorInfo &info,\n> +\tint configure(const IPACameraSensorInfo &info,\n> \t\t const std::map &streamConfig,\n> \t\t const std::map &entityControls) override;\n> \tvoid mapBuffers(const std::vector &buffers) override;\n> @@ -90,12 +90,12 @@ int IPARkISP1::start()\n> }\n> \n> /**\n> - * \\todo The RkISP1 pipeline currently provides an empty CameraSensorInfo\n> + * \\todo The RkISP1 pipeline currently provides an empty IPACameraSensorInfo\n> * if the connected sensor does not provide enough information to properly\n> * assemble one. Make sure the reported sensor information are relevant\n> * before accessing them.\n> */\n> -int IPARkISP1::configure([[maybe_unused]] const CameraSensorInfo &info,\n> +int IPARkISP1::configure([[maybe_unused]] const IPACameraSensorInfo &info,\n> \t\t\t [[maybe_unused]] const std::map &streamConfig,\n> \t\t\t const std::map &entityControls)\n> {\n> diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp\n> index 170de827..0fb8a258 100644\n> --- a/src/libcamera/camera_sensor.cpp\n> +++ b/src/libcamera/camera_sensor.cpp\n> @@ -205,7 +205,7 @@ int CameraSensor::validateSensorDriver()\n> \t *\n> \t * Failures in reading any of the targets are not deemed to be fatal,\n> \t * but some properties and features, like constructing a\n> -\t * CameraSensorInfo for the IPA module, won't be supported.\n> +\t * IPACameraSensorInfo for the IPA module, won't be supported.\n> \t *\n> \t * \\todo Make support for selection targets mandatory as soon as all\n> \t * test platforms have been updated.\n> @@ -674,7 +674,7 @@ int CameraSensor::setControls(ControlList *ctrls)\n> *\n> * \\return 0 on success, a negative error code otherwise\n> */\n> -int CameraSensor::sensorInfo(CameraSensorInfo *info) const\n> +int CameraSensor::sensorInfo(IPACameraSensorInfo *info) const\n> {\n> \tif (!bayerFormat_)\n> \t\treturn -EINVAL;\n> @@ -701,7 +701,7 @@ int CameraSensor::sensorInfo(CameraSensorInfo *info) const\n> \t}\n> \n> \t/*\n> -\t * CameraSensorInfo::analogCrop::x and CameraSensorInfo::analogCrop::y\n> +\t * IPACameraSensorInfo::analogCrop::x and IPACameraSensorInfo::analogCrop::y\n> \t * are defined relatively to the active pixel area, while V4L2's\n> \t * TGT_CROP target is defined in respect to the full pixel array.\n> \t *\n> diff --git a/src/libcamera/ipa/core_ipa_interface.cpp b/src/libcamera/ipa/core_ipa_interface.cpp\n> index 7fef2c36..47c8e93d 100644\n> --- a/src/libcamera/ipa/core_ipa_interface.cpp\n> +++ b/src/libcamera/ipa/core_ipa_interface.cpp\n> @@ -103,7 +103,7 @@ namespace libcamera {\n> */\n> \n> /**\n> - * \\struct CameraSensorInfo\n> + * \\struct IPACameraSensorInfo\n> * \\brief Report the image sensor characteristics\n> *\n> * The structure reports image sensor characteristics used by IPA modules to\n> @@ -121,15 +121,15 @@ namespace libcamera {\n> */\n> \n> /**\n> - * \\fn CameraSensorInfo::CameraSensorInfo(const std::string &model,\n> - * uint32_t bitsPerPixel,\n> - * const Size &activeAreaSize,\n> - * const Rectangle &analogCrop,\n> - * const Size &outputSize,\n> - * uint64_t pixelRate,\n> - * uint32_t lineLength,\n> - * uint32_t minFrameLength,\n> - * uint32_t maxFrameLength)\n> + * \\fn IPACameraSensorInfo::IPACameraSensorInfo(const std::string &model,\n> + * uint32_t bitsPerPixel,\n> + * const Size &activeAreaSize,\n> + * const Rectangle &analogCrop,\n> + * const Size &outputSize,\n> + * uint64_t pixelRate,\n> + * uint32_t lineLength,\n> + * uint32_t minFrameLength,\n> + * uint32_t maxFrameLength)\n> *\n> * \\param[in] model\n> * \\param[in] bitsPerPixel\n> @@ -143,7 +143,7 @@ namespace libcamera {\n> */\n> \n> /**\n> - * \\var CameraSensorInfo::model\n> + * \\var IPACameraSensorInfo::model\n> * \\brief The image sensor model name\n> *\n> * The sensor model name is a free-formed string that uniquely identifies the\n> @@ -151,18 +151,18 @@ namespace libcamera {\n> */\n> \n> /**\n> - * \\var CameraSensorInfo::bitsPerPixel\n> + * \\var IPACameraSensorInfo::bitsPerPixel\n> * \\brief The number of bits per pixel of the image format produced by the\n> * image sensor\n> */\n> \n> /**\n> - * \\var CameraSensorInfo::activeAreaSize\n> + * \\var IPACameraSensorInfo::activeAreaSize\n> * \\brief The size of the pixel array active area of the sensor\n> */\n> \n> /**\n> - * \\var CameraSensorInfo::analogCrop\n> + * \\var IPACameraSensorInfo::analogCrop\n> * \\brief The portion of the pixel array active area which is read-out and\n> * processed\n> *\n> @@ -175,7 +175,7 @@ namespace libcamera {\n> */\n> \n> /**\n> - * \\var CameraSensorInfo::outputSize\n> + * \\var IPACameraSensorInfo::outputSize\n> * \\brief The size of the images produced by the camera sensor\n> *\n> * The output image size defines the horizontal and vertical sizes of the images\n> @@ -188,7 +188,7 @@ namespace libcamera {\n> */\n> \n> /**\n> - * \\var CameraSensorInfo::pixelRate\n> + * \\var IPACameraSensorInfo::pixelRate\n> * \\brief The number of pixels produced in a second\n> *\n> * To obtain the read-out time in seconds of a full line:\n> @@ -199,14 +199,14 @@ namespace libcamera {\n> */\n> \n> /**\n> - * \\var CameraSensorInfo::lineLength\n> + * \\var IPACameraSensorInfo::lineLength\n> * \\brief Total line length in pixels\n> *\n> * The total line length in pixel clock periods, including blanking.\n> */\n> \n> /**\n> - * \\var CameraSensorInfo::minFrameLength\n> + * \\var IPACameraSensorInfo::minFrameLength\n> * \\brief The minimum allowable frame length in units of lines\n> *\n> * The sensor frame length comprises of active output lines and blanking lines\n> @@ -221,7 +221,7 @@ namespace libcamera {\n> */\n> \n> /**\n> - * \\var CameraSensorInfo::maxFrameLength\n> + * \\var IPACameraSensorInfo::maxFrameLength\n> * \\brief The maximum allowable frame length in units of lines\n> *\n> * The sensor frame length comprises of active output lines and blanking lines\n> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> index ade8ffbd..98c6160f 100644\n> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> @@ -544,7 +544,7 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)\n> \tif (ret)\n> \t\treturn ret;\n> \n> -\tCameraSensorInfo sensorInfo;\n> +\tIPACameraSensorInfo sensorInfo;\n> \tcio2->sensor()->sensorInfo(&sensorInfo);\n> \tdata->cropRegion_ = sensorInfo.analogCrop;\n> \n> @@ -883,7 +883,7 @@ int PipelineHandlerIPU3::initControls(IPU3CameraData *data)\n> \tif (ret)\n> \t\treturn ret;\n> \n> -\tCameraSensorInfo sensorInfo{};\n> +\tIPACameraSensorInfo sensorInfo{};\n> \tret = sensor->sensorInfo(&sensorInfo);\n> \tif (ret)\n> \t\treturn ret;\n> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> index 0a71325a..aa4a0e72 100644\n> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> @@ -211,7 +211,7 @@ public:\n> \tBayerFormat::Order nativeBayerOrder_;\n> \n> \t/* For handling digital zoom. */\n> -\tCameraSensorInfo sensorInfo_;\n> +\tIPACameraSensorInfo sensorInfo_;\n> \tRectangle ispCrop_; /* crop in ISP (camera mode) pixels */\n> \tRectangle scalerCrop_; /* crop in sensor native pixels */\n> \tSize ispMinCropSize_;\n> @@ -1276,7 +1276,7 @@ int RPiCameraData::configureIPA(const CameraConfiguration *config)\n> \t\tipaConfig.lsTableHandle = lsTable_;\n> \t}\n> \n> -\t/* We store the CameraSensorInfo for digital zoom calculations. */\n> +\t/* We store the IPACameraSensorInfo for digital zoom calculations. */\n> \tint ret = sensor_->sensorInfo(&sensorInfo_);\n> \tif (ret) {\n> \t\tLOG(RPI, Error) << \"Failed to retrieve camera sensor info\";\n> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> index adebe8b5..6699839c 100644\n> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> @@ -648,7 +648,7 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)\n> \t\treturn ret;\n> \n> \t/* Inform IPA of stream configuration and sensor controls. */\n> -\tCameraSensorInfo sensorInfo = {};\n> +\tIPACameraSensorInfo sensorInfo = {};\n> \tret = data->sensor_->sensorInfo(&sensorInfo);\n> \tif (ret) {\n> \t\t/* \\todo Turn this into a hard failure. */","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 D1395C3201\n\tfor ;\n\tMon, 24 May 2021 00:56:23 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 514A16891B;\n\tMon, 24 May 2021 02:56:23 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 96AEB6050F\n\tfor ;\n\tMon, 24 May 2021 02:56:22 +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 E8270476;\n\tMon, 24 May 2021 02:56:21 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"IOcF05Ef\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1621817782;\n\tbh=lolcXXNQnMCvWgXq6uXQLtG9r7TmGhzZiYY9CiEkczU=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=IOcF05EftCS/+VUJfpVXSJCEu0odwAVNDI0H3US/OeaPJ5lrPP3HLMAATpoM9FHPc\n\t2UJmUcOdMMZ/H3jTj7s8dcA1g1TkqWu8nkK9bVn3nJ2MHPZgp8Y2ez8z372NPyaAU1\n\tEoDtUwmEtQvh3R98grZr5V73pAIGspWad0m+dnww=","Date":"Mon, 24 May 2021 03:56:18 +0300","From":"Laurent Pinchart ","To":"Umang Jain ","Message-ID":"","References":"<20210521132823.322076-1-umang.jain@ideasonboard.com>\n\t<20210521132823.322076-4-umang.jain@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20210521132823.322076-4-umang.jain@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v3 3/6] ipa: ipc: Rename\n\tCameraSensorInfo to IPACameraSensorInfo","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]