From patchwork Mon Oct 24 00:03:51 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 17671 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 946E7C3289 for ; Mon, 24 Oct 2022 00:04:37 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id D33FB62EFD; Mon, 24 Oct 2022 02:04:36 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org; s=mail; t=1666569876; bh=w3agMv/Il0W8IEOQXHr443qEB6P2AARPOi0AzE1asvM=; h=To:Date:In-Reply-To:References:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=ZzIITo4lv70RWso/P9nDz4lk826ywDDVLYCKOy1Sgel57Y818lWWWVlhJVWcWpvJ8 lKFgBo8c8dv+T0ouT1N4cAkEvIrafsQoTcFRtovE8oLDFtAupl9wJW+vuaINocPrQT 1qk+R7BN1i7+i4h1mrqVDTtXwQJE9Ow82gHlC/cENEUVqQM9tuBBYJeTV1hr/PEO7p gskHTqKsu/ZX3u9P5z9JCuiSqpe2BqqSPdgm6/04vcysRMR9YqoWOhp6OMx2I5GxDx EOzYL4V1qdCLeMaz025nOi+H4GFCnF1yAUuxzChaPK5B9IRUDnhhs8+aDwskmAqEs/ 26ts7lpoUr0EA== Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 64C8F62ED2 for ; Mon, 24 Oct 2022 02:04:32 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="RqgHJXyh"; dkim-atps=neutral Received: from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi [62.78.145.57]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id EDB4088F; Mon, 24 Oct 2022 02:04:31 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1666569872; bh=w3agMv/Il0W8IEOQXHr443qEB6P2AARPOi0AzE1asvM=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=RqgHJXyhPldfMhE/CzxP5pc1BkTpPz+fhQL8nLFltmm1pwt82b4rfLWoo3wvEm1pY GrdxwC/qpQvoSifgyCpN6X0HMzFCoEqhY5hQ1w1Q4iobI3jbE7Xnlf7IlMvc0KZLlq 9zAB/YefuoKIsgCDVxRyT4pK5bTal5PEUJtAFupc= To: libcamera-devel@lists.libcamera.org Date: Mon, 24 Oct 2022 03:03:51 +0300 Message-Id: <20221024000356.29521-9-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.37.4 In-Reply-To: <20221024000356.29521-1-laurent.pinchart@ideasonboard.com> References: <20221024000356.29521-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v3 08/13] libcamera: pipeline_handler: Return unique_ptr from generateConfiguration() 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-Patchwork-Original-From: Laurent Pinchart via libcamera-devel From: Laurent Pinchart Reply-To: Laurent Pinchart Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" The PipelineHandler::generateConfiguration() function allocates a CameraConfiguration instance and returns it. The ownership of the instance is transferred to the caller. This is a perfect match for a std::unique_ptr<>, which the Camera::generateConfiguration() function already returns. Update PipelineHandler::generateConfiguration() to match it. This fixes a memory leak in one of the error return paths in the IPU3 pipeline handler. While at it, update the Camera::generateConfiguration() function documentation to drop the sentence that describes the ownership transfer, as that is implied by usage of std::unique_ptr<>. Signed-off-by: Laurent Pinchart Reviewed-by: Paul Elder Reviewed-by: Jacopo Mondi Reviewed-by: Kieran Bingham --- include/libcamera/internal/pipeline_handler.h | 2 +- src/libcamera/camera.cpp | 8 ++++---- src/libcamera/pipeline/ipu3/ipu3.cpp | 10 +++++----- src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 12 ++++++------ src/libcamera/pipeline/rkisp1/rkisp1.cpp | 9 +++++---- src/libcamera/pipeline/simple/simple.cpp | 12 ++++++------ src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 8 +++++--- src/libcamera/pipeline/vimc/vimc.cpp | 8 +++++--- src/libcamera/pipeline_handler.cpp | 3 +-- 9 files changed, 38 insertions(+), 34 deletions(-) diff --git a/include/libcamera/internal/pipeline_handler.h b/include/libcamera/internal/pipeline_handler.h index b6139a88d421..96aab9d6459e 100644 --- a/include/libcamera/internal/pipeline_handler.h +++ b/include/libcamera/internal/pipeline_handler.h @@ -48,7 +48,7 @@ public: bool acquire(); void release(); - virtual CameraConfiguration *generateConfiguration(Camera *camera, + virtual std::unique_ptr generateConfiguration(Camera *camera, const StreamRoles &roles) = 0; virtual int configure(Camera *camera, CameraConfiguration *config) = 0; diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp index 9fe29ca9dca5..daef77016971 100644 --- a/src/libcamera/camera.cpp +++ b/src/libcamera/camera.cpp @@ -934,8 +934,7 @@ const std::set &Camera::streams() const * \context This function is \threadsafe. * * \return A CameraConfiguration if the requested roles can be satisfied, or a - * null pointer otherwise. The ownership of the returned configuration is - * passed to the caller. + * null pointer otherwise. */ std::unique_ptr Camera::generateConfiguration(const StreamRoles &roles) { @@ -949,7 +948,8 @@ std::unique_ptr Camera::generateConfiguration(const StreamR if (roles.size() > streams().size()) return nullptr; - CameraConfiguration *config = d->pipe_->generateConfiguration(this, roles); + std::unique_ptr config = + d->pipe_->generateConfiguration(this, roles); if (!config) { LOG(Camera, Debug) << "Pipeline handler failed to generate configuration"; @@ -966,7 +966,7 @@ std::unique_ptr Camera::generateConfiguration(const StreamR LOG(Camera, Debug) << msg.str(); - return std::unique_ptr(config); + return config; } /** diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp index 3b892d9671c5..e4d79ea44aed 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -136,7 +136,7 @@ public: PipelineHandlerIPU3(CameraManager *manager); - CameraConfiguration *generateConfiguration(Camera *camera, + std::unique_ptr generateConfiguration(Camera *camera, const StreamRoles &roles) override; int configure(Camera *camera, CameraConfiguration *config) override; @@ -424,11 +424,12 @@ PipelineHandlerIPU3::PipelineHandlerIPU3(CameraManager *manager) { } -CameraConfiguration *PipelineHandlerIPU3::generateConfiguration(Camera *camera, - const StreamRoles &roles) +std::unique_ptr +PipelineHandlerIPU3::generateConfiguration(Camera *camera, const StreamRoles &roles) { IPU3CameraData *data = cameraData(camera); - IPU3CameraConfiguration *config = new IPU3CameraConfiguration(data); + std::unique_ptr config = + std::make_unique(data); if (roles.empty()) return config; @@ -494,7 +495,6 @@ CameraConfiguration *PipelineHandlerIPU3::generateConfiguration(Camera *camera, default: LOG(IPU3, Error) << "Requested stream role not supported: " << role; - delete config; return nullptr; } diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp index 343f8cb2c7ed..7c54550005fa 100644 --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp @@ -323,7 +323,8 @@ class PipelineHandlerRPi : public PipelineHandler public: PipelineHandlerRPi(CameraManager *manager); - CameraConfiguration *generateConfiguration(Camera *camera, const StreamRoles &roles) override; + std::unique_ptr generateConfiguration(Camera *camera, + const StreamRoles &roles) override; int configure(Camera *camera, CameraConfiguration *config) override; int exportFrameBuffers(Camera *camera, Stream *stream, @@ -561,11 +562,12 @@ PipelineHandlerRPi::PipelineHandlerRPi(CameraManager *manager) { } -CameraConfiguration *PipelineHandlerRPi::generateConfiguration(Camera *camera, - const StreamRoles &roles) +std::unique_ptr +PipelineHandlerRPi::generateConfiguration(Camera *camera, const StreamRoles &roles) { RPiCameraData *data = cameraData(camera); - CameraConfiguration *config = new RPiCameraConfiguration(data); + std::unique_ptr config = + std::make_unique(data); V4L2SubdeviceFormat sensorFormat; unsigned int bufferCount; PixelFormat pixelFormat; @@ -640,13 +642,11 @@ CameraConfiguration *PipelineHandlerRPi::generateConfiguration(Camera *camera, default: LOG(RPI, Error) << "Requested stream role not supported: " << role; - delete config; return nullptr; } if (rawCount > 1 || outCount > 2) { LOG(RPI, Error) << "Invalid stream roles requested"; - delete config; return nullptr; } diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index 1418dc9a47fb..50da92d4d6f8 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -142,7 +142,7 @@ class PipelineHandlerRkISP1 : public PipelineHandler public: PipelineHandlerRkISP1(CameraManager *manager); - CameraConfiguration *generateConfiguration(Camera *camera, + std::unique_ptr generateConfiguration(Camera *camera, const StreamRoles &roles) override; int configure(Camera *camera, CameraConfiguration *config) override; @@ -539,7 +539,8 @@ PipelineHandlerRkISP1::PipelineHandlerRkISP1(CameraManager *manager) * Pipeline Operations */ -CameraConfiguration *PipelineHandlerRkISP1::generateConfiguration(Camera *camera, +std::unique_ptr +PipelineHandlerRkISP1::generateConfiguration(Camera *camera, const StreamRoles &roles) { RkISP1CameraData *data = cameraData(camera); @@ -550,7 +551,8 @@ CameraConfiguration *PipelineHandlerRkISP1::generateConfiguration(Camera *camera return nullptr; } - CameraConfiguration *config = new RkISP1CameraConfiguration(camera, data); + std::unique_ptr config = + std::make_unique(camera, data); if (roles.empty()) return config; @@ -595,7 +597,6 @@ CameraConfiguration *PipelineHandlerRkISP1::generateConfiguration(Camera *camera default: LOG(RkISP1, Warning) << "Requested stream role not supported: " << role; - delete config; return nullptr; } diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp index 37b3e7acd0a1..a32de7f36e13 100644 --- a/src/libcamera/pipeline/simple/simple.cpp +++ b/src/libcamera/pipeline/simple/simple.cpp @@ -311,8 +311,8 @@ class SimplePipelineHandler : public PipelineHandler public: SimplePipelineHandler(CameraManager *manager); - CameraConfiguration *generateConfiguration(Camera *camera, - const StreamRoles &roles) override; + std::unique_ptr generateConfiguration(Camera *camera, + const StreamRoles &roles) override; int configure(Camera *camera, CameraConfiguration *config) override; int exportFrameBuffers(Camera *camera, Stream *stream, @@ -1037,12 +1037,12 @@ SimplePipelineHandler::SimplePipelineHandler(CameraManager *manager) { } -CameraConfiguration *SimplePipelineHandler::generateConfiguration(Camera *camera, - const StreamRoles &roles) +std::unique_ptr +SimplePipelineHandler::generateConfiguration(Camera *camera, const StreamRoles &roles) { SimpleCameraData *data = cameraData(camera); - CameraConfiguration *config = - new SimpleCameraConfiguration(camera, data); + std::unique_ptr config = + std::make_unique(camera, data); if (roles.empty()) return config; diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp index a28195450634..277465b72164 100644 --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp @@ -74,7 +74,7 @@ class PipelineHandlerUVC : public PipelineHandler public: PipelineHandlerUVC(CameraManager *manager); - CameraConfiguration *generateConfiguration(Camera *camera, + std::unique_ptr generateConfiguration(Camera *camera, const StreamRoles &roles) override; int configure(Camera *camera, CameraConfiguration *config) override; @@ -178,11 +178,13 @@ PipelineHandlerUVC::PipelineHandlerUVC(CameraManager *manager) { } -CameraConfiguration *PipelineHandlerUVC::generateConfiguration(Camera *camera, +std::unique_ptr +PipelineHandlerUVC::generateConfiguration(Camera *camera, const StreamRoles &roles) { UVCCameraData *data = cameraData(camera); - CameraConfiguration *config = new UVCCameraConfiguration(data); + std::unique_ptr config = + std::make_unique(data); if (roles.empty()) return config; diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp index f85d05f77a61..204f5ad73f6d 100644 --- a/src/libcamera/pipeline/vimc/vimc.cpp +++ b/src/libcamera/pipeline/vimc/vimc.cpp @@ -84,7 +84,7 @@ class PipelineHandlerVimc : public PipelineHandler public: PipelineHandlerVimc(CameraManager *manager); - CameraConfiguration *generateConfiguration(Camera *camera, + std::unique_ptr generateConfiguration(Camera *camera, const StreamRoles &roles) override; int configure(Camera *camera, CameraConfiguration *config) override; @@ -189,11 +189,13 @@ PipelineHandlerVimc::PipelineHandlerVimc(CameraManager *manager) { } -CameraConfiguration *PipelineHandlerVimc::generateConfiguration(Camera *camera, +std::unique_ptr +PipelineHandlerVimc::generateConfiguration(Camera *camera, const StreamRoles &roles) { VimcCameraData *data = cameraData(camera); - CameraConfiguration *config = new VimcCameraConfiguration(data); + std::unique_ptr config = + std::make_unique(data); if (roles.empty()) return config; diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp index 588a3db30e82..825aff5ac20a 100644 --- a/src/libcamera/pipeline_handler.cpp +++ b/src/libcamera/pipeline_handler.cpp @@ -233,8 +233,7 @@ void PipelineHandler::unlockMediaDevices() * handler. * * \return A valid CameraConfiguration if the requested roles can be satisfied, - * or a null pointer otherwise. The ownership of the returned configuration is - * passed to the caller. + * or a null pointer otherwise. */ /**