[libcamera-devel,3/3] libcamera: pipeline: Manage resources with std::unique_ptr<>
diff mbox series

Message ID 20201212185116.29611-4-laurent.pinchart@ideasonboard.com
State Accepted
Delegated to: Laurent Pinchart
Headers show
Series
  • libcamera: Fix leaks and simplify object management with std::unique_ptr<>
Related show

Commit Message

Laurent Pinchart Dec. 12, 2020, 6:51 p.m. UTC
Replace manual resource destruction with std::unique_ptr<> where
applicable. This removes the need for several destructors.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 src/libcamera/pipeline/ipu3/cio2.cpp         | 11 ++---------
 src/libcamera/pipeline/ipu3/cio2.h           |  9 ++++-----
 src/libcamera/pipeline/rkisp1/rkisp1.cpp     | 19 +++++++------------
 src/libcamera/pipeline/simple/converter.cpp  |  8 +-------
 src/libcamera/pipeline/simple/converter.h    |  3 +--
 src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 12 ++++--------
 src/libcamera/pipeline/vimc/vimc.cpp         | 11 +++--------
 7 files changed, 22 insertions(+), 51 deletions(-)

Comments

Paul Elder Dec. 14, 2020, 2:20 a.m. UTC | #1
Hi Laurent,

On Sat, Dec 12, 2020 at 08:51:16PM +0200, Laurent Pinchart wrote:
> Replace manual resource destruction with std::unique_ptr<> where
> applicable. This removes the need for several destructors.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  src/libcamera/pipeline/ipu3/cio2.cpp         | 11 ++---------
>  src/libcamera/pipeline/ipu3/cio2.h           |  9 ++++-----
>  src/libcamera/pipeline/rkisp1/rkisp1.cpp     | 19 +++++++------------
>  src/libcamera/pipeline/simple/converter.cpp  |  8 +-------
>  src/libcamera/pipeline/simple/converter.h    |  3 +--
>  src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 12 ++++--------
>  src/libcamera/pipeline/vimc/vimc.cpp         | 11 +++--------
>  7 files changed, 22 insertions(+), 51 deletions(-)
> 
> diff --git a/src/libcamera/pipeline/ipu3/cio2.cpp b/src/libcamera/pipeline/ipu3/cio2.cpp
> index 821715e325b4..bd5260d5b288 100644
> --- a/src/libcamera/pipeline/ipu3/cio2.cpp
> +++ b/src/libcamera/pipeline/ipu3/cio2.cpp
> @@ -33,16 +33,9 @@ const std::map<uint32_t, PixelFormat> mbusCodesToPixelFormat = {
>  } /* namespace */
>  
>  CIO2Device::CIO2Device()
> -	: sensor_(nullptr), csi2_(nullptr)
>  {
>  }
>  
> -CIO2Device::~CIO2Device()
> -{
> -	delete csi2_;
> -	delete sensor_;
> -}
> -
>  /**
>   * \brief Retrieve the list of supported PixelFormats
>   *
> @@ -117,7 +110,7 @@ int CIO2Device::init(const MediaDevice *media, unsigned int index)
>  
>  	MediaLink *link = links[0];
>  	MediaEntity *sensorEntity = link->source()->entity();
> -	sensor_ = new CameraSensor(sensorEntity);
> +	sensor_ = std::make_unique<CameraSensor>(sensorEntity);
>  	ret = sensor_->init();
>  	if (ret)
>  		return ret;
> @@ -148,7 +141,7 @@ int CIO2Device::init(const MediaDevice *media, unsigned int index)
>  	 * might impact on power consumption.
>  	 */
>  
> -	csi2_ = new V4L2Subdevice(csi2Entity);
> +	csi2_ = std::make_unique<V4L2Subdevice>(csi2Entity);
>  	ret = csi2_->open();
>  	if (ret)
>  		return ret;
> diff --git a/src/libcamera/pipeline/ipu3/cio2.h b/src/libcamera/pipeline/ipu3/cio2.h
> index 0dca9673ca07..236ad287354a 100644
> --- a/src/libcamera/pipeline/ipu3/cio2.h
> +++ b/src/libcamera/pipeline/ipu3/cio2.h
> @@ -33,7 +33,6 @@ public:
>  	static constexpr unsigned int CIO2_BUFFER_COUNT = 4;
>  
>  	CIO2Device();
> -	~CIO2Device();
>  
>  	std::vector<PixelFormat> formats() const;
>  	std::vector<SizeRange> sizes() const;
> @@ -49,8 +48,8 @@ public:
>  	int start();
>  	int stop();
>  
> -	CameraSensor *sensor() { return sensor_; }
> -	const CameraSensor *sensor() const { return sensor_; }
> +	CameraSensor *sensor() { return sensor_.get(); }
> +	const CameraSensor *sensor() const { return sensor_.get(); }
>  
>  	int queueBuffer(Request *request, FrameBuffer *rawBuffer);
>  	void tryReturnBuffer(FrameBuffer *buffer);
> @@ -61,8 +60,8 @@ private:
>  
>  	void cio2BufferReady(FrameBuffer *buffer);
>  
> -	CameraSensor *sensor_;
> -	V4L2Subdevice *csi2_;
> +	std::unique_ptr<CameraSensor> sensor_;
> +	std::unique_ptr<V4L2Subdevice> csi2_;
>  	std::unique_ptr<V4L2VideoDevice> output_;
>  
>  	std::vector<std::unique_ptr<FrameBuffer>> buffers_;
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> index 4d98c9027f42..021d0ffe3ffb 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> @@ -121,21 +121,16 @@ class RkISP1CameraData : public CameraData
>  public:
>  	RkISP1CameraData(PipelineHandler *pipe, RkISP1MainPath *mainPath,
>  			 RkISP1SelfPath *selfPath)
> -		: CameraData(pipe), sensor_(nullptr), frame_(0),
> -		  frameInfo_(pipe), mainPath_(mainPath), selfPath_(selfPath)
> +		: CameraData(pipe), frame_(0), frameInfo_(pipe),
> +		  mainPath_(mainPath), selfPath_(selfPath)
>  	{
>  	}
>  
> -	~RkISP1CameraData()
> -	{
> -		delete sensor_;
> -	}
> -
>  	int loadIPA();
>  
>  	Stream mainPathStream_;
>  	Stream selfPathStream_;
> -	CameraSensor *sensor_;
> +	std::unique_ptr<CameraSensor> sensor_;
>  	unsigned int frame_;
>  	std::vector<IPABuffer> ipaBuffers_;
>  	RkISP1Frames frameInfo_;
> @@ -430,7 +425,7 @@ void RkISP1CameraData::queueFrameAction(unsigned int frame,
>  	case RKISP1_IPA_ACTION_V4L2_SET: {
>  		const ControlList &controls = action.controls[0];
>  		timeline_.scheduleAction(std::make_unique<RkISP1ActionSetSensor>(frame,
> -										 sensor_,
> +										 sensor_.get(),
>  										 controls));
>  		break;
>  	}
> @@ -489,7 +484,7 @@ bool RkISP1CameraConfiguration::fitsAllPaths(const StreamConfiguration &cfg)
>  
>  CameraConfiguration::Status RkISP1CameraConfiguration::validate()
>  {
> -	const CameraSensor *sensor = data_->sensor_;
> +	const CameraSensor *sensor = data_->sensor_.get();
>  	Status status = Valid;

What's wrong with using the unique_ptr here directly?

Just wondering though, so

Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>

>  	if (config_.empty())
> @@ -660,7 +655,7 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)
>  	RkISP1CameraConfiguration *config =
>  		static_cast<RkISP1CameraConfiguration *>(c);
>  	RkISP1CameraData *data = cameraData(camera);
> -	CameraSensor *sensor = data->sensor_;
> +	CameraSensor *sensor = data->sensor_.get();
>  	int ret;
>  
>  	ret = initLinks(camera, sensor, *config);
> @@ -1035,7 +1030,7 @@ int PipelineHandlerRkISP1::createCamera(MediaEntity *sensor)
>  
>  	data->controlInfo_ = std::move(ctrls);
>  
> -	data->sensor_ = new CameraSensor(sensor);
> +	data->sensor_ = std::make_unique<CameraSensor>(sensor);
>  	ret = data->sensor_->init();
>  	if (ret)
>  		return ret;
> diff --git a/src/libcamera/pipeline/simple/converter.cpp b/src/libcamera/pipeline/simple/converter.cpp
> index 67e6e864aa0a..a6a8e139cb3e 100644
> --- a/src/libcamera/pipeline/simple/converter.cpp
> +++ b/src/libcamera/pipeline/simple/converter.cpp
> @@ -24,7 +24,6 @@ namespace libcamera {
>  LOG_DECLARE_CATEGORY(SimplePipeline)
>  
>  SimpleConverter::SimpleConverter(MediaDevice *media)
> -	: m2m_(nullptr)
>  {
>  	/*
>  	 * Locate the video node. There's no need to validate the pipeline
> @@ -38,17 +37,12 @@ SimpleConverter::SimpleConverter(MediaDevice *media)
>  	if (it == entities.end())
>  		return;
>  
> -	m2m_ = new V4L2M2MDevice((*it)->deviceNode());
> +	m2m_ = std::make_unique<V4L2M2MDevice>((*it)->deviceNode());
>  
>  	m2m_->output()->bufferReady.connect(this, &SimpleConverter::outputBufferReady);
>  	m2m_->capture()->bufferReady.connect(this, &SimpleConverter::captureBufferReady);
>  }
>  
> -SimpleConverter::~SimpleConverter()
> -{
> -	delete m2m_;
> -}
> -
>  int SimpleConverter::open()
>  {
>  	if (!m2m_)
> diff --git a/src/libcamera/pipeline/simple/converter.h b/src/libcamera/pipeline/simple/converter.h
> index 78296680aa14..a3c4d899cfc8 100644
> --- a/src/libcamera/pipeline/simple/converter.h
> +++ b/src/libcamera/pipeline/simple/converter.h
> @@ -29,7 +29,6 @@ class SimpleConverter
>  {
>  public:
>  	SimpleConverter(MediaDevice *media);
> -	~SimpleConverter();
>  
>  	int open();
>  	void close();
> @@ -56,7 +55,7 @@ private:
>  	void captureBufferReady(FrameBuffer *buffer);
>  	void outputBufferReady(FrameBuffer *buffer);
>  
> -	V4L2M2MDevice *m2m_;
> +	std::unique_ptr<V4L2M2MDevice> m2m_;
>  
>  	std::queue<FrameBuffer *> captureDoneQueue_;
>  	std::queue<FrameBuffer *> outputDoneQueue_;
> diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> index 87b0f03d143a..e54f45bb8825 100644
> --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> @@ -9,6 +9,7 @@
>  #include <fstream>
>  #include <iomanip>
>  #include <math.h>
> +#include <memory>
>  #include <tuple>
>  
>  #include <libcamera/camera.h>
> @@ -35,21 +36,16 @@ class UVCCameraData : public CameraData
>  {
>  public:
>  	UVCCameraData(PipelineHandler *pipe)
> -		: CameraData(pipe), video_(nullptr)
> +		: CameraData(pipe)
>  	{
>  	}
>  
> -	~UVCCameraData()
> -	{
> -		delete video_;
> -	}
> -
>  	int init(MediaDevice *media);
>  	void addControl(uint32_t cid, const ControlInfo &v4l2info,
>  			ControlInfoMap::Map *ctrls);
>  	void bufferReady(FrameBuffer *buffer);
>  
> -	V4L2VideoDevice *video_;
> +	std::unique_ptr<V4L2VideoDevice> video_;
>  	Stream stream_;
>  };
>  
> @@ -499,7 +495,7 @@ int UVCCameraData::init(MediaDevice *media)
>  	}
>  
>  	/* Create and open the video device. */
> -	video_ = new V4L2VideoDevice(*entity);
> +	video_ = std::make_unique<V4L2VideoDevice>(*entity);
>  	ret = video_->open();
>  	if (ret)
>  		return ret;
> diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp
> index 72256f5b4190..8bda746f3136 100644
> --- a/src/libcamera/pipeline/vimc/vimc.cpp
> +++ b/src/libcamera/pipeline/vimc/vimc.cpp
> @@ -42,20 +42,15 @@ class VimcCameraData : public CameraData
>  {
>  public:
>  	VimcCameraData(PipelineHandler *pipe, MediaDevice *media)
> -		: CameraData(pipe), media_(media), sensor_(nullptr)
> +		: CameraData(pipe), media_(media)
>  	{
>  	}
>  
> -	~VimcCameraData()
> -	{
> -		delete sensor_;
> -	}
> -
>  	int init();
>  	void bufferReady(FrameBuffer *buffer);
>  
>  	MediaDevice *media_;
> -	CameraSensor *sensor_;
> +	std::unique_ptr<CameraSensor> sensor_;
>  	std::unique_ptr<V4L2Subdevice> debayer_;
>  	std::unique_ptr<V4L2Subdevice> scaler_;
>  	std::unique_ptr<V4L2VideoDevice> video_;
> @@ -461,7 +456,7 @@ int VimcCameraData::init()
>  		return ret;
>  
>  	/* Create and open the camera sensor, debayer, scaler and video device. */
> -	sensor_ = new CameraSensor(media_->getEntityByName("Sensor B"));
> +	sensor_ = std::make_unique<CameraSensor>(media_->getEntityByName("Sensor B"));
>  	ret = sensor_->init();
>  	if (ret)
>  		return ret;
> -- 
> Regards,
> 
> Laurent Pinchart
> 
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
Laurent Pinchart Dec. 14, 2020, 1:47 p.m. UTC | #2
Hi Paul,

On Mon, Dec 14, 2020 at 11:20:51AM +0900, paul.elder@ideasonboard.com wrote:
> On Sat, Dec 12, 2020 at 08:51:16PM +0200, Laurent Pinchart wrote:
> > Replace manual resource destruction with std::unique_ptr<> where
> > applicable. This removes the need for several destructors.
> > 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> >  src/libcamera/pipeline/ipu3/cio2.cpp         | 11 ++---------
> >  src/libcamera/pipeline/ipu3/cio2.h           |  9 ++++-----
> >  src/libcamera/pipeline/rkisp1/rkisp1.cpp     | 19 +++++++------------
> >  src/libcamera/pipeline/simple/converter.cpp  |  8 +-------
> >  src/libcamera/pipeline/simple/converter.h    |  3 +--
> >  src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 12 ++++--------
> >  src/libcamera/pipeline/vimc/vimc.cpp         | 11 +++--------
> >  7 files changed, 22 insertions(+), 51 deletions(-)
> > 
> > diff --git a/src/libcamera/pipeline/ipu3/cio2.cpp b/src/libcamera/pipeline/ipu3/cio2.cpp
> > index 821715e325b4..bd5260d5b288 100644
> > --- a/src/libcamera/pipeline/ipu3/cio2.cpp
> > +++ b/src/libcamera/pipeline/ipu3/cio2.cpp
> > @@ -33,16 +33,9 @@ const std::map<uint32_t, PixelFormat> mbusCodesToPixelFormat = {
> >  } /* namespace */
> >  
> >  CIO2Device::CIO2Device()
> > -	: sensor_(nullptr), csi2_(nullptr)
> >  {
> >  }
> >  
> > -CIO2Device::~CIO2Device()
> > -{
> > -	delete csi2_;
> > -	delete sensor_;
> > -}
> > -
> >  /**
> >   * \brief Retrieve the list of supported PixelFormats
> >   *
> > @@ -117,7 +110,7 @@ int CIO2Device::init(const MediaDevice *media, unsigned int index)
> >  
> >  	MediaLink *link = links[0];
> >  	MediaEntity *sensorEntity = link->source()->entity();
> > -	sensor_ = new CameraSensor(sensorEntity);
> > +	sensor_ = std::make_unique<CameraSensor>(sensorEntity);
> >  	ret = sensor_->init();
> >  	if (ret)
> >  		return ret;
> > @@ -148,7 +141,7 @@ int CIO2Device::init(const MediaDevice *media, unsigned int index)
> >  	 * might impact on power consumption.
> >  	 */
> >  
> > -	csi2_ = new V4L2Subdevice(csi2Entity);
> > +	csi2_ = std::make_unique<V4L2Subdevice>(csi2Entity);
> >  	ret = csi2_->open();
> >  	if (ret)
> >  		return ret;
> > diff --git a/src/libcamera/pipeline/ipu3/cio2.h b/src/libcamera/pipeline/ipu3/cio2.h
> > index 0dca9673ca07..236ad287354a 100644
> > --- a/src/libcamera/pipeline/ipu3/cio2.h
> > +++ b/src/libcamera/pipeline/ipu3/cio2.h
> > @@ -33,7 +33,6 @@ public:
> >  	static constexpr unsigned int CIO2_BUFFER_COUNT = 4;
> >  
> >  	CIO2Device();
> > -	~CIO2Device();
> >  
> >  	std::vector<PixelFormat> formats() const;
> >  	std::vector<SizeRange> sizes() const;
> > @@ -49,8 +48,8 @@ public:
> >  	int start();
> >  	int stop();
> >  
> > -	CameraSensor *sensor() { return sensor_; }
> > -	const CameraSensor *sensor() const { return sensor_; }
> > +	CameraSensor *sensor() { return sensor_.get(); }
> > +	const CameraSensor *sensor() const { return sensor_.get(); }
> >  
> >  	int queueBuffer(Request *request, FrameBuffer *rawBuffer);
> >  	void tryReturnBuffer(FrameBuffer *buffer);
> > @@ -61,8 +60,8 @@ private:
> >  
> >  	void cio2BufferReady(FrameBuffer *buffer);
> >  
> > -	CameraSensor *sensor_;
> > -	V4L2Subdevice *csi2_;
> > +	std::unique_ptr<CameraSensor> sensor_;
> > +	std::unique_ptr<V4L2Subdevice> csi2_;
> >  	std::unique_ptr<V4L2VideoDevice> output_;
> >  
> >  	std::vector<std::unique_ptr<FrameBuffer>> buffers_;
> > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> > index 4d98c9027f42..021d0ffe3ffb 100644
> > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> > @@ -121,21 +121,16 @@ class RkISP1CameraData : public CameraData
> >  public:
> >  	RkISP1CameraData(PipelineHandler *pipe, RkISP1MainPath *mainPath,
> >  			 RkISP1SelfPath *selfPath)
> > -		: CameraData(pipe), sensor_(nullptr), frame_(0),
> > -		  frameInfo_(pipe), mainPath_(mainPath), selfPath_(selfPath)
> > +		: CameraData(pipe), frame_(0), frameInfo_(pipe),
> > +		  mainPath_(mainPath), selfPath_(selfPath)
> >  	{
> >  	}
> >  
> > -	~RkISP1CameraData()
> > -	{
> > -		delete sensor_;
> > -	}
> > -
> >  	int loadIPA();
> >  
> >  	Stream mainPathStream_;
> >  	Stream selfPathStream_;
> > -	CameraSensor *sensor_;
> > +	std::unique_ptr<CameraSensor> sensor_;
> >  	unsigned int frame_;
> >  	std::vector<IPABuffer> ipaBuffers_;
> >  	RkISP1Frames frameInfo_;
> > @@ -430,7 +425,7 @@ void RkISP1CameraData::queueFrameAction(unsigned int frame,
> >  	case RKISP1_IPA_ACTION_V4L2_SET: {
> >  		const ControlList &controls = action.controls[0];
> >  		timeline_.scheduleAction(std::make_unique<RkISP1ActionSetSensor>(frame,
> > -										 sensor_,
> > +										 sensor_.get(),
> >  										 controls));
> >  		break;
> >  	}
> > @@ -489,7 +484,7 @@ bool RkISP1CameraConfiguration::fitsAllPaths(const StreamConfiguration &cfg)
> >  
> >  CameraConfiguration::Status RkISP1CameraConfiguration::validate()
> >  {
> > -	const CameraSensor *sensor = data_->sensor_;
> > +	const CameraSensor *sensor = data_->sensor_.get();
> >  	Status status = Valid;
> 
> What's wrong with using the unique_ptr here directly?

Nothing as such. We could s/sensor/data_->sensor_/ in the function. The
local sensor variable is just a shorthand. We could also write

	const std::unique_ptr<CameraSensor> &sensor = data_->sensor_;

but that seems a bit more complicated. Would you prefer that ?

> Just wondering though, so
> 
> Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
> 
> >  	if (config_.empty())
> > @@ -660,7 +655,7 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)
> >  	RkISP1CameraConfiguration *config =
> >  		static_cast<RkISP1CameraConfiguration *>(c);
> >  	RkISP1CameraData *data = cameraData(camera);
> > -	CameraSensor *sensor = data->sensor_;
> > +	CameraSensor *sensor = data->sensor_.get();
> >  	int ret;
> >  
> >  	ret = initLinks(camera, sensor, *config);
> > @@ -1035,7 +1030,7 @@ int PipelineHandlerRkISP1::createCamera(MediaEntity *sensor)
> >  
> >  	data->controlInfo_ = std::move(ctrls);
> >  
> > -	data->sensor_ = new CameraSensor(sensor);
> > +	data->sensor_ = std::make_unique<CameraSensor>(sensor);
> >  	ret = data->sensor_->init();
> >  	if (ret)
> >  		return ret;
> > diff --git a/src/libcamera/pipeline/simple/converter.cpp b/src/libcamera/pipeline/simple/converter.cpp
> > index 67e6e864aa0a..a6a8e139cb3e 100644
> > --- a/src/libcamera/pipeline/simple/converter.cpp
> > +++ b/src/libcamera/pipeline/simple/converter.cpp
> > @@ -24,7 +24,6 @@ namespace libcamera {
> >  LOG_DECLARE_CATEGORY(SimplePipeline)
> >  
> >  SimpleConverter::SimpleConverter(MediaDevice *media)
> > -	: m2m_(nullptr)
> >  {
> >  	/*
> >  	 * Locate the video node. There's no need to validate the pipeline
> > @@ -38,17 +37,12 @@ SimpleConverter::SimpleConverter(MediaDevice *media)
> >  	if (it == entities.end())
> >  		return;
> >  
> > -	m2m_ = new V4L2M2MDevice((*it)->deviceNode());
> > +	m2m_ = std::make_unique<V4L2M2MDevice>((*it)->deviceNode());
> >  
> >  	m2m_->output()->bufferReady.connect(this, &SimpleConverter::outputBufferReady);
> >  	m2m_->capture()->bufferReady.connect(this, &SimpleConverter::captureBufferReady);
> >  }
> >  
> > -SimpleConverter::~SimpleConverter()
> > -{
> > -	delete m2m_;
> > -}
> > -
> >  int SimpleConverter::open()
> >  {
> >  	if (!m2m_)
> > diff --git a/src/libcamera/pipeline/simple/converter.h b/src/libcamera/pipeline/simple/converter.h
> > index 78296680aa14..a3c4d899cfc8 100644
> > --- a/src/libcamera/pipeline/simple/converter.h
> > +++ b/src/libcamera/pipeline/simple/converter.h
> > @@ -29,7 +29,6 @@ class SimpleConverter
> >  {
> >  public:
> >  	SimpleConverter(MediaDevice *media);
> > -	~SimpleConverter();
> >  
> >  	int open();
> >  	void close();
> > @@ -56,7 +55,7 @@ private:
> >  	void captureBufferReady(FrameBuffer *buffer);
> >  	void outputBufferReady(FrameBuffer *buffer);
> >  
> > -	V4L2M2MDevice *m2m_;
> > +	std::unique_ptr<V4L2M2MDevice> m2m_;
> >  
> >  	std::queue<FrameBuffer *> captureDoneQueue_;
> >  	std::queue<FrameBuffer *> outputDoneQueue_;
> > diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> > index 87b0f03d143a..e54f45bb8825 100644
> > --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> > +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> > @@ -9,6 +9,7 @@
> >  #include <fstream>
> >  #include <iomanip>
> >  #include <math.h>
> > +#include <memory>
> >  #include <tuple>
> >  
> >  #include <libcamera/camera.h>
> > @@ -35,21 +36,16 @@ class UVCCameraData : public CameraData
> >  {
> >  public:
> >  	UVCCameraData(PipelineHandler *pipe)
> > -		: CameraData(pipe), video_(nullptr)
> > +		: CameraData(pipe)
> >  	{
> >  	}
> >  
> > -	~UVCCameraData()
> > -	{
> > -		delete video_;
> > -	}
> > -
> >  	int init(MediaDevice *media);
> >  	void addControl(uint32_t cid, const ControlInfo &v4l2info,
> >  			ControlInfoMap::Map *ctrls);
> >  	void bufferReady(FrameBuffer *buffer);
> >  
> > -	V4L2VideoDevice *video_;
> > +	std::unique_ptr<V4L2VideoDevice> video_;
> >  	Stream stream_;
> >  };
> >  
> > @@ -499,7 +495,7 @@ int UVCCameraData::init(MediaDevice *media)
> >  	}
> >  
> >  	/* Create and open the video device. */
> > -	video_ = new V4L2VideoDevice(*entity);
> > +	video_ = std::make_unique<V4L2VideoDevice>(*entity);
> >  	ret = video_->open();
> >  	if (ret)
> >  		return ret;
> > diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp
> > index 72256f5b4190..8bda746f3136 100644
> > --- a/src/libcamera/pipeline/vimc/vimc.cpp
> > +++ b/src/libcamera/pipeline/vimc/vimc.cpp
> > @@ -42,20 +42,15 @@ class VimcCameraData : public CameraData
> >  {
> >  public:
> >  	VimcCameraData(PipelineHandler *pipe, MediaDevice *media)
> > -		: CameraData(pipe), media_(media), sensor_(nullptr)
> > +		: CameraData(pipe), media_(media)
> >  	{
> >  	}
> >  
> > -	~VimcCameraData()
> > -	{
> > -		delete sensor_;
> > -	}
> > -
> >  	int init();
> >  	void bufferReady(FrameBuffer *buffer);
> >  
> >  	MediaDevice *media_;
> > -	CameraSensor *sensor_;
> > +	std::unique_ptr<CameraSensor> sensor_;
> >  	std::unique_ptr<V4L2Subdevice> debayer_;
> >  	std::unique_ptr<V4L2Subdevice> scaler_;
> >  	std::unique_ptr<V4L2VideoDevice> video_;
> > @@ -461,7 +456,7 @@ int VimcCameraData::init()
> >  		return ret;
> >  
> >  	/* Create and open the camera sensor, debayer, scaler and video device. */
> > -	sensor_ = new CameraSensor(media_->getEntityByName("Sensor B"));
> > +	sensor_ = std::make_unique<CameraSensor>(media_->getEntityByName("Sensor B"));
> >  	ret = sensor_->init();
> >  	if (ret)
> >  		return ret;
Paul Elder Dec. 15, 2020, 4:10 a.m. UTC | #3
Hi Laurent,

On Mon, Dec 14, 2020 at 03:47:39PM +0200, Laurent Pinchart wrote:
> Hi Paul,
> 
> On Mon, Dec 14, 2020 at 11:20:51AM +0900, paul.elder@ideasonboard.com wrote:
> > On Sat, Dec 12, 2020 at 08:51:16PM +0200, Laurent Pinchart wrote:
> > > Replace manual resource destruction with std::unique_ptr<> where
> > > applicable. This removes the need for several destructors.
> > > 
> > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > > ---
> > >  src/libcamera/pipeline/ipu3/cio2.cpp         | 11 ++---------
> > >  src/libcamera/pipeline/ipu3/cio2.h           |  9 ++++-----
> > >  src/libcamera/pipeline/rkisp1/rkisp1.cpp     | 19 +++++++------------
> > >  src/libcamera/pipeline/simple/converter.cpp  |  8 +-------
> > >  src/libcamera/pipeline/simple/converter.h    |  3 +--
> > >  src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 12 ++++--------
> > >  src/libcamera/pipeline/vimc/vimc.cpp         | 11 +++--------
> > >  7 files changed, 22 insertions(+), 51 deletions(-)
> > > 
> > > diff --git a/src/libcamera/pipeline/ipu3/cio2.cpp b/src/libcamera/pipeline/ipu3/cio2.cpp
> > > index 821715e325b4..bd5260d5b288 100644
> > > --- a/src/libcamera/pipeline/ipu3/cio2.cpp
> > > +++ b/src/libcamera/pipeline/ipu3/cio2.cpp
> > > @@ -33,16 +33,9 @@ const std::map<uint32_t, PixelFormat> mbusCodesToPixelFormat = {
> > >  } /* namespace */
> > >  
> > >  CIO2Device::CIO2Device()
> > > -	: sensor_(nullptr), csi2_(nullptr)
> > >  {
> > >  }
> > >  
> > > -CIO2Device::~CIO2Device()
> > > -{
> > > -	delete csi2_;
> > > -	delete sensor_;
> > > -}
> > > -
> > >  /**
> > >   * \brief Retrieve the list of supported PixelFormats
> > >   *
> > > @@ -117,7 +110,7 @@ int CIO2Device::init(const MediaDevice *media, unsigned int index)
> > >  
> > >  	MediaLink *link = links[0];
> > >  	MediaEntity *sensorEntity = link->source()->entity();
> > > -	sensor_ = new CameraSensor(sensorEntity);
> > > +	sensor_ = std::make_unique<CameraSensor>(sensorEntity);
> > >  	ret = sensor_->init();
> > >  	if (ret)
> > >  		return ret;
> > > @@ -148,7 +141,7 @@ int CIO2Device::init(const MediaDevice *media, unsigned int index)
> > >  	 * might impact on power consumption.
> > >  	 */
> > >  
> > > -	csi2_ = new V4L2Subdevice(csi2Entity);
> > > +	csi2_ = std::make_unique<V4L2Subdevice>(csi2Entity);
> > >  	ret = csi2_->open();
> > >  	if (ret)
> > >  		return ret;
> > > diff --git a/src/libcamera/pipeline/ipu3/cio2.h b/src/libcamera/pipeline/ipu3/cio2.h
> > > index 0dca9673ca07..236ad287354a 100644
> > > --- a/src/libcamera/pipeline/ipu3/cio2.h
> > > +++ b/src/libcamera/pipeline/ipu3/cio2.h
> > > @@ -33,7 +33,6 @@ public:
> > >  	static constexpr unsigned int CIO2_BUFFER_COUNT = 4;
> > >  
> > >  	CIO2Device();
> > > -	~CIO2Device();
> > >  
> > >  	std::vector<PixelFormat> formats() const;
> > >  	std::vector<SizeRange> sizes() const;
> > > @@ -49,8 +48,8 @@ public:
> > >  	int start();
> > >  	int stop();
> > >  
> > > -	CameraSensor *sensor() { return sensor_; }
> > > -	const CameraSensor *sensor() const { return sensor_; }
> > > +	CameraSensor *sensor() { return sensor_.get(); }
> > > +	const CameraSensor *sensor() const { return sensor_.get(); }
> > >  
> > >  	int queueBuffer(Request *request, FrameBuffer *rawBuffer);
> > >  	void tryReturnBuffer(FrameBuffer *buffer);
> > > @@ -61,8 +60,8 @@ private:
> > >  
> > >  	void cio2BufferReady(FrameBuffer *buffer);
> > >  
> > > -	CameraSensor *sensor_;
> > > -	V4L2Subdevice *csi2_;
> > > +	std::unique_ptr<CameraSensor> sensor_;
> > > +	std::unique_ptr<V4L2Subdevice> csi2_;
> > >  	std::unique_ptr<V4L2VideoDevice> output_;
> > >  
> > >  	std::vector<std::unique_ptr<FrameBuffer>> buffers_;
> > > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> > > index 4d98c9027f42..021d0ffe3ffb 100644
> > > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> > > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> > > @@ -121,21 +121,16 @@ class RkISP1CameraData : public CameraData
> > >  public:
> > >  	RkISP1CameraData(PipelineHandler *pipe, RkISP1MainPath *mainPath,
> > >  			 RkISP1SelfPath *selfPath)
> > > -		: CameraData(pipe), sensor_(nullptr), frame_(0),
> > > -		  frameInfo_(pipe), mainPath_(mainPath), selfPath_(selfPath)
> > > +		: CameraData(pipe), frame_(0), frameInfo_(pipe),
> > > +		  mainPath_(mainPath), selfPath_(selfPath)
> > >  	{
> > >  	}
> > >  
> > > -	~RkISP1CameraData()
> > > -	{
> > > -		delete sensor_;
> > > -	}
> > > -
> > >  	int loadIPA();
> > >  
> > >  	Stream mainPathStream_;
> > >  	Stream selfPathStream_;
> > > -	CameraSensor *sensor_;
> > > +	std::unique_ptr<CameraSensor> sensor_;
> > >  	unsigned int frame_;
> > >  	std::vector<IPABuffer> ipaBuffers_;
> > >  	RkISP1Frames frameInfo_;
> > > @@ -430,7 +425,7 @@ void RkISP1CameraData::queueFrameAction(unsigned int frame,
> > >  	case RKISP1_IPA_ACTION_V4L2_SET: {
> > >  		const ControlList &controls = action.controls[0];
> > >  		timeline_.scheduleAction(std::make_unique<RkISP1ActionSetSensor>(frame,
> > > -										 sensor_,
> > > +										 sensor_.get(),
> > >  										 controls));
> > >  		break;
> > >  	}
> > > @@ -489,7 +484,7 @@ bool RkISP1CameraConfiguration::fitsAllPaths(const StreamConfiguration &cfg)
> > >  
> > >  CameraConfiguration::Status RkISP1CameraConfiguration::validate()
> > >  {
> > > -	const CameraSensor *sensor = data_->sensor_;
> > > +	const CameraSensor *sensor = data_->sensor_.get();
> > >  	Status status = Valid;
> > 
> > What's wrong with using the unique_ptr here directly?
> 
> Nothing as such. We could s/sensor/data_->sensor_/ in the function. The
> local sensor variable is just a shorthand. We could also write
> 
> 	const std::unique_ptr<CameraSensor> &sensor = data_->sensor_;
> 
> but that seems a bit more complicated. Would you prefer that ?

Ah yeah, true. Nah, what you have is fine.


Paul

> > Just wondering though, so
> > 
> > Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
> > 
> > >  	if (config_.empty())
> > > @@ -660,7 +655,7 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)
> > >  	RkISP1CameraConfiguration *config =
> > >  		static_cast<RkISP1CameraConfiguration *>(c);
> > >  	RkISP1CameraData *data = cameraData(camera);
> > > -	CameraSensor *sensor = data->sensor_;
> > > +	CameraSensor *sensor = data->sensor_.get();
> > >  	int ret;
> > >  
> > >  	ret = initLinks(camera, sensor, *config);
> > > @@ -1035,7 +1030,7 @@ int PipelineHandlerRkISP1::createCamera(MediaEntity *sensor)
> > >  
> > >  	data->controlInfo_ = std::move(ctrls);
> > >  
> > > -	data->sensor_ = new CameraSensor(sensor);
> > > +	data->sensor_ = std::make_unique<CameraSensor>(sensor);
> > >  	ret = data->sensor_->init();
> > >  	if (ret)
> > >  		return ret;
> > > diff --git a/src/libcamera/pipeline/simple/converter.cpp b/src/libcamera/pipeline/simple/converter.cpp
> > > index 67e6e864aa0a..a6a8e139cb3e 100644
> > > --- a/src/libcamera/pipeline/simple/converter.cpp
> > > +++ b/src/libcamera/pipeline/simple/converter.cpp
> > > @@ -24,7 +24,6 @@ namespace libcamera {
> > >  LOG_DECLARE_CATEGORY(SimplePipeline)
> > >  
> > >  SimpleConverter::SimpleConverter(MediaDevice *media)
> > > -	: m2m_(nullptr)
> > >  {
> > >  	/*
> > >  	 * Locate the video node. There's no need to validate the pipeline
> > > @@ -38,17 +37,12 @@ SimpleConverter::SimpleConverter(MediaDevice *media)
> > >  	if (it == entities.end())
> > >  		return;
> > >  
> > > -	m2m_ = new V4L2M2MDevice((*it)->deviceNode());
> > > +	m2m_ = std::make_unique<V4L2M2MDevice>((*it)->deviceNode());
> > >  
> > >  	m2m_->output()->bufferReady.connect(this, &SimpleConverter::outputBufferReady);
> > >  	m2m_->capture()->bufferReady.connect(this, &SimpleConverter::captureBufferReady);
> > >  }
> > >  
> > > -SimpleConverter::~SimpleConverter()
> > > -{
> > > -	delete m2m_;
> > > -}
> > > -
> > >  int SimpleConverter::open()
> > >  {
> > >  	if (!m2m_)
> > > diff --git a/src/libcamera/pipeline/simple/converter.h b/src/libcamera/pipeline/simple/converter.h
> > > index 78296680aa14..a3c4d899cfc8 100644
> > > --- a/src/libcamera/pipeline/simple/converter.h
> > > +++ b/src/libcamera/pipeline/simple/converter.h
> > > @@ -29,7 +29,6 @@ class SimpleConverter
> > >  {
> > >  public:
> > >  	SimpleConverter(MediaDevice *media);
> > > -	~SimpleConverter();
> > >  
> > >  	int open();
> > >  	void close();
> > > @@ -56,7 +55,7 @@ private:
> > >  	void captureBufferReady(FrameBuffer *buffer);
> > >  	void outputBufferReady(FrameBuffer *buffer);
> > >  
> > > -	V4L2M2MDevice *m2m_;
> > > +	std::unique_ptr<V4L2M2MDevice> m2m_;
> > >  
> > >  	std::queue<FrameBuffer *> captureDoneQueue_;
> > >  	std::queue<FrameBuffer *> outputDoneQueue_;
> > > diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> > > index 87b0f03d143a..e54f45bb8825 100644
> > > --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> > > +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> > > @@ -9,6 +9,7 @@
> > >  #include <fstream>
> > >  #include <iomanip>
> > >  #include <math.h>
> > > +#include <memory>
> > >  #include <tuple>
> > >  
> > >  #include <libcamera/camera.h>
> > > @@ -35,21 +36,16 @@ class UVCCameraData : public CameraData
> > >  {
> > >  public:
> > >  	UVCCameraData(PipelineHandler *pipe)
> > > -		: CameraData(pipe), video_(nullptr)
> > > +		: CameraData(pipe)
> > >  	{
> > >  	}
> > >  
> > > -	~UVCCameraData()
> > > -	{
> > > -		delete video_;
> > > -	}
> > > -
> > >  	int init(MediaDevice *media);
> > >  	void addControl(uint32_t cid, const ControlInfo &v4l2info,
> > >  			ControlInfoMap::Map *ctrls);
> > >  	void bufferReady(FrameBuffer *buffer);
> > >  
> > > -	V4L2VideoDevice *video_;
> > > +	std::unique_ptr<V4L2VideoDevice> video_;
> > >  	Stream stream_;
> > >  };
> > >  
> > > @@ -499,7 +495,7 @@ int UVCCameraData::init(MediaDevice *media)
> > >  	}
> > >  
> > >  	/* Create and open the video device. */
> > > -	video_ = new V4L2VideoDevice(*entity);
> > > +	video_ = std::make_unique<V4L2VideoDevice>(*entity);
> > >  	ret = video_->open();
> > >  	if (ret)
> > >  		return ret;
> > > diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp
> > > index 72256f5b4190..8bda746f3136 100644
> > > --- a/src/libcamera/pipeline/vimc/vimc.cpp
> > > +++ b/src/libcamera/pipeline/vimc/vimc.cpp
> > > @@ -42,20 +42,15 @@ class VimcCameraData : public CameraData
> > >  {
> > >  public:
> > >  	VimcCameraData(PipelineHandler *pipe, MediaDevice *media)
> > > -		: CameraData(pipe), media_(media), sensor_(nullptr)
> > > +		: CameraData(pipe), media_(media)
> > >  	{
> > >  	}
> > >  
> > > -	~VimcCameraData()
> > > -	{
> > > -		delete sensor_;
> > > -	}
> > > -
> > >  	int init();
> > >  	void bufferReady(FrameBuffer *buffer);
> > >  
> > >  	MediaDevice *media_;
> > > -	CameraSensor *sensor_;
> > > +	std::unique_ptr<CameraSensor> sensor_;
> > >  	std::unique_ptr<V4L2Subdevice> debayer_;
> > >  	std::unique_ptr<V4L2Subdevice> scaler_;
> > >  	std::unique_ptr<V4L2VideoDevice> video_;
> > > @@ -461,7 +456,7 @@ int VimcCameraData::init()
> > >  		return ret;
> > >  
> > >  	/* Create and open the camera sensor, debayer, scaler and video device. */
> > > -	sensor_ = new CameraSensor(media_->getEntityByName("Sensor B"));
> > > +	sensor_ = std::make_unique<CameraSensor>(media_->getEntityByName("Sensor B"));
> > >  	ret = sensor_->init();
> > >  	if (ret)
> > >  		return ret;
> 
> -- 
> Regards,
> 
> Laurent Pinchart

Patch
diff mbox series

diff --git a/src/libcamera/pipeline/ipu3/cio2.cpp b/src/libcamera/pipeline/ipu3/cio2.cpp
index 821715e325b4..bd5260d5b288 100644
--- a/src/libcamera/pipeline/ipu3/cio2.cpp
+++ b/src/libcamera/pipeline/ipu3/cio2.cpp
@@ -33,16 +33,9 @@  const std::map<uint32_t, PixelFormat> mbusCodesToPixelFormat = {
 } /* namespace */
 
 CIO2Device::CIO2Device()
-	: sensor_(nullptr), csi2_(nullptr)
 {
 }
 
-CIO2Device::~CIO2Device()
-{
-	delete csi2_;
-	delete sensor_;
-}
-
 /**
  * \brief Retrieve the list of supported PixelFormats
  *
@@ -117,7 +110,7 @@  int CIO2Device::init(const MediaDevice *media, unsigned int index)
 
 	MediaLink *link = links[0];
 	MediaEntity *sensorEntity = link->source()->entity();
-	sensor_ = new CameraSensor(sensorEntity);
+	sensor_ = std::make_unique<CameraSensor>(sensorEntity);
 	ret = sensor_->init();
 	if (ret)
 		return ret;
@@ -148,7 +141,7 @@  int CIO2Device::init(const MediaDevice *media, unsigned int index)
 	 * might impact on power consumption.
 	 */
 
-	csi2_ = new V4L2Subdevice(csi2Entity);
+	csi2_ = std::make_unique<V4L2Subdevice>(csi2Entity);
 	ret = csi2_->open();
 	if (ret)
 		return ret;
diff --git a/src/libcamera/pipeline/ipu3/cio2.h b/src/libcamera/pipeline/ipu3/cio2.h
index 0dca9673ca07..236ad287354a 100644
--- a/src/libcamera/pipeline/ipu3/cio2.h
+++ b/src/libcamera/pipeline/ipu3/cio2.h
@@ -33,7 +33,6 @@  public:
 	static constexpr unsigned int CIO2_BUFFER_COUNT = 4;
 
 	CIO2Device();
-	~CIO2Device();
 
 	std::vector<PixelFormat> formats() const;
 	std::vector<SizeRange> sizes() const;
@@ -49,8 +48,8 @@  public:
 	int start();
 	int stop();
 
-	CameraSensor *sensor() { return sensor_; }
-	const CameraSensor *sensor() const { return sensor_; }
+	CameraSensor *sensor() { return sensor_.get(); }
+	const CameraSensor *sensor() const { return sensor_.get(); }
 
 	int queueBuffer(Request *request, FrameBuffer *rawBuffer);
 	void tryReturnBuffer(FrameBuffer *buffer);
@@ -61,8 +60,8 @@  private:
 
 	void cio2BufferReady(FrameBuffer *buffer);
 
-	CameraSensor *sensor_;
-	V4L2Subdevice *csi2_;
+	std::unique_ptr<CameraSensor> sensor_;
+	std::unique_ptr<V4L2Subdevice> csi2_;
 	std::unique_ptr<V4L2VideoDevice> output_;
 
 	std::vector<std::unique_ptr<FrameBuffer>> buffers_;
diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
index 4d98c9027f42..021d0ffe3ffb 100644
--- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
+++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
@@ -121,21 +121,16 @@  class RkISP1CameraData : public CameraData
 public:
 	RkISP1CameraData(PipelineHandler *pipe, RkISP1MainPath *mainPath,
 			 RkISP1SelfPath *selfPath)
-		: CameraData(pipe), sensor_(nullptr), frame_(0),
-		  frameInfo_(pipe), mainPath_(mainPath), selfPath_(selfPath)
+		: CameraData(pipe), frame_(0), frameInfo_(pipe),
+		  mainPath_(mainPath), selfPath_(selfPath)
 	{
 	}
 
-	~RkISP1CameraData()
-	{
-		delete sensor_;
-	}
-
 	int loadIPA();
 
 	Stream mainPathStream_;
 	Stream selfPathStream_;
-	CameraSensor *sensor_;
+	std::unique_ptr<CameraSensor> sensor_;
 	unsigned int frame_;
 	std::vector<IPABuffer> ipaBuffers_;
 	RkISP1Frames frameInfo_;
@@ -430,7 +425,7 @@  void RkISP1CameraData::queueFrameAction(unsigned int frame,
 	case RKISP1_IPA_ACTION_V4L2_SET: {
 		const ControlList &controls = action.controls[0];
 		timeline_.scheduleAction(std::make_unique<RkISP1ActionSetSensor>(frame,
-										 sensor_,
+										 sensor_.get(),
 										 controls));
 		break;
 	}
@@ -489,7 +484,7 @@  bool RkISP1CameraConfiguration::fitsAllPaths(const StreamConfiguration &cfg)
 
 CameraConfiguration::Status RkISP1CameraConfiguration::validate()
 {
-	const CameraSensor *sensor = data_->sensor_;
+	const CameraSensor *sensor = data_->sensor_.get();
 	Status status = Valid;
 
 	if (config_.empty())
@@ -660,7 +655,7 @@  int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)
 	RkISP1CameraConfiguration *config =
 		static_cast<RkISP1CameraConfiguration *>(c);
 	RkISP1CameraData *data = cameraData(camera);
-	CameraSensor *sensor = data->sensor_;
+	CameraSensor *sensor = data->sensor_.get();
 	int ret;
 
 	ret = initLinks(camera, sensor, *config);
@@ -1035,7 +1030,7 @@  int PipelineHandlerRkISP1::createCamera(MediaEntity *sensor)
 
 	data->controlInfo_ = std::move(ctrls);
 
-	data->sensor_ = new CameraSensor(sensor);
+	data->sensor_ = std::make_unique<CameraSensor>(sensor);
 	ret = data->sensor_->init();
 	if (ret)
 		return ret;
diff --git a/src/libcamera/pipeline/simple/converter.cpp b/src/libcamera/pipeline/simple/converter.cpp
index 67e6e864aa0a..a6a8e139cb3e 100644
--- a/src/libcamera/pipeline/simple/converter.cpp
+++ b/src/libcamera/pipeline/simple/converter.cpp
@@ -24,7 +24,6 @@  namespace libcamera {
 LOG_DECLARE_CATEGORY(SimplePipeline)
 
 SimpleConverter::SimpleConverter(MediaDevice *media)
-	: m2m_(nullptr)
 {
 	/*
 	 * Locate the video node. There's no need to validate the pipeline
@@ -38,17 +37,12 @@  SimpleConverter::SimpleConverter(MediaDevice *media)
 	if (it == entities.end())
 		return;
 
-	m2m_ = new V4L2M2MDevice((*it)->deviceNode());
+	m2m_ = std::make_unique<V4L2M2MDevice>((*it)->deviceNode());
 
 	m2m_->output()->bufferReady.connect(this, &SimpleConverter::outputBufferReady);
 	m2m_->capture()->bufferReady.connect(this, &SimpleConverter::captureBufferReady);
 }
 
-SimpleConverter::~SimpleConverter()
-{
-	delete m2m_;
-}
-
 int SimpleConverter::open()
 {
 	if (!m2m_)
diff --git a/src/libcamera/pipeline/simple/converter.h b/src/libcamera/pipeline/simple/converter.h
index 78296680aa14..a3c4d899cfc8 100644
--- a/src/libcamera/pipeline/simple/converter.h
+++ b/src/libcamera/pipeline/simple/converter.h
@@ -29,7 +29,6 @@  class SimpleConverter
 {
 public:
 	SimpleConverter(MediaDevice *media);
-	~SimpleConverter();
 
 	int open();
 	void close();
@@ -56,7 +55,7 @@  private:
 	void captureBufferReady(FrameBuffer *buffer);
 	void outputBufferReady(FrameBuffer *buffer);
 
-	V4L2M2MDevice *m2m_;
+	std::unique_ptr<V4L2M2MDevice> m2m_;
 
 	std::queue<FrameBuffer *> captureDoneQueue_;
 	std::queue<FrameBuffer *> outputDoneQueue_;
diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
index 87b0f03d143a..e54f45bb8825 100644
--- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
+++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
@@ -9,6 +9,7 @@ 
 #include <fstream>
 #include <iomanip>
 #include <math.h>
+#include <memory>
 #include <tuple>
 
 #include <libcamera/camera.h>
@@ -35,21 +36,16 @@  class UVCCameraData : public CameraData
 {
 public:
 	UVCCameraData(PipelineHandler *pipe)
-		: CameraData(pipe), video_(nullptr)
+		: CameraData(pipe)
 	{
 	}
 
-	~UVCCameraData()
-	{
-		delete video_;
-	}
-
 	int init(MediaDevice *media);
 	void addControl(uint32_t cid, const ControlInfo &v4l2info,
 			ControlInfoMap::Map *ctrls);
 	void bufferReady(FrameBuffer *buffer);
 
-	V4L2VideoDevice *video_;
+	std::unique_ptr<V4L2VideoDevice> video_;
 	Stream stream_;
 };
 
@@ -499,7 +495,7 @@  int UVCCameraData::init(MediaDevice *media)
 	}
 
 	/* Create and open the video device. */
-	video_ = new V4L2VideoDevice(*entity);
+	video_ = std::make_unique<V4L2VideoDevice>(*entity);
 	ret = video_->open();
 	if (ret)
 		return ret;
diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp
index 72256f5b4190..8bda746f3136 100644
--- a/src/libcamera/pipeline/vimc/vimc.cpp
+++ b/src/libcamera/pipeline/vimc/vimc.cpp
@@ -42,20 +42,15 @@  class VimcCameraData : public CameraData
 {
 public:
 	VimcCameraData(PipelineHandler *pipe, MediaDevice *media)
-		: CameraData(pipe), media_(media), sensor_(nullptr)
+		: CameraData(pipe), media_(media)
 	{
 	}
 
-	~VimcCameraData()
-	{
-		delete sensor_;
-	}
-
 	int init();
 	void bufferReady(FrameBuffer *buffer);
 
 	MediaDevice *media_;
-	CameraSensor *sensor_;
+	std::unique_ptr<CameraSensor> sensor_;
 	std::unique_ptr<V4L2Subdevice> debayer_;
 	std::unique_ptr<V4L2Subdevice> scaler_;
 	std::unique_ptr<V4L2VideoDevice> video_;
@@ -461,7 +456,7 @@  int VimcCameraData::init()
 		return ret;
 
 	/* Create and open the camera sensor, debayer, scaler and video device. */
-	sensor_ = new CameraSensor(media_->getEntityByName("Sensor B"));
+	sensor_ = std::make_unique<CameraSensor>(media_->getEntityByName("Sensor B"));
 	ret = sensor_->init();
 	if (ret)
 		return ret;