[libcamera-devel] [PATCH v3 26/33] libcamera: Switch to FrameBuffer interface
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Sat Jan 11 02:49:43 CET 2020
Hi Niklas,
Thank you for the patch.
On Fri, Jan 10, 2020 at 08:38:01PM +0100, Niklas Söderlund wrote:
> Switch to the FrameBuffer interface where all buffers are treated as
> external buffers and are allocated outside the camera. Applications
> allocating buffers using libcamera are switched to use the
> FrameBufferAllocator helper.
>
> Follow-up changes to this one will finalize the transition to the new
> FrameBuffer interface by removing code that is left unused after this
> change.
>
> Signed-off-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> ---
> * Changes since v2
> - Fix compile tested only code path in the HAL
> - Update documentation
> - Add a setCookie() call in the V4L2 compat layer which was lost in
> Laurent's protoype patch which lead to the index handling in V4L2 to
> break.
> ---
> include/libcamera/camera.h | 4 +-
> include/libcamera/request.h | 14 +--
> src/android/camera_device.cpp | 31 ++++--
> src/cam/buffer_writer.cpp | 5 +-
> src/cam/buffer_writer.h | 3 +-
> src/cam/capture.cpp | 42 ++++----
> src/cam/capture.h | 5 +-
> src/libcamera/camera.cpp | 48 +++------
> src/libcamera/include/pipeline_handler.h | 5 +-
> src/libcamera/pipeline/ipu3/ipu3.cpp | 124 +++++++----------------
> src/libcamera/pipeline/rkisp1/rkisp1.cpp | 41 ++------
> src/libcamera/pipeline/uvcvideo.cpp | 22 ++--
> src/libcamera/pipeline/vimc.cpp | 22 ++--
> src/libcamera/pipeline_handler.cpp | 2 +-
> src/libcamera/request.cpp | 30 +++---
> src/qcam/main_window.cpp | 44 ++++----
> src/qcam/main_window.h | 6 +-
> src/v4l2/v4l2_camera.cpp | 66 +++++++-----
> src/v4l2/v4l2_camera.h | 39 +++----
> src/v4l2/v4l2_camera_proxy.cpp | 17 ++--
> test/camera/buffer_import.cpp | 19 ++--
> test/camera/capture.cpp | 38 ++++---
> test/camera/statemachine.cpp | 12 ++-
> 23 files changed, 273 insertions(+), 366 deletions(-)
>
> diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h
> index 2fd5870b15c69087..28655bec9ebc89ce 100644
> --- a/include/libcamera/camera.h
> +++ b/include/libcamera/camera.h
> @@ -19,7 +19,7 @@
>
> namespace libcamera {
>
> -class Buffer;
> +class FrameBuffer;
> class FrameBufferAllocator;
> class PipelineHandler;
> class Request;
> @@ -78,7 +78,7 @@ public:
>
> const std::string &name() const;
>
> - Signal<Request *, Buffer *> bufferCompleted;
> + Signal<Request *, FrameBuffer *> bufferCompleted;
> Signal<Request *> requestCompleted;
> Signal<Camera *> disconnected;
>
> diff --git a/include/libcamera/request.h b/include/libcamera/request.h
> index a8708010ec1247a2..02c0ef5f6b028bd4 100644
> --- a/include/libcamera/request.h
> +++ b/include/libcamera/request.h
> @@ -17,9 +17,9 @@
>
> namespace libcamera {
>
> -class Buffer;
> class Camera;
> class CameraControlValidator;
> +class FrameBuffer;
> class Stream;
>
> class Request
> @@ -38,9 +38,9 @@ public:
>
> ControlList &controls() { return *controls_; }
> ControlList &metadata() { return *metadata_; }
> - const std::map<Stream *, Buffer *> &buffers() const { return bufferMap_; }
> - int addBuffer(Stream *stream, std::unique_ptr<Buffer> buffer);
> - Buffer *findBuffer(Stream *stream) const;
> + const std::map<Stream *, FrameBuffer *> &buffers() const { return bufferMap_; }
> + int addBuffer(Stream *stream, FrameBuffer *buffer);
> + FrameBuffer *findBuffer(Stream *stream) const;
>
> uint64_t cookie() const { return cookie_; }
> Status status() const { return status_; }
> @@ -54,14 +54,14 @@ private:
> int prepare();
> void complete();
>
> - bool completeBuffer(Buffer *buffer);
> + bool completeBuffer(FrameBuffer *buffer);
>
> Camera *camera_;
> CameraControlValidator *validator_;
> ControlList *controls_;
> ControlList *metadata_;
> - std::map<Stream *, Buffer *> bufferMap_;
> - std::unordered_set<Buffer *> pending_;
> + std::map<Stream *, FrameBuffer *> bufferMap_;
> + std::unordered_set<FrameBuffer *> pending_;
>
> const uint64_t cookie_;
> Status status_;
> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> index ebe91ea8af4f6436..ab3e44889f8b7ada 100644
> --- a/src/android/camera_device.cpp
> +++ b/src/android/camera_device.cpp
> @@ -739,13 +739,21 @@ void CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reque
> * and (currently) only supported request buffer.
> */
> const buffer_handle_t camera3Handle = *camera3Buffers[0].buffer;
> - std::array<int, 3> fds = {
> - camera3Handle->data[0],
> - camera3Handle->data[1],
> - camera3Handle->data[2],
> - };
>
> - std::unique_ptr<Buffer> buffer = stream->createBuffer(fds);
> + std::vector<FrameBuffer::Plane> planes;
> + for (int i = 0; i < 3; i++) {
> + FrameBuffer::Plane plane;
> + plane.fd = FileDescriptor(camera3Handle->data[i]);
> + /*
> + * Setting length to zero here is OK as the length is only used
> + * to map the memory of the plane. Libcamera do not need to poke
> + * at the memory content queued by the HAL.
> + */
> + plane.length = 0;
> + planes.push_back(std::move(plane));
> + }
> +
> + FrameBuffer *buffer = new FrameBuffer(std::move(planes));
> if (!buffer) {
> LOG(HAL, Error) << "Failed to create buffer";
> delete descriptor;
> @@ -754,7 +762,7 @@ void CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reque
>
> Request *request =
> camera_->createRequest(reinterpret_cast<uint64_t>(descriptor));
> - request->addBuffer(stream, std::move(buffer));
> + request->addBuffer(stream, buffer);
>
> int ret = camera_->queueRequest(request);
> if (ret) {
> @@ -771,8 +779,8 @@ error:
>
> void CameraDevice::requestComplete(Request *request)
> {
> - const std::map<Stream *, Buffer *> &buffers = request->buffers();
> - Buffer *libcameraBuffer = buffers.begin()->second;
> + const std::map<Stream *, FrameBuffer *> &buffers = request->buffers();
> + FrameBuffer *buffer = buffers.begin()->second;
> camera3_buffer_status status = CAMERA3_BUFFER_STATUS_OK;
> std::unique_ptr<CameraMetadata> resultMetadata;
>
> @@ -803,11 +811,11 @@ void CameraDevice::requestComplete(Request *request)
>
> if (status == CAMERA3_BUFFER_STATUS_OK) {
> notifyShutter(descriptor->frameNumber,
> - libcameraBuffer->metadata().timestamp);
> + buffer->metadata().timestamp);
>
> captureResult.partial_result = 1;
> resultMetadata = getResultMetadata(descriptor->frameNumber,
> - libcameraBuffer->metadata().timestamp);
> + buffer->metadata().timestamp);
> captureResult.result = resultMetadata->get();
> }
>
> @@ -825,6 +833,7 @@ void CameraDevice::requestComplete(Request *request)
> callbacks_->process_capture_result(callbacks_, &captureResult);
>
> delete descriptor;
> + delete buffer;
> }
>
> void CameraDevice::notifyShutter(uint32_t frameNumber, uint64_t timestamp)
> diff --git a/src/cam/buffer_writer.cpp b/src/cam/buffer_writer.cpp
> index 41ef4b0a36d61b03..1d7366c87714cd91 100644
> --- a/src/cam/buffer_writer.cpp
> +++ b/src/cam/buffer_writer.cpp
> @@ -22,7 +22,7 @@ BufferWriter::BufferWriter(const std::string &pattern)
> {
> }
>
> -int BufferWriter::write(Buffer *buffer, const std::string &streamName)
> +int BufferWriter::write(FrameBuffer *buffer, const std::string &streamName)
> {
> std::string filename;
> size_t pos;
> @@ -43,8 +43,7 @@ int BufferWriter::write(Buffer *buffer, const std::string &streamName)
> if (fd == -1)
> return -errno;
>
> - BufferMemory *mem = buffer->mem();
> - for (const FrameBuffer::Plane &plane : mem->planes()) {
> + for (const FrameBuffer::Plane &plane : buffer->planes()) {
> /* \todo Once the FrameBuffer is done cache mapped memory. */
> void *data = mmap(NULL, plane.length, PROT_READ, MAP_SHARED,
> plane.fd.fd(), 0);
> diff --git a/src/cam/buffer_writer.h b/src/cam/buffer_writer.h
> index 7bf785d1e83235ff..5917a7dfb5e28106 100644
> --- a/src/cam/buffer_writer.h
> +++ b/src/cam/buffer_writer.h
> @@ -16,7 +16,8 @@ class BufferWriter
> public:
> BufferWriter(const std::string &pattern = "frame-#.bin");
>
> - int write(libcamera::Buffer *buffer, const std::string &streamName);
> + int write(libcamera::FrameBuffer *buffer,
> + const std::string &streamName);
>
> private:
> std::string pattern_;
> diff --git a/src/cam/capture.cpp b/src/cam/capture.cpp
> index da942f56118983bd..dd078eb0ae4a2c62 100644
> --- a/src/cam/capture.cpp
> +++ b/src/cam/capture.cpp
> @@ -57,7 +57,10 @@ int Capture::run(EventLoop *loop, const OptionsParser::Options &options)
> writer_ = new BufferWriter();
> }
>
> - ret = capture(loop);
> +
> + FrameBufferAllocator *allocator = FrameBufferAllocator::create(camera_);
> +
> + ret = capture(loop, allocator);
>
> if (options.isSet(OptFile)) {
> delete writer_;
> @@ -66,17 +69,27 @@ int Capture::run(EventLoop *loop, const OptionsParser::Options &options)
>
> camera_->freeBuffers();
>
> + delete allocator;
> +
> return ret;
> }
>
> -int Capture::capture(EventLoop *loop)
> +int Capture::capture(EventLoop *loop, FrameBufferAllocator *allocator)
> {
> int ret;
>
> /* Identify the stream with the least number of buffers. */
> unsigned int nbuffers = UINT_MAX;
> - for (StreamConfiguration &cfg : *config_)
> - nbuffers = std::min(nbuffers, cfg.bufferCount);
> + for (StreamConfiguration &cfg : *config_) {
> + ret = allocator->allocate(cfg.stream());
> + if (ret < 0) {
> + std::cerr << "Can't allocate buffers" << std::endl;
> + return -ENOMEM;
> + }
> +
> + unsigned int allocated = allocator->buffers(cfg.stream()).size();
> + nbuffers = std::min(nbuffers, allocated);
> + }
>
> /*
> * TODO: make cam tool smarter to support still capture by for
> @@ -93,9 +106,11 @@ int Capture::capture(EventLoop *loop)
>
> for (StreamConfiguration &cfg : *config_) {
> Stream *stream = cfg.stream();
> - std::unique_ptr<Buffer> buffer = stream->createBuffer(i);
> + const std::vector<std::unique_ptr<FrameBuffer>> &buffers =
> + allocator->buffers(stream);
> + const std::unique_ptr<FrameBuffer> &buffer = buffers[i];
>
> - ret = request->addBuffer(stream, std::move(buffer));
> + ret = request->addBuffer(stream, buffer.get());
> if (ret < 0) {
> std::cerr << "Can't set buffer for request"
> << std::endl;
> @@ -138,7 +153,7 @@ void Capture::requestComplete(Request *request)
> if (request->status() == Request::RequestCancelled)
> return;
>
> - const std::map<Stream *, Buffer *> &buffers = request->buffers();
> + const std::map<Stream *, FrameBuffer *> &buffers = request->buffers();
>
> std::chrono::steady_clock::time_point now = std::chrono::steady_clock::now();
> double fps = std::chrono::duration_cast<std::chrono::milliseconds>(now - last_).count();
> @@ -151,7 +166,7 @@ void Capture::requestComplete(Request *request)
>
> for (auto it = buffers.begin(); it != buffers.end(); ++it) {
> Stream *stream = it->first;
> - Buffer *buffer = it->second;
> + FrameBuffer *buffer = it->second;
> const std::string &name = streamName_[stream];
>
> const FrameMetadata &metadata = buffer->metadata();
> @@ -185,16 +200,9 @@ void Capture::requestComplete(Request *request)
>
> for (auto it = buffers.begin(); it != buffers.end(); ++it) {
> Stream *stream = it->first;
> - Buffer *buffer = it->second;
> - unsigned int index = buffer->index();
> -
> - std::unique_ptr<Buffer> newBuffer = stream->createBuffer(index);
> - if (!newBuffer) {
> - std::cerr << "Can't create buffer" << std::endl;
> - return;
> - }
> + FrameBuffer *buffer = it->second;
>
> - request->addBuffer(stream, std::move(newBuffer));
> + request->addBuffer(stream, buffer);
> }
>
> camera_->queueRequest(request);
> diff --git a/src/cam/capture.h b/src/cam/capture.h
> index c692d48918f2de1d..9bca5661070efcf5 100644
> --- a/src/cam/capture.h
> +++ b/src/cam/capture.h
> @@ -10,7 +10,9 @@
> #include <chrono>
> #include <memory>
>
> +#include <libcamera/buffer.h>
> #include <libcamera/camera.h>
> +#include <libcamera/framebuffer_allocator.h>
> #include <libcamera/request.h>
> #include <libcamera/stream.h>
>
> @@ -26,7 +28,8 @@ public:
>
> int run(EventLoop *loop, const OptionsParser::Options &options);
> private:
> - int capture(EventLoop *loop);
> + int capture(EventLoop *loop,
> + libcamera::FrameBufferAllocator *allocator);
>
> void requestComplete(libcamera::Request *request);
>
> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp
> index 91f5ef7771782eba..963b78d87f8eba22 100644
> --- a/src/libcamera/camera.cpp
> +++ b/src/libcamera/camera.cpp
> @@ -695,12 +695,6 @@ int Camera::configure(CameraConfiguration *config)
>
> stream->configuration_ = cfg;
> activeStreams_.insert(stream);
> -
> - /*
> - * Allocate buffer objects in the pool.
> - * Memory will be allocated and assigned later.
> - */
> - stream->createBuffers(cfg.memoryType, cfg.bufferCount);
> }
>
> state_ = CameraConfigured;
> @@ -756,14 +750,6 @@ int Camera::freeBuffers()
> if (!stateIs(CameraPrepared))
> return -EACCES;
>
> - for (Stream *stream : activeStreams_) {
> - /*
> - * All mappings must be destroyed before buffers can be freed
> - * by the V4L2 device that has allocated them.
> - */
> - stream->destroyBuffers();
> - }
> -
> state_ = CameraConfigured;
>
> return pipe_->freeBuffers(this, activeStreams_);
> @@ -829,24 +815,11 @@ int Camera::queueRequest(Request *request)
>
> for (auto const &it : request->buffers()) {
> Stream *stream = it.first;
> - Buffer *buffer = it.second;
>
> if (activeStreams_.find(stream) == activeStreams_.end()) {
> LOG(Camera, Error) << "Invalid request";
> return -EINVAL;
> }
> -
> - if (stream->memoryType() == ExternalMemory) {
> - int index = stream->mapBuffer(buffer);
> - if (index < 0) {
> - LOG(Camera, Error) << "No buffer memory available";
> - return -ENOMEM;
> - }
> -
> - buffer->index_ = index;
> - }
> -
> - buffer->mem_ = &stream->buffers()[buffer->index_];
> }
>
> int ret = request->prepare();
> @@ -881,6 +854,13 @@ int Camera::start()
>
> LOG(Camera, Debug) << "Starting capture";
>
> + for (Stream *stream : activeStreams_) {
> + if (allocator_ && !allocator_->buffers(stream).empty())
> + continue;
> +
> + pipe_->importFrameBuffers(this, stream);
> + }
> +
> int ret = pipe_->start(this);
> if (ret)
> return ret;
> @@ -916,6 +896,13 @@ int Camera::stop()
>
> pipe_->stop(this);
>
> + for (Stream *stream : activeStreams_) {
> + if (allocator_ && !allocator_->buffers(stream).empty())
> + continue;
> +
> + pipe_->freeFrameBuffers(this, stream);
> + }
> +
> return 0;
> }
>
> @@ -929,13 +916,6 @@ int Camera::stop()
> */
> void Camera::requestComplete(Request *request)
> {
> - for (auto it : request->buffers()) {
> - Stream *stream = it.first;
> - Buffer *buffer = it.second;
> - if (stream->memoryType() == ExternalMemory)
> - stream->unmapBuffer(buffer);
> - }
> -
> requestCompleted.emit(request);
> delete request;
> }
> diff --git a/src/libcamera/include/pipeline_handler.h b/src/libcamera/include/pipeline_handler.h
> index 560be3bad8d59b20..27f3852c6c87843f 100644
> --- a/src/libcamera/include/pipeline_handler.h
> +++ b/src/libcamera/include/pipeline_handler.h
> @@ -21,12 +21,12 @@
>
> namespace libcamera {
>
> -class Buffer;
> class Camera;
> class CameraConfiguration;
> class CameraManager;
> class DeviceEnumerator;
> class DeviceMatch;
> +class FrameBuffer;
> class MediaDevice;
> class PipelineHandler;
> class Request;
> @@ -85,7 +85,8 @@ public:
>
> int queueRequest(Camera *camera, Request *request);
>
> - bool completeBuffer(Camera *camera, Request *request, Buffer *buffer);
> + bool completeBuffer(Camera *camera, Request *request,
> + FrameBuffer *buffer);
> void completeRequest(Camera *camera, Request *request);
>
> const char *name() const { return name_; }
> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> index b73f0f0725ee2613..065f5d980b68e1cf 100644
> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> @@ -31,6 +31,8 @@ namespace libcamera {
>
> LOG_DEFINE_CATEGORY(IPU3)
>
> +class IPU3CameraData;
> +
> class ImgUDevice
> {
> public:
> @@ -44,7 +46,6 @@ public:
> V4L2VideoDevice *dev;
> unsigned int pad;
> std::string name;
> - BufferPool *pool;
> std::vector<std::unique_ptr<FrameBuffer>> buffers;
> };
>
> @@ -71,9 +72,7 @@ public:
> int configureOutput(ImgUOutput *output,
> const StreamConfiguration &cfg);
>
> - int importOutputBuffers(ImgUOutput *output, BufferPool *pool);
> - int exportOutputBuffers(ImgUOutput *output, BufferPool *pool);
> - void freeBuffers();
> + void freeBuffers(IPU3CameraData *data);
>
> int start();
> int stop();
> @@ -93,9 +92,6 @@ public:
> ImgUOutput viewfinder_;
> ImgUOutput stat_;
> /* \todo Add param video device for 3A tuning */
> -
> - BufferPool vfPool_;
> - BufferPool outPool_;
> };
>
> class CIO2Device
> @@ -156,7 +152,7 @@ public:
> {
> }
>
> - void imguOutputBufferReady(Buffer *buffer);
> + void imguOutputBufferReady(FrameBuffer *buffer);
> void imguInputBufferReady(FrameBuffer *buffer);
> void cio2BufferReady(FrameBuffer *buffer);
>
> @@ -691,39 +687,30 @@ int PipelineHandlerIPU3::allocateBuffers(Camera *camera,
> goto error;
> }
>
> - /* Allocate buffers for each active stream. */
> - for (Stream *s : streams) {
> - IPU3Stream *stream = static_cast<IPU3Stream *>(s);
> - ImgUDevice::ImgUOutput *dev = stream->device_;
> -
> - if (stream->memoryType() == InternalMemory)
> - ret = imgu->exportOutputBuffers(dev, &stream->bufferPool());
> - else
> - ret = imgu->importOutputBuffers(dev, &stream->bufferPool());
> - if (ret)
> - goto error;
> - }
> -
> /*
> * Allocate buffers also on non-active outputs; use the same number
> * of buffers as the active ones.
> */
> if (!outStream->active_) {
> - bufferCount = vfStream->configuration().bufferCount;
> - outStream->device_->pool->createBuffers(bufferCount);
> - ret = imgu->exportOutputBuffers(outStream->device_,
> - outStream->device_->pool);
> - if (ret)
> + ImgUDevice::ImgUOutput *output = outStream->device_;
> +
> + ret = output->dev->exportBuffers(bufferCount, &output->buffers);
> + if (ret < 0) {
> + LOG(IPU3, Error) << "Failed to allocate ImgU "
> + << output->name << " buffers";
> goto error;
> + }
> }
>
> if (!vfStream->active_) {
> - bufferCount = outStream->configuration().bufferCount;
> - vfStream->device_->pool->createBuffers(bufferCount);
> - ret = imgu->exportOutputBuffers(vfStream->device_,
> - vfStream->device_->pool);
> - if (ret)
> + ImgUDevice::ImgUOutput *output = vfStream->device_;
> +
> + ret = output->dev->exportBuffers(bufferCount, &output->buffers);
> + if (ret < 0) {
> + LOG(IPU3, Error) << "Failed to allocate ImgU "
> + << output->name << " buffers";
> goto error;
> + }
> }
>
> return 0;
> @@ -740,7 +727,7 @@ int PipelineHandlerIPU3::freeBuffers(Camera *camera,
> IPU3CameraData *data = cameraData(camera);
>
> data->cio2_.freeBuffers();
> - data->imgu_->freeBuffers();
> + data->imgu_->freeBuffers(data);
>
> return 0;
> }
> @@ -793,7 +780,7 @@ int PipelineHandlerIPU3::queueRequestDevice(Camera *camera, Request *request)
>
> for (auto it : request->buffers()) {
> IPU3Stream *stream = static_cast<IPU3Stream *>(it.first);
> - Buffer *buffer = it.second;
> + FrameBuffer *buffer = it.second;
>
> int ret = stream->device_->dev->queueBuffer(buffer);
> if (ret < 0)
> @@ -920,9 +907,9 @@ int PipelineHandlerIPU3::registerCameras()
> &IPU3CameraData::cio2BufferReady);
> data->imgu_->input_->frameBufferReady.connect(data.get(),
> &IPU3CameraData::imguInputBufferReady);
> - data->imgu_->output_.dev->bufferReady.connect(data.get(),
> + data->imgu_->output_.dev->frameBufferReady.connect(data.get(),
> &IPU3CameraData::imguOutputBufferReady);
> - data->imgu_->viewfinder_.dev->bufferReady.connect(data.get(),
> + data->imgu_->viewfinder_.dev->frameBufferReady.connect(data.get(),
> &IPU3CameraData::imguOutputBufferReady);
>
> /* Create and register the Camera instance. */
> @@ -971,7 +958,7 @@ void IPU3CameraData::imguInputBufferReady(FrameBuffer *buffer)
> *
> * Buffers completed from the ImgU output are directed to the application.
> */
> -void IPU3CameraData::imguOutputBufferReady(Buffer *buffer)
> +void IPU3CameraData::imguOutputBufferReady(FrameBuffer *buffer)
> {
> Request *request = buffer->request();
>
> @@ -1046,7 +1033,6 @@ int ImgUDevice::init(MediaDevice *media, unsigned int index)
>
> output_.pad = PAD_OUTPUT;
> output_.name = "output";
> - output_.pool = &outPool_;
>
> viewfinder_.dev = V4L2VideoDevice::fromEntityName(media,
> name_ + " viewfinder");
> @@ -1056,7 +1042,6 @@ int ImgUDevice::init(MediaDevice *media, unsigned int index)
>
> viewfinder_.pad = PAD_VF;
> viewfinder_.name = "viewfinder";
> - viewfinder_.pool = &vfPool_;
>
> stat_.dev = V4L2VideoDevice::fromEntityName(media, name_ + " 3a stat");
> ret = stat_.dev->open();
> @@ -1164,69 +1149,28 @@ int ImgUDevice::configureOutput(ImgUOutput *output,
> return 0;
> }
>
> -/**
> - * \brief Export buffers from \a output to the provided \a pool
> - * \param[in] output The ImgU output device
> - * \param[in] pool The buffer pool where to export buffers
> - *
> - * Export memory buffers reserved in the video device memory associated with
> - * \a output id to the buffer pool provided as argument.
> - *
> - * \return 0 on success or a negative error code otherwise
> - */
> -int ImgUDevice::exportOutputBuffers(ImgUOutput *output, BufferPool *pool)
> -{
> - int ret = output->dev->exportBuffers(pool);
> - if (ret) {
> - LOG(IPU3, Error) << "Failed to export ImgU "
> - << output->name << " buffers";
> - return ret;
> - }
> -
> - return 0;
> -}
> -
> -/**
> - * \brief Reserve buffers in \a output from the provided \a pool
> - * \param[in] output The ImgU output device
> - * \param[in] pool The buffer pool used to reserve buffers in \a output
> - *
> - * Reserve a number of buffers equal to the number of buffers in \a pool
> - * in the \a output device.
> - *
> - * \return 0 on success or a negative error code otherwise
> - */
> -int ImgUDevice::importOutputBuffers(ImgUOutput *output, BufferPool *pool)
> -{
> - int ret = output->dev->importBuffers(pool);
> - if (ret) {
> - LOG(IPU3, Error)
> - << "Failed to import buffer in " << output->name
> - << " ImgU device";
> - return ret;
> - }
> -
> - return 0;
> -}
> -
> /**
> * \brief Release buffers for all the ImgU video devices
> */
> -void ImgUDevice::freeBuffers()
> +void ImgUDevice::freeBuffers(IPU3CameraData *data)
> {
> int ret;
>
> - ret = output_.dev->releaseBuffers();
> - if (ret)
> - LOG(IPU3, Error) << "Failed to release ImgU output buffers";
> + if (!data->outStream_.active_) {
> + ret = output_.dev->releaseBuffers();
> + if (ret)
> + LOG(IPU3, Error) << "Failed to release ImgU output buffers";
> + }
>
> ret = stat_.dev->releaseBuffers();
> if (ret)
> LOG(IPU3, Error) << "Failed to release ImgU stat buffers";
>
> - ret = viewfinder_.dev->releaseBuffers();
> - if (ret)
> - LOG(IPU3, Error) << "Failed to release ImgU viewfinder buffers";
> + if (!data->vfStream_.active_) {
> + ret = viewfinder_.dev->releaseBuffers();
> + if (ret)
> + LOG(IPU3, Error) << "Failed to release ImgU viewfinder buffers";
> + }
>
> ret = input_->releaseBuffers();
> if (ret)
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> index ba8f93e8584c97a9..d669d2ded89607d2 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> @@ -51,7 +51,7 @@ struct RkISP1FrameInfo {
>
> FrameBuffer *paramBuffer;
> FrameBuffer *statBuffer;
> - Buffer *videoBuffer;
> + FrameBuffer *videoBuffer;
>
> bool paramFilled;
> bool paramDequeued;
> @@ -67,7 +67,6 @@ public:
> int destroy(unsigned int frame);
>
> RkISP1FrameInfo *find(unsigned int frame);
> - RkISP1FrameInfo *find(Buffer *buffer);
> RkISP1FrameInfo *find(FrameBuffer *buffer);
> RkISP1FrameInfo *find(Request *request);
>
> @@ -87,7 +86,7 @@ public:
> setDelay(QueueBuffers, -1, 10);
> }
>
> - void bufferReady(Buffer *buffer)
> + void bufferReady(FrameBuffer *buffer)
> {
> /*
> * Calculate SOE by taking the end of DMA set by the kernel and applying
> @@ -205,7 +204,7 @@ private:
> int initLinks();
> int createCamera(MediaEntity *sensor);
> void tryCompleteRequest(Request *request);
> - void bufferReady(Buffer *buffer);
> + void bufferReady(FrameBuffer *buffer);
> void paramReady(FrameBuffer *buffer);
> void statReady(FrameBuffer *buffer);
>
> @@ -243,7 +242,7 @@ RkISP1FrameInfo *RkISP1Frames::create(unsigned int frame, Request *request, Stre
> }
> FrameBuffer *statBuffer = pipe_->availableStatBuffers_.front();
>
> - Buffer *videoBuffer = request->findBuffer(stream);
> + FrameBuffer *videoBuffer = request->findBuffer(stream);
> if (!videoBuffer) {
> LOG(RkISP1, Error)
> << "Attempt to queue request with invalid stream";
> @@ -296,26 +295,14 @@ RkISP1FrameInfo *RkISP1Frames::find(unsigned int frame)
> return nullptr;
> }
>
> -RkISP1FrameInfo *RkISP1Frames::find(Buffer *buffer)
> -{
> - for (auto &itInfo : frameInfo_) {
> - RkISP1FrameInfo *info = itInfo.second;
> -
> - if (info->videoBuffer == buffer)
> - return info;
> - }
> -
> - LOG(RkISP1, Error) << "Can't locate info from buffer";
> - return nullptr;
> -}
> -
> RkISP1FrameInfo *RkISP1Frames::find(FrameBuffer *buffer)
> {
> for (auto &itInfo : frameInfo_) {
> RkISP1FrameInfo *info = itInfo.second;
>
> if (info->paramBuffer == buffer ||
> - info->statBuffer == buffer)
> + info->statBuffer == buffer ||
> + info->videoBuffer == buffer)
> return info;
> }
>
> @@ -692,18 +679,9 @@ int PipelineHandlerRkISP1::allocateBuffers(Camera *camera,
> const std::set<Stream *> &streams)
> {
> RkISP1CameraData *data = cameraData(camera);
> - Stream *stream = *streams.begin();
> unsigned int count = 1;
> int ret;
>
> - if (stream->memoryType() == InternalMemory)
> - ret = video_->exportBuffers(&stream->bufferPool());
> - else
> - ret = video_->importBuffers(&stream->bufferPool());
> -
> - if (ret)
> - return ret;
> -
> unsigned int maxBuffers = 0;
> for (const Stream *s : camera->streams())
> maxBuffers = std::max(maxBuffers, s->configuration().bufferCount);
> @@ -769,9 +747,6 @@ int PipelineHandlerRkISP1::freeBuffers(Camera *camera,
> if (stat_->releaseBuffers())
> LOG(RkISP1, Error) << "Failed to release stat buffers";
>
> - if (video_->releaseBuffers())
> - LOG(RkISP1, Error) << "Failed to release video buffers";
> -
> return 0;
> }
>
> @@ -975,7 +950,7 @@ bool PipelineHandlerRkISP1::match(DeviceEnumerator *enumerator)
> if (param_->open() < 0)
> return false;
>
> - video_->bufferReady.connect(this, &PipelineHandlerRkISP1::bufferReady);
> + video_->frameBufferReady.connect(this, &PipelineHandlerRkISP1::bufferReady);
> stat_->frameBufferReady.connect(this, &PipelineHandlerRkISP1::statReady);
> param_->frameBufferReady.connect(this, &PipelineHandlerRkISP1::paramReady);
>
> @@ -1024,7 +999,7 @@ void PipelineHandlerRkISP1::tryCompleteRequest(Request *request)
> completeRequest(activeCamera_, request);
> }
>
> -void PipelineHandlerRkISP1::bufferReady(Buffer *buffer)
> +void PipelineHandlerRkISP1::bufferReady(FrameBuffer *buffer)
> {
> ASSERT(activeCamera_);
> RkISP1CameraData *data = cameraData(activeCamera_);
> diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp
> index b72841edee572f99..66380e29e1a78e13 100644
> --- a/src/libcamera/pipeline/uvcvideo.cpp
> +++ b/src/libcamera/pipeline/uvcvideo.cpp
> @@ -42,7 +42,7 @@ public:
> }
>
> int init(MediaEntity *entity);
> - void bufferReady(Buffer *buffer);
> + void bufferReady(FrameBuffer *buffer);
>
> V4L2VideoDevice *video_;
> Stream stream_;
> @@ -225,23 +225,13 @@ void PipelineHandlerUVC::freeFrameBuffers(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";
> -
> - if (stream->memoryType() == InternalMemory)
> - return data->video_->exportBuffers(&stream->bufferPool());
> - else
> - return data->video_->importBuffers(&stream->bufferPool());
> + return 0;
> }
>
> int PipelineHandlerUVC::freeBuffers(Camera *camera,
> const std::set<Stream *> &streams)
> {
> - UVCCameraData *data = cameraData(camera);
> - return data->video_->releaseBuffers();
> + return 0;
> }
>
> int PipelineHandlerUVC::start(Camera *camera)
> @@ -295,7 +285,7 @@ int PipelineHandlerUVC::processControls(UVCCameraData *data, Request *request)
> int PipelineHandlerUVC::queueRequestDevice(Camera *camera, Request *request)
> {
> UVCCameraData *data = cameraData(camera);
> - Buffer *buffer = request->findBuffer(&data->stream_);
> + FrameBuffer *buffer = request->findBuffer(&data->stream_);
> if (!buffer) {
> LOG(UVC, Error)
> << "Attempt to queue request with invalid stream";
> @@ -362,7 +352,7 @@ int UVCCameraData::init(MediaEntity *entity)
> if (ret)
> return ret;
>
> - video_->bufferReady.connect(this, &UVCCameraData::bufferReady);
> + video_->frameBufferReady.connect(this, &UVCCameraData::bufferReady);
>
> /* Initialise the supported controls. */
> const ControlInfoMap &controls = video_->controls();
> @@ -402,7 +392,7 @@ int UVCCameraData::init(MediaEntity *entity)
> return 0;
> }
>
> -void UVCCameraData::bufferReady(Buffer *buffer)
> +void UVCCameraData::bufferReady(FrameBuffer *buffer)
> {
> Request *request = buffer->request();
>
> diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp
> index 3fb5cacde0dab5b6..5e0f5c63b7fbe86a 100644
> --- a/src/libcamera/pipeline/vimc.cpp
> +++ b/src/libcamera/pipeline/vimc.cpp
> @@ -55,7 +55,7 @@ public:
> }
>
> int init(MediaDevice *media);
> - void bufferReady(Buffer *buffer);
> + void bufferReady(FrameBuffer *buffer);
>
> CameraSensor *sensor_;
> V4L2Subdevice *debayer_;
> @@ -291,23 +291,13 @@ void PipelineHandlerVimc::freeFrameBuffers(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";
> -
> - if (stream->memoryType() == InternalMemory)
> - return data->video_->exportBuffers(&stream->bufferPool());
> - else
> - return data->video_->importBuffers(&stream->bufferPool());
> + return 0;
> }
>
> int PipelineHandlerVimc::freeBuffers(Camera *camera,
> const std::set<Stream *> &streams)
> {
> - VimcCameraData *data = cameraData(camera);
> - return data->video_->releaseBuffers();
> + return 0;
> }
>
> int PipelineHandlerVimc::start(Camera *camera)
> @@ -355,7 +345,7 @@ int PipelineHandlerVimc::processControls(VimcCameraData *data, Request *request)
> int PipelineHandlerVimc::queueRequestDevice(Camera *camera, Request *request)
> {
> VimcCameraData *data = cameraData(camera);
> - Buffer *buffer = request->findBuffer(&data->stream_);
> + FrameBuffer *buffer = request->findBuffer(&data->stream_);
> if (!buffer) {
> LOG(VIMC, Error)
> << "Attempt to queue request with invalid stream";
> @@ -447,7 +437,7 @@ int VimcCameraData::init(MediaDevice *media)
> if (video_->open())
> return -ENODEV;
>
> - video_->bufferReady.connect(this, &VimcCameraData::bufferReady);
> + video_->frameBufferReady.connect(this, &VimcCameraData::bufferReady);
>
> raw_ = new V4L2VideoDevice(media->getEntityByName("Raw Capture 1"));
> if (raw_->open())
> @@ -484,7 +474,7 @@ int VimcCameraData::init(MediaDevice *media)
> return 0;
> }
>
> -void VimcCameraData::bufferReady(Buffer *buffer)
> +void VimcCameraData::bufferReady(FrameBuffer *buffer)
> {
> Request *request = buffer->request();
>
> diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp
> index 572f751b5217eb5f..0348e3cfa68ed6ec 100644
> --- a/src/libcamera/pipeline_handler.cpp
> +++ b/src/libcamera/pipeline_handler.cpp
> @@ -467,7 +467,7 @@ int PipelineHandler::queueRequest(Camera *camera, Request *request)
> * otherwise
> */
> bool PipelineHandler::completeBuffer(Camera *camera, Request *request,
> - Buffer *buffer)
> + FrameBuffer *buffer)
> {
> camera->bufferCompleted.emit(request, buffer);
> return request->completeBuffer(buffer);
> diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp
> index b17a6c2278361f00..66eab0cdead0f563 100644
> --- a/src/libcamera/request.cpp
> +++ b/src/libcamera/request.cpp
> @@ -75,11 +75,6 @@ Request::Request(Camera *camera, uint64_t cookie)
>
> Request::~Request()
> {
> - for (auto it : bufferMap_) {
> - Buffer *buffer = it.second;
> - delete buffer;
> - }
> -
> delete metadata_;
> delete controls_;
> delete validator_;
> @@ -106,18 +101,19 @@ Request::~Request()
> * \brief Retrieve the request's streams to buffers map
> *
> * Return a reference to the map that associates each Stream part of the
> - * request to the Buffer the Stream output should be directed to.
> + * request to the FrameBuffer the Stream output should be directed to.
> *
> - * \return The map of Stream to Buffer
> + * \return The map of Stream to FrameBuffer
> */
>
> /**
> - * \brief Store a Buffer with its associated Stream in the Request
> + * \brief Add a FrameBuffer with its associated Stream to the Request
> * \param[in] stream The stream the buffer belongs to
> - * \param[in] buffer The Buffer to store in the request
> + * \param[in] buffer The FrameBuffer to add to the request
> *
> - * Ownership of the buffer is passed to the request. It will be deleted when
> - * the request is destroyed after completing.
> + * A reference to the buffer is stored in the request. The caller is responsible
> + * for ensuring that the buffer will remain valid until the request complete
> + * callback is called.
> *
> * A request can only contain one buffer per stream. If a buffer has already
> * been added to the request for the same stream, this method returns -EEXIST.
> @@ -126,7 +122,7 @@ Request::~Request()
> * \retval -EEXIST The request already contains a buffer for the stream
> * \retval -EINVAL The buffer does not reference a valid Stream
> */
> -int Request::addBuffer(Stream *stream, std::unique_ptr<Buffer> buffer)
> +int Request::addBuffer(Stream *stream, FrameBuffer *buffer)
> {
> if (!stream) {
> LOG(Request, Error) << "Invalid stream reference";
> @@ -135,11 +131,11 @@ int Request::addBuffer(Stream *stream, std::unique_ptr<Buffer> buffer)
>
> auto it = bufferMap_.find(stream);
> if (it != bufferMap_.end()) {
> - LOG(Request, Error) << "Buffer already set for stream";
> + LOG(Request, Error) << "FrameBuffer already set for stream";
> return -EEXIST;
> }
>
> - bufferMap_[stream] = buffer.release();
> + bufferMap_[stream] = buffer;
>
> return 0;
> }
> @@ -159,7 +155,7 @@ int Request::addBuffer(Stream *stream, std::unique_ptr<Buffer> buffer)
> * \return The buffer associated with the stream, or nullptr if the stream is
> * not part of this request
> */
> -Buffer *Request::findBuffer(Stream *stream) const
> +FrameBuffer *Request::findBuffer(Stream *stream) const
> {
> auto it = bufferMap_.find(stream);
> if (it == bufferMap_.end())
> @@ -219,7 +215,7 @@ int Request::prepare()
> }
>
> for (auto const &pair : bufferMap_) {
> - Buffer *buffer = pair.second;
> + FrameBuffer *buffer = pair.second;
> buffer->request_ = this;
> pending_.insert(buffer);
> }
> @@ -253,7 +249,7 @@ void Request::complete()
> * \return True if all buffers contained in the request have completed, false
> * otherwise
> */
> -bool Request::completeBuffer(Buffer *buffer)
> +bool Request::completeBuffer(FrameBuffer *buffer)
> {
> int ret = pending_.erase(buffer);
> ASSERT(ret == 1);
> diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp
> index ca3464babdc1c313..701a2b9a73d53d96 100644
> --- a/src/qcam/main_window.cpp
> +++ b/src/qcam/main_window.cpp
> @@ -23,7 +23,7 @@
> using namespace libcamera;
>
> MainWindow::MainWindow(CameraManager *cm, const OptionsParser::Options &options)
> - : options_(options), isCapturing_(false)
> + : options_(options), allocator_(nullptr), isCapturing_(false)
> {
> int ret;
>
> @@ -37,8 +37,10 @@ MainWindow::MainWindow(CameraManager *cm, const OptionsParser::Options &options)
> adjustSize();
>
> ret = openCamera(cm);
> - if (!ret)
> + if (!ret) {
> + allocator_ = FrameBufferAllocator::create(camera_);
> ret = startCapture();
> + }
>
> if (ret < 0)
> QTimer::singleShot(0, QCoreApplication::instance(),
> @@ -49,6 +51,7 @@ MainWindow::~MainWindow()
> {
> if (camera_) {
> stopCapture();
> + delete allocator_;
> camera_->release();
> camera_.reset();
> }
> @@ -176,8 +179,14 @@ int MainWindow::startCapture()
> return ret;
> }
>
> + ret = allocator_->allocate(stream);
> + if (ret < 0) {
> + std::cerr << "Failed to allocate capture buffers" << std::endl;
> + return ret;
> + }
> +
> std::vector<Request *> requests;
> - for (unsigned int i = 0; i < cfg.bufferCount; ++i) {
> + for (const std::unique_ptr<FrameBuffer> &buffer : allocator_->buffers(stream)) {
> Request *request = camera_->createRequest();
> if (!request) {
> std::cerr << "Can't create request" << std::endl;
> @@ -185,13 +194,7 @@ int MainWindow::startCapture()
> goto error;
> }
>
> - std::unique_ptr<Buffer> buffer = stream->createBuffer(i);
> - if (!buffer) {
> - std::cerr << "Can't create buffer " << i << std::endl;
> - goto error;
> - }
> -
> - ret = request->addBuffer(stream, std::move(buffer));
> + ret = request->addBuffer(stream, buffer.get());
> if (ret < 0) {
> std::cerr << "Can't set buffer for request" << std::endl;
> goto error;
> @@ -254,11 +257,11 @@ void MainWindow::requestComplete(Request *request)
> if (request->status() == Request::RequestCancelled)
> return;
>
> - const std::map<Stream *, Buffer *> &buffers = request->buffers();
> + const std::map<Stream *, FrameBuffer *> &buffers = request->buffers();
>
> framesCaptured_++;
>
> - Buffer *buffer = buffers.begin()->second;
> + FrameBuffer *buffer = buffers.begin()->second;
> const FrameMetadata &metadata = buffer->metadata();
>
> double fps = metadata.timestamp - lastBufferTime_;
> @@ -281,28 +284,21 @@ void MainWindow::requestComplete(Request *request)
>
> for (auto it = buffers.begin(); it != buffers.end(); ++it) {
> Stream *stream = it->first;
> - Buffer *buffer = it->second;
> - unsigned int index = buffer->index();
> -
> - std::unique_ptr<Buffer> newBuffer = stream->createBuffer(index);
> - if (!newBuffer) {
> - std::cerr << "Can't create buffer" << std::endl;
> - return;
> - }
> + FrameBuffer *buffer = it->second;
>
> - request->addBuffer(stream, std::move(newBuffer));
> + request->addBuffer(stream, buffer);
> }
>
> camera_->queueRequest(request);
> }
>
> -int MainWindow::display(Buffer *buffer)
> +int MainWindow::display(FrameBuffer *buffer)
> {
> - if (buffer->mem()->planes().size() != 1)
> + if (buffer->planes().size() != 1)
> return -EINVAL;
>
> /* \todo Once the FrameBuffer is done cache mapped memory. */
> - const FrameBuffer::Plane &plane = buffer->mem()->planes().front();
> + const FrameBuffer::Plane &plane = buffer->planes().front();
> void *memory = mmap(NULL, plane.length, PROT_READ, MAP_SHARED,
> plane.fd.fd(), 0);
>
> diff --git a/src/qcam/main_window.h b/src/qcam/main_window.h
> index 0786e915ec512255..05cde4ceab5f7ea1 100644
> --- a/src/qcam/main_window.h
> +++ b/src/qcam/main_window.h
> @@ -14,8 +14,10 @@
> #include <QObject>
> #include <QTimer>
>
> +#include <libcamera/buffer.h>
> #include <libcamera/camera.h>
> #include <libcamera/camera_manager.h>
> +#include <libcamera/framebuffer_allocator.h>
> #include <libcamera/stream.h>
>
> #include "../cam/options.h"
> @@ -49,7 +51,7 @@ private:
> void stopCapture();
>
> void requestComplete(Request *request);
> - int display(Buffer *buffer);
> + int display(FrameBuffer *buffer);
>
> QString title_;
> QTimer titleTimer_;
> @@ -57,6 +59,8 @@ private:
> const OptionsParser::Options &options_;
>
> std::shared_ptr<Camera> camera_;
> + FrameBufferAllocator *allocator_;
> +
> bool isCapturing_;
> std::unique_ptr<CameraConfiguration> config_;
>
> diff --git a/src/v4l2/v4l2_camera.cpp b/src/v4l2/v4l2_camera.cpp
> index 53981bdf36e58767..6f17d60814047cf4 100644
> --- a/src/v4l2/v4l2_camera.cpp
> +++ b/src/v4l2/v4l2_camera.cpp
> @@ -16,24 +16,15 @@ using namespace libcamera;
>
> LOG_DECLARE_CATEGORY(V4L2Compat);
>
> -V4L2FrameMetadata::V4L2FrameMetadata(Buffer *buffer)
> - : index_(buffer->index()),
> - bytesused_(buffer->metadata().planes[0].bytesused),
> - timestamp_(buffer->metadata().timestamp),
> - sequence_(buffer->metadata().sequence),
> - status_(buffer->metadata().status)
> -{
> -}
> -
> V4L2Camera::V4L2Camera(std::shared_ptr<Camera> camera)
> - : camera_(camera), isRunning_(false)
> + : camera_(camera), isRunning_(false), bufferAllocator_(nullptr)
> {
> camera_->requestCompleted.connect(this, &V4L2Camera::requestComplete);
> }
>
> V4L2Camera::~V4L2Camera()
> {
> - camera_->release();
> + close();
> }
>
> int V4L2Camera::open()
> @@ -50,11 +41,16 @@ int V4L2Camera::open()
> return -EINVAL;
> }
>
> + bufferAllocator_ = FrameBufferAllocator::create(camera_);
> +
> return 0;
> }
>
> void V4L2Camera::close()
> {
> + delete bufferAllocator_;
> + bufferAllocator_ = nullptr;
> +
> camera_->release();
> }
>
> @@ -63,12 +59,12 @@ void V4L2Camera::getStreamConfig(StreamConfiguration *streamConfig)
> *streamConfig = config_->at(0);
> }
>
> -std::vector<V4L2FrameMetadata> V4L2Camera::completedBuffers()
> +std::vector<V4L2Camera::Buffer> V4L2Camera::completedBuffers()
> {
> - std::vector<V4L2FrameMetadata> v;
> + std::vector<Buffer> v;
>
> bufferLock_.lock();
> - for (std::unique_ptr<V4L2FrameMetadata> &metadata : completedBuffers_)
> + for (std::unique_ptr<Buffer> &metadata : completedBuffers_)
> v.push_back(*metadata.get());
> completedBuffers_.clear();
> bufferLock_.unlock();
> @@ -83,9 +79,9 @@ void V4L2Camera::requestComplete(Request *request)
>
> /* We only have one stream at the moment. */
> bufferLock_.lock();
> - Buffer *buffer = request->buffers().begin()->second;
> - std::unique_ptr<V4L2FrameMetadata> metadata =
> - utils::make_unique<V4L2FrameMetadata>(buffer);
> + FrameBuffer *buffer = request->buffers().begin()->second;
> + std::unique_ptr<Buffer> metadata =
> + utils::make_unique<Buffer>(buffer->cookie(), buffer->metadata());
> completedBuffers_.push_back(std::move(metadata));
> bufferLock_.unlock();
>
> @@ -126,18 +122,39 @@ int V4L2Camera::configure(StreamConfiguration *streamConfigOut,
> int V4L2Camera::allocBuffers(unsigned int count)
> {
> int ret = camera_->allocateBuffers();
> - return ret == -EACCES ? -EBUSY : ret;
> + if (ret)
> + return ret == -EACCES ? -EBUSY : ret;
> +
> + Stream *stream = *camera_->streams().begin();
> +
> + ret = bufferAllocator_->allocate(stream);
> + if (ret < 0)
> + return ret;
> +
> + unsigned int index = 0;
> + for (const std::unique_ptr<FrameBuffer> &buffer : bufferAllocator_->buffers(stream))
> + buffer->setCookie(index++);
How about moving this to V4L2Camera::qbuf() and passing it through the
request cookie ?
diff --git a/src/v4l2/v4l2_camera.cpp b/src/v4l2/v4l2_camera.cpp
index 3713a62e149d..a8a5b788d7c0 100644
--- a/src/v4l2/v4l2_camera.cpp
+++ b/src/v4l2/v4l2_camera.cpp
@@ -81,7 +81,7 @@ void V4L2Camera::requestComplete(Request *request)
bufferLock_.lock();
FrameBuffer *buffer = request->buffers().begin()->second;
std::unique_ptr<Buffer> metadata =
- utils::make_unique<Buffer>(buffer->cookie(), buffer->metadata());
+ utils::make_unique<Buffer>(request->cookie(), buffer->metadata());
completedBuffers_.push_back(std::move(metadata));
bufferLock_.unlock();
@@ -123,15 +123,7 @@ int V4L2Camera::allocBuffers(unsigned int count)
{
Stream *stream = *camera_->streams().begin();
- int ret = bufferAllocator_->allocate(stream);
- if (ret < 0)
- return ret;
-
- unsigned int index = 0;
- for (const std::unique_ptr<FrameBuffer> &buffer : bufferAllocator_->buffers(stream))
- buffer->setCookie(index++);
-
- return ret;
+ return bufferAllocator_->allocate(stream);
}
void V4L2Camera::freeBuffers()
@@ -193,7 +185,7 @@ int V4L2Camera::streamOff()
int V4L2Camera::qbuf(unsigned int index)
{
std::unique_ptr<Request> request =
- std::unique_ptr<Request>(camera_->createRequest());
+ std::unique_ptr<Request>(camera_->createRequest(index));
if (!request) {
LOG(V4L2Compat, Error) << "Can't create request";
return -ENOMEM;
With this,
Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> +
> + return ret;
> }
>
> void V4L2Camera::freeBuffers()
> {
> + Stream *stream = *camera_->streams().begin();
> + bufferAllocator_->free(stream);
> camera_->freeBuffers();
> }
>
> int V4L2Camera::getBufferFd(unsigned int index)
> {
> Stream *stream = *camera_->streams().begin();
> - return stream->buffers()[index].planes()[0].fd.fd();
> + const std::vector<std::unique_ptr<FrameBuffer>> &buffers =
> + bufferAllocator_->buffers(stream);
> +
> + if (buffers.size() <= index)
> + return -EINVAL;
> +
> + return buffers[index]->planes()[0].fd.fd();
> }
>
> int V4L2Camera::streamOn()
> @@ -180,13 +197,6 @@ int V4L2Camera::streamOff()
>
> int V4L2Camera::qbuf(unsigned int index)
> {
> - Stream *stream = config_->at(0).stream();
> - std::unique_ptr<Buffer> buffer = stream->createBuffer(index);
> - if (!buffer) {
> - LOG(V4L2Compat, Error) << "Can't create buffer";
> - return -ENOMEM;
> - }
> -
> std::unique_ptr<Request> request =
> std::unique_ptr<Request>(camera_->createRequest());
> if (!request) {
> @@ -194,7 +204,9 @@ int V4L2Camera::qbuf(unsigned int index)
> return -ENOMEM;
> }
>
> - int ret = request->addBuffer(stream, std::move(buffer));
> + Stream *stream = config_->at(0).stream();
> + FrameBuffer *buffer = bufferAllocator_->buffers(stream)[index].get();
> + int ret = request->addBuffer(stream, buffer);
> if (ret < 0) {
> LOG(V4L2Compat, Error) << "Can't set buffer for request";
> return -ENOMEM;
> diff --git a/src/v4l2/v4l2_camera.h b/src/v4l2/v4l2_camera.h
> index 9d0506e512a7aeb9..84e4562d93e186fd 100644
> --- a/src/v4l2/v4l2_camera.h
> +++ b/src/v4l2/v4l2_camera.h
> @@ -9,49 +9,37 @@
> #define __V4L2_CAMERA_H__
>
> #include <deque>
> -#include <linux/videodev2.h>
> #include <mutex>
> +#include <utility>
>
> #include <libcamera/buffer.h>
> #include <libcamera/camera.h>
> +#include <libcamera/framebuffer_allocator.h>
>
> #include "semaphore.h"
>
> using namespace libcamera;
>
> -class V4L2FrameMetadata
> +class V4L2Camera : public Object
> {
> public:
> - V4L2FrameMetadata(Buffer *buffer);
> -
> - int index() const { return index_; }
> -
> - unsigned int bytesused() const { return bytesused_; }
> - uint64_t timestamp() const { return timestamp_; }
> - unsigned int sequence() const { return sequence_; }
> -
> - FrameMetadata::Status status() const { return status_; }
> -
> -private:
> - int index_;
> + struct Buffer {
> + Buffer(unsigned int index, const FrameMetadata &data)
> + : index(index), data(data)
> + {
> + }
>
> - unsigned int bytesused_;
> - uint64_t timestamp_;
> - unsigned int sequence_;
> + unsigned int index;
> + FrameMetadata data;
> + };
>
> - FrameMetadata::Status status_;
> -};
> -
> -class V4L2Camera : public Object
> -{
> -public:
> V4L2Camera(std::shared_ptr<Camera> camera);
> ~V4L2Camera();
>
> int open();
> void close();
> void getStreamConfig(StreamConfiguration *streamConfig);
> - std::vector<V4L2FrameMetadata> completedBuffers();
> + std::vector<Buffer> completedBuffers();
>
> int configure(StreamConfiguration *streamConfigOut,
> const Size &size, PixelFormat pixelformat,
> @@ -77,9 +65,10 @@ private:
> bool isRunning_;
>
> std::mutex bufferLock_;
> + FrameBufferAllocator *bufferAllocator_;
>
> std::deque<std::unique_ptr<Request>> pendingRequests_;
> - std::deque<std::unique_ptr<V4L2FrameMetadata>> completedBuffers_;
> + std::deque<std::unique_ptr<Buffer>> completedBuffers_;
> };
>
> #endif /* __V4L2_CAMERA_H__ */
> diff --git a/src/v4l2/v4l2_camera_proxy.cpp b/src/v4l2/v4l2_camera_proxy.cpp
> index 303d5f5d5302b98e..465e54e8dad0c31a 100644
> --- a/src/v4l2/v4l2_camera_proxy.cpp
> +++ b/src/v4l2/v4l2_camera_proxy.cpp
> @@ -183,17 +183,18 @@ void V4L2CameraProxy::querycap(std::shared_ptr<Camera> camera)
>
> void V4L2CameraProxy::updateBuffers()
> {
> - std::vector<V4L2FrameMetadata> completedBuffers = vcam_->completedBuffers();
> - for (V4L2FrameMetadata &fmd : completedBuffers) {
> - struct v4l2_buffer &buf = buffers_[fmd.index()];
> + std::vector<V4L2Camera::Buffer> completedBuffers = vcam_->completedBuffers();
> + for (const V4L2Camera::Buffer &buffer : completedBuffers) {
> + const FrameMetadata &fmd = buffer.data;
> + struct v4l2_buffer &buf = buffers_[buffer.index];
>
> - switch (fmd.status()) {
> + switch (fmd.status) {
> case FrameMetadata::FrameSuccess:
> - buf.bytesused = fmd.bytesused();
> + buf.bytesused = fmd.planes[0].bytesused;
> buf.field = V4L2_FIELD_NONE;
> - buf.timestamp.tv_sec = fmd.timestamp() / 1000000000;
> - buf.timestamp.tv_usec = fmd.timestamp() % 1000000;
> - buf.sequence = fmd.sequence();
> + buf.timestamp.tv_sec = fmd.timestamp / 1000000000;
> + buf.timestamp.tv_usec = fmd.timestamp % 1000000;
> + buf.sequence = fmd.sequence;
>
> buf.flags |= V4L2_BUF_FLAG_DONE;
> break;
> diff --git a/test/camera/buffer_import.cpp b/test/camera/buffer_import.cpp
> index 327db7912c09ed2d..f506d1b221e568ad 100644
> --- a/test/camera/buffer_import.cpp
> +++ b/test/camera/buffer_import.cpp
> @@ -120,7 +120,7 @@ public:
> }
>
> protected:
> - void bufferComplete(Request *request, Buffer *buffer)
> + void bufferComplete(Request *request, FrameBuffer *buffer)
> {
> if (buffer->metadata().status != FrameMetadata::FrameSuccess)
> return;
> @@ -133,17 +133,16 @@ protected:
> if (request->status() != Request::RequestComplete)
> return;
>
> - const std::map<Stream *, Buffer *> &buffers = request->buffers();
> + const std::map<Stream *, FrameBuffer *> &buffers = request->buffers();
>
> completeRequestsCount_++;
>
> /* Create a new request. */
> Stream *stream = buffers.begin()->first;
> - int dmabuf = buffers.begin()->second->dmabufs()[0];
> - std::unique_ptr<Buffer> buffer = stream->createBuffer({ dmabuf, -1, -1 });
> + FrameBuffer *buffer = buffers.begin()->second;
>
> request = camera_->createRequest();
> - request->addBuffer(stream, std::move(buffer));
> + request->addBuffer(stream, buffer);
> camera_->queueRequest(request);
> }
>
> @@ -158,9 +157,6 @@ protected:
> return TestFail;
> }
>
> - StreamConfiguration &cfg = config_->at(0);
> - cfg.memoryType = ExternalMemory;
> -
> return TestPass;
> }
>
> @@ -191,17 +187,14 @@ protected:
> return TestFail;
>
> std::vector<Request *> requests;
> - for (const std::unique_ptr<FrameBuffer> &framebuffer : source.buffers()) {
> - int dmabuf = framebuffer->planes()[0].fd.fd();
> -
> + for (const std::unique_ptr<FrameBuffer> &buffer : source.buffers()) {
> Request *request = camera_->createRequest();
> if (!request) {
> std::cout << "Failed to create request" << std::endl;
> return TestFail;
> }
>
> - std::unique_ptr<Buffer> buffer = stream->createBuffer({ dmabuf, -1, -1 });
> - if (request->addBuffer(stream, std::move(buffer))) {
> + if (request->addBuffer(stream, buffer.get())) {
> std::cout << "Failed to associating buffer with request" << std::endl;
> return TestFail;
> }
> diff --git a/test/camera/capture.cpp b/test/camera/capture.cpp
> index 0d9ffc476650f414..de879ee4eb1420a6 100644
> --- a/test/camera/capture.cpp
> +++ b/test/camera/capture.cpp
> @@ -26,7 +26,7 @@ protected:
> unsigned int completeBuffersCount_;
> unsigned int completeRequestsCount_;
>
> - void bufferComplete(Request *request, Buffer *buffer)
> + void bufferComplete(Request *request, FrameBuffer *buffer)
> {
> if (buffer->metadata().status != FrameMetadata::FrameSuccess)
> return;
> @@ -39,17 +39,16 @@ protected:
> if (request->status() != Request::RequestComplete)
> return;
>
> - const std::map<Stream *, Buffer *> &buffers = request->buffers();
> + const std::map<Stream *, FrameBuffer *> &buffers = request->buffers();
>
> completeRequestsCount_++;
>
> /* Create a new request. */
> Stream *stream = buffers.begin()->first;
> - Buffer *buffer = buffers.begin()->second;
> - std::unique_ptr<Buffer> newBuffer = stream->createBuffer(buffer->index());
> + FrameBuffer *buffer = buffers.begin()->second;
>
> request = camera_->createRequest();
> - request->addBuffer(stream, std::move(newBuffer));
> + request->addBuffer(stream, buffer);
> camera_->queueRequest(request);
> }
>
> @@ -64,9 +63,16 @@ protected:
> return TestFail;
> }
>
> + allocator_ = FrameBufferAllocator::create(camera_);
> +
> return TestPass;
> }
>
> + void cleanup() override
> + {
> + delete allocator_;
> + }
> +
> int run() override
> {
> StreamConfiguration &cfg = config_->at(0);
> @@ -87,21 +93,20 @@ protected:
> }
>
> Stream *stream = cfg.stream();
> +
> + int ret = allocator_->allocate(stream);
> + if (ret < 0)
> + return TestFail;
> +
> std::vector<Request *> requests;
> - for (unsigned int i = 0; i < cfg.bufferCount; ++i) {
> + for (const std::unique_ptr<FrameBuffer> &buffer : allocator_->buffers(stream)) {
> Request *request = camera_->createRequest();
> if (!request) {
> cout << "Failed to create request" << endl;
> return TestFail;
> }
>
> - std::unique_ptr<Buffer> buffer = stream->createBuffer(i);
> - if (!buffer) {
> - cout << "Failed to create buffer " << i << endl;
> - return TestFail;
> - }
> -
> - if (request->addBuffer(stream, std::move(buffer))) {
> + if (request->addBuffer(stream, buffer.get())) {
> cout << "Failed to associating buffer with request" << endl;
> return TestFail;
> }
> @@ -134,10 +139,12 @@ protected:
> while (timer.isRunning())
> dispatcher->processEvents();
>
> - if (completeRequestsCount_ <= cfg.bufferCount * 2) {
> + unsigned int nbuffers = allocator_->buffers(stream).size();
> +
> + if (completeRequestsCount_ <= nbuffers * 2) {
> cout << "Failed to capture enough frames (got "
> << completeRequestsCount_ << " expected at least "
> - << cfg.bufferCount * 2 << ")" << endl;
> + << nbuffers * 2 << ")" << endl;
> return TestFail;
> }
>
> @@ -160,6 +167,7 @@ protected:
> }
>
> std::unique_ptr<CameraConfiguration> config_;
> + FrameBufferAllocator *allocator_;
> };
>
> } /* namespace */
> diff --git a/test/camera/statemachine.cpp b/test/camera/statemachine.cpp
> index f627b8f37422350e..f3a7ca7c32a5ec97 100644
> --- a/test/camera/statemachine.cpp
> +++ b/test/camera/statemachine.cpp
> @@ -185,6 +185,12 @@ protected:
> if (camera_->allocateBuffers())
> return TestFail;
>
> + /* Use internally allocated buffers. */
> + allocator_ = FrameBufferAllocator::create(camera_);
> + Stream *stream = *camera_->streams().begin();
> + if (allocator_->allocate(stream) < 0)
> + return TestFail;
> +
> if (camera_->start())
> return TestFail;
>
> @@ -218,8 +224,7 @@ protected:
> return TestFail;
>
> Stream *stream = *camera_->streams().begin();
> - std::unique_ptr<Buffer> buffer = stream->createBuffer(0);
> - if (request->addBuffer(stream, std::move(buffer)))
> + if (request->addBuffer(stream, allocator_->buffers(stream)[0].get()))
> return TestFail;
>
> if (camera_->queueRequest(request))
> @@ -229,6 +234,8 @@ protected:
> if (camera_->stop())
> return TestFail;
>
> + delete allocator_;
> +
> if (camera_->freeBuffers())
> return TestFail;
>
> @@ -283,6 +290,7 @@ protected:
> }
>
> std::unique_ptr<CameraConfiguration> defconf_;
> + FrameBufferAllocator *allocator_;
> };
>
> } /* namespace */
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list