From patchwork Fri Jun 5 09:01:00 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Paul Elder X-Patchwork-Id: 3936 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 C5F2661027 for ; Fri, 5 Jun 2020 11:01:17 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="HrHlCjzR"; dkim-atps=neutral Received: from emerald.amanokami.net (fs76eef344.knge213.ap.nuro.jp [118.238.243.68]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id A151F27C; Fri, 5 Jun 2020 11:01:16 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1591347677; bh=7zrH4zk88iLncnUuGh1zlgEHRYvx0Sw1lvltKXxCbvA=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=HrHlCjzR8AFfyUijcYSh2DzR+Woevg5Ffe86t5LY8k4prnFGyOjjdL9R5g8StaMZg K+grA0kBAS/AlPr13S/xJ/1XRUYmzWylTd8m0D1SybLbTUOVMcv+sZK5nwXla40Y2U LPHUHndWLjmVKi4jeEKmOkKYP0qLGhyiHQhhkXBA= From: Paul Elder To: libcamera-devel@lists.libcamera.org Date: Fri, 5 Jun 2020 18:01:00 +0900 Message-Id: <20200605090106.15424-2-paul.elder@ideasonboard.com> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20200605090106.15424-1-paul.elder@ideasonboard.com> References: <20200605090106.15424-1-paul.elder@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 1/7] IPAManager: make IPAManager lifetime explicitly managed 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: Fri, 05 Jun 2020 09:01:18 -0000 If any ipa_context instances are destroyed after the IPAManager is destroyed, then a segfault will occur, since the modules have been unloaded by the IPAManager and the context function pointers have been freed. Fix this by making the lifetime of the IPAManager explicit, and make the CameraManager construct and deconstruct (automatically, via a unique pointer) the IPAManager. Also update the IPA interface test to do the construction and deconstruction of the IPAManager, as it does not use the CameraManager. Signed-off-by: Paul Elder Reviewed-by: Laurent Pinchart Reviewed-by: Niklas Söderlund --- Changes in v2: Move the IPAManager instance into CameraManager::Private from CameraManager --- include/libcamera/camera_manager.h | 1 - include/libcamera/internal/ipa_manager.h | 6 ++++-- src/libcamera/camera_manager.cpp | 3 +++ src/libcamera/ipa_manager.cpp | 20 ++++++++++++++++++-- test/ipa/ipa_interface_test.cpp | 5 +++++ 5 files changed, 30 insertions(+), 5 deletions(-) diff --git a/include/libcamera/camera_manager.h b/include/libcamera/camera_manager.h index 079f848a..00658a70 100644 --- a/include/libcamera/camera_manager.h +++ b/include/libcamera/camera_manager.h @@ -18,7 +18,6 @@ namespace libcamera { class Camera; class EventDispatcher; - class CameraManager : public Object { public: diff --git a/include/libcamera/internal/ipa_manager.h b/include/libcamera/internal/ipa_manager.h index 16d74291..43f700d3 100644 --- a/include/libcamera/internal/ipa_manager.h +++ b/include/libcamera/internal/ipa_manager.h @@ -22,6 +22,9 @@ namespace libcamera { class IPAManager { public: + IPAManager(); + ~IPAManager(); + static IPAManager *instance(); std::unique_ptr createIPA(PipelineHandler *pipe, @@ -29,8 +32,7 @@ public: uint32_t minVersion); private: - IPAManager(); - ~IPAManager(); + static IPAManager *self_; void parseDir(const char *libDir, unsigned int maxDepth, std::vector &files); diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp index 849377ad..da806fa7 100644 --- a/src/libcamera/camera_manager.cpp +++ b/src/libcamera/camera_manager.cpp @@ -15,6 +15,7 @@ #include "libcamera/internal/device_enumerator.h" #include "libcamera/internal/event_dispatcher_poll.h" +#include "libcamera/internal/ipa_manager.h" #include "libcamera/internal/log.h" #include "libcamera/internal/pipeline_handler.h" #include "libcamera/internal/thread.h" @@ -63,6 +64,8 @@ private: std::vector> pipes_; std::unique_ptr enumerator_; + + IPAManager ipaManager_; }; CameraManager::Private::Private(CameraManager *cm) diff --git a/src/libcamera/ipa_manager.cpp b/src/libcamera/ipa_manager.cpp index 505cf610..abb681a3 100644 --- a/src/libcamera/ipa_manager.cpp +++ b/src/libcamera/ipa_manager.cpp @@ -93,8 +93,21 @@ LOG_DEFINE_CATEGORY(IPAManager) * plain C API, or to transmit the data to the isolated process through IPC. */ +IPAManager *IPAManager::self_ = nullptr; + +/** + * \brief Construct an IPAManager instance + * + * The IPAManager class is meant to only be instantiated once, by the + * CameraManager. Pipeline handlers shall use the instance() function to access + * the IPAManager instance. + */ IPAManager::IPAManager() { + if (self_) + LOG(IPAManager, Fatal) + << "Multiple IPAManager objects are not allowed"; + unsigned int ipaCount = 0; /* User-specified paths take precedence. */ @@ -134,12 +147,16 @@ IPAManager::IPAManager() if (!ipaCount) LOG(IPAManager, Warning) << "No IPA found in '" IPA_MODULE_DIR "'"; + + self_ = this; } IPAManager::~IPAManager() { for (IPAModule *module : modules_) delete module; + + self_ = nullptr; } /** @@ -153,8 +170,7 @@ IPAManager::~IPAManager() */ IPAManager *IPAManager::instance() { - static IPAManager ipaManager; - return &ipaManager; + return self_; } /** diff --git a/test/ipa/ipa_interface_test.cpp b/test/ipa/ipa_interface_test.cpp index 2f02af49..153493ba 100644 --- a/test/ipa/ipa_interface_test.cpp +++ b/test/ipa/ipa_interface_test.cpp @@ -39,11 +39,15 @@ public: ~IPAInterfaceTest() { delete notifier_; + ipa_.reset(); + ipaManager_.reset(); } protected: int init() override { + ipaManager_ = make_unique(); + /* Create a pipeline handler for vimc. */ std::vector &factories = PipelineHandlerFactory::factories(); @@ -161,6 +165,7 @@ private: std::shared_ptr pipe_; std::unique_ptr ipa_; + std::unique_ptr ipaManager_; enum IPAOperationCode trace_; EventNotifier *notifier_; int fd_; From patchwork Fri Jun 5 09:01:01 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Paul Elder X-Patchwork-Id: 3937 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 5AAFB61375 for ; Fri, 5 Jun 2020 11:01:19 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="AqUswxUR"; dkim-atps=neutral Received: from emerald.amanokami.net (fs76eef344.knge213.ap.nuro.jp [118.238.243.68]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 2480327C; Fri, 5 Jun 2020 11:01:17 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1591347679; bh=yylEpV+peh6zKRxo/3lJOgX0clLhFV5SY/1JmZdufnQ=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=AqUswxURYp9Z0Cj3ZVOmUOItzZjuLwoMCBnY06mKWrGX+WI3EIbunzmwrd1gwU6TY /5QSR3mr7EnfXk8DSppGANU0AWLnaG8wkgje2GqIlcvWGr5AVtGxoRNFYOZPLHHXYT P4pXZByPmb8riHQxu/8GTUegS3Bmsm8V3ti34cH4= From: Paul Elder To: libcamera-devel@lists.libcamera.org Date: Fri, 5 Jun 2020 18:01:01 +0900 Message-Id: <20200605090106.15424-3-paul.elder@ideasonboard.com> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20200605090106.15424-1-paul.elder@ideasonboard.com> References: <20200605090106.15424-1-paul.elder@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 2/7] IPAManager: remove instance() and make createIPA() static 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: Fri, 05 Jun 2020 09:01:19 -0000 As the only usage of IPAManager::instance() is by the pipeline handlers to call IPAManager::createIPA(), remove the former and make the latter static. Update the pipeline handlers and tests accordingly. Signed-off-by: Paul Elder Reviewed-by: Laurent Pinchart Reviewed-by: Niklas Söderlund --- New in v2 --- include/libcamera/internal/ipa_manager.h | 8 +++----- src/libcamera/ipa_manager.cpp | 18 ++---------------- .../pipeline/raspberrypi/raspberrypi.cpp | 2 +- src/libcamera/pipeline/rkisp1/rkisp1.cpp | 2 +- src/libcamera/pipeline/vimc/vimc.cpp | 2 +- test/ipa/ipa_interface_test.cpp | 2 +- 6 files changed, 9 insertions(+), 25 deletions(-) diff --git a/include/libcamera/internal/ipa_manager.h b/include/libcamera/internal/ipa_manager.h index 43f700d3..05643e5e 100644 --- a/include/libcamera/internal/ipa_manager.h +++ b/include/libcamera/internal/ipa_manager.h @@ -25,11 +25,9 @@ public: IPAManager(); ~IPAManager(); - static IPAManager *instance(); - - std::unique_ptr createIPA(PipelineHandler *pipe, - uint32_t maxVersion, - uint32_t minVersion); + static std::unique_ptr createIPA(PipelineHandler *pipe, + uint32_t maxVersion, + uint32_t minVersion); private: static IPAManager *self_; diff --git a/src/libcamera/ipa_manager.cpp b/src/libcamera/ipa_manager.cpp index abb681a3..9d0c2069 100644 --- a/src/libcamera/ipa_manager.cpp +++ b/src/libcamera/ipa_manager.cpp @@ -159,20 +159,6 @@ IPAManager::~IPAManager() self_ = nullptr; } -/** - * \brief Retrieve the IPA manager instance - * - * The IPAManager is a singleton and can't be constructed manually. This - * function shall instead be used to retrieve the single global instance of the - * manager. - * - * \return The IPA manager instance - */ -IPAManager *IPAManager::instance() -{ - return self_; -} - /** * \brief Identify shared library objects within a directory * \param[in] libDir The directory to search for shared objects @@ -274,7 +260,7 @@ std::unique_ptr IPAManager::createIPA(PipelineHandler *pipe, { IPAModule *m = nullptr; - for (IPAModule *module : modules_) { + for (IPAModule *module : self_->modules_) { if (module->match(pipe, minVersion, maxVersion)) { m = module; break; @@ -290,7 +276,7 @@ std::unique_ptr IPAManager::createIPA(PipelineHandler *pipe, * * \todo Implement a better proxy selection */ - const char *proxyName = isSignatureValid(m) + const char *proxyName = self_->isSignatureValid(m) ? "IPAProxyThread" : "IPAProxyLinux"; IPAProxyFactory *pf = nullptr; diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp index e16a9c7f..b9b88506 100644 --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp @@ -1118,7 +1118,7 @@ void RPiCameraData::frameStarted(uint32_t sequence) int RPiCameraData::loadIPA() { - ipa_ = IPAManager::instance()->createIPA(pipe_, 1, 1); + ipa_ = IPAManager::createIPA(pipe_, 1, 1); if (!ipa_) return -ENOENT; diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index d807fc2c..900f873a 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -395,7 +395,7 @@ private: int RkISP1CameraData::loadIPA() { - ipa_ = IPAManager::instance()->createIPA(pipe_, 1, 1); + ipa_ = IPAManager::createIPA(pipe_, 1, 1); if (!ipa_) return -ENOENT; diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp index ba9fca50..3881545b 100644 --- a/src/libcamera/pipeline/vimc/vimc.cpp +++ b/src/libcamera/pipeline/vimc/vimc.cpp @@ -410,7 +410,7 @@ bool PipelineHandlerVimc::match(DeviceEnumerator *enumerator) std::unique_ptr data = std::make_unique(this, media); - data->ipa_ = IPAManager::instance()->createIPA(this, 0, 0); + data->ipa_ = IPAManager::createIPA(this, 0, 0); if (data->ipa_ != nullptr) { std::string conf = data->ipa_->configurationFile("vimc.conf"); data->ipa_->init(IPASettings{ conf }); diff --git a/test/ipa/ipa_interface_test.cpp b/test/ipa/ipa_interface_test.cpp index 153493ba..1bc93a63 100644 --- a/test/ipa/ipa_interface_test.cpp +++ b/test/ipa/ipa_interface_test.cpp @@ -95,7 +95,7 @@ protected: EventDispatcher *dispatcher = thread()->eventDispatcher(); Timer timer; - ipa_ = IPAManager::instance()->createIPA(pipe_.get(), 0, 0); + ipa_ = IPAManager::createIPA(pipe_.get(), 0, 0); if (!ipa_) { cerr << "Failed to create VIMC IPA interface" << endl; return TestFail; From patchwork Fri Jun 5 09:01:02 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Paul Elder X-Patchwork-Id: 3938 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 103A761167 for ; Fri, 5 Jun 2020 11:01:21 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="rSipi9ye"; dkim-atps=neutral Received: from emerald.amanokami.net (fs76eef344.knge213.ap.nuro.jp [118.238.243.68]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 9D8CD27C; Fri, 5 Jun 2020 11:01:19 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1591347680; bh=whVEwgtlX/iSVJrGUco2WPc9k7dIH3q1tfmlXsWDbyo=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=rSipi9yec82ESskSp9RLLQ517tjclg83QZTFdgJsJuDLVEpUXtTkzvmKBXt2Devgk Fn12z1gOXwWKD+SWW1gzuWqKILb6aOFDxrm2DDaUZnS1MujH4EwSrP8Vd2EoaC5nB1 p24CD1mLGQSPvedmmu3isx2eail9DtKP0998WoVE= From: Paul Elder To: libcamera-devel@lists.libcamera.org Date: Fri, 5 Jun 2020 18:01:02 +0900 Message-Id: <20200605090106.15424-4-paul.elder@ideasonboard.com> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20200605090106.15424-1-paul.elder@ideasonboard.com> References: <20200605090106.15424-1-paul.elder@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 3/7] v4l2: v4l2_camera_proxy: Fix bounds check for VIDIOC_ENUM_FMT 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: Fri, 05 Jun 2020 09:01:21 -0000 VIDIOC_ENUM_FMT is meant to return -EINVAL if the requested index is out of bounds. This bounds is obtained from the libcamera Camera's list of formats. The bounds check for this list was incorrect; fix it. Signed-off-by: Paul Elder Reviewed-by: Laurent Pinchart Reviewed-by: Niklas Söderlund --- No change in v2 --- src/v4l2/v4l2_camera_proxy.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/v4l2/v4l2_camera_proxy.cpp b/src/v4l2/v4l2_camera_proxy.cpp index 3df4d42b..ec6d265d 100644 --- a/src/v4l2/v4l2_camera_proxy.cpp +++ b/src/v4l2/v4l2_camera_proxy.cpp @@ -224,7 +224,7 @@ int V4L2CameraProxy::vidioc_enum_fmt(struct v4l2_fmtdesc *arg) LOG(V4L2Compat, Debug) << "Servicing vidioc_enum_fmt"; if (!validateBufferType(arg->type) || - arg->index > streamConfig_.formats().pixelformats().size()) + arg->index >= streamConfig_.formats().pixelformats().size()) return -EINVAL; /* \todo Add map from format to description. */ From patchwork Fri Jun 5 09:01:03 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Paul Elder X-Patchwork-Id: 3939 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 8591E61027 for ; Fri, 5 Jun 2020 11:01:22 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="XZ7280gF"; dkim-atps=neutral Received: from emerald.amanokami.net (fs76eef344.knge213.ap.nuro.jp [118.238.243.68]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 57D6B27C; Fri, 5 Jun 2020 11:01:21 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1591347682; bh=QB20CXQv4BOODnqeSakNDMIm1j3itxDVfiqH16KTQ/Q=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=XZ7280gFjPQF05Rh1AnVS28JHClLRD5mYIAFZB53Y6xfhLkH9mqjG12GfoId/ttCZ mwpxgN4AnW0FSbUhByLZt3BrHD+OrZ1QsjvsAfD1HNdR/q1jrYoZsPcax5AvLkZRMz Y0iu31m51U8zVExOxLMLsxuZI0tZ7rvifD5aS/AY= From: Paul Elder To: libcamera-devel@lists.libcamera.org Date: Fri, 5 Jun 2020 18:01:03 +0900 Message-Id: <20200605090106.15424-5-paul.elder@ideasonboard.com> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20200605090106.15424-1-paul.elder@ideasonboard.com> References: <20200605090106.15424-1-paul.elder@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 4/7] v4l2: v4l2_camera_proxy: Acquire only one buffer semaphore on VIDIOC_DQBUF 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: Fri, 05 Jun 2020 09:01:22 -0000 We use a semaphore to atomically keep track of how many buffers are available for dequeueing. The check for how to acquire the semaphore was incorrect, leading to a double acquire upon a successful nonblocking acquire. Fix this. Signed-off-by: Paul Elder Reviewed-by: Laurent Pinchart Reviewed-by: Niklas Söderlund --- Changes in v2: restructured the if block so it's easier to read --- src/v4l2/v4l2_camera_proxy.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/v4l2/v4l2_camera_proxy.cpp b/src/v4l2/v4l2_camera_proxy.cpp index ec6d265d..a0c6deea 100644 --- a/src/v4l2/v4l2_camera_proxy.cpp +++ b/src/v4l2/v4l2_camera_proxy.cpp @@ -426,10 +426,10 @@ int V4L2CameraProxy::vidioc_dqbuf(struct v4l2_buffer *arg) !validateMemoryType(arg->memory)) return -EINVAL; - if (nonBlocking_ && !vcam_->bufferSema_.tryAcquire()) - return -EAGAIN; - else + if (!nonBlocking_) vcam_->bufferSema_.acquire(); + else if (!vcam_->bufferSema_.tryAcquire()) + return -EAGAIN; updateBuffers(); From patchwork Fri Jun 5 09:01:04 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Paul Elder X-Patchwork-Id: 3940 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 006EE6124F for ; Fri, 5 Jun 2020 11:01:24 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="lsl5tw/y"; dkim-atps=neutral Received: from emerald.amanokami.net (fs76eef344.knge213.ap.nuro.jp [118.238.243.68]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id CD86F27C; Fri, 5 Jun 2020 11:01:22 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1591347683; bh=eqVGWxLcEElmdrGmRBqiyy8NYrlqldzJ/gMwEMcKe1c=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=lsl5tw/yoUsF3yW/pfke3lz+9DNju3R5uvrgkR0bnt9/CpjWmPIm6WIi/Bh+7HdGw 1cxW4w8/plWitFl9IwG/93G9DQ461Xs1WHaCCRW3KnXTgwSO51X4C4rCQOwFDICClY RH5GI5UPZxjTYRSpcVx06ZorxtoQelD5Tv+AHGi0= From: Paul Elder To: libcamera-devel@lists.libcamera.org Date: Fri, 5 Jun 2020 18:01:04 +0900 Message-Id: <20200605090106.15424-6-paul.elder@ideasonboard.com> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20200605090106.15424-1-paul.elder@ideasonboard.com> References: <20200605090106.15424-1-paul.elder@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 5/7] v4l2: v4l2_camera_proxy: Don't return -EINVAL for zero sizeimage in 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: Fri, 05 Jun 2020 09:01:24 -0000 If VIDIOC_REQBUFS returns -EINVAL, it signals to the application that the requested buffer or memory type is not supported. If we return -EINVAL due to a zero sizeimage, then the application will think that we don't support a memory type that we actually do. We cannot error on a zero sizeimage, because reqbufs could be called merely to probe what IO methods we support; qv4l2, for example, called reqbufs once with userptr and once more with mmap, both times with count=1. On the other hand, sizeimage will be zero for formats whose size we don't know how to calculate, such as MJPEG. If we try to stream such formats anyway, we will get a floating point exception and crash. Issue a warning for now, and don't return -EINVAL, so that we can continue operation. Signed-off-by: Paul Elder Reviewed-by: Laurent Pinchart --- Changes in v2: expand changelog --- src/v4l2/v4l2_camera_proxy.cpp | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/v4l2/v4l2_camera_proxy.cpp b/src/v4l2/v4l2_camera_proxy.cpp index a0c6deea..cbe9e026 100644 --- a/src/v4l2/v4l2_camera_proxy.cpp +++ b/src/v4l2/v4l2_camera_proxy.cpp @@ -352,8 +352,18 @@ int V4L2CameraProxy::vidioc_reqbufs(struct v4l2_requestbuffers *arg) return -EINVAL; sizeimage_ = calculateSizeImage(streamConfig_); + /* + * If we return -EINVAL here then the application will think that we + * don't support streaming mmap. Since we don't support readwrite and + * userptr either, the application will get confused and think that + * we don't support anything. + * On the other hand, if a format has a zero sizeimage (eg. MJPEG), + * we'll get a floating point exception when we try to stream it. + */ if (sizeimage_ == 0) - return -EINVAL; + LOG(V4L2Compat, Warning) + << "sizeimage of at least one format is zero. " + << "Streaming this format will cause a floating point exception."; setFmtFromConfig(streamConfig_); From patchwork Fri Jun 5 09:01:05 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Paul Elder X-Patchwork-Id: 3941 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 74C7E61368 for ; Fri, 5 Jun 2020 11:01:25 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="mCJ23lnb"; dkim-atps=neutral Received: from emerald.amanokami.net (fs76eef344.knge213.ap.nuro.jp [118.238.243.68]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 5169D27C; Fri, 5 Jun 2020 11:01:24 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1591347685; bh=8ue+lVlQb2TeMNQ6WnLK9Xnp+aHKtMRPuaD3Yl8Q8jU=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=mCJ23lnblxDiua68mcvhqH5iIMygOrlFBJlO6+vW9pYu44Qct+vgARCrIrPb4eroO m1yHWmN+uItP0WhGavSeAL7YZeDt4HhKx4tcqZP6pEIrUwQZXQlxVO0t4TvRerFaod ipPBXMMDATbanqQ03YYdNQZkmUhpoFlKmJt8LZPM= From: Paul Elder To: libcamera-devel@lists.libcamera.org Date: Fri, 5 Jun 2020 18:01:05 +0900 Message-Id: <20200605090106.15424-7-paul.elder@ideasonboard.com> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20200605090106.15424-1-paul.elder@ideasonboard.com> References: <20200605090106.15424-1-paul.elder@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 6/7] v4l2: v4l2_compat: Add eventfd signaling to support polling 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: Fri, 05 Jun 2020 09:01:25 -0000 To support polling, we need to be able to signal when data is available to be read (POLLIN), as well as events (POLLPRI). Add the necessary calls to eventfd to allow signaling POLLIN. We signal POLLIN by writing writing to the eventfd, and clear it by reading from the eventfd, upon VIDIOC_DQBUF. Note that eventfd does not support signaling POLLPRI, so we don't yet support V4L2 events. Signed-off-by: Paul Elder Reviewed-by: Laurent Pinchart --- Changes in v2: use eventfd instead of socket --- src/v4l2/v4l2_camera.cpp | 9 +++++++++ src/v4l2/v4l2_camera.h | 3 +++ src/v4l2/v4l2_camera_proxy.cpp | 10 ++++++++++ src/v4l2/v4l2_camera_proxy.h | 4 ++++ src/v4l2/v4l2_compat_manager.cpp | 7 ++++++- 5 files changed, 32 insertions(+), 1 deletion(-) diff --git a/src/v4l2/v4l2_camera.cpp b/src/v4l2/v4l2_camera.cpp index 4da01a45..3c369328 100644 --- a/src/v4l2/v4l2_camera.cpp +++ b/src/v4l2/v4l2_camera.cpp @@ -8,6 +8,7 @@ #include "v4l2_camera.h" #include +#include #include "libcamera/internal/log.h" @@ -53,6 +54,11 @@ void V4L2Camera::close() camera_->release(); } +void V4L2Camera::bind(int efd) +{ + efd_ = efd; +} + void V4L2Camera::getStreamConfig(StreamConfiguration *streamConfig) { *streamConfig = config_->at(0); @@ -84,6 +90,9 @@ void V4L2Camera::requestComplete(Request *request) completedBuffers_.push_back(std::move(metadata)); bufferLock_.unlock(); + uint64_t data = 1; + ::write(efd_, &data, sizeof(data)); + bufferSema_.release(); } diff --git a/src/v4l2/v4l2_camera.h b/src/v4l2/v4l2_camera.h index 303eda44..33f5eb02 100644 --- a/src/v4l2/v4l2_camera.h +++ b/src/v4l2/v4l2_camera.h @@ -39,6 +39,7 @@ public: int open(); void close(); + void bind(int efd); void getStreamConfig(StreamConfiguration *streamConfig); std::vector completedBuffers(); @@ -70,6 +71,8 @@ private: std::deque> pendingRequests_; std::deque> completedBuffers_; + + int efd_; }; #endif /* __V4L2_CAMERA_H__ */ diff --git a/src/v4l2/v4l2_camera_proxy.cpp b/src/v4l2/v4l2_camera_proxy.cpp index cbe9e026..7ee4c0cb 100644 --- a/src/v4l2/v4l2_camera_proxy.cpp +++ b/src/v4l2/v4l2_camera_proxy.cpp @@ -13,6 +13,7 @@ #include #include #include +#include #include #include @@ -451,6 +452,9 @@ int V4L2CameraProxy::vidioc_dqbuf(struct v4l2_buffer *arg) currentBuf_ = (currentBuf_ + 1) % bufferCount_; + uint64_t data; + ::read(efd_, &data, sizeof(data)); + return 0; } @@ -529,6 +533,12 @@ int V4L2CameraProxy::ioctl(unsigned long request, void *arg) return ret; } +void V4L2CameraProxy::bind(int fd) +{ + efd_ = fd; + vcam_->bind(fd); +} + struct PixelFormatPlaneInfo { unsigned int bitsPerPixel; unsigned int hSubSampling; diff --git a/src/v4l2/v4l2_camera_proxy.h b/src/v4l2/v4l2_camera_proxy.h index af9f9bbe..7c65c886 100644 --- a/src/v4l2/v4l2_camera_proxy.h +++ b/src/v4l2/v4l2_camera_proxy.h @@ -33,6 +33,8 @@ public: int ioctl(unsigned long request, void *arg); + void bind(int fd); + private: bool validateBufferType(uint32_t type); bool validateMemoryType(uint32_t memory); @@ -77,6 +79,8 @@ private: std::map mmaps_; std::unique_ptr vcam_; + + int efd_; }; #endif /* __V4L2_CAMERA_PROXY_H__ */ diff --git a/src/v4l2/v4l2_compat_manager.cpp b/src/v4l2/v4l2_compat_manager.cpp index 2338a0ee..56533c4f 100644 --- a/src/v4l2/v4l2_compat_manager.cpp +++ b/src/v4l2/v4l2_compat_manager.cpp @@ -155,12 +155,17 @@ int V4L2CompatManager::openat(int dirfd, const char *path, int oflag, mode_t mod if (ret < 0) return ret; - int efd = eventfd(0, oflag & (O_CLOEXEC | O_NONBLOCK)); + int efd = eventfd(0, EFD_SEMAPHORE | + ((oflag & O_CLOEXEC) ? EFD_CLOEXEC : 0) | + ((oflag & O_NONBLOCK) ? EFD_NONBLOCK : 0)); if (efd < 0) { + int err = errno; proxy->close(); + errno = err; return efd; } + proxy->bind(efd); devices_.emplace(efd, proxy); return efd; From patchwork Fri Jun 5 09:01:06 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Paul Elder X-Patchwork-Id: 3942 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 F01946124F for ; Fri, 5 Jun 2020 11:01:26 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="QDIG04Jd"; dkim-atps=neutral Received: from emerald.amanokami.net (fs76eef344.knge213.ap.nuro.jp [118.238.243.68]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id C7CC227C; Fri, 5 Jun 2020 11:01:25 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1591347686; bh=5qweElhF0wT6dCUQnqxn3x3FOoB0mlpHWKTIFoMp5eQ=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=QDIG04Jdn5sWHF+rciXkRtWNdVJz+CnRyalRSIPvwuP12n3/ByAxWoqvBNqnOFwHk lyqHzVPxgp3gzozEpbrQXog9m+E3OfE0UVmnpWb/MGh9WuB+/gPl8zftTuISbIDfJh EL6KhVqjYeiZHajzya89H+/FG+oePzNI+zWAabro= From: Paul Elder To: libcamera-devel@lists.libcamera.org Date: Fri, 5 Jun 2020 18:01:06 +0900 Message-Id: <20200605090106.15424-8-paul.elder@ideasonboard.com> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20200605090106.15424-1-paul.elder@ideasonboard.com> References: <20200605090106.15424-1-paul.elder@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 7/7] v4l2: v4l2_camera_proxy: Fix segfault on restarting 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: Fri, 05 Jun 2020 09:01:27 -0000 The V4L2 compatibility layer keeps track of the index of the next buffer to be dequeued, to handle VIDIOC_DQBUF. This index is set to 0 on startup and incremented (modulo #frames), and is otherwise never reset. This means that if the last handled frame index is not #frames-1, and the stream is restarted without restarting libcamera and the V4L2 compatilibity layer, the buffer index number won't match with the libcamera buffer index number, causing a segfault. Fix this by resetting the current buffer index to zero on VIDIOC_STREAMON. Signed-off-by: Paul Elder Reviewed-by: Laurent Pinchart Reviewed-by: Niklas Söderlund --- New in v2 --- src/v4l2/v4l2_camera_proxy.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/v4l2/v4l2_camera_proxy.cpp b/src/v4l2/v4l2_camera_proxy.cpp index 7ee4c0cb..059f3cbe 100644 --- a/src/v4l2/v4l2_camera_proxy.cpp +++ b/src/v4l2/v4l2_camera_proxy.cpp @@ -465,6 +465,8 @@ int V4L2CameraProxy::vidioc_streamon(int *arg) if (!validateBufferType(*arg)) return -EINVAL; + currentBuf_ = 0; + return vcam_->streamOn(); }