[libcamera-devel] [PATCH v2 20/25] libcamera: Switch to FrameBuffer interface
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Wed Jan 8 05:17:01 CET 2020
Hi Niklas,
Thank you for the patch.
On Mon, Dec 30, 2019 at 01:05:05PM +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.
>
> A follow up changes to this one will finalize the transition to the new
s/A follow up/Follow-up/
> FrameBuffer interface by removing code that is left unused after this
> change.
>
> Signed-off-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> ---
> include/libcamera/camera.h | 4 +-
> include/libcamera/request.h | 14 +--
> src/android/camera_device.cpp | 29 ++++--
> 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 | 49 +++------
> 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 | 29 +++---
> src/qcam/main_window.cpp | 44 ++++----
> src/qcam/main_window.h | 6 +-
> test/camera/buffer_import.cpp | 19 ++--
> test/camera/capture.cpp | 38 ++++---
> test/camera/statemachine.cpp | 12 ++-
> 20 files changed, 209 insertions(+), 306 deletions(-)
>
> diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h
> index f2827438871189a1..20f48faed62da7a7 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..21417aba19135de5 100644
> --- a/src/android/camera_device.cpp
> +++ b/src/android/camera_device.cpp
> @@ -739,13 +739,19 @@ 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++) {
> + planes[i].fd = FileDescriptor(camera3Handle->data[i]);
You haven't tested this, have you ? :-) planes is empty at this point.
> + /*
> + * 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.
> + */
> + planes[i].length = 0;
> + }
> +
> + FrameBuffer *buffer = new FrameBuffer(planes);
> if (!buffer) {
> LOG(HAL, Error) << "Failed to create buffer";
> delete descriptor;
> @@ -754,7 +760,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 +777,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 +809,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 +831,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 7c58a1f50829f290..320ae3332a4db4a6 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()) {
> void *data = mmap(NULL, plane.length, PROT_READ, MAP_SHARED,
> plane.fd.fd(), 0);
> unsigned int length = plane.length;
> 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 222c6811d5698f0f..5ea663ef85720f2e 100644
> --- a/src/libcamera/camera.cpp
> +++ b/src/libcamera/camera.cpp
> @@ -691,12 +691,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;
> @@ -752,14 +746,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_);
> @@ -825,24 +811,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();
> @@ -877,6 +850,14 @@ int Camera::start()
>
> LOG(Camera, Debug) << "Starting capture";
>
> + for (Stream * stream : activeStreams_) {
> + if (allocator_ && !allocator_->buffers(stream).empty())
> + continue;
> +
> + unsigned int bufferCount = stream->configuration_.bufferCount;
> + pipe_->importFrameBuffers(this, stream, bufferCount);
Maybe ditch the bufferCount local variable ? And actually, how about
ditching the bufferCount parameter to importFrameBuffers() ? The
pipeline handlers can get it from stream->configuration_.bufferCount
themselves.
> + }
> +
> int ret = pipe_->start(this);
> if (ret)
> return ret;
> @@ -912,6 +893,13 @@ int Camera::stop()
>
> pipe_->stop(this);
>
> + for (Stream * stream : activeStreams_) {
> + if (allocator_ && !allocator_->buffers(stream).empty())
> + continue;
> +
> + pipe_->freeFrameBuffers(this, stream);
> + }
> +
> return 0;
> }
>
> @@ -925,13 +913,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 520d3fccaaa130cc..20b866c4a511387a 100644
> --- a/src/libcamera/include/pipeline_handler.h
> +++ b/src/libcamera/include/pipeline_handler.h
> @@ -20,12 +20,12 @@
>
> namespace libcamera {
>
> -class Buffer;
> class Camera;
> class CameraConfiguration;
> class CameraManager;
> class DeviceEnumerator;
> class DeviceMatch;
> +class FrameBuffer;
> class MediaDevice;
> class PipelineHandler;
> class Request;
> @@ -86,7 +86,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 f9bddcc88523301f..2f3ba2bf10f2f177 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);
>
> @@ -693,39 +689,30 @@ int PipelineHandlerIPU3::allocateBuffers(Camera *camera,
Hmmmm... allocateFrameBuffers() and importFrameBuffers() are supposed to
allocate other internal resources for the streams, but we still call
PipelineHandlerIPU3::allocateBuffers() from
PipelineHandlerIPU3::start(). There's something not quite right here :-S
I may accept to fix this on top of this series, but we're getting very
close to a point where I would consider this as a blocker, as it has
implications on the API. More work is clearly needed, the
allocateFrameBuffers(), importFrameBuffers() and start() operations
probably need to be revisited.
> 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;
> @@ -742,7 +729,7 @@ int PipelineHandlerIPU3::freeBuffers(Camera *camera,
> IPU3CameraData *data = cameraData(camera);
>
> data->cio2_.freeBuffers();
> - data->imgu_->freeBuffers();
> + data->imgu_->freeBuffers(data);
>
> return 0;
> }
> @@ -795,7 +782,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)
> @@ -922,9 +909,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. */
> @@ -973,7 +960,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();
>
> @@ -1048,7 +1035,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");
> @@ -1058,7 +1044,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();
> @@ -1166,69 +1151,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_) {
This makes me even more convinced that we're not there yet. It's too
complicated for pipeline handlers.
> + 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 e77925f6f9deff08..cc9c9ff961e948dc 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
> @@ -208,7 +207,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);
>
> @@ -246,7 +245,7 @@ RkISP1FrameInfo *RkISP1Frames::create(unsigned int frame, Request *request, Stre
> }
> FrameBuffer *statBuffer = pipe_->statBuffers_.front();
>
> - Buffer *videoBuffer = request->findBuffer(stream);
> + FrameBuffer *videoBuffer = request->findBuffer(stream);
> if (!videoBuffer) {
> LOG(RkISP1, Error)
> << "Attempt to queue request with invalid stream";
> @@ -299,26 +298,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;
> }
>
> @@ -696,17 +683,8 @@ int PipelineHandlerRkISP1::allocateBuffers(Camera *camera,
> {
> RkISP1CameraData *data = cameraData(camera);
> unsigned int count = 1;
> - Stream *stream = *streams.begin();
> 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);
> @@ -772,9 +750,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;
> }
>
> @@ -980,7 +955,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);
>
> @@ -1029,7 +1004,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 655c5e6d96a696d6..c29bad707b464bcd 100644
> --- a/src/libcamera/pipeline/uvcvideo.cpp
> +++ b/src/libcamera/pipeline/uvcvideo.cpp
> @@ -41,7 +41,7 @@ public:
> }
>
> int init(MediaEntity *entity);
> - void bufferReady(Buffer *buffer);
> + void bufferReady(FrameBuffer *buffer);
>
> V4L2VideoDevice *video_;
> Stream stream_;
> @@ -226,23 +226,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)
> @@ -296,7 +286,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";
> @@ -361,7 +351,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();
> @@ -401,7 +391,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 02235ffb0515982f..33fec6520240b328 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_;
> @@ -293,23 +293,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)
> @@ -357,7 +347,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";
> @@ -449,7 +439,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())
> @@ -486,7 +476,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 fb69ca8e97216e6c..11286d31d4e54213 100644
> --- a/src/libcamera/pipeline_handler.cpp
> +++ b/src/libcamera/pipeline_handler.cpp
> @@ -460,7 +460,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..4ac0d9684e5be801 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,18 @@ 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 Store a FrameBuffer with its associated Stream in the Request
s/Store/Add/
s/in the/to the/
> * \param[in] stream The stream the buffer belongs to
> - * \param[in] buffer The Buffer to store in the request
s/to store in/to add to/
> + * \param[in] buffer The FrameBuffer to store in the request
Ditto.
> *
> - * Ownership of the buffer is passed to the request. It will be deleted when
> - * the request is destroyed after completing.
> + * Ownership of the buffer is passed to the request. Owner will be returned to
> + * the user when Request complete callback is called.
This isn't accurate, the application still owns the buffer.
* 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 +121,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 +130,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 +154,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 +214,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 +248,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 47793702b9aa0ee9..e53ed2801291f2c5 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/test/camera/buffer_import.cpp b/test/camera/buffer_import.cpp
> index 6559a89ce914a183..63e5fb100e49f958 100644
> --- a/test/camera/buffer_import.cpp
> +++ b/test/camera/buffer_import.cpp
> @@ -117,7 +117,7 @@ public:
> }
>
> protected:
> - void bufferComplete(Request *request, Buffer *buffer)
> + void bufferComplete(Request *request, FrameBuffer *buffer)
> {
> if (buffer->metadata().status != FrameMetadata::FrameSuccess)
> return;
> @@ -130,17 +130,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);
> }
>
> @@ -155,9 +154,6 @@ protected:
> return TestFail;
> }
>
> - StreamConfiguration &cfg = config_->at(0);
> - cfg.memoryType = ExternalMemory;
> -
> return TestPass;
> }
>
> @@ -188,17 +184,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