[libcamera-devel] [PATCH v3 24/33] libcamera: pipeline: Add FrameBuffer handlers
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Sat Jan 11 02:07:39 CET 2020
Hi Niklas,
Thank you for the patch.
On Fri, Jan 10, 2020 at 08:37:59PM +0100, Niklas Söderlund wrote:
> Extend the pipeline handlers to support the FrameBuffer API with three
> new methods to handle allocation, importing and freeing of buffers. The
> new methods will replace allocateBuffers() and freeBuffers().
>
> The FrameBuffer API will use the methods on a stream level and either
> allocate or import buffers for each active stream controlled from the
> Camera class and an upcoming FrameBufferAllocator helper. With this new
> API the implementation in pipeline handlers can be made simpler as all
> streams don't need to be handled in allocateBuffers().
>
> Signed-off-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> ---
> * Changes since v2
> - In the subject line, s/pipelines/pipeline/
> - Rename allocateFrameBuffers() to exportFrameBuffers()
> - Rewrite of documentation, thanks Laurent!
> - Remove count argument from exportFrameBuffers() and
> importFrameBuffers() and instead get the buffer count from the stream
> configuration.
> ---
> src/libcamera/include/pipeline_handler.h | 5 ++
> src/libcamera/pipeline/ipu3/ipu3.cpp | 32 +++++++++++
> src/libcamera/pipeline/rkisp1/rkisp1.cpp | 23 ++++++++
> src/libcamera/pipeline/uvcvideo.cpp | 29 ++++++++++
> src/libcamera/pipeline/vimc.cpp | 29 ++++++++++
> src/libcamera/pipeline_handler.cpp | 67 ++++++++++++++++++++++++
> 6 files changed, 185 insertions(+)
>
> diff --git a/src/libcamera/include/pipeline_handler.h b/src/libcamera/include/pipeline_handler.h
> index 067baef56278b577..560be3bad8d59b20 100644
> --- a/src/libcamera/include/pipeline_handler.h
> +++ b/src/libcamera/include/pipeline_handler.h
> @@ -70,6 +70,11 @@ public:
> const StreamRoles &roles) = 0;
> virtual int configure(Camera *camera, CameraConfiguration *config) = 0;
>
> + virtual int exportFrameBuffers(Camera *camera, Stream *stream,
> + std::vector<std::unique_ptr<FrameBuffer>> *buffers) = 0;
> + virtual int importFrameBuffers(Camera *camera, Stream *stream) = 0;
> + virtual void freeFrameBuffers(Camera *camera, Stream *stream) = 0;
> +
> virtual int allocateBuffers(Camera *camera,
> const std::set<Stream *> &streams) = 0;
> virtual int freeBuffers(Camera *camera,
> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> index 7c4539d488bd2d26..b73f0f0725ee2613 100644
> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> @@ -210,6 +210,11 @@ public:
> const StreamRoles &roles) override;
> int configure(Camera *camera, CameraConfiguration *config) override;
>
> + int exportFrameBuffers(Camera *camera, Stream *stream,
> + std::vector<std::unique_ptr<FrameBuffer>> *buffers) override;
> + int importFrameBuffers(Camera *camera, Stream *stream) override;
> + void freeFrameBuffers(Camera *camera, Stream *stream) override;
> +
> int allocateBuffers(Camera *camera,
> const std::set<Stream *> &streams) override;
> int freeBuffers(Camera *camera,
> @@ -616,6 +621,33 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)
> return 0;
> }
>
> +int PipelineHandlerIPU3::exportFrameBuffers(Camera *camera, Stream *stream,
> + std::vector<std::unique_ptr<FrameBuffer>> *buffers)
> +{
> + IPU3Stream *ipu3stream = static_cast<IPU3Stream *>(stream);
> + V4L2VideoDevice *video = ipu3stream->device_->dev;
> + unsigned int count = stream->configuration().bufferCount;
> +
> + return video->exportBuffers(count, buffers);
> +}
> +
> +int PipelineHandlerIPU3::importFrameBuffers(Camera *camera, Stream *stream)
> +{
> + IPU3Stream *ipu3stream = static_cast<IPU3Stream *>(stream);
> + V4L2VideoDevice *video = ipu3stream->device_->dev;
> + unsigned int count = stream->configuration().bufferCount;
> +
> + return video->importBuffers(count);
> +}
> +
> +void PipelineHandlerIPU3::freeFrameBuffers(Camera *camera, Stream *stream)
> +{
> + IPU3Stream *ipu3stream = static_cast<IPU3Stream *>(stream);
> + V4L2VideoDevice *video = ipu3stream->device_->dev;
> +
> + video->releaseBuffers();
> +}
> +
> /**
> * \todo Clarify if 'viewfinder' and 'stat' nodes have to be set up and
> * started even if not in use. As of now, if not properly configured and
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> index dbc5df801f30e76b..ba8f93e8584c97a9 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> @@ -174,6 +174,11 @@ public:
> const StreamRoles &roles) override;
> int configure(Camera *camera, CameraConfiguration *config) override;
>
> + int exportFrameBuffers(Camera *camera, Stream *stream,
> + std::vector<std::unique_ptr<FrameBuffer>> *buffers) override;
> + int importFrameBuffers(Camera *camera, Stream *stream) override;
> + void freeFrameBuffers(Camera *camera, Stream *stream) override;
> +
> int allocateBuffers(Camera *camera,
> const std::set<Stream *> &streams) override;
> int freeBuffers(Camera *camera,
> @@ -665,6 +670,24 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)
> return 0;
> }
>
> +int PipelineHandlerRkISP1::exportFrameBuffers(Camera *camera, Stream *stream,
> + std::vector<std::unique_ptr<FrameBuffer>> *buffers)
> +{
> + unsigned int count = stream->configuration().bufferCount;
> + return video_->exportBuffers(count, buffers);
> +}
> +
> +int PipelineHandlerRkISP1::importFrameBuffers(Camera *camera, Stream *stream)
> +{
> + unsigned int count = stream->configuration().bufferCount;
> + return video_->importBuffers(count);
> +}
> +
> +void PipelineHandlerRkISP1::freeFrameBuffers(Camera *camera, Stream *stream)
> +{
> + video_->releaseBuffers();
> +}
> +
> int PipelineHandlerRkISP1::allocateBuffers(Camera *camera,
> const std::set<Stream *> &streams)
> {
> diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp
> index ce38445d2a48ca82..b72841edee572f99 100644
> --- a/src/libcamera/pipeline/uvcvideo.cpp
> +++ b/src/libcamera/pipeline/uvcvideo.cpp
> @@ -65,6 +65,11 @@ public:
> const StreamRoles &roles) override;
> int configure(Camera *camera, CameraConfiguration *config) override;
>
> + int exportFrameBuffers(Camera *camera, Stream *stream,
> + std::vector<std::unique_ptr<FrameBuffer>> *buffers) override;
> + int importFrameBuffers(Camera *camera, Stream *stream) override;
> + void freeFrameBuffers(Camera *camera, Stream *stream) override;
> +
> int allocateBuffers(Camera *camera,
> const std::set<Stream *> &streams) override;
> int freeBuffers(Camera *camera,
> @@ -193,6 +198,30 @@ int PipelineHandlerUVC::configure(Camera *camera, CameraConfiguration *config)
> return 0;
> }
>
> +int PipelineHandlerUVC::exportFrameBuffers(Camera *camera, Stream *stream,
> + std::vector<std::unique_ptr<FrameBuffer>> *buffers)
> +{
> + UVCCameraData *data = cameraData(camera);
> + unsigned int count = stream->configuration().bufferCount;
> +
> + return data->video_->exportBuffers(count, buffers);
> +}
> +
> +int PipelineHandlerUVC::importFrameBuffers(Camera *camera, Stream *stream)
> +{
> + UVCCameraData *data = cameraData(camera);
> + unsigned int count = stream->configuration().bufferCount;
> +
> + return data->video_->importBuffers(count);
> +}
> +
> +void PipelineHandlerUVC::freeFrameBuffers(Camera *camera, Stream *stream)
> +{
> + UVCCameraData *data = cameraData(camera);
> +
> + data->video_->releaseBuffers();
> +}
> +
> int PipelineHandlerUVC::allocateBuffers(Camera *camera,
> const std::set<Stream *> &streams)
> {
> diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp
> index 292900bcf8d1b359..3fb5cacde0dab5b6 100644
> --- a/src/libcamera/pipeline/vimc.cpp
> +++ b/src/libcamera/pipeline/vimc.cpp
> @@ -82,6 +82,11 @@ public:
> const StreamRoles &roles) override;
> int configure(Camera *camera, CameraConfiguration *config) override;
>
> + int exportFrameBuffers(Camera *camera, Stream *stream,
> + std::vector<std::unique_ptr<FrameBuffer>> *buffers) override;
> + int importFrameBuffers(Camera *camera, Stream *stream) override;
> + void freeFrameBuffers(Camera *camera, Stream *stream) override;
> +
> int allocateBuffers(Camera *camera,
> const std::set<Stream *> &streams) override;
> int freeBuffers(Camera *camera,
> @@ -259,6 +264,30 @@ int PipelineHandlerVimc::configure(Camera *camera, CameraConfiguration *config)
> return 0;
> }
>
> +int PipelineHandlerVimc::exportFrameBuffers(Camera *camera, Stream *stream,
> + std::vector<std::unique_ptr<FrameBuffer>> *buffers)
> +{
> + VimcCameraData *data = cameraData(camera);
> + unsigned int count = stream->configuration().bufferCount;
> +
> + return data->video_->exportBuffers(count, buffers);
> +}
> +
> +int PipelineHandlerVimc::importFrameBuffers(Camera *camera, Stream *stream)
> +{
> + VimcCameraData *data = cameraData(camera);
> + unsigned int count = stream->configuration().bufferCount;
> +
> + return data->video_->importBuffers(count);
> +}
> +
> +void PipelineHandlerVimc::freeFrameBuffers(Camera *camera, Stream *stream)
> +{
> + VimcCameraData *data = cameraData(camera);
> +
> + data->video_->releaseBuffers();
> +}
> +
> int PipelineHandlerVimc::allocateBuffers(Camera *camera,
> const std::set<Stream *> &streams)
> {
> diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp
> index 698dd52560797d7c..572f751b5217eb5f 100644
> --- a/src/libcamera/pipeline_handler.cpp
> +++ b/src/libcamera/pipeline_handler.cpp
> @@ -289,6 +289,73 @@ const ControlInfoMap &PipelineHandler::controls(Camera *camera)
> * \return 0 on success or a negative error code otherwise
> */
>
> +/**
> + * \fn PipelineHandler::exportFrameBuffers()
> + * \brief Allocate buffers for \a stream
> + * \param[in] camera The camera
> + * \param[in] stream The stream to allocate buffers for
> + * \param[out] buffers Array of buffers successfully allocated
> + *
> + * This method allocates buffers for the \a stream from the devices associated
> + * with the stream in the corresponding pipeline handler. Those buffers shall be
> + * suitable to be added to a Request for the stream, and shall be mappable to
> + * the CPU through their associated dmabufs with mmap().
> + *
Is there a reason why you haven't included the following paragraph,
proposed in the review of v2 ?
* The method may only be called after the Camera has been configured and before
* it gets started, or after it gets stopped. It shall be called only for
* streams that are part of the active camera configuration, and at most once
* per stream until buffers for the stream are freed with freeFrameBuffers().
> + * exportFrameBuffers() shall also allocate all other resources required by
> + * the pipeline handler for the stream to prepare for starting the Camera. This
> + * responsibility is shared with importFrameBuffers(), and one and only one of
> + * those two methods shall be called for each stream until the buffers are
> + * freed. The pipeline handler shall support all combinations of
> + * exportFrameBuffers() and importFrameBuffers() for the streams contained in
> + * any camera configuration.
> + *
> + * The only intended caller is the FrameBufferAllocator helper.
> + *
> + * \return 0 on success or a negative error code otherwise
> + */
> +
> +/**
> + * \fn PipelineHandler::importFrameBuffers()
> + * \brief Prepare \a stream to use external buffers
> + * \param[in] camera The camera
> + * \param[in] stream The stream to prepare for import
> + *
> + * This method prepares the pipeline handler to use buffers provided by the
> + * application for the \a stream.
> + *
> + * The method may only be called after the Camera has been configured and before
> + * it gets started, or after it gets stopped. It shall be called only for
> + * streams that are part of the active camera configuration, and at most once
> + * per stream until buffers for the stream are freed with freeFrameBuffers().
> + *
> + * importFrameBuffers() shall also allocate all other resources required by the
> + * pipeline handler for the stream to prepare for starting the Camera. This
> + * responsibility is shared with exportFrameBuffers(), and one and only one of
> + * those two methods shall be called for each stream until the buffers are
> + * freed. The pipeline handler shall support all combinations of
> + * exportFrameBuffers() and importFrameBuffers() for the streams contained in
> + * any camera configuration.
> + *
> + * The only intended caller is Camera::start().
> + *
> + * \return 0 on success or a negative error code otherwise
> + */
> +
> +/**
> + * \fn PipelineHandler::freeFrameBuffers()
> + * \brief Free buffers allocated from the stream
> + * \param[in] camera The camera
> + * \param[in] stream The stream to free buffers for
> + *
> + * This method shall free all buffers and all other resources allocated for the
> + * \a stream by exportFrameBuffers() or importFrameBuffers(). It shall be
> + * called only after a successful call to either of these two methods, and only
> + * once per stream.
> + *
> + * The only intended callers are Camera::stop() and the FrameBufferAllocator
> + * helper.
> + */
> +
> /**
> * \fn PipelineHandler::allocateBuffers()
> * \brief Allocate buffers for a stream
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list