From a07968bed27638f799adb825266a8417f6f3cdb3 Mon Sep 17 00:00:00 2001 From: Laurent Pinchart Date: Sat, 22 Oct 2022 23:53:26 +0300 Subject: libcamera: pipeline_handler: Return unique_ptr from generateConfiguration() 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 b6139a88..96aab9d6 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 f0575c13..c4f65c1a 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 3b892d96..e4d79ea4 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 1b599fcc..f15fa28b 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 455ee2a0..56e4a62a 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 37b3e7ac..a32de7f3 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 a2819545..277465b7 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 f85d05f7..204f5ad7 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 588a3db3..825aff5a 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. */ /** -- cgit v1.2.1