| Message ID | 20200627030043.2088585-14-niklas.soderlund@ragnatech.se |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
On Sat, Jun 27, 2020 at 05:00:43AM +0200, Niklas Söderlund wrote: > The struct ImgUOutput now only contains one member that are in use, the is in use > video device. Remove the struct and use the video device directly > instead. > > Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> > --- > src/libcamera/pipeline/ipu3/imgu.cpp | 55 ++++++++++++---------------- > src/libcamera/pipeline/ipu3/imgu.h | 25 ++++--------- > src/libcamera/pipeline/ipu3/ipu3.cpp | 12 +++--- > 3 files changed, 37 insertions(+), 55 deletions(-) > > diff --git a/src/libcamera/pipeline/ipu3/imgu.cpp b/src/libcamera/pipeline/ipu3/imgu.cpp > index d54e844d9bfc5147..a601ca3fa2b1803f 100644 > --- a/src/libcamera/pipeline/ipu3/imgu.cpp > +++ b/src/libcamera/pipeline/ipu3/imgu.cpp > @@ -55,31 +55,22 @@ int ImgUDevice::init(MediaDevice *media, unsigned int index) > if (ret) > return ret; > > - output_.dev = V4L2VideoDevice::fromEntityName(media, name_ + " output"); > - ret = output_.dev->open(); > + output_ = V4L2VideoDevice::fromEntityName(media, name_ + " output"); > + ret = output_->open(); > if (ret) > return ret; > > - output_.pad = PAD_OUTPUT; > - output_.name = "output"; > - > - viewfinder_.dev = V4L2VideoDevice::fromEntityName(media, > + viewfinder_ = V4L2VideoDevice::fromEntityName(media, > name_ + " viewfinder"); > - ret = viewfinder_.dev->open(); > + ret = viewfinder_->open(); > if (ret) > return ret; > > - viewfinder_.pad = PAD_VF; > - viewfinder_.name = "viewfinder"; > - > - stat_.dev = V4L2VideoDevice::fromEntityName(media, name_ + " 3a stat"); > - ret = stat_.dev->open(); > + stat_ = V4L2VideoDevice::fromEntityName(media, name_ + " 3a stat"); > + ret = stat_->open(); > if (ret) > return ret; > > - stat_.pad = PAD_STAT; > - stat_.name = "stat"; > - > return 0; > } > > @@ -142,19 +133,19 @@ int ImgUDevice::configureInput(const Size &size, > int ImgUDevice::configureOutput(const StreamConfiguration &cfg, > V4L2DeviceFormat *outputFormat) > { > - return configureVideoDevice(output_.dev, PAD_OUTPUT, cfg, outputFormat); > + return configureVideoDevice(output_, PAD_OUTPUT, cfg, outputFormat); > } > > int ImgUDevice::configureViewfinder(const StreamConfiguration &cfg, > V4L2DeviceFormat *outputFormat) > { > - return configureVideoDevice(viewfinder_.dev, PAD_VF, cfg, outputFormat); > + return configureVideoDevice(viewfinder_, PAD_VF, cfg, outputFormat); > } > > int ImgUDevice::configureStat(const StreamConfiguration &cfg, > V4L2DeviceFormat *outputFormat) > { > - return configureVideoDevice(stat_.dev, PAD_STAT, cfg, outputFormat); > + return configureVideoDevice(stat_, PAD_STAT, cfg, outputFormat); > } > > int ImgUDevice::configureVideoDevice(V4L2VideoDevice *dev, unsigned int pad, > @@ -170,7 +161,7 @@ int ImgUDevice::configureVideoDevice(V4L2VideoDevice *dev, unsigned int pad, > return ret; > > /* No need to apply format to the stat node. */ > - if (dev == stat_.dev) > + if (dev == stat_) > return 0; > > *outputFormat = {}; > @@ -182,7 +173,7 @@ int ImgUDevice::configureVideoDevice(V4L2VideoDevice *dev, unsigned int pad, > if (ret) > return ret; > > - const char *name = dev == output_.dev ? "output" : "viewfinder"; > + const char *name = dev == output_ ? "output" : "viewfinder"; > LOG(IPU3, Debug) << "ImgU " << name << " format = " > << outputFormat->toString(); > > @@ -208,19 +199,19 @@ int ImgUDevice::allocateBuffers(unsigned int bufferCount) > * > * \todo To be revised when we'll actually use the stat node. > */ > - ret = stat_.dev->importBuffers(bufferCount); > + ret = stat_->importBuffers(bufferCount); > if (ret < 0) { > LOG(IPU3, Error) << "Failed to allocate ImgU stat buffers"; > goto error; > } > > - ret = output_.dev->importBuffers(bufferCount); > + ret = output_->importBuffers(bufferCount); > if (ret < 0) { > LOG(IPU3, Error) << "Failed to import ImgU output buffers"; > goto error; > } > > - ret = viewfinder_.dev->importBuffers(bufferCount); > + ret = viewfinder_->importBuffers(bufferCount); > if (ret < 0) { > LOG(IPU3, Error) << "Failed to import ImgU viewfinder buffers"; > goto error; > @@ -241,15 +232,15 @@ void ImgUDevice::freeBuffers() > { > int ret; > > - ret = output_.dev->releaseBuffers(); > + ret = output_->releaseBuffers(); > if (ret) > LOG(IPU3, Error) << "Failed to release ImgU output buffers"; > > - ret = stat_.dev->releaseBuffers(); > + ret = stat_->releaseBuffers(); > if (ret) > LOG(IPU3, Error) << "Failed to release ImgU stat buffers"; > > - ret = viewfinder_.dev->releaseBuffers(); > + ret = viewfinder_->releaseBuffers(); > if (ret) > LOG(IPU3, Error) << "Failed to release ImgU viewfinder buffers"; > > @@ -263,19 +254,19 @@ int ImgUDevice::start() > int ret; > > /* Start the ImgU video devices. */ > - ret = output_.dev->streamOn(); > + ret = output_->streamOn(); > if (ret) { > LOG(IPU3, Error) << "Failed to start ImgU output"; > return ret; > } > > - ret = viewfinder_.dev->streamOn(); > + ret = viewfinder_->streamOn(); > if (ret) { > LOG(IPU3, Error) << "Failed to start ImgU viewfinder"; > return ret; > } > > - ret = stat_.dev->streamOn(); > + ret = stat_->streamOn(); > if (ret) { > LOG(IPU3, Error) << "Failed to start ImgU stat"; > return ret; > @@ -294,9 +285,9 @@ int ImgUDevice::stop() > { > int ret; > > - ret = output_.dev->streamOff(); > - ret |= viewfinder_.dev->streamOff(); > - ret |= stat_.dev->streamOff(); > + ret = output_->streamOff(); > + ret |= viewfinder_->streamOff(); > + ret |= stat_->streamOff(); > ret |= input_->streamOff(); > > return ret; > diff --git a/src/libcamera/pipeline/ipu3/imgu.h b/src/libcamera/pipeline/ipu3/imgu.h > index f8fd54089753b40e..2ac77ce5719f88f1 100644 > --- a/src/libcamera/pipeline/ipu3/imgu.h > +++ b/src/libcamera/pipeline/ipu3/imgu.h > @@ -24,28 +24,19 @@ struct StreamConfiguration; > class ImgUDevice > { > public: > - /* ImgU output descriptor: group data specific to an ImgU output. */ > - struct ImgUOutput { > - V4L2VideoDevice *dev; > - unsigned int pad; > - std::string name; > - }; > - > ImgUDevice() > - : imgu_(nullptr), input_(nullptr) > + : imgu_(nullptr), input_(nullptr), output_(nullptr), > + viewfinder_(nullptr), stat_(nullptr) > { > - output_.dev = nullptr; > - viewfinder_.dev = nullptr; > - stat_.dev = nullptr; > } > > ~ImgUDevice() > { > delete imgu_; > delete input_; > - delete output_.dev; > - delete viewfinder_.dev; > - delete stat_.dev; > + delete output_; > + delete viewfinder_; > + delete stat_; > } > > int init(MediaDevice *media, unsigned int index); > @@ -68,9 +59,9 @@ public: > > V4L2Subdevice *imgu_; > V4L2VideoDevice *input_; > - ImgUOutput output_; > - ImgUOutput viewfinder_; > - ImgUOutput stat_; > + V4L2VideoDevice *output_; > + V4L2VideoDevice *viewfinder_; > + V4L2VideoDevice *stat_; > /* \todo Add param video device for 3A tuning */ > > private: > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp > index 6ae4728b470f9487..c25bcaaeb5fb813f 100644 > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp > @@ -561,9 +561,9 @@ int PipelineHandlerIPU3::exportFrameBuffers(Camera *camera, Stream *stream, > unsigned int count = stream->configuration().bufferCount; > > if (stream == &data->outStream_) > - return data->imgu_.output_.dev->exportBuffers(count, buffers); > + return data->imgu_.output_->exportBuffers(count, buffers); > else if (stream == &data->vfStream_) > - return data->imgu_.viewfinder_.dev->exportBuffers(count, buffers); > + return data->imgu_.viewfinder_->exportBuffers(count, buffers); > else if (stream == &data->rawStream_) > return data->cio2_.exportBuffers(count, buffers); > > @@ -678,9 +678,9 @@ int PipelineHandlerIPU3::queueRequestDevice(Camera *camera, Request *request) > int ret; > > if (stream == &data->outStream_) > - ret = data->imgu_.output_.dev->queueBuffer(buffer); > + ret = data->imgu_.output_->queueBuffer(buffer); > else if (stream == &data->vfStream_) > - ret = data->imgu_.viewfinder_.dev->queueBuffer(buffer); > + ret = data->imgu_.viewfinder_->queueBuffer(buffer); > else > continue; > > @@ -802,9 +802,9 @@ int PipelineHandlerIPU3::registerCameras() > &IPU3CameraData::cio2BufferReady); > data->imgu_.input_->bufferReady.connect(&data->cio2_, > &CIO2Device::tryReturnBuffer); > - data->imgu_.output_.dev->bufferReady.connect(data.get(), > + data->imgu_.output_->bufferReady.connect(data.get(), > &IPU3CameraData::imguOutputBufferReady); > - data->imgu_.viewfinder_.dev->bufferReady.connect(data.get(), > + data->imgu_.viewfinder_->bufferReady.connect(data.get(), > &IPU3CameraData::imguOutputBufferReady); > > /* Create and register the Camera instance. */ > -- > 2.27.0 > > _______________________________________________ > libcamera-devel mailing list > libcamera-devel@lists.libcamera.org > https://lists.libcamera.org/listinfo/libcamera-devel
Hi Niklas, Thank you for the patch. On Sat, Jun 27, 2020 at 05:00:43AM +0200, Niklas Söderlund wrote: > The struct ImgUOutput now only contains one member that are in use, the > video device. Remove the struct and use the video device directly > instead. > > Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> > --- > src/libcamera/pipeline/ipu3/imgu.cpp | 55 ++++++++++++---------------- > src/libcamera/pipeline/ipu3/imgu.h | 25 ++++--------- > src/libcamera/pipeline/ipu3/ipu3.cpp | 12 +++--- > 3 files changed, 37 insertions(+), 55 deletions(-) > > diff --git a/src/libcamera/pipeline/ipu3/imgu.cpp b/src/libcamera/pipeline/ipu3/imgu.cpp > index d54e844d9bfc5147..a601ca3fa2b1803f 100644 > --- a/src/libcamera/pipeline/ipu3/imgu.cpp > +++ b/src/libcamera/pipeline/ipu3/imgu.cpp > @@ -55,31 +55,22 @@ int ImgUDevice::init(MediaDevice *media, unsigned int index) > if (ret) > return ret; > > - output_.dev = V4L2VideoDevice::fromEntityName(media, name_ + " output"); > - ret = output_.dev->open(); > + output_ = V4L2VideoDevice::fromEntityName(media, name_ + " output"); > + ret = output_->open(); > if (ret) > return ret; > > - output_.pad = PAD_OUTPUT; > - output_.name = "output"; > - > - viewfinder_.dev = V4L2VideoDevice::fromEntityName(media, > + viewfinder_ = V4L2VideoDevice::fromEntityName(media, > name_ + " viewfinder"); Wrong indentation. > - ret = viewfinder_.dev->open(); > + ret = viewfinder_->open(); > if (ret) > return ret; > > - viewfinder_.pad = PAD_VF; > - viewfinder_.name = "viewfinder"; > - > - stat_.dev = V4L2VideoDevice::fromEntityName(media, name_ + " 3a stat"); > - ret = stat_.dev->open(); > + stat_ = V4L2VideoDevice::fromEntityName(media, name_ + " 3a stat"); > + ret = stat_->open(); > if (ret) > return ret; > > - stat_.pad = PAD_STAT; > - stat_.name = "stat"; > - > return 0; > } > > @@ -142,19 +133,19 @@ int ImgUDevice::configureInput(const Size &size, > int ImgUDevice::configureOutput(const StreamConfiguration &cfg, > V4L2DeviceFormat *outputFormat) > { > - return configureVideoDevice(output_.dev, PAD_OUTPUT, cfg, outputFormat); > + return configureVideoDevice(output_, PAD_OUTPUT, cfg, outputFormat); > } > > int ImgUDevice::configureViewfinder(const StreamConfiguration &cfg, > V4L2DeviceFormat *outputFormat) > { > - return configureVideoDevice(viewfinder_.dev, PAD_VF, cfg, outputFormat); > + return configureVideoDevice(viewfinder_, PAD_VF, cfg, outputFormat); > } > > int ImgUDevice::configureStat(const StreamConfiguration &cfg, > V4L2DeviceFormat *outputFormat) > { > - return configureVideoDevice(stat_.dev, PAD_STAT, cfg, outputFormat); > + return configureVideoDevice(stat_, PAD_STAT, cfg, outputFormat); > } > > int ImgUDevice::configureVideoDevice(V4L2VideoDevice *dev, unsigned int pad, > @@ -170,7 +161,7 @@ int ImgUDevice::configureVideoDevice(V4L2VideoDevice *dev, unsigned int pad, > return ret; > > /* No need to apply format to the stat node. */ > - if (dev == stat_.dev) > + if (dev == stat_) > return 0; > > *outputFormat = {}; > @@ -182,7 +173,7 @@ int ImgUDevice::configureVideoDevice(V4L2VideoDevice *dev, unsigned int pad, > if (ret) > return ret; > > - const char *name = dev == output_.dev ? "output" : "viewfinder"; > + const char *name = dev == output_ ? "output" : "viewfinder"; > LOG(IPU3, Debug) << "ImgU " << name << " format = " > << outputFormat->toString(); > > @@ -208,19 +199,19 @@ int ImgUDevice::allocateBuffers(unsigned int bufferCount) > * > * \todo To be revised when we'll actually use the stat node. > */ > - ret = stat_.dev->importBuffers(bufferCount); > + ret = stat_->importBuffers(bufferCount); > if (ret < 0) { > LOG(IPU3, Error) << "Failed to allocate ImgU stat buffers"; > goto error; > } > > - ret = output_.dev->importBuffers(bufferCount); > + ret = output_->importBuffers(bufferCount); > if (ret < 0) { > LOG(IPU3, Error) << "Failed to import ImgU output buffers"; > goto error; > } > > - ret = viewfinder_.dev->importBuffers(bufferCount); > + ret = viewfinder_->importBuffers(bufferCount); > if (ret < 0) { > LOG(IPU3, Error) << "Failed to import ImgU viewfinder buffers"; > goto error; > @@ -241,15 +232,15 @@ void ImgUDevice::freeBuffers() > { > int ret; > > - ret = output_.dev->releaseBuffers(); > + ret = output_->releaseBuffers(); > if (ret) > LOG(IPU3, Error) << "Failed to release ImgU output buffers"; > > - ret = stat_.dev->releaseBuffers(); > + ret = stat_->releaseBuffers(); > if (ret) > LOG(IPU3, Error) << "Failed to release ImgU stat buffers"; > > - ret = viewfinder_.dev->releaseBuffers(); > + ret = viewfinder_->releaseBuffers(); > if (ret) > LOG(IPU3, Error) << "Failed to release ImgU viewfinder buffers"; > > @@ -263,19 +254,19 @@ int ImgUDevice::start() > int ret; > > /* Start the ImgU video devices. */ > - ret = output_.dev->streamOn(); > + ret = output_->streamOn(); > if (ret) { > LOG(IPU3, Error) << "Failed to start ImgU output"; > return ret; > } > > - ret = viewfinder_.dev->streamOn(); > + ret = viewfinder_->streamOn(); > if (ret) { > LOG(IPU3, Error) << "Failed to start ImgU viewfinder"; > return ret; > } > > - ret = stat_.dev->streamOn(); > + ret = stat_->streamOn(); > if (ret) { > LOG(IPU3, Error) << "Failed to start ImgU stat"; > return ret; > @@ -294,9 +285,9 @@ int ImgUDevice::stop() > { > int ret; > > - ret = output_.dev->streamOff(); > - ret |= viewfinder_.dev->streamOff(); > - ret |= stat_.dev->streamOff(); > + ret = output_->streamOff(); > + ret |= viewfinder_->streamOff(); > + ret |= stat_->streamOff(); > ret |= input_->streamOff(); > > return ret; > diff --git a/src/libcamera/pipeline/ipu3/imgu.h b/src/libcamera/pipeline/ipu3/imgu.h > index f8fd54089753b40e..2ac77ce5719f88f1 100644 > --- a/src/libcamera/pipeline/ipu3/imgu.h > +++ b/src/libcamera/pipeline/ipu3/imgu.h > @@ -24,28 +24,19 @@ struct StreamConfiguration; > class ImgUDevice > { > public: > - /* ImgU output descriptor: group data specific to an ImgU output. */ > - struct ImgUOutput { > - V4L2VideoDevice *dev; > - unsigned int pad; > - std::string name; > - }; > - > ImgUDevice() > - : imgu_(nullptr), input_(nullptr) > + : imgu_(nullptr), input_(nullptr), output_(nullptr), > + viewfinder_(nullptr), stat_(nullptr) > { > - output_.dev = nullptr; > - viewfinder_.dev = nullptr; > - stat_.dev = nullptr; > } > > ~ImgUDevice() > { > delete imgu_; > delete input_; > - delete output_.dev; > - delete viewfinder_.dev; > - delete stat_.dev; > + delete output_; > + delete viewfinder_; > + delete stat_; > } > > int init(MediaDevice *media, unsigned int index); > @@ -68,9 +59,9 @@ public: > > V4L2Subdevice *imgu_; > V4L2VideoDevice *input_; > - ImgUOutput output_; > - ImgUOutput viewfinder_; > - ImgUOutput stat_; > + V4L2VideoDevice *output_; > + V4L2VideoDevice *viewfinder_; > + V4L2VideoDevice *stat_; You could turn these into unique_ptr to simplify the constructor and destructor. Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > /* \todo Add param video device for 3A tuning */ > > private: > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp > index 6ae4728b470f9487..c25bcaaeb5fb813f 100644 > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp > @@ -561,9 +561,9 @@ int PipelineHandlerIPU3::exportFrameBuffers(Camera *camera, Stream *stream, > unsigned int count = stream->configuration().bufferCount; > > if (stream == &data->outStream_) > - return data->imgu_.output_.dev->exportBuffers(count, buffers); > + return data->imgu_.output_->exportBuffers(count, buffers); > else if (stream == &data->vfStream_) > - return data->imgu_.viewfinder_.dev->exportBuffers(count, buffers); > + return data->imgu_.viewfinder_->exportBuffers(count, buffers); > else if (stream == &data->rawStream_) > return data->cio2_.exportBuffers(count, buffers); > > @@ -678,9 +678,9 @@ int PipelineHandlerIPU3::queueRequestDevice(Camera *camera, Request *request) > int ret; > > if (stream == &data->outStream_) > - ret = data->imgu_.output_.dev->queueBuffer(buffer); > + ret = data->imgu_.output_->queueBuffer(buffer); > else if (stream == &data->vfStream_) > - ret = data->imgu_.viewfinder_.dev->queueBuffer(buffer); > + ret = data->imgu_.viewfinder_->queueBuffer(buffer); > else > continue; > > @@ -802,9 +802,9 @@ int PipelineHandlerIPU3::registerCameras() > &IPU3CameraData::cio2BufferReady); > data->imgu_.input_->bufferReady.connect(&data->cio2_, > &CIO2Device::tryReturnBuffer); > - data->imgu_.output_.dev->bufferReady.connect(data.get(), > + data->imgu_.output_->bufferReady.connect(data.get(), > &IPU3CameraData::imguOutputBufferReady); > - data->imgu_.viewfinder_.dev->bufferReady.connect(data.get(), > + data->imgu_.viewfinder_->bufferReady.connect(data.get(), > &IPU3CameraData::imguOutputBufferReady); > > /* Create and register the Camera instance. */
Hi Laurent, Thanks for your feedback. On 2020-06-27 19:52:04 +0300, Laurent Pinchart wrote: > Hi Niklas, > > Thank you for the patch. > > On Sat, Jun 27, 2020 at 05:00:43AM +0200, Niklas Söderlund wrote: > > The struct ImgUOutput now only contains one member that are in use, the > > video device. Remove the struct and use the video device directly > > instead. > > > > Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> > > --- > > src/libcamera/pipeline/ipu3/imgu.cpp | 55 ++++++++++++---------------- > > src/libcamera/pipeline/ipu3/imgu.h | 25 ++++--------- > > src/libcamera/pipeline/ipu3/ipu3.cpp | 12 +++--- > > 3 files changed, 37 insertions(+), 55 deletions(-) > > > > diff --git a/src/libcamera/pipeline/ipu3/imgu.cpp b/src/libcamera/pipeline/ipu3/imgu.cpp > > index d54e844d9bfc5147..a601ca3fa2b1803f 100644 > > --- a/src/libcamera/pipeline/ipu3/imgu.cpp > > +++ b/src/libcamera/pipeline/ipu3/imgu.cpp > > @@ -55,31 +55,22 @@ int ImgUDevice::init(MediaDevice *media, unsigned int index) > > if (ret) > > return ret; > > > > - output_.dev = V4L2VideoDevice::fromEntityName(media, name_ + " output"); > > - ret = output_.dev->open(); > > + output_ = V4L2VideoDevice::fromEntityName(media, name_ + " output"); > > + ret = output_->open(); > > if (ret) > > return ret; > > > > - output_.pad = PAD_OUTPUT; > > - output_.name = "output"; > > - > > - viewfinder_.dev = V4L2VideoDevice::fromEntityName(media, > > + viewfinder_ = V4L2VideoDevice::fromEntityName(media, > > name_ + " viewfinder"); > > Wrong indentation. > > > - ret = viewfinder_.dev->open(); > > + ret = viewfinder_->open(); > > if (ret) > > return ret; > > > > - viewfinder_.pad = PAD_VF; > > - viewfinder_.name = "viewfinder"; > > - > > - stat_.dev = V4L2VideoDevice::fromEntityName(media, name_ + " 3a stat"); > > - ret = stat_.dev->open(); > > + stat_ = V4L2VideoDevice::fromEntityName(media, name_ + " 3a stat"); > > + ret = stat_->open(); > > if (ret) > > return ret; > > > > - stat_.pad = PAD_STAT; > > - stat_.name = "stat"; > > - > > return 0; > > } > > > > @@ -142,19 +133,19 @@ int ImgUDevice::configureInput(const Size &size, > > int ImgUDevice::configureOutput(const StreamConfiguration &cfg, > > V4L2DeviceFormat *outputFormat) > > { > > - return configureVideoDevice(output_.dev, PAD_OUTPUT, cfg, outputFormat); > > + return configureVideoDevice(output_, PAD_OUTPUT, cfg, outputFormat); > > } > > > > int ImgUDevice::configureViewfinder(const StreamConfiguration &cfg, > > V4L2DeviceFormat *outputFormat) > > { > > - return configureVideoDevice(viewfinder_.dev, PAD_VF, cfg, outputFormat); > > + return configureVideoDevice(viewfinder_, PAD_VF, cfg, outputFormat); > > } > > > > int ImgUDevice::configureStat(const StreamConfiguration &cfg, > > V4L2DeviceFormat *outputFormat) > > { > > - return configureVideoDevice(stat_.dev, PAD_STAT, cfg, outputFormat); > > + return configureVideoDevice(stat_, PAD_STAT, cfg, outputFormat); > > } > > > > int ImgUDevice::configureVideoDevice(V4L2VideoDevice *dev, unsigned int pad, > > @@ -170,7 +161,7 @@ int ImgUDevice::configureVideoDevice(V4L2VideoDevice *dev, unsigned int pad, > > return ret; > > > > /* No need to apply format to the stat node. */ > > - if (dev == stat_.dev) > > + if (dev == stat_) > > return 0; > > > > *outputFormat = {}; > > @@ -182,7 +173,7 @@ int ImgUDevice::configureVideoDevice(V4L2VideoDevice *dev, unsigned int pad, > > if (ret) > > return ret; > > > > - const char *name = dev == output_.dev ? "output" : "viewfinder"; > > + const char *name = dev == output_ ? "output" : "viewfinder"; > > LOG(IPU3, Debug) << "ImgU " << name << " format = " > > << outputFormat->toString(); > > > > @@ -208,19 +199,19 @@ int ImgUDevice::allocateBuffers(unsigned int bufferCount) > > * > > * \todo To be revised when we'll actually use the stat node. > > */ > > - ret = stat_.dev->importBuffers(bufferCount); > > + ret = stat_->importBuffers(bufferCount); > > if (ret < 0) { > > LOG(IPU3, Error) << "Failed to allocate ImgU stat buffers"; > > goto error; > > } > > > > - ret = output_.dev->importBuffers(bufferCount); > > + ret = output_->importBuffers(bufferCount); > > if (ret < 0) { > > LOG(IPU3, Error) << "Failed to import ImgU output buffers"; > > goto error; > > } > > > > - ret = viewfinder_.dev->importBuffers(bufferCount); > > + ret = viewfinder_->importBuffers(bufferCount); > > if (ret < 0) { > > LOG(IPU3, Error) << "Failed to import ImgU viewfinder buffers"; > > goto error; > > @@ -241,15 +232,15 @@ void ImgUDevice::freeBuffers() > > { > > int ret; > > > > - ret = output_.dev->releaseBuffers(); > > + ret = output_->releaseBuffers(); > > if (ret) > > LOG(IPU3, Error) << "Failed to release ImgU output buffers"; > > > > - ret = stat_.dev->releaseBuffers(); > > + ret = stat_->releaseBuffers(); > > if (ret) > > LOG(IPU3, Error) << "Failed to release ImgU stat buffers"; > > > > - ret = viewfinder_.dev->releaseBuffers(); > > + ret = viewfinder_->releaseBuffers(); > > if (ret) > > LOG(IPU3, Error) << "Failed to release ImgU viewfinder buffers"; > > > > @@ -263,19 +254,19 @@ int ImgUDevice::start() > > int ret; > > > > /* Start the ImgU video devices. */ > > - ret = output_.dev->streamOn(); > > + ret = output_->streamOn(); > > if (ret) { > > LOG(IPU3, Error) << "Failed to start ImgU output"; > > return ret; > > } > > > > - ret = viewfinder_.dev->streamOn(); > > + ret = viewfinder_->streamOn(); > > if (ret) { > > LOG(IPU3, Error) << "Failed to start ImgU viewfinder"; > > return ret; > > } > > > > - ret = stat_.dev->streamOn(); > > + ret = stat_->streamOn(); > > if (ret) { > > LOG(IPU3, Error) << "Failed to start ImgU stat"; > > return ret; > > @@ -294,9 +285,9 @@ int ImgUDevice::stop() > > { > > int ret; > > > > - ret = output_.dev->streamOff(); > > - ret |= viewfinder_.dev->streamOff(); > > - ret |= stat_.dev->streamOff(); > > + ret = output_->streamOff(); > > + ret |= viewfinder_->streamOff(); > > + ret |= stat_->streamOff(); > > ret |= input_->streamOff(); > > > > return ret; > > diff --git a/src/libcamera/pipeline/ipu3/imgu.h b/src/libcamera/pipeline/ipu3/imgu.h > > index f8fd54089753b40e..2ac77ce5719f88f1 100644 > > --- a/src/libcamera/pipeline/ipu3/imgu.h > > +++ b/src/libcamera/pipeline/ipu3/imgu.h > > @@ -24,28 +24,19 @@ struct StreamConfiguration; > > class ImgUDevice > > { > > public: > > - /* ImgU output descriptor: group data specific to an ImgU output. */ > > - struct ImgUOutput { > > - V4L2VideoDevice *dev; > > - unsigned int pad; > > - std::string name; > > - }; > > - > > ImgUDevice() > > - : imgu_(nullptr), input_(nullptr) > > + : imgu_(nullptr), input_(nullptr), output_(nullptr), > > + viewfinder_(nullptr), stat_(nullptr) > > { > > - output_.dev = nullptr; > > - viewfinder_.dev = nullptr; > > - stat_.dev = nullptr; > > } > > > > ~ImgUDevice() > > { > > delete imgu_; > > delete input_; > > - delete output_.dev; > > - delete viewfinder_.dev; > > - delete stat_.dev; > > + delete output_; > > + delete viewfinder_; > > + delete stat_; > > } > > > > int init(MediaDevice *media, unsigned int index); > > @@ -68,9 +59,9 @@ public: > > > > V4L2Subdevice *imgu_; > > V4L2VideoDevice *input_; > > - ImgUOutput output_; > > - ImgUOutput viewfinder_; > > - ImgUOutput stat_; > > + V4L2VideoDevice *output_; > > + V4L2VideoDevice *viewfinder_; > > + V4L2VideoDevice *stat_; > > You could turn these into unique_ptr to simplify the constructor and > destructor. Good idea, I will add a patch for v2 to do just that. > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > > /* \todo Add param video device for 3A tuning */ > > > > private: > > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp > > index 6ae4728b470f9487..c25bcaaeb5fb813f 100644 > > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp > > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp > > @@ -561,9 +561,9 @@ int PipelineHandlerIPU3::exportFrameBuffers(Camera *camera, Stream *stream, > > unsigned int count = stream->configuration().bufferCount; > > > > if (stream == &data->outStream_) > > - return data->imgu_.output_.dev->exportBuffers(count, buffers); > > + return data->imgu_.output_->exportBuffers(count, buffers); > > else if (stream == &data->vfStream_) > > - return data->imgu_.viewfinder_.dev->exportBuffers(count, buffers); > > + return data->imgu_.viewfinder_->exportBuffers(count, buffers); > > else if (stream == &data->rawStream_) > > return data->cio2_.exportBuffers(count, buffers); > > > > @@ -678,9 +678,9 @@ int PipelineHandlerIPU3::queueRequestDevice(Camera *camera, Request *request) > > int ret; > > > > if (stream == &data->outStream_) > > - ret = data->imgu_.output_.dev->queueBuffer(buffer); > > + ret = data->imgu_.output_->queueBuffer(buffer); > > else if (stream == &data->vfStream_) > > - ret = data->imgu_.viewfinder_.dev->queueBuffer(buffer); > > + ret = data->imgu_.viewfinder_->queueBuffer(buffer); > > else > > continue; > > > > @@ -802,9 +802,9 @@ int PipelineHandlerIPU3::registerCameras() > > &IPU3CameraData::cio2BufferReady); > > data->imgu_.input_->bufferReady.connect(&data->cio2_, > > &CIO2Device::tryReturnBuffer); > > - data->imgu_.output_.dev->bufferReady.connect(data.get(), > > + data->imgu_.output_->bufferReady.connect(data.get(), > > &IPU3CameraData::imguOutputBufferReady); > > - data->imgu_.viewfinder_.dev->bufferReady.connect(data.get(), > > + data->imgu_.viewfinder_->bufferReady.connect(data.get(), > > &IPU3CameraData::imguOutputBufferReady); > > > > /* Create and register the Camera instance. */ > > -- > Regards, > > Laurent Pinchart
diff --git a/src/libcamera/pipeline/ipu3/imgu.cpp b/src/libcamera/pipeline/ipu3/imgu.cpp index d54e844d9bfc5147..a601ca3fa2b1803f 100644 --- a/src/libcamera/pipeline/ipu3/imgu.cpp +++ b/src/libcamera/pipeline/ipu3/imgu.cpp @@ -55,31 +55,22 @@ int ImgUDevice::init(MediaDevice *media, unsigned int index) if (ret) return ret; - output_.dev = V4L2VideoDevice::fromEntityName(media, name_ + " output"); - ret = output_.dev->open(); + output_ = V4L2VideoDevice::fromEntityName(media, name_ + " output"); + ret = output_->open(); if (ret) return ret; - output_.pad = PAD_OUTPUT; - output_.name = "output"; - - viewfinder_.dev = V4L2VideoDevice::fromEntityName(media, + viewfinder_ = V4L2VideoDevice::fromEntityName(media, name_ + " viewfinder"); - ret = viewfinder_.dev->open(); + ret = viewfinder_->open(); if (ret) return ret; - viewfinder_.pad = PAD_VF; - viewfinder_.name = "viewfinder"; - - stat_.dev = V4L2VideoDevice::fromEntityName(media, name_ + " 3a stat"); - ret = stat_.dev->open(); + stat_ = V4L2VideoDevice::fromEntityName(media, name_ + " 3a stat"); + ret = stat_->open(); if (ret) return ret; - stat_.pad = PAD_STAT; - stat_.name = "stat"; - return 0; } @@ -142,19 +133,19 @@ int ImgUDevice::configureInput(const Size &size, int ImgUDevice::configureOutput(const StreamConfiguration &cfg, V4L2DeviceFormat *outputFormat) { - return configureVideoDevice(output_.dev, PAD_OUTPUT, cfg, outputFormat); + return configureVideoDevice(output_, PAD_OUTPUT, cfg, outputFormat); } int ImgUDevice::configureViewfinder(const StreamConfiguration &cfg, V4L2DeviceFormat *outputFormat) { - return configureVideoDevice(viewfinder_.dev, PAD_VF, cfg, outputFormat); + return configureVideoDevice(viewfinder_, PAD_VF, cfg, outputFormat); } int ImgUDevice::configureStat(const StreamConfiguration &cfg, V4L2DeviceFormat *outputFormat) { - return configureVideoDevice(stat_.dev, PAD_STAT, cfg, outputFormat); + return configureVideoDevice(stat_, PAD_STAT, cfg, outputFormat); } int ImgUDevice::configureVideoDevice(V4L2VideoDevice *dev, unsigned int pad, @@ -170,7 +161,7 @@ int ImgUDevice::configureVideoDevice(V4L2VideoDevice *dev, unsigned int pad, return ret; /* No need to apply format to the stat node. */ - if (dev == stat_.dev) + if (dev == stat_) return 0; *outputFormat = {}; @@ -182,7 +173,7 @@ int ImgUDevice::configureVideoDevice(V4L2VideoDevice *dev, unsigned int pad, if (ret) return ret; - const char *name = dev == output_.dev ? "output" : "viewfinder"; + const char *name = dev == output_ ? "output" : "viewfinder"; LOG(IPU3, Debug) << "ImgU " << name << " format = " << outputFormat->toString(); @@ -208,19 +199,19 @@ int ImgUDevice::allocateBuffers(unsigned int bufferCount) * * \todo To be revised when we'll actually use the stat node. */ - ret = stat_.dev->importBuffers(bufferCount); + ret = stat_->importBuffers(bufferCount); if (ret < 0) { LOG(IPU3, Error) << "Failed to allocate ImgU stat buffers"; goto error; } - ret = output_.dev->importBuffers(bufferCount); + ret = output_->importBuffers(bufferCount); if (ret < 0) { LOG(IPU3, Error) << "Failed to import ImgU output buffers"; goto error; } - ret = viewfinder_.dev->importBuffers(bufferCount); + ret = viewfinder_->importBuffers(bufferCount); if (ret < 0) { LOG(IPU3, Error) << "Failed to import ImgU viewfinder buffers"; goto error; @@ -241,15 +232,15 @@ void ImgUDevice::freeBuffers() { int ret; - ret = output_.dev->releaseBuffers(); + ret = output_->releaseBuffers(); if (ret) LOG(IPU3, Error) << "Failed to release ImgU output buffers"; - ret = stat_.dev->releaseBuffers(); + ret = stat_->releaseBuffers(); if (ret) LOG(IPU3, Error) << "Failed to release ImgU stat buffers"; - ret = viewfinder_.dev->releaseBuffers(); + ret = viewfinder_->releaseBuffers(); if (ret) LOG(IPU3, Error) << "Failed to release ImgU viewfinder buffers"; @@ -263,19 +254,19 @@ int ImgUDevice::start() int ret; /* Start the ImgU video devices. */ - ret = output_.dev->streamOn(); + ret = output_->streamOn(); if (ret) { LOG(IPU3, Error) << "Failed to start ImgU output"; return ret; } - ret = viewfinder_.dev->streamOn(); + ret = viewfinder_->streamOn(); if (ret) { LOG(IPU3, Error) << "Failed to start ImgU viewfinder"; return ret; } - ret = stat_.dev->streamOn(); + ret = stat_->streamOn(); if (ret) { LOG(IPU3, Error) << "Failed to start ImgU stat"; return ret; @@ -294,9 +285,9 @@ int ImgUDevice::stop() { int ret; - ret = output_.dev->streamOff(); - ret |= viewfinder_.dev->streamOff(); - ret |= stat_.dev->streamOff(); + ret = output_->streamOff(); + ret |= viewfinder_->streamOff(); + ret |= stat_->streamOff(); ret |= input_->streamOff(); return ret; diff --git a/src/libcamera/pipeline/ipu3/imgu.h b/src/libcamera/pipeline/ipu3/imgu.h index f8fd54089753b40e..2ac77ce5719f88f1 100644 --- a/src/libcamera/pipeline/ipu3/imgu.h +++ b/src/libcamera/pipeline/ipu3/imgu.h @@ -24,28 +24,19 @@ struct StreamConfiguration; class ImgUDevice { public: - /* ImgU output descriptor: group data specific to an ImgU output. */ - struct ImgUOutput { - V4L2VideoDevice *dev; - unsigned int pad; - std::string name; - }; - ImgUDevice() - : imgu_(nullptr), input_(nullptr) + : imgu_(nullptr), input_(nullptr), output_(nullptr), + viewfinder_(nullptr), stat_(nullptr) { - output_.dev = nullptr; - viewfinder_.dev = nullptr; - stat_.dev = nullptr; } ~ImgUDevice() { delete imgu_; delete input_; - delete output_.dev; - delete viewfinder_.dev; - delete stat_.dev; + delete output_; + delete viewfinder_; + delete stat_; } int init(MediaDevice *media, unsigned int index); @@ -68,9 +59,9 @@ public: V4L2Subdevice *imgu_; V4L2VideoDevice *input_; - ImgUOutput output_; - ImgUOutput viewfinder_; - ImgUOutput stat_; + V4L2VideoDevice *output_; + V4L2VideoDevice *viewfinder_; + V4L2VideoDevice *stat_; /* \todo Add param video device for 3A tuning */ private: diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp index 6ae4728b470f9487..c25bcaaeb5fb813f 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -561,9 +561,9 @@ int PipelineHandlerIPU3::exportFrameBuffers(Camera *camera, Stream *stream, unsigned int count = stream->configuration().bufferCount; if (stream == &data->outStream_) - return data->imgu_.output_.dev->exportBuffers(count, buffers); + return data->imgu_.output_->exportBuffers(count, buffers); else if (stream == &data->vfStream_) - return data->imgu_.viewfinder_.dev->exportBuffers(count, buffers); + return data->imgu_.viewfinder_->exportBuffers(count, buffers); else if (stream == &data->rawStream_) return data->cio2_.exportBuffers(count, buffers); @@ -678,9 +678,9 @@ int PipelineHandlerIPU3::queueRequestDevice(Camera *camera, Request *request) int ret; if (stream == &data->outStream_) - ret = data->imgu_.output_.dev->queueBuffer(buffer); + ret = data->imgu_.output_->queueBuffer(buffer); else if (stream == &data->vfStream_) - ret = data->imgu_.viewfinder_.dev->queueBuffer(buffer); + ret = data->imgu_.viewfinder_->queueBuffer(buffer); else continue; @@ -802,9 +802,9 @@ int PipelineHandlerIPU3::registerCameras() &IPU3CameraData::cio2BufferReady); data->imgu_.input_->bufferReady.connect(&data->cio2_, &CIO2Device::tryReturnBuffer); - data->imgu_.output_.dev->bufferReady.connect(data.get(), + data->imgu_.output_->bufferReady.connect(data.get(), &IPU3CameraData::imguOutputBufferReady); - data->imgu_.viewfinder_.dev->bufferReady.connect(data.get(), + data->imgu_.viewfinder_->bufferReady.connect(data.get(), &IPU3CameraData::imguOutputBufferReady); /* Create and register the Camera instance. */
The struct ImgUOutput now only contains one member that are in use, the video device. Remove the struct and use the video device directly instead. Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> --- src/libcamera/pipeline/ipu3/imgu.cpp | 55 ++++++++++++---------------- src/libcamera/pipeline/ipu3/imgu.h | 25 ++++--------- src/libcamera/pipeline/ipu3/ipu3.cpp | 12 +++--- 3 files changed, 37 insertions(+), 55 deletions(-)