From patchwork Sun Sep 29 19:02:44 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 2058 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 49FFF6165A for ; Sun, 29 Sep 2019 21:03:12 +0200 (CEST) Received: from pendragon.bb.dnainternet.fi (81-175-216-236.bb.dnainternet.fi [81.175.216.236]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id DEA13813 for ; Sun, 29 Sep 2019 21:03:11 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1569783792; bh=icoU5fJP8tsRcqI8mMUhffNRrhHR6pzlmD3k1yu5z9I=; h=From:To:Subject:Date:In-Reply-To:References:From; b=wgXPzB3wl9QmBnbZZ4IHxu64UjqRShFXLdV1zyhRX9gnqQQcrZfNcDk6qCBrUo/fm lqq/AHLcSYjGFgUmnAusazcBXQ1+zA+4YxqRqXg1ZFiq+/6nilcDj/wtVFN0W7Xxbf xvWAoEoNHr8TpwkX/Clt+1C+cJFV8jVR2s5VTkos= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Sun, 29 Sep 2019 22:02:44 +0300 Message-Id: <20190929190254.18920-4-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.21.0 In-Reply-To: <20190929190254.18920-1-laurent.pinchart@ideasonboard.com> References: <20190929190254.18920-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 03/13] libcamera: controls: Use explicit 32-bit integer types X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 29 Sep 2019 19:03:12 -0000 Make the control API more explicit when dealing with integer controls by specifying the size. We already do so for 64-bit integers, using int64_t and ControlTypeInteger64, do the same for 32-bit integers. Signed-off-by: Laurent Pinchart Reviewed-by: Jacopo Mondi Reviewed-by: Niklas Söderlund --- include/libcamera/controls.h | 6 ++--- src/libcamera/controls.cpp | 36 ++++++++++++++--------------- src/libcamera/pipeline/uvcvideo.cpp | 10 ++++---- src/libcamera/pipeline/vimc.cpp | 6 ++--- test/controls/control_info.cpp | 10 ++++---- test/controls/control_list.cpp | 16 ++++++------- test/controls/control_value.cpp | 6 ++--- 7 files changed, 46 insertions(+), 44 deletions(-) diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h index 0886370f0901..a3089064c4b5 100644 --- a/include/libcamera/controls.h +++ b/include/libcamera/controls.h @@ -21,7 +21,7 @@ class Camera; enum ControlType { ControlTypeNone, ControlTypeBool, - ControlTypeInteger, + ControlTypeInteger32, ControlTypeInteger64, }; @@ -30,7 +30,7 @@ class ControlValue public: ControlValue(); ControlValue(bool value); - ControlValue(int value); + ControlValue(int32_t value); ControlValue(int64_t value); ControlType type() const { return type_; }; @@ -48,7 +48,7 @@ private: union { bool bool_; - int integer_; + int32_t integer32_; int64_t integer64_; }; }; diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp index 88aab88db327..295ccd55a622 100644 --- a/src/libcamera/controls.cpp +++ b/src/libcamera/controls.cpp @@ -31,8 +31,8 @@ LOG_DEFINE_CATEGORY(Controls) * Invalid type, for empty values * \var ControlTypeBool * The control stores a boolean value - * \var ControlTypeInteger - * The control stores an integer value + * \var ControlTypeInteger32 + * The control stores a 32-bit integer value * \var ControlTypeInteger64 * The control stores a 64-bit integer value */ @@ -63,8 +63,8 @@ ControlValue::ControlValue(bool value) * \brief Construct an integer ControlValue * \param[in] value Integer value to store */ -ControlValue::ControlValue(int value) - : type_(ControlTypeInteger), integer_(value) +ControlValue::ControlValue(int32_t value) + : type_(ControlTypeInteger32), integer32_(value) { } @@ -115,17 +115,17 @@ const bool &ControlValue::get() const } template<> -const int &ControlValue::get() const +const int32_t &ControlValue::get() const { - ASSERT(type_ == ControlTypeInteger || type_ == ControlTypeInteger64); + ASSERT(type_ == ControlTypeInteger32 || type_ == ControlTypeInteger64); - return integer_; + return integer32_; } template<> const int64_t &ControlValue::get() const { - ASSERT(type_ == ControlTypeInteger || type_ == ControlTypeInteger64); + ASSERT(type_ == ControlTypeInteger32 || type_ == ControlTypeInteger64); return integer64_; } @@ -138,10 +138,10 @@ void ControlValue::set(const bool &value) } template<> -void ControlValue::set(const int &value) +void ControlValue::set(const int32_t &value) { - type_ = ControlTypeInteger; - integer_ = value; + type_ = ControlTypeInteger32; + integer32_ = value; } template<> @@ -163,8 +163,8 @@ std::string ControlValue::toString() const return ""; case ControlTypeBool: return bool_ ? "True" : "False"; - case ControlTypeInteger: - return std::to_string(integer_); + case ControlTypeInteger32: + return std::to_string(integer32_); case ControlTypeInteger64: return std::to_string(integer64_); } @@ -186,35 +186,35 @@ std::string ControlValue::toString() const /** * \var Brightness - * ControlType: Integer + * ControlType: Integer32 * * Specify a fixed brightness parameter. */ /** * \var Contrast - * ControlType: Integer + * ControlType: Integer32 * * Specify a fixed contrast parameter. */ /** * \var Saturation - * ControlType: Integer + * ControlType: Integer32 * * Specify a fixed saturation parameter. */ /** * \var ManualExposure - * ControlType: Integer + * ControlType: Integer32 * * Specify a fixed exposure time in milli-seconds */ /** * \var ManualGain - * ControlType: Integer + * ControlType: Integer32 * * Specify a fixed gain parameter */ diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp index 81c548af2c64..0d56758e3e1d 100644 --- a/src/libcamera/pipeline/uvcvideo.cpp +++ b/src/libcamera/pipeline/uvcvideo.cpp @@ -235,24 +235,24 @@ int PipelineHandlerUVC::processControls(UVCCameraData *data, Request *request) switch (ci->id()) { case Brightness: - controls.add(V4L2_CID_BRIGHTNESS, value.get()); + controls.add(V4L2_CID_BRIGHTNESS, value.get()); break; case Contrast: - controls.add(V4L2_CID_CONTRAST, value.get()); + controls.add(V4L2_CID_CONTRAST, value.get()); break; case Saturation: - controls.add(V4L2_CID_SATURATION, value.get()); + controls.add(V4L2_CID_SATURATION, value.get()); break; case ManualExposure: controls.add(V4L2_CID_EXPOSURE_AUTO, 1); - controls.add(V4L2_CID_EXPOSURE_ABSOLUTE, value.get()); + controls.add(V4L2_CID_EXPOSURE_ABSOLUTE, value.get()); break; case ManualGain: - controls.add(V4L2_CID_GAIN, value.get()); + controls.add(V4L2_CID_GAIN, value.get()); break; default: diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp index 3e34e7a0edbf..e549dc64b996 100644 --- a/src/libcamera/pipeline/vimc.cpp +++ b/src/libcamera/pipeline/vimc.cpp @@ -288,15 +288,15 @@ int PipelineHandlerVimc::processControls(VimcCameraData *data, Request *request) switch (ci->id()) { case Brightness: - controls.add(V4L2_CID_BRIGHTNESS, value.get()); + controls.add(V4L2_CID_BRIGHTNESS, value.get()); break; case Contrast: - controls.add(V4L2_CID_CONTRAST, value.get()); + controls.add(V4L2_CID_CONTRAST, value.get()); break; case Saturation: - controls.add(V4L2_CID_SATURATION, value.get()); + controls.add(V4L2_CID_SATURATION, value.get()); break; default: diff --git a/test/controls/control_info.cpp b/test/controls/control_info.cpp index faefaaa444d9..2aba568a302e 100644 --- a/test/controls/control_info.cpp +++ b/test/controls/control_info.cpp @@ -26,13 +26,14 @@ protected: ControlInfo info(Brightness); if (info.id() != Brightness || - info.type() != ControlTypeInteger || + info.type() != ControlTypeInteger32 || info.name() != std::string("Brightness")) { cout << "Invalid control identification for Brightness" << endl; return TestFail; } - if (info.min().get() != 0 || info.max().get() != 0) { + if (info.min().get() != 0 || + info.max().get() != 0) { cout << "Invalid control range for Brightness" << endl; return TestFail; } @@ -44,13 +45,14 @@ protected: info = ControlInfo(Contrast, 10, 200); if (info.id() != Contrast || - info.type() != ControlTypeInteger || + info.type() != ControlTypeInteger32 || info.name() != std::string("Contrast")) { cout << "Invalid control identification for Contrast" << endl; return TestFail; } - if (info.min().get() != 10 || info.max().get() != 200) { + if (info.min().get() != 10 || + info.max().get() != 200) { cout << "Invalid control range for Contrast" << endl; return TestFail; } diff --git a/test/controls/control_list.cpp b/test/controls/control_list.cpp index 0402e7c23dba..5215840b1c4e 100644 --- a/test/controls/control_list.cpp +++ b/test/controls/control_list.cpp @@ -96,7 +96,7 @@ protected: return TestFail; } - if (list[Brightness].get() != 255) { + if (list[Brightness].get() != 255) { cout << "Incorrest Brightness control value" << endl; return TestFail; } @@ -125,8 +125,8 @@ protected: /* * Test control value retrieval and update through ControlInfo. */ - if (list[brightness].get() != 64 || - list[contrast].get() != 128) { + if (list[brightness].get() != 64 || + list[contrast].get() != 128) { cout << "Failed to retrieve control value" << endl; return TestFail; } @@ -134,8 +134,8 @@ protected: list[brightness] = 10; list[contrast] = 20; - if (list[brightness].get() != 10 || - list[contrast].get() != 20) { + if (list[brightness].get() != 10 || + list[contrast].get() != 20) { cout << "Failed to update control value" << endl; return TestFail; } @@ -185,9 +185,9 @@ protected: return TestFail; } - if (newList[Brightness].get() != 10 || - newList[Contrast].get() != 20 || - newList[Saturation].get() != 255) { + if (newList[Brightness].get() != 10 || + newList[Contrast].get() != 20 || + newList[Saturation].get() != 255) { cout << "New list contains incorrect values" << endl; return TestFail; } diff --git a/test/controls/control_value.cpp b/test/controls/control_value.cpp index 397c43f799ad..a1ffa842f29e 100644 --- a/test/controls/control_value.cpp +++ b/test/controls/control_value.cpp @@ -27,7 +27,7 @@ protected: << " Bool: " << boolean.toString() << endl; - if (integer.get() != 1234) { + if (integer.get() != 1234) { cerr << "Failed to get Integer" << endl; return TestFail; } @@ -56,8 +56,8 @@ protected: return TestFail; } - value.set(10); - if (value.get() != 10) { + value.set(10); + if (value.get() != 10) { cerr << "Failed to get Integer" << endl; return TestFail; }