From patchwork Thu Mar 19 13:29:17 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 3197 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 59B5560418 for ; Thu, 19 Mar 2020 14:29:32 +0100 (CET) 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 E92B9DC4 for ; Thu, 19 Mar 2020 14:29:31 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1584624572; bh=bUTuSOV5wXoHGNDUOjiPzK8aicUrZSsPVTo7YDe+vlA=; h=From:To:Subject:Date:In-Reply-To:References:From; b=eP50FjtDmoad7iUiW3tvjG81ZL7NjamfO1ijtqI3J3knSycockxaB+7Eq5Ddqnvwx ax0XTkEUtqHYGom31AJQZwABOtLhpgJN0pybkhf+T4jH/Z6Yw4pFUedCqP6JPq0V8a 4mBdH+cFhj1vtgjvmRBXmxLuIc34d331Z6Q0GyCw= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Thu, 19 Mar 2020 15:29:17 +0200 Message-Id: <20200319132919.9563-2-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.24.1 In-Reply-To: <20200319132919.9563-1-laurent.pinchart@ideasonboard.com> References: <20200319132919.9563-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v3 1/3] libcamera: v4l2_videodevice: Add V4L2PixelFormat class 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: Thu, 19 Mar 2020 13:29:32 -0000 The V4L2PixelFormat class describes the pixel format of a V4L2 buffer. It wraps the V4L2 numerical FourCC, and shall be used in all APIs that deal with V4L2 pixel formats. Its purpose is to prevent unintentional confusion of V4L2 and DRM FourCCs in code by catching implicit conversion attempts at compile time. The constructor taking a V4L2 FourCC integer value will be made explicit in a further commit to minimize the size of this change and keep it reviewable. Signed-off-by: Laurent Pinchart Reviewed-by: Niklas Söderlund Reviewed-by: Jacopo Mondi --- Changes since v2: - Renamed value() to fourcc() - Documentation updates Changes since v1: - Add and use V4L2PixelFormat::toString() --- src/libcamera/include/v4l2_videodevice.h | 39 +++++++--- src/libcamera/pipeline/uvcvideo.cpp | 4 +- src/libcamera/v4l2_videodevice.cpp | 91 ++++++++++++++++++++---- 3 files changed, 109 insertions(+), 25 deletions(-) diff --git a/src/libcamera/include/v4l2_videodevice.h b/src/libcamera/include/v4l2_videodevice.h index bc1c6a4d10c7..2a380c0e61cd 100644 --- a/src/libcamera/include/v4l2_videodevice.h +++ b/src/libcamera/include/v4l2_videodevice.h @@ -149,10 +149,33 @@ private: unsigned int missCounter_; }; +class V4L2PixelFormat +{ +public: + V4L2PixelFormat() + : fourcc_(0) + { + } + + V4L2PixelFormat(uint32_t fourcc) + : fourcc_(fourcc) + { + } + + bool isValid() const { return fourcc_ != 0; } + uint32_t fourcc() const { return fourcc_; } + operator uint32_t() const { return fourcc_; } + + std::string toString() const; + +private: + uint32_t fourcc_; +}; + class V4L2DeviceFormat { public: - uint32_t fourcc; + V4L2PixelFormat fourcc; Size size; struct { @@ -184,7 +207,7 @@ public: int getFormat(V4L2DeviceFormat *format); int setFormat(V4L2DeviceFormat *format); - ImageFormats formats(); + std::map> formats(); int setCrop(Rectangle *rect); int setCompose(Rectangle *rect); @@ -205,10 +228,10 @@ public: static V4L2VideoDevice *fromEntityName(const MediaDevice *media, const std::string &entity); - static PixelFormat toPixelFormat(uint32_t v4l2Fourcc); - uint32_t toV4L2Fourcc(const PixelFormat &pixelFormat); - static uint32_t toV4L2Fourcc(const PixelFormat &pixelFormat, - bool multiplanar); + static PixelFormat toPixelFormat(V4L2PixelFormat v4l2Fourcc); + V4L2PixelFormat toV4L2Fourcc(const PixelFormat &pixelFormat); + static V4L2PixelFormat toV4L2Fourcc(const PixelFormat &pixelFormat, + bool multiplanar); protected: std::string logPrefix() const; @@ -223,8 +246,8 @@ private: int getFormatSingleplane(V4L2DeviceFormat *format); int setFormatSingleplane(V4L2DeviceFormat *format); - std::vector enumPixelformats(); - std::vector enumSizes(unsigned int pixelFormat); + std::vector enumPixelformats(); + std::vector enumSizes(V4L2PixelFormat pixelFormat); int setSelection(unsigned int target, Rectangle *rect); diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp index 731149755728..67750fbc7c0c 100644 --- a/src/libcamera/pipeline/uvcvideo.cpp +++ b/src/libcamera/pipeline/uvcvideo.cpp @@ -154,8 +154,8 @@ CameraConfiguration *PipelineHandlerUVC::generateConfiguration(Camera *camera, if (roles.empty()) return config; - std::map> v4l2Formats = - data->video_->formats().data(); + std::map> v4l2Formats = + data->video_->formats(); std::map> deviceFormats; std::transform(v4l2Formats.begin(), v4l2Formats.end(), std::inserter(deviceFormats, deviceFormats.begin()), diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp index 56251a465807..1c60014caa6f 100644 --- a/src/libcamera/v4l2_videodevice.cpp +++ b/src/libcamera/v4l2_videodevice.cpp @@ -278,6 +278,65 @@ bool V4L2BufferCache::Entry::operator==(const FrameBuffer &buffer) const return true; } +/** + * \class V4L2PixelFormat + * \brief V4L2 pixel format FourCC wrapper + * + * The V4L2PixelFormat class describes the pixel format of a V4L2 buffer. It + * wraps the V4L2 numerical FourCC, and shall be used in all APIs that deal with + * V4L2 pixel formats. Its purpose is to prevent unintentional confusion of + * V4L2 and DRM FourCCs in code by catching implicit conversion attempts at + * compile time. + */ + +/** + * \fn V4L2PixelFormat::V4L2PixelFormat() + * \brief Construct a V4L2PixelFormat with an invalid format + * + * V4L2PixelFormat instances constructed with the default constructor are + * invalid, calling the isValid() function returns false. + */ + +/** + * \fn V4L2PixelFormat::V4L2PixelFormat(uint32_t fourcc) + * \brief Construct a V4L2PixelFormat from a FourCC value + * \param[in] fourcc The pixel format FourCC numerical value + */ + +/** + * \fn bool V4L2PixelFormat::isValid() const + * \brief Check if the pixel format is valid + * + * V4L2PixelFormat instances constructed with the default constructor are + * invalid. Instances constructed with a FourCC defined in the V4L2 API are + * valid. The behaviour is undefined otherwise. + * + * \return True if the pixel format is valid, false otherwise + */ + +/** + * \fn uint32_t V4L2PixelFormat::fourcc() const + * \brief Retrieve the pixel format FourCC numerical value + * \return The pixel format FourCC numerical value + */ + +/** + * \fn V4L2PixelFormat::operator uint32_t() const + * \brief Convert to the pixel format FourCC numerical value + * \return The pixel format FourCC numerical value + */ + +/** + * \brief Assemble and return a string describing the pixel format + * \return A string describing the pixel format + */ +std::string V4L2PixelFormat::toString() const +{ + char str[11]; + snprintf(str, 11, "0x%08x", fourcc_); + return str; +} + /** * \class V4L2DeviceFormat * \brief The V4L2 video device image format and sizes @@ -386,7 +445,7 @@ bool V4L2BufferCache::Entry::operator==(const FrameBuffer &buffer) const const std::string V4L2DeviceFormat::toString() const { std::stringstream ss; - ss << size.toString() << "-" << utils::hex(fourcc); + ss << size.toString() << "-" << fourcc.toString(); return ss.str(); } @@ -901,29 +960,31 @@ int V4L2VideoDevice::setFormatSingleplane(V4L2DeviceFormat *format) * * \return A list of the supported video device formats */ -ImageFormats V4L2VideoDevice::formats() +std::map> V4L2VideoDevice::formats() { - ImageFormats formats; + std::map> formats; - for (unsigned int pixelformat : enumPixelformats()) { - std::vector sizes = enumSizes(pixelformat); + for (V4L2PixelFormat pixelFormat : enumPixelformats()) { + std::vector sizes = enumSizes(pixelFormat); if (sizes.empty()) return {}; - if (formats.addFormat(pixelformat, sizes)) { + if (formats.find(pixelFormat) != formats.end()) { LOG(V4L2, Error) << "Could not add sizes for pixel format " - << pixelformat; + << pixelFormat; return {}; } + + formats.emplace(pixelFormat, sizes); } return formats; } -std::vector V4L2VideoDevice::enumPixelformats() +std::vector V4L2VideoDevice::enumPixelformats() { - std::vector formats; + std::vector formats; int ret; for (unsigned int index = 0; ; index++) { @@ -948,7 +1009,7 @@ std::vector V4L2VideoDevice::enumPixelformats() return formats; } -std::vector V4L2VideoDevice::enumSizes(unsigned int pixelFormat) +std::vector V4L2VideoDevice::enumSizes(V4L2PixelFormat pixelFormat) { std::vector sizes; int ret; @@ -1563,7 +1624,7 @@ V4L2VideoDevice *V4L2VideoDevice::fromEntityName(const MediaDevice *media, * \param[in] v4l2Fourcc The V4L2 pixel format (V4L2_PIX_FORMAT_*) * \return The PixelFormat corresponding to \a v4l2Fourcc */ -PixelFormat V4L2VideoDevice::toPixelFormat(uint32_t v4l2Fourcc) +PixelFormat V4L2VideoDevice::toPixelFormat(V4L2PixelFormat v4l2Fourcc) { switch (v4l2Fourcc) { /* RGB formats. */ @@ -1612,7 +1673,7 @@ PixelFormat V4L2VideoDevice::toPixelFormat(uint32_t v4l2Fourcc) libcamera::_log(__FILE__, __LINE__, _LOG_CATEGORY(V4L2)(), LogError).stream() << "Unsupported V4L2 pixel format " - << utils::hex(v4l2Fourcc); + << v4l2Fourcc.toString(); return PixelFormat(); } } @@ -1628,7 +1689,7 @@ PixelFormat V4L2VideoDevice::toPixelFormat(uint32_t v4l2Fourcc) * * \return The V4L2_PIX_FMT_* pixel format code corresponding to \a pixelFormat */ -uint32_t V4L2VideoDevice::toV4L2Fourcc(const PixelFormat &pixelFormat) +V4L2PixelFormat V4L2VideoDevice::toV4L2Fourcc(const PixelFormat &pixelFormat) { return V4L2VideoDevice::toV4L2Fourcc(pixelFormat, caps_.isMultiplanar()); } @@ -1646,8 +1707,8 @@ uint32_t V4L2VideoDevice::toV4L2Fourcc(const PixelFormat &pixelFormat) * * \return The V4L2_PIX_FMT_* pixel format code corresponding to \a pixelFormat */ -uint32_t V4L2VideoDevice::toV4L2Fourcc(const PixelFormat &pixelFormat, - bool multiplanar) +V4L2PixelFormat V4L2VideoDevice::toV4L2Fourcc(const PixelFormat &pixelFormat, + bool multiplanar) { switch (pixelFormat) { /* RGB formats. */ From patchwork Thu Mar 19 13:29:18 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 3198 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 9F25260418 for ; Thu, 19 Mar 2020 14:29:32 +0100 (CET) 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 47025A53 for ; Thu, 19 Mar 2020 14:29:32 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1584624572; bh=R5rI25mDATFbMNVt1IovmuGW/3TyOnkakPB/CKkJIdI=; h=From:To:Subject:Date:In-Reply-To:References:From; b=QAbT6fxaB4sD12VHpjnArMMdlhLSUm6zJwh6WCqnEWyDwHsgVi912p3qCK5P1biUw RoOfzJbzUWxYiePoWIqzFEMXqYjqGmMBpzee1yRF0P/5cgAz9Xa12qtbECWHQvP0u9 7p7S2YY0Rz4UTfQHJGgQ3C/pRnW2hGZx7uRaDEZQ= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Thu, 19 Mar 2020 15:29:18 +0200 Message-Id: <20200319132919.9563-3-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.24.1 In-Reply-To: <20200319132919.9563-1-laurent.pinchart@ideasonboard.com> References: <20200319132919.9563-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v3 2/3] libcamera: v4l2_videodevice: Rename toV4L2Fourcc to toV4L2PixelFormat 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: Thu, 19 Mar 2020 13:29:32 -0000 Now that the functions return a V4L2PixelFormat, adapt their name accordingly. Signed-off-by: Laurent Pinchart Reviewed-by: Jacopo Mondi --- src/libcamera/include/v4l2_videodevice.h | 6 +++--- src/libcamera/pipeline/ipu3/ipu3.cpp | 2 +- src/libcamera/pipeline/rkisp1/rkisp1.cpp | 4 ++-- src/libcamera/pipeline/uvcvideo.cpp | 4 ++-- src/libcamera/pipeline/vimc.cpp | 4 ++-- src/libcamera/v4l2_videodevice.cpp | 8 ++++---- test/libtest/buffer_source.cpp | 3 ++- 7 files changed, 16 insertions(+), 15 deletions(-) diff --git a/src/libcamera/include/v4l2_videodevice.h b/src/libcamera/include/v4l2_videodevice.h index 2a380c0e61cd..5e40c0c57df9 100644 --- a/src/libcamera/include/v4l2_videodevice.h +++ b/src/libcamera/include/v4l2_videodevice.h @@ -229,9 +229,9 @@ public: const std::string &entity); static PixelFormat toPixelFormat(V4L2PixelFormat v4l2Fourcc); - V4L2PixelFormat toV4L2Fourcc(const PixelFormat &pixelFormat); - static V4L2PixelFormat toV4L2Fourcc(const PixelFormat &pixelFormat, - bool multiplanar); + V4L2PixelFormat toV4L2PixelFormat(const PixelFormat &pixelFormat); + static V4L2PixelFormat toV4L2PixelFormat(const PixelFormat &pixelFormat, + bool multiplanar); protected: std::string logPrefix() const; diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp index 55ce8fa16af1..e9085f2f0c7e 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -1078,7 +1078,7 @@ int ImgUDevice::configureOutput(ImgUOutput *output, return 0; V4L2DeviceFormat outputFormat = {}; - outputFormat.fourcc = dev->toV4L2Fourcc(PixelFormat(DRM_FORMAT_NV12)); + outputFormat.fourcc = dev->toV4L2PixelFormat(PixelFormat(DRM_FORMAT_NV12)); outputFormat.size = cfg.size; outputFormat.planesCount = 2; diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index 737e331459f3..07d837abe1ac 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -626,7 +626,7 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c) LOG(RkISP1, Debug) << "Resizer output pad configured with " << format.toString(); V4L2DeviceFormat outputFormat = {}; - outputFormat.fourcc = video_->toV4L2Fourcc(cfg.pixelFormat); + outputFormat.fourcc = video_->toV4L2PixelFormat(cfg.pixelFormat); outputFormat.size = cfg.size; outputFormat.planesCount = 2; @@ -635,7 +635,7 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c) return ret; if (outputFormat.size != cfg.size || - outputFormat.fourcc != video_->toV4L2Fourcc(cfg.pixelFormat)) { + outputFormat.fourcc != video_->toV4L2PixelFormat(cfg.pixelFormat)) { LOG(RkISP1, Error) << "Unable to configure capture in " << cfg.toString(); return -EINVAL; diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp index 67750fbc7c0c..15c8baef1a24 100644 --- a/src/libcamera/pipeline/uvcvideo.cpp +++ b/src/libcamera/pipeline/uvcvideo.cpp @@ -187,7 +187,7 @@ int PipelineHandlerUVC::configure(Camera *camera, CameraConfiguration *config) int ret; V4L2DeviceFormat format = {}; - format.fourcc = data->video_->toV4L2Fourcc(cfg.pixelFormat); + format.fourcc = data->video_->toV4L2PixelFormat(cfg.pixelFormat); format.size = cfg.size; ret = data->video_->setFormat(&format); @@ -195,7 +195,7 @@ int PipelineHandlerUVC::configure(Camera *camera, CameraConfiguration *config) return ret; if (format.size != cfg.size || - format.fourcc != data->video_->toV4L2Fourcc(cfg.pixelFormat)) + format.fourcc != data->video_->toV4L2PixelFormat(cfg.pixelFormat)) return -EINVAL; cfg.setStream(&data->stream_); diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp index cbf330614bd6..1097eea243a1 100644 --- a/src/libcamera/pipeline/vimc.cpp +++ b/src/libcamera/pipeline/vimc.cpp @@ -229,7 +229,7 @@ int PipelineHandlerVimc::configure(Camera *camera, CameraConfiguration *config) return ret; V4L2DeviceFormat format = {}; - format.fourcc = data->video_->toV4L2Fourcc(cfg.pixelFormat); + format.fourcc = data->video_->toV4L2PixelFormat(cfg.pixelFormat); format.size = cfg.size; ret = data->video_->setFormat(&format); @@ -237,7 +237,7 @@ int PipelineHandlerVimc::configure(Camera *camera, CameraConfiguration *config) return ret; if (format.size != cfg.size || - format.fourcc != data->video_->toV4L2Fourcc(cfg.pixelFormat)) + format.fourcc != data->video_->toV4L2PixelFormat(cfg.pixelFormat)) return -EINVAL; /* diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp index 1c60014caa6f..496bc56b6f89 100644 --- a/src/libcamera/v4l2_videodevice.cpp +++ b/src/libcamera/v4l2_videodevice.cpp @@ -1689,9 +1689,9 @@ PixelFormat V4L2VideoDevice::toPixelFormat(V4L2PixelFormat v4l2Fourcc) * * \return The V4L2_PIX_FMT_* pixel format code corresponding to \a pixelFormat */ -V4L2PixelFormat V4L2VideoDevice::toV4L2Fourcc(const PixelFormat &pixelFormat) +V4L2PixelFormat V4L2VideoDevice::toV4L2PixelFormat(const PixelFormat &pixelFormat) { - return V4L2VideoDevice::toV4L2Fourcc(pixelFormat, caps_.isMultiplanar()); + return toV4L2PixelFormat(pixelFormat, caps_.isMultiplanar()); } /** @@ -1707,8 +1707,8 @@ V4L2PixelFormat V4L2VideoDevice::toV4L2Fourcc(const PixelFormat &pixelFormat) * * \return The V4L2_PIX_FMT_* pixel format code corresponding to \a pixelFormat */ -V4L2PixelFormat V4L2VideoDevice::toV4L2Fourcc(const PixelFormat &pixelFormat, - bool multiplanar) +V4L2PixelFormat V4L2VideoDevice::toV4L2PixelFormat(const PixelFormat &pixelFormat, + bool multiplanar) { switch (pixelFormat) { /* RGB formats. */ diff --git a/test/libtest/buffer_source.cpp b/test/libtest/buffer_source.cpp index 26d2764d5f8f..dae3cb9f7a6c 100644 --- a/test/libtest/buffer_source.cpp +++ b/test/libtest/buffer_source.cpp @@ -70,7 +70,8 @@ int BufferSource::allocate(const StreamConfiguration &config) } format.size = config.size; - format.fourcc = V4L2VideoDevice::toV4L2Fourcc(config.pixelFormat, false); + format.fourcc = V4L2VideoDevice::toV4L2PixelFormat(config.pixelFormat, + false); if (video->setFormat(&format)) { std::cout << "Failed to set format on output device" << std::endl; return TestFail; From patchwork Thu Mar 19 13:29:19 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 3199 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 05B3060418 for ; Thu, 19 Mar 2020 14:29:33 +0100 (CET) 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 990D4DC4 for ; Thu, 19 Mar 2020 14:29:32 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1584624572; bh=njSmoKzy56KouFiiq1hsvFm3Hudc7adDRYvxNre+ArE=; h=From:To:Subject:Date:In-Reply-To:References:From; b=BcyJNaQpzVByd6i51W5nEsLmYpyENoSc3SgyyJnpg6BS7RXOou4ARU23P5mM2tdnB 0NP3CDl2HvVMtgCH9AJmqdfWw2SjX/USFedcAb5x0lNV1vPNEG3BmBujClVxN05/WY /PXzyzX50zh2FNaJI0Bh5gB24y+ocD8ncHVFxH7I= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Thu, 19 Mar 2020 15:29:19 +0200 Message-Id: <20200319132919.9563-4-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.24.1 In-Reply-To: <20200319132919.9563-1-laurent.pinchart@ideasonboard.com> References: <20200319132919.9563-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v3 3/3] libcamera: v4l2_videodevice: Make V4L2PixelFormat constructor explicit 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: Thu, 19 Mar 2020 13:29:33 -0000 To achieve the goal of preventing unwanted conversion between a DRM and a V4L2 FourCC, make the V4L2PixelFormat constructor that takes an integer value explicit. All users of V4L2 pixel formats flagged by the compiler are fixed. Signed-off-by: Laurent Pinchart Reviewed-by: Niklas Söderlund Reviewed-by: Jacopo Mondi --- src/libcamera/include/v4l2_videodevice.h | 2 +- src/libcamera/pipeline/ipu3/ipu3.cpp | 14 +++---- src/libcamera/pipeline/rkisp1/rkisp1.cpp | 4 +- src/libcamera/pipeline/vimc.cpp | 2 +- src/libcamera/v4l2_videodevice.cpp | 42 ++++++++++--------- .../v4l2_videodevice_test.cpp | 2 +- 6 files changed, 35 insertions(+), 31 deletions(-) diff --git a/src/libcamera/include/v4l2_videodevice.h b/src/libcamera/include/v4l2_videodevice.h index 5e40c0c57df9..7d7c4a9e6ebd 100644 --- a/src/libcamera/include/v4l2_videodevice.h +++ b/src/libcamera/include/v4l2_videodevice.h @@ -157,7 +157,7 @@ public: { } - V4L2PixelFormat(uint32_t fourcc) + explicit V4L2PixelFormat(uint32_t fourcc) : fourcc_(fourcc) { } diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp index e9085f2f0c7e..1b44460e4d88 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -121,7 +121,7 @@ public: int start(); int stop(); - static int mediaBusToFormat(unsigned int code); + static V4L2PixelFormat mediaBusToFormat(unsigned int code); V4L2VideoDevice *output_; V4L2Subdevice *csi2_; @@ -1447,19 +1447,19 @@ int CIO2Device::stop() return output_->streamOff(); } -int CIO2Device::mediaBusToFormat(unsigned int code) +V4L2PixelFormat CIO2Device::mediaBusToFormat(unsigned int code) { switch (code) { case MEDIA_BUS_FMT_SBGGR10_1X10: - return V4L2_PIX_FMT_IPU3_SBGGR10; + return V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SBGGR10); case MEDIA_BUS_FMT_SGBRG10_1X10: - return V4L2_PIX_FMT_IPU3_SGBRG10; + return V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SGBRG10); case MEDIA_BUS_FMT_SGRBG10_1X10: - return V4L2_PIX_FMT_IPU3_SGRBG10; + return V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SGRBG10); case MEDIA_BUS_FMT_SRGGB10_1X10: - return V4L2_PIX_FMT_IPU3_SRGGB10; + return V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SRGGB10); default: - return -EINVAL; + return {}; } } diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index 07d837abe1ac..97bb4f72cde5 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -642,13 +642,13 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c) } V4L2DeviceFormat paramFormat = {}; - paramFormat.fourcc = V4L2_META_FMT_RK_ISP1_PARAMS; + paramFormat.fourcc = V4L2PixelFormat(V4L2_META_FMT_RK_ISP1_PARAMS); ret = param_->setFormat(¶mFormat); if (ret) return ret; V4L2DeviceFormat statFormat = {}; - statFormat.fourcc = V4L2_META_FMT_RK_ISP1_STAT_3A; + statFormat.fourcc = V4L2PixelFormat(V4L2_META_FMT_RK_ISP1_STAT_3A); ret = stat_->setFormat(&statFormat); if (ret) return ret; diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp index 1097eea243a1..50cab8aff7a3 100644 --- a/src/libcamera/pipeline/vimc.cpp +++ b/src/libcamera/pipeline/vimc.cpp @@ -244,7 +244,7 @@ int PipelineHandlerVimc::configure(Camera *camera, CameraConfiguration *config) * Format has to be set on the raw capture video node, otherwise the * vimc driver will fail pipeline validation. */ - format.fourcc = V4L2_PIX_FMT_SGRBG8; + format.fourcc = V4L2PixelFormat(V4L2_PIX_FMT_SGRBG8); format.size = { cfg.size.width / 3, cfg.size.height / 3 }; ret = data->raw_->setFormat(&format); diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp index 496bc56b6f89..b20c8c77260f 100644 --- a/src/libcamera/v4l2_videodevice.cpp +++ b/src/libcamera/v4l2_videodevice.cpp @@ -287,6 +287,10 @@ bool V4L2BufferCache::Entry::operator==(const FrameBuffer &buffer) const * V4L2 pixel formats. Its purpose is to prevent unintentional confusion of * V4L2 and DRM FourCCs in code by catching implicit conversion attempts at * compile time. + * + * To achieve this goal, construction of a V4L2PixelFormat from an integer value + * is explicit. To retrieve the integer value of a V4L2PixelFormat, both the + * explicit value() and implicit uint32_t conversion operators may be used. */ /** @@ -795,7 +799,7 @@ int V4L2VideoDevice::getFormatMeta(V4L2DeviceFormat *format) format->size.width = 0; format->size.height = 0; - format->fourcc = pix->dataformat; + format->fourcc = V4L2PixelFormat(pix->dataformat); format->planesCount = 1; format->planes[0].bpl = pix->buffersize; format->planes[0].size = pix->buffersize; @@ -847,7 +851,7 @@ int V4L2VideoDevice::getFormatMultiplane(V4L2DeviceFormat *format) format->size.width = pix->width; format->size.height = pix->height; - format->fourcc = pix->pixelformat; + format->fourcc = V4L2PixelFormat(pix->pixelformat); format->planesCount = pix->num_planes; for (unsigned int i = 0; i < format->planesCount; ++i) { @@ -888,7 +892,7 @@ int V4L2VideoDevice::setFormatMultiplane(V4L2DeviceFormat *format) */ format->size.width = pix->width; format->size.height = pix->height; - format->fourcc = pix->pixelformat; + format->fourcc = V4L2PixelFormat(pix->pixelformat); format->planesCount = pix->num_planes; for (unsigned int i = 0; i < format->planesCount; ++i) { format->planes[i].bpl = pix->plane_fmt[i].bytesperline; @@ -913,7 +917,7 @@ int V4L2VideoDevice::getFormatSingleplane(V4L2DeviceFormat *format) format->size.width = pix->width; format->size.height = pix->height; - format->fourcc = pix->pixelformat; + format->fourcc = V4L2PixelFormat(pix->pixelformat); format->planesCount = 1; format->planes[0].bpl = pix->bytesperline; format->planes[0].size = pix->sizeimage; @@ -945,7 +949,7 @@ int V4L2VideoDevice::setFormatSingleplane(V4L2DeviceFormat *format) */ format->size.width = pix->width; format->size.height = pix->height; - format->fourcc = pix->pixelformat; + format->fourcc = V4L2PixelFormat(pix->pixelformat); format->planesCount = 1; format->planes[0].bpl = pix->bytesperline; format->planes[0].size = pix->sizeimage; @@ -996,7 +1000,7 @@ std::vector V4L2VideoDevice::enumPixelformats() if (ret) break; - formats.push_back(pixelformatEnum.pixelformat); + formats.push_back(V4L2PixelFormat(pixelformatEnum.pixelformat)); } if (ret && ret != -EINVAL) { @@ -1713,21 +1717,21 @@ V4L2PixelFormat V4L2VideoDevice::toV4L2PixelFormat(const PixelFormat &pixelForma switch (pixelFormat) { /* RGB formats. */ case DRM_FORMAT_BGR888: - return V4L2_PIX_FMT_RGB24; + return V4L2PixelFormat(V4L2_PIX_FMT_RGB24); case DRM_FORMAT_RGB888: - return V4L2_PIX_FMT_BGR24; + return V4L2PixelFormat(V4L2_PIX_FMT_BGR24); case DRM_FORMAT_BGRA8888: - return V4L2_PIX_FMT_ARGB32; + return V4L2PixelFormat(V4L2_PIX_FMT_ARGB32); /* YUV packed formats. */ case DRM_FORMAT_YUYV: - return V4L2_PIX_FMT_YUYV; + return V4L2PixelFormat(V4L2_PIX_FMT_YUYV); case DRM_FORMAT_YVYU: - return V4L2_PIX_FMT_YVYU; + return V4L2PixelFormat(V4L2_PIX_FMT_YVYU); case DRM_FORMAT_UYVY: - return V4L2_PIX_FMT_UYVY; + return V4L2PixelFormat(V4L2_PIX_FMT_UYVY); case DRM_FORMAT_VYUY: - return V4L2_PIX_FMT_VYUY; + return V4L2PixelFormat(V4L2_PIX_FMT_VYUY); /* * YUY planar formats. @@ -1736,17 +1740,17 @@ V4L2PixelFormat V4L2VideoDevice::toV4L2PixelFormat(const PixelFormat &pixelForma * also take into account the formats supported by the device. */ case DRM_FORMAT_NV16: - return V4L2_PIX_FMT_NV16; + return V4L2PixelFormat(V4L2_PIX_FMT_NV16); case DRM_FORMAT_NV61: - return V4L2_PIX_FMT_NV61; + return V4L2PixelFormat(V4L2_PIX_FMT_NV61); case DRM_FORMAT_NV12: - return V4L2_PIX_FMT_NV12; + return V4L2PixelFormat(V4L2_PIX_FMT_NV12); case DRM_FORMAT_NV21: - return V4L2_PIX_FMT_NV21; + return V4L2PixelFormat(V4L2_PIX_FMT_NV21); /* Compressed formats. */ case DRM_FORMAT_MJPEG: - return V4L2_PIX_FMT_MJPEG; + return V4L2PixelFormat(V4L2_PIX_FMT_MJPEG); } /* @@ -1755,7 +1759,7 @@ V4L2PixelFormat V4L2VideoDevice::toV4L2PixelFormat(const PixelFormat &pixelForma */ libcamera::_log(__FILE__, __LINE__, _LOG_CATEGORY(V4L2)(), LogError).stream() << "Unsupported V4L2 pixel format " << pixelFormat.toString(); - return 0; + return {}; } /** diff --git a/test/v4l2_videodevice/v4l2_videodevice_test.cpp b/test/v4l2_videodevice/v4l2_videodevice_test.cpp index 577da4cb601c..93b9e72da5b4 100644 --- a/test/v4l2_videodevice/v4l2_videodevice_test.cpp +++ b/test/v4l2_videodevice/v4l2_videodevice_test.cpp @@ -69,7 +69,7 @@ int V4L2VideoDeviceTest::init() if (debayer_->open()) return TestFail; - format.fourcc = V4L2_PIX_FMT_SBGGR8; + format.fourcc = V4L2PixelFormat(V4L2_PIX_FMT_SBGGR8); V4L2SubdeviceFormat subformat = {}; subformat.mbus_code = MEDIA_BUS_FMT_SBGGR8_1X8;