[libcamera-devel] [PATCH v3 24/33] libcamera: pipeline: Add FrameBuffer handlers
Niklas Söderlund
niklas.soderlund at ragnatech.se
Sun Jan 12 00:38:39 CET 2020
Hi Laurent,
Thanks for your feedback.
On 2020-01-11 03:07:39 +0200, Laurent Pinchart wrote:
> 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 ?
I must have missed it :-( Sorry about that, will add for v4.
>
> * 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
--
Regards,
Niklas Söderlund
More information about the libcamera-devel
mailing list