Message ID | 20200628001532.2685967-14-niklas.soderlund@ragnatech.se |
---|---|
State | Accepted |
Headers | show |
Series |
|
Related | show |
Hi Niklas, Thank you for the patch. On Sun, Jun 28, 2020 at 02:15:32AM +0200, Niklas Söderlund wrote: > Instead of manually deleting the video and subdevices in the destructor > use std::unique_ptr. > > Suggested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> > --- > src/libcamera/pipeline/ipu3/imgu.cpp | 24 ++++++++++++++------- > src/libcamera/pipeline/ipu3/imgu.h | 32 ++++++++-------------------- > 2 files changed, 25 insertions(+), 31 deletions(-) > > diff --git a/src/libcamera/pipeline/ipu3/imgu.cpp b/src/libcamera/pipeline/ipu3/imgu.cpp > index 6a721d47cdacf3d6..4f11dc0dbb5fe94a 100644 > --- a/src/libcamera/pipeline/ipu3/imgu.cpp > +++ b/src/libcamera/pipeline/ipu3/imgu.cpp > @@ -44,28 +44,36 @@ int ImgUDevice::init(MediaDevice *media, unsigned int index) > * by the match() function: no need to check for newly created > * video devices and subdevice validity here. > */ > - imgu_ = V4L2Subdevice::fromEntityName(media, name_); > + imgu_ = std::unique_ptr<V4L2Subdevice>( > + V4L2Subdevice::fromEntityName(media, name_)); How about imgu_.reset(V4L2Subdevice::fromEntityName(media, name_)); and similarly below ? I'm considering a patch on top of this to make V4L2Subdevice::fromEntityName() return a unique_ptr. Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > ret = imgu_->open(); > if (ret) > return ret; > > - input_ = V4L2VideoDevice::fromEntityName(media, name_ + " input"); > + input_ = std::unique_ptr<V4L2VideoDevice>( > + V4L2VideoDevice::fromEntityName(media, > + name_ + " input")); > ret = input_->open(); > if (ret) > return ret; > > - output_ = V4L2VideoDevice::fromEntityName(media, name_ + " output"); > + output_ = std::unique_ptr<V4L2VideoDevice>( > + V4L2VideoDevice::fromEntityName(media, > + name_ + " output")); > ret = output_->open(); > if (ret) > return ret; > > - viewfinder_ = V4L2VideoDevice::fromEntityName(media, > - name_ + " viewfinder"); > + viewfinder_ = std::unique_ptr<V4L2VideoDevice>( > + V4L2VideoDevice::fromEntityName(media, > + name_ + " viewfinder")); > ret = viewfinder_->open(); > if (ret) > return ret; > > - stat_ = V4L2VideoDevice::fromEntityName(media, name_ + " 3a stat"); > + stat_ = std::unique_ptr<V4L2VideoDevice>( > + V4L2VideoDevice::fromEntityName(media, > + name_ + " 3a stat")); > ret = stat_->open(); > if (ret) > return ret; > @@ -150,7 +158,7 @@ int ImgUDevice::configureVideoDevice(V4L2VideoDevice *dev, unsigned int pad, > return ret; > > /* No need to apply format to the stat node. */ > - if (dev == stat_) > + if (dev == stat_.get()) > return 0; > > *outputFormat = {}; > @@ -162,7 +170,7 @@ int ImgUDevice::configureVideoDevice(V4L2VideoDevice *dev, unsigned int pad, > if (ret) > return ret; > > - const char *name = dev == output_ ? "output" : "viewfinder"; > + const char *name = dev == output_.get() ? "output" : "viewfinder"; > LOG(IPU3, Debug) << "ImgU " << name << " format = " > << outputFormat->toString(); > > diff --git a/src/libcamera/pipeline/ipu3/imgu.h b/src/libcamera/pipeline/ipu3/imgu.h > index 7be25e40957fcb03..308bf26ee56e4e82 100644 > --- a/src/libcamera/pipeline/ipu3/imgu.h > +++ b/src/libcamera/pipeline/ipu3/imgu.h > @@ -24,21 +24,6 @@ struct StreamConfiguration; > class ImgUDevice > { > public: > - ImgUDevice() > - : imgu_(nullptr), input_(nullptr), output_(nullptr), > - viewfinder_(nullptr), stat_(nullptr) > - { > - } > - > - ~ImgUDevice() > - { > - delete imgu_; > - delete input_; > - delete output_; > - delete viewfinder_; > - delete stat_; > - } > - > int init(MediaDevice *media, unsigned int index); > > int configureInput(const Size &size, V4L2DeviceFormat *inputFormat); > @@ -46,21 +31,22 @@ public: > int configureOutput(const StreamConfiguration &cfg, > V4L2DeviceFormat *outputFormat) > { > - return configureVideoDevice(output_, PAD_OUTPUT, cfg, > + return configureVideoDevice(output_.get(), PAD_OUTPUT, cfg, > outputFormat); > } > > int configureViewfinder(const StreamConfiguration &cfg, > V4L2DeviceFormat *outputFormat) > { > - return configureVideoDevice(viewfinder_, PAD_VF, cfg, > + return configureVideoDevice(viewfinder_.get(), PAD_VF, cfg, > outputFormat); > } > > int configureStat(const StreamConfiguration &cfg, > V4L2DeviceFormat *outputFormat) > { > - return configureVideoDevice(stat_, PAD_STAT, cfg, outputFormat); > + return configureVideoDevice(stat_.get(), PAD_STAT, cfg, > + outputFormat); > } > > int allocateBuffers(unsigned int bufferCount); > @@ -71,11 +57,11 @@ public: > > int enableLinks(bool enable); > > - V4L2Subdevice *imgu_; > - V4L2VideoDevice *input_; > - V4L2VideoDevice *output_; > - V4L2VideoDevice *viewfinder_; > - V4L2VideoDevice *stat_; > + std::unique_ptr<V4L2Subdevice> imgu_; > + std::unique_ptr<V4L2VideoDevice> input_; > + std::unique_ptr<V4L2VideoDevice> output_; > + std::unique_ptr<V4L2VideoDevice> viewfinder_; > + std::unique_ptr<V4L2VideoDevice> stat_; > /* \todo Add param video device for 3A tuning */ > > private:
diff --git a/src/libcamera/pipeline/ipu3/imgu.cpp b/src/libcamera/pipeline/ipu3/imgu.cpp index 6a721d47cdacf3d6..4f11dc0dbb5fe94a 100644 --- a/src/libcamera/pipeline/ipu3/imgu.cpp +++ b/src/libcamera/pipeline/ipu3/imgu.cpp @@ -44,28 +44,36 @@ int ImgUDevice::init(MediaDevice *media, unsigned int index) * by the match() function: no need to check for newly created * video devices and subdevice validity here. */ - imgu_ = V4L2Subdevice::fromEntityName(media, name_); + imgu_ = std::unique_ptr<V4L2Subdevice>( + V4L2Subdevice::fromEntityName(media, name_)); ret = imgu_->open(); if (ret) return ret; - input_ = V4L2VideoDevice::fromEntityName(media, name_ + " input"); + input_ = std::unique_ptr<V4L2VideoDevice>( + V4L2VideoDevice::fromEntityName(media, + name_ + " input")); ret = input_->open(); if (ret) return ret; - output_ = V4L2VideoDevice::fromEntityName(media, name_ + " output"); + output_ = std::unique_ptr<V4L2VideoDevice>( + V4L2VideoDevice::fromEntityName(media, + name_ + " output")); ret = output_->open(); if (ret) return ret; - viewfinder_ = V4L2VideoDevice::fromEntityName(media, - name_ + " viewfinder"); + viewfinder_ = std::unique_ptr<V4L2VideoDevice>( + V4L2VideoDevice::fromEntityName(media, + name_ + " viewfinder")); ret = viewfinder_->open(); if (ret) return ret; - stat_ = V4L2VideoDevice::fromEntityName(media, name_ + " 3a stat"); + stat_ = std::unique_ptr<V4L2VideoDevice>( + V4L2VideoDevice::fromEntityName(media, + name_ + " 3a stat")); ret = stat_->open(); if (ret) return ret; @@ -150,7 +158,7 @@ int ImgUDevice::configureVideoDevice(V4L2VideoDevice *dev, unsigned int pad, return ret; /* No need to apply format to the stat node. */ - if (dev == stat_) + if (dev == stat_.get()) return 0; *outputFormat = {}; @@ -162,7 +170,7 @@ int ImgUDevice::configureVideoDevice(V4L2VideoDevice *dev, unsigned int pad, if (ret) return ret; - const char *name = dev == output_ ? "output" : "viewfinder"; + const char *name = dev == output_.get() ? "output" : "viewfinder"; LOG(IPU3, Debug) << "ImgU " << name << " format = " << outputFormat->toString(); diff --git a/src/libcamera/pipeline/ipu3/imgu.h b/src/libcamera/pipeline/ipu3/imgu.h index 7be25e40957fcb03..308bf26ee56e4e82 100644 --- a/src/libcamera/pipeline/ipu3/imgu.h +++ b/src/libcamera/pipeline/ipu3/imgu.h @@ -24,21 +24,6 @@ struct StreamConfiguration; class ImgUDevice { public: - ImgUDevice() - : imgu_(nullptr), input_(nullptr), output_(nullptr), - viewfinder_(nullptr), stat_(nullptr) - { - } - - ~ImgUDevice() - { - delete imgu_; - delete input_; - delete output_; - delete viewfinder_; - delete stat_; - } - int init(MediaDevice *media, unsigned int index); int configureInput(const Size &size, V4L2DeviceFormat *inputFormat); @@ -46,21 +31,22 @@ public: int configureOutput(const StreamConfiguration &cfg, V4L2DeviceFormat *outputFormat) { - return configureVideoDevice(output_, PAD_OUTPUT, cfg, + return configureVideoDevice(output_.get(), PAD_OUTPUT, cfg, outputFormat); } int configureViewfinder(const StreamConfiguration &cfg, V4L2DeviceFormat *outputFormat) { - return configureVideoDevice(viewfinder_, PAD_VF, cfg, + return configureVideoDevice(viewfinder_.get(), PAD_VF, cfg, outputFormat); } int configureStat(const StreamConfiguration &cfg, V4L2DeviceFormat *outputFormat) { - return configureVideoDevice(stat_, PAD_STAT, cfg, outputFormat); + return configureVideoDevice(stat_.get(), PAD_STAT, cfg, + outputFormat); } int allocateBuffers(unsigned int bufferCount); @@ -71,11 +57,11 @@ public: int enableLinks(bool enable); - V4L2Subdevice *imgu_; - V4L2VideoDevice *input_; - V4L2VideoDevice *output_; - V4L2VideoDevice *viewfinder_; - V4L2VideoDevice *stat_; + std::unique_ptr<V4L2Subdevice> imgu_; + std::unique_ptr<V4L2VideoDevice> input_; + std::unique_ptr<V4L2VideoDevice> output_; + std::unique_ptr<V4L2VideoDevice> viewfinder_; + std::unique_ptr<V4L2VideoDevice> stat_; /* \todo Add param video device for 3A tuning */ private:
Instead of manually deleting the video and subdevices in the destructor use std::unique_ptr. Suggested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> --- src/libcamera/pipeline/ipu3/imgu.cpp | 24 ++++++++++++++------- src/libcamera/pipeline/ipu3/imgu.h | 32 ++++++++-------------------- 2 files changed, 25 insertions(+), 31 deletions(-)