[libcamera-devel] [PATCH 13/13] libcamera: ipu3: imgu: Remove ImgUOutput
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Sat Jun 27 18:52:04 CEST 2020
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 at 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 at 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
More information about the libcamera-devel
mailing list