[libcamera-devel] [PATCH v6 2/7] libcamera: camera: Pass the stream set to allocate/freeBuffers()
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Wed Apr 17 00:14:18 CEST 2019
Hi Jacopo,
Thank you for the patch.
On Tue, Apr 16, 2019 at 03:42:05PM +0200, Jacopo Mondi wrote:
> Pipeline handlers might need to perform allocation of internal buffers,
> setup operations, or simple sanity check before going into the
> per-stream buffer allocation.
>
> As of now, PipelineHandler::allocateBuffers() is called once for each
> active stream, leaving no space for stream-independent configuration.
>
> Change this by providing to the pipeline handlers the full set of active
> streams, and ask them to loop over them to perform per-streams
> memory allocations and freeing.
>
> Reviewed-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> ---
> src/libcamera/camera.cpp | 15 ++++++---------
> src/libcamera/include/pipeline_handler.h | 6 ++++--
> src/libcamera/pipeline/ipu3/ipu3.cpp | 13 +++++++++----
> src/libcamera/pipeline/uvcvideo.cpp | 13 +++++++++----
> src/libcamera/pipeline/vimc.cpp | 13 +++++++++----
> src/libcamera/pipeline_handler.cpp | 11 ++++++-----
> 6 files changed, 43 insertions(+), 28 deletions(-)
>
> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp
> index bdf14b31d8ee..21caa24b90b5 100644
> --- a/src/libcamera/camera.cpp
> +++ b/src/libcamera/camera.cpp
> @@ -647,13 +647,11 @@ int Camera::allocateBuffers()
> return -EINVAL;
> }
>
> - for (Stream *stream : activeStreams_) {
> - int ret = pipe_->allocateBuffers(this, stream);
> - if (ret) {
> - LOG(Camera, Error) << "Failed to allocate buffers";
> - freeBuffers();
> - return ret;
> - }
> + int ret = pipe_->allocateBuffers(this, activeStreams_);
> + if (ret) {
> + LOG(Camera, Error) << "Failed to allocate buffers";
> + freeBuffers();
I would squash patch 3/7 with this patch. Apart from that,
Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> + return ret;
> }
>
> state_ = CameraPrepared;
> @@ -683,12 +681,11 @@ int Camera::freeBuffers()
> * by the V4L2 device that has allocated them.
> */
> stream->bufferPool().destroyBuffers();
> - pipe_->freeBuffers(this, stream);
> }
>
> state_ = CameraConfigured;
>
> - return 0;
> + return pipe_->freeBuffers(this, activeStreams_);
> }
>
> /**
> diff --git a/src/libcamera/include/pipeline_handler.h b/src/libcamera/include/pipeline_handler.h
> index 33b820e706cc..a0862ebf35df 100644
> --- a/src/libcamera/include/pipeline_handler.h
> +++ b/src/libcamera/include/pipeline_handler.h
> @@ -58,8 +58,10 @@ public:
> streamConfiguration(Camera *camera, const std::vector<StreamUsage> &usages) = 0;
> virtual int configureStreams(Camera *camera, const CameraConfiguration &config) = 0;
>
> - virtual int allocateBuffers(Camera *camera, Stream *stream) = 0;
> - virtual int freeBuffers(Camera *camera, Stream *stream) = 0;
> + virtual int allocateBuffers(Camera *camera,
> + const std::set<Stream *> &streams) = 0;
> + virtual int freeBuffers(Camera *camera,
> + const std::set<Stream *> &streams) = 0;
>
> virtual int start(Camera *camera) = 0;
> virtual void stop(Camera *camera);
> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> index ca09da753b90..f96e8763bce9 100644
> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> @@ -145,8 +145,10 @@ public:
> int configureStreams(Camera *camera,
> const CameraConfiguration &config) override;
>
> - int allocateBuffers(Camera *camera, Stream *stream) override;
> - int freeBuffers(Camera *camera, Stream *stream) override;
> + int allocateBuffers(Camera *camera,
> + const std::set<Stream *> &streams) override;
> + int freeBuffers(Camera *camera,
> + const std::set<Stream *> &streams) override;
>
> int start(Camera *camera) override;
> void stop(Camera *camera) override;
> @@ -305,9 +307,11 @@ int PipelineHandlerIPU3::configureStreams(Camera *camera,
> return 0;
> }
>
> -int PipelineHandlerIPU3::allocateBuffers(Camera *camera, Stream *stream)
> +int PipelineHandlerIPU3::allocateBuffers(Camera *camera,
> + const std::set<Stream *> &streams)
> {
> IPU3CameraData *data = cameraData(camera);
> + Stream *stream = *streams.begin();
> CIO2Device *cio2 = &data->cio2_;
> ImgUDevice *imgu = data->imgu_;
> int ret;
> @@ -346,7 +350,8 @@ int PipelineHandlerIPU3::allocateBuffers(Camera *camera, Stream *stream)
> return 0;
> }
>
> -int PipelineHandlerIPU3::freeBuffers(Camera *camera, Stream *stream)
> +int PipelineHandlerIPU3::freeBuffers(Camera *camera,
> + const std::set<Stream *> &streams)
> {
> IPU3CameraData *data = cameraData(camera);
>
> diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp
> index cd472cfadd86..b8f634d88b46 100644
> --- a/src/libcamera/pipeline/uvcvideo.cpp
> +++ b/src/libcamera/pipeline/uvcvideo.cpp
> @@ -32,8 +32,10 @@ public:
> int configureStreams(Camera *camera,
> const CameraConfiguration &config) override;
>
> - int allocateBuffers(Camera *camera, Stream *stream) override;
> - int freeBuffers(Camera *camera, Stream *stream) override;
> + int allocateBuffers(Camera *camera,
> + const std::set<Stream *> &streams) override;
> + int freeBuffers(Camera *camera,
> + const std::set<Stream *> &streams) override;
>
> int start(Camera *camera) override;
> void stop(Camera *camera) override;
> @@ -127,9 +129,11 @@ int PipelineHandlerUVC::configureStreams(Camera *camera,
> return 0;
> }
>
> -int PipelineHandlerUVC::allocateBuffers(Camera *camera, Stream *stream)
> +int PipelineHandlerUVC::allocateBuffers(Camera *camera,
> + const std::set<Stream *> &streams)
> {
> UVCCameraData *data = cameraData(camera);
> + Stream *stream = *streams.begin();
> const StreamConfiguration &cfg = stream->configuration();
>
> LOG(UVC, Debug) << "Requesting " << cfg.bufferCount << " buffers";
> @@ -137,7 +141,8 @@ int PipelineHandlerUVC::allocateBuffers(Camera *camera, Stream *stream)
> return data->video_->exportBuffers(&stream->bufferPool());
> }
>
> -int PipelineHandlerUVC::freeBuffers(Camera *camera, Stream *stream)
> +int PipelineHandlerUVC::freeBuffers(Camera *camera,
> + const std::set<Stream *> &streams)
> {
> UVCCameraData *data = cameraData(camera);
> return data->video_->releaseBuffers();
> diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp
> index c8bbe2a19847..22449e47bc2d 100644
> --- a/src/libcamera/pipeline/vimc.cpp
> +++ b/src/libcamera/pipeline/vimc.cpp
> @@ -32,8 +32,10 @@ public:
> int configureStreams(Camera *camera,
> const CameraConfiguration &config) override;
>
> - int allocateBuffers(Camera *camera, Stream *stream) override;
> - int freeBuffers(Camera *camera, Stream *stream) override;
> + int allocateBuffers(Camera *camera,
> + const std::set<Stream *> &streams) override;
> + int freeBuffers(Camera *camera,
> + const std::set<Stream *> &streams) override;
>
> int start(Camera *camera) override;
> void stop(Camera *camera) override;
> @@ -127,9 +129,11 @@ int PipelineHandlerVimc::configureStreams(Camera *camera,
> return 0;
> }
>
> -int PipelineHandlerVimc::allocateBuffers(Camera *camera, Stream *stream)
> +int PipelineHandlerVimc::allocateBuffers(Camera *camera,
> + const std::set<Stream *> &streams)
> {
> VimcCameraData *data = cameraData(camera);
> + Stream *stream = *streams.begin();
> const StreamConfiguration &cfg = stream->configuration();
>
> LOG(VIMC, Debug) << "Requesting " << cfg.bufferCount << " buffers";
> @@ -137,7 +141,8 @@ int PipelineHandlerVimc::allocateBuffers(Camera *camera, Stream *stream)
> return data->video_->exportBuffers(&stream->bufferPool());
> }
>
> -int PipelineHandlerVimc::freeBuffers(Camera *camera, Stream *stream)
> +int PipelineHandlerVimc::freeBuffers(Camera *camera,
> + const std::set<Stream *> &streams)
> {
> VimcCameraData *data = cameraData(camera);
> return data->video_->releaseBuffers();
> diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp
> index 43550c0e0210..911d08448e69 100644
> --- a/src/libcamera/pipeline_handler.cpp
> +++ b/src/libcamera/pipeline_handler.cpp
> @@ -193,10 +193,11 @@ PipelineHandler::~PipelineHandler()
> * \fn PipelineHandler::allocateBuffers()
> * \brief Allocate buffers for a stream
> * \param[in] camera The camera the \a stream belongs to
> - * \param[in] stream The stream to allocate buffers for
> + * \param[in] streams The set of streams to allocate buffers for
> *
> - * This method allocates buffers internally in the pipeline handler and
> - * associates them with the stream's buffer pool.
> + * This method allocates buffers internally in the pipeline handler for each
> + * stream in the \a streams buffer set, and associates them with the stream's
> + * buffer pool.
> *
> * The intended caller of this method is the Camera class.
> *
> @@ -207,9 +208,9 @@ PipelineHandler::~PipelineHandler()
> * \fn PipelineHandler::freeBuffers()
> * \brief Free all buffers associated with a stream
> * \param[in] camera The camera the \a stream belongs to
> - * \param[in] stream The stream to free buffers from
> + * \param[in] streams The set of streams to free buffers from
> *
> - * After a capture session has been stopped all buffers associated with the
> + * After a capture session has been stopped all buffers associated with each
> * stream shall be freed.
> *
> * The intended caller of this method is the Camera class.
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list