From patchwork Wed Nov 4 07:48:39 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 10331 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id 27567BE080 for ; Wed, 4 Nov 2020 07:49:36 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 1BD9C62C74; Wed, 4 Nov 2020 08:49:35 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="vOYY5VUf"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id A8E7760344 for ; Wed, 4 Nov 2020 08:49:33 +0100 (CET) Received: from pendragon.lan (62-78-145-57.bb.dnainternet.fi [62.78.145.57]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 498B6BB5 for ; Wed, 4 Nov 2020 08:49:33 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1604476173; bh=FFO9/3LKr9nbjjQiIH1pYvqISBGmITwX54tqboMw+Sk=; h=From:To:Subject:Date:In-Reply-To:References:From; b=vOYY5VUf/Sq0DgwSU8QYfHQtT67AKoFrpb6UDFFS7R0V6kH0IcuynceldZD+bUCkD fteGQXRpeE4ie8N3xM544CY3EdhAcAOolzno1AHlHLZo8/lNT7NF4B1rW7zBSskdLs y4V1kO/sNLCgKVmHAsEbRnin1U09wsAf0EimO6EI= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Wed, 4 Nov 2020 09:48:39 +0200 Message-Id: <20201104074841.21676-2-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.27.0 In-Reply-To: <20201104074841.21676-1-laurent.pinchart@ideasonboard.com> References: <20201104074841.21676-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 1/3] libcamera: v4l2_videodevice: Zero-initialize planes in V4L2DeviceFormat 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: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" The V4L2DeviceFormat class doesn't have a default constructor, neither does it specifies default member initializers for the plane-related members. This results in the planes array and planesCount members being uninitialized by default, leading to undefined behaviour if the user of the class doesn't initialize it explicitly. Most users initialize V4L2DeviceFormat instances, but some don't. We could fix them, but that would likely turn into a game of whack-a-mole. As there's no use case for instantiating a large number of V4L2DeviceFormat instances in a performance-critical code path, let's instead add default initializers to avoid future issues. While at it, define a type of the structures containing plane information, and use an std::array. Signed-off-by: Laurent Pinchart Reviewed-by: Niklas Söderlund --- include/libcamera/internal/v4l2_videodevice.h | 13 ++++++++----- src/libcamera/v4l2_videodevice.cpp | 9 +++++++++ 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/include/libcamera/internal/v4l2_videodevice.h b/include/libcamera/internal/v4l2_videodevice.h index 53f6a2d5515b..dcb9654a34b8 100644 --- a/include/libcamera/internal/v4l2_videodevice.h +++ b/include/libcamera/internal/v4l2_videodevice.h @@ -7,6 +7,7 @@ #ifndef __LIBCAMERA_INTERNAL_V4L2_VIDEODEVICE_H__ #define __LIBCAMERA_INTERNAL_V4L2_VIDEODEVICE_H__ +#include #include #include #include @@ -153,14 +154,16 @@ private: class V4L2DeviceFormat { public: + struct Plane { + uint32_t size = 0; + uint32_t bpl = 0; + }; + V4L2PixelFormat fourcc; Size size; - struct { - uint32_t size; - uint32_t bpl; - } planes[3]; - unsigned int planesCount; + std::array planes; + unsigned int planesCount = 0; const std::string toString() const; }; diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp index 36d7d9a0f27a..d07c3530eea5 100644 --- a/src/libcamera/v4l2_videodevice.cpp +++ b/src/libcamera/v4l2_videodevice.cpp @@ -351,6 +351,15 @@ bool V4L2BufferCache::Entry::operator==(const FrameBuffer &buffer) const * V4L2DeviceFormat::planesCount value. */ +/** + * \struct V4L2DeviceFormat::Plane + * \brief Per-plane memory size information + * \var V4L2DeviceFormat::Plane::size + * \brief The plane total memory size (in bytes) + * \var V4L2DeviceFormat::Plane::bpl + * \brief The plane line stride (in bytes) + */ + /** * \var V4L2DeviceFormat::size * \brief The image size in pixels From patchwork Wed Nov 4 07:48:40 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 10332 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id ABF40BE081 for ; Wed, 4 Nov 2020 07:49:36 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 69E2062C81; Wed, 4 Nov 2020 08:49:35 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="d2CMbr4U"; dkim-atps=neutral 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 05F1060344 for ; Wed, 4 Nov 2020 08:49:34 +0100 (CET) Received: from pendragon.lan (62-78-145-57.bb.dnainternet.fi [62.78.145.57]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 9A115563 for ; Wed, 4 Nov 2020 08:49:33 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1604476173; bh=zCiPuyjQJ5xYMd/bWE5br2qVmIl/8gyav1Gsv9wkT04=; h=From:To:Subject:Date:In-Reply-To:References:From; b=d2CMbr4UPUbQda15Tr6Aram8L6FH6SarEqC/pdHMHVjPdQI2mZnSIVfOWuwLaq7DX VZKEAV22E7Cd3ZZfrE8UUkAc0svyeJlxhBKYURgq/X8/UbQDJ0MYs5stcpn89xBNJk 3bogYYgsyC2YMtfCyligJoTgS+9ngVfzTcHtsxak= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Wed, 4 Nov 2020 09:48:40 +0200 Message-Id: <20201104074841.21676-3-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.27.0 In-Reply-To: <20201104074841.21676-1-laurent.pinchart@ideasonboard.com> References: <20201104074841.21676-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 2/3] libcamera: v4l2_videodevice: Check plane count when setting format 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: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" When setting (or trying) a format with a multiplanar device, the V4L2VideoDevice::trySetFormatMeta() function iterates over all planes available in the V4L2DeviceFormat structure. The caller is responsible for setting the plane count, and failure to do so properly may result in memory corruption. This can lead to a crash way after the function returns, making the problem difficult to debug. As the issue is caused by a bug in the caller, use an assertion to catch it. Signed-off-by: Laurent Pinchart Reviewed-by: Niklas Söderlund Reviewed-by: Kieran Bingham --- src/libcamera/v4l2_videodevice.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp index d07c3530eea5..ac3c8c7dd97a 100644 --- a/src/libcamera/v4l2_videodevice.cpp +++ b/src/libcamera/v4l2_videodevice.cpp @@ -870,6 +870,8 @@ int V4L2VideoDevice::trySetFormatMultiplane(V4L2DeviceFormat *format, bool set) pix->num_planes = format->planesCount; pix->field = V4L2_FIELD_NONE; + ASSERT(pix->num_planes <= ARRAY_SIZE(pix->plane_fmt)); + for (unsigned int i = 0; i < pix->num_planes; ++i) { pix->plane_fmt[i].bytesperline = format->planes[i].bpl; pix->plane_fmt[i].sizeimage = format->planes[i].size; From patchwork Wed Nov 4 07:48:41 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 10333 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id CE586BE080 for ; Wed, 4 Nov 2020 07:49:36 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id D555962BDA; Wed, 4 Nov 2020 08:49:35 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="PWAilPfr"; dkim-atps=neutral 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 549D160344 for ; Wed, 4 Nov 2020 08:49:34 +0100 (CET) Received: from pendragon.lan (62-78-145-57.bb.dnainternet.fi [62.78.145.57]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id E9B44BB5 for ; Wed, 4 Nov 2020 08:49:33 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1604476174; bh=i6XuY8kIGS+d9rubR4xMyp3nMRcwAIa6IUuwOvZAuqg=; h=From:To:Subject:Date:In-Reply-To:References:From; b=PWAilPfrbNpKSAHVM/oJ4xiUzsmgYOIPhor6rDvfWjT0Nsb1FcI3U6bQuVie7mx4o +hFsb7DZnNMZ05mB4DRha30Ak/pisoJZShKX5XKqdbDoCIwx4R9HS9wjtfyet7hi66 JFkjY3J/lsAzndMWRzuvWD4d76egZiL24+23nWew= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Wed, 4 Nov 2020 09:48:41 +0200 Message-Id: <20201104074841.21676-4-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.27.0 In-Reply-To: <20201104074841.21676-1-laurent.pinchart@ideasonboard.com> References: <20201104074841.21676-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 3/3] libcamera: Drop unnecessary explicit initialization of V4L2DeviceFormat 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: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" The V4L2DeviceFormat class now has default initializers for all members, explicit initialization isn't needed anymore. Signed-off-by: Laurent Pinchart Reviewed-by: Niklas Söderlund --- src/libcamera/pipeline/ipu3/ipu3.cpp | 2 +- src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 6 +++--- src/libcamera/pipeline/rkisp1/rkisp1.cpp | 4 ++-- src/libcamera/pipeline/rkisp1/rkisp1_path.cpp | 4 ++-- src/libcamera/pipeline/simple/converter.cpp | 2 +- src/libcamera/pipeline/simple/simple.cpp | 4 ++-- src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 4 ++-- src/libcamera/pipeline/vimc/vimc.cpp | 4 ++-- 8 files changed, 15 insertions(+), 15 deletions(-) diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp index 5a6ee1a83e45..4cedb32bc239 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -463,7 +463,7 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c) * adjusted format to be propagated to the ImgU output devices. */ const Size &sensorSize = config->cio2Format().size; - V4L2DeviceFormat cio2Format = {}; + V4L2DeviceFormat cio2Format; ret = cio2->configure(sensorSize, &cio2Format); if (ret) return ret; diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp index 0a60442cf570..7ad66f21ffa3 100644 --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp @@ -73,7 +73,7 @@ V4L2DeviceFormat findBestMode(V4L2VideoDevice::Formats &formatsMap, const Size &req) { double bestScore = std::numeric_limits::max(), score; - V4L2DeviceFormat bestMode = {}; + V4L2DeviceFormat bestMode; #define PENALTY_AR 1500.0 #define PENALTY_8BIT 2000.0 @@ -429,7 +429,7 @@ CameraConfiguration::Status RPiCameraConfiguration::validate() status = Adjusted; } - V4L2DeviceFormat format = {}; + V4L2DeviceFormat format; format.fourcc = dev->toV4L2PixelFormat(cfg.pixelFormat); format.size = cfg.size; @@ -600,7 +600,7 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config) * See which streams are requested, and route the user * StreamConfiguration appropriately. */ - V4L2DeviceFormat format = {}; + V4L2DeviceFormat format; for (unsigned i = 0; i < config->size(); i++) { StreamConfiguration &cfg = config->at(i); diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index c74a2e9bd548..1b1922a9896c 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -720,13 +720,13 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c) return ret; } - V4L2DeviceFormat paramFormat = {}; + V4L2DeviceFormat paramFormat; paramFormat.fourcc = V4L2PixelFormat(V4L2_META_FMT_RK_ISP1_PARAMS); ret = param_->setFormat(¶mFormat); if (ret) return ret; - V4L2DeviceFormat statFormat = {}; + V4L2DeviceFormat statFormat; statFormat.fourcc = V4L2PixelFormat(V4L2_META_FMT_RK_ISP1_STAT_3A); ret = stat_->setFormat(&statFormat); if (ret) diff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp index e98515c878aa..80e1818d7907 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp @@ -85,7 +85,7 @@ CameraConfiguration::Status RkISP1Path::validate(StreamConfiguration *cfg) cfg->size.expandTo(minResolution_); cfg->bufferCount = RKISP1_BUFFER_COUNT; - V4L2DeviceFormat format = {}; + V4L2DeviceFormat format; format.fourcc = video_->toV4L2PixelFormat(cfg->pixelFormat); format.size = cfg->size; @@ -146,7 +146,7 @@ int RkISP1Path::configure(const StreamConfiguration &config, << ispFormat.toString(); const PixelFormatInfo &info = PixelFormatInfo::info(config.pixelFormat); - V4L2DeviceFormat outputFormat = {}; + V4L2DeviceFormat outputFormat; outputFormat.fourcc = video_->toV4L2PixelFormat(config.pixelFormat); outputFormat.size = config.size; outputFormat.planesCount = info.numPlanes(); diff --git a/src/libcamera/pipeline/simple/converter.cpp b/src/libcamera/pipeline/simple/converter.cpp index c21479a191fe..57538ab08fcd 100644 --- a/src/libcamera/pipeline/simple/converter.cpp +++ b/src/libcamera/pipeline/simple/converter.cpp @@ -265,7 +265,7 @@ std::tuple SimpleConverter::strideAndFrameSize(const Size &size, const PixelFormat &pixelFormat) { - V4L2DeviceFormat format = {}; + V4L2DeviceFormat format; format.fourcc = m2m_->capture()->toV4L2PixelFormat(pixelFormat); format.size = size; diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp index 3d2039f3f269..0d3078f770cf 100644 --- a/src/libcamera/pipeline/simple/simple.cpp +++ b/src/libcamera/pipeline/simple/simple.cpp @@ -482,7 +482,7 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate() /* Set the stride and frameSize. */ if (!needConversion_) { - V4L2DeviceFormat format = {}; + V4L2DeviceFormat format; format.fourcc = data_->video_->toV4L2PixelFormat(cfg.pixelFormat); format.size = cfg.size; @@ -587,7 +587,7 @@ int SimplePipelineHandler::configure(Camera *camera, CameraConfiguration *c) /* Configure the video node. */ V4L2PixelFormat videoFormat = video->toV4L2PixelFormat(pipeConfig.pixelFormat); - V4L2DeviceFormat captureFormat = {}; + V4L2DeviceFormat captureFormat; captureFormat.fourcc = videoFormat; captureFormat.size = pipeConfig.captureSize; diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp index 3862631b7bed..0f3241cc873a 100644 --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp @@ -154,7 +154,7 @@ CameraConfiguration::Status UVCCameraConfiguration::validate() cfg.bufferCount = 4; - V4L2DeviceFormat format = {}; + V4L2DeviceFormat format; format.fourcc = data_->video_->toV4L2PixelFormat(cfg.pixelFormat); format.size = cfg.size; @@ -210,7 +210,7 @@ int PipelineHandlerUVC::configure(Camera *camera, CameraConfiguration *config) StreamConfiguration &cfg = config->at(0); int ret; - V4L2DeviceFormat format = {}; + V4L2DeviceFormat format; format.fourcc = data->video_->toV4L2PixelFormat(cfg.pixelFormat); format.size = cfg.size; diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp index 7416c37c6f5a..914b6b54b0c4 100644 --- a/src/libcamera/pipeline/vimc/vimc.cpp +++ b/src/libcamera/pipeline/vimc/vimc.cpp @@ -172,7 +172,7 @@ CameraConfiguration::Status VimcCameraConfiguration::validate() cfg.bufferCount = 4; - V4L2DeviceFormat format = {}; + V4L2DeviceFormat format; format.fourcc = data_->video_->toV4L2PixelFormat(cfg.pixelFormat); format.size = cfg.size; @@ -276,7 +276,7 @@ int PipelineHandlerVimc::configure(Camera *camera, CameraConfiguration *config) if (ret) return ret; - V4L2DeviceFormat format = {}; + V4L2DeviceFormat format; format.fourcc = data->video_->toV4L2PixelFormat(cfg.pixelFormat); format.size = cfg.size;