[libcamera-devel] [RFC 2/8] libcamera: stream: Provide accessors to buffers
Niklas Söderlund
niklas.soderlund at ragnatech.se
Tue Jul 2 00:36:14 CEST 2019
Hi Jacopo,
Thanks for your work.
On 2019-06-30 20:10:43 +0200, Jacopo Mondi wrote:
> All interactions with the Stream's buffers currently go through the
> BufferPool. In order to shorten accessing the buffers array, and
> restrict access to the Stream's internal buffer pool, provide operations
> to access the buffers, create and destriy them.
s/destriy/destroy/
>
> It is still possible to access the pool for pipeline handlers to
> populate it by exporting buffers from a video device to the pool.
>
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> ---
> include/libcamera/stream.h | 12 ++++++++++
> src/cam/capture.cpp | 4 ++--
> src/libcamera/camera.cpp | 6 ++---
> src/libcamera/pipeline/ipu3/ipu3.cpp | 4 ++--
> src/libcamera/stream.cpp | 35 +++++++++++++++++++++++-----
> src/qcam/main_window.cpp | 4 +---
> test/camera/capture.cpp | 3 +--
> test/camera/statemachine.cpp | 3 +--
> 8 files changed, 51 insertions(+), 20 deletions(-)
>
> diff --git a/include/libcamera/stream.h b/include/libcamera/stream.h
> index 5b4fea324ce4..fa7d6ba4987c 100644
> --- a/include/libcamera/stream.h
> +++ b/include/libcamera/stream.h
> @@ -66,12 +66,24 @@ class Stream
> {
> public:
> Stream();
> + /*
> + * FIXME:
> + * If I could find a way to export buffers in pipeline handlers
> + * without accessing the pool with
> + * video_->exportBuffers(&stream->bufferPool());
> + * we could remove access to the internal pool completely.
> + */
I see no real harm in exposing these functions but if you really wish to
hide them could you not do something like,
int importBuffers(V4L2VideoDevice *video)
{
return video->exportBuffers(&bufferPool());
}
> BufferPool &bufferPool() { return bufferPool_; }
> + std::vector<Buffer> &buffers() { return bufferPool_.buffers(); }
> + unsigned int bufferCount() const { return bufferPool_.count(); }
> const StreamConfiguration &configuration() const { return configuration_; }
>
> protected:
> friend class Camera;
>
> + void createBuffers(unsigned int count);
> + void destroyBuffers();
> +
> BufferPool bufferPool_;
> StreamConfiguration configuration_;
> };
> diff --git a/src/cam/capture.cpp b/src/cam/capture.cpp
> index 6b842d73390d..1bcc9c7e9cf4 100644
> --- a/src/cam/capture.cpp
> +++ b/src/cam/capture.cpp
> @@ -76,7 +76,7 @@ int Capture::capture(EventLoop *loop)
> unsigned int nbuffers = UINT_MAX;
> for (StreamConfiguration &cfg : *config_) {
> Stream *stream = cfg.stream();
> - nbuffers = std::min(nbuffers, stream->bufferPool().count());
> + nbuffers = std::min(nbuffers, stream->bufferCount());
> }
>
> /*
> @@ -95,7 +95,7 @@ int Capture::capture(EventLoop *loop)
> std::map<Stream *, Buffer *> map;
> for (StreamConfiguration &cfg : *config_) {
> Stream *stream = cfg.stream();
> - map[stream] = &stream->bufferPool().buffers()[i];
> + map[stream] = &stream->buffers()[i];
> }
>
> ret = request->setBuffers(map);
> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp
> index 617ea99cdf71..023ae53e5f9d 100644
> --- a/src/libcamera/camera.cpp
> +++ b/src/libcamera/camera.cpp
> @@ -671,7 +671,7 @@ int Camera::configure(CameraConfiguration *config)
> * Allocate buffer objects in the pool.
> * Memory will be allocated and assigned later.
> */
> - stream->bufferPool().createBuffers(cfg.bufferCount);
> + stream->createBuffers(cfg.bufferCount);
> }
>
> state_ = CameraConfigured;
> @@ -728,14 +728,14 @@ int Camera::freeBuffers()
> return -EACCES;
>
> for (Stream *stream : activeStreams_) {
> - if (!stream->bufferPool().count())
> + if (!stream->bufferCount())
> continue;
>
> /*
> * All mappings must be destroyed before buffers can be freed
> * by the V4L2 device that has allocated them.
> */
> - stream->bufferPool().destroyBuffers();
> + stream->destroyBuffers();
> }
>
> state_ = CameraConfigured;
> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> index e4efb9722f76..2de0892138a8 100644
> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> @@ -634,7 +634,7 @@ int PipelineHandlerIPU3::allocateBuffers(Camera *camera,
> * of buffers as the active ones.
> */
> if (!outStream->active_) {
> - bufferCount = vfStream->bufferPool().count();
> + bufferCount = vfStream->bufferCount();
> outStream->device_->pool->createBuffers(bufferCount);
> ret = imgu->exportBuffers(outStream->device_,
> outStream->device_->pool);
> @@ -643,7 +643,7 @@ int PipelineHandlerIPU3::allocateBuffers(Camera *camera,
> }
>
> if (!vfStream->active_) {
> - bufferCount = outStream->bufferPool().count();
> + bufferCount = outStream->bufferCount();
> vfStream->device_->pool->createBuffers(bufferCount);
> ret = imgu->exportBuffers(vfStream->device_,
> vfStream->device_->pool);
> diff --git a/src/libcamera/stream.cpp b/src/libcamera/stream.cpp
> index d8e87c62281c..c6701e5f9921 100644
> --- a/src/libcamera/stream.cpp
> +++ b/src/libcamera/stream.cpp
> @@ -408,14 +408,12 @@ Stream::Stream()
> }
>
> /**
> - * \fn Stream::bufferPool()
> - * \brief Retrieve the buffer pool for the stream
> + * \fn Stream::buffers()
> + * \brief Retrieve the stream's buffers
> *
> - * The buffer pool handles the buffers used to capture frames at the output of
> - * the stream. It is initially created empty and shall be populated with
> - * buffers before being used.
> + * \todo
> *
> - * \return A reference to the buffer pool
> + * \return The list of stream's buffers
> */
>
> /**
> @@ -424,6 +422,31 @@ Stream::Stream()
> * \return The active configuration of the stream
> */
>
> +/**
> + * \brief Create buffers for the stream
> + *
> + * \todo
> + */
> +void Stream::createBuffers(unsigned int count)
> +{
> + bufferPool_.destroyBuffers();
> +
> + if (count == 0)
> + return;
> +
> + bufferPool_.createBuffers(count);
> +}
> +
> +/**
> + * \brief Destroy buffers in the stream
> + *
> + * \todo
> + */
> +void Stream::destroyBuffers()
> +{
> + createBuffers(0);
> +}
> +
> /**
> * \var Stream::bufferPool_
> * \brief The pool of buffers associated with the stream
> diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp
> index 16b123132dd9..a0703b322c16 100644
> --- a/src/qcam/main_window.cpp
> +++ b/src/qcam/main_window.cpp
> @@ -122,10 +122,8 @@ int MainWindow::startCapture()
> return ret;
> }
>
> - BufferPool &pool = stream->bufferPool();
> std::vector<Request *> requests;
> -
> - for (Buffer &buffer : pool.buffers()) {
> + for (Buffer &buffer : stream->buffers()) {
> Request *request = camera_->createRequest();
> if (!request) {
> std::cerr << "Can't create request" << std::endl;
> diff --git a/test/camera/capture.cpp b/test/camera/capture.cpp
> index 7ce247cc482d..a0385ec2c74d 100644
> --- a/test/camera/capture.cpp
> +++ b/test/camera/capture.cpp
> @@ -76,9 +76,8 @@ protected:
> }
>
> Stream *stream = cfg.stream();
> - BufferPool &pool = stream->bufferPool();
> std::vector<Request *> requests;
> - for (Buffer &buffer : pool.buffers()) {
> + for (Buffer &buffer : stream->buffers()) {
> Request *request = camera_->createRequest();
> if (!request) {
> cout << "Failed to create request" << endl;
> diff --git a/test/camera/statemachine.cpp b/test/camera/statemachine.cpp
> index 84d2a6fab5f0..c23455b5bb21 100644
> --- a/test/camera/statemachine.cpp
> +++ b/test/camera/statemachine.cpp
> @@ -211,8 +211,7 @@ protected:
> return TestFail;
>
> Stream *stream = *camera_->streams().begin();
> - BufferPool &pool = stream->bufferPool();
> - Buffer &buffer = pool.buffers().front();
> + Buffer &buffer = stream->buffers().front();
> std::map<Stream *, Buffer *> map = { { stream, &buffer } };
> if (request->setBuffers(map))
> return TestFail;
> --
> 2.21.0
>
> _______________________________________________
> 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