| Message ID | 20221024000356.29521-9-laurent.pinchart@ideasonboard.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
On Mon, Oct 24, 2022 at 03:03:51AM +0300, Laurent Pinchart wrote: > 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 <laurent.pinchart@ideasonboard.com> Reviewed-by: Paul Elder <paul.elder@ideasonboard.com> > --- > 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<CameraConfiguration> 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<Stream *> &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<CameraConfiguration> Camera::generateConfiguration(const StreamRoles &roles) > { > @@ -949,7 +948,8 @@ std::unique_ptr<CameraConfiguration> Camera::generateConfiguration(const StreamR > if (roles.size() > streams().size()) > return nullptr; > > - CameraConfiguration *config = d->pipe_->generateConfiguration(this, roles); > + std::unique_ptr<CameraConfiguration> config = > + d->pipe_->generateConfiguration(this, roles); > if (!config) { > LOG(Camera, Debug) > << "Pipeline handler failed to generate configuration"; > @@ -966,7 +966,7 @@ std::unique_ptr<CameraConfiguration> Camera::generateConfiguration(const StreamR > > LOG(Camera, Debug) << msg.str(); > > - return std::unique_ptr<CameraConfiguration>(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<CameraConfiguration> 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<CameraConfiguration> > +PipelineHandlerIPU3::generateConfiguration(Camera *camera, const StreamRoles &roles) > { > IPU3CameraData *data = cameraData(camera); > - IPU3CameraConfiguration *config = new IPU3CameraConfiguration(data); > + std::unique_ptr<IPU3CameraConfiguration> config = > + std::make_unique<IPU3CameraConfiguration>(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<CameraConfiguration> 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<CameraConfiguration> > +PipelineHandlerRPi::generateConfiguration(Camera *camera, const StreamRoles &roles) > { > RPiCameraData *data = cameraData(camera); > - CameraConfiguration *config = new RPiCameraConfiguration(data); > + std::unique_ptr<CameraConfiguration> config = > + std::make_unique<RPiCameraConfiguration>(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<CameraConfiguration> 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<CameraConfiguration> > +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<CameraConfiguration> config = > + std::make_unique<RkISP1CameraConfiguration>(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<CameraConfiguration> 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<CameraConfiguration> > +SimplePipelineHandler::generateConfiguration(Camera *camera, const StreamRoles &roles) > { > SimpleCameraData *data = cameraData(camera); > - CameraConfiguration *config = > - new SimpleCameraConfiguration(camera, data); > + std::unique_ptr<CameraConfiguration> config = > + std::make_unique<SimpleCameraConfiguration>(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<CameraConfiguration> 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<CameraConfiguration> > +PipelineHandlerUVC::generateConfiguration(Camera *camera, > const StreamRoles &roles) > { > UVCCameraData *data = cameraData(camera); > - CameraConfiguration *config = new UVCCameraConfiguration(data); > + std::unique_ptr<CameraConfiguration> config = > + std::make_unique<UVCCameraConfiguration>(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<CameraConfiguration> 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<CameraConfiguration> > +PipelineHandlerVimc::generateConfiguration(Camera *camera, > const StreamRoles &roles) > { > VimcCameraData *data = cameraData(camera); > - CameraConfiguration *config = new VimcCameraConfiguration(data); > + std::unique_ptr<CameraConfiguration> config = > + std::make_unique<VimcCameraConfiguration>(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. > */ > > /** > -- > Regards, > > Laurent Pinchart >
Hi Laurent, On Mon, Oct 24, 2022 at 03:03:51AM +0300, Laurent Pinchart via libcamera-devel wrote: > 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. Great! Reviewed-by: Jacopo Mondi <jacopo@jmondi.org> Thanks j > > 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 <laurent.pinchart@ideasonboard.com> > --- > 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<CameraConfiguration> 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<Stream *> &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<CameraConfiguration> Camera::generateConfiguration(const StreamRoles &roles) > { > @@ -949,7 +948,8 @@ std::unique_ptr<CameraConfiguration> Camera::generateConfiguration(const StreamR > if (roles.size() > streams().size()) > return nullptr; > > - CameraConfiguration *config = d->pipe_->generateConfiguration(this, roles); > + std::unique_ptr<CameraConfiguration> config = > + d->pipe_->generateConfiguration(this, roles); > if (!config) { > LOG(Camera, Debug) > << "Pipeline handler failed to generate configuration"; > @@ -966,7 +966,7 @@ std::unique_ptr<CameraConfiguration> Camera::generateConfiguration(const StreamR > > LOG(Camera, Debug) << msg.str(); > > - return std::unique_ptr<CameraConfiguration>(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<CameraConfiguration> 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<CameraConfiguration> > +PipelineHandlerIPU3::generateConfiguration(Camera *camera, const StreamRoles &roles) > { > IPU3CameraData *data = cameraData(camera); > - IPU3CameraConfiguration *config = new IPU3CameraConfiguration(data); > + std::unique_ptr<IPU3CameraConfiguration> config = > + std::make_unique<IPU3CameraConfiguration>(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<CameraConfiguration> 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<CameraConfiguration> > +PipelineHandlerRPi::generateConfiguration(Camera *camera, const StreamRoles &roles) > { > RPiCameraData *data = cameraData(camera); > - CameraConfiguration *config = new RPiCameraConfiguration(data); > + std::unique_ptr<CameraConfiguration> config = > + std::make_unique<RPiCameraConfiguration>(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<CameraConfiguration> 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<CameraConfiguration> > +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<CameraConfiguration> config = > + std::make_unique<RkISP1CameraConfiguration>(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<CameraConfiguration> 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<CameraConfiguration> > +SimplePipelineHandler::generateConfiguration(Camera *camera, const StreamRoles &roles) > { > SimpleCameraData *data = cameraData(camera); > - CameraConfiguration *config = > - new SimpleCameraConfiguration(camera, data); > + std::unique_ptr<CameraConfiguration> config = > + std::make_unique<SimpleCameraConfiguration>(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<CameraConfiguration> 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<CameraConfiguration> > +PipelineHandlerUVC::generateConfiguration(Camera *camera, > const StreamRoles &roles) > { > UVCCameraData *data = cameraData(camera); > - CameraConfiguration *config = new UVCCameraConfiguration(data); > + std::unique_ptr<CameraConfiguration> config = > + std::make_unique<UVCCameraConfiguration>(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<CameraConfiguration> 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<CameraConfiguration> > +PipelineHandlerVimc::generateConfiguration(Camera *camera, > const StreamRoles &roles) > { > VimcCameraData *data = cameraData(camera); > - CameraConfiguration *config = new VimcCameraConfiguration(data); > + std::unique_ptr<CameraConfiguration> config = > + std::make_unique<VimcCameraConfiguration>(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. > */ > > /** > -- > Regards, > > Laurent Pinchart >
Quoting Laurent Pinchart via libcamera-devel (2022-10-24 01:03:51) > 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 <laurent.pinchart@ideasonboard.com> > --- > 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<CameraConfiguration> 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<Stream *> &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<CameraConfiguration> Camera::generateConfiguration(const StreamRoles &roles) > { > @@ -949,7 +948,8 @@ std::unique_ptr<CameraConfiguration> Camera::generateConfiguration(const StreamR > if (roles.size() > streams().size()) > return nullptr; > > - CameraConfiguration *config = d->pipe_->generateConfiguration(this, roles); > + std::unique_ptr<CameraConfiguration> config = > + d->pipe_->generateConfiguration(this, roles); > if (!config) { > LOG(Camera, Debug) > << "Pipeline handler failed to generate configuration"; > @@ -966,7 +966,7 @@ std::unique_ptr<CameraConfiguration> Camera::generateConfiguration(const StreamR > > LOG(Camera, Debug) << msg.str(); > > - return std::unique_ptr<CameraConfiguration>(config); > + return config; Ok - so this really makes sense.! Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > } > > /** > 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<CameraConfiguration> 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<CameraConfiguration> > +PipelineHandlerIPU3::generateConfiguration(Camera *camera, const StreamRoles &roles) > { > IPU3CameraData *data = cameraData(camera); > - IPU3CameraConfiguration *config = new IPU3CameraConfiguration(data); > + std::unique_ptr<IPU3CameraConfiguration> config = > + std::make_unique<IPU3CameraConfiguration>(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<CameraConfiguration> 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<CameraConfiguration> > +PipelineHandlerRPi::generateConfiguration(Camera *camera, const StreamRoles &roles) > { > RPiCameraData *data = cameraData(camera); > - CameraConfiguration *config = new RPiCameraConfiguration(data); > + std::unique_ptr<CameraConfiguration> config = > + std::make_unique<RPiCameraConfiguration>(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<CameraConfiguration> 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<CameraConfiguration> > +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<CameraConfiguration> config = > + std::make_unique<RkISP1CameraConfiguration>(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<CameraConfiguration> 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<CameraConfiguration> > +SimplePipelineHandler::generateConfiguration(Camera *camera, const StreamRoles &roles) > { > SimpleCameraData *data = cameraData(camera); > - CameraConfiguration *config = > - new SimpleCameraConfiguration(camera, data); > + std::unique_ptr<CameraConfiguration> config = > + std::make_unique<SimpleCameraConfiguration>(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<CameraConfiguration> 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<CameraConfiguration> > +PipelineHandlerUVC::generateConfiguration(Camera *camera, > const StreamRoles &roles) > { > UVCCameraData *data = cameraData(camera); > - CameraConfiguration *config = new UVCCameraConfiguration(data); > + std::unique_ptr<CameraConfiguration> config = > + std::make_unique<UVCCameraConfiguration>(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<CameraConfiguration> 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<CameraConfiguration> > +PipelineHandlerVimc::generateConfiguration(Camera *camera, > const StreamRoles &roles) > { > VimcCameraData *data = cameraData(camera); > - CameraConfiguration *config = new VimcCameraConfiguration(data); > + std::unique_ptr<CameraConfiguration> config = > + std::make_unique<VimcCameraConfiguration>(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. > */ > > /** > -- > Regards, > > Laurent Pinchart >
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<CameraConfiguration> 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<Stream *> &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<CameraConfiguration> Camera::generateConfiguration(const StreamRoles &roles) { @@ -949,7 +948,8 @@ std::unique_ptr<CameraConfiguration> Camera::generateConfiguration(const StreamR if (roles.size() > streams().size()) return nullptr; - CameraConfiguration *config = d->pipe_->generateConfiguration(this, roles); + std::unique_ptr<CameraConfiguration> config = + d->pipe_->generateConfiguration(this, roles); if (!config) { LOG(Camera, Debug) << "Pipeline handler failed to generate configuration"; @@ -966,7 +966,7 @@ std::unique_ptr<CameraConfiguration> Camera::generateConfiguration(const StreamR LOG(Camera, Debug) << msg.str(); - return std::unique_ptr<CameraConfiguration>(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<CameraConfiguration> 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<CameraConfiguration> +PipelineHandlerIPU3::generateConfiguration(Camera *camera, const StreamRoles &roles) { IPU3CameraData *data = cameraData(camera); - IPU3CameraConfiguration *config = new IPU3CameraConfiguration(data); + std::unique_ptr<IPU3CameraConfiguration> config = + std::make_unique<IPU3CameraConfiguration>(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<CameraConfiguration> 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<CameraConfiguration> +PipelineHandlerRPi::generateConfiguration(Camera *camera, const StreamRoles &roles) { RPiCameraData *data = cameraData(camera); - CameraConfiguration *config = new RPiCameraConfiguration(data); + std::unique_ptr<CameraConfiguration> config = + std::make_unique<RPiCameraConfiguration>(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<CameraConfiguration> 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<CameraConfiguration> +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<CameraConfiguration> config = + std::make_unique<RkISP1CameraConfiguration>(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<CameraConfiguration> 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<CameraConfiguration> +SimplePipelineHandler::generateConfiguration(Camera *camera, const StreamRoles &roles) { SimpleCameraData *data = cameraData(camera); - CameraConfiguration *config = - new SimpleCameraConfiguration(camera, data); + std::unique_ptr<CameraConfiguration> config = + std::make_unique<SimpleCameraConfiguration>(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<CameraConfiguration> 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<CameraConfiguration> +PipelineHandlerUVC::generateConfiguration(Camera *camera, const StreamRoles &roles) { UVCCameraData *data = cameraData(camera); - CameraConfiguration *config = new UVCCameraConfiguration(data); + std::unique_ptr<CameraConfiguration> config = + std::make_unique<UVCCameraConfiguration>(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<CameraConfiguration> 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<CameraConfiguration> +PipelineHandlerVimc::generateConfiguration(Camera *camera, const StreamRoles &roles) { VimcCameraData *data = cameraData(camera); - CameraConfiguration *config = new VimcCameraConfiguration(data); + std::unique_ptr<CameraConfiguration> config = + std::make_unique<VimcCameraConfiguration>(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. */ /**
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 <laurent.pinchart@ideasonboard.com> --- 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(-)