[libcamera-devel] [PATCH 3/9] libcamera: pipeline: Support external buffers

Jacopo Mondi jacopo at jmondi.org
Wed Jul 10 11:38:05 CEST 2019


Hi Niklas,

On Sat, Jul 06, 2019 at 01:45:52PM +0200, Niklas Söderlund wrote:
> Hi Jacopo,
>
> Thanks for your patch.
>
> On 2019-07-05 00:53:28 +0200, Jacopo Mondi wrote:
> > Add support for use external buffers to libcamera pipeline handlers.
> >
> > Use the memory type flag in pipeline handlers (todo: change all pipeline
> > handlers) during buffer setup operations, to decide if the Stream's
> > internal memory has to be exported to applications, or the Stream
> > should prepare to use buffers whose memory is allocated elsewhere.
>
> I think you can drop the TODO now as it seems all pipeline handlers are
> now supprted, right?
>
> >
> > To support the last use case, a translation mechanism between the external
> > buffers provided by applications and internal ones used by pipeline
> > handlers to feed the video device will be implemented on top of this
> > change.
> >
> > Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> > ---
> >  src/libcamera/pipeline/ipu3/ipu3.cpp     | 31 +++++++++++++++++++++++-
> >  src/libcamera/pipeline/rkisp1/rkisp1.cpp |  9 ++++++-
> >  src/libcamera/pipeline/uvcvideo.cpp      |  8 +++++-
> >  src/libcamera/pipeline/vimc.cpp          |  8 +++++-
> >  4 files changed, 52 insertions(+), 4 deletions(-)
> >
> > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> > index 2de0892138a8..6f14da1f8c70 100644
> > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> > @@ -71,6 +71,7 @@ public:
> >
> >  	int importBuffers(BufferPool *pool);
> >  	int exportBuffers(ImgUOutput *output, BufferPool *pool);
> > +	int reserveDmabufBuffers(ImgUOutput *output, BufferPool *pool);
> >  	void freeBuffers();
> >
> >  	int start();
> > @@ -624,7 +625,11 @@ int PipelineHandlerIPU3::allocateBuffers(Camera *camera,
> >  		IPU3Stream *stream = static_cast<IPU3Stream *>(s);
> >  		ImgUDevice::ImgUOutput *dev = stream->device_;
> >
> > -		ret = imgu->exportBuffers(dev, &stream->bufferPool());
> > +		if (stream->memoryType() == ExternalMemory)
> > +			ret = imgu->reserveDmabufBuffers(dev,
> > +							 &stream->bufferPool());
>
> I would s/reserveDmabufBuffers/importBuffers/ to match the logic in
> other pipeline handlers.
>

If not that ImgU::importBuffers() already exists, and import CIO2
buffers into the ImgU input.

But yes, reserveDmabufBuffers is incredibl ugly, but I wanted to
convey the idea that 'import' actually just reserve buffers in the
video device.

So I've now renamed importBuffers() in importInputBuffers() and made
this new one importOutputBuffers().

Thanks
   j

> > +		else
> > +			ret = imgu->exportBuffers(dev, &stream->bufferPool());
> >  		if (ret)
> >  			goto error;
> >  	}
> > @@ -1153,6 +1158,30 @@ int ImgUDevice::exportBuffers(ImgUOutput *output, BufferPool *pool)
> >  	return 0;
> >  }
> >
> > +/**
> > + * \brief Reserve buffers in \a output from the provided \a pool
> > + * \param[in] output The ImgU output device
> > + * \param[in] pool The buffer pool used to reserve buffers in \a output
> > + *
> > + * Reserve a number of buffers equal to the number of buffers in \a pool
> > + * in the \a output device to prepare for streaming operations using buffers
> > + * whose memory has been reserved elsewhere identified with DMABUF file
> > + * descriptors.
> > + *
> > + * \return 0 on success or a negative error code otherwise
> > + */
> > +int ImgUDevice::reserveDmabufBuffers(ImgUOutput *output, BufferPool *pool)
> > +{
> > +	int ret = output->dev->importBuffers(pool);
> > +	if (ret) {
> > +		LOG(IPU3, Error) << "Failed to import buffer in "
> > +				 << output->name << " ImgU device";
> > +		return ret;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> >  /**
> >   * \brief Release buffers for all the ImgU video devices
> >   */
> > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> > index 4a5898d25f91..dbb61a21354d 100644
> > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> > @@ -319,7 +319,14 @@ int PipelineHandlerRkISP1::allocateBuffers(Camera *camera,
> >  					   const std::set<Stream *> &streams)
> >  {
> >  	Stream *stream = *streams.begin();
> > -	return video_->exportBuffers(&stream->bufferPool());
> > +	int ret;
> > +
> > +	if (stream->memoryType() == InternalMemory)
> > +		ret = video_->exportBuffers(&stream->bufferPool());
> > +	else
> > +		ret = video_->importBuffers(&stream->bufferPool());
> > +
> > +	return ret;
> >  }
> >
> >  int PipelineHandlerRkISP1::freeBuffers(Camera *camera,
> > diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp
> > index 72313cfd246f..5b3c79dda6dd 100644
> > --- a/src/libcamera/pipeline/uvcvideo.cpp
> > +++ b/src/libcamera/pipeline/uvcvideo.cpp
> > @@ -197,10 +197,16 @@ int PipelineHandlerUVC::allocateBuffers(Camera *camera,
> >  	UVCCameraData *data = cameraData(camera);
> >  	Stream *stream = *streams.begin();
> >  	const StreamConfiguration &cfg = stream->configuration();
> > +	int ret;
> >
> >  	LOG(UVC, Debug) << "Requesting " << cfg.bufferCount << " buffers";
> >
> > -	return data->video_->exportBuffers(&stream->bufferPool());
> > +	if (stream->memoryType() == InternalMemory)
> > +		ret = data->video_->exportBuffers(&stream->bufferPool());
> > +	else
> > +		ret = data->video_->importBuffers(&stream->bufferPool());
> > +
> > +	return ret;
> >  }
> >
> >  int PipelineHandlerUVC::freeBuffers(Camera *camera,
> > diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp
> > index f8a58be060bb..c67bdb0ecbff 100644
> > --- a/src/libcamera/pipeline/vimc.cpp
> > +++ b/src/libcamera/pipeline/vimc.cpp
> > @@ -199,10 +199,16 @@ int PipelineHandlerVimc::allocateBuffers(Camera *camera,
> >  	VimcCameraData *data = cameraData(camera);
> >  	Stream *stream = *streams.begin();
> >  	const StreamConfiguration &cfg = stream->configuration();
> > +	int ret;
> >
> >  	LOG(VIMC, Debug) << "Requesting " << cfg.bufferCount << " buffers";
> >
> > -	return data->video_->exportBuffers(&stream->bufferPool());
> > +	if (stream->memoryType() == InternalMemory)
> > +		ret = data->video_->exportBuffers(&stream->bufferPool());
> > +	else
> > +		ret = data->video_->importBuffers(&stream->bufferPool());
> > +
> > +	return ret;
> >  }
> >
> >  int PipelineHandlerVimc::freeBuffers(Camera *camera,
> > --
> > 2.21.0
> >
> > _______________________________________________
> > libcamera-devel mailing list
> > libcamera-devel at lists.libcamera.org
> > https://lists.libcamera.org/listinfo/libcamera-devel
>
> --
> Regards,
> Niklas Söderlund
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20190710/1b4fab45/attachment.sig>


More information about the libcamera-devel mailing list