From patchwork Mon Oct 28 02:25:14 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Niklas_S=C3=B6derlund?= X-Patchwork-Id: 2261 Return-Path: Received: from vsp-unauthed02.binero.net (vsp-unauthed02.binero.net [195.74.38.227]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 064DB6017C for ; Mon, 28 Oct 2019 03:25:47 +0100 (CET) X-Halon-ID: 3e3a1701-f92a-11e9-903a-005056917f90 Authorized-sender: niklas@soderlund.pp.se Received: from localhost.localdomain (unknown [93.2.121.143]) by bin-vsp-out-02.atm.binero.net (Halon) with ESMTPA id 3e3a1701-f92a-11e9-903a-005056917f90; Mon, 28 Oct 2019 03:25:44 +0100 (CET) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org Date: Mon, 28 Oct 2019 03:25:14 +0100 Message-Id: <20191028022525.796995-2-niklas.soderlund@ragnatech.se> X-Mailer: git-send-email 2.23.0 In-Reply-To: <20191028022525.796995-1-niklas.soderlund@ragnatech.se> References: <20191028022525.796995-1-niklas.soderlund@ragnatech.se> MIME-Version: 1.0 Subject: [libcamera-devel] [RFC 01/12] test: camera: buffer_import: Remove test X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 28 Oct 2019 02:25:48 -0000 The buffer interface is about to be reworked to hide the specifics of buffer importing and its mapping to V4L2 devices from applications. In preparation of that remove the camera test which examines this. The proper operation of buffer importing on the V4L2 level is still tested in the V4L2 specific buffer sharing test case. Signed-off-by: Niklas Söderlund Acked-by: Laurent Pinchart --- test/camera/buffer_import.cpp | 432 ---------------------------------- test/camera/meson.build | 1 - 2 files changed, 433 deletions(-) delete mode 100644 test/camera/buffer_import.cpp diff --git a/test/camera/buffer_import.cpp b/test/camera/buffer_import.cpp deleted file mode 100644 index 9cac19d8ce81fa4d..0000000000000000 --- a/test/camera/buffer_import.cpp +++ /dev/null @@ -1,432 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0-or-later */ -/* - * Copyright (C) 2019, Google Inc. - * - * libcamera Camera API tests - * - * Test importing buffers exported from the VIVID output device into a Camera - */ - -#include -#include -#include -#include -#include - -#include "device_enumerator.h" -#include "media_device.h" -#include "v4l2_videodevice.h" - -#include "camera_test.h" - -using namespace libcamera; - -/* Keep SINK_BUFFER_COUNT > CAMERA_BUFFER_COUNT + 1 */ -static constexpr unsigned int SINK_BUFFER_COUNT = 8; -static constexpr unsigned int CAMERA_BUFFER_COUNT = 4; - -class FrameSink -{ -public: - FrameSink() - : video_(nullptr) - { - } - - int init() - { - int ret; - - /* Locate and open the video device. */ - std::string videoDeviceName = "vivid-000-vid-out"; - - std::unique_ptr enumerator = - DeviceEnumerator::create(); - if (!enumerator) { - std::cout << "Failed to create device enumerator" << std::endl; - return TestFail; - } - - if (enumerator->enumerate()) { - std::cout << "Failed to enumerate media devices" << std::endl; - return TestFail; - } - - DeviceMatch dm("vivid"); - dm.add(videoDeviceName); - - media_ = enumerator->search(dm); - if (!media_) { - std::cout << "No vivid output device available" << std::endl; - return TestSkip; - } - - video_ = V4L2VideoDevice::fromEntityName(media_.get(), videoDeviceName); - if (!video_) { - std::cout << "Unable to open " << videoDeviceName << std::endl; - return TestFail; - } - - if (video_->open()) - return TestFail; - - /* Configure the format. */ - ret = video_->getFormat(&format_); - if (ret) { - std::cout << "Failed to get format on output device" << std::endl; - return ret; - } - - format_.size.width = 1920; - format_.size.height = 1080; - format_.fourcc = V4L2_PIX_FMT_RGB24; - format_.planesCount = 1; - format_.planes[0].size = 1920 * 1080 * 3; - format_.planes[0].bpl = 1920 * 3; - - if (video_->setFormat(&format_)) { - cleanup(); - return TestFail; - } - - /* Export the buffers to a pool. */ - pool_.createBuffers(SINK_BUFFER_COUNT); - ret = video_->exportBuffers(&pool_); - if (ret) { - std::cout << "Failed to export buffers" << std::endl; - cleanup(); - return TestFail; - } - - /* Only use the first CAMERA_BUFFER_COUNT buffers to start with. */ - availableBuffers_.resize(CAMERA_BUFFER_COUNT); - std::iota(availableBuffers_.begin(), availableBuffers_.end(), 0); - - /* Connect the buffer ready signal. */ - video_->bufferReady.connect(this, &FrameSink::bufferComplete); - - return TestPass; - } - - void cleanup() - { - if (video_) { - video_->streamOff(); - video_->releaseBuffers(); - video_->close(); - - delete video_; - video_ = nullptr; - } - - if (media_) - media_->release(); - } - - int start() - { - requestsCount_ = 0; - done_ = false; - - int ret = video_->streamOn(); - if (ret < 0) - return ret; - - /* Queue all the initial requests. */ - for (unsigned int index = 0; index < CAMERA_BUFFER_COUNT; ++index) - queueRequest(index); - - return 0; - } - - int stop() - { - return video_->streamOff(); - } - - void requestComplete(uint64_t cookie, const Buffer *metadata) - { - unsigned int index = cookie; - - Buffer *buffer = new Buffer(index, metadata); - int ret = video_->queueBuffer(buffer); - if (ret < 0) - std::cout << "Failed to queue buffer to sink" << std::endl; - } - - bool done() const { return done_; } - const V4L2DeviceFormat &format() const { return format_; } - - Signal requestReady; - -private: - void queueRequest(unsigned int index) - { - auto it = std::find(availableBuffers_.begin(), - availableBuffers_.end(), index); - availableBuffers_.erase(it); - - uint64_t cookie = index; - BufferMemory &mem = pool_.buffers()[index]; - int dmabuf = mem.planes()[0].dmabuf(); - - requestReady.emit(cookie, dmabuf); - - requestsCount_++; - } - - void bufferComplete(Buffer *buffer) - { - availableBuffers_.push_back(buffer->index()); - - /* - * Pick the buffer for the next request among the available - * buffers. - * - * For the first 20 frames, select the buffer that has just - * completed to keep the mapping of dmabuf fds to buffers - * unchanged in the camera. - * - * For the next 20 frames, cycle randomly over the available - * buffers. The mapping should still be kept unchanged, as the - * camera should map using the cached fds. - * - * For the last 20 frames, cycles through all buffers, which - * should trash the mappings. - */ - unsigned int index = buffer->index(); - delete buffer; - - std::cout << "Completed buffer, request=" << requestsCount_ - << ", available buffers=" << availableBuffers_.size() - << std::endl; - - if (requestsCount_ >= 60) { - if (availableBuffers_.size() == SINK_BUFFER_COUNT) - done_ = true; - return; - } - - if (requestsCount_ == 40) { - /* Add the remaining of the buffers. */ - for (unsigned int i = CAMERA_BUFFER_COUNT; - i < SINK_BUFFER_COUNT; ++i) - availableBuffers_.push_back(i); - } - - if (requestsCount_ >= 20) { - /* - * Wait until we have enough buffers to make this - * meaningful. Preferably half of the camera buffers, - * but no less than 2 in any case. - */ - const unsigned int min_pool_size = - std::min(CAMERA_BUFFER_COUNT / 2, 2U); - if (availableBuffers_.size() < min_pool_size) - return; - - /* Pick a buffer at random. */ - unsigned int pos = random_() % availableBuffers_.size(); - index = availableBuffers_[pos]; - } - - queueRequest(index); - } - - std::shared_ptr media_; - V4L2VideoDevice *video_; - BufferPool pool_; - V4L2DeviceFormat format_; - - unsigned int requestsCount_; - std::vector availableBuffers_; - std::random_device random_; - - bool done_; -}; - -class BufferImportTest : public CameraTest -{ -public: - BufferImportTest() - : CameraTest() - { - } - - void queueRequest(uint64_t cookie, int dmabuf) - { - Request *request = camera_->createRequest(cookie); - - std::unique_ptr buffer = stream_->createBuffer({ dmabuf, -1, -1 }); - request->addBuffer(move(buffer)); - camera_->queueRequest(request); - } - -protected: - void bufferComplete(Request *request, Buffer *buffer) - { - if (buffer->status() != Buffer::BufferSuccess) - return; - - unsigned int index = buffer->index(); - int dmabuf = buffer->dmabufs()[0]; - - /* Record dmabuf to index remappings. */ - bool remapped = false; - if (bufferMappings_.find(index) != bufferMappings_.end()) { - if (bufferMappings_[index] != dmabuf) - remapped = true; - } - - std::cout << "Completed request " << framesCaptured_ - << ": dmabuf fd " << dmabuf - << " -> index " << index - << " (" << (remapped ? 'R' : '-') << ")" - << std::endl; - - if (remapped) - bufferRemappings_.push_back(framesCaptured_); - - bufferMappings_[index] = dmabuf; - framesCaptured_++; - - sink_.requestComplete(request->cookie(), buffer); - - if (framesCaptured_ == 60) - sink_.stop(); - } - - int initCamera() - { - if (camera_->acquire()) { - std::cout << "Failed to acquire the camera" << std::endl; - return TestFail; - } - - /* - * Configure the Stream to work with externally allocated - * buffers by setting the memoryType to ExternalMemory. - */ - std::unique_ptr config; - config = camera_->generateConfiguration({ StreamRole::VideoRecording }); - if (!config || config->size() != 1) { - std::cout << "Failed to generate configuration" << std::endl; - return TestFail; - } - - const V4L2DeviceFormat &format = sink_.format(); - - StreamConfiguration &cfg = config->at(0); - cfg.size = format.size; - cfg.pixelFormat = format.fourcc; - cfg.bufferCount = CAMERA_BUFFER_COUNT; - cfg.memoryType = ExternalMemory; - - if (camera_->configure(config.get())) { - std::cout << "Failed to set configuration" << std::endl; - return TestFail; - } - - stream_ = cfg.stream(); - - /* Allocate buffers. */ - if (camera_->allocateBuffers()) { - std::cout << "Failed to allocate buffers" << std::endl; - return TestFail; - } - - /* Connect the buffer completed signal. */ - camera_->bufferCompleted.connect(this, &BufferImportTest::bufferComplete); - - return TestPass; - } - - int init() - { - int ret = CameraTest::init(); - if (ret) - return ret; - - ret = sink_.init(); - if (ret != TestPass) { - cleanup(); - return ret; - } - - ret = initCamera(); - if (ret != TestPass) { - cleanup(); - return ret; - } - - sink_.requestReady.connect(this, &BufferImportTest::queueRequest); - return TestPass; - } - - int run() - { - int ret; - - framesCaptured_ = 0; - - if (camera_->start()) { - std::cout << "Failed to start camera" << std::endl; - return TestFail; - } - - ret = sink_.start(); - if (ret < 0) { - std::cout << "Failed to start sink" << std::endl; - return TestFail; - } - - EventDispatcher *dispatcher = cm_->eventDispatcher(); - - Timer timer; - timer.start(5000); - while (timer.isRunning() && !sink_.done()) - dispatcher->processEvents(); - - std::cout << framesCaptured_ << " frames captured, " - << bufferRemappings_.size() << " buffers remapped" - << std::endl; - - if (framesCaptured_ < 60) { - std::cout << "Too few frames captured" << std::endl; - return TestFail; - } - - if (bufferRemappings_.empty()) { - std::cout << "No buffer remappings" << std::endl; - return TestFail; - } - - if (bufferRemappings_[0] < 40) { - std::cout << "Early buffer remapping" << std::endl; - return TestFail; - } - - return TestPass; - } - - void cleanup() - { - sink_.cleanup(); - - camera_->stop(); - camera_->freeBuffers(); - - CameraTest::cleanup(); - } - -private: - Stream *stream_; - - std::map bufferMappings_; - std::vector bufferRemappings_; - unsigned int framesCaptured_; - - FrameSink sink_; -}; - -TEST_REGISTER(BufferImportTest); diff --git a/test/camera/meson.build b/test/camera/meson.build index d6fd66b8f89e21b4..35e97ce5de1a72ca 100644 --- a/test/camera/meson.build +++ b/test/camera/meson.build @@ -3,7 +3,6 @@ camera_tests = [ [ 'configuration_default', 'configuration_default.cpp' ], [ 'configuration_set', 'configuration_set.cpp' ], - [ 'buffer_import', 'buffer_import.cpp' ], [ 'statemachine', 'statemachine.cpp' ], [ 'capture', 'capture.cpp' ], ] From patchwork Mon Oct 28 02:25:15 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Niklas_S=C3=B6derlund?= X-Patchwork-Id: 2262 Return-Path: Received: from bin-mail-out-05.binero.net (bin-mail-out-05.binero.net [195.74.38.228]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 972546017D for ; Mon, 28 Oct 2019 03:25:49 +0100 (CET) X-Halon-ID: 3f242e72-f92a-11e9-903a-005056917f90 Authorized-sender: niklas@soderlund.pp.se Received: from localhost.localdomain (unknown [93.2.121.143]) by bin-vsp-out-02.atm.binero.net (Halon) with ESMTPA id 3f242e72-f92a-11e9-903a-005056917f90; Mon, 28 Oct 2019 03:25:45 +0100 (CET) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org Date: Mon, 28 Oct 2019 03:25:15 +0100 Message-Id: <20191028022525.796995-3-niklas.soderlund@ragnatech.se> X-Mailer: git-send-email 2.23.0 In-Reply-To: <20191028022525.796995-1-niklas.soderlund@ragnatech.se> References: <20191028022525.796995-1-niklas.soderlund@ragnatech.se> MIME-Version: 1.0 Subject: [libcamera-devel] [RFC 02/12] libcamera: pipelines: Explicitly allocate streams X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 28 Oct 2019 02:25:50 -0000 Prepare for sub-classing the Stream class with a V4L2 specific implementation which will need to be constructed later when knowledge about the V4L2 video device is available. Signed-off-by: Niklas Söderlund Reviewed-by: Jacopo Mondi --- src/libcamera/pipeline/ipu3/ipu3.cpp | 60 +++++++++++++----------- src/libcamera/pipeline/rkisp1/rkisp1.cpp | 19 ++++---- src/libcamera/pipeline/uvcvideo.cpp | 13 +++-- src/libcamera/pipeline/vimc.cpp | 14 ++++-- 4 files changed, 60 insertions(+), 46 deletions(-) diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp index 06ee6ccac641eb41..ff90b729e558c3a3 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -137,8 +137,8 @@ public: class IPU3Stream : public Stream { public: - IPU3Stream() - : active_(false), device_(nullptr) + IPU3Stream(ImgUDevice::ImgUOutput *device, const std::string &name) + : active_(false), name_(name), device_(device) { } @@ -151,10 +151,16 @@ class IPU3CameraData : public CameraData { public: IPU3CameraData(PipelineHandler *pipe) - : CameraData(pipe) + : CameraData(pipe), outStream_(nullptr), vfStream_(nullptr) { } + ~IPU3CameraData() + { + delete outStream_; + delete vfStream_; + } + void imguOutputBufferReady(Buffer *buffer); void imguInputBufferReady(Buffer *buffer); void cio2BufferReady(Buffer *buffer); @@ -162,8 +168,8 @@ public: CIO2Device cio2_; ImgUDevice *imgu_; - IPU3Stream outStream_; - IPU3Stream vfStream_; + IPU3Stream *outStream_; + IPU3Stream *vfStream_; std::vector> rawBuffers_; }; @@ -342,8 +348,8 @@ CameraConfiguration::Status IPU3CameraConfiguration::validate() * stream otherwise. */ std::set availableStreams = { - &data_->outStream_, - &data_->vfStream_, + data_->outStream_, + data_->vfStream_, }; streams_.clear(); @@ -356,9 +362,9 @@ CameraConfiguration::Status IPU3CameraConfiguration::validate() const IPU3Stream *stream; if (cfg.size == sensorFormat_.size) - stream = &data_->outStream_; + stream = data_->outStream_; else - stream = &data_->vfStream_; + stream = data_->vfStream_; if (availableStreams.find(stream) == availableStreams.end()) stream = *availableStreams.begin(); @@ -366,7 +372,7 @@ CameraConfiguration::Status IPU3CameraConfiguration::validate() LOG(IPU3, Debug) << "Assigned '" << stream->name_ << "' to stream " << i; - bool scale = stream == &data_->vfStream_; + bool scale = stream == data_->vfStream_; adjustStream(config_[i], scale); if (cfg.pixelFormat != pixelFormat || cfg.size != size) { @@ -394,8 +400,8 @@ CameraConfiguration *PipelineHandlerIPU3::generateConfiguration(Camera *camera, IPU3CameraData *data = cameraData(camera); IPU3CameraConfiguration *config; std::set streams = { - &data->outStream_, - &data->vfStream_, + data->outStream_, + data->vfStream_, }; config = new IPU3CameraConfiguration(camera, data); @@ -413,10 +419,10 @@ CameraConfiguration *PipelineHandlerIPU3::generateConfiguration(Camera *camera, * and VideoRecording roles are not allowed on * the output stream. */ - if (streams.find(&data->outStream_) != streams.end()) { - stream = &data->outStream_; - } else if (streams.find(&data->vfStream_) != streams.end()) { - stream = &data->vfStream_; + if (streams.find(data->outStream_) != streams.end()) { + stream = data->outStream_; + } else if (streams.find(data->vfStream_) != streams.end()) { + stream = data->vfStream_; } else { LOG(IPU3, Error) << "No stream available for requested role " @@ -446,14 +452,14 @@ CameraConfiguration *PipelineHandlerIPU3::generateConfiguration(Camera *camera, * \todo This is an artificial limitation until we * figure out the exact capabilities of the hardware. */ - if (streams.find(&data->vfStream_) == streams.end()) { + if (streams.find(data->vfStream_) == streams.end()) { LOG(IPU3, Error) << "No stream available for requested role " << role; break; } - stream = &data->vfStream_; + stream = data->vfStream_; /* * Align the default viewfinder size to the maximum @@ -494,8 +500,8 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c) IPU3CameraConfiguration *config = static_cast(c); IPU3CameraData *data = cameraData(camera); - IPU3Stream *outStream = &data->outStream_; - IPU3Stream *vfStream = &data->vfStream_; + IPU3Stream *outStream = data->outStream_; + IPU3Stream *vfStream = data->vfStream_; CIO2Device *cio2 = &data->cio2_; ImgUDevice *imgu = data->imgu_; int ret; @@ -629,8 +635,8 @@ int PipelineHandlerIPU3::allocateBuffers(Camera *camera, const std::set &streams) { IPU3CameraData *data = cameraData(camera); - IPU3Stream *outStream = &data->outStream_; - IPU3Stream *vfStream = &data->vfStream_; + IPU3Stream *outStream = data->outStream_; + IPU3Stream *vfStream = data->vfStream_; CIO2Device *cio2 = &data->cio2_; ImgUDevice *imgu = data->imgu_; unsigned int bufferCount; @@ -858,8 +864,8 @@ int PipelineHandlerIPU3::registerCameras() std::unique_ptr data = utils::make_unique(this); std::set streams = { - &data->outStream_, - &data->vfStream_, + data->outStream_, + data->vfStream_, }; CIO2Device *cio2 = &data->cio2_; @@ -874,10 +880,8 @@ int PipelineHandlerIPU3::registerCameras() * second. */ data->imgu_ = numCameras ? &imgu1_ : &imgu0_; - data->outStream_.device_ = &data->imgu_->output_; - data->outStream_.name_ = "output"; - data->vfStream_.device_ = &data->imgu_->viewfinder_; - data->vfStream_.name_ = "viewfinder"; + data->outStream_ = new IPU3Stream(&data->imgu_->output_, "output"); + data->vfStream_ = new IPU3Stream(&data->imgu_->viewfinder_, "viewfinder"); /* * Connect video devices' 'bufferReady' signals to their diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index a99df4f8b846d8b7..cefdb54a06d440fb 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -116,19 +116,20 @@ class RkISP1CameraData : public CameraData { public: RkISP1CameraData(PipelineHandler *pipe) - : CameraData(pipe), sensor_(nullptr), frame_(0), - frameInfo_(pipe) + : CameraData(pipe), stream_(nullptr), sensor_(nullptr), + frame_(0), frameInfo_(pipe) { } ~RkISP1CameraData() { + delete stream_; delete sensor_; } int loadIPA(); - Stream stream_; + Stream *stream_; CameraSensor *sensor_; unsigned int frame_; std::vector ipaBuffers_; @@ -650,7 +651,7 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c) if (ret) return ret; - cfg.setStream(&data->stream_); + cfg.setStream(data->stream_); return 0; } @@ -772,8 +773,8 @@ int PipelineHandlerRkISP1::start(Camera *camera) /* Inform IPA of stream configuration and sensor controls. */ std::map streamConfig; streamConfig[0] = { - .pixelFormat = data->stream_.configuration().pixelFormat, - .size = data->stream_.configuration().size, + .pixelFormat = data->stream_->configuration().pixelFormat, + .size = data->stream_->configuration().size, }; std::map entityControls; @@ -812,7 +813,7 @@ void PipelineHandlerRkISP1::stop(Camera *camera) int PipelineHandlerRkISP1::queueRequest(Camera *camera, Request *request) { RkISP1CameraData *data = cameraData(camera); - Stream *stream = &data->stream_; + Stream *stream = data->stream_; RkISP1FrameInfo *info = data->frameInfo_.create(data->frame_, request, stream); @@ -873,6 +874,8 @@ int PipelineHandlerRkISP1::createCamera(MediaEntity *sensor) std::unique_ptr data = utils::make_unique(this); + data->stream_ = new Stream(); + ControlInfoMap::Map ctrls; ctrls.emplace(std::piecewise_construct, std::forward_as_tuple(&controls::AeEnable), @@ -889,7 +892,7 @@ int PipelineHandlerRkISP1::createCamera(MediaEntity *sensor) if (ret) return ret; - std::set streams{ &data->stream_ }; + std::set streams{ data->stream_ }; std::shared_ptr camera = Camera::create(this, sensor->name(), streams); registerCamera(std::move(camera), std::move(data)); diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp index dc17b456af6987e6..5aa6a8f9e0b2b7a4 100644 --- a/src/libcamera/pipeline/uvcvideo.cpp +++ b/src/libcamera/pipeline/uvcvideo.cpp @@ -31,12 +31,13 @@ class UVCCameraData : public CameraData { public: UVCCameraData(PipelineHandler *pipe) - : CameraData(pipe), video_(nullptr) + : CameraData(pipe), video_(nullptr), stream_(nullptr) { } ~UVCCameraData() { + delete stream_; delete video_; } @@ -44,7 +45,7 @@ public: void bufferReady(Buffer *buffer); V4L2VideoDevice *video_; - Stream stream_; + Stream *stream_; }; class UVCCameraConfiguration : public CameraConfiguration @@ -187,7 +188,7 @@ int PipelineHandlerUVC::configure(Camera *camera, CameraConfiguration *config) format.fourcc != cfg.pixelFormat) return -EINVAL; - cfg.setStream(&data->stream_); + cfg.setStream(data->stream_); return 0; } @@ -265,7 +266,7 @@ int PipelineHandlerUVC::processControls(UVCCameraData *data, Request *request) int PipelineHandlerUVC::queueRequest(Camera *camera, Request *request) { UVCCameraData *data = cameraData(camera); - Buffer *buffer = request->findBuffer(&data->stream_); + Buffer *buffer = request->findBuffer(data->stream_); if (!buffer) { LOG(UVC, Error) << "Attempt to queue request with invalid stream"; @@ -310,7 +311,7 @@ bool PipelineHandlerUVC::match(DeviceEnumerator *enumerator) } /* Create and register the camera. */ - std::set streams{ &data->stream_ }; + std::set streams{ data->stream_ }; std::shared_ptr camera = Camera::create(this, media->model(), streams); registerCamera(std::move(camera), std::move(data)); @@ -330,6 +331,8 @@ int UVCCameraData::init(MediaEntity *entity) if (ret) return ret; + stream_ = new Stream(); + video_->bufferReady.connect(this, &UVCCameraData::bufferReady); /* Initialise the supported controls. */ diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp index 5f83ae2b85997828..49b850cf0153020f 100644 --- a/src/libcamera/pipeline/vimc.cpp +++ b/src/libcamera/pipeline/vimc.cpp @@ -40,7 +40,8 @@ class VimcCameraData : public CameraData public: VimcCameraData(PipelineHandler *pipe) : CameraData(pipe), sensor_(nullptr), debayer_(nullptr), - scaler_(nullptr), video_(nullptr), raw_(nullptr) + scaler_(nullptr), video_(nullptr), raw_(nullptr), + stream_(nullptr) { } @@ -51,6 +52,7 @@ public: delete scaler_; delete video_; delete raw_; + delete stream_; } int init(MediaDevice *media); @@ -61,7 +63,7 @@ public: V4L2Subdevice *scaler_; V4L2VideoDevice *video_; V4L2VideoDevice *raw_; - Stream stream_; + Stream *stream_; }; class VimcCameraConfiguration : public CameraConfiguration @@ -253,7 +255,7 @@ int PipelineHandlerVimc::configure(Camera *camera, CameraConfiguration *config) if (ret) return ret; - cfg.setStream(&data->stream_); + cfg.setStream(data->stream_); return 0; } @@ -325,7 +327,7 @@ int PipelineHandlerVimc::processControls(VimcCameraData *data, Request *request) int PipelineHandlerVimc::queueRequest(Camera *camera, Request *request) { VimcCameraData *data = cameraData(camera); - Buffer *buffer = request->findBuffer(&data->stream_); + Buffer *buffer = request->findBuffer(data->stream_); if (!buffer) { LOG(VIMC, Error) << "Attempt to queue request with invalid stream"; @@ -375,7 +377,7 @@ bool PipelineHandlerVimc::match(DeviceEnumerator *enumerator) return false; /* Create and register the camera. */ - std::set streams{ &data->stream_ }; + std::set streams{ data->stream_ }; std::shared_ptr camera = Camera::create(this, "VIMC Sensor B", streams); registerCamera(std::move(camera), std::move(data)); @@ -417,6 +419,8 @@ int VimcCameraData::init(MediaDevice *media) if (video_->open()) return -ENODEV; + stream_ = new Stream(); + video_->bufferReady.connect(this, &VimcCameraData::bufferReady); raw_ = new V4L2VideoDevice(media->getEntityByName("Raw Capture 1")); From patchwork Mon Oct 28 02:25:16 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Niklas_S=C3=B6derlund?= X-Patchwork-Id: 2263 Return-Path: Received: from bin-mail-out-05.binero.net (bin-mail-out-05.binero.net [195.74.38.228]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 3073D6017C for ; Mon, 28 Oct 2019 03:25:51 +0100 (CET) X-Halon-ID: 400e3bcd-f92a-11e9-903a-005056917f90 Authorized-sender: niklas@soderlund.pp.se Received: from localhost.localdomain (unknown [93.2.121.143]) by bin-vsp-out-02.atm.binero.net (Halon) with ESMTPA id 400e3bcd-f92a-11e9-903a-005056917f90; Mon, 28 Oct 2019 03:25:47 +0100 (CET) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org Date: Mon, 28 Oct 2019 03:25:16 +0100 Message-Id: <20191028022525.796995-4-niklas.soderlund@ragnatech.se> X-Mailer: git-send-email 2.23.0 In-Reply-To: <20191028022525.796995-1-niklas.soderlund@ragnatech.se> References: <20191028022525.796995-1-niklas.soderlund@ragnatech.se> MIME-Version: 1.0 Subject: [libcamera-devel] [RFC 03/12] libcamera: pipeline: Add helper to find request from buffer X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 28 Oct 2019 02:25:51 -0000 The pipeline knows which buffer coming from the application belongs to which request. Add a helper to allow pipeline implementations to access this knowledge. Signed-off-by: Niklas Söderlund --- src/libcamera/include/pipeline_handler.h | 2 ++ src/libcamera/pipeline_handler.cpp | 10 ++++++++++ 2 files changed, 12 insertions(+) diff --git a/src/libcamera/include/pipeline_handler.h b/src/libcamera/include/pipeline_handler.h index 320740746bc6e998..6024357e266c2e2b 100644 --- a/src/libcamera/include/pipeline_handler.h +++ b/src/libcamera/include/pipeline_handler.h @@ -39,6 +39,8 @@ public: } virtual ~CameraData() {} + Request *requestFromBuffer(Buffer *buffer); + Camera *camera_; PipelineHandler *pipe_; std::list queuedRequests_; diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp index f9ae767d529d44d9..d70e286661aded8e 100644 --- a/src/libcamera/pipeline_handler.cpp +++ b/src/libcamera/pipeline_handler.cpp @@ -58,6 +58,16 @@ LOG_DEFINE_CATEGORY(Pipeline) * exists. */ +Request *CameraData::requestFromBuffer(Buffer *buffer) +{ + for (Request *request : queuedRequests_) + for (const auto &it : request->buffers()) + if (it.second == buffer) + return request; + + return nullptr; +} + /** * \var CameraData::camera_ * \brief The camera related to this CameraData instance From patchwork Mon Oct 28 02:25:17 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Niklas_S=C3=B6derlund?= X-Patchwork-Id: 2264 Return-Path: Received: from bin-mail-out-05.binero.net (bin-mail-out-05.binero.net [195.74.38.228]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 894B661506 for ; Mon, 28 Oct 2019 03:25:51 +0100 (CET) X-Halon-ID: 408b182b-f92a-11e9-903a-005056917f90 Authorized-sender: niklas@soderlund.pp.se Received: from localhost.localdomain (unknown [93.2.121.143]) by bin-vsp-out-02.atm.binero.net (Halon) with ESMTPA id 408b182b-f92a-11e9-903a-005056917f90; Mon, 28 Oct 2019 03:25:48 +0100 (CET) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org Date: Mon, 28 Oct 2019 03:25:17 +0100 Message-Id: <20191028022525.796995-5-niklas.soderlund@ragnatech.se> X-Mailer: git-send-email 2.23.0 In-Reply-To: <20191028022525.796995-1-niklas.soderlund@ragnatech.se> References: <20191028022525.796995-1-niklas.soderlund@ragnatech.se> MIME-Version: 1.0 Subject: [libcamera-devel] [RFC 04/12] libcamera: pipelines: Switch to helper to resolve request from buffer X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 28 Oct 2019 02:25:51 -0000 Use the pipeline helper instead of the soon to be removed request recorded in the buffer. Signed-off-by: Niklas Söderlund Reviewed-by: Laurent Pinchart --- src/libcamera/pipeline/ipu3/ipu3.cpp | 2 +- src/libcamera/pipeline/rkisp1/rkisp1.cpp | 2 +- src/libcamera/pipeline/uvcvideo.cpp | 2 +- src/libcamera/pipeline/vimc.cpp | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp index ff90b729e558c3a3..8aa5f34febf16585 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -948,7 +948,7 @@ void IPU3CameraData::imguInputBufferReady(Buffer *buffer) */ void IPU3CameraData::imguOutputBufferReady(Buffer *buffer) { - Request *request = buffer->request(); + Request *request = requestFromBuffer(buffer); if (!pipe_->completeBuffer(camera_, request, buffer)) /* Request not completed yet, return here. */ diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index cefdb54a06d440fb..0803572754364beb 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -991,7 +991,7 @@ void PipelineHandlerRkISP1::bufferReady(Buffer *buffer) { ASSERT(activeCamera_); RkISP1CameraData *data = cameraData(activeCamera_); - Request *request = buffer->request(); + Request *request = data->requestFromBuffer(buffer); data->timeline_.bufferReady(buffer); diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp index 5aa6a8f9e0b2b7a4..679d82d38227b991 100644 --- a/src/libcamera/pipeline/uvcvideo.cpp +++ b/src/libcamera/pipeline/uvcvideo.cpp @@ -375,7 +375,7 @@ int UVCCameraData::init(MediaEntity *entity) void UVCCameraData::bufferReady(Buffer *buffer) { - Request *request = buffer->request(); + Request *request = requestFromBuffer(buffer); pipe_->completeBuffer(camera_, request, buffer); pipe_->completeRequest(camera_, request); diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp index 49b850cf0153020f..56898716a8cde074 100644 --- a/src/libcamera/pipeline/vimc.cpp +++ b/src/libcamera/pipeline/vimc.cpp @@ -460,7 +460,7 @@ int VimcCameraData::init(MediaDevice *media) void VimcCameraData::bufferReady(Buffer *buffer) { - Request *request = buffer->request(); + Request *request = requestFromBuffer(buffer); pipe_->completeBuffer(camera_, request, buffer); pipe_->completeRequest(camera_, request); From patchwork Mon Oct 28 02:25:18 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Niklas_S=C3=B6derlund?= X-Patchwork-Id: 2266 Return-Path: Received: from vsp-unauthed02.binero.net (vsp-unauthed02.binero.net [195.74.38.227]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 1E5A46150A for ; Mon, 28 Oct 2019 03:25:53 +0100 (CET) X-Halon-ID: 412752f6-f92a-11e9-903a-005056917f90 Authorized-sender: niklas@soderlund.pp.se Received: from localhost.localdomain (unknown [93.2.121.143]) by bin-vsp-out-02.atm.binero.net (Halon) with ESMTPA id 412752f6-f92a-11e9-903a-005056917f90; Mon, 28 Oct 2019 03:25:49 +0100 (CET) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org Date: Mon, 28 Oct 2019 03:25:18 +0100 Message-Id: <20191028022525.796995-6-niklas.soderlund@ragnatech.se> X-Mailer: git-send-email 2.23.0 In-Reply-To: <20191028022525.796995-1-niklas.soderlund@ragnatech.se> References: <20191028022525.796995-1-niklas.soderlund@ragnatech.se> MIME-Version: 1.0 Subject: [libcamera-devel] [RFC 05/12] libcamera: buffer: Remove request tracking X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 28 Oct 2019 02:25:53 -0000 It's no longer needed to track which request the application provided buffer belongs to, drop the helpers for this. Signed-off-by: Niklas Söderlund Reviewed-by: Laurent Pinchart --- include/libcamera/buffer.h | 4 ---- src/libcamera/buffer.cpp | 3 +-- src/libcamera/request.cpp | 3 --- 3 files changed, 1 insertion(+), 9 deletions(-) diff --git a/include/libcamera/buffer.h b/include/libcamera/buffer.h index c349b995e1eb35ee..54c757ef7db8b5f6 100644 --- a/include/libcamera/buffer.h +++ b/include/libcamera/buffer.h @@ -83,7 +83,6 @@ public: unsigned int sequence() const { return sequence_; } Status status() const { return status_; } - Request *request() const { return request_; } Stream *stream() const { return stream_; } private: @@ -94,8 +93,6 @@ private: void cancel(); - void setRequest(Request *request) { request_ = request; } - unsigned int index_; std::array dmabuf_; BufferMemory *mem_; @@ -105,7 +102,6 @@ private: unsigned int sequence_; Status status_; - Request *request_; Stream *stream_; }; diff --git a/src/libcamera/buffer.cpp b/src/libcamera/buffer.cpp index 4407201bd81c368d..10b16a862393b536 100644 --- a/src/libcamera/buffer.cpp +++ b/src/libcamera/buffer.cpp @@ -270,8 +270,7 @@ void BufferPool::destroyBuffers() */ Buffer::Buffer(unsigned int index, const Buffer *metadata) : index_(index), dmabuf_({ -1, -1, -1 }), - status_(Buffer::BufferSuccess), request_(nullptr), - stream_(nullptr) + status_(Buffer::BufferSuccess), stream_(nullptr) { if (metadata) { bytesused_ = metadata->bytesused_; diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp index c14ed1a4d3ce55d0..a9468ed4b0512a7f 100644 --- a/src/libcamera/request.cpp +++ b/src/libcamera/request.cpp @@ -220,7 +220,6 @@ int Request::prepare() for (auto const &pair : bufferMap_) { Buffer *buffer = pair.second; - buffer->setRequest(this); pending_.insert(buffer); } @@ -258,8 +257,6 @@ bool Request::completeBuffer(Buffer *buffer) int ret = pending_.erase(buffer); ASSERT(ret == 1); - buffer->setRequest(nullptr); - if (buffer->status() == Buffer::BufferCancelled) cancelled_ = true; From patchwork Mon Oct 28 02:25:19 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Niklas_S=C3=B6derlund?= X-Patchwork-Id: 2265 Return-Path: Received: from vsp-unauthed02.binero.net (vsp-unauthed02.binero.net [195.74.38.227]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 1AA3F61509 for ; Mon, 28 Oct 2019 03:25:53 +0100 (CET) X-Halon-ID: 41b3fa67-f92a-11e9-903a-005056917f90 Authorized-sender: niklas@soderlund.pp.se Received: from localhost.localdomain (unknown [93.2.121.143]) by bin-vsp-out-02.atm.binero.net (Halon) with ESMTPA id 41b3fa67-f92a-11e9-903a-005056917f90; Mon, 28 Oct 2019 03:25:50 +0100 (CET) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org Date: Mon, 28 Oct 2019 03:25:19 +0100 Message-Id: <20191028022525.796995-7-niklas.soderlund@ragnatech.se> X-Mailer: git-send-email 2.23.0 In-Reply-To: <20191028022525.796995-1-niklas.soderlund@ragnatech.se> References: <20191028022525.796995-1-niklas.soderlund@ragnatech.se> MIME-Version: 1.0 Subject: [libcamera-devel] [RFC 06/12] libcamera: stream: Add prototypes for new interface X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 28 Oct 2019 02:25:53 -0000 The buffer allocation rework will remove most of the methods in the Stream class. The methods who deals with buffers will be virtual and implemented in a subclass. This change adds the prototypes for the new interface with an empty default implementation. Once the new buffer allocation work is completed the two added prototypes here will be turned into pure virtual functions preventing the base Stream class from being instantiated. Signed-off-by: Niklas Söderlund --- include/libcamera/stream.h | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/include/libcamera/stream.h b/include/libcamera/stream.h index 2e619cdf0e89bbc7..dac4831cfa1a9b1d 100644 --- a/include/libcamera/stream.h +++ b/include/libcamera/stream.h @@ -73,6 +73,7 @@ class Stream { public: Stream(); + virtual ~Stream(){}; std::unique_ptr createBuffer(unsigned int index); std::unique_ptr createBuffer(const std::array &fds); @@ -85,6 +86,9 @@ public: protected: friend class Camera; + virtual int allocateBuffers(std::vector *buffers) { return -EINVAL; } + virtual int importBuffers(bool enable) { return -EINVAL; } + int mapBuffer(const Buffer *buffer); void unmapBuffer(const Buffer *buffer); From patchwork Mon Oct 28 02:25:20 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Niklas_S=C3=B6derlund?= X-Patchwork-Id: 2267 Return-Path: Received: from bin-mail-out-05.binero.net (bin-mail-out-05.binero.net [195.74.38.228]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id AE3AE61515 for ; Mon, 28 Oct 2019 03:25:54 +0100 (CET) X-Halon-ID: 4240a8b0-f92a-11e9-903a-005056917f90 Authorized-sender: niklas@soderlund.pp.se Received: from localhost.localdomain (unknown [93.2.121.143]) by bin-vsp-out-02.atm.binero.net (Halon) with ESMTPA id 4240a8b0-f92a-11e9-903a-005056917f90; Mon, 28 Oct 2019 03:25:50 +0100 (CET) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org Date: Mon, 28 Oct 2019 03:25:20 +0100 Message-Id: <20191028022525.796995-8-niklas.soderlund@ragnatech.se> X-Mailer: git-send-email 2.23.0 In-Reply-To: <20191028022525.796995-1-niklas.soderlund@ragnatech.se> References: <20191028022525.796995-1-niklas.soderlund@ragnatech.se> MIME-Version: 1.0 Subject: [libcamera-devel] [RFC 07/12] libcamera: buffer: Add dedicated container for buffer information X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 28 Oct 2019 02:25:55 -0000 The Buffer object will be split in two, one containing the memory and one containing the information recorded from when the buffer is dequeued. Add a container for the later in preparation for the split. Signed-off-by: Niklas Söderlund --- include/libcamera/buffer.h | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/include/libcamera/buffer.h b/include/libcamera/buffer.h index 54c757ef7db8b5f6..c626f669040b3c04 100644 --- a/include/libcamera/buffer.h +++ b/include/libcamera/buffer.h @@ -105,6 +105,40 @@ private: Stream *stream_; }; +class BufferInfo +{ +public: + enum Status { + BufferSuccess, + BufferError, + BufferCancelled, + }; + + struct PlaneInfo { + unsigned int bytesused; + }; + + BufferInfo(Status status, unsigned int sequence, uint64_t timestamp, + const std::vector &planes) + : status_(status), sequence_(sequence), timestamp_(timestamp), + planes_(planes) + { + } + + Status status() const { return status_; } + unsigned int sequence() const { return sequence_; } + unsigned int timestamp() const { return timestamp_; } + + unsigned int planes() const { return planes_.size(); } + const PlaneInfo &plane(unsigned int plane) const { return planes_.at(plane); } + +private: + Status status_; + unsigned int sequence_; + uint64_t timestamp_; + std::vector planes_; +}; + } /* namespace libcamera */ #endif /* __LIBCAMERA_BUFFER_H__ */ From patchwork Mon Oct 28 02:25:21 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Niklas_S=C3=B6derlund?= X-Patchwork-Id: 2268 Return-Path: Received: from vsp-unauthed02.binero.net (vsp-unauthed02.binero.net [195.74.38.227]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 298816017C for ; Mon, 28 Oct 2019 03:25:55 +0100 (CET) X-Halon-ID: 42bdd136-f92a-11e9-903a-005056917f90 Authorized-sender: niklas@soderlund.pp.se Received: from localhost.localdomain (unknown [93.2.121.143]) by bin-vsp-out-02.atm.binero.net (Halon) with ESMTPA id 42bdd136-f92a-11e9-903a-005056917f90; Mon, 28 Oct 2019 03:25:51 +0100 (CET) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org Date: Mon, 28 Oct 2019 03:25:21 +0100 Message-Id: <20191028022525.796995-9-niklas.soderlund@ragnatech.se> X-Mailer: git-send-email 2.23.0 In-Reply-To: <20191028022525.796995-1-niklas.soderlund@ragnatech.se> References: <20191028022525.796995-1-niklas.soderlund@ragnatech.se> MIME-Version: 1.0 Subject: [libcamera-devel] [RFC 08/12] libcamera: buffer: Add a buffer allocator X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 28 Oct 2019 02:25:55 -0000 >From an applications point of view buffers shall not be allocated directly by a camera or stream. Instead they shall be allocated externally and used by a camera. To model this behavior add a buffer allocator that is exposed to applications. How the allocator creates buffers is pipeline specific and handled by sub-classing the stream class. Signed-off-by: Niklas Söderlund --- include/libcamera/buffer.h | 17 ++++++++++++++ include/libcamera/stream.h | 1 + src/libcamera/buffer.cpp | 48 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 66 insertions(+) diff --git a/include/libcamera/buffer.h b/include/libcamera/buffer.h index c626f669040b3c04..adb642ad5da072d2 100644 --- a/include/libcamera/buffer.h +++ b/include/libcamera/buffer.h @@ -8,11 +8,14 @@ #define __LIBCAMERA_BUFFER_H__ #include +#include +#include #include #include namespace libcamera { +class Camera; class Request; class Stream; @@ -139,6 +142,20 @@ private: std::vector planes_; }; +class BufferAllocator +{ +public: + BufferAllocator(std::shared_ptr camera); + ~BufferAllocator(); + + int allocate(Stream *stream); + std::vector get(Stream *stream); + +private: + std::shared_ptr camera_; + std::map> allocated_; +}; + } /* namespace libcamera */ #endif /* __LIBCAMERA_BUFFER_H__ */ diff --git a/include/libcamera/stream.h b/include/libcamera/stream.h index dac4831cfa1a9b1d..b051341511a7ab7c 100644 --- a/include/libcamera/stream.h +++ b/include/libcamera/stream.h @@ -84,6 +84,7 @@ public: MemoryType memoryType() const { return memoryType_; } protected: + friend class BufferAllocator; friend class Camera; virtual int allocateBuffers(std::vector *buffers) { return -EINVAL; } diff --git a/src/libcamera/buffer.cpp b/src/libcamera/buffer.cpp index 10b16a862393b536..fce1ce5e49cbbf42 100644 --- a/src/libcamera/buffer.cpp +++ b/src/libcamera/buffer.cpp @@ -12,6 +12,9 @@ #include #include +#include +#include + #include "log.h" /** @@ -393,4 +396,49 @@ void Buffer::cancel() * The intended callers are Request::prepare() and Request::completeBuffer(). */ +BufferAllocator::BufferAllocator(std::shared_ptr camera) + : camera_(camera) +{ +} + +BufferAllocator::~BufferAllocator() +{ + for (auto &it : allocated_) { + Stream *stream = it.first; + std::vector &buffers = it.second; + + for (Buffer *buffer : buffers) + delete buffer; + + buffers.clear(); + + stream->allocateBuffers(nullptr); + } +} + +int BufferAllocator::allocate(Stream *stream) +{ + bool found = false; + + for (const Stream *s : camera_->streams()) { + if (stream == s) { + found = true; + break; + } + } + + if (!found) + return -EINVAL; + + return stream->allocateBuffers(&allocated_[stream]); +} + +std::vector BufferAllocator::get(Stream *stream) +{ + if (allocated_.find(stream) == allocated_.end()) + return {}; + + return allocated_[stream]; +} + } /* namespace libcamera */ From patchwork Mon Oct 28 02:25:22 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Niklas_S=C3=B6derlund?= X-Patchwork-Id: 2269 Return-Path: Received: from bin-mail-out-05.binero.net (bin-mail-out-05.binero.net [195.74.38.228]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id AFACB6017E for ; Mon, 28 Oct 2019 03:25:55 +0100 (CET) X-Halon-ID: 434a2b97-f92a-11e9-903a-005056917f90 Authorized-sender: niklas@soderlund.pp.se Received: from localhost.localdomain (unknown [93.2.121.143]) by bin-vsp-out-02.atm.binero.net (Halon) with ESMTPA id 434a2b97-f92a-11e9-903a-005056917f90; Mon, 28 Oct 2019 03:25:52 +0100 (CET) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org Date: Mon, 28 Oct 2019 03:25:22 +0100 Message-Id: <20191028022525.796995-10-niklas.soderlund@ragnatech.se> X-Mailer: git-send-email 2.23.0 In-Reply-To: <20191028022525.796995-1-niklas.soderlund@ragnatech.se> References: <20191028022525.796995-1-niklas.soderlund@ragnatech.se> MIME-Version: 1.0 Subject: [libcamera-devel] [RFC 09/12] libcamera: v4l2_videodevice: Add a buffer cache class X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 28 Oct 2019 02:25:56 -0000 In preparation for the new buffer handling add a class which will deal with keeping the cache between dmafds and V4L2 video device buffer indexes. Previously this responsibility have been split between multiple classes. This initial implement ensures that no hot association is lost while its eviction strategy could be improved in the future. Signed-off-by: Niklas Söderlund --- src/libcamera/include/v4l2_videodevice.h | 19 ++++++++++ src/libcamera/v4l2_videodevice.cpp | 47 ++++++++++++++++++++++++ 2 files changed, 66 insertions(+) diff --git a/src/libcamera/include/v4l2_videodevice.h b/src/libcamera/include/v4l2_videodevice.h index 4b8cf9394eb9516f..5b178339d0ce7e2c 100644 --- a/src/libcamera/include/v4l2_videodevice.h +++ b/src/libcamera/include/v4l2_videodevice.h @@ -104,6 +104,25 @@ struct V4L2Capability final : v4l2_capability { } }; +class V4L2BufferCache +{ +public: + V4L2BufferCache(unsigned int size); + + void populate(unsigned int index, const std::array &fds); + + int pop(const std::array &fds); + void push(unsigned int index); + +private: + struct CacheInfo { + bool free; + std::array last; + }; + + std::map cache_; +}; + class V4L2DeviceFormat { public: diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp index a2a9eab2bcc0d7e8..8bc2e439e4faeb68 100644 --- a/src/libcamera/v4l2_videodevice.cpp +++ b/src/libcamera/v4l2_videodevice.cpp @@ -132,6 +132,53 @@ LOG_DECLARE_CATEGORY(V4L2) * \return True if the video device provides Streaming I/O IOCTLs */ +V4L2BufferCache::V4L2BufferCache(unsigned int size) +{ + for (unsigned int i = 0; i < size; i++) + cache_[i] = { .free = true, .last = { -1, -1, -1 } }; +} + +void V4L2BufferCache::populate(unsigned int index, const std::array &fds) +{ + ASSERT(index < cache_.size()); + cache_[index].last = fds; +} + +int V4L2BufferCache::pop(const std::array &fds) +{ + int use = -1; + + for (auto &it : cache_) { + int index = it.first; + CacheInfo &info = it.second; + + if (!info.free) + continue; + + if (use < 0) + use = index; + + if (info.last == fds) { + use = index; + break; + } + } + + if (use < 0) + return -ENOENT; + + cache_[use].free = false; + cache_[use].last = fds; + + return use; +} + +void V4L2BufferCache::push(unsigned int index) +{ + ASSERT(index < cache_.size()); + cache_[index].free = true; +} + /** * \class V4L2DeviceFormat * \brief The V4L2 video device image format and sizes From patchwork Mon Oct 28 02:25:23 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Niklas_S=C3=B6derlund?= X-Patchwork-Id: 2270 Return-Path: Received: from vsp-unauthed02.binero.net (vsp-unauthed02.binero.net [195.74.38.227]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 50EF36017E for ; Mon, 28 Oct 2019 03:25:58 +0100 (CET) X-Halon-ID: 43d81398-f92a-11e9-903a-005056917f90 Authorized-sender: niklas@soderlund.pp.se Received: from localhost.localdomain (unknown [93.2.121.143]) by bin-vsp-out-02.atm.binero.net (Halon) with ESMTPA id 43d81398-f92a-11e9-903a-005056917f90; Mon, 28 Oct 2019 03:25:53 +0100 (CET) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org Date: Mon, 28 Oct 2019 03:25:23 +0100 Message-Id: <20191028022525.796995-11-niklas.soderlund@ragnatech.se> X-Mailer: git-send-email 2.23.0 In-Reply-To: <20191028022525.796995-1-niklas.soderlund@ragnatech.se> References: <20191028022525.796995-1-niklas.soderlund@ragnatech.se> MIME-Version: 1.0 Subject: [libcamera-devel] [RFC 10/12] libcamera: buffer: Store buffer information in separate container X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 28 Oct 2019 02:25:58 -0000 Use the new BufferInfo container to store information from when a buffer is dequeued. This will aid in the ongoing buffer rework and simplify the code. This commits breaks the buffer sharing test case as it only deals with buffer information going out of the video device. The next patch will restore the test as it will address the incoming information. All other tests as well as cam and qcam works as expected on pipelines not require buffer importing. Signed-off-by: Niklas Söderlund --- include/libcamera/request.h | 6 ++- src/cam/buffer_writer.cpp | 5 ++- src/cam/buffer_writer.h | 4 +- src/cam/capture.cpp | 21 ++++----- src/libcamera/include/pipeline_handler.h | 3 +- src/libcamera/include/v4l2_videodevice.h | 4 +- src/libcamera/pipeline/ipu3/ipu3.cpp | 20 ++++----- src/libcamera/pipeline/rkisp1/rkisp1.cpp | 40 ++++++++--------- src/libcamera/pipeline/uvcvideo.cpp | 6 +-- src/libcamera/pipeline/vimc.cpp | 6 +-- src/libcamera/pipeline_handler.cpp | 5 ++- src/libcamera/request.cpp | 6 ++- src/libcamera/v4l2_videodevice.cpp | 55 ++++++++---------------- src/qcam/main_window.cpp | 18 ++++---- src/qcam/main_window.h | 2 +- test/camera/capture.cpp | 4 +- test/v4l2_videodevice/buffer_sharing.cpp | 17 ++++---- test/v4l2_videodevice/capture_async.cpp | 4 +- test/v4l2_videodevice/v4l2_m2mdevice.cpp | 8 ++-- 19 files changed, 114 insertions(+), 120 deletions(-) diff --git a/include/libcamera/request.h b/include/libcamera/request.h index 2d5a5964e99eb75f..88ef7bf03fcfb77b 100644 --- a/include/libcamera/request.h +++ b/include/libcamera/request.h @@ -12,12 +12,12 @@ #include #include +#include #include #include namespace libcamera { -class Buffer; class Camera; class CameraControlValidator; class Stream; @@ -39,6 +39,7 @@ public: ControlList &controls() { return *controls_; } ControlList &metadata() { return *metadata_; } const std::map &buffers() const { return bufferMap_; } + const BufferInfo &info(Buffer *frame) const { return info_.find(frame)->second; }; int addBuffer(std::unique_ptr buffer); Buffer *findBuffer(Stream *stream) const; @@ -54,13 +55,14 @@ private: int prepare(); void complete(); - bool completeBuffer(Buffer *buffer); + bool completeBuffer(Buffer *buffer, const BufferInfo &info); Camera *camera_; CameraControlValidator *validator_; ControlList *controls_; ControlList *metadata_; std::map bufferMap_; + std::map info_; std::unordered_set pending_; const uint64_t cookie_; diff --git a/src/cam/buffer_writer.cpp b/src/cam/buffer_writer.cpp index c33e99c5f8173db8..3ee9e82ba216abb6 100644 --- a/src/cam/buffer_writer.cpp +++ b/src/cam/buffer_writer.cpp @@ -21,7 +21,8 @@ BufferWriter::BufferWriter(const std::string &pattern) { } -int BufferWriter::write(Buffer *buffer, const std::string &streamName) +int BufferWriter::write(Buffer *buffer, const BufferInfo &info, + const std::string &streamName) { std::string filename; size_t pos; @@ -32,7 +33,7 @@ int BufferWriter::write(Buffer *buffer, const std::string &streamName) if (pos != std::string::npos) { std::stringstream ss; ss << streamName << "-" << std::setw(6) - << std::setfill('0') << buffer->sequence(); + << std::setfill('0') << info.sequence(); filename.replace(pos, 1, ss.str()); } diff --git a/src/cam/buffer_writer.h b/src/cam/buffer_writer.h index 7bf785d1e83235ff..38f7045c7d4e68a4 100644 --- a/src/cam/buffer_writer.h +++ b/src/cam/buffer_writer.h @@ -16,7 +16,9 @@ class BufferWriter public: BufferWriter(const std::string &pattern = "frame-#.bin"); - int write(libcamera::Buffer *buffer, const std::string &streamName); + int write(libcamera::Buffer *buffer, + const libcamera::BufferInfo &info, + const std::string &streamName); private: std::string pattern_; diff --git a/src/cam/capture.cpp b/src/cam/capture.cpp index e665d819fb777a90..251e9f86c86b508d 100644 --- a/src/cam/capture.cpp +++ b/src/cam/capture.cpp @@ -138,32 +138,33 @@ void Capture::requestComplete(Request *request) if (request->status() == Request::RequestCancelled) return; - const std::map &buffers = request->buffers(); - std::chrono::steady_clock::time_point now = std::chrono::steady_clock::now(); double fps = std::chrono::duration_cast(now - last_).count(); fps = last_ != std::chrono::steady_clock::time_point() && fps ? 1000.0 / fps : 0.0; last_ = now; - std::stringstream info; - info << "fps: " << std::fixed << std::setprecision(2) << fps; + std::stringstream infostr; + infostr << "fps: " << std::fixed << std::setprecision(2) << fps; + const std::map &buffers = request->buffers(); for (auto it = buffers.begin(); it != buffers.end(); ++it) { Stream *stream = it->first; Buffer *buffer = it->second; + const BufferInfo &info = request->info(buffer); const std::string &name = streamName_[stream]; - info << " " << name - << " (" << buffer->index() << ")" - << " seq: " << std::setw(6) << std::setfill('0') << buffer->sequence() - << " bytesused: " << buffer->bytesused(); + infostr << " " << name + << " seq: " << std::setw(6) << std::setfill('0') << info.sequence(); + + for (unsigned int i = 0; i < info.planes(); i++) + infostr << " bytesused(" << i << "): " << info.plane(i).bytesused; if (writer_) - writer_->write(buffer, name); + writer_->write(buffer, info, name); } - std::cout << info.str() << std::endl; + std::cout << infostr.str() << std::endl; /* * Create a new request and populate it with one buffer for each diff --git a/src/libcamera/include/pipeline_handler.h b/src/libcamera/include/pipeline_handler.h index 6024357e266c2e2b..e6dbd7687021e4ff 100644 --- a/src/libcamera/include/pipeline_handler.h +++ b/src/libcamera/include/pipeline_handler.h @@ -81,7 +81,8 @@ public: virtual int queueRequest(Camera *camera, Request *request); - bool completeBuffer(Camera *camera, Request *request, Buffer *buffer); + bool completeBuffer(Camera *camera, Request *request, Buffer *buffer, + const BufferInfo &info); void completeRequest(Camera *camera, Request *request); const char *name() const { return name_; } diff --git a/src/libcamera/include/v4l2_videodevice.h b/src/libcamera/include/v4l2_videodevice.h index 5b178339d0ce7e2c..01b90ab4654bfba2 100644 --- a/src/libcamera/include/v4l2_videodevice.h +++ b/src/libcamera/include/v4l2_videodevice.h @@ -12,6 +12,7 @@ #include +#include #include #include @@ -166,7 +167,7 @@ public: int queueBuffer(Buffer *buffer); std::vector> queueAllBuffers(); - Signal bufferReady; + Signal bufferReady; int streamOn(); int streamOff(); @@ -194,7 +195,6 @@ private: int createPlane(BufferMemory *buffer, unsigned int index, unsigned int plane, unsigned int length); - Buffer *dequeueBuffer(); void bufferAvailable(EventNotifier *notifier); V4L2Capability caps_; diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp index 8aa5f34febf16585..01064ac09859155d 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -161,9 +161,9 @@ public: delete vfStream_; } - void imguOutputBufferReady(Buffer *buffer); - void imguInputBufferReady(Buffer *buffer); - void cio2BufferReady(Buffer *buffer); + void imguOutputBufferReady(Buffer *buffer, const BufferInfo &info); + void imguInputBufferReady(Buffer *buffer, const BufferInfo &info); + void cio2BufferReady(Buffer *buffer, const BufferInfo &info); CIO2Device cio2_; ImgUDevice *imgu_; @@ -931,10 +931,10 @@ int PipelineHandlerIPU3::registerCameras() * Buffers completed from the ImgU input are immediately queued back to the * CIO2 unit to continue frame capture. */ -void IPU3CameraData::imguInputBufferReady(Buffer *buffer) +void IPU3CameraData::imguInputBufferReady(Buffer *buffer, const BufferInfo &info) { /* \todo Handle buffer failures when state is set to BufferError. */ - if (buffer->status() == Buffer::BufferCancelled) + if (info.status() == BufferInfo::BufferCancelled) return; cio2_.output_->queueBuffer(buffer); @@ -946,15 +946,13 @@ void IPU3CameraData::imguInputBufferReady(Buffer *buffer) * * Buffers completed from the ImgU output are directed to the application. */ -void IPU3CameraData::imguOutputBufferReady(Buffer *buffer) +void IPU3CameraData::imguOutputBufferReady(Buffer *buffer, const BufferInfo &info) { Request *request = requestFromBuffer(buffer); - if (!pipe_->completeBuffer(camera_, request, buffer)) - /* Request not completed yet, return here. */ + if (!pipe_->completeBuffer(camera_, request, buffer, info)) return; - /* Mark the request as complete. */ pipe_->completeRequest(camera_, request); } @@ -965,10 +963,10 @@ void IPU3CameraData::imguOutputBufferReady(Buffer *buffer) * Buffers completed from the CIO2 are immediately queued to the ImgU unit * for further processing. */ -void IPU3CameraData::cio2BufferReady(Buffer *buffer) +void IPU3CameraData::cio2BufferReady(Buffer *buffer, const BufferInfo &info) { /* \todo Handle buffer failures when state is set to BufferError. */ - if (buffer->status() == Buffer::BufferCancelled) + if (info.status() == BufferInfo::BufferCancelled) return; imgu_->input_->queueBuffer(buffer); diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index 0803572754364beb..33a058de18b8cf2e 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -88,7 +88,7 @@ public: setDelay(QueueBuffers, -1, 10); } - void bufferReady(Buffer *buffer) + void bufferReady(Buffer *buffer, const BufferInfo &info) { /* * Calculate SOE by taking the end of DMA set by the kernel and applying @@ -99,10 +99,10 @@ public: ASSERT(frameOffset(SOE) == 0); utils::time_point soe = std::chrono::time_point() - + std::chrono::nanoseconds(buffer->timestamp()) + + std::chrono::nanoseconds(info.timestamp()) + timeOffset(SOE); - notifyStartOfExposure(buffer->sequence(), soe); + notifyStartOfExposure(info.sequence(), soe); } void setDelay(unsigned int type, int frame, int msdelay) @@ -202,9 +202,9 @@ private: int initLinks(); int createCamera(MediaEntity *sensor); void tryCompleteRequest(Request *request); - void bufferReady(Buffer *buffer); - void paramReady(Buffer *buffer); - void statReady(Buffer *buffer); + void bufferReady(Buffer *buffer, const BufferInfo &info); + void paramReady(Buffer *buffer, const BufferInfo &info); + void statReady(Buffer *buffer, const BufferInfo &info); MediaDevice *media_; V4L2Subdevice *dphy_; @@ -987,43 +987,43 @@ void PipelineHandlerRkISP1::tryCompleteRequest(Request *request) data->frameInfo_.destroy(info->frame); } -void PipelineHandlerRkISP1::bufferReady(Buffer *buffer) +void PipelineHandlerRkISP1::bufferReady(Buffer *buffer, const BufferInfo &info) { ASSERT(activeCamera_); RkISP1CameraData *data = cameraData(activeCamera_); Request *request = data->requestFromBuffer(buffer); - data->timeline_.bufferReady(buffer); + data->timeline_.bufferReady(buffer, info); - if (data->frame_ <= buffer->sequence()) - data->frame_ = buffer->sequence() + 1; + if (data->frame_ <= info.sequence()) + data->frame_ = info.sequence() + 1; - completeBuffer(activeCamera_, request, buffer); + completeBuffer(activeCamera_, request, buffer, info); tryCompleteRequest(request); } -void PipelineHandlerRkISP1::paramReady(Buffer *buffer) +void PipelineHandlerRkISP1::paramReady(Buffer *buffer, const BufferInfo &info) { ASSERT(activeCamera_); RkISP1CameraData *data = cameraData(activeCamera_); - RkISP1FrameInfo *info = data->frameInfo_.find(buffer); + RkISP1FrameInfo *rkinfo = data->frameInfo_.find(buffer); - info->paramDequeued = true; - tryCompleteRequest(info->request); + rkinfo->paramDequeued = true; + tryCompleteRequest(rkinfo->request); } -void PipelineHandlerRkISP1::statReady(Buffer *buffer) +void PipelineHandlerRkISP1::statReady(Buffer *buffer, const BufferInfo &info) { ASSERT(activeCamera_); RkISP1CameraData *data = cameraData(activeCamera_); - RkISP1FrameInfo *info = data->frameInfo_.find(buffer); - if (!info) + RkISP1FrameInfo *rkinfo = data->frameInfo_.find(buffer); + if (!rkinfo) return; - unsigned int frame = info->frame; - unsigned int statid = RKISP1_STAT_BASE | info->statBuffer->index(); + unsigned int frame = rkinfo->frame; + unsigned int statid = RKISP1_STAT_BASE | rkinfo->statBuffer->index(); IPAOperationData op; op.operation = RKISP1_IPA_EVENT_SIGNAL_STAT_BUFFER; diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp index 679d82d38227b991..ef2e8c9734f844ce 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(Buffer *buffer, const BufferInfo &info); V4L2VideoDevice *video_; Stream *stream_; @@ -373,11 +373,11 @@ int UVCCameraData::init(MediaEntity *entity) return 0; } -void UVCCameraData::bufferReady(Buffer *buffer) +void UVCCameraData::bufferReady(Buffer *buffer, const BufferInfo &info) { Request *request = requestFromBuffer(buffer); - pipe_->completeBuffer(camera_, request, buffer); + pipe_->completeBuffer(camera_, request, buffer, info); pipe_->completeRequest(camera_, request); } diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp index 56898716a8cde074..e3eefc49135179f2 100644 --- a/src/libcamera/pipeline/vimc.cpp +++ b/src/libcamera/pipeline/vimc.cpp @@ -56,7 +56,7 @@ public: } int init(MediaDevice *media); - void bufferReady(Buffer *buffer); + void bufferReady(Buffer *buffer, const BufferInfo &info); CameraSensor *sensor_; V4L2Subdevice *debayer_; @@ -458,11 +458,11 @@ int VimcCameraData::init(MediaDevice *media) return 0; } -void VimcCameraData::bufferReady(Buffer *buffer) +void VimcCameraData::bufferReady(Buffer *buffer, const BufferInfo &info) { Request *request = requestFromBuffer(buffer); - pipe_->completeBuffer(camera_, request, buffer); + pipe_->completeBuffer(camera_, request, buffer, info); pipe_->completeRequest(camera_, request); } diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp index d70e286661aded8e..9ce9432449e0e133 100644 --- a/src/libcamera/pipeline_handler.cpp +++ b/src/libcamera/pipeline_handler.cpp @@ -393,10 +393,11 @@ int PipelineHandler::queueRequest(Camera *camera, Request *request) * otherwise */ bool PipelineHandler::completeBuffer(Camera *camera, Request *request, - Buffer *buffer) + Buffer *buffer, const BufferInfo &info) { + bool ret = request->completeBuffer(buffer, info); camera->bufferCompleted.emit(request, buffer); - return request->completeBuffer(buffer); + return ret; } /** diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp index a9468ed4b0512a7f..9a0e439a3d05d780 100644 --- a/src/libcamera/request.cpp +++ b/src/libcamera/request.cpp @@ -252,12 +252,14 @@ void Request::complete() * \return True if all buffers contained in the request have completed, false * otherwise */ -bool Request::completeBuffer(Buffer *buffer) +bool Request::completeBuffer(Buffer *buffer, const BufferInfo &info) { int ret = pending_.erase(buffer); ASSERT(ret == 1); - if (buffer->status() == Buffer::BufferCancelled) + info_.emplace(buffer, info); + + if (info.status() == BufferInfo::BufferCancelled) cancelled_ = true; return !hasPendingBuffers(); diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp index 8bc2e439e4faeb68..97c6722b8c4c98cf 100644 --- a/src/libcamera/v4l2_videodevice.cpp +++ b/src/libcamera/v4l2_videodevice.cpp @@ -1119,14 +1119,16 @@ std::vector> V4L2VideoDevice::queueAllBuffers() } /** - * \brief Dequeue the next available buffer from the video device + * \brief Slot to handle completed buffer events from the V4L2 video device + * \param[in] notifier The event notifier * - * This method dequeues the next available buffer from the device. If no buffer - * is available to be dequeued it will return nullptr immediately. + * When this slot is called, a Buffer has become available from the device, and + * will be emitted through the bufferReady Signal. * - * \return A pointer to the dequeued buffer on success, or nullptr otherwise + * For Capture video devices the Buffer will contain valid data. + * For Output video devices the Buffer can be considered empty. */ -Buffer *V4L2VideoDevice::dequeueBuffer() +void V4L2VideoDevice::bufferAvailable(EventNotifier *notifier) { struct v4l2_buffer buf = {}; struct v4l2_plane planes[VIDEO_MAX_PLANES] = {}; @@ -1144,10 +1146,10 @@ Buffer *V4L2VideoDevice::dequeueBuffer() if (ret < 0) { LOG(V4L2, Error) << "Failed to dequeue buffer: " << strerror(-ret); - return nullptr; + return; } - ASSERT(buf.index < bufferPool_->count()); + LOG(V4L2, Debug) << "Buffer " << buf.index << " is available"; auto it = queuedBuffers_.find(buf.index); Buffer *buffer = it->second; @@ -1156,37 +1158,16 @@ Buffer *V4L2VideoDevice::dequeueBuffer() if (queuedBuffers_.empty()) fdEvent_->setEnabled(false); - buffer->index_ = buf.index; - buffer->bytesused_ = buf.bytesused; - buffer->timestamp_ = buf.timestamp.tv_sec * 1000000000ULL - + buf.timestamp.tv_usec * 1000ULL; - buffer->sequence_ = buf.sequence; - buffer->status_ = buf.flags & V4L2_BUF_FLAG_ERROR - ? Buffer::BufferError : Buffer::BufferSuccess; - - return buffer; -} + BufferInfo::Status status = buf.flags & V4L2_BUF_FLAG_ERROR + ? BufferInfo::BufferError : BufferInfo::BufferSuccess; + uint64_t timestamp = buf.timestamp.tv_sec * 1000000000ULL + + buf.timestamp.tv_usec * 1000ULL; -/** - * \brief Slot to handle completed buffer events from the V4L2 video device - * \param[in] notifier The event notifier - * - * When this slot is called, a Buffer has become available from the device, and - * will be emitted through the bufferReady Signal. - * - * For Capture video devices the Buffer will contain valid data. - * For Output video devices the Buffer can be considered empty. - */ -void V4L2VideoDevice::bufferAvailable(EventNotifier *notifier) -{ - Buffer *buffer = dequeueBuffer(); - if (!buffer) - return; - - LOG(V4L2, Debug) << "Buffer " << buffer->index() << " is available"; + BufferInfo info(status, buf.sequence, timestamp, { { buf.bytesused } }); + buffer->index_ = buf.index; /* Notify anyone listening to the device. */ - bufferReady.emit(buffer); + bufferReady.emit(buffer, info); } /** @@ -1238,9 +1219,11 @@ int V4L2VideoDevice::streamOff() unsigned int index = it.first; Buffer *buffer = it.second; + BufferInfo info(BufferInfo::BufferCancelled, 0, 0, { {} }); + buffer->index_ = index; buffer->cancel(); - bufferReady.emit(buffer); + bufferReady.emit(buffer, info); } queuedBuffers_.clear(); diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp index cca7365ae75687f9..2d7f4ba84fbb6838 100644 --- a/src/qcam/main_window.cpp +++ b/src/qcam/main_window.cpp @@ -258,19 +258,19 @@ void MainWindow::requestComplete(Request *request) framesCaptured_++; Buffer *buffer = buffers.begin()->second; + const BufferInfo &info = request->info(buffer); - double fps = buffer->timestamp() - lastBufferTime_; + double fps = info.timestamp() - lastBufferTime_; fps = lastBufferTime_ && fps ? 1000000000.0 / fps : 0.0; - lastBufferTime_ = buffer->timestamp(); + lastBufferTime_ = info.timestamp(); - std::cout << "seq: " << std::setw(6) << std::setfill('0') << buffer->sequence() - << " buf: " << buffer->index() - << " bytesused: " << buffer->bytesused() - << " timestamp: " << buffer->timestamp() + std::cout << "seq: " << std::setw(6) << std::setfill('0') << info.sequence() + << " bytesused: " << info.plane(0).bytesused + << " timestamp: " << info.timestamp() << " fps: " << std::fixed << std::setprecision(2) << fps << std::endl; - display(buffer); + display(buffer, info); request = camera_->createRequest(); if (!request) { @@ -295,7 +295,7 @@ void MainWindow::requestComplete(Request *request) camera_->queueRequest(request); } -int MainWindow::display(Buffer *buffer) +int MainWindow::display(Buffer *buffer, const BufferInfo &info) { BufferMemory *mem = buffer->mem(); if (mem->planes().size() != 1) @@ -303,7 +303,7 @@ int MainWindow::display(Buffer *buffer) Plane &plane = mem->planes().front(); unsigned char *raw = static_cast(plane.mem()); - viewfinder_->display(raw, buffer->bytesused()); + viewfinder_->display(raw, info.plane(0).bytesused); return 0; } diff --git a/src/qcam/main_window.h b/src/qcam/main_window.h index 78511581a8d5c025..8cbd72eb0d63cbea 100644 --- a/src/qcam/main_window.h +++ b/src/qcam/main_window.h @@ -50,7 +50,7 @@ private: void stopCapture(); void requestComplete(Request *request); - int display(Buffer *buffer); + int display(Buffer *frame, const BufferInfo &info); QString title_; QTimer titleTimer_; diff --git a/test/camera/capture.cpp b/test/camera/capture.cpp index 83f974749affd3cd..3cf5b798448d01db 100644 --- a/test/camera/capture.cpp +++ b/test/camera/capture.cpp @@ -21,7 +21,9 @@ protected: void bufferComplete(Request *request, Buffer *buffer) { - if (buffer->status() != Buffer::BufferSuccess) + const BufferInfo &info = request->info(buffer); + + if (info.status() != BufferInfo::BufferSuccess) return; completeBuffersCount_++; diff --git a/test/v4l2_videodevice/buffer_sharing.cpp b/test/v4l2_videodevice/buffer_sharing.cpp index 1629f34cfa6c79fe..3609c331cf10e056 100644 --- a/test/v4l2_videodevice/buffer_sharing.cpp +++ b/test/v4l2_videodevice/buffer_sharing.cpp @@ -90,24 +90,25 @@ protected: return 0; } - void captureBufferReady(Buffer *buffer) + void captureBufferReady(Buffer *buffer, const BufferInfo &info) { - std::cout << "Received capture buffer: " << buffer->index() - << " sequence " << buffer->sequence() << std::endl; + std::cout << "Received capture buffer sequence " + << info.sequence() << std::endl; - if (buffer->status() != Buffer::BufferSuccess) + if (info.status() != BufferInfo::BufferSuccess) return; + BufferInfo infocopy = info; output_->queueBuffer(buffer); framesCaptured_++; } - void outputBufferReady(Buffer *buffer) + void outputBufferReady(Buffer *buffer, const BufferInfo &info) { - std::cout << "Received output buffer: " << buffer->index() - << " sequence " << buffer->sequence() << std::endl; + std::cout << "Received output buffer sequence " + << info.sequence() << std::endl; - if (buffer->status() != Buffer::BufferSuccess) + if (info.status() != BufferInfo::BufferSuccess) return; capture_->queueBuffer(buffer); diff --git a/test/v4l2_videodevice/capture_async.cpp b/test/v4l2_videodevice/capture_async.cpp index 442a4fe56eace57e..26ea1d17fd901ef8 100644 --- a/test/v4l2_videodevice/capture_async.cpp +++ b/test/v4l2_videodevice/capture_async.cpp @@ -20,9 +20,9 @@ public: CaptureAsyncTest() : V4L2VideoDeviceTest("vimc", "Raw Capture 0"), frames(0) {} - void receiveBuffer(Buffer *buffer) + void receiveBuffer(Buffer *buffer, const BufferInfo &info) { - std::cout << "Received buffer " << buffer->index() << std::endl; + std::cout << "Received buffer" << std::endl; frames++; /* Requeue the buffer for further use. */ diff --git a/test/v4l2_videodevice/v4l2_m2mdevice.cpp b/test/v4l2_videodevice/v4l2_m2mdevice.cpp index 4d3644c2d28792f1..e6ca90a4604dfcda 100644 --- a/test/v4l2_videodevice/v4l2_m2mdevice.cpp +++ b/test/v4l2_videodevice/v4l2_m2mdevice.cpp @@ -29,9 +29,9 @@ public: { } - void outputBufferComplete(Buffer *buffer) + void outputBufferComplete(Buffer *buffer, const BufferInfo &info) { - cout << "Received output buffer " << buffer->index() << endl; + cout << "Received output buffer" << endl; outputFrames_++; @@ -39,9 +39,9 @@ public: vim2m_->output()->queueBuffer(buffer); } - void receiveCaptureBuffer(Buffer *buffer) + void receiveCaptureBuffer(Buffer *buffer, const BufferInfo &info) { - cout << "Received capture buffer " << buffer->index() << endl; + cout << "Received capture buffer" << endl; captureFrames_++; From patchwork Mon Oct 28 02:25:24 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Niklas_S=C3=B6derlund?= X-Patchwork-Id: 2271 Return-Path: Received: from bin-mail-out-05.binero.net (bin-mail-out-05.binero.net [195.74.38.228]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id EB3CF6017C for ; Mon, 28 Oct 2019 03:26:01 +0100 (CET) X-Halon-ID: 44f25a5d-f92a-11e9-903a-005056917f90 Authorized-sender: niklas@soderlund.pp.se Received: from localhost.localdomain (unknown [93.2.121.143]) by bin-vsp-out-02.atm.binero.net (Halon) with ESMTPA id 44f25a5d-f92a-11e9-903a-005056917f90; Mon, 28 Oct 2019 03:25:55 +0100 (CET) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org Date: Mon, 28 Oct 2019 03:25:24 +0100 Message-Id: <20191028022525.796995-12-niklas.soderlund@ragnatech.se> X-Mailer: git-send-email 2.23.0 In-Reply-To: <20191028022525.796995-1-niklas.soderlund@ragnatech.se> References: <20191028022525.796995-1-niklas.soderlund@ragnatech.se> MIME-Version: 1.0 Subject: [libcamera-devel] [RFC 11/12] libcamera: buffer: Switch to new buffer API X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 28 Oct 2019 02:26:02 -0000 Switch to the new buffer API where all buffers are treated as external buffers and are allocated outside the camera. For V4L2 backed cameras a helper class BufferAllocator is provided to help applications create buffers. This patch is quiet large as it need to touch most areas of libcamera to swap the API. It also restores the buffer sharing test and functionality which was broken in the previous change. A follow up change to this one will finalize the transition to the new API by removing code that is left unused after this change. Signed-off-by: Niklas Söderlund --- include/ipa/ipa_interface.h | 2 +- include/libcamera/buffer.h | 47 +-- include/libcamera/request.h | 2 +- src/cam/buffer_writer.cpp | 7 +- src/cam/capture.cpp | 32 ++- src/cam/capture.h | 3 +- src/ipa/rkisp1/rkisp1.cpp | 14 +- src/libcamera/buffer.cpp | 243 +++------------- src/libcamera/camera.cpp | 47 +-- src/libcamera/include/v4l2_videodevice.h | 33 ++- src/libcamera/pipeline/ipu3/ipu3.cpp | 199 ++++--------- src/libcamera/pipeline/rkisp1/rkisp1.cpp | 74 ++--- src/libcamera/pipeline/uvcvideo.cpp | 16 +- src/libcamera/pipeline/vimc.cpp | 16 +- src/libcamera/request.cpp | 15 +- src/libcamera/v4l2_videodevice.cpp | 267 ++++++++++-------- src/qcam/main_window.cpp | 43 ++- src/qcam/main_window.h | 3 + test/camera/capture.cpp | 29 +- test/camera/statemachine.cpp | 12 +- test/v4l2_videodevice/buffer_sharing.cpp | 21 +- test/v4l2_videodevice/capture_async.cpp | 12 +- test/v4l2_videodevice/request_buffers.cpp | 11 +- test/v4l2_videodevice/stream_on_off.cpp | 6 +- test/v4l2_videodevice/v4l2_m2mdevice.cpp | 36 +-- test/v4l2_videodevice/v4l2_videodevice_test.h | 2 +- 26 files changed, 435 insertions(+), 757 deletions(-) diff --git a/include/ipa/ipa_interface.h b/include/ipa/ipa_interface.h index 8fd658af5fdb2e6b..efb86a33f6f427ae 100644 --- a/include/ipa/ipa_interface.h +++ b/include/ipa/ipa_interface.h @@ -26,7 +26,7 @@ struct IPAStream { struct IPABuffer { unsigned int id; - BufferMemory memory; + std::vector> planes; }; struct IPAOperationData { diff --git a/include/libcamera/buffer.h b/include/libcamera/buffer.h index adb642ad5da072d2..3568c2e0cdf8d95b 100644 --- a/include/libcamera/buffer.h +++ b/include/libcamera/buffer.h @@ -22,11 +22,10 @@ class Stream; class Plane final { public: - Plane(); + Plane(int fd, unsigned int length); ~Plane(); - int dmabuf() const { return fd_; } - int setDmabuf(int fd, unsigned int length); + int fd() const { return fd_; } void *mem(); unsigned int length() const { return length_; } @@ -67,45 +66,17 @@ private: class Buffer final { public: - enum Status { - BufferSuccess, - BufferError, - BufferCancelled, - }; - - Buffer(unsigned int index = -1, const Buffer *metadata = nullptr); - Buffer(const Buffer &) = delete; - Buffer &operator=(const Buffer &) = delete; + Buffer(std::vector> planes); + ~Buffer(); - unsigned int index() const { return index_; } - const std::array &dmabufs() const { return dmabuf_; } - BufferMemory *mem() { return mem_; } - - unsigned int bytesused() const { return bytesused_; } - uint64_t timestamp() const { return timestamp_; } - unsigned int sequence() const { return sequence_; } + unsigned int numPlanes() { return planes_.size(); } + Plane *plane(unsigned int plane); - Status status() const { return status_; } - Stream *stream() const { return stream_; } + std::array fds() const; + std::vector> description() const; private: - friend class Camera; - friend class Request; - friend class Stream; - friend class V4L2VideoDevice; - - void cancel(); - - unsigned int index_; - std::array dmabuf_; - BufferMemory *mem_; - - unsigned int bytesused_; - uint64_t timestamp_; - unsigned int sequence_; - - Status status_; - Stream *stream_; + std::vector planes_; }; class BufferInfo diff --git a/include/libcamera/request.h b/include/libcamera/request.h index 88ef7bf03fcfb77b..ac2beab46bea6466 100644 --- a/include/libcamera/request.h +++ b/include/libcamera/request.h @@ -40,7 +40,7 @@ public: ControlList &metadata() { return *metadata_; } const std::map &buffers() const { return bufferMap_; } const BufferInfo &info(Buffer *frame) const { return info_.find(frame)->second; }; - int addBuffer(std::unique_ptr buffer); + int addBuffer(Stream *stream, Buffer *buffer); Buffer *findBuffer(Stream *stream) const; uint64_t cookie() const { return cookie_; } diff --git a/src/cam/buffer_writer.cpp b/src/cam/buffer_writer.cpp index 3ee9e82ba216abb6..c9df851f3ecec056 100644 --- a/src/cam/buffer_writer.cpp +++ b/src/cam/buffer_writer.cpp @@ -43,10 +43,9 @@ int BufferWriter::write(Buffer *buffer, const BufferInfo &info, if (fd == -1) return -errno; - BufferMemory *mem = buffer->mem(); - for (Plane &plane : mem->planes()) { - void *data = plane.mem(); - unsigned int length = plane.length(); + for (unsigned int i = 0; i < buffer->numPlanes(); i++) { + void *data = buffer->plane(i)->mem(); + unsigned int length = buffer->plane(i)->length(); ret = ::write(fd, data, length); if (ret < 0) { diff --git a/src/cam/capture.cpp b/src/cam/capture.cpp index 251e9f86c86b508d..8bd77dcec2d15f1c 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); + + BufferAllocator allocator(camera_); + + ret = capture(loop, allocator); if (options.isSet(OptFile)) { delete writer_; @@ -69,14 +72,21 @@ int Capture::run(EventLoop *loop, const OptionsParser::Options &options) return ret; } -int Capture::capture(EventLoop *loop) +int Capture::capture(EventLoop *loop, BufferAllocator &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; + } + + nbuffers = std::min(nbuffers, static_cast(ret)); + } /* * TODO: make cam tool smarter to support still capture by for @@ -93,9 +103,9 @@ int Capture::capture(EventLoop *loop) for (StreamConfiguration &cfg : *config_) { Stream *stream = cfg.stream(); - std::unique_ptr buffer = stream->createBuffer(i); + Buffer *buffer = allocator.get(stream)[i]; - ret = request->addBuffer(std::move(buffer)); + ret = request->addBuffer(stream, buffer); if (ret < 0) { std::cerr << "Can't set buffer for request" << std::endl; @@ -179,15 +189,7 @@ 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 newBuffer = stream->createBuffer(index); - if (!newBuffer) { - std::cerr << "Can't create buffer " << index << std::endl; - return; - } - - request->addBuffer(std::move(newBuffer)); + request->addBuffer(stream, buffer); } camera_->queueRequest(request); diff --git a/src/cam/capture.h b/src/cam/capture.h index c692d48918f2de1d..ee5adad7cbaaae47 100644 --- a/src/cam/capture.h +++ b/src/cam/capture.h @@ -10,6 +10,7 @@ #include #include +#include #include #include #include @@ -26,7 +27,7 @@ public: int run(EventLoop *loop, const OptionsParser::Options &options); private: - int capture(EventLoop *loop); + int capture(EventLoop *loop, libcamera::BufferAllocator &allocator); void requestComplete(libcamera::Request *request); diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp index 9a13f5c7df17f335..42d3379ce84d600a 100644 --- a/src/ipa/rkisp1/rkisp1.cpp +++ b/src/ipa/rkisp1/rkisp1.cpp @@ -47,7 +47,7 @@ private: void setControls(unsigned int frame); void metadataReady(unsigned int frame, unsigned int aeState); - std::map bufferInfo_; + std::map bufferInfo_; ControlInfoMap ctrls_; @@ -101,15 +101,17 @@ void IPARkISP1::configure(const std::map &streamConfig, void IPARkISP1::mapBuffers(const std::vector &buffers) { for (const IPABuffer &buffer : buffers) { - bufferInfo_[buffer.id] = buffer.memory; - bufferInfo_[buffer.id].planes()[0].mem(); + bufferInfo_[buffer.id] = new Buffer(buffer.planes); + bufferInfo_[buffer.id]->plane(0)->mem(); } } void IPARkISP1::unmapBuffers(const std::vector &ids) { - for (unsigned int id : ids) + for (unsigned int id : ids) { + delete bufferInfo_[id]; bufferInfo_.erase(id); + } } void IPARkISP1::processEvent(const IPAOperationData &event) @@ -120,7 +122,7 @@ void IPARkISP1::processEvent(const IPAOperationData &event) unsigned int bufferId = event.data[1]; const rkisp1_stat_buffer *stats = - static_cast(bufferInfo_[bufferId].planes()[0].mem()); + static_cast(bufferInfo_[bufferId]->plane(0)->mem()); updateStatistics(frame, stats); break; @@ -130,7 +132,7 @@ void IPARkISP1::processEvent(const IPAOperationData &event) unsigned int bufferId = event.data[1]; rkisp1_isp_params_cfg *params = - static_cast(bufferInfo_[bufferId].planes()[0].mem()); + static_cast(bufferInfo_[bufferId]->plane(0)->mem()); queueRequest(frame, params, event.controls[0]); break; diff --git a/src/libcamera/buffer.cpp b/src/libcamera/buffer.cpp index fce1ce5e49cbbf42..d00849520bc2b51c 100644 --- a/src/libcamera/buffer.cpp +++ b/src/libcamera/buffer.cpp @@ -48,69 +48,44 @@ LOG_DEFINE_CATEGORY(Buffer) * an image may or may not be contiguous. */ -Plane::Plane() - : fd_(-1), length_(0), mem_(0) -{ -} - -Plane::~Plane() -{ - munmap(); - - if (fd_ != -1) - close(fd_); -} - -/** - * \fn Plane::dmabuf() - * \brief Get the dmabuf file handle backing the buffer - */ - /** * \brief Set the dmabuf file handle backing the buffer * \param[in] fd The dmabuf file handle * \param[in] length The size of the memory region * - * The \a fd dmabuf file handle is duplicated and stored. The caller may close - * the original file handle. - * - * \return 0 on success or a negative error code otherwise + * The \a fd dmabuf file handle is duplicated and stored. */ -int Plane::setDmabuf(int fd, unsigned int length) +Plane::Plane(int fd, unsigned int length) + : fd_(-1), length_(length), mem_(nullptr) { if (fd < 0) { - LOG(Buffer, Error) << "Invalid dmabuf fd provided"; - return -EINVAL; - } - - if (fd_ != -1) { - close(fd_); - fd_ = -1; + LOG(Buffer, Fatal) << "Invalid dmabuf fd provided"; + return; } fd_ = dup(fd); + if (fd_ == -1) { int ret = -errno; - LOG(Buffer, Error) + LOG(Buffer, Fatal) << "Failed to duplicate dmabuf: " << strerror(-ret); - return ret; + return; } +} - length_ = length; +Plane::~Plane() +{ + munmap(); - return 0; + if (fd_ != -1) + close(fd_); } /** - * \brief Map the plane memory data to a CPU accessible address - * - * The file descriptor to map the memory from must be set by a call to - * setDmaBuf() before calling this function. - * - * \sa setDmaBuf() - * - * \return 0 on success or a negative error code otherwise + * \fn Plane::fd() + * \brief Get the dmabuf file handle backing the buffer */ + int Plane::mmap() { void *map; @@ -131,13 +106,6 @@ int Plane::mmap() return 0; } -/** - * \brief Unmap any existing CPU accessible mapping - * - * Unmap the memory mapped by an earlier call to mmap(). - * - * \return 0 on success or a negative error code otherwise - */ int Plane::munmap() { int ret = 0; @@ -236,166 +204,49 @@ void BufferPool::destroyBuffers() * \return A vector containing all the buffers in the pool. */ -/** - * \class Buffer - * \brief A buffer handle and dynamic metadata - * - * The Buffer class references a buffer memory and associates dynamic metadata - * related to the frame contained in the buffer. It allows referencing buffer - * memory through a single interface regardless of whether the memory is - * allocated internally in libcamera or provided externally through dmabuf. - * - * Buffer instances are allocated dynamically for a stream through - * Stream::createBuffer(), added to a request with Request::addBuffer() and - * deleted automatically after the request complete handler returns. - */ - -/** - * \enum Buffer::Status - * Buffer completion status - * \var Buffer::BufferSuccess - * The buffer has completed with success and contains valid data. All its other - * metadata (such as bytesused(), timestamp() or sequence() number) are valid. - * \var Buffer::BufferError - * The buffer has completed with an error and doesn't contain valid data. Its - * other metadata are valid. - * \var Buffer::BufferCancelled - * The buffer has been cancelled due to capture stop. Its other metadata are - * invalid and shall not be used. - */ - -/** - * \brief Construct a buffer not associated with any stream - * - * This method constructs an orphaned buffer not associated with any stream. It - * is not meant to be called by applications, they should instead create buffers - * for a stream with Stream::createBuffer(). - */ -Buffer::Buffer(unsigned int index, const Buffer *metadata) - : index_(index), dmabuf_({ -1, -1, -1 }), - status_(Buffer::BufferSuccess), stream_(nullptr) +Buffer::Buffer(std::vector> planes) { - if (metadata) { - bytesused_ = metadata->bytesused_; - sequence_ = metadata->sequence_; - timestamp_ = metadata->timestamp_; - } else { - bytesused_ = 0; - sequence_ = 0; - timestamp_ = 0; - } + for (std::pair plane : planes) + planes_.push_back(new Plane(plane.first, plane.second)); } -/** - * \fn Buffer::index() - * \brief Retrieve the Buffer index - * \return The buffer index - */ +Buffer::~Buffer() +{ + for (Plane *plane : planes_) + delete plane; +} -/** - * \fn Buffer::dmabufs() - * \brief Retrieve the dmabuf file descriptors for all buffer planes - * - * The dmabufs array contains one dmabuf file descriptor per plane. Unused - * entries are set to -1. - * - * \return The dmabuf file descriptors - */ +Plane *Buffer::plane(unsigned int plane) +{ + if (plane > planes_.size()) { + LOG(Buffer, Error) << "Plane " << plane << " is out of range"; + return nullptr; + } -/** - * \fn Buffer::mem() - * \brief Retrieve the BufferMemory this buffer is associated with - * - * The association between the buffer and a BufferMemory instance is valid from - * the time the request containing this buffer is queued to a camera to the end - * of that request's completion handler. - * - * \return The BufferMemory this buffer is associated with - */ + return planes_[plane]; +} -/** - * \fn Buffer::bytesused() - * \brief Retrieve the number of bytes occupied by the data in the buffer - * \return Number of bytes occupied in the buffer - */ +std::array Buffer::fds() const +{ + std::array ret = { -1, -1, -1 }; -/** - * \fn Buffer::timestamp() - * \brief Retrieve the time when the buffer was processed - * - * The timestamp is expressed as a number of nanoseconds since the epoch. - * - * \return Timestamp when the buffer was processed - */ + unsigned int i = 0; + for (const Plane *plane : planes_) + ret[i++] = plane->fd(); -/** - * \fn Buffer::sequence() - * \brief Retrieve the buffer sequence number - * - * The sequence number is a monotonically increasing number assigned to the - * buffer processed by the stream. Gaps in the sequence numbers indicate - * dropped frames. - * - * \return Sequence number of the buffer - */ - -/** - * \fn Buffer::status() - * \brief Retrieve the buffer status - * - * The buffer status reports whether the buffer has completed successfully - * (BufferSuccess) or if an error occurred (BufferError). - * - * \return The buffer status - */ + return ret; +} -/** - * \fn Buffer::request() - * \brief Retrieve the request this buffer belongs to - * - * The intended callers of this method are buffer completion handlers that - * need to associate a buffer to the request it belongs to. - * - * A Buffer is associated to a request by Request::prepare() and the - * association is valid until the buffer completes. The returned request - * pointer is valid only during that interval. - * - * \return The Request the Buffer belongs to, or nullptr if the buffer is - * either completed or not associated with a request - * \sa Buffer::setRequest() - */ +std::vector> Buffer::description() const +{ + std::vector> desc; -/** - * \fn Buffer::stream() - * \brief Retrieve the stream this buffer is associated with - * - * A Buffer is associated to the stream that created it with - * Stream::createBuffer() and the association is valid until the buffer is - * destroyed. Buffer instances that are created directly are not associated - * with any stream. - * - * \return The Stream the Buffer is associated with, or nullptr if the buffer - * is not associated with a stream - */ + for (const Plane *plane : planes_) + desc.emplace_back(plane->fd(), plane->length()); -/** - * \brief Mark a buffer as cancel by setting its status to BufferCancelled - */ -void Buffer::cancel() -{ - bytesused_ = 0; - timestamp_ = 0; - sequence_ = 0; - status_ = BufferCancelled; + return desc; } -/** - * \fn Buffer::setRequest() - * \brief Set the request this buffer belongs to - * - * The intended callers are Request::prepare() and Request::completeBuffer(). - */ - BufferAllocator::BufferAllocator(std::shared_ptr camera) : camera_(camera) { diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp index e810fb725d81350d..63944c0199338f39 100644 --- a/src/libcamera/camera.cpp +++ b/src/libcamera/camera.cpp @@ -678,12 +678,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; @@ -739,14 +733,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_); @@ -812,24 +798,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(); @@ -856,15 +829,23 @@ int Camera::queueRequest(Request *request) */ int Camera::start() { + int ret; + if (disconnected_) return -ENODEV; if (!stateIs(CameraPrepared)) return -EACCES; + for (Stream *stream : streams_) { + ret = stream->importBuffers(true); + if (ret < 0) + return ret; + } + LOG(Camera, Debug) << "Starting capture"; - int ret = pipe_->start(this); + ret = pipe_->start(this); if (ret) return ret; @@ -899,6 +880,9 @@ int Camera::stop() pipe_->stop(this); + for (Stream *stream : streams_) + stream->importBuffers(false); + return 0; } @@ -912,13 +896,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/v4l2_videodevice.h b/src/libcamera/include/v4l2_videodevice.h index 01b90ab4654bfba2..cbbb6cd955bf527f 100644 --- a/src/libcamera/include/v4l2_videodevice.h +++ b/src/libcamera/include/v4l2_videodevice.h @@ -14,7 +14,9 @@ #include #include +#include #include +#include #include "formats.h" #include "log.h" @@ -22,9 +24,6 @@ namespace libcamera { -class Buffer; -class BufferMemory; -class BufferPool; class EventNotifier; class MediaDevice; class MediaEntity; @@ -161,12 +160,11 @@ public: int setFormat(V4L2DeviceFormat *format); ImageFormats formats(); - int exportBuffers(BufferPool *pool); - int importBuffers(BufferPool *pool); + int allocateBuffers(unsigned int count, std::vector *buffers); + int importBuffers(unsigned int count); int releaseBuffers(); - int queueBuffer(Buffer *buffer); - std::vector> queueAllBuffers(); + int queueBuffer(Buffer *buffer, const BufferInfo *info = nullptr); Signal bufferReady; int streamOn(); @@ -192,8 +190,8 @@ private: std::vector enumSizes(unsigned int pixelFormat); int requestBuffers(unsigned int count); - int createPlane(BufferMemory *buffer, unsigned int index, - unsigned int plane, unsigned int length); + Buffer *createBuffer(struct v4l2_buffer buf); + int expbuf(unsigned int index, unsigned int plane); void bufferAvailable(EventNotifier *notifier); @@ -202,7 +200,7 @@ private: enum v4l2_buf_type bufferType_; enum v4l2_memory memoryType_; - BufferPool *bufferPool_; + V4L2BufferCache *cache_; std::map queuedBuffers_; EventNotifier *fdEvent_; @@ -227,6 +225,21 @@ private: V4L2VideoDevice *capture_; }; +class V4L2Stream : public Stream +{ +public: + V4L2Stream(V4L2VideoDevice *video, unsigned int bufferCount); + +protected: + int allocateBuffers(std::vector *buffers) override; + int importBuffers(bool enable) override; + +private: + V4L2VideoDevice *video_; + unsigned int bufferCount_; + bool allocated; +}; + } /* namespace libcamera */ #endif /* __LIBCAMERA_V4L2_VIDEODEVICE_H__ */ diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp index 01064ac09859155d..af8e93c04bc40f04 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -43,7 +43,7 @@ public: V4L2VideoDevice *dev; unsigned int pad; std::string name; - BufferPool *pool; + std::vector buffers; }; ImgUDevice() @@ -69,9 +69,8 @@ public: int configureOutput(ImgUOutput *output, const StreamConfiguration &cfg); - int importInputBuffers(BufferPool *pool); - int importOutputBuffers(ImgUOutput *output, BufferPool *pool); - int exportOutputBuffers(ImgUOutput *output, BufferPool *pool); + int importInputBuffers(unsigned int count); + int exportOutputBuffers(ImgUOutput *output, unsigned int count); void freeBuffers(); int start(); @@ -92,10 +91,6 @@ public: ImgUOutput viewfinder_; ImgUOutput stat_; /* \todo Add param video device for 3A tuning */ - - BufferPool vfPool_; - BufferPool statPool_; - BufferPool outPool_; }; class CIO2Device @@ -119,10 +114,10 @@ public: int configure(const Size &size, V4L2DeviceFormat *outputFormat); - BufferPool *exportBuffers(); + int exportBuffers(); void freeBuffers(); - int start(std::vector> *buffers); + int start(); int stop(); static int mediaBusToFormat(unsigned int code); @@ -131,14 +126,17 @@ public: V4L2Subdevice *csi2_; CameraSensor *sensor_; - BufferPool pool_; +private: + std::vector buffers_; }; -class IPU3Stream : public Stream +class IPU3Stream : public V4L2Stream { public: - IPU3Stream(ImgUDevice::ImgUOutput *device, const std::string &name) - : active_(false), name_(name), device_(device) + IPU3Stream(ImgUDevice::ImgUOutput *device, const std::string &name, + unsigned int count) + : V4L2Stream(device->dev, count), active_(false), name_(name), + device_(device) { } @@ -170,8 +168,6 @@ public: IPU3Stream *outStream_; IPU3Stream *vfStream_; - - std::vector> rawBuffers_; }; class IPU3CameraConfiguration : public CameraConfiguration @@ -296,8 +292,6 @@ void IPU3CameraConfiguration::adjustStream(StreamConfiguration &cfg, bool scale) cfg.size.width &= ~7; cfg.size.height &= ~3; } - - cfg.bufferCount = IPU3_BUFFER_COUNT; } CameraConfiguration::Status IPU3CameraConfiguration::validate() @@ -635,70 +629,42 @@ int PipelineHandlerIPU3::allocateBuffers(Camera *camera, const std::set &streams) { IPU3CameraData *data = cameraData(camera); - IPU3Stream *outStream = data->outStream_; - IPU3Stream *vfStream = data->vfStream_; - CIO2Device *cio2 = &data->cio2_; - ImgUDevice *imgu = data->imgu_; unsigned int bufferCount; int ret; - /* Share buffers between CIO2 output and ImgU input. */ - BufferPool *pool = cio2->exportBuffers(); - if (!pool) - return -ENOMEM; + ret = data->cio2_.exportBuffers(); + if (ret < 0) + return ret; - ret = imgu->importInputBuffers(pool); - if (ret) - goto error; + bufferCount = ret; - /* - * Use for the stat's internal pool the same number of buffer as - * for the input pool. - * \todo To be revised when we'll actually use the stat node. - */ - bufferCount = pool->count(); - imgu->stat_.pool->createBuffers(bufferCount); - ret = imgu->exportOutputBuffers(&imgu->stat_, imgu->stat_.pool); + ret = data->imgu_->importInputBuffers(bufferCount); if (ret) goto error; - /* Allocate buffers for each active stream. */ - for (Stream *s : streams) { - IPU3Stream *stream = static_cast(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; - } + ret = data->imgu_->exportOutputBuffers(&data->imgu_->stat_, bufferCount); + if (ret < 0) + 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) + if (!data->outStream_->active_) { + ret = data->imgu_->exportOutputBuffers(data->outStream_->device_, + bufferCount); + if (ret < 0) goto error; } - if (!vfStream->active_) { - bufferCount = outStream->configuration().bufferCount; - vfStream->device_->pool->createBuffers(bufferCount); - ret = imgu->exportOutputBuffers(vfStream->device_, - vfStream->device_->pool); - if (ret) + if (!data->vfStream_->active_) { + ret = data->imgu_->exportOutputBuffers(data->vfStream_->device_, + bufferCount); + if (ret < 0) goto error; } return 0; - error: freeBuffers(camera, streams); @@ -727,7 +693,7 @@ int PipelineHandlerIPU3::start(Camera *camera) * Start the ImgU video devices, buffers will be queued to the * ImgU output and viewfinder when requests will be queued. */ - ret = cio2->start(&data->rawBuffers_); + ret = cio2->start(); if (ret) goto error; @@ -743,7 +709,6 @@ int PipelineHandlerIPU3::start(Camera *camera) error: LOG(IPU3, Error) << "Failed to start camera " << camera->name(); - data->rawBuffers_.clear(); return ret; } @@ -757,8 +722,6 @@ void PipelineHandlerIPU3::stop(Camera *camera) if (ret) LOG(IPU3, Warning) << "Failed to stop camera " << camera->name(); - - data->rawBuffers_.clear(); } int PipelineHandlerIPU3::queueRequest(Camera *camera, Request *request) @@ -880,8 +843,8 @@ int PipelineHandlerIPU3::registerCameras() * second. */ data->imgu_ = numCameras ? &imgu1_ : &imgu0_; - data->outStream_ = new IPU3Stream(&data->imgu_->output_, "output"); - data->vfStream_ = new IPU3Stream(&data->imgu_->viewfinder_, "viewfinder"); + data->outStream_ = new IPU3Stream(&data->imgu_->output_, "output", 4); + data->vfStream_ = new IPU3Stream(&data->imgu_->viewfinder_, "viewfinder", 4); /* * Connect video devices' 'bufferReady' signals to their @@ -1019,7 +982,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"); @@ -1029,7 +991,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(); @@ -1038,7 +999,6 @@ int ImgUDevice::init(MediaDevice *media, unsigned int index) stat_.pad = PAD_STAT; stat_.name = "stat"; - stat_.pool = &statPool_; return 0; } @@ -1138,65 +1098,25 @@ int ImgUDevice::configureOutput(ImgUOutput *output, return 0; } -/** - * \brief Import buffers from \a pool into the ImgU input - * \param[in] pool The buffer pool to import - * \return 0 on success or a negative error code otherwise - */ -int ImgUDevice::importInputBuffers(BufferPool *pool) +int ImgUDevice::importInputBuffers(unsigned int count) { - int ret = input_->importBuffers(pool); - if (ret) { + int ret = input_->importBuffers(count); + if (ret) LOG(IPU3, Error) << "Failed to import ImgU input buffers"; - return ret; - } - return 0; + return ret; } -/** - * \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 ImgUDevice::exportOutputBuffers(ImgUOutput *output, unsigned int count) { - int ret = output->dev->exportBuffers(pool); - if (ret) { - LOG(IPU3, Error) << "Failed to export ImgU " - << output->name << " buffers"; - return ret; - } - - return 0; -} + int ret; -/** - * \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; - } + ret = output->dev->allocateBuffers(count, &output->buffers); + if (ret < 0) + LOG(IPU3, Error) << "Failed to allocate ImgU " + << output->name << " buffers"; - return 0; + return ret; } /** @@ -1451,38 +1371,33 @@ int CIO2Device::configure(const Size &size, return 0; } -/** - * \brief Allocate CIO2 memory buffers and export them in a BufferPool - * - * Allocate memory buffers in the CIO2 video device and export them to - * a buffer pool that can be imported by another device. - * - * \return The buffer pool with export buffers on success or nullptr otherwise - */ -BufferPool *CIO2Device::exportBuffers() +int CIO2Device::exportBuffers() { - pool_.createBuffers(CIO2_BUFFER_COUNT); - - int ret = output_->exportBuffers(&pool_); - if (ret) { + int ret = output_->allocateBuffers(CIO2_BUFFER_COUNT, &buffers_); + if (ret < 0) LOG(IPU3, Error) << "Failed to export CIO2 buffers"; - return nullptr; - } - return &pool_; + return ret; } void CIO2Device::freeBuffers() { + for (Buffer *buffer : buffers_) + delete buffer; + if (output_->releaseBuffers()) LOG(IPU3, Error) << "Failed to release CIO2 buffers"; } -int CIO2Device::start(std::vector> *buffers) +int CIO2Device::start() { - *buffers = output_->queueAllBuffers(); - if (buffers->empty()) - return -EINVAL; + for (Buffer *buffer : buffers_) { + int ret = output_->queueBuffer(buffer); + if (ret) { + LOG(IPU3, Error) << "Failed to queue CIO2 buffer"; + return ret; + } + } return output_->streamOn(); } diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index 33a058de18b8cf2e..fb224370de19303d 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -31,8 +31,7 @@ #include "v4l2_subdevice.h" #include "v4l2_videodevice.h" -#define RKISP1_PARAM_BASE 0x100 -#define RKISP1_STAT_BASE 0x200 +#define RKISP1_BUFFER_COUNT 4 namespace libcamera { @@ -153,8 +152,6 @@ public: const V4L2SubdeviceFormat &sensorFormat() { return sensorFormat_; } private: - static constexpr unsigned int RKISP1_BUFFER_COUNT = 4; - /* * The RkISP1CameraData instance is guaranteed to be valid as long as the * corresponding Camera instance is valid. In order to borrow a @@ -213,9 +210,6 @@ private: V4L2VideoDevice *param_; V4L2VideoDevice *stat_; - BufferPool paramPool_; - BufferPool statPool_; - std::queue paramBuffers_; std::queue statBuffers_; @@ -508,8 +502,6 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate() status = Adjusted; } - cfg.bufferCount = RKISP1_BUFFER_COUNT; - return status; } @@ -660,46 +652,40 @@ int PipelineHandlerRkISP1::allocateBuffers(Camera *camera, const std::set &streams) { RkISP1CameraData *data = cameraData(camera); - Stream *stream = *streams.begin(); + std::vector paramBuffers, statBuffers; int ret; - if (stream->memoryType() == InternalMemory) - ret = video_->exportBuffers(&stream->bufferPool()); - else - ret = video_->importBuffers(&stream->bufferPool()); - - if (ret) - return ret; - paramPool_.createBuffers(stream->configuration().bufferCount + 1); - ret = param_->exportBuffers(¶mPool_); - if (ret) { - video_->releaseBuffers(); - return ret; - } + ret = param_->allocateBuffers(RKISP1_BUFFER_COUNT, ¶mBuffers); + if (ret < 0) + goto err; - statPool_.createBuffers(stream->configuration().bufferCount + 1); - ret = stat_->exportBuffers(&statPool_); - if (ret) { - param_->releaseBuffers(); - video_->releaseBuffers(); - return ret; - } + ret = stat_->allocateBuffers(RKISP1_BUFFER_COUNT, &statBuffers); + if (ret < 0) + goto err; - for (unsigned int i = 0; i < stream->configuration().bufferCount + 1; i++) { - data->ipaBuffers_.push_back({ .id = RKISP1_PARAM_BASE | i, - .memory = paramPool_.buffers()[i] }); - paramBuffers_.push(new Buffer(i)); + for (Buffer *buffer : paramBuffers) { + data->ipaBuffers_.push_back({ .id = static_cast(buffer->plane(0)->fd()), + .planes = buffer->description() }); + paramBuffers_.push(buffer); } - for (unsigned int i = 0; i < stream->configuration().bufferCount + 1; i++) { - data->ipaBuffers_.push_back({ .id = RKISP1_STAT_BASE | i, - .memory = statPool_.buffers()[i] }); - statBuffers_.push(new Buffer(i)); + for (Buffer *buffer : statBuffers) { + data->ipaBuffers_.push_back({ .id = static_cast(buffer->plane(0)->fd()), + .planes = buffer->description() }); + statBuffers_.push(buffer); } data->ipa_->mapBuffers(data->ipaBuffers_); + return 0; +err: + for (Buffer *buffer : paramBuffers) + delete buffer; + + for (Buffer *buffer : statBuffers) + delete buffer; + return ret; } @@ -820,9 +806,11 @@ int PipelineHandlerRkISP1::queueRequest(Camera *camera, Request *request) if (!info) return -ENOENT; + unsigned int paramid = static_cast(info->paramBuffer->plane(0)->fd()); + IPAOperationData op; op.operation = RKISP1_IPA_EVENT_QUEUE_REQUEST; - op.data = { data->frame_, RKISP1_PARAM_BASE | info->paramBuffer->index() }; + op.data = { data->frame_, paramid }; op.controls = { request->controls() }; data->ipa_->processEvent(op); @@ -874,7 +862,7 @@ int PipelineHandlerRkISP1::createCamera(MediaEntity *sensor) std::unique_ptr data = utils::make_unique(this); - data->stream_ = new Stream(); + data->stream_ = new V4L2Stream(video_, RKISP1_BUFFER_COUNT); ControlInfoMap::Map ctrls; ctrls.emplace(std::piecewise_construct, @@ -982,9 +970,9 @@ void PipelineHandlerRkISP1::tryCompleteRequest(Request *request) if (!info->paramDequeued) return; - completeRequest(activeCamera_, request); - data->frameInfo_.destroy(info->frame); + + completeRequest(activeCamera_, request); } void PipelineHandlerRkISP1::bufferReady(Buffer *buffer, const BufferInfo &info) @@ -1023,7 +1011,7 @@ void PipelineHandlerRkISP1::statReady(Buffer *buffer, const BufferInfo &info) return; unsigned int frame = rkinfo->frame; - unsigned int statid = RKISP1_STAT_BASE | rkinfo->statBuffer->index(); + unsigned int statid = static_cast(rkinfo->statBuffer->plane(0)->fd()); IPAOperationData op; op.operation = RKISP1_IPA_EVENT_SIGNAL_STAT_BUFFER; diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp index ef2e8c9734f844ce..21e594075eefffdc 100644 --- a/src/libcamera/pipeline/uvcvideo.cpp +++ b/src/libcamera/pipeline/uvcvideo.cpp @@ -136,8 +136,6 @@ CameraConfiguration::Status UVCCameraConfiguration::validate() status = Adjusted; } - cfg.bufferCount = 4; - return status; } @@ -161,7 +159,6 @@ CameraConfiguration *PipelineHandlerUVC::generateConfiguration(Camera *camera, cfg.pixelFormat = formats.pixelformats().front(); cfg.size = formats.sizes(cfg.pixelFormat).back(); - cfg.bufferCount = 4; config->addConfiguration(cfg); @@ -196,16 +193,7 @@ int PipelineHandlerUVC::configure(Camera *camera, CameraConfiguration *config) int PipelineHandlerUVC::allocateBuffers(Camera *camera, const std::set &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, @@ -331,7 +319,7 @@ int UVCCameraData::init(MediaEntity *entity) if (ret) return ret; - stream_ = new Stream(); + stream_ = new V4L2Stream(video_, 4); video_->bufferReady.connect(this, &UVCCameraData::bufferReady); diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp index e3eefc49135179f2..a14b7c8834174a5f 100644 --- a/src/libcamera/pipeline/vimc.cpp +++ b/src/libcamera/pipeline/vimc.cpp @@ -158,8 +158,6 @@ CameraConfiguration::Status VimcCameraConfiguration::validate() status = Adjusted; } - cfg.bufferCount = 4; - return status; } @@ -190,7 +188,6 @@ CameraConfiguration *PipelineHandlerVimc::generateConfiguration(Camera *camera, cfg.pixelFormat = V4L2_PIX_FMT_RGB24; cfg.size = { 1920, 1080 }; - cfg.bufferCount = 4; config->addConfiguration(cfg); @@ -263,16 +260,7 @@ int PipelineHandlerVimc::configure(Camera *camera, CameraConfiguration *config) int PipelineHandlerVimc::allocateBuffers(Camera *camera, const std::set &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, @@ -419,7 +407,7 @@ int VimcCameraData::init(MediaDevice *media) if (video_->open()) return -ENODEV; - stream_ = new Stream(); + stream_ = new V4L2Stream(video_, 4); video_->bufferReady.connect(this, &VimcCameraData::bufferReady); diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp index 9a0e439a3d05d780..2bda7a57e8d6381e 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_; @@ -125,21 +120,15 @@ 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(std::unique_ptr buffer) +int Request::addBuffer(Stream *stream, Buffer *buffer) { - Stream *stream = buffer->stream(); - if (!stream) { - LOG(Request, Error) << "Invalid stream reference"; - return -EINVAL; - } - auto it = bufferMap_.find(stream); if (it != bufferMap_.end()) { LOG(Request, Error) << "Buffer already set for stream"; return -EEXIST; } - bufferMap_[stream] = buffer.release(); + bufferMap_[stream] = buffer; return 0; } diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp index 97c6722b8c4c98cf..873452c9608fc9f1 100644 --- a/src/libcamera/v4l2_videodevice.cpp +++ b/src/libcamera/v4l2_videodevice.cpp @@ -19,6 +19,7 @@ #include #include +#include #include "log.h" #include "media_device.h" @@ -319,7 +320,7 @@ const std::string V4L2DeviceFormat::toString() const * \param[in] deviceNode The file-system path to the video device node */ V4L2VideoDevice::V4L2VideoDevice(const std::string &deviceNode) - : V4L2Device(deviceNode), bufferPool_(nullptr), fdEvent_(nullptr) + : V4L2Device(deviceNode), cache_(nullptr), fdEvent_(nullptr) { /* * We default to an MMAP based CAPTURE video device, however this will @@ -879,27 +880,32 @@ int V4L2VideoDevice::requestBuffers(unsigned int count) } /** - * \brief Request buffers to be allocated from the video device and stored in - * the buffer pool provided. - * \param[out] pool BufferPool to populate with buffers + * \brief Operate using buffers allocated from local video device + * \param[in] count Number of buffers to allocate + * \param[out] buffer Vector to store local buffers * \return 0 on success or a negative error code otherwise */ -int V4L2VideoDevice::exportBuffers(BufferPool *pool) +int V4L2VideoDevice::allocateBuffers(unsigned int count, std::vector *buffers) { unsigned int i; int ret; + if (cache_) { + LOG(V4L2, Error) << "Can't export buffers with existing cache"; + return -EINVAL; + } + + cache_ = new V4L2BufferCache(count); + memoryType_ = V4L2_MEMORY_MMAP; - ret = requestBuffers(pool->count()); - if (ret) - return ret; + ret = requestBuffers(count); + if (ret < 0) + goto err_cache; - /* Map the buffers. */ - for (i = 0; i < pool->count(); ++i) { - struct v4l2_plane planes[VIDEO_MAX_PLANES] = {}; + for (i = 0; i < count; ++i) { struct v4l2_buffer buf = {}; - BufferMemory &buffer = pool->buffers()[i]; + struct v4l2_plane planes[VIDEO_MAX_PLANES] = {}; buf.index = i; buf.type = bufferType_; @@ -912,51 +918,71 @@ int V4L2VideoDevice::exportBuffers(BufferPool *pool) LOG(V4L2, Error) << "Unable to query buffer " << i << ": " << strerror(-ret); - break; + goto err_buf; } - if (V4L2_TYPE_IS_MULTIPLANAR(buf.type)) { - for (unsigned int p = 0; p < buf.length; ++p) { - ret = createPlane(&buffer, i, p, - buf.m.planes[p].length); - if (ret) - break; - } - } else { - ret = createPlane(&buffer, i, 0, buf.length); + Buffer *buffer = createBuffer(buf); + if (!buffer) { + LOG(V4L2, Error) << "Unable to create buffer"; + ret = -EINVAL; + goto err_buf; } - if (ret) { - LOG(V4L2, Error) << "Failed to create plane"; - break; - } + buffers->push_back(buffer); + cache_->populate(i, buffer->fds()); } - if (ret) { - requestBuffers(0); - pool->destroyBuffers(); - return ret; + return count; +err_buf: + requestBuffers(0); + + for (Buffer *buffer : *buffers) + delete buffer; + + buffers->clear(); +err_cache: + delete cache_; + cache_ = nullptr; + + return ret; +} + +Buffer *V4L2VideoDevice::createBuffer(struct v4l2_buffer buf) +{ + const unsigned int numPlanes = V4L2_TYPE_IS_MULTIPLANAR(buf.type) ? buf.length : 1; + std::vector> planes; + Buffer *frame = nullptr; + + for (unsigned int plane = 0; plane < numPlanes; plane++) { + int fd = expbuf(buf.index, plane); + if (fd < 0) + goto out; + + if (V4L2_TYPE_IS_MULTIPLANAR(buf.type)) + planes.emplace_back(fd, buf.m.planes[plane].length); + else + planes.emplace_back(fd, buf.length); } - bufferPool_ = pool; + if (!planes.empty()) + frame = new Buffer(planes); + else + LOG(V4L2, Error) << "Failed to get planes"; +out: + for (std::pair plane : planes) + ::close(plane.first); - return 0; + return frame; } -int V4L2VideoDevice::createPlane(BufferMemory *buffer, unsigned int index, - unsigned int planeIndex, unsigned int length) +int V4L2VideoDevice::expbuf(unsigned int index, unsigned int plane) { struct v4l2_exportbuffer expbuf = {}; int ret; - LOG(V4L2, Debug) - << "Buffer " << index - << " plane " << planeIndex - << ": length=" << length; - expbuf.type = bufferType_; expbuf.index = index; - expbuf.plane = planeIndex; + expbuf.plane = plane; expbuf.flags = O_RDWR; ret = ioctl(VIDIOC_EXPBUF, &expbuf); @@ -966,31 +992,31 @@ int V4L2VideoDevice::createPlane(BufferMemory *buffer, unsigned int index, return ret; } - buffer->planes().emplace_back(); - Plane &plane = buffer->planes().back(); - plane.setDmabuf(expbuf.fd, length); - ::close(expbuf.fd); - - return 0; + return expbuf.fd; } /** - * \brief Import the externally allocated \a pool of buffers - * \param[in] pool BufferPool of buffers to import + * \brief Operate using buffers imported from external source + * \param[in] count Number of buffers to prepare for * \return 0 on success or a negative error code otherwise */ -int V4L2VideoDevice::importBuffers(BufferPool *pool) +int V4L2VideoDevice::importBuffers(unsigned int count) { + /* Only prepare for external buffers if internals not in use. */ + if (cache_) + return 0; + int ret; memoryType_ = V4L2_MEMORY_DMABUF; - ret = requestBuffers(pool->count()); + ret = requestBuffers(count); if (ret) return ret; - LOG(V4L2, Debug) << "provided pool of " << pool->count() << " buffers"; - bufferPool_ = pool; + cache_ = new V4L2BufferCache(count); + + LOG(V4L2, Debug) << "provided for " << count << " external buffers"; return 0; } @@ -1000,9 +1026,10 @@ int V4L2VideoDevice::importBuffers(BufferPool *pool) */ int V4L2VideoDevice::releaseBuffers() { - LOG(V4L2, Debug) << "Releasing bufferPool"; + LOG(V4L2, Debug) << "Releasing buffers"; - bufferPool_ = nullptr; + delete cache_; + cache_ = nullptr; return requestBuffers(0); } @@ -1018,40 +1045,42 @@ int V4L2VideoDevice::releaseBuffers() * * \return 0 on success or a negative error code otherwise */ -int V4L2VideoDevice::queueBuffer(Buffer *buffer) +int V4L2VideoDevice::queueBuffer(Buffer *buffer, const BufferInfo *info) { - struct v4l2_plane v4l2Planes[VIDEO_MAX_PLANES] = {}; struct v4l2_buffer buf = {}; + struct v4l2_plane v4l2Planes[VIDEO_MAX_PLANES] = {}; int ret; - buf.index = buffer->index(); + ASSERT(cache_); + + buf.index = cache_->pop(buffer->fds()); buf.type = bufferType_; buf.memory = memoryType_; buf.field = V4L2_FIELD_NONE; - bool multiPlanar = V4L2_TYPE_IS_MULTIPLANAR(buf.type); - BufferMemory *mem = &bufferPool_->buffers()[buf.index]; - const std::vector &planes = mem->planes(); + if (V4L2_TYPE_IS_MULTIPLANAR(buf.type)) { + buf.length = buffer->numPlanes(); + buf.m.planes = v4l2Planes; + } if (buf.memory == V4L2_MEMORY_DMABUF) { - if (multiPlanar) { - for (unsigned int p = 0; p < planes.size(); ++p) - v4l2Planes[p].m.fd = planes[p].dmabuf(); - } else { - buf.m.fd = planes[0].dmabuf(); - } + if (V4L2_TYPE_IS_MULTIPLANAR(buf.type)) + for (unsigned int i = 0; i < buffer->numPlanes(); i++) + v4l2Planes[i].m.fd = buffer->plane(i)->fd(); + else + buf.m.fd = buffer->plane(0)->fd(); } - if (multiPlanar) { - buf.length = planes.size(); - buf.m.planes = v4l2Planes; - } + if (V4L2_TYPE_IS_OUTPUT(bufferType_) && info) { + if (V4L2_TYPE_IS_MULTIPLANAR(buf.type)) + for (unsigned int i = 0; i < buffer->numPlanes(); i++) + v4l2Planes[i].bytesused = info->plane(i).bytesused; + else + buf.bytesused = info->plane(0).bytesused; - if (V4L2_TYPE_IS_OUTPUT(bufferType_)) { - buf.bytesused = buffer->bytesused_; - buf.sequence = buffer->sequence_; - buf.timestamp.tv_sec = buffer->timestamp_ / 1000000000; - buf.timestamp.tv_usec = (buffer->timestamp_ / 1000) % 1000000; + buf.sequence = info->sequence(); + buf.timestamp.tv_sec = info->timestamp() / 1000000000; + buf.timestamp.tv_usec = (info->timestamp() / 1000) % 1000000; } LOG(V4L2, Debug) << "Queueing buffer " << buf.index; @@ -1072,52 +1101,6 @@ int V4L2VideoDevice::queueBuffer(Buffer *buffer) return 0; } -/** - * \brief Queue all buffers into the video device - * - * When starting video capture users of the video device often need to queue - * all allocated buffers to the device. This helper method simplifies the - * implementation of the user by queuing all buffers and returning a vector of - * Buffer instances for each queued buffer. - * - * This method is meant to be used with video capture devices internal to a - * pipeline handler, such as ISP statistics capture devices, or raw CSI-2 - * receivers. For video capture devices facing applications, buffers shall - * instead be queued when requests are received, and for video output devices, - * buffers shall be queued when frames are ready to be output. - * - * The caller shall ensure that the returned buffers vector remains valid until - * all the queued buffers are dequeued, either during capture, or by stopping - * the video device. - * - * Calling this method on an output device or on a device that has buffers - * already queued is an error and will return an empty vector. - * - * \return A vector of queued buffers, which will be empty if an error occurs - */ -std::vector> V4L2VideoDevice::queueAllBuffers() -{ - int ret; - - if (!queuedBuffers_.empty()) - return {}; - - if (V4L2_TYPE_IS_OUTPUT(bufferType_)) - return {}; - - std::vector> buffers; - - for (unsigned int i = 0; i < bufferPool_->count(); ++i) { - Buffer *buffer = new Buffer(i); - buffers.emplace_back(buffer); - ret = queueBuffer(buffer); - if (ret) - return {}; - } - - return buffers; -} - /** * \brief Slot to handle completed buffer events from the V4L2 video device * \param[in] notifier The event notifier @@ -1149,6 +1132,7 @@ void V4L2VideoDevice::bufferAvailable(EventNotifier *notifier) return; } + cache_->push(buf.index); LOG(V4L2, Debug) << "Buffer " << buf.index << " is available"; auto it = queuedBuffers_.find(buf.index); @@ -1164,7 +1148,6 @@ void V4L2VideoDevice::bufferAvailable(EventNotifier *notifier) + buf.timestamp.tv_usec * 1000ULL; BufferInfo info(status, buf.sequence, timestamp, { { buf.bytesused } }); - buffer->index_ = buf.index; /* Notify anyone listening to the device. */ bufferReady.emit(buffer, info); @@ -1216,13 +1199,10 @@ int V4L2VideoDevice::streamOff() /* Send back all queued buffers. */ for (auto it : queuedBuffers_) { - unsigned int index = it.first; Buffer *buffer = it.second; BufferInfo info(BufferInfo::BufferCancelled, 0, 0, { {} }); - buffer->index_ = index; - buffer->cancel(); bufferReady.emit(buffer, info); } @@ -1353,4 +1333,41 @@ void V4L2M2MDevice::close() output_->close(); } +V4L2Stream::V4L2Stream(V4L2VideoDevice *video, unsigned int bufferCount) + : Stream(), video_(video), bufferCount_(bufferCount), + allocated(false) +{ +} + +int V4L2Stream::allocateBuffers(std::vector *buffers) +{ + if (buffers) { + if (allocated) + return -EINVAL; + + allocated = true; + return video_->allocateBuffers(bufferCount_, buffers); + } else { + if (!allocated) + return -EINVAL; + + return video_->releaseBuffers(); + } +} + +int V4L2Stream::importBuffers(bool enable) +{ + /* + * No need to prepare for buffer importing if internal buffers + * are allocated. + */ + if (allocated) + return 0; + + if (enable) + return video_->importBuffers(bufferCount_); + else + return video_->releaseBuffers(); +} + } /* namespace libcamera */ diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp index 2d7f4ba84fbb6838..d17aec42c82bbd98 100644 --- a/src/qcam/main_window.cpp +++ b/src/qcam/main_window.cpp @@ -22,7 +22,7 @@ using namespace libcamera; MainWindow::MainWindow(CameraManager *cm, const OptionsParser::Options &options) - : options_(options), isCapturing_(false) + : options_(options), allocator_(nullptr), isCapturing_(false) { int ret; @@ -36,8 +36,10 @@ MainWindow::MainWindow(CameraManager *cm, const OptionsParser::Options &options) adjustSize(); ret = openCamera(cm); - if (!ret) + if (!ret) { + allocator_ = new BufferAllocator(camera_); ret = startCapture(); + } if (ret < 0) QTimer::singleShot(0, QCoreApplication::instance(), @@ -51,6 +53,8 @@ MainWindow::~MainWindow() camera_->release(); camera_.reset(); } + + delete allocator_; } void MainWindow::updateTitle() @@ -175,22 +179,24 @@ int MainWindow::startCapture() return ret; } + ret = allocator_->allocate(stream); + if (ret < 0) { + std::cerr << "Failed to allocate internal buffers" << std::endl; + return ret; + } + unsigned int nbuffers = static_cast(ret); + std::vector requests; - for (unsigned int i = 0; i < cfg.bufferCount; ++i) { + for (unsigned int i = 0; i < nbuffers; ++i) { Request *request = camera_->createRequest(); - if (!request) { + Buffer *buffer = allocator_->get(stream)[i]; + if (!request || !buffer) { std::cerr << "Can't create request" << std::endl; ret = -ENOMEM; goto error; } - std::unique_ptr buffer = stream->createBuffer(i); - if (!buffer) { - std::cerr << "Can't create buffer " << i << std::endl; - goto error; - } - - ret = request->addBuffer(std::move(buffer)); + ret = request->addBuffer(stream, buffer); if (ret < 0) { std::cerr << "Can't set buffer for request" << std::endl; goto error; @@ -281,15 +287,8 @@ 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 newBuffer = stream->createBuffer(index); - if (!newBuffer) { - std::cerr << "Can't create buffer " << index << std::endl; - return; - } - request->addBuffer(std::move(newBuffer)); + request->addBuffer(stream, buffer); } camera_->queueRequest(request); @@ -297,12 +296,10 @@ void MainWindow::requestComplete(Request *request) int MainWindow::display(Buffer *buffer, const BufferInfo &info) { - BufferMemory *mem = buffer->mem(); - if (mem->planes().size() != 1) + if (buffer->numPlanes() != 1) return -EINVAL; - Plane &plane = mem->planes().front(); - unsigned char *raw = static_cast(plane.mem()); + unsigned char *raw = static_cast(buffer->plane(0)->mem()); viewfinder_->display(raw, info.plane(0).bytesused); return 0; diff --git a/src/qcam/main_window.h b/src/qcam/main_window.h index 8cbd72eb0d63cbea..eeb28eaae0dfc135 100644 --- a/src/qcam/main_window.h +++ b/src/qcam/main_window.h @@ -15,6 +15,7 @@ #include #include +#include #include #include #include @@ -58,6 +59,8 @@ private: const OptionsParser::Options &options_; std::shared_ptr camera_; + BufferAllocator *allocator_; + bool isCapturing_; std::unique_ptr config_; diff --git a/test/camera/capture.cpp b/test/camera/capture.cpp index 3cf5b798448d01db..8d3d612222a3b21e 100644 --- a/test/camera/capture.cpp +++ b/test/camera/capture.cpp @@ -41,10 +41,9 @@ protected: /* Create a new request. */ Stream *stream = buffers.begin()->first; Buffer *buffer = buffers.begin()->second; - std::unique_ptr newBuffer = stream->createBuffer(buffer->index()); request = camera_->createRequest(); - request->addBuffer(std::move(newBuffer)); + request->addBuffer(stream, buffer); camera_->queueRequest(request); } @@ -84,21 +83,25 @@ protected: } Stream *stream = cfg.stream(); + + BufferAllocator allocator(camera_); + int ret = allocator.allocate(stream); + if (ret < 0) + return TestFail; + + unsigned int nbuffers = static_cast(ret); + std::vector requests; - for (unsigned int i = 0; i < cfg.bufferCount; ++i) { + for (unsigned int i = 0; i < nbuffers; ++i) { Request *request = camera_->createRequest(); - if (!request) { - cout << "Failed to create request" << endl; - return TestFail; - } + Buffer *buffer = allocator.get(stream)[i]; - std::unique_ptr buffer = stream->createBuffer(i); - if (!buffer) { - cout << "Failed to create buffer " << i << endl; + if (!request || !buffer) { + cout << "Failed to create request" << endl; return TestFail; } - if (request->addBuffer(std::move(buffer))) { + if (request->addBuffer(stream, buffer)) { cout << "Failed to associating buffer with request" << endl; return TestFail; } @@ -131,10 +134,10 @@ protected: while (timer.isRunning()) dispatcher->processEvents(); - if (completeRequestsCount_ <= cfg.bufferCount * 2) { + if (completeRequestsCount_ <= nbuffers * 2) { cout << "Failed to capture enough frames (got " << completeRequestsCount_ << " expected at least " - << cfg.bufferCount * 2 << ")" << endl; + << nbuffers * 2 << ")" << endl; return TestFail; } diff --git a/test/camera/statemachine.cpp b/test/camera/statemachine.cpp index 12d5e0e1d5343f28..a77dec0f8b986634 100644 --- a/test/camera/statemachine.cpp +++ b/test/camera/statemachine.cpp @@ -178,6 +178,12 @@ protected: if (camera_->allocateBuffers()) return TestFail; + /* Use internally allocated buffers. */ + allocator_ = new BufferAllocator(camera_); + Stream *stream = *camera_->streams().begin(); + if (allocator_->allocate(stream) < 0) + return TestFail; + if (camera_->start()) return TestFail; @@ -211,8 +217,7 @@ protected: return TestFail; Stream *stream = *camera_->streams().begin(); - std::unique_ptr buffer = stream->createBuffer(0); - if (request->addBuffer(std::move(buffer))) + if (request->addBuffer(stream, allocator_->get(stream)[0])) return TestFail; if (camera_->queueRequest(request)) @@ -222,6 +227,8 @@ protected: if (camera_->stop()) return TestFail; + delete allocator_; + if (camera_->freeBuffers()) return TestFail; @@ -278,6 +285,7 @@ protected: } std::unique_ptr defconf_; + BufferAllocator *allocator_; }; } /* namespace */ diff --git a/test/v4l2_videodevice/buffer_sharing.cpp b/test/v4l2_videodevice/buffer_sharing.cpp index 3609c331cf10e056..f0c4ace2f47cc58b 100644 --- a/test/v4l2_videodevice/buffer_sharing.cpp +++ b/test/v4l2_videodevice/buffer_sharing.cpp @@ -73,16 +73,14 @@ protected: return TestFail; } - pool_.createBuffers(bufferCount); - - ret = capture_->exportBuffers(&pool_); - if (ret) { - std::cout << "Failed to export buffers" << std::endl; + ret = capture_->allocateBuffers(bufferCount, &buffers_); + if (ret < 0) { + std::cout << "Failed to allocate buffers" << std::endl; return TestFail; } - ret = output_->importBuffers(&pool_); - if (ret) { + ret = output_->importBuffers(bufferCount); + if (ret < 0) { std::cout << "Failed to import buffers" << std::endl; return TestFail; } @@ -98,8 +96,7 @@ protected: if (info.status() != BufferInfo::BufferSuccess) return; - BufferInfo infocopy = info; - output_->queueBuffer(buffer); + output_->queueBuffer(buffer, &info); framesCaptured_++; } @@ -124,10 +121,8 @@ protected: capture_->bufferReady.connect(this, &BufferSharingTest::captureBufferReady); output_->bufferReady.connect(this, &BufferSharingTest::outputBufferReady); - std::vector> buffers; - buffers = capture_->queueAllBuffers(); - if (buffers.empty()) - return TestFail; + for (Buffer *buffer : buffers_) + capture_->queueBuffer(buffer); ret = capture_->streamOn(); if (ret) { diff --git a/test/v4l2_videodevice/capture_async.cpp b/test/v4l2_videodevice/capture_async.cpp index 26ea1d17fd901ef8..a12c968f36e937ab 100644 --- a/test/v4l2_videodevice/capture_async.cpp +++ b/test/v4l2_videodevice/capture_async.cpp @@ -38,18 +38,14 @@ protected: Timer timeout; int ret; - pool_.createBuffers(bufferCount); - - ret = capture_->exportBuffers(&pool_); - if (ret) + ret = capture_->allocateBuffers(bufferCount, &buffers_); + if (ret < 0) return TestFail; capture_->bufferReady.connect(this, &CaptureAsyncTest::receiveBuffer); - std::vector> buffers; - buffers = capture_->queueAllBuffers(); - if (buffers.empty()) - return TestFail; + for (Buffer *buffer : buffers_) + capture_->queueBuffer(buffer); ret = capture_->streamOn(); if (ret) diff --git a/test/v4l2_videodevice/request_buffers.cpp b/test/v4l2_videodevice/request_buffers.cpp index c4aedf7b3cd61e80..2f8dfe1cafb111df 100644 --- a/test/v4l2_videodevice/request_buffers.cpp +++ b/test/v4l2_videodevice/request_buffers.cpp @@ -16,17 +16,10 @@ public: protected: int run() { - /* - * TODO: - * Test invalid requests - * Test different buffer allocations - */ const unsigned int bufferCount = 8; - pool_.createBuffers(bufferCount); - - int ret = capture_->exportBuffers(&pool_); - if (ret) + int ret = capture_->allocateBuffers(bufferCount, &buffers_); + if (ret != bufferCount) return TestFail; return TestPass; diff --git a/test/v4l2_videodevice/stream_on_off.cpp b/test/v4l2_videodevice/stream_on_off.cpp index 7664adc4c1f07046..ce48310aa2b7c3a8 100644 --- a/test/v4l2_videodevice/stream_on_off.cpp +++ b/test/v4l2_videodevice/stream_on_off.cpp @@ -17,10 +17,8 @@ protected: { const unsigned int bufferCount = 8; - pool_.createBuffers(bufferCount); - - int ret = capture_->exportBuffers(&pool_); - if (ret) + int ret = capture_->allocateBuffers(bufferCount, &buffers_); + if (ret < 0) return TestFail; ret = capture_->streamOn(); diff --git a/test/v4l2_videodevice/v4l2_m2mdevice.cpp b/test/v4l2_videodevice/v4l2_m2mdevice.cpp index e6ca90a4604dfcda..def8d82972d754f9 100644 --- a/test/v4l2_videodevice/v4l2_m2mdevice.cpp +++ b/test/v4l2_videodevice/v4l2_m2mdevice.cpp @@ -112,17 +112,14 @@ protected: return TestFail; } - capturePool_.createBuffers(bufferCount); - outputPool_.createBuffers(bufferCount); - - ret = capture->exportBuffers(&capturePool_); - if (ret) { + ret = capture->allocateBuffers(bufferCount, &captureBuffers_); + if (ret < 0) { cerr << "Failed to export Capture Buffers" << endl; return TestFail; } - ret = output->exportBuffers(&outputPool_); - if (ret) { + ret = output->allocateBuffers(bufferCount, &outputBuffers_); + if (ret < 0) { cerr << "Failed to export Output Buffers" << endl; return TestFail; } @@ -130,24 +127,11 @@ protected: capture->bufferReady.connect(this, &V4L2M2MDeviceTest::receiveCaptureBuffer); output->bufferReady.connect(this, &V4L2M2MDeviceTest::outputBufferComplete); - std::vector> captureBuffers; - captureBuffers = capture->queueAllBuffers(); - if (captureBuffers.empty()) { - cerr << "Failed to queue all Capture Buffers" << endl; - return TestFail; - } + for (Buffer *buffer : captureBuffers_) + capture->queueBuffer(buffer); - /* We can't "queueAllBuffers()" on an output device, so we do it manually */ - std::vector> outputBuffers; - for (unsigned int i = 0; i < outputPool_.count(); ++i) { - Buffer *buffer = new Buffer(i); - outputBuffers.emplace_back(buffer); - ret = output->queueBuffer(buffer); - if (ret) { - cerr << "Failed to queue output buffer" << i << endl; - return TestFail; - } - } + for (Buffer *buffer : outputBuffers_) + output->queueBuffer(buffer); ret = capture->streamOn(); if (ret) { @@ -202,8 +186,8 @@ private: std::shared_ptr media_; V4L2M2MDevice *vim2m_; - BufferPool capturePool_; - BufferPool outputPool_; + std::vector captureBuffers_; + std::vector outputBuffers_; unsigned int outputFrames_; unsigned int captureFrames_; diff --git a/test/v4l2_videodevice/v4l2_videodevice_test.h b/test/v4l2_videodevice/v4l2_videodevice_test.h index 34dd231c6d9d108d..1b6f4b862fbae020 100644 --- a/test/v4l2_videodevice/v4l2_videodevice_test.h +++ b/test/v4l2_videodevice/v4l2_videodevice_test.h @@ -41,7 +41,7 @@ protected: CameraSensor *sensor_; V4L2Subdevice *debayer_; V4L2VideoDevice *capture_; - BufferPool pool_; + std::vector buffers_; }; #endif /* __LIBCAMERA_V4L2_DEVICE_TEST_H_ */ From patchwork Mon Oct 28 02:25:25 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Niklas_S=C3=B6derlund?= X-Patchwork-Id: 2272 Return-Path: Received: from bin-mail-out-05.binero.net (bin-mail-out-05.binero.net [195.74.38.228]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 3BF286017F for ; Mon, 28 Oct 2019 03:26:04 +0100 (CET) X-Halon-ID: 4732cd8d-f92a-11e9-903a-005056917f90 Authorized-sender: niklas@soderlund.pp.se Received: from localhost.localdomain (unknown [93.2.121.143]) by bin-vsp-out-02.atm.binero.net (Halon) with ESMTPA id 4732cd8d-f92a-11e9-903a-005056917f90; Mon, 28 Oct 2019 03:26:00 +0100 (CET) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org Date: Mon, 28 Oct 2019 03:25:25 +0100 Message-Id: <20191028022525.796995-13-niklas.soderlund@ragnatech.se> X-Mailer: git-send-email 2.23.0 In-Reply-To: <20191028022525.796995-1-niklas.soderlund@ragnatech.se> References: <20191028022525.796995-1-niklas.soderlund@ragnatech.se> MIME-Version: 1.0 Subject: [libcamera-devel] [RFC 12/12] libcamera: buffer: Clean up after buffer API switch X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 28 Oct 2019 02:26:04 -0000 Remove dead code after the switch to the new buffer API. Signed-off-by: Niklas Söderlund --- include/libcamera/buffer.h | 24 ---- include/libcamera/stream.h | 22 +--- src/libcamera/buffer.cpp | 61 ---------- src/libcamera/stream.cpp | 241 ------------------------------------- 4 files changed, 2 insertions(+), 346 deletions(-) diff --git a/include/libcamera/buffer.h b/include/libcamera/buffer.h index 3568c2e0cdf8d95b..c9887fc6a9aeeb9f 100644 --- a/include/libcamera/buffer.h +++ b/include/libcamera/buffer.h @@ -39,30 +39,6 @@ private: void *mem_; }; -class BufferMemory final -{ -public: - std::vector &planes() { return planes_; } - -private: - std::vector planes_; -}; - -class BufferPool final -{ -public: - ~BufferPool(); - - void createBuffers(unsigned int count); - void destroyBuffers(); - - unsigned int count() const { return buffers_.size(); } - std::vector &buffers() { return buffers_; } - -private: - std::vector buffers_; -}; - class Buffer final { public: diff --git a/include/libcamera/stream.h b/include/libcamera/stream.h index b051341511a7ab7c..5ba1306176fcaeb0 100644 --- a/include/libcamera/stream.h +++ b/include/libcamera/stream.h @@ -48,7 +48,6 @@ struct StreamConfiguration { Size size; MemoryType memoryType; - unsigned int bufferCount; Stream *stream() const { return stream_; } void setStream(Stream *stream) { stream_ = stream; } @@ -75,33 +74,16 @@ public: Stream(); virtual ~Stream(){}; - std::unique_ptr createBuffer(unsigned int index); - std::unique_ptr createBuffer(const std::array &fds); - - BufferPool &bufferPool() { return bufferPool_; } - std::vector &buffers() { return bufferPool_.buffers(); } const StreamConfiguration &configuration() const { return configuration_; } - MemoryType memoryType() const { return memoryType_; } protected: friend class BufferAllocator; friend class Camera; - virtual int allocateBuffers(std::vector *buffers) { return -EINVAL; } - virtual int importBuffers(bool enable) { return -EINVAL; } - - int mapBuffer(const Buffer *buffer); - void unmapBuffer(const Buffer *buffer); - - void createBuffers(MemoryType memory, unsigned int count); - void destroyBuffers(); + virtual int allocateBuffers(std::vector *buffers) = 0; + virtual int importBuffers(bool enable) = 0; - BufferPool bufferPool_; StreamConfiguration configuration_; - MemoryType memoryType_; - -private: - std::vector, unsigned int>> bufferCache_; }; } /* namespace libcamera */ diff --git a/src/libcamera/buffer.cpp b/src/libcamera/buffer.cpp index d00849520bc2b51c..0d14e1d19d7abce5 100644 --- a/src/libcamera/buffer.cpp +++ b/src/libcamera/buffer.cpp @@ -143,67 +143,6 @@ void *Plane::mem() * \return The length of the memory region */ -/** - * \class BufferMemory - * \brief A memory buffer to store an image - * - * The BufferMemory class represents the memory buffers used to store full frame - * images, which may contain multiple separate memory Plane objects if the - * image format is multi-planar. - */ - -/** - * \fn BufferMemory::planes() - * \brief Retrieve the planes within the buffer - * \return A reference to a vector holding all Planes within the buffer - */ - -/** - * \class BufferPool - * \brief A pool of buffers - * - * The BufferPool class groups together a collection of Buffers to store frames. - * The buffers must be exported by a device before they can be imported into - * another device for further use. - */ - -BufferPool::~BufferPool() -{ - destroyBuffers(); -} - -/** - * \brief Create buffers in the Pool - * \param[in] count The number of buffers to create - */ -void BufferPool::createBuffers(unsigned int count) -{ - buffers_.resize(count); -} - -/** - * \brief Release all buffers from pool - * - * If no buffers have been created or if buffers have already been released no - * operation is performed. - */ -void BufferPool::destroyBuffers() -{ - buffers_.resize(0); -} - -/** - * \fn BufferPool::count() - * \brief Retrieve the number of buffers contained within the pool - * \return The number of buffers contained in the pool - */ - -/** - * \fn BufferPool::buffers() - * \brief Retrieve all the buffers in the pool - * \return A vector containing all the buffers in the pool. - */ - Buffer::Buffer(std::vector> planes) { for (std::pair plane : planes) diff --git a/src/libcamera/stream.cpp b/src/libcamera/stream.cpp index b8e7209c10477f16..66d124beada0af0b 100644 --- a/src/libcamera/stream.cpp +++ b/src/libcamera/stream.cpp @@ -423,246 +423,5 @@ Stream::Stream() { } -/** - * \brief Create a Buffer instance referencing the memory buffer \a index - * \param[in] index The desired buffer index - * - * This method creates a Buffer instance that references a BufferMemory from - * the stream's buffers pool by its \a index. The index shall be lower than the - * number of buffers in the pool. - * - * This method is only valid for streams that use the InternalMemory type. It - * will return a null pointer when called on streams using the ExternalMemory - * type. - * - * \return A newly created Buffer on success or nullptr otherwise - */ -std::unique_ptr Stream::createBuffer(unsigned int index) -{ - if (memoryType_ != InternalMemory) { - LOG(Stream, Error) << "Invalid stream memory type"; - return nullptr; - } - - if (index >= bufferPool_.count()) { - LOG(Stream, Error) << "Invalid buffer index " << index; - return nullptr; - } - - Buffer *buffer = new Buffer(); - buffer->index_ = index; - buffer->stream_ = this; - - return std::unique_ptr(buffer); -} - -/** - * \brief Create a Buffer instance that represents a memory area identified by - * dmabuf file descriptors - * \param[in] fds The dmabuf file descriptors for each plane - * - * This method creates a Buffer instance that references buffer memory - * allocated outside of libcamera through dmabuf file descriptors. The \a - * dmabuf array shall contain a file descriptor for each plane in the buffer, - * and unused entries shall be set to -1. - * - * The buffer is created without a valid index, as it does not yet map to any of - * the stream's BufferMemory instances. An index will be assigned at the time - * the buffer is queued to the camera in a request. Applications may thus - * create any number of Buffer instances, providing that no more than the - * number of buffers allocated for the stream are queued at any given time. - * - * This method is only valid for streams that use the ExternalMemory type. It - * will return a null pointer when called on streams using the InternalMemory - * type. - * - * \sa Stream::mapBuffer() - * - * \return A newly created Buffer on success or nullptr otherwise - */ -std::unique_ptr Stream::createBuffer(const std::array &fds) -{ - if (memoryType_ != ExternalMemory) { - LOG(Stream, Error) << "Invalid stream memory type"; - return nullptr; - } - - Buffer *buffer = new Buffer(); - buffer->dmabuf_ = fds; - buffer->stream_ = this; - - return std::unique_ptr(buffer); -} - -/** - * \fn Stream::bufferPool() - * \brief Retrieve the buffer pool for the stream - * - * The buffer pool handles the memory buffers used to store frames for the - * stream. It is initially created empty and shall be populated with - * buffers before being used. - * - * \return A reference to the buffer pool - */ - -/** - * \fn Stream::buffers() - * \brief Retrieve the memory buffers in the Stream's buffer pool - * \return The list of stream's memory buffers - */ - -/** - * \fn Stream::configuration() - * \brief Retrieve the active configuration of the stream - * \return The active configuration of the stream - */ - -/** - * \fn Stream::memoryType() - * \brief Retrieve the stream memory type - * \return The memory type used by the stream - */ - -/** - * \brief Map a Buffer to a buffer memory index - * \param[in] buffer The buffer to map to a buffer memory index - * - * Streams configured to use externally allocated memory need to maintain a - * best-effort association between the memory area the \a buffer represents - * and the associated buffer memory in the Stream's pool. - * - * The buffer memory to use, once the \a buffer reaches the video device, - * is selected using the index assigned to the \a buffer and to minimize - * relocations in the V4L2 back-end, this operation provides a best-effort - * caching mechanism that associates to the dmabuf file descriptors contained - * in the \a buffer the index of the buffer memory that was lastly queued with - * those file descriptors set. - * - * If the Stream uses internally allocated memory, the index of the memory - * buffer to use will match the one request at Stream::createBuffer(unsigned int) - * time, and no mapping is thus required. - * - * \return The buffer memory index for the buffer on success, or a negative - * error code otherwise - * \retval -ENOMEM No buffer memory was available to map the buffer - */ -int Stream::mapBuffer(const Buffer *buffer) -{ - ASSERT(memoryType_ == ExternalMemory); - - if (bufferCache_.empty()) - return -ENOMEM; - - const std::array &dmabufs = buffer->dmabufs(); - - /* - * Try to find a previously mapped buffer in the cache. If we miss, use - * the oldest entry in the cache. - */ - auto map = std::find_if(bufferCache_.begin(), bufferCache_.end(), - [&](std::pair, unsigned int> &entry) { - return entry.first == dmabufs; - }); - if (map == bufferCache_.end()) - map = bufferCache_.begin(); - - /* - * Update the dmabuf file descriptors of the entry. We can't assume that - * identical file descriptor numbers refer to the same dmabuf object as - * it may have been closed and its file descriptor reused. We thus need - * to update the plane's internally cached mmap()ed memory. - */ - unsigned int index = map->second; - BufferMemory *mem = &bufferPool_.buffers()[index]; - mem->planes().clear(); - - for (unsigned int i = 0; i < dmabufs.size(); ++i) { - if (dmabufs[i] == -1) - break; - - mem->planes().emplace_back(); - mem->planes().back().setDmabuf(dmabufs[i], 0); - } - - /* Remove the buffer from the cache and return its index. */ - bufferCache_.erase(map); - return index; -} - -/** - * \brief Unmap a Buffer from its buffer memory - * \param[in] buffer The buffer to unmap - * - * This method releases the buffer memory entry that was mapped by mapBuffer(), - * making it available for new mappings. - */ -void Stream::unmapBuffer(const Buffer *buffer) -{ - ASSERT(memoryType_ == ExternalMemory); - - bufferCache_.emplace_back(buffer->dmabufs(), buffer->index()); -} - -/** - * \brief Create buffers for the stream - * \param[in] count The number of buffers to create - * \param[in] memory The stream memory type - * - * Create \a count empty buffers in the Stream's buffer pool. - */ -void Stream::createBuffers(MemoryType memory, unsigned int count) -{ - destroyBuffers(); - if (count == 0) - return; - - memoryType_ = memory; - bufferPool_.createBuffers(count); - - /* Streams with internal memory usage do not need buffer mapping. */ - if (memoryType_ == InternalMemory) - return; - - /* - * Prepare for buffer mapping by adding all buffer memory entries to the - * cache. - */ - bufferCache_.clear(); - for (unsigned int i = 0; i < bufferPool_.count(); ++i) - bufferCache_.emplace_back(std::array{ -1, -1, -1 }, i); -} - -/** - * \brief Destroy buffers in the stream - * - * If no buffers have been created or if buffers have already been destroyed no - * operation is performed. - */ -void Stream::destroyBuffers() -{ - bufferPool_.destroyBuffers(); -} - -/** - * \var Stream::bufferPool_ - * \brief The pool of buffers associated with the stream - * - * The stream buffer pool is populated by the Camera class after a successful - * stream configuration. - */ - -/** - * \var Stream::configuration_ - * \brief The stream configuration - * - * The configuration for the stream is set by any successful call to - * Camera::configure() that includes the stream, and remains valid until the - * next call to Camera::configure() regardless of if it includes the stream. - */ - -/** - * \var Stream::memoryType_ - * \brief The stream memory type - */ } /* namespace libcamera */