From patchwork Thu Mar 25 13:42:21 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kieran Bingham X-Patchwork-Id: 11711 X-Patchwork-Delegate: kieran.bingham@ideasonboard.com Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id B7042C32E9 for ; Thu, 25 Mar 2021 13:42:43 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 0619468D6F; Thu, 25 Mar 2021 14:42:42 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="XFLiqdm1"; dkim-atps=neutral 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 D952268D65 for ; Thu, 25 Mar 2021 14:42:36 +0100 (CET) Received: from Q.local (cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 1F5A89E2; Thu, 25 Mar 2021 14:42:36 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1616679756; bh=vWzp2bDJ+S4i2TlHdo96t+Gp4Gtj4US4xzNZZNTCwd0=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=XFLiqdm1lkJPqQeCOMI4E/fpelQkE+11NyWwDJ0jwesZ7ZQZXiRrBNEdOYK4w4wwj CJhLNmA5mhMkgU2mur/lvhl6mijBVpdhXsE73GUgU7cXh8/MO77XYSLqkYwB4VrsnT qZjJSevcAAphj4WtQhknyy0MszVmrdARwy9Ql0io= From: Kieran Bingham To: libcamera devel Date: Thu, 25 Mar 2021 13:42:21 +0000 Message-Id: <20210325134231.1400051-4-kieran.bingham@ideasonboard.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20210325134231.1400051-1-kieran.bingham@ideasonboard.com> References: <20210325134231.1400051-1-kieran.bingham@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v3 01/11] utils: ipc: proxy: Track IPA with a state machine 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: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" Asynchronous tasks can only be submitted while the IPA is running. Further more, the shutdown sequence can not be tracked with a simple running flag. We can also be in the state 'Stopping' where we have not yet completed all events, but we must not commence anything new. Refactor the running_ boolean into a stateful enum to track this. Signed-off-by: Kieran Bingham Reviewed-by: Laurent Pinchart Reviewed-by: Paul Elder --- .../libcamera_templates/module_ipa_proxy.cpp.tmpl | 8 ++++---- .../libcamera_templates/module_ipa_proxy.h.tmpl | 8 +++++++- .../generators/libcamera_templates/proxy_functions.tmpl | 7 +++++-- 3 files changed, 16 insertions(+), 7 deletions(-) diff --git a/utils/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl b/utils/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl index e3b541db4e36..dd0f4d3f64b6 100644 --- a/utils/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl +++ b/utils/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl @@ -45,7 +45,7 @@ namespace {{ns}} { {%- endif %} {{proxy_name}}::{{proxy_name}}(IPAModule *ipam, bool isolate) - : IPAProxy(ipam), running_(false), + : IPAProxy(ipam), state_(Stopped), isolate_(isolate), seq_(0) { LOG(IPAProxy, Debug) @@ -155,7 +155,7 @@ void {{proxy_name}}::recvMessage(const IPCMessage &data) return {{ "_ret" if method|method_return_value != "void" }}; {%- elif method.mojom_name == "start" %} - running_ = true; + state_ = Running; thread_.start(); {{ "return " if method|method_return_value != "void" -}} @@ -173,7 +173,7 @@ void {{proxy_name}}::recvMessage(const IPCMessage &data) {%- endfor -%} ); {% elif method|is_async %} - ASSERT(running_); + ASSERT(state_ == Running); proxy_.invokeMethod(&ThreadProxy::{{method.mojom_name}}, ConnectionTypeQueued, {%- for param in method|method_param_names -%} {{param}}{{- ", " if not loop.last}} @@ -226,7 +226,7 @@ void {{proxy_name}}::recvMessage(const IPCMessage &data) {% for method in interface_event.methods %} {{proxy_funcs.func_sig(proxy_name, method, "Thread")}} { - ASSERT(running_); + ASSERT(state_ != Stopped); {{method.mojom_name}}.emit({{method.parameters|params_comma_sep}}); } diff --git a/utils/ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl b/utils/ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl index efb09a180b90..e33c26492d91 100644 --- a/utils/ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl +++ b/utils/ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl @@ -104,7 +104,13 @@ private: {{interface_name}} *ipa_; }; - bool running_; + enum {{proxy_name}}States { + Stopped, + Stopping, + Running, + }; + enum {{proxy_name}}States state_; + Thread thread_; ThreadProxy proxy_; std::unique_ptr<{{interface_name}}> ipa_; diff --git a/utils/ipc/generators/libcamera_templates/proxy_functions.tmpl b/utils/ipc/generators/libcamera_templates/proxy_functions.tmpl index 8addc2fad0a8..09394a4fec83 100644 --- a/utils/ipc/generators/libcamera_templates/proxy_functions.tmpl +++ b/utils/ipc/generators/libcamera_templates/proxy_functions.tmpl @@ -23,9 +23,12 @@ # \brief Generate function body for IPA stop() function for thread #} {%- macro stop_thread_body() -%} - if (!running_) + ASSERT(state_ != Stopping); + if (state_ != Running) return; + state_ = Stopping; + proxy_.invokeMethod(&ThreadProxy::stop, ConnectionTypeBlocking); thread_.exit(); @@ -33,7 +36,7 @@ Thread::current()->dispatchMessages(Message::Type::InvokeMessage); - running_ = false; + state_ = Stopped; {%- endmacro -%} From patchwork Thu Mar 25 13:42:22 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kieran Bingham X-Patchwork-Id: 11712 X-Patchwork-Delegate: kieran.bingham@ideasonboard.com Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id 39537C32E9 for ; Thu, 25 Mar 2021 13:42:44 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id D715268D91; Thu, 25 Mar 2021 14:42:42 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="Bpm/M6lo"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 1416F68D66 for ; Thu, 25 Mar 2021 14:42:37 +0100 (CET) Received: from Q.local (cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 976909E7; Thu, 25 Mar 2021 14:42:36 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1616679756; bh=hQ0sgFCoscJlltfHBgSTK6SP8Xn/x/dsP5cWWB5UW/0=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=Bpm/M6loVcx5vy07eUGNdxT1n4nvZL/xYQdbUMH7GBR2Er8cLhqfBwSqnTwZL2RFo i/8kCtVq8lh4EpXXxMB6WKw/mOtol6ulO4OX1cKxuLJsWqsvBp3T8YuQA3ihccPoJ7 +IPM2IESNDxQQVBMwJgMtLkHHa1oJqTsZ3uXJ1MI= From: Kieran Bingham To: libcamera devel Date: Thu, 25 Mar 2021 13:42:22 +0000 Message-Id: <20210325134231.1400051-5-kieran.bingham@ideasonboard.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20210325134231.1400051-1-kieran.bingham@ideasonboard.com> References: <20210325134231.1400051-1-kieran.bingham@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v3 02/11] libcamera: buffer: Break friendship with Request 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: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" The FrameBuffer class is only friends with Request so that the request can be associated with the buffers. FrameBuffer already has a helper to setRequest(), so let's use that directly instead. Signed-off-by: Kieran Bingham Reviewed-by: Laurent Pinchart Reviewed-by: Paul Elder --- include/libcamera/buffer.h | 1 - src/libcamera/request.cpp | 9 +++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/include/libcamera/buffer.h b/include/libcamera/buffer.h index 302fe3d3e86b..620f8a66f6a2 100644 --- a/include/libcamera/buffer.h +++ b/include/libcamera/buffer.h @@ -56,7 +56,6 @@ public: private: LIBCAMERA_DISABLE_COPY_AND_MOVE(FrameBuffer) - friend class Request; /* Needed to update request_. */ friend class V4L2VideoDevice; /* Needed to update metadata_. */ std::vector planes_; diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp index 0071667e4a2c..3ad83f3b8418 100644 --- a/src/libcamera/request.cpp +++ b/src/libcamera/request.cpp @@ -118,8 +118,9 @@ void Request::reuse(ReuseFlag flags) pending_.clear(); if (flags & ReuseBuffers) { for (auto pair : bufferMap_) { - pair.second->request_ = this; - pending_.insert(pair.second); + FrameBuffer *buffer = pair.second; + buffer->setRequest(this); + pending_.insert(buffer); } } else { bufferMap_.clear(); @@ -187,7 +188,7 @@ int Request::addBuffer(const Stream *stream, FrameBuffer *buffer) return -EEXIST; } - buffer->request_ = this; + buffer->setRequest(this); pending_.insert(buffer); bufferMap_[stream] = buffer; @@ -294,7 +295,7 @@ bool Request::completeBuffer(FrameBuffer *buffer) int ret = pending_.erase(buffer); ASSERT(ret == 1); - buffer->request_ = nullptr; + buffer->setRequest(nullptr); if (buffer->metadata().status == FrameMetadata::FrameCancelled) cancelled_ = true; From patchwork Thu Mar 25 13:42:23 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kieran Bingham X-Patchwork-Id: 11713 X-Patchwork-Delegate: kieran.bingham@ideasonboard.com Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id 912ECC32EA for ; Thu, 25 Mar 2021 13:42:44 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id BBE7168D7B; Thu, 25 Mar 2021 14:42:43 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="kMf0D8aK"; dkim-atps=neutral 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 4FE2C68D6D for ; Thu, 25 Mar 2021 14:42:37 +0100 (CET) Received: from Q.local (cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id D959B560; Thu, 25 Mar 2021 14:42:36 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1616679757; bh=RSzG7AEw4TJuvB9rhe+Mrc8vkXoQZBWw+Jy4hCX5WaI=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=kMf0D8aKNl89+uqXGmkuSlDf/D0t1uPe9JiInmat0p3BlehkDLpjnS5byShgygsDI A6Wq2Z1GsUqT2G8lT8c1k2fp8E2KqDwDqzuc/J/vnnEJkFwYX1gU3fsalvok/7fjIm am6INmiacIjvIlHsKvjJr/i5Bt3uuQMnHQsQzWLo= From: Kieran Bingham To: libcamera devel Date: Thu, 25 Mar 2021 13:42:23 +0000 Message-Id: <20210325134231.1400051-6-kieran.bingham@ideasonboard.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20210325134231.1400051-1-kieran.bingham@ideasonboard.com> References: <20210325134231.1400051-1-kieran.bingham@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v3 03/11] libcamera: request: Provide a sequence number 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: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" Provide a sequence number on Requests which are added by the pipeline handler. Each pipeline handler keeps a requestSequence per CameraData and increments everytime a request is queued on that camera. The sequence number is associated with the Request and can be utilised for assisting with debugging, and printing the queueing sequence of in flight requests. Signed-off-by: Kieran Bingham Reviewed-by: Laurent Pinchart --- include/libcamera/internal/pipeline_handler.h | 4 +++- include/libcamera/request.h | 2 ++ src/libcamera/pipeline_handler.cpp | 2 ++ src/libcamera/request.cpp | 11 +++++++++-- 4 files changed, 16 insertions(+), 3 deletions(-) diff --git a/include/libcamera/internal/pipeline_handler.h b/include/libcamera/internal/pipeline_handler.h index 6aca0b46f43d..9bdda8f3b799 100644 --- a/include/libcamera/internal/pipeline_handler.h +++ b/include/libcamera/internal/pipeline_handler.h @@ -38,7 +38,7 @@ class CameraData { public: explicit CameraData(PipelineHandler *pipe) - : pipe_(pipe) + : pipe_(pipe), requestSequence_(0) { } virtual ~CameraData() = default; @@ -48,6 +48,8 @@ public: ControlInfoMap controlInfo_; ControlList properties_; + uint32_t requestSequence_; + private: LIBCAMERA_DISABLE_COPY(CameraData) }; diff --git a/include/libcamera/request.h b/include/libcamera/request.h index 6e5aad5f6b75..cd5a24741f8a 100644 --- a/include/libcamera/request.h +++ b/include/libcamera/request.h @@ -50,6 +50,7 @@ public: int addBuffer(const Stream *stream, FrameBuffer *buffer); FrameBuffer *findBuffer(const Stream *stream) const; + uint32_t sequence() const { return sequence_; } uint64_t cookie() const { return cookie_; } Status status() const { return status_; } @@ -71,6 +72,7 @@ private: BufferMap bufferMap_; std::unordered_set pending_; + uint32_t sequence_; const uint64_t cookie_; Status status_; bool cancelled_; diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp index d22991d318c9..e3d4975d9ffd 100644 --- a/src/libcamera/pipeline_handler.cpp +++ b/src/libcamera/pipeline_handler.cpp @@ -382,6 +382,8 @@ int PipelineHandler::queueRequest(Request *request) CameraData *data = cameraData(camera); data->queuedRequests_.push_back(request); + request->sequence_ = data->requestSequence_++; + int ret = queueRequestDevice(camera, request); if (ret) data->queuedRequests_.remove(request); diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp index 3ad83f3b8418..fc16b148a599 100644 --- a/src/libcamera/request.cpp +++ b/src/libcamera/request.cpp @@ -72,8 +72,8 @@ LOG_DEFINE_CATEGORY(Request) * */ Request::Request(Camera *camera, uint64_t cookie) - : camera_(camera), cookie_(cookie), status_(RequestPending), - cancelled_(false) + : camera_(camera), sequence_(0), cookie_(cookie), + status_(RequestPending), cancelled_(false) { /** * \todo Should the Camera expose a validator instance, to avoid @@ -126,6 +126,7 @@ void Request::reuse(ReuseFlag flags) bufferMap_.clear(); } + sequence_ = 0; status_ = RequestPending; cancelled_ = false; @@ -227,6 +228,12 @@ FrameBuffer *Request::findBuffer(const Stream *stream) const * \return The metadata associated with the request */ +/** + * \fn Request::sequence() + * \brief Retrieve the sequence number for the request + * \return The request sequence number + */ + /** * \fn Request::cookie() * \brief Retrieve the cookie set when the request was created From patchwork Thu Mar 25 13:42:24 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kieran Bingham X-Patchwork-Id: 11714 X-Patchwork-Delegate: kieran.bingham@ideasonboard.com Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id 0856AC32E9 for ; Thu, 25 Mar 2021 13:42:45 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 82D1668D80; Thu, 25 Mar 2021 14:42:44 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="pjze5PUN"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 8E03A602D5 for ; Thu, 25 Mar 2021 14:42:37 +0100 (CET) Received: from Q.local (cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 362268C9; Thu, 25 Mar 2021 14:42:37 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1616679757; bh=U/GFlKH0quUlUYD8QEmuZuLLxlaij3Pq//oVy3qUhFU=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=pjze5PUNUJkttqIjQnorxSvnPntRbslCRCNP28JyrmDZagt7ULxPBL1oXU3zUdOr3 Ui71Xbz828eCVAmMqauBI3Ib8+IAeBUwTs8TnFjNnvY3Mj6tTX+6uE1dN0xbKpKVpy mrmF8yUFpqiysUtWvWj9KTnLffcuIFzOtZqQqk8A= From: Kieran Bingham To: libcamera devel Date: Thu, 25 Mar 2021 13:42:24 +0000 Message-Id: <20210325134231.1400051-7-kieran.bingham@ideasonboard.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20210325134231.1400051-1-kieran.bingham@ideasonboard.com> References: <20210325134231.1400051-1-kieran.bingham@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v3 04/11] libcamera: request: Add a toString() 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: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" Provide a toString helper to assist in printing Request state for debug and logging contexts. Signed-off-by: Kieran Bingham Reviewed-by: Laurent Pinchart --- include/libcamera/request.h | 3 +++ src/libcamera/request.cpp | 30 +++++++++++++++++++++++++++--- 2 files changed, 30 insertions(+), 3 deletions(-) diff --git a/include/libcamera/request.h b/include/libcamera/request.h index cd5a24741f8a..4cf5ff3f7d3b 100644 --- a/include/libcamera/request.h +++ b/include/libcamera/request.h @@ -10,6 +10,7 @@ #include #include #include +#include #include #include @@ -56,6 +57,8 @@ public: bool hasPendingBuffers() const { return !pending_.empty(); } + std::string toString() const; + private: LIBCAMERA_DISABLE_COPY(Request) diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp index fc16b148a599..7b7ef6814686 100644 --- a/src/libcamera/request.cpp +++ b/src/libcamera/request.cpp @@ -8,6 +8,7 @@ #include #include +#include #include #include @@ -261,6 +262,16 @@ FrameBuffer *Request::findBuffer(const Stream *stream) const * otherwise */ +/** + * \fn Request::toString() + * \brief Generate a string representation of the Request internals + * + * This function facilitates debugging of Request state while it is used + * internally within libcamera. + * + * \return A string representing the current state of the request + */ + /** * \brief Complete a queued request * @@ -275,9 +286,7 @@ void Request::complete() status_ = cancelled_ ? RequestCancelled : RequestComplete; - LOG(Request, Debug) - << "Request has completed - cookie: " << cookie_ - << (cancelled_ ? " [Cancelled]" : ""); + LOG(Request, Debug) << toString(); LIBCAMERA_TRACEPOINT(request_complete, this); } @@ -310,4 +319,19 @@ bool Request::completeBuffer(FrameBuffer *buffer) return !hasPendingBuffers(); } +std::string Request::toString() const +{ + std::stringstream ss; + + /* Pending, Completed, Cancelled(X). */ + static const char *statuses = "PCX"; + + /* Example Output: Request(55:P:1/2:6523524) */ + ss << "Request (" << sequence_ << ":" << statuses[status_] << ":" + << pending_.size() << "/" << bufferMap_.size() << ":" + << cookie_ << ")"; + + return ss.str(); +} + } /* namespace libcamera */ From patchwork Thu Mar 25 13:42:25 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kieran Bingham X-Patchwork-Id: 11715 X-Patchwork-Delegate: kieran.bingham@ideasonboard.com Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id 6B5EFC32EA for ; Thu, 25 Mar 2021 13:42:45 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 2640D68D82; Thu, 25 Mar 2021 14:42:45 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="dYr77qpk"; dkim-atps=neutral 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 C7F7D6084F for ; Thu, 25 Mar 2021 14:42:37 +0100 (CET) Received: from Q.local (cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 71A678F0; Thu, 25 Mar 2021 14:42:37 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1616679757; bh=x4dfp0V8U+JXITHQd/RnB9wllql332QTPjl8OdeHnv8=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=dYr77qpkLKywaywu0N9duaQYKT3FjPeJ5b2utyCDUsQh84lU+eChwf+36jNipS1ph khRTFplOwnhovVchTRUahyYKMXUbAyLwDL9Cnpih51WTSkmAEKEUl6a4wUiPQuCyXh x0BX/wEazD5JOtPjlw/PpGroqKvUF0c06Ba5qdkc= From: Kieran Bingham To: libcamera devel Date: Thu, 25 Mar 2021 13:42:25 +0000 Message-Id: <20210325134231.1400051-8-kieran.bingham@ideasonboard.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20210325134231.1400051-1-kieran.bingham@ideasonboard.com> References: <20210325134231.1400051-1-kieran.bingham@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v3 05/11] libcamera: request: A request canary 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: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" Request objects are created and owned by the application, but are of course utilised widely throughout the internals of libcamera. If the application free's the requests while they are still active within libcamera a use after free will occur. While this can be trapped by tools such as valgrind, given the importance of this object and the relationship of external ownership, it may have some value to provide Debug build (disabled at Release build) assertions on the condition of these objects. Make sure the request fails an assertion immediately if used after free. Signed-off-by: Kieran Bingham --- I've removed the RFC from this, and I actually even more so believe this is a good facility to provide on the Request object. Mostly because the Request object is the only object which is given to libcamera from the application (yes, created by libcamera, but that's separate), and is expected to be free'd by the application. If an application free's requests while they are still in use within libcamera, the symptoms can be distinctly misleading and lead to rabbit holes. Therefore, I think the Request object is the one place where extra safety checking (in debug builds) is worth while. Yes, of course if an application free's a request while it's in use with libcamera - then it's the applications fault, not ours - but because of the nature of requests, this could be an easy trap to fall into - and I don't want to find that reported as bugs in libcamera. --- include/libcamera/request.h | 2 ++ src/libcamera/request.cpp | 18 ++++++++++++++++++ 2 files changed, 20 insertions(+) diff --git a/include/libcamera/request.h b/include/libcamera/request.h index 4cf5ff3f7d3b..965ffa6b45b2 100644 --- a/include/libcamera/request.h +++ b/include/libcamera/request.h @@ -79,6 +79,8 @@ private: const uint64_t cookie_; Status status_; bool cancelled_; + + uint32_t canary_; }; } /* namespace libcamera */ diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp index 7b7ef6814686..c4258480b12b 100644 --- a/src/libcamera/request.cpp +++ b/src/libcamera/request.cpp @@ -19,6 +19,8 @@ #include "libcamera/internal/log.h" #include "libcamera/internal/tracepoints.h" +#define REQUEST_CANARY 0x1F2E3D4C + /** * \file request.h * \brief Describes a frame capture request to be processed by a camera @@ -90,6 +92,8 @@ Request::Request(Camera *camera, uint64_t cookie) LIBCAMERA_TRACEPOINT(request_construct, this); + canary_ = REQUEST_CANARY; + LOG(Request, Debug) << "Created request - cookie: " << cookie_; } @@ -100,6 +104,8 @@ Request::~Request() delete metadata_; delete controls_; delete validator_; + + canary_ = 0; } /** @@ -114,6 +120,8 @@ Request::~Request() */ void Request::reuse(ReuseFlag flags) { + ASSERT(canary_ == REQUEST_CANARY); + LIBCAMERA_TRACEPOINT(request_reuse, this); pending_.clear(); @@ -179,6 +187,8 @@ void Request::reuse(ReuseFlag flags) */ int Request::addBuffer(const Stream *stream, FrameBuffer *buffer) { + ASSERT(canary_ == REQUEST_CANARY); + if (!stream) { LOG(Request, Error) << "Invalid stream reference"; return -EINVAL; @@ -214,6 +224,8 @@ int Request::addBuffer(const Stream *stream, FrameBuffer *buffer) */ FrameBuffer *Request::findBuffer(const Stream *stream) const { + ASSERT(canary_ == REQUEST_CANARY); + const auto it = bufferMap_.find(stream); if (it == bufferMap_.end()) return nullptr; @@ -281,6 +293,7 @@ FrameBuffer *Request::findBuffer(const Stream *stream) const */ void Request::complete() { + ASSERT(canary_ == REQUEST_CANARY); ASSERT(status_ == RequestPending); ASSERT(!hasPendingBuffers()); @@ -306,6 +319,8 @@ void Request::complete() */ bool Request::completeBuffer(FrameBuffer *buffer) { + ASSERT(canary_ == REQUEST_CANARY); + LIBCAMERA_TRACEPOINT(request_complete_buffer, this, buffer); int ret = pending_.erase(buffer); @@ -326,6 +341,9 @@ std::string Request::toString() const /* Pending, Completed, Cancelled(X). */ static const char *statuses = "PCX"; + if (canary_ != REQUEST_CANARY) + return "Invalid Canary on Request"; + /* Example Output: Request(55:P:1/2:6523524) */ ss << "Request (" << sequence_ << ":" << statuses[status_] << ":" << pending_.size() << "/" << bufferMap_.size() << ":" From patchwork Thu Mar 25 13:42:26 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Kieran Bingham X-Patchwork-Id: 11716 X-Patchwork-Delegate: kieran.bingham@ideasonboard.com Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id 3382DC32E9 for ; Thu, 25 Mar 2021 13:42:48 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id E66BB68D7E; Thu, 25 Mar 2021 14:42:47 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="A7soGS87"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 17F9668D66 for ; Thu, 25 Mar 2021 14:42:38 +0100 (CET) Received: from Q.local (cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id AC2F7560; Thu, 25 Mar 2021 14:42:37 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1616679757; bh=3phRjmHWCr/7dGInyNkMdaCLwsrOcGcBRQnA3ggRd3Y=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=A7soGS874I1jwxt6MIfuTt6rh51m4qieuJKGwB6HzqwfIUq0KXlg7Wkp8iW1ww7dc dnLQ/EvrnhtWnqbgKjkDNAiishOlzjOsn2GQtML9Wc4VBrvsRbj0PCfcuHd01cnVku 2teM3a8kMpfxk/HCBm7ZEtAskUqSOtX9kTvmX4Q4= From: Kieran Bingham To: libcamera devel Date: Thu, 25 Mar 2021 13:42:26 +0000 Message-Id: <20210325134231.1400051-9-kieran.bingham@ideasonboard.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20210325134231.1400051-1-kieran.bingham@ideasonboard.com> References: <20210325134231.1400051-1-kieran.bingham@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v3 06/11] libcamera: camera: Validate requests are completed in Running state 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: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" All requests must have completed before the Camera has fully stopped. Requests completing when the camera is not running represent an internal pipeline handler bug. Trap this event with a fatal error. Reviewed-by: Niklas Söderlund Signed-off-by: Kieran Bingham Reviewed-by: Laurent Pinchart --- src/libcamera/camera.cpp | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp index 84edbb8f494d..b86bff4745b2 100644 --- a/src/libcamera/camera.cpp +++ b/src/libcamera/camera.cpp @@ -1062,11 +1062,11 @@ int Camera::stop() LOG(Camera, Debug) << "Stopping capture"; - d->setState(Private::CameraConfigured); - d->pipe_->invokeMethod(&PipelineHandler::stop, ConnectionTypeBlocking, this); + d->setState(Private::CameraConfigured); + return 0; } @@ -1079,6 +1079,13 @@ int Camera::stop() */ void Camera::requestComplete(Request *request) { + Private *const d = LIBCAMERA_D_PTR(); + + /* Disconnected cameras are still able to complete requests. */ + int ret = d->isAccessAllowed(Private::CameraRunning); + if (ret < 0 && ret != -ENODEV) + LOG(Camera, Fatal) << "Trying to complete a request when stopped"; + requestCompleted.emit(request); } From patchwork Thu Mar 25 13:42:27 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kieran Bingham X-Patchwork-Id: 11717 X-Patchwork-Delegate: kieran.bingham@ideasonboard.com Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id C9C97C32E9 for ; Thu, 25 Mar 2021 13:42:48 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 84A9F68D79; Thu, 25 Mar 2021 14:42:48 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="kTUJlEUP"; dkim-atps=neutral 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 5675368D6D for ; Thu, 25 Mar 2021 14:42:38 +0100 (CET) Received: from Q.local (cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id F177F8C9; Thu, 25 Mar 2021 14:42:37 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1616679758; bh=Bo5mxA6szGRlwnU8mKkgLU3L3+ybPx3+GNMM9XEOCac=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=kTUJlEUPaii7Lwa9vLYXBklFY988p1DgXWhfpwNcNTFqeWKW+vlqqiS2cUwfQxjCJ lIWqvknEKPpn7KRCtIt3cSa5qRBJ5DR4sN4BC3hSPShKZcxR7BfHuxC7/EWfxEz9Vt cntArSsrmxgdJ1KQa+TDKjeNcKR17q9vyI1M7PoE= From: Kieran Bingham To: libcamera devel Date: Thu, 25 Mar 2021 13:42:27 +0000 Message-Id: <20210325134231.1400051-10-kieran.bingham@ideasonboard.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20210325134231.1400051-1-kieran.bingham@ideasonboard.com> References: <20210325134231.1400051-1-kieran.bingham@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v3 07/11] libcamera: camera: Report function which fails access control 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: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" The camera object has a state machine to ensure calls are only made when in the correct state. It isn't easy to identify where things happen when assertions fail so add extra information to make this clearer. Signed-off-by: Kieran Bingham Reviewed-by: Laurent Pinchart --- src/libcamera/camera.cpp | 51 +++++++++++++++++++++++----------------- 1 file changed, 30 insertions(+), 21 deletions(-) diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp index b86bff4745b2..336ab8695ab3 100644 --- a/src/libcamera/camera.cpp +++ b/src/libcamera/camera.cpp @@ -346,8 +346,9 @@ public: const std::set &streams); ~Private(); - int isAccessAllowed(State state, bool allowDisconnected = false) const; - int isAccessAllowed(State low, State high, + int isAccessAllowed(const char *from, State state, + bool allowDisconnected = false) const; + int isAccessAllowed(const char *from, State low, State high, bool allowDisconnected = false) const; void disconnect(); @@ -384,7 +385,8 @@ static const char *const camera_state_names[] = { "Running", }; -int Camera::Private::isAccessAllowed(State state, bool allowDisconnected) const +int Camera::Private::isAccessAllowed(const char *from, State state, + bool allowDisconnected) const { if (!allowDisconnected && disconnected_) return -ENODEV; @@ -395,14 +397,16 @@ int Camera::Private::isAccessAllowed(State state, bool allowDisconnected) const ASSERT(static_cast(state) < std::size(camera_state_names)); - LOG(Camera, Debug) << "Camera in " << camera_state_names[currentState] - << " state trying operation requiring state " - << camera_state_names[state]; + LOG(Camera, Warning) << "Camera in " << camera_state_names[currentState] + << " state trying operation [" + << from << "] requiring state " + << camera_state_names[state]; return -EACCES; } -int Camera::Private::isAccessAllowed(State low, State high, +int Camera::Private::isAccessAllowed(const char *from, + State low, State high, bool allowDisconnected) const { if (!allowDisconnected && disconnected_) @@ -415,10 +419,11 @@ int Camera::Private::isAccessAllowed(State low, State high, ASSERT(static_cast(low) < std::size(camera_state_names) && static_cast(high) < std::size(camera_state_names)); - LOG(Camera, Debug) << "Camera in " << camera_state_names[currentState] - << " state trying operation requiring state between " - << camera_state_names[low] << " and " - << camera_state_names[high]; + LOG(Camera, Warning) << "Camera in " << camera_state_names[currentState] + << " state trying operation [" << from + << "] requiring state between " + << camera_state_names[low] << " and " + << camera_state_names[high]; return -EACCES; } @@ -646,7 +651,7 @@ int Camera::exportFrameBuffers(Stream *stream, { Private *const d = LIBCAMERA_D_PTR(); - int ret = d->isAccessAllowed(Private::CameraConfigured); + int ret = d->isAccessAllowed(__FUNCTION__, Private::CameraConfigured); if (ret < 0) return ret; @@ -693,7 +698,7 @@ int Camera::acquire() * No manual locking is required as PipelineHandler::lock() is * thread-safe. */ - int ret = d->isAccessAllowed(Private::CameraAvailable); + int ret = d->isAccessAllowed(__FUNCTION__, Private::CameraAvailable); if (ret < 0) return ret == -EACCES ? -EBUSY : ret; @@ -726,7 +731,8 @@ int Camera::release() { Private *const d = LIBCAMERA_D_PTR(); - int ret = d->isAccessAllowed(Private::CameraAvailable, + int ret = d->isAccessAllowed(__FUNCTION__, + Private::CameraAvailable, Private::CameraConfigured, true); if (ret < 0) return ret == -EACCES ? -EBUSY : ret; @@ -805,7 +811,8 @@ std::unique_ptr Camera::generateConfiguration(const StreamR { Private *const d = LIBCAMERA_D_PTR(); - int ret = d->isAccessAllowed(Private::CameraAvailable, + int ret = d->isAccessAllowed(__FUNCTION__, + Private::CameraAvailable, Private::CameraRunning); if (ret < 0) return nullptr; @@ -866,7 +873,8 @@ int Camera::configure(CameraConfiguration *config) { Private *const d = LIBCAMERA_D_PTR(); - int ret = d->isAccessAllowed(Private::CameraAcquired, + int ret = d->isAccessAllowed(__FUNCTION__, + Private::CameraAcquired, Private::CameraConfigured); if (ret < 0) return ret; @@ -938,7 +946,8 @@ std::unique_ptr Camera::createRequest(uint64_t cookie) { Private *const d = LIBCAMERA_D_PTR(); - int ret = d->isAccessAllowed(Private::CameraConfigured, + int ret = d->isAccessAllowed(__FUNCTION__, + Private::CameraConfigured, Private::CameraRunning); if (ret < 0) return nullptr; @@ -972,7 +981,7 @@ int Camera::queueRequest(Request *request) { Private *const d = LIBCAMERA_D_PTR(); - int ret = d->isAccessAllowed(Private::CameraRunning); + int ret = d->isAccessAllowed(__FUNCTION__, Private::CameraRunning); if (ret < 0) return ret; @@ -1022,7 +1031,7 @@ int Camera::start(const ControlList *controls) { Private *const d = LIBCAMERA_D_PTR(); - int ret = d->isAccessAllowed(Private::CameraConfigured); + int ret = d->isAccessAllowed(__FUNCTION__, Private::CameraConfigured); if (ret < 0) return ret; @@ -1056,7 +1065,7 @@ int Camera::stop() { Private *const d = LIBCAMERA_D_PTR(); - int ret = d->isAccessAllowed(Private::CameraRunning); + int ret = d->isAccessAllowed(__FUNCTION__, Private::CameraRunning); if (ret < 0) return ret; @@ -1082,7 +1091,7 @@ void Camera::requestComplete(Request *request) Private *const d = LIBCAMERA_D_PTR(); /* Disconnected cameras are still able to complete requests. */ - int ret = d->isAccessAllowed(Private::CameraRunning); + int ret = d->isAccessAllowed(__FUNCTION__, Private::CameraRunning); if (ret < 0 && ret != -ENODEV) LOG(Camera, Fatal) << "Trying to complete a request when stopped"; From patchwork Thu Mar 25 13:42:28 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kieran Bingham X-Patchwork-Id: 11718 X-Patchwork-Delegate: kieran.bingham@ideasonboard.com Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id 5826BC32E9 for ; Thu, 25 Mar 2021 13:42:49 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 166E268D74; Thu, 25 Mar 2021 14:42:49 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="Wsqc09ge"; dkim-atps=neutral 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 92FA368D78 for ; Thu, 25 Mar 2021 14:42:38 +0100 (CET) Received: from Q.local (cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 3805E8F0; Thu, 25 Mar 2021 14:42:38 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1616679758; bh=jy5bYpvfy4XXzv0fMaKRnM42klCMw03KqumuNftoGeA=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=Wsqc09gez3ixKcPSpBs1qrJJ2oglb6bTSKvF9IrbzJYz9KPQQNyd2M5D8BC8R+LZh 0IYj7ShWb3eky9zGI5xvHErDmPdUlpLMctcN1APq1fQTDf68zTeUmaPKgl7QTnTDgI yPi5ENC8L7pdaetMQpZd6hpxiz3MAFKDNtBrOZes= From: Kieran Bingham To: libcamera devel Date: Thu, 25 Mar 2021 13:42:28 +0000 Message-Id: <20210325134231.1400051-11-kieran.bingham@ideasonboard.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20210325134231.1400051-1-kieran.bingham@ideasonboard.com> References: <20210325134231.1400051-1-kieran.bingham@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v3 08/11] libcamera: camera: Extend with a Stopping state 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: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" When the camera is being stop()ped, active requests will complete. These may trigger an application to re-queue those requests to the camera but that is not permitted. Extend the camera state to include a stopping state which is entered as soon as a call to stop() is made. At this point, any request queued will be rejected with a warning, while any pending requests are either successfully completed or cancelled. When the pipeline handler has finished stopping, the camera state will transition to the CameraConfigured state where it can begin to accept requests again, and be restarted. Signed-off-by: Kieran Bingham Reviewed-by: Laurent Pinchart --- src/libcamera/camera.cpp | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp index 336ab8695ab3..7f7956ba732f 100644 --- a/src/libcamera/camera.cpp +++ b/src/libcamera/camera.cpp @@ -339,6 +339,7 @@ public: CameraAvailable, CameraAcquired, CameraConfigured, + CameraStopping, CameraRunning, }; @@ -382,6 +383,7 @@ static const char *const camera_state_names[] = { "Available", "Acquired", "Configured", + "Stopping", "Running", }; @@ -492,6 +494,7 @@ void Camera::Private::setState(State state) * node [shape = doublecircle ]; Available; * node [shape = circle ]; Acquired; * node [shape = circle ]; Configured; + * node [shape = circle ]; Stopping; * node [shape = circle ]; Running; * * Available -> Available [label = "release()"]; @@ -504,7 +507,8 @@ void Camera::Private::setState(State state) * Configured -> Configured [label = "configure(), createRequest()"]; * Configured -> Running [label = "start()"]; * - * Running -> Configured [label = "stop()"]; + * Running -> Stopping [label = "stop()"]; + * Stopping -> Configured; * Running -> Running [label = "createRequest(), queueRequest()"]; * } * \enddot @@ -524,6 +528,12 @@ void Camera::Private::setState(State state) * release() the camera and to get back to the Available state or start() * it to progress to the Running state. * + * \subsubsection Stopping + * The camera has been asked to stop. Pending reqeusts are being completed or + * cancelled, and no new requests are permitted to be queued. The camera will + * transition to the Configured state when all queued requests have been + * returned to the application. + * * \subsubsection Running * The camera is running and ready to process requests queued by the * application. The camera remains in this state until it is stopped and moved @@ -1071,6 +1081,8 @@ int Camera::stop() LOG(Camera, Debug) << "Stopping capture"; + d->setState(Private::CameraStopping); + d->pipe_->invokeMethod(&PipelineHandler::stop, ConnectionTypeBlocking, this); @@ -1091,7 +1103,9 @@ void Camera::requestComplete(Request *request) Private *const d = LIBCAMERA_D_PTR(); /* Disconnected cameras are still able to complete requests. */ - int ret = d->isAccessAllowed(__FUNCTION__, Private::CameraRunning); + int ret = d->isAccessAllowed(__FUNCTION__, + Private::CameraStopping, + Private::CameraRunning); if (ret < 0 && ret != -ENODEV) LOG(Camera, Fatal) << "Trying to complete a request when stopped"; From patchwork Thu Mar 25 13:42:29 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kieran Bingham X-Patchwork-Id: 11719 X-Patchwork-Delegate: kieran.bingham@ideasonboard.com Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id E250CC32E9 for ; Thu, 25 Mar 2021 13:42:49 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 9E99A68D94; Thu, 25 Mar 2021 14:42:49 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="FI8Q3kwg"; dkim-atps=neutral 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 181C6602D9 for ; Thu, 25 Mar 2021 14:42:39 +0100 (CET) Received: from Q.local (cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 776009E2; Thu, 25 Mar 2021 14:42:38 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1616679758; bh=xdRm2p4suosxsWk6793Kifkf8epaM/MJJjtDVNTULmc=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=FI8Q3kwghS24OKmzJ/N2/USea9KV1CnvMsMu8ixbXHV8qcyK01bnpwq0vylwt7QvR RXwrgkGRVQzUz2PicTYoxcJpozQI+ficJOOsOz4q+K9aRALQhS1pJRP02Yl8pswMAE bOtP7qIIoR/I0i2U3KA6Iz4yV8bV3QE2JuXyHD6w= From: Kieran Bingham To: libcamera devel Date: Thu, 25 Mar 2021 13:42:29 +0000 Message-Id: <20210325134231.1400051-12-kieran.bingham@ideasonboard.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20210325134231.1400051-1-kieran.bingham@ideasonboard.com> References: <20210325134231.1400051-1-kieran.bingham@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v3 09/11] libcamera: camera: Assert pipelines complete all requests 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: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" When the camera manager calls stop on a pipeline, it is expected that the pipeline handler guarantees all requests are returned back to the application before the camera has stopped. Ensure that this guarantee is met by providing an accessor on the pipeline handler to validate that all pending requests are removed. Signed-off-by: Kieran Bingham --- Note: I wanted to make PipelineHandler::active() a const function, but then I'm unable to call it through invokeMethod(). Does invokeMethod support calling const functions? Or did I do something obviously wrong? --- include/libcamera/internal/pipeline_handler.h | 1 + src/libcamera/camera.cpp | 3 +++ src/libcamera/pipeline_handler.cpp | 17 +++++++++++++++++ 3 files changed, 21 insertions(+) diff --git a/include/libcamera/internal/pipeline_handler.h b/include/libcamera/internal/pipeline_handler.h index 9bdda8f3b799..1410a06ebabe 100644 --- a/include/libcamera/internal/pipeline_handler.h +++ b/include/libcamera/internal/pipeline_handler.h @@ -80,6 +80,7 @@ public: virtual int start(Camera *camera, const ControlList *controls) = 0; virtual void stop(Camera *camera) = 0; + bool active(const Camera *camera); int queueRequest(Request *request); diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp index 7f7956ba732f..99b01633ff5f 100644 --- a/src/libcamera/camera.cpp +++ b/src/libcamera/camera.cpp @@ -1086,6 +1086,9 @@ int Camera::stop() d->pipe_->invokeMethod(&PipelineHandler::stop, ConnectionTypeBlocking, this); + ASSERT(!d->pipe_->invokeMethod(&PipelineHandler::active, + ConnectionTypeBlocking, this)); + d->setState(Private::CameraConfigured); return 0; diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp index e3d4975d9ffd..c1ea4374b445 100644 --- a/src/libcamera/pipeline_handler.cpp +++ b/src/libcamera/pipeline_handler.cpp @@ -357,6 +357,23 @@ const ControlList &PipelineHandler::properties(const Camera *camera) const * \context This function is called from the CameraManager thread. */ +/** + * \brief Determine if the camera has any requests pending + * \param[in] camera The camera to check + * + * This method determines if there are any requests queued to the pipeline + * awaiting processing. + * + * \context This function is called from the CameraManager thread. + * + * \return True if there are pending requests, or false otherwise + */ +bool PipelineHandler::active(const Camera *camera) +{ + const CameraData *data = cameraData(camera); + return !data->queuedRequests_.empty(); +} + /** * \fn PipelineHandler::queueRequest() * \brief Queue a request From patchwork Thu Mar 25 13:42:18 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kieran Bingham X-Patchwork-Id: 11708 X-Patchwork-Delegate: kieran.bingham@ideasonboard.com Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id DAC9EC32E9 for ; Thu, 25 Mar 2021 13:42:39 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 3BE7D68D74; Thu, 25 Mar 2021 14:42:38 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="jPmdKJAK"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 6543B602D5 for ; Thu, 25 Mar 2021 14:42:36 +0100 (CET) Received: from Q.local (cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 3BF9B560; Thu, 25 Mar 2021 14:42:35 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1616679755; bh=Zx2LcXfUZnX8WTgOgtW7xBH5lkQR0N1PFz7qAVL3mlk=; h=From:To:Cc:Subject:Date:From; b=jPmdKJAKSEZHjj8AndE1kU/6d25wrAl5bmKXXnu2UHTJ3ZV0TWwalvBMTRgGrHfof Na2RCpJZViEY/DSix91FmlWswY1hhSE24tdwnNb63LVZRLF6wOMQVe25vjm8LH5EB8 YhHy7GhSZQ6oLXDqRbuKtsCLUhooF/3ASLzaAnxY= From: Kieran Bingham To: libcamera devel Date: Thu, 25 Mar 2021 13:42:18 +0000 Message-Id: <20210325134231.1400051-1-kieran.bingham@ideasonboard.com> X-Mailer: git-send-email 2.25.1 MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 10/11] libcamera: pipeline: ipu3: frames: Fail if the FrameInfo can't be found 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: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" The FrameInfo structure associates the data sent to the IPA and is essential for handling events. If it can not be found, this is a fatal error which must be fixed. Signed-off-by: Kieran Bingham --- v3: - Make all occurrences of failing to find a frame info fatal. --- src/libcamera/pipeline/ipu3/frames.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/libcamera/pipeline/ipu3/frames.cpp b/src/libcamera/pipeline/ipu3/frames.cpp index e8eb1c5103e8..34ab4be6711b 100644 --- a/src/libcamera/pipeline/ipu3/frames.cpp +++ b/src/libcamera/pipeline/ipu3/frames.cpp @@ -115,7 +115,8 @@ IPU3Frames::Info *IPU3Frames::find(unsigned int id) if (itInfo != frameInfo_.end()) return itInfo->second.get(); - LOG(IPU3, Error) << "Can't find tracking informaton for frame " << id; + LOG(IPU3, Fatal) << "Can't find tracking informaton for frame " << id; + return nullptr; } @@ -133,7 +134,8 @@ IPU3Frames::Info *IPU3Frames::find(FrameBuffer *buffer) return info; } - LOG(IPU3, Error) << "Can't find tracking informaton from buffer"; + LOG(IPU3, Fatal) << "Can't find tracking informaton from buffer"; + return nullptr; } From patchwork Thu Mar 25 13:42:19 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kieran Bingham X-Patchwork-Id: 11709 X-Patchwork-Delegate: kieran.bingham@ideasonboard.com Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id 5939BC32E9 for ; Thu, 25 Mar 2021 13:42:41 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 320F768D7A; Thu, 25 Mar 2021 14:42:39 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="durc1w8A"; dkim-atps=neutral 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 9AE7D602D9 for ; Thu, 25 Mar 2021 14:42:36 +0100 (CET) Received: from Q.local (cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 7AA3A8C9; Thu, 25 Mar 2021 14:42:35 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1616679755; bh=+eJVhfP/EG7xCEzMb3zus1otZQVQ8RQJ4sSGGnO+zY4=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=durc1w8ASfSF9zCd9jVUmDH0AOdUAAM7+XAoWlCRQOztlF6C8+o2x7jvb5wwNLPvN 0UX8y0ZPeTuSBWL/Qt5WtSW0eu68CHgqTurJqD7RP6I1FuGQPl3ilCHoWfnmcTzzif wV2d9a7Q6iDUvzNsbCnaivff3VSFgwl7FYJdQcrI= From: Kieran Bingham To: libcamera devel Date: Thu, 25 Mar 2021 13:42:19 +0000 Message-Id: <20210325134231.1400051-2-kieran.bingham@ideasonboard.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20210325134231.1400051-1-kieran.bingham@ideasonboard.com> References: <20210325134231.1400051-1-kieran.bingham@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 11/11] libcamera: pipeline: ipu3: frames: Use the request sequence 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: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" For all frame indexes, use the same sequence number as generated by the Request object. This allows clear matching of what operations occurred to which request. Reviewed-by: Laurent Pinchart Signed-off-by: Kieran Bingham --- src/libcamera/pipeline/ipu3/frames.cpp | 4 +--- src/libcamera/pipeline/ipu3/frames.h | 1 - 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/src/libcamera/pipeline/ipu3/frames.cpp b/src/libcamera/pipeline/ipu3/frames.cpp index 34ab4be6711b..a1b014eed8d7 100644 --- a/src/libcamera/pipeline/ipu3/frames.cpp +++ b/src/libcamera/pipeline/ipu3/frames.cpp @@ -18,7 +18,6 @@ namespace libcamera { LOG_DECLARE_CATEGORY(IPU3) IPU3Frames::IPU3Frames() - : nextId_(0) { } @@ -31,7 +30,6 @@ void IPU3Frames::init(const std::vector> ¶mBuff for (const std::unique_ptr &buffer : statBuffers) availableStatBuffers_.push(buffer.get()); - nextId_ = 0; frameInfo_.clear(); } @@ -43,7 +41,7 @@ void IPU3Frames::clear() IPU3Frames::Info *IPU3Frames::create(Request *request) { - unsigned int id = nextId_++; + unsigned int id = request->sequence(); if (availableParamBuffers_.empty()) { LOG(IPU3, Error) << "Parameters buffer underrun"; diff --git a/src/libcamera/pipeline/ipu3/frames.h b/src/libcamera/pipeline/ipu3/frames.h index 106e5c15cc7a..4acdf48eca9d 100644 --- a/src/libcamera/pipeline/ipu3/frames.h +++ b/src/libcamera/pipeline/ipu3/frames.h @@ -53,7 +53,6 @@ private: std::queue availableParamBuffers_; std::queue availableStatBuffers_; - unsigned int nextId_; std::map> frameInfo_; };