[libcamera-devel] [PATCH 7/9] libcamera: pipeline_handler: Decouple buffer import and export
Niklas Söderlund
niklas.soderlund at ragnatech.se
Mon Mar 16 16:44:59 CET 2020
Hi Laurent,
Thanks for your work.
On 2020-03-15 01:57:26 +0200, Laurent Pinchart wrote:
> Use the V4L2 buffer orphaning feature, exposed through
> V4L2VideoDevice::exportBuffers(), to decouple buffer import and export.
> The PipelineHandler::importFrameBuffers() function is now called for all
> streams regardless of whether exportFrameBuffers() has been called or
> not. This simplifies the Camera implementation slightly, and opens the
> door to additional simplifications.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
Reviewed-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> ---
> include/libcamera/camera.h | 1 -
> src/libcamera/camera.cpp | 21 +-------------------
> src/libcamera/framebuffer_allocator.cpp | 9 ---------
> src/libcamera/pipeline/ipu3/ipu3.cpp | 2 +-
> src/libcamera/pipeline/rkisp1/rkisp1.cpp | 2 +-
> src/libcamera/pipeline/uvcvideo.cpp | 2 +-
> src/libcamera/pipeline/vimc.cpp | 2 +-
> src/libcamera/pipeline_handler.cpp | 25 +++++-------------------
> 8 files changed, 10 insertions(+), 54 deletions(-)
>
> diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h
> index c453b95228a7..a5e2b49f0f25 100644
> --- a/include/libcamera/camera.h
> +++ b/include/libcamera/camera.h
> @@ -113,7 +113,6 @@ private:
> friend class FrameBufferAllocator;
> int exportFrameBuffers(Stream *stream,
> std::vector<std::unique_ptr<FrameBuffer>> *buffers);
> - int freeFrameBuffers(Stream *stream);
> /* \todo Remove allocator_ from the exposed API */
> FrameBufferAllocator *allocator_;
> };
> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp
> index 9c432adb1d97..3192dfb42d01 100644
> --- a/src/libcamera/camera.cpp
> +++ b/src/libcamera/camera.cpp
> @@ -554,18 +554,6 @@ int Camera::exportFrameBuffers(Stream *stream,
> buffers);
> }
>
> -int Camera::freeFrameBuffers(Stream *stream)
> -{
> - int ret = p_->isAccessAllowed(Private::CameraConfigured, true);
> - if (ret < 0)
> - return ret;
> -
> - p_->pipe_->invokeMethod(&PipelineHandler::freeFrameBuffers,
> - ConnectionTypeBlocking, this, stream);
> -
> - return 0;
> -}
> -
> /**
> * \brief Acquire the camera device for exclusive access
> *
> @@ -928,9 +916,6 @@ int Camera::start()
> LOG(Camera, Debug) << "Starting capture";
>
> for (Stream *stream : p_->activeStreams_) {
> - if (allocator_ && !allocator_->buffers(stream).empty())
> - continue;
> -
> ret = p_->pipe_->invokeMethod(&PipelineHandler::importFrameBuffers,
> ConnectionTypeDirect, this, stream);
> if (ret < 0)
> @@ -974,13 +959,9 @@ int Camera::stop()
> p_->pipe_->invokeMethod(&PipelineHandler::stop, ConnectionTypeBlocking,
> this);
>
> - for (Stream *stream : p_->activeStreams_) {
> - if (allocator_ && !allocator_->buffers(stream).empty())
> - continue;
> -
> + for (Stream *stream : p_->activeStreams_)
> p_->pipe_->invokeMethod(&PipelineHandler::freeFrameBuffers,
> ConnectionTypeBlocking, this, stream);
> - }
>
> return 0;
> }
> diff --git a/src/libcamera/framebuffer_allocator.cpp b/src/libcamera/framebuffer_allocator.cpp
> index e79f4a8f65c8..6f7a2e90b08a 100644
> --- a/src/libcamera/framebuffer_allocator.cpp
> +++ b/src/libcamera/framebuffer_allocator.cpp
> @@ -87,11 +87,6 @@ FrameBufferAllocator::FrameBufferAllocator(std::shared_ptr<Camera> camera)
>
> FrameBufferAllocator::~FrameBufferAllocator()
> {
> - for (auto &value : buffers_) {
> - Stream *stream = value.first;
> - camera_->freeFrameBuffers(stream);
> - }
> -
> buffers_.clear();
>
> camera_->allocator_ = nullptr;
> @@ -148,10 +143,6 @@ int FrameBufferAllocator::free(Stream *stream)
> if (iter == buffers_.end())
> return -EINVAL;
>
> - int ret = camera_->freeFrameBuffers(stream);
> - if (ret < 0)
> - return ret;
> -
> std::vector<std::unique_ptr<FrameBuffer>> &buffers = iter->second;
> buffers.clear();
> buffers_.erase(iter);
> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> index 10a2698bad09..b6db8d567ea4 100644
> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> @@ -622,7 +622,7 @@ int PipelineHandlerIPU3::exportFrameBuffers(Camera *camera, Stream *stream,
> V4L2VideoDevice *video = ipu3stream->device_->dev;
> unsigned int count = stream->configuration().bufferCount;
>
> - return video->allocateBuffers(count, buffers);
> + return video->exportBuffers(count, buffers);
> }
>
> int PipelineHandlerIPU3::importFrameBuffers(Camera *camera, Stream *stream)
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> index f6934324c5a3..1ad53fbde112 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> @@ -665,7 +665,7 @@ int PipelineHandlerRkISP1::exportFrameBuffers(Camera *camera, Stream *stream,
> std::vector<std::unique_ptr<FrameBuffer>> *buffers)
> {
> unsigned int count = stream->configuration().bufferCount;
> - return video_->allocateBuffers(count, buffers);
> + return video_->exportBuffers(count, buffers);
> }
>
> int PipelineHandlerRkISP1::importFrameBuffers(Camera *camera, Stream *stream)
> diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp
> index d81627c224ea..29afb121aa46 100644
> --- a/src/libcamera/pipeline/uvcvideo.cpp
> +++ b/src/libcamera/pipeline/uvcvideo.cpp
> @@ -199,7 +199,7 @@ int PipelineHandlerUVC::exportFrameBuffers(Camera *camera, Stream *stream,
> UVCCameraData *data = cameraData(camera);
> unsigned int count = stream->configuration().bufferCount;
>
> - return data->video_->allocateBuffers(count, buffers);
> + return data->video_->exportBuffers(count, buffers);
> }
>
> int PipelineHandlerUVC::importFrameBuffers(Camera *camera, Stream *stream)
> diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp
> index bb94ef7fd38d..5d3d12fef30b 100644
> --- a/src/libcamera/pipeline/vimc.cpp
> +++ b/src/libcamera/pipeline/vimc.cpp
> @@ -265,7 +265,7 @@ int PipelineHandlerVimc::exportFrameBuffers(Camera *camera, Stream *stream,
> VimcCameraData *data = cameraData(camera);
> unsigned int count = stream->configuration().bufferCount;
>
> - return data->video_->allocateBuffers(count, buffers);
> + return data->video_->exportBuffers(count, buffers);
> }
>
> int PipelineHandlerVimc::importFrameBuffers(Camera *camera, Stream *stream)
> diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp
> index 8d623f5431fa..e5034c54e2fb 100644
> --- a/src/libcamera/pipeline_handler.cpp
> +++ b/src/libcamera/pipeline_handler.cpp
> @@ -337,16 +337,7 @@ const ControlList &PipelineHandler::properties(Camera *camera)
> *
> * 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.
> + * streams that are part of the active camera configuration.
> *
> * The only intended caller is Camera::exportFrameBuffers().
> *
> @@ -371,12 +362,7 @@ const ControlList &PipelineHandler::properties(Camera *camera)
> * 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.
> + * pipeline handler for the stream to prepare for starting the Camera.
> *
> * The only intended caller is Camera::start().
> *
> @@ -391,10 +377,9 @@ const ControlList &PipelineHandler::properties(Camera *camera)
> * \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.
> + * This method shall release all resources allocated for the \a stream by
> + * importFrameBuffers(). It shall be called only after a successful call that
> + * method, and only once per stream.
> *
> * The only intended callers are Camera::stop() and Camera::freeFrameBuffers().
> *
> --
> Regards,
>
> Laurent Pinchart
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
--
Regards,
Niklas Söderlund
More information about the libcamera-devel
mailing list