From patchwork Sat Mar 14 23:57:20 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 3094 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 9CEC5628BD for ; Sun, 15 Mar 2020 00:57:40 +0100 (CET) Received: from pendragon.bb.dnainternet.fi (81-175-216-236.bb.dnainternet.fi [81.175.216.236]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id E0C0A1163 for ; Sun, 15 Mar 2020 00:57:39 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1584230260; bh=tS/Qhjaj/Mk9qvlplVy+zXORf+c3A/+tFfh6GxIz5C8=; h=From:To:Subject:Date:In-Reply-To:References:From; b=hcvnmiKFH//6OqJ2xzOKCAarw1J7wh/LhRynvypsgZ+NitEbfXA/W0ekXeO7KF7M0 UisvsN0POEEXjWZ+sbXjl1OMBYsMuQdpkAZ7YdySQe1kk5PzcAIBi0juNvNPAUW8WL zI9GPmN1Cvyn4NhSMeiGNs3fchO1a9JJYFgVaLtQ= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Sun, 15 Mar 2020 01:57:20 +0200 Message-Id: <20200314235728.15495-2-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.24.1 In-Reply-To: <20200314235728.15495-1-laurent.pinchart@ideasonboard.com> References: <20200314235728.15495-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 1/9] test: libtest: buffer_source: Close video device right after allocation 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: Sat, 14 Mar 2020 23:57:40 -0000 There's no need to keep the video device open after allocating buffers, as V4L2 supports buffer orphaning and the exported buffers will still be usable. Close the device right after allocation to avoid the need for delayed cleanups. Signed-off-by: Laurent Pinchart Reviewed-by: Niklas Söderlund --- test/libtest/buffer_source.cpp | 24 +++++++++--------------- test/libtest/buffer_source.h | 1 - 2 files changed, 9 insertions(+), 16 deletions(-) diff --git a/test/libtest/buffer_source.cpp b/test/libtest/buffer_source.cpp index 066049d342a4..0c33200b47ad 100644 --- a/test/libtest/buffer_source.cpp +++ b/test/libtest/buffer_source.cpp @@ -8,26 +8,18 @@ #include "buffer_source.h" #include +#include #include "device_enumerator.h" #include "test.h" BufferSource::BufferSource() - : video_(nullptr) { } BufferSource::~BufferSource() { - if (video_) { - video_->releaseBuffers(); - video_->close(); - } - - delete video_; - video_ = nullptr; - if (media_) media_->release(); } @@ -58,37 +50,39 @@ int BufferSource::allocate(const StreamConfiguration &config) return TestSkip; } - video_ = V4L2VideoDevice::fromEntityName(media_.get(), videoDeviceName); - if (!video_) { + std::unique_ptr video{ V4L2VideoDevice::fromEntityName(media_.get(), videoDeviceName) }; + if (!video) { std::cout << "Failed to get video device from entity " << videoDeviceName << std::endl; return TestFail; } - if (video_->open()) { + if (video->open()) { std::cout << "Unable to open " << videoDeviceName << std::endl; return TestFail; } /* Configure the format. */ V4L2DeviceFormat format; - if (video_->getFormat(&format)) { + if (video->getFormat(&format)) { std::cout << "Failed to get format on output device" << std::endl; return TestFail; } format.size = config.size; format.fourcc = V4L2VideoDevice::toV4L2Fourcc(config.pixelFormat, false); - if (video_->setFormat(&format)) { + if (video->setFormat(&format)) { std::cout << "Failed to set format on output device" << std::endl; return TestFail; } - if (video_->exportBuffers(config.bufferCount, &buffers_) < 0) { + if (video->exportBuffers(config.bufferCount, &buffers_) < 0) { std::cout << "Failed to export buffers" << std::endl; return TestFail; } + video->close(); + return TestPass; } diff --git a/test/libtest/buffer_source.h b/test/libtest/buffer_source.h index 2d8fc5acf6d7..ae0879c99480 100644 --- a/test/libtest/buffer_source.h +++ b/test/libtest/buffer_source.h @@ -25,7 +25,6 @@ public: private: std::shared_ptr media_; - V4L2VideoDevice *video_; std::vector> buffers_; }; From patchwork Sat Mar 14 23:57:21 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 3095 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 09881628BD for ; Sun, 15 Mar 2020 00:57:41 +0100 (CET) Received: from pendragon.bb.dnainternet.fi (81-175-216-236.bb.dnainternet.fi [81.175.216.236]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 9ED9B10CA for ; Sun, 15 Mar 2020 00:57:40 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1584230260; bh=1EUUWoiFfph+nt+smLZHt5PXLTsde666XMOEaCwqOEo=; h=From:To:Subject:Date:In-Reply-To:References:From; b=iwbKD7z5aELApCK1GlriyS8PhP5bnPI8+B7r1aGXevoF3igsIKB8yzc9mJZvR0e61 quUO8op0dVfw191SzSkMi6FJBXFJrhC7+MNEG9vB0s4JVDDffaqZUBk1w0SVpQlySB dYtCtnkGxTUfVZuWlKILSYN0MtHm3ukJEK8fSVak= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Sun, 15 Mar 2020 01:57:21 +0200 Message-Id: <20200314235728.15495-3-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.24.1 In-Reply-To: <20200314235728.15495-1-laurent.pinchart@ideasonboard.com> References: <20200314235728.15495-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 2/9] libcamera: v4l2_videodevice: Rename exportBuffers() to allocateBuffers() 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: Sat, 14 Mar 2020 23:57:41 -0000 To prepare for the rework of buffer allocation that will differentiate export and allocation, rename exportBuffers() to allocateBuffers(). Signed-off-by: Laurent Pinchart Reviewed-by: Niklas Söderlund --- src/libcamera/include/v4l2_videodevice.h | 4 ++-- src/libcamera/pipeline/ipu3/ipu3.cpp | 12 ++++++------ src/libcamera/pipeline/rkisp1/rkisp1.cpp | 6 +++--- src/libcamera/pipeline/uvcvideo.cpp | 2 +- src/libcamera/pipeline/vimc.cpp | 2 +- src/libcamera/v4l2_videodevice.cpp | 4 ++-- test/libtest/buffer_source.cpp | 4 ++-- test/v4l2_videodevice/buffer_sharing.cpp | 2 +- test/v4l2_videodevice/capture_async.cpp | 6 ++++-- test/v4l2_videodevice/request_buffers.cpp | 2 +- test/v4l2_videodevice/stream_on_off.cpp | 2 +- test/v4l2_videodevice/v4l2_m2mdevice.cpp | 8 ++++---- 12 files changed, 28 insertions(+), 26 deletions(-) diff --git a/src/libcamera/include/v4l2_videodevice.h b/src/libcamera/include/v4l2_videodevice.h index b2e12608084a..893d28c7db88 100644 --- a/src/libcamera/include/v4l2_videodevice.h +++ b/src/libcamera/include/v4l2_videodevice.h @@ -191,8 +191,8 @@ public: int setCrop(Rectangle *rect); int setCompose(Rectangle *rect); - int exportBuffers(unsigned int count, - std::vector> *buffers); + int allocateBuffers(unsigned int count, + std::vector> *buffers); int importBuffers(unsigned int count); int releaseBuffers(); diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp index 387bb070b505..10a2698bad09 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -622,7 +622,7 @@ int PipelineHandlerIPU3::exportFrameBuffers(Camera *camera, Stream *stream, V4L2VideoDevice *video = ipu3stream->device_->dev; unsigned int count = stream->configuration().bufferCount; - return video->exportBuffers(count, buffers); + return video->allocateBuffers(count, buffers); } int PipelineHandlerIPU3::importFrameBuffers(Camera *camera, Stream *stream) @@ -678,7 +678,7 @@ int PipelineHandlerIPU3::allocateBuffers(Camera *camera) * the input pool. * \todo To be revised when we'll actually use the stat node. */ - ret = imgu->stat_.dev->exportBuffers(bufferCount, &imgu->stat_.buffers); + ret = imgu->stat_.dev->allocateBuffers(bufferCount, &imgu->stat_.buffers); if (ret < 0) { LOG(IPU3, Error) << "Failed to allocate ImgU stat buffers"; goto error; @@ -691,7 +691,7 @@ int PipelineHandlerIPU3::allocateBuffers(Camera *camera) if (!outStream->active_) { ImgUDevice::ImgUOutput *output = outStream->device_; - ret = output->dev->exportBuffers(bufferCount, &output->buffers); + ret = output->dev->allocateBuffers(bufferCount, &output->buffers); if (ret < 0) { LOG(IPU3, Error) << "Failed to allocate ImgU " << output->name << " buffers"; @@ -702,7 +702,7 @@ int PipelineHandlerIPU3::allocateBuffers(Camera *camera) if (!vfStream->active_) { ImgUDevice::ImgUOutput *output = vfStream->device_; - ret = output->dev->exportBuffers(bufferCount, &output->buffers); + ret = output->dev->allocateBuffers(bufferCount, &output->buffers); if (ret < 0) { LOG(IPU3, Error) << "Failed to allocate ImgU " << output->name << " buffers"; @@ -1423,9 +1423,9 @@ int CIO2Device::configure(const Size &size, */ int CIO2Device::allocateBuffers() { - int ret = output_->exportBuffers(CIO2_BUFFER_COUNT, &buffers_); + int ret = output_->allocateBuffers(CIO2_BUFFER_COUNT, &buffers_); if (ret < 0) - LOG(IPU3, Error) << "Failed to export CIO2 buffers"; + LOG(IPU3, Error) << "Failed to allocate CIO2 buffers"; return ret; } diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index 13433b216747..f6934324c5a3 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -665,7 +665,7 @@ int PipelineHandlerRkISP1::exportFrameBuffers(Camera *camera, Stream *stream, std::vector> *buffers) { unsigned int count = stream->configuration().bufferCount; - return video_->exportBuffers(count, buffers); + return video_->allocateBuffers(count, buffers); } int PipelineHandlerRkISP1::importFrameBuffers(Camera *camera, Stream *stream) @@ -689,11 +689,11 @@ int PipelineHandlerRkISP1::allocateBuffers(Camera *camera) for (const Stream *s : camera->streams()) maxBuffers = std::max(maxBuffers, s->configuration().bufferCount); - ret = param_->exportBuffers(maxBuffers, ¶mBuffers_); + ret = param_->allocateBuffers(maxBuffers, ¶mBuffers_); if (ret < 0) goto error; - ret = stat_->exportBuffers(maxBuffers, &statBuffers_); + ret = stat_->allocateBuffers(maxBuffers, &statBuffers_); if (ret < 0) goto error; diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp index 29afb121aa46..d81627c224ea 100644 --- a/src/libcamera/pipeline/uvcvideo.cpp +++ b/src/libcamera/pipeline/uvcvideo.cpp @@ -199,7 +199,7 @@ int PipelineHandlerUVC::exportFrameBuffers(Camera *camera, Stream *stream, UVCCameraData *data = cameraData(camera); unsigned int count = stream->configuration().bufferCount; - return data->video_->exportBuffers(count, buffers); + return data->video_->allocateBuffers(count, buffers); } int PipelineHandlerUVC::importFrameBuffers(Camera *camera, Stream *stream) diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp index 5d3d12fef30b..bb94ef7fd38d 100644 --- a/src/libcamera/pipeline/vimc.cpp +++ b/src/libcamera/pipeline/vimc.cpp @@ -265,7 +265,7 @@ int PipelineHandlerVimc::exportFrameBuffers(Camera *camera, Stream *stream, VimcCameraData *data = cameraData(camera); unsigned int count = stream->configuration().bufferCount; - return data->video_->exportBuffers(count, buffers); + return data->video_->allocateBuffers(count, buffers); } int PipelineHandlerVimc::importFrameBuffers(Camera *camera, Stream *stream) diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp index f89bf2ff781e..aea7a4ea3a23 100644 --- a/src/libcamera/v4l2_videodevice.cpp +++ b/src/libcamera/v4l2_videodevice.cpp @@ -1049,8 +1049,8 @@ int V4L2VideoDevice::requestBuffers(unsigned int count) * \return The number of allocated buffers on success or a negative error code * otherwise */ -int V4L2VideoDevice::exportBuffers(unsigned int count, - std::vector> *buffers) +int V4L2VideoDevice::allocateBuffers(unsigned int count, + std::vector> *buffers) { if (cache_) { LOG(V4L2, Error) << "Buffers already allocated"; diff --git a/test/libtest/buffer_source.cpp b/test/libtest/buffer_source.cpp index 0c33200b47ad..26d2764d5f8f 100644 --- a/test/libtest/buffer_source.cpp +++ b/test/libtest/buffer_source.cpp @@ -76,8 +76,8 @@ int BufferSource::allocate(const StreamConfiguration &config) return TestFail; } - if (video->exportBuffers(config.bufferCount, &buffers_) < 0) { - std::cout << "Failed to export buffers" << std::endl; + if (video->allocateBuffers(config.bufferCount, &buffers_) < 0) { + std::cout << "Failed to allocate buffers" << std::endl; return TestFail; } diff --git a/test/v4l2_videodevice/buffer_sharing.cpp b/test/v4l2_videodevice/buffer_sharing.cpp index fefa969a5f39..14d3055ad7d1 100644 --- a/test/v4l2_videodevice/buffer_sharing.cpp +++ b/test/v4l2_videodevice/buffer_sharing.cpp @@ -73,7 +73,7 @@ protected: return TestFail; } - ret = capture_->exportBuffers(bufferCount, &buffers_); + ret = capture_->allocateBuffers(bufferCount, &buffers_); if (ret < 0) { std::cout << "Failed to allocate buffers" << std::endl; return TestFail; diff --git a/test/v4l2_videodevice/capture_async.cpp b/test/v4l2_videodevice/capture_async.cpp index 6a103a035f3d..b38aabc6263d 100644 --- a/test/v4l2_videodevice/capture_async.cpp +++ b/test/v4l2_videodevice/capture_async.cpp @@ -38,9 +38,11 @@ protected: Timer timeout; int ret; - ret = capture_->exportBuffers(bufferCount, &buffers_); - if (ret < 0) + ret = capture_->allocateBuffers(bufferCount, &buffers_); + if (ret < 0) { + std::cout << "Failed to allocate buffers" << std::endl; return TestFail; + } capture_->bufferReady.connect(this, &CaptureAsyncTest::receiveBuffer); diff --git a/test/v4l2_videodevice/request_buffers.cpp b/test/v4l2_videodevice/request_buffers.cpp index 1dd65b05da43..2f8dfe1cafb1 100644 --- a/test/v4l2_videodevice/request_buffers.cpp +++ b/test/v4l2_videodevice/request_buffers.cpp @@ -18,7 +18,7 @@ protected: { const unsigned int bufferCount = 8; - int ret = capture_->exportBuffers(bufferCount, &buffers_); + int ret = capture_->allocateBuffers(bufferCount, &buffers_); if (ret != bufferCount) return TestFail; diff --git a/test/v4l2_videodevice/stream_on_off.cpp b/test/v4l2_videodevice/stream_on_off.cpp index 552df0963633..ce48310aa2b7 100644 --- a/test/v4l2_videodevice/stream_on_off.cpp +++ b/test/v4l2_videodevice/stream_on_off.cpp @@ -17,7 +17,7 @@ protected: { const unsigned int bufferCount = 8; - int ret = capture_->exportBuffers(bufferCount, &buffers_); + int ret = capture_->allocateBuffers(bufferCount, &buffers_); if (ret < 0) return TestFail; diff --git a/test/v4l2_videodevice/v4l2_m2mdevice.cpp b/test/v4l2_videodevice/v4l2_m2mdevice.cpp index 203afc4fc033..d20e5dfc3077 100644 --- a/test/v4l2_videodevice/v4l2_m2mdevice.cpp +++ b/test/v4l2_videodevice/v4l2_m2mdevice.cpp @@ -112,15 +112,15 @@ protected: return TestFail; } - ret = capture->exportBuffers(bufferCount, &captureBuffers_); + ret = capture->allocateBuffers(bufferCount, &captureBuffers_); if (ret < 0) { - cerr << "Failed to export Capture Buffers" << endl; + cerr << "Failed to allocate Capture Buffers" << endl; return TestFail; } - ret = output->exportBuffers(bufferCount, &outputBuffers_); + ret = output->allocateBuffers(bufferCount, &outputBuffers_); if (ret < 0) { - cerr << "Failed to export Output Buffers" << endl; + cerr << "Failed to allocate Output Buffers" << endl; return TestFail; } From patchwork Sat Mar 14 23:57:22 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 3096 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 5C8CD628BD for ; Sun, 15 Mar 2020 00:57:41 +0100 (CET) Received: from pendragon.bb.dnainternet.fi (81-175-216-236.bb.dnainternet.fi [81.175.216.236]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id EF7351163 for ; Sun, 15 Mar 2020 00:57:40 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1584230261; bh=e6xscaDEWFpZBhVwPrGz6mQnBSH4JfkxTy8WU1akbNA=; h=From:To:Subject:Date:In-Reply-To:References:From; b=mCoW6YFT8CtZ2VkZ5eEj0ai6iCE6g/kjjhds/53uI9J4wa40jC8ogFA5Cu582MPTW nJNaqnJidY5Drw6uzUnDmvV5NucMoUd/RNHhCYS92IDL1H+IbkS8u91UXcAkgzVgJ1 2c0tVOrCnTy6A9PWQ0L8Alj6PqxDSk428h+X9ou4= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Sun, 15 Mar 2020 01:57:22 +0200 Message-Id: <20200314235728.15495-4-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.24.1 In-Reply-To: <20200314235728.15495-1-laurent.pinchart@ideasonboard.com> References: <20200314235728.15495-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 3/9] libcamera: v4l2_videodevice: Pass memory type to reqbufs() 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: Sat, 14 Mar 2020 23:57:41 -0000 To prepare for the rework of buffer export, pass the memory type explicitly to the V4L2VideoDevice::reqbufs() function. Signed-off-by: Laurent Pinchart Reviewed-by: Niklas Söderlund --- src/libcamera/include/v4l2_videodevice.h | 2 +- src/libcamera/v4l2_videodevice.cpp | 15 ++++++++------- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/src/libcamera/include/v4l2_videodevice.h b/src/libcamera/include/v4l2_videodevice.h index 893d28c7db88..26f5e5917716 100644 --- a/src/libcamera/include/v4l2_videodevice.h +++ b/src/libcamera/include/v4l2_videodevice.h @@ -227,7 +227,7 @@ private: int setSelection(unsigned int target, Rectangle *rect); - int requestBuffers(unsigned int count); + int requestBuffers(unsigned int count, enum v4l2_memory memoryType); std::unique_ptr createBuffer(const struct v4l2_buffer &buf); FileDescriptor exportDmabufFd(unsigned int index, unsigned int plane); diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp index aea7a4ea3a23..6911ab024fd7 100644 --- a/src/libcamera/v4l2_videodevice.cpp +++ b/src/libcamera/v4l2_videodevice.cpp @@ -1013,14 +1013,15 @@ int V4L2VideoDevice::setSelection(unsigned int target, Rectangle *rect) return 0; } -int V4L2VideoDevice::requestBuffers(unsigned int count) +int V4L2VideoDevice::requestBuffers(unsigned int count, + enum v4l2_memory memoryType) { struct v4l2_requestbuffers rb = {}; int ret; rb.count = count; rb.type = bufferType_; - rb.memory = memoryType_; + rb.memory = memoryType; ret = ioctl(VIDIOC_REQBUFS, &rb); if (ret < 0) { @@ -1033,7 +1034,7 @@ int V4L2VideoDevice::requestBuffers(unsigned int count) if (rb.count < count) { LOG(V4L2, Error) << "Not enough buffers provided by V4L2VideoDevice"; - requestBuffers(0); + requestBuffers(0, memoryType); return -ENOMEM; } @@ -1059,7 +1060,7 @@ int V4L2VideoDevice::allocateBuffers(unsigned int count, memoryType_ = V4L2_MEMORY_MMAP; - int ret = requestBuffers(count); + int ret = requestBuffers(count, V4L2_MEMORY_MMAP); if (ret < 0) return ret; @@ -1096,7 +1097,7 @@ int V4L2VideoDevice::allocateBuffers(unsigned int count, return count; err_buf: - requestBuffers(0); + requestBuffers(0, V4L2_MEMORY_MMAP); buffers->clear(); @@ -1166,7 +1167,7 @@ int V4L2VideoDevice::importBuffers(unsigned int count) memoryType_ = V4L2_MEMORY_DMABUF; - int ret = requestBuffers(count); + int ret = requestBuffers(count, V4L2_MEMORY_DMABUF); if (ret) return ret; @@ -1187,7 +1188,7 @@ int V4L2VideoDevice::releaseBuffers() delete cache_; cache_ = nullptr; - return requestBuffers(0); + return requestBuffers(0, memoryType_); } /** From patchwork Sat Mar 14 23:57:23 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 3097 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id BC5D962940 for ; Sun, 15 Mar 2020 00:57:41 +0100 (CET) Received: from pendragon.bb.dnainternet.fi (81-175-216-236.bb.dnainternet.fi [81.175.216.236]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 4BD8F10CA for ; Sun, 15 Mar 2020 00:57:41 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1584230261; bh=ZKmchvz4kyDX2o0XrP57at1LLXhlVJEcZoIsoDsPCVk=; h=From:To:Subject:Date:In-Reply-To:References:From; b=mCp1LaFILALfTP5vvFygx6oaUKa39tx67Q95xmgJlTQr3xUasEhIjMxatZ+H3zfSy 6PX/zODZ5+VSjUD2mH2TIsc7xyrJtVRYlrdoqPt3xg2JyLminVT0clk6o/eUsxySmM C7Mt7edFmB0BoAIuaW4IFHE2ovLRsfz13LcLEOlU= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Sun, 15 Mar 2020 01:57:23 +0200 Message-Id: <20200314235728.15495-5-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.24.1 In-Reply-To: <20200314235728.15495-1-laurent.pinchart@ideasonboard.com> References: <20200314235728.15495-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 4/9] libcamera: v4l2_videodevice: Refactor allocateBuffers() 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: Sat, 14 Mar 2020 23:57:42 -0000 Move the buffer creation code out of allocateBuffers() to a createBuffers() function. This prepare for the rework of buffer export and will avoid code duplication. Signed-off-by: Laurent Pinchart Reviewed-by: Niklas Söderlund --- src/libcamera/include/v4l2_videodevice.h | 4 +- src/libcamera/v4l2_videodevice.cpp | 68 +++++++++++++----------- 2 files changed, 39 insertions(+), 33 deletions(-) diff --git a/src/libcamera/include/v4l2_videodevice.h b/src/libcamera/include/v4l2_videodevice.h index 26f5e5917716..358d89e57414 100644 --- a/src/libcamera/include/v4l2_videodevice.h +++ b/src/libcamera/include/v4l2_videodevice.h @@ -228,7 +228,9 @@ private: int setSelection(unsigned int target, Rectangle *rect); int requestBuffers(unsigned int count, enum v4l2_memory memoryType); - std::unique_ptr createBuffer(const struct v4l2_buffer &buf); + int createBuffers(unsigned int count, + std::vector> *buffers); + std::unique_ptr createBuffer(unsigned int index); FileDescriptor exportDmabufFd(unsigned int index, unsigned int plane); void bufferAvailable(EventNotifier *notifier); diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp index 6911ab024fd7..d02b02ef77d6 100644 --- a/src/libcamera/v4l2_videodevice.cpp +++ b/src/libcamera/v4l2_videodevice.cpp @@ -1052,61 +1052,65 @@ int V4L2VideoDevice::requestBuffers(unsigned int count, */ int V4L2VideoDevice::allocateBuffers(unsigned int count, std::vector> *buffers) +{ + int ret = createBuffers(count, buffers); + if (ret < 0) + return ret; + + cache_ = new V4L2BufferCache(*buffers); + memoryType_ = V4L2_MEMORY_MMAP; + + return ret; +} + +int V4L2VideoDevice::createBuffers(unsigned int count, + std::vector> *buffers) { if (cache_) { LOG(V4L2, Error) << "Buffers already allocated"; return -EINVAL; } - memoryType_ = V4L2_MEMORY_MMAP; - int ret = requestBuffers(count, V4L2_MEMORY_MMAP); if (ret < 0) return ret; for (unsigned i = 0; i < count; ++i) { - struct v4l2_buffer buf = {}; - struct v4l2_plane planes[VIDEO_MAX_PLANES] = {}; - - buf.index = i; - buf.type = bufferType_; - buf.memory = memoryType_; - buf.length = ARRAY_SIZE(planes); - buf.m.planes = planes; - - ret = ioctl(VIDIOC_QUERYBUF, &buf); - if (ret < 0) { - LOG(V4L2, Error) - << "Unable to query buffer " << i << ": " - << strerror(-ret); - goto err_buf; - } - - std::unique_ptr buffer = createBuffer(buf); + std::unique_ptr buffer = createBuffer(i); if (!buffer) { LOG(V4L2, Error) << "Unable to create buffer"; - ret = -EINVAL; - goto err_buf; + + requestBuffers(0, V4L2_MEMORY_MMAP); + buffers->clear(); + + return -EINVAL; } buffers->push_back(std::move(buffer)); } - cache_ = new V4L2BufferCache(*buffers); - return count; +} -err_buf: - requestBuffers(0, V4L2_MEMORY_MMAP); +std::unique_ptr V4L2VideoDevice::createBuffer(unsigned int index) +{ + struct v4l2_plane v4l2Planes[VIDEO_MAX_PLANES] = {}; + struct v4l2_buffer buf = {}; - buffers->clear(); + buf.index = index; + buf.type = bufferType_; + buf.memory = V4L2_MEMORY_MMAP; + buf.length = ARRAY_SIZE(v4l2Planes); + buf.m.planes = v4l2Planes; - return ret; -} + int ret = ioctl(VIDIOC_QUERYBUF, &buf); + if (ret < 0) { + LOG(V4L2, Error) + << "Unable to query buffer " << index << ": " + << strerror(-ret); + return nullptr; + } -std::unique_ptr -V4L2VideoDevice::createBuffer(const struct v4l2_buffer &buf) -{ const bool multiPlanar = V4L2_TYPE_IS_MULTIPLANAR(buf.type); const unsigned int numPlanes = multiPlanar ? buf.length : 1; From patchwork Sat Mar 14 23:57:24 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 3098 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id F313B62923 for ; Sun, 15 Mar 2020 00:57:41 +0100 (CET) Received: from pendragon.bb.dnainternet.fi (81-175-216-236.bb.dnainternet.fi [81.175.216.236]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 9BEF11163 for ; Sun, 15 Mar 2020 00:57:41 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1584230261; bh=ezADNw/o4D4Pet+bQ8YSJFwhP5rJfvw5vP4b3ecqyXw=; h=From:To:Subject:Date:In-Reply-To:References:From; b=fiMpjcL3rvElcjgP3DrYx2X1ShxpmoBS69WLor5WtFyqsvQOEKl71ZHtNpkHRkU/l 2YKSTqkoL6z7nx5k3rNIyeyxNIWuFihfGZy9t55UasPoZ2hsKPWP5OhLTEbL8rbZf+ KyfhHZicXQjlvTYlX0Hs0a+XwQb/7IUAmc/zi6mw= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Sun, 15 Mar 2020 01:57:24 +0200 Message-Id: <20200314235728.15495-6-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.24.1 In-Reply-To: <20200314235728.15495-1-laurent.pinchart@ideasonboard.com> References: <20200314235728.15495-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 5/9] libcamera: v4l2_videodevice: Add standalone buffer export support 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: Sat, 14 Mar 2020 23:57:42 -0000 Add a new exportBuffers() function that only performs buffer allocation and export, but leaves the V4L2 buffer queue unallocated on return. This function will be used to simplify buffer allocation for pipeline handlers. This is made possible by the V4L2 buffer orphaning feature introduced in Linux v5.0, so add a version check to catch and report issues early. Signed-off-by: Laurent Pinchart Reviewed-by: Niklas Söderlund --- src/libcamera/include/v4l2_videodevice.h | 2 + src/libcamera/v4l2_videodevice.cpp | 140 ++++++++++++++++++++++- 2 files changed, 140 insertions(+), 2 deletions(-) diff --git a/src/libcamera/include/v4l2_videodevice.h b/src/libcamera/include/v4l2_videodevice.h index 358d89e57414..16fc6b60de02 100644 --- a/src/libcamera/include/v4l2_videodevice.h +++ b/src/libcamera/include/v4l2_videodevice.h @@ -193,6 +193,8 @@ public: int allocateBuffers(unsigned int count, std::vector> *buffers); + int exportBuffers(unsigned int count, + std::vector> *buffers); int importBuffers(unsigned int count); int releaseBuffers(); diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp index d02b02ef77d6..e37136af8eb7 100644 --- a/src/libcamera/v4l2_videodevice.cpp +++ b/src/libcamera/v4l2_videodevice.cpp @@ -19,6 +19,7 @@ #include #include +#include #include #include @@ -404,6 +405,53 @@ const std::string V4L2DeviceFormat::toString() const * No API call other than open(), isOpen() and close() shall be called on an * unopened device instance. * + * The V4L2VideoDevice class supports the V4L2 MMAP and DMABUF memory types: + * + * - The allocateBuffers() function wraps buffer allocation with the V4L2 MMAP + * memory type. It requests buffers from the driver, allocating the + * corresponding memory, and exports them as a set of FrameBuffer objects. + * Upon successful return the driver's internal buffer management is + * initialized in MMAP mode, and the video device is ready to accept + * queueBuffer() calls. + * + * This is the most traditional V4L2 buffer management, and is mostly useful + * to support internal buffer pools in pipeline handlers, either for CPU + * consumption (such as statistics or parameters pools), or for internal + * image buffers shared between devices. + * + * - The exportBuffers() function operates similarly to allocateBuffers(), but + * leaves the driver's internal buffer management uninitialized. It uses the + * V4L2 buffer orphaning support to allocate buffers with the MMAP method, + * export them as a set of FrameBuffer objects, and reset the driver's + * internal buffer management. The video device shall be initialized with + * importBuffers() or allocateBuffers() before it can accept queueBuffer() + * calls. The exported buffers are directly usable with any V4L2 video device + * in DMABUF mode, or with other dmabuf importers. + * + * This method is mostly useful to implement buffer allocation helpers or to + * allocate ancillary buffers, when a V4L2 video device is used in DMABUF + * mode but no other source of buffers is available. An example use case + * would be allocation of scratch buffers to be used in case of buffer + * underruns on a video device that is otherwise supplied with external + * buffers. + * + * - The importBuffers() function initializes the driver's buffer management to + * import buffers in DMABUF mode. It requests buffers from the driver, but + * doesn't allocate memory. Upon successful return, the video device is ready + * to accept queueBuffer() calls. The buffers to be imported are provided to + * queueBuffer(), and may be supplied externally, or come from a previous + * exportBuffers() call. + * + * This is the usual buffers initialization method for video devices whose + * buffers are exposed outside of libcamera. It is also typically used on one + * of the two video device that participate in buffer sharing inside + * pipelines, the other video device typically using allocateBuffers(). + * + * - The releaseBuffers() function resets the driver's internal buffer + * management that was initialized by a previous call to allocateBuffers() or + * importBuffers(). Any memory allocated by allocateBuffers() is freed. + * Buffer exported by exportBuffers() are not affected by this function. + * * The V4L2VideoDevice class tracks queued buffers and handles buffer events. It * automatically dequeues completed buffers and emits the \ref bufferReady * signal. @@ -466,6 +514,15 @@ int V4L2VideoDevice::open() return ret; } + if (caps_.version < KERNEL_VERSION(5, 0, 0)) { + LOG(V4L2, Error) + << "V4L2 API v" << (caps_.version >> 16) + << "." << ((caps_.version >> 8) & 0xff) + << "." << (caps_.version & 0xff) + << " too old, v5.0.0 or later is required"; + return -EINVAL; + } + if (!caps_.hasStreaming()) { LOG(V4L2, Error) << "Device does not support streaming I/O"; return -EINVAL; @@ -1044,11 +1101,27 @@ int V4L2VideoDevice::requestBuffers(unsigned int count, } /** - * \brief Allocate buffers from the video device + * \brief Allocate and export buffers from the video device * \param[in] count Number of buffers to allocate * \param[out] buffers Vector to store allocated buffers + * + * This function wraps buffer allocation with the V4L2 MMAP memory type. It + * requests \a count buffers from the driver, allocating the corresponding + * memory, and exports them as a set of FrameBuffer objects in \a buffers. Upon + * successful return the driver's internal buffer management is initialized in + * MMAP mode, and the video device is ready to accept queueBuffer() calls. + * + * The number of planes and the plane sizes for the allocation are determined + * by the currently active format on the device as set by setFormat(). + * + * Buffers allocated with this function shall later be free with + * releaseBuffers(). If buffers have already been allocated with + * allocateBuffers() or imported with importBuffers(), this function returns + * -EBUSY. + * * \return The number of allocated buffers on success or a negative error code * otherwise + * \retval -EBUSY buffers have already been allocated or imported */ int V4L2VideoDevice::allocateBuffers(unsigned int count, std::vector> *buffers) @@ -1063,6 +1136,49 @@ int V4L2VideoDevice::allocateBuffers(unsigned int count, return ret; } +/** + * \brief Export buffers from the video device + * \param[in] count Number of buffers to allocate + * \param[out] buffers Vector to store allocated buffers + * + * This function allocates \a count buffer from the video device and exports + * them as dmabuf objects, stored in \a buffers. Unlike allocateBuffers(), this + * function leaves the driver's internal buffer management uninitialized. The + * video device shall be initialized with importBuffers() or allocateBuffers() + * before it can accept queueBuffer() calls. The exported buffers are directly + * usable with any V4L2 video device in DMABUF mode, or with other dmabuf + * importers. + * + * The number of planes and the plane sizes for the allocation are determined + * by the currently active format on the device as set by setFormat(). + * + * Multiple independent sets of buffers can be allocated with multiple calls to + * this function. Device-specific limitations may apply regarding the minimum + * and maximum number of buffers per set, or to total amount of allocated + * memory. The exported dmabuf lifetime is tied to the returned \a buffers. To + * free a buffer, the caller shall delete the corresponding FrameBuffer + * instance. No bookkeeping and automatic free is performed by the + * V4L2VideoDevice class. + * + * If buffers have already been allocated with allocateBuffers() or imported + * with importBuffers(), this function returns -EBUSY. + * + * \return The number of allocated buffers on success or a negative error code + * otherwise + * \retval -EBUSY buffers have already been allocated or imported + */ +int V4L2VideoDevice::exportBuffers(unsigned int count, + std::vector> *buffers) +{ + int ret = createBuffers(count, buffers); + if (ret < 0) + return ret; + + requestBuffers(0, V4L2_MEMORY_MMAP); + + return ret; +} + int V4L2VideoDevice::createBuffers(unsigned int count, std::vector> *buffers) { @@ -1160,7 +1276,22 @@ FileDescriptor V4L2VideoDevice::exportDmabufFd(unsigned int index, /** * \brief Prepare the device to import \a count buffers * \param[in] count Number of buffers to prepare to import + * + * This function initializes the driver's buffer management to import buffers + * in DMABUF mode. It requests buffers from the driver, but doesn't allocate + * memory. + * + * Upon successful return, the video device is ready to accept queueBuffer() + * calls. The buffers to be imported are provided to queueBuffer(), and may be + * supplied externally, or come from a previous exportBuffers() call. + * + * Device initialization performed by this function shall later be cleaned up + * with releaseBuffers(). If buffers have already been allocated with + * allocateBuffers() or imported with importBuffers(), this function returns + * -EBUSY. + * * \return 0 on success or a negative error code otherwise + * \retval -EBUSY buffers have already been allocated or imported */ int V4L2VideoDevice::importBuffers(unsigned int count) { @@ -1183,7 +1314,12 @@ int V4L2VideoDevice::importBuffers(unsigned int count) } /** - * \brief Release all internally allocated buffers + * \brief Release resources allocated by importBuffers() + * + * This function resets the driver's internal buffer management that was + * initialized by a previous call to allocateBuffers() or importBuffers(). Any + * memory allocated by allocateBuffers() is freed. Buffer exported by + * exportBuffers(), if any, are not affected. */ int V4L2VideoDevice::releaseBuffers() { From patchwork Sat Mar 14 23:57:25 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 3099 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 59C1062944 for ; Sun, 15 Mar 2020 00:57:42 +0100 (CET) Received: from pendragon.bb.dnainternet.fi (81-175-216-236.bb.dnainternet.fi [81.175.216.236]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id EB8DE10CA for ; Sun, 15 Mar 2020 00:57:41 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1584230262; bh=20TSaj08OwTg/qymb9XsHYDz17qD53Z5bgEU6Xr9v5w=; h=From:To:Subject:Date:In-Reply-To:References:From; b=ZT/8qzbHcWAd6h0YvN1y4xAch0N7Lpc7E6LnxPi/HnHV3me+OVTQh9kXbR/0hyLEW GSSkBtIf2tXtkF93kBwYvP5TIdQRW20D04v2L5ZtU0KD5zISyqE4XjOxTAYpMH20Fk QcHXsb6URpTEKxzHK3Ww6eC7pGTy2l31YorgY7lg= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Sun, 15 Mar 2020 01:57:25 +0200 Message-Id: <20200314235728.15495-7-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.24.1 In-Reply-To: <20200314235728.15495-1-laurent.pinchart@ideasonboard.com> References: <20200314235728.15495-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 6/9] libcamera: camera: Propagate error value from importFrameBuffer 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: Sat, 14 Mar 2020 23:57:42 -0000 The PipelineHandler::importFrameBuffer() function, called by Camera::start() may return an error, but its return value is ignored. Propagate it to the caller to fix this. Signed-off-by: Laurent Pinchart Reviewed-by: Niklas Söderlund --- src/libcamera/camera.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp index 63b1f7729937..9c432adb1d97 100644 --- a/src/libcamera/camera.cpp +++ b/src/libcamera/camera.cpp @@ -931,8 +931,10 @@ int Camera::start() if (allocator_ && !allocator_->buffers(stream).empty()) continue; - p_->pipe_->invokeMethod(&PipelineHandler::importFrameBuffers, - ConnectionTypeDirect, this, stream); + ret = p_->pipe_->invokeMethod(&PipelineHandler::importFrameBuffers, + ConnectionTypeDirect, this, stream); + if (ret < 0) + return ret; } ret = p_->pipe_->invokeMethod(&PipelineHandler::start, From patchwork Sat Mar 14 23:57:26 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 3100 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id AE9BF62948 for ; Sun, 15 Mar 2020 00:57:42 +0100 (CET) Received: from pendragon.bb.dnainternet.fi (81-175-216-236.bb.dnainternet.fi [81.175.216.236]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 48AD21163 for ; Sun, 15 Mar 2020 00:57:42 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1584230262; bh=4AldzKHX3fs3lMoI0VJvST0CJIkkeFe56cH0WOUeeHU=; h=From:To:Subject:Date:In-Reply-To:References:From; b=alVadJvC+MM8KMQiICJhMxJBJ5j8YjuTAo+odOQj4LL9Yr/4LsyZhElfJZaN1ywID rJ18EB1oOoVR5SbZ5XdY8WKZwVdREKDHyRliVSnsptZyvNVtjEkRBfimfE6eThPwWm QOovo1A4zYE+80EK+Q9TWz3jsoYHS5y0TyeVC48U= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Sun, 15 Mar 2020 01:57:26 +0200 Message-Id: <20200314235728.15495-8-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.24.1 In-Reply-To: <20200314235728.15495-1-laurent.pinchart@ideasonboard.com> References: <20200314235728.15495-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 7/9] libcamera: pipeline_handler: Decouple buffer import and export 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: Sat, 14 Mar 2020 23:57:43 -0000 Use the V4L2 buffer orphaning feature, exposed through V4L2VideoDevice::exportBuffers(), to decouple buffer import and export. The PipelineHandler::importFrameBuffers() function is now called for all streams regardless of whether exportFrameBuffers() has been called or not. This simplifies the Camera implementation slightly, and opens the door to additional simplifications. Signed-off-by: Laurent Pinchart Reviewed-by: Niklas Söderlund --- include/libcamera/camera.h | 1 - src/libcamera/camera.cpp | 21 +------------------- src/libcamera/framebuffer_allocator.cpp | 9 --------- src/libcamera/pipeline/ipu3/ipu3.cpp | 2 +- src/libcamera/pipeline/rkisp1/rkisp1.cpp | 2 +- src/libcamera/pipeline/uvcvideo.cpp | 2 +- src/libcamera/pipeline/vimc.cpp | 2 +- src/libcamera/pipeline_handler.cpp | 25 +++++------------------- 8 files changed, 10 insertions(+), 54 deletions(-) diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h index c453b95228a7..a5e2b49f0f25 100644 --- a/include/libcamera/camera.h +++ b/include/libcamera/camera.h @@ -113,7 +113,6 @@ private: friend class FrameBufferAllocator; int exportFrameBuffers(Stream *stream, std::vector> *buffers); - int freeFrameBuffers(Stream *stream); /* \todo Remove allocator_ from the exposed API */ FrameBufferAllocator *allocator_; }; diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp index 9c432adb1d97..3192dfb42d01 100644 --- a/src/libcamera/camera.cpp +++ b/src/libcamera/camera.cpp @@ -554,18 +554,6 @@ int Camera::exportFrameBuffers(Stream *stream, buffers); } -int Camera::freeFrameBuffers(Stream *stream) -{ - int ret = p_->isAccessAllowed(Private::CameraConfigured, true); - if (ret < 0) - return ret; - - p_->pipe_->invokeMethod(&PipelineHandler::freeFrameBuffers, - ConnectionTypeBlocking, this, stream); - - return 0; -} - /** * \brief Acquire the camera device for exclusive access * @@ -928,9 +916,6 @@ int Camera::start() LOG(Camera, Debug) << "Starting capture"; for (Stream *stream : p_->activeStreams_) { - if (allocator_ && !allocator_->buffers(stream).empty()) - continue; - ret = p_->pipe_->invokeMethod(&PipelineHandler::importFrameBuffers, ConnectionTypeDirect, this, stream); if (ret < 0) @@ -974,13 +959,9 @@ int Camera::stop() p_->pipe_->invokeMethod(&PipelineHandler::stop, ConnectionTypeBlocking, this); - for (Stream *stream : p_->activeStreams_) { - if (allocator_ && !allocator_->buffers(stream).empty()) - continue; - + for (Stream *stream : p_->activeStreams_) p_->pipe_->invokeMethod(&PipelineHandler::freeFrameBuffers, ConnectionTypeBlocking, this, stream); - } return 0; } diff --git a/src/libcamera/framebuffer_allocator.cpp b/src/libcamera/framebuffer_allocator.cpp index e79f4a8f65c8..6f7a2e90b08a 100644 --- a/src/libcamera/framebuffer_allocator.cpp +++ b/src/libcamera/framebuffer_allocator.cpp @@ -87,11 +87,6 @@ FrameBufferAllocator::FrameBufferAllocator(std::shared_ptr camera) FrameBufferAllocator::~FrameBufferAllocator() { - for (auto &value : buffers_) { - Stream *stream = value.first; - camera_->freeFrameBuffers(stream); - } - buffers_.clear(); camera_->allocator_ = nullptr; @@ -148,10 +143,6 @@ int FrameBufferAllocator::free(Stream *stream) if (iter == buffers_.end()) return -EINVAL; - int ret = camera_->freeFrameBuffers(stream); - if (ret < 0) - return ret; - std::vector> &buffers = iter->second; buffers.clear(); buffers_.erase(iter); diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp index 10a2698bad09..b6db8d567ea4 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -622,7 +622,7 @@ int PipelineHandlerIPU3::exportFrameBuffers(Camera *camera, Stream *stream, V4L2VideoDevice *video = ipu3stream->device_->dev; unsigned int count = stream->configuration().bufferCount; - return video->allocateBuffers(count, buffers); + return video->exportBuffers(count, buffers); } int PipelineHandlerIPU3::importFrameBuffers(Camera *camera, Stream *stream) diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index f6934324c5a3..1ad53fbde112 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -665,7 +665,7 @@ int PipelineHandlerRkISP1::exportFrameBuffers(Camera *camera, Stream *stream, std::vector> *buffers) { unsigned int count = stream->configuration().bufferCount; - return video_->allocateBuffers(count, buffers); + return video_->exportBuffers(count, buffers); } int PipelineHandlerRkISP1::importFrameBuffers(Camera *camera, Stream *stream) diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp index d81627c224ea..29afb121aa46 100644 --- a/src/libcamera/pipeline/uvcvideo.cpp +++ b/src/libcamera/pipeline/uvcvideo.cpp @@ -199,7 +199,7 @@ int PipelineHandlerUVC::exportFrameBuffers(Camera *camera, Stream *stream, UVCCameraData *data = cameraData(camera); unsigned int count = stream->configuration().bufferCount; - return data->video_->allocateBuffers(count, buffers); + return data->video_->exportBuffers(count, buffers); } int PipelineHandlerUVC::importFrameBuffers(Camera *camera, Stream *stream) diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp index bb94ef7fd38d..5d3d12fef30b 100644 --- a/src/libcamera/pipeline/vimc.cpp +++ b/src/libcamera/pipeline/vimc.cpp @@ -265,7 +265,7 @@ int PipelineHandlerVimc::exportFrameBuffers(Camera *camera, Stream *stream, VimcCameraData *data = cameraData(camera); unsigned int count = stream->configuration().bufferCount; - return data->video_->allocateBuffers(count, buffers); + return data->video_->exportBuffers(count, buffers); } int PipelineHandlerVimc::importFrameBuffers(Camera *camera, Stream *stream) diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp index 8d623f5431fa..e5034c54e2fb 100644 --- a/src/libcamera/pipeline_handler.cpp +++ b/src/libcamera/pipeline_handler.cpp @@ -337,16 +337,7 @@ const ControlList &PipelineHandler::properties(Camera *camera) * * The method may only be called after the Camera has been configured and before * it gets started, or after it gets stopped. It shall be called only for - * streams that are part of the active camera configuration, and at most once - * per stream until buffers for the stream are freed with freeFrameBuffers(). - * - * exportFrameBuffers() shall also allocate all other resources required by - * the pipeline handler for the stream to prepare for starting the Camera. This - * responsibility is shared with importFrameBuffers(), and one and only one of - * those two methods shall be called for each stream until the buffers are - * freed. The pipeline handler shall support all combinations of - * exportFrameBuffers() and importFrameBuffers() for the streams contained in - * any camera configuration. + * streams that are part of the active camera configuration. * * The only intended caller is Camera::exportFrameBuffers(). * @@ -371,12 +362,7 @@ const ControlList &PipelineHandler::properties(Camera *camera) * per stream until buffers for the stream are freed with freeFrameBuffers(). * * importFrameBuffers() shall also allocate all other resources required by the - * pipeline handler for the stream to prepare for starting the Camera. This - * responsibility is shared with exportFrameBuffers(), and one and only one of - * those two methods shall be called for each stream until the buffers are - * freed. The pipeline handler shall support all combinations of - * exportFrameBuffers() and importFrameBuffers() for the streams contained in - * any camera configuration. + * pipeline handler for the stream to prepare for starting the Camera. * * The only intended caller is Camera::start(). * @@ -391,10 +377,9 @@ const ControlList &PipelineHandler::properties(Camera *camera) * \param[in] camera The camera * \param[in] stream The stream to free buffers for * - * This method shall free all buffers and all other resources allocated for the - * \a stream by exportFrameBuffers() or importFrameBuffers(). It shall be - * called only after a successful call to either of these two methods, and only - * once per stream. + * This method shall release all resources allocated for the \a stream by + * importFrameBuffers(). It shall be called only after a successful call that + * method, and only once per stream. * * The only intended callers are Camera::stop() and Camera::freeFrameBuffers(). * From patchwork Sat Mar 14 23:57:27 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 3101 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 046DA62944 for ; Sun, 15 Mar 2020 00:57:43 +0100 (CET) Received: from pendragon.bb.dnainternet.fi (81-175-216-236.bb.dnainternet.fi [81.175.216.236]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 97B87121F for ; Sun, 15 Mar 2020 00:57:42 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1584230262; bh=ZXpQxE+RBSOk4eFuzWGen7dFq2kNAjCjqHDCxMp1A34=; h=From:To:Subject:Date:In-Reply-To:References:From; b=gRqhdzqBefhd5q6zR/uhey5zOd+Jpm5G5KG3OhMIPmH1xGqSI8/czFuhoKTRrKWmh QVSz64y9s18W/Kk3Ag9psljZxg0F8qDF2SRkHdIlntX6/NGKahFl48do3MR59/qZt4 phMlQ5IArM8uFTh+eiqu5TduE2CgMVigB+oCryQg= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Sun, 15 Mar 2020 01:57:27 +0200 Message-Id: <20200314235728.15495-9-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.24.1 In-Reply-To: <20200314235728.15495-1-laurent.pinchart@ideasonboard.com> References: <20200314235728.15495-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 8/9] libcamera: pipeline_handler: Fold buffer management with start/stop 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: Sat, 14 Mar 2020 23:57:43 -0000 There's no need anymore to have the Camera object control how and when pipeline handlers allocate and free the buffers for the application-facing video devices. Fold those operations, currently performed by importFrameBuffers() and freeFrameBuffers(), into the start() and stop() functions. This simplifies the pipeline handler API, its implementation, and the implementation of the Camera class. Signed-off-by: Laurent Pinchart Reviewed-by: Niklas Söderlund --- src/libcamera/camera.cpp | 11 -- src/libcamera/include/pipeline_handler.h | 2 - src/libcamera/pipeline/ipu3/ipu3.cpp | 146 +++++++++++------------ src/libcamera/pipeline/rkisp1/rkisp1.cpp | 20 ++-- src/libcamera/pipeline/uvcvideo.cpp | 28 ++--- src/libcamera/pipeline/vimc.cpp | 28 ++--- src/libcamera/pipeline_handler.cpp | 41 +------ 7 files changed, 101 insertions(+), 175 deletions(-) diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp index 3192dfb42d01..5593c1b317a0 100644 --- a/src/libcamera/camera.cpp +++ b/src/libcamera/camera.cpp @@ -915,13 +915,6 @@ int Camera::start() LOG(Camera, Debug) << "Starting capture"; - for (Stream *stream : p_->activeStreams_) { - ret = p_->pipe_->invokeMethod(&PipelineHandler::importFrameBuffers, - ConnectionTypeDirect, this, stream); - if (ret < 0) - return ret; - } - ret = p_->pipe_->invokeMethod(&PipelineHandler::start, ConnectionTypeBlocking, this); if (ret) @@ -959,10 +952,6 @@ int Camera::stop() p_->pipe_->invokeMethod(&PipelineHandler::stop, ConnectionTypeBlocking, this); - for (Stream *stream : p_->activeStreams_) - p_->pipe_->invokeMethod(&PipelineHandler::freeFrameBuffers, - ConnectionTypeBlocking, this, stream); - return 0; } diff --git a/src/libcamera/include/pipeline_handler.h b/src/libcamera/include/pipeline_handler.h index db6c3104d812..3fcfeda4bfee 100644 --- a/src/libcamera/include/pipeline_handler.h +++ b/src/libcamera/include/pipeline_handler.h @@ -76,8 +76,6 @@ public: virtual int exportFrameBuffers(Camera *camera, Stream *stream, std::vector> *buffers) = 0; - virtual int importFrameBuffers(Camera *camera, Stream *stream) = 0; - virtual void freeFrameBuffers(Camera *camera, Stream *stream) = 0; virtual int start(Camera *camera) = 0; virtual void stop(Camera *camera) = 0; diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp index b6db8d567ea4..6b93c50978a7 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -72,6 +72,7 @@ public: int configureOutput(ImgUOutput *output, const StreamConfiguration &cfg); + int allocateBuffers(IPU3CameraData *data, unsigned int bufferCount); void freeBuffers(IPU3CameraData *data); int start(); @@ -208,8 +209,6 @@ public: int exportFrameBuffers(Camera *camera, Stream *stream, std::vector> *buffers) override; - int importFrameBuffers(Camera *camera, Stream *stream) override; - void freeFrameBuffers(Camera *camera, Stream *stream) override; int start(Camera *camera) override; void stop(Camera *camera) override; @@ -625,23 +624,6 @@ int PipelineHandlerIPU3::exportFrameBuffers(Camera *camera, Stream *stream, return video->exportBuffers(count, buffers); } -int PipelineHandlerIPU3::importFrameBuffers(Camera *camera, Stream *stream) -{ - IPU3Stream *ipu3stream = static_cast(stream); - V4L2VideoDevice *video = ipu3stream->device_->dev; - unsigned int count = stream->configuration().bufferCount; - - return video->importBuffers(count); -} - -void PipelineHandlerIPU3::freeFrameBuffers(Camera *camera, Stream *stream) -{ - IPU3Stream *ipu3stream = static_cast(stream); - V4L2VideoDevice *video = ipu3stream->device_->dev; - - video->releaseBuffers(); -} - /** * \todo Clarify if 'viewfinder' and 'stat' nodes have to be set up and * started even if not in use. As of now, if not properly configured and @@ -653,69 +635,24 @@ void PipelineHandlerIPU3::freeFrameBuffers(Camera *camera, Stream *stream) int PipelineHandlerIPU3::allocateBuffers(Camera *camera) { 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. */ ret = cio2->allocateBuffers(); if (ret < 0) return ret; bufferCount = ret; - ret = imgu->input_->importBuffers(bufferCount); - if (ret) { - LOG(IPU3, Error) << "Failed to import ImgU input buffers"; - goto error; - } - - /* - * Use for the stat's internal pool the same number of buffers as for - * the input pool. - * \todo To be revised when we'll actually use the stat node. - */ - ret = imgu->stat_.dev->allocateBuffers(bufferCount, &imgu->stat_.buffers); + ret = imgu->allocateBuffers(data, bufferCount); if (ret < 0) { - LOG(IPU3, Error) << "Failed to allocate ImgU stat buffers"; - goto error; - } - - /* - * Allocate buffers also on non-active outputs; use the same number - * of buffers as the active ones. - */ - if (!outStream->active_) { - ImgUDevice::ImgUOutput *output = outStream->device_; - - ret = output->dev->allocateBuffers(bufferCount, &output->buffers); - if (ret < 0) { - LOG(IPU3, Error) << "Failed to allocate ImgU " - << output->name << " buffers"; - goto error; - } - } - - if (!vfStream->active_) { - ImgUDevice::ImgUOutput *output = vfStream->device_; - - ret = output->dev->allocateBuffers(bufferCount, &output->buffers); - if (ret < 0) { - LOG(IPU3, Error) << "Failed to allocate ImgU " - << output->name << " buffers"; - goto error; - } + cio2->freeBuffers(); + return ret; } return 0; - -error: - freeBuffers(camera); - - return ret; } int PipelineHandlerIPU3::freeBuffers(Camera *camera) @@ -1156,6 +1093,65 @@ int ImgUDevice::configureOutput(ImgUOutput *output, return 0; } +/** + * \brief Allocate buffers for all the ImgU video devices + */ +int ImgUDevice::allocateBuffers(IPU3CameraData *data, unsigned int bufferCount) +{ + IPU3Stream *outStream = &data->outStream_; + IPU3Stream *vfStream = &data->vfStream_; + + /* Share buffers between CIO2 output and ImgU input. */ + int ret = input_->importBuffers(bufferCount); + if (ret) { + LOG(IPU3, Error) << "Failed to import ImgU input buffers"; + return ret; + } + + /* + * Use for the stat's internal pool the same number of buffers as for + * the input pool. + * \todo To be revised when we'll actually use the stat node. + */ + ret = stat_.dev->allocateBuffers(bufferCount, &stat_.buffers); + if (ret < 0) { + LOG(IPU3, Error) << "Failed to allocate ImgU stat buffers"; + goto error; + } + + /* + * Allocate buffers for both outputs. If an output is active, prepare + * for buffer import, otherwise allocate internal buffers. Use the same + * number of buffers in either case. + */ + if (outStream->active_) + ret = output_.dev->importBuffers(bufferCount); + else + ret = output_.dev->allocateBuffers(bufferCount, + &output_.buffers); + if (ret < 0) { + LOG(IPU3, Error) << "Failed to allocate ImgU output buffers"; + goto error; + } + + if (vfStream->active_) + ret = viewfinder_.dev->importBuffers(bufferCount); + else + ret = viewfinder_.dev->allocateBuffers(bufferCount, + &viewfinder_.buffers); + if (ret < 0) { + LOG(IPU3, Error) << "Failed to allocate ImgU viewfinder buffers"; + goto error; + } + + return 0; + +error: + freeBuffers(data); + + return ret; +} + /** * \brief Release buffers for all the ImgU video devices */ @@ -1163,21 +1159,17 @@ void ImgUDevice::freeBuffers(IPU3CameraData *data) { int ret; - if (!data->outStream_.active_) { - ret = output_.dev->releaseBuffers(); - if (ret) - LOG(IPU3, Error) << "Failed to release ImgU output buffers"; - } + ret = output_.dev->releaseBuffers(); + if (ret) + LOG(IPU3, Error) << "Failed to release ImgU output buffers"; ret = stat_.dev->releaseBuffers(); if (ret) LOG(IPU3, Error) << "Failed to release ImgU stat buffers"; - if (!data->vfStream_.active_) { - ret = viewfinder_.dev->releaseBuffers(); - if (ret) - LOG(IPU3, Error) << "Failed to release ImgU viewfinder buffers"; - } + ret = viewfinder_.dev->releaseBuffers(); + if (ret) + LOG(IPU3, Error) << "Failed to release ImgU viewfinder buffers"; ret = input_->releaseBuffers(); if (ret) diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index 1ad53fbde112..01977ad697a9 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -175,8 +175,6 @@ public: int exportFrameBuffers(Camera *camera, Stream *stream, std::vector> *buffers) override; - int importFrameBuffers(Camera *camera, Stream *stream) override; - void freeFrameBuffers(Camera *camera, Stream *stream) override; int start(Camera *camera) override; void stop(Camera *camera) override; @@ -668,17 +666,6 @@ int PipelineHandlerRkISP1::exportFrameBuffers(Camera *camera, Stream *stream, return video_->exportBuffers(count, buffers); } -int PipelineHandlerRkISP1::importFrameBuffers(Camera *camera, Stream *stream) -{ - unsigned int count = stream->configuration().bufferCount; - return video_->importBuffers(count); -} - -void PipelineHandlerRkISP1::freeFrameBuffers(Camera *camera, Stream *stream) -{ - video_->releaseBuffers(); -} - int PipelineHandlerRkISP1::allocateBuffers(Camera *camera) { RkISP1CameraData *data = cameraData(camera); @@ -689,6 +676,10 @@ int PipelineHandlerRkISP1::allocateBuffers(Camera *camera) for (const Stream *s : camera->streams()) maxBuffers = std::max(maxBuffers, s->configuration().bufferCount); + ret = video_->importBuffers(count); + if (ret < 0) + goto error; + ret = param_->allocateBuffers(maxBuffers, ¶mBuffers_); if (ret < 0) goto error; @@ -749,6 +740,9 @@ int PipelineHandlerRkISP1::freeBuffers(Camera *camera) if (stat_->releaseBuffers()) LOG(RkISP1, Error) << "Failed to release stat buffers"; + if (video_->releaseBuffers()) + LOG(RkISP1, Error) << "Failed to release video buffers"; + return 0; } diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp index 29afb121aa46..40cc3ee7d098 100644 --- a/src/libcamera/pipeline/uvcvideo.cpp +++ b/src/libcamera/pipeline/uvcvideo.cpp @@ -67,8 +67,6 @@ public: int exportFrameBuffers(Camera *camera, Stream *stream, std::vector> *buffers) override; - int importFrameBuffers(Camera *camera, Stream *stream) override; - void freeFrameBuffers(Camera *camera, Stream *stream) override; int start(Camera *camera) override; void stop(Camera *camera) override; @@ -202,31 +200,29 @@ int PipelineHandlerUVC::exportFrameBuffers(Camera *camera, Stream *stream, return data->video_->exportBuffers(count, buffers); } -int PipelineHandlerUVC::importFrameBuffers(Camera *camera, Stream *stream) +int PipelineHandlerUVC::start(Camera *camera) { UVCCameraData *data = cameraData(camera); - unsigned int count = stream->configuration().bufferCount; + unsigned int count = data->stream_.configuration().bufferCount; - return data->video_->importBuffers(count); -} - -void PipelineHandlerUVC::freeFrameBuffers(Camera *camera, Stream *stream) -{ - UVCCameraData *data = cameraData(camera); + int ret = data->video_->importBuffers(count); + if (ret < 0) + return ret; - data->video_->releaseBuffers(); -} + ret = data->video_->streamOn(); + if (ret < 0) { + data->video_->releaseBuffers(); + return ret; + } -int PipelineHandlerUVC::start(Camera *camera) -{ - UVCCameraData *data = cameraData(camera); - return data->video_->streamOn(); + return 0; } void PipelineHandlerUVC::stop(Camera *camera) { UVCCameraData *data = cameraData(camera); data->video_->streamOff(); + data->video_->releaseBuffers(); } int PipelineHandlerUVC::processControls(UVCCameraData *data, Request *request) diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp index 5d3d12fef30b..eceb16d5586a 100644 --- a/src/libcamera/pipeline/vimc.cpp +++ b/src/libcamera/pipeline/vimc.cpp @@ -84,8 +84,6 @@ public: int exportFrameBuffers(Camera *camera, Stream *stream, std::vector> *buffers) override; - int importFrameBuffers(Camera *camera, Stream *stream) override; - void freeFrameBuffers(Camera *camera, Stream *stream) override; int start(Camera *camera) override; void stop(Camera *camera) override; @@ -268,31 +266,29 @@ int PipelineHandlerVimc::exportFrameBuffers(Camera *camera, Stream *stream, return data->video_->exportBuffers(count, buffers); } -int PipelineHandlerVimc::importFrameBuffers(Camera *camera, Stream *stream) +int PipelineHandlerVimc::start(Camera *camera) { VimcCameraData *data = cameraData(camera); - unsigned int count = stream->configuration().bufferCount; + unsigned int count = data->stream_.configuration().bufferCount; - return data->video_->importBuffers(count); -} - -void PipelineHandlerVimc::freeFrameBuffers(Camera *camera, Stream *stream) -{ - VimcCameraData *data = cameraData(camera); + int ret = data->video_->importBuffers(count); + if (ret < 0) + return ret; - data->video_->releaseBuffers(); -} + ret = data->video_->streamOn(); + if (ret < 0) { + data->video_->releaseBuffers(); + return ret; + } -int PipelineHandlerVimc::start(Camera *camera) -{ - VimcCameraData *data = cameraData(camera); - return data->video_->streamOn(); + return 0; } void PipelineHandlerVimc::stop(Camera *camera) { VimcCameraData *data = cameraData(camera); data->video_->streamOff(); + data->video_->releaseBuffers(); } int PipelineHandlerVimc::processControls(VimcCameraData *data, Request *request) diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp index e5034c54e2fb..254d341fb8a4 100644 --- a/src/libcamera/pipeline_handler.cpp +++ b/src/libcamera/pipeline_handler.cpp @@ -325,7 +325,7 @@ const ControlList &PipelineHandler::properties(Camera *camera) /** * \fn PipelineHandler::exportFrameBuffers() - * \brief Allocate buffers for \a stream + * \brief Allocate and export buffers for \a stream * \param[in] camera The camera * \param[in] stream The stream to allocate buffers for * \param[out] buffers Array of buffers successfully allocated @@ -347,45 +347,6 @@ const ControlList &PipelineHandler::properties(Camera *camera) * otherwise */ -/** - * \fn PipelineHandler::importFrameBuffers() - * \brief Prepare \a stream to use external buffers - * \param[in] camera The camera - * \param[in] stream The stream to prepare for import - * - * This method prepares the pipeline handler to use buffers provided by the - * application for the \a stream. - * - * The method may only be called after the Camera has been configured and before - * it gets started, or after it gets stopped. It shall be called only for - * streams that are part of the active camera configuration, and at most once - * per stream until buffers for the stream are freed with freeFrameBuffers(). - * - * importFrameBuffers() shall also allocate all other resources required by the - * pipeline handler for the stream to prepare for starting the Camera. - * - * The only intended caller is Camera::start(). - * - * \context This function is called from the CameraManager thread. - * - * \return 0 on success or a negative error code otherwise - */ - -/** - * \fn PipelineHandler::freeFrameBuffers() - * \brief Free buffers allocated from the stream - * \param[in] camera The camera - * \param[in] stream The stream to free buffers for - * - * This method shall release all resources allocated for the \a stream by - * importFrameBuffers(). It shall be called only after a successful call that - * method, and only once per stream. - * - * The only intended callers are Camera::stop() and Camera::freeFrameBuffers(). - * - * \context This function is called from the CameraManager thread. - */ - /** * \fn PipelineHandler::start() * \brief Start capturing from a group of streams From patchwork Sat Mar 14 23:57:28 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 3102 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 4B7F362958 for ; Sun, 15 Mar 2020 00:57:43 +0100 (CET) Received: from pendragon.bb.dnainternet.fi (81-175-216-236.bb.dnainternet.fi [81.175.216.236]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id E8A4B1163 for ; Sun, 15 Mar 2020 00:57:42 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1584230263; bh=GC9I3JWIygZO07zgkhuf9lIP4Gq7GK8ea2U5ugQHlwI=; h=From:To:Subject:Date:In-Reply-To:References:From; b=OYxSoE1SmxBo0FW017Tzwo6jMI/0t0Cnt5RkeaKm+C9+nJgftmJGdyrCykgbveTm3 D6aQprSz8gnE+3CBN+eU8pz1LxV/bdojXc2YzOM5Rm+3IC3dZZ5TVmttoogP7fFL1w 7aoTqXCwUGv99CVMM9ch+zYueLuKsKsL1ArH8jM0= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Sun, 15 Mar 2020 01:57:28 +0200 Message-Id: <20200314235728.15495-10-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.24.1 In-Reply-To: <20200314235728.15495-1-laurent.pinchart@ideasonboard.com> References: <20200314235728.15495-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 9/9] libcamera: framebuffer_allocator: Lift camera restrictions on 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: Sat, 14 Mar 2020 23:57:43 -0000 The Camera class currently requires the allocator to have no allocated buffer before the camera is reconfigured, and the allocator to be destroyed before the camera is released. There's no basis for these restrictions anymore, remove them. Signed-off-by: Laurent Pinchart Reviewed-by: Niklas Söderlund --- include/libcamera/camera.h | 2 -- include/libcamera/framebuffer_allocator.h | 5 +---- src/cam/capture.cpp | 2 +- src/gstreamer/gstlibcameraallocator.cpp | 2 +- src/libcamera/camera.cpp | 18 +--------------- src/libcamera/framebuffer_allocator.cpp | 25 ----------------------- src/qcam/main_window.cpp | 2 +- src/v4l2/v4l2_camera.cpp | 2 +- test/camera/capture.cpp | 2 +- test/camera/statemachine.cpp | 2 +- 10 files changed, 8 insertions(+), 54 deletions(-) diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h index a5e2b49f0f25..9c0e58f7864b 100644 --- a/include/libcamera/camera.h +++ b/include/libcamera/camera.h @@ -113,8 +113,6 @@ private: friend class FrameBufferAllocator; int exportFrameBuffers(Stream *stream, std::vector> *buffers); - /* \todo Remove allocator_ from the exposed API */ - FrameBufferAllocator *allocator_; }; } /* namespace libcamera */ diff --git a/include/libcamera/framebuffer_allocator.h b/include/libcamera/framebuffer_allocator.h index 42812253ada9..78f1353964eb 100644 --- a/include/libcamera/framebuffer_allocator.h +++ b/include/libcamera/framebuffer_allocator.h @@ -20,8 +20,7 @@ class Stream; class FrameBufferAllocator { public: - static FrameBufferAllocator *create(std::shared_ptr camera); - + FrameBufferAllocator(std::shared_ptr camera); FrameBufferAllocator(const Camera &) = delete; FrameBufferAllocator &operator=(const Camera &) = delete; @@ -34,8 +33,6 @@ public: const std::vector> &buffers(Stream *stream) const; private: - FrameBufferAllocator(std::shared_ptr camera); - std::shared_ptr camera_; std::map>> buffers_; }; diff --git a/src/cam/capture.cpp b/src/cam/capture.cpp index 7d970f991d3a..b62a9b24b216 100644 --- a/src/cam/capture.cpp +++ b/src/cam/capture.cpp @@ -52,7 +52,7 @@ int Capture::run(EventLoop *loop, const OptionsParser::Options &options) } - FrameBufferAllocator *allocator = FrameBufferAllocator::create(camera_); + FrameBufferAllocator *allocator = new FrameBufferAllocator(camera_); ret = capture(loop, allocator); diff --git a/src/gstreamer/gstlibcameraallocator.cpp b/src/gstreamer/gstlibcameraallocator.cpp index d0b90ecaa873..1d5959c076cb 100644 --- a/src/gstreamer/gstlibcameraallocator.cpp +++ b/src/gstreamer/gstlibcameraallocator.cpp @@ -188,7 +188,7 @@ gst_libcamera_allocator_new(std::shared_ptr camera) auto *self = GST_LIBCAMERA_ALLOCATOR(g_object_new(GST_TYPE_LIBCAMERA_ALLOCATOR, nullptr)); - self->fb_allocator = FrameBufferAllocator::create(camera); + self->fb_allocator = new FrameBufferAllocator(camera); for (Stream *stream : camera->streams()) { gint ret; diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp index 5593c1b317a0..8c3bb2c2a01f 100644 --- a/src/libcamera/camera.cpp +++ b/src/libcamera/camera.cpp @@ -508,7 +508,7 @@ const std::string &Camera::name() const Camera::Camera(PipelineHandler *pipe, const std::string &name, const std::set &streams) - : p_(new Private(pipe, name, streams)), allocator_(nullptr) + : p_(new Private(pipe, name, streams)) { } @@ -620,16 +620,6 @@ int Camera::release() if (ret < 0) return ret == -EACCES ? -EBUSY : ret; - if (allocator_) { - /* - * \todo Try to find a better API that would make this error - * impossible. - */ - LOG(Camera, Error) - << "Buffers must be freed before the camera can be reconfigured"; - return -EBUSY; - } - p_->pipe_->unlock(); p_->setState(Private::CameraAvailable); @@ -763,12 +753,6 @@ int Camera::configure(CameraConfiguration *config) if (ret < 0) return ret; - if (allocator_ && allocator_->allocated()) { - LOG(Camera, Error) - << "Allocator must be deleted before camera can be reconfigured"; - return -EBUSY; - } - if (config->validate() != CameraConfiguration::Valid) { LOG(Camera, Error) << "Can't configure camera with invalid configuration"; diff --git a/src/libcamera/framebuffer_allocator.cpp b/src/libcamera/framebuffer_allocator.cpp index 6f7a2e90b08a..a37b564c6701 100644 --- a/src/libcamera/framebuffer_allocator.cpp +++ b/src/libcamera/framebuffer_allocator.cpp @@ -53,29 +53,6 @@ LOG_DEFINE_CATEGORY(Allocator) * are provided externally applications shall not use this class. */ -/** - * \brief Create a FrameBuffer allocator - * \param[in] camera The camera the allocator serves - * - * A single allocator may be created for a Camera instance. - * - * The caller is responsible for deleting the allocator before the camera is - * released. - * - * \return A pointer to the newly created allocator object or nullptr on error - */ -FrameBufferAllocator * -FrameBufferAllocator::create(std::shared_ptr camera) -{ - if (camera->allocator_) { - LOG(Allocator, Error) << "Camera already has an allocator"; - return nullptr; - } - - camera->allocator_ = new FrameBufferAllocator(camera); - return camera->allocator_; -} - /** * \brief Construct a FrameBufferAllocator serving a camera * \param[in] camera The camera @@ -88,8 +65,6 @@ FrameBufferAllocator::FrameBufferAllocator(std::shared_ptr camera) FrameBufferAllocator::~FrameBufferAllocator() { buffers_.clear(); - - camera_->allocator_ = nullptr; } /** diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp index ae1760dfd647..47d37c3e62ce 100644 --- a/src/qcam/main_window.cpp +++ b/src/qcam/main_window.cpp @@ -240,7 +240,7 @@ int MainWindow::startCapture() adjustSize(); - allocator_ = FrameBufferAllocator::create(camera_); + allocator_ = new FrameBufferAllocator(camera_); ret = allocator_->allocate(stream); if (ret < 0) { std::cerr << "Failed to allocate capture buffers" << std::endl; diff --git a/src/v4l2/v4l2_camera.cpp b/src/v4l2/v4l2_camera.cpp index e7018b566475..f0b9f1804c94 100644 --- a/src/v4l2/v4l2_camera.cpp +++ b/src/v4l2/v4l2_camera.cpp @@ -40,7 +40,7 @@ int V4L2Camera::open() return -EINVAL; } - bufferAllocator_ = FrameBufferAllocator::create(camera_); + bufferAllocator_ = new FrameBufferAllocator(camera_); return 0; } diff --git a/test/camera/capture.cpp b/test/camera/capture.cpp index b304d59c1c2a..f6b2f348bda5 100644 --- a/test/camera/capture.cpp +++ b/test/camera/capture.cpp @@ -63,7 +63,7 @@ protected: return TestFail; } - allocator_ = FrameBufferAllocator::create(camera_); + allocator_ = new FrameBufferAllocator(camera_); return TestPass; } diff --git a/test/camera/statemachine.cpp b/test/camera/statemachine.cpp index 20541b3e4752..325b4674bcc9 100644 --- a/test/camera/statemachine.cpp +++ b/test/camera/statemachine.cpp @@ -117,7 +117,7 @@ protected: return TestFail; /* Use internally allocated buffers. */ - allocator_ = FrameBufferAllocator::create(camera_); + allocator_ = new FrameBufferAllocator(camera_); Stream *stream = *camera_->streams().begin(); if (allocator_->allocate(stream) < 0) return TestFail;