From patchwork Mon Oct 21 13:37:16 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Milan Zamazal X-Patchwork-Id: 21717 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 AE3C8C32A3 for ; Mon, 21 Oct 2024 13:37:46 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 3B74F65396; Mon, 21 Oct 2024 15:37:46 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.b="Dvb4XaL/"; dkim-atps=neutral Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 2C3DB65391 for ; Mon, 21 Oct 2024 15:37:44 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1729517862; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=wYyoqLgpnrGxadYdhZIHa01vjrE1UhBE+glbzSDU8DA=; b=Dvb4XaL/R+zFJggwT880+V1d1CZQzhJJKpKOaVq/PboAGYCWnMDXV2zKSls5XNiuNBTBSQ 0PwF5SNNcPYse26Vr4wS3kmhy6PmRXtfm6t/SM/JkCBzmvtQpl9lQ04fAbBRZwc/z2IWzF iQOvWQMbFFesAqyq13KHdyHfWSVi5ZI= Received: from mx-prod-mc-02.mail-002.prod.us-west-2.aws.redhat.com (ec2-54-186-198-63.us-west-2.compute.amazonaws.com [54.186.198.63]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-584-FOQJTR7jP66C4ieVlYGNRA-1; Mon, 21 Oct 2024 09:37:39 -0400 X-MC-Unique: FOQJTR7jP66C4ieVlYGNRA-1 Received: from mx-prod-int-04.mail-002.prod.us-west-2.aws.redhat.com (mx-prod-int-04.mail-002.prod.us-west-2.aws.redhat.com [10.30.177.40]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mx-prod-mc-02.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTPS id A38DB1954226; Mon, 21 Oct 2024 13:37:38 +0000 (UTC) Received: from nuthatch.redhat.com (unknown [10.45.225.218]) by mx-prod-int-04.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTP id 8503E1955F42; Mon, 21 Oct 2024 13:37:36 +0000 (UTC) From: Milan Zamazal To: libcamera-devel@lists.libcamera.org Cc: Milan Zamazal , robert.mader@posteo.de, kieran.bingham@ideasonboard.com, laurent.pinchart@ideasonboard.com Subject: [PATCH v5 1/3] libcamera: pipeline_handler: Provide cancelRequest Date: Mon, 21 Oct 2024 15:37:16 +0200 Message-ID: <20241021133718.894374-2-mzamazal@redhat.com> In-Reply-To: <20241021133718.894374-1-mzamazal@redhat.com> References: <20241021133718.894374-1-mzamazal@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.0 on 10.30.177.40 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com 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" Let's extract the two occurrences of canceling a request to a common helper. This is especially useful for the followup patch, which needs to cancel a request from outside. Signed-off-by: Milan Zamazal Reviewed-by: Kieran Bingham Tested-by: Kieran Bingham Reviewed-by: Hans de Goede Reviewed-by: Laurent Pinchart --- include/libcamera/internal/pipeline_handler.h | 1 + src/libcamera/pipeline_handler.cpp | 23 +++++++++++++------ 2 files changed, 17 insertions(+), 7 deletions(-) diff --git a/include/libcamera/internal/pipeline_handler.h b/include/libcamera/internal/pipeline_handler.h index 0d3808036..fb28a18d0 100644 --- a/include/libcamera/internal/pipeline_handler.h +++ b/include/libcamera/internal/pipeline_handler.h @@ -60,6 +60,7 @@ public: bool completeBuffer(Request *request, FrameBuffer *buffer); void completeRequest(Request *request); + void cancelRequest(Request *request); std::string configurationFile(const std::string &subdir, const std::string &name) const; diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp index e59404691..c9cb11f0f 100644 --- a/src/libcamera/pipeline_handler.cpp +++ b/src/libcamera/pipeline_handler.cpp @@ -367,9 +367,7 @@ void PipelineHandler::stop(Camera *camera) while (!waitingRequests_.empty()) { Request *request = waitingRequests_.front(); waitingRequests_.pop(); - - request->_d()->cancel(); - completeRequest(request); + cancelRequest(request); } /* Make sure no requests are pending. */ @@ -470,10 +468,8 @@ void PipelineHandler::doQueueRequest(Request *request) } int ret = queueRequestDevice(camera, request); - if (ret) { - request->_d()->cancel(); - completeRequest(request); - } + if (ret) + cancelRequest(request); } /** @@ -568,6 +564,19 @@ void PipelineHandler::completeRequest(Request *request) } } +/** + * \brief Cancel request and signal its completion + * \param[in] request The request to cancel + * + * This function cancels the request in addition to its completion. The same + * rules as for completeRequest() apply. + */ +void PipelineHandler::cancelRequest(Request *request) +{ + request->_d()->cancel(); + completeRequest(request); +} + /** * \brief Retrieve the absolute path to a platform configuration file * \param[in] subdir The pipeline handler specific subdirectory name From patchwork Mon Oct 21 13:37:17 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Milan Zamazal X-Patchwork-Id: 21718 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 8B9C7C3309 for ; Mon, 21 Oct 2024 13:37:50 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id E596E65399; Mon, 21 Oct 2024 15:37:49 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.b="VNoeK+D/"; dkim-atps=neutral Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 6CCD765398 for ; Mon, 21 Oct 2024 15:37:47 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1729517866; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=/KjUV+jV2J4k0c6iGkRURF6qQjdQhvNIhS+4USjZRPw=; b=VNoeK+D/LsByMuNH5NsW+A8jfBJA9cXt577JU6Wyhh/xg6MOPrpcc7BoWKK62gd5j9jgkd i5HLe5ydaHauLrW3dwvAZP4NOo5abi6nXqlny2f/bQmK498iX+WMDSlTSOkxVXb5bdC98n hOazuEVnFpQ0l6l2tXMUFQdHu1OVRiU= Received: from mx-prod-mc-01.mail-002.prod.us-west-2.aws.redhat.com (ec2-54-186-198-63.us-west-2.compute.amazonaws.com [54.186.198.63]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-427--zghdlq9P0-e4RW-PQmOcA-1; Mon, 21 Oct 2024 09:37:42 -0400 X-MC-Unique: -zghdlq9P0-e4RW-PQmOcA-1 Received: from mx-prod-int-04.mail-002.prod.us-west-2.aws.redhat.com (mx-prod-int-04.mail-002.prod.us-west-2.aws.redhat.com [10.30.177.40]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mx-prod-mc-01.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTPS id 6AE401955F43; Mon, 21 Oct 2024 13:37:41 +0000 (UTC) Received: from nuthatch.redhat.com (unknown [10.45.225.218]) by mx-prod-int-04.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTP id 4AB6819560AA; Mon, 21 Oct 2024 13:37:38 +0000 (UTC) From: Milan Zamazal To: libcamera-devel@lists.libcamera.org Cc: Milan Zamazal , robert.mader@posteo.de, kieran.bingham@ideasonboard.com, laurent.pinchart@ideasonboard.com Subject: [PATCH v5 2/3] libcamera: simple: Track requests in conversionQueue_ Date: Mon, 21 Oct 2024 15:37:17 +0200 Message-ID: <20241021133718.894374-3-mzamazal@redhat.com> In-Reply-To: <20241021133718.894374-1-mzamazal@redhat.com> References: <20241021133718.894374-1-mzamazal@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.0 on 10.30.177.40 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com 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" Simple pipeline retrieves the requests to complete from the conversionQueue_. This patch stores the requests in conversionQueue_ explicitly. This explicit tracking is supposed to be preferred to implicit retrieval and it simplifies the completion code a bit here and in the followup patch that adds request cleanup on stop. The change as implemented assumes that all the buffers in each of the conversionQueue_ elements point to the same request, the one specified. Signed-off-by: Milan Zamazal Tested-by: Kieran Bingham Reviewed-by: Kieran Bingham Reviewed-by: Hans de Goede Reviewed-by: Laurent Pinchart --- src/libcamera/pipeline/simple/simple.cpp | 26 ++++++++++++------------ 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp index 3ddce71d3..f3a6b1635 100644 --- a/src/libcamera/pipeline/simple/simple.cpp +++ b/src/libcamera/pipeline/simple/simple.cpp @@ -282,7 +282,11 @@ public: std::unique_ptr delayedCtrls_; std::vector> conversionBuffers_; - std::queue> conversionQueue_; + struct RequestOutputs { + Request *request; + std::map outputs; + }; + std::queue conversionQueue_; bool useConversion_; std::unique_ptr converter_; @@ -813,16 +817,12 @@ void SimpleCameraData::bufferReady(FrameBuffer *buffer) if (conversionQueue_.empty()) return; - Request *request = nullptr; - for (auto &item : conversionQueue_.front()) { - FrameBuffer *outputBuffer = item.second; - request = outputBuffer->request(); - pipe->completeBuffer(request, outputBuffer); - } + Request *request = conversionQueue_.front().request; + for (auto &item : conversionQueue_.front().outputs) + pipe->completeBuffer(request, item.second); + pipe->completeRequest(request); conversionQueue_.pop(); - if (request) - pipe->completeRequest(request); return; } @@ -838,7 +838,7 @@ void SimpleCameraData::bufferReady(FrameBuffer *buffer) if (useConversion_ && !conversionQueue_.empty()) { const std::map &outputs = - conversionQueue_.front(); + conversionQueue_.front().outputs; if (!outputs.empty()) { FrameBuffer *outputBuffer = outputs.begin()->second; if (outputBuffer) @@ -862,7 +862,7 @@ void SimpleCameraData::bufferReady(FrameBuffer *buffer) } if (converter_) - converter_->queueBuffers(buffer, conversionQueue_.front()); + converter_->queueBuffers(buffer, conversionQueue_.front().outputs); else /* * request->sequence() cannot be retrieved from `buffer' inside @@ -870,7 +870,7 @@ void SimpleCameraData::bufferReady(FrameBuffer *buffer) * already here. */ swIsp_->queueBuffers(request->sequence(), buffer, - conversionQueue_.front()); + conversionQueue_.front().outputs); conversionQueue_.pop(); return; @@ -1434,7 +1434,7 @@ int SimplePipelineHandler::queueRequestDevice(Camera *camera, Request *request) } if (data->useConversion_) { - data->conversionQueue_.push(std::move(buffers)); + data->conversionQueue_.push({ .request = request, .outputs = std::move(buffers) }); if (data->swIsp_) data->swIsp_->queueRequest(request->sequence(), request->controls()); } From patchwork Mon Oct 21 13:37:18 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Milan Zamazal X-Patchwork-Id: 21719 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 060D3C32A3 for ; Mon, 21 Oct 2024 13:37:53 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 77A156539A; Mon, 21 Oct 2024 15:37:53 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.b="WRKCYu+T"; dkim-atps=neutral Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id A978765395 for ; Mon, 21 Oct 2024 15:37:50 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1729517868; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=3WoTpRJAP8R/tmlv6Ebuy4dTtvfx1ooV6jThdJPrys4=; b=WRKCYu+TTJjEPLmGxJHzAPEOlukZBOjPvDfkI7jE8nSPX3E53Hjvw19MjRqAcHeRfKfV09 XxpVv1neOSETGMRYB5SLxY97XgEVK0dX546ks7ozXQei5rx4rVmDW5jnVKy5DQHd4SpUFf 4vcXZFi4ROWO0oOIWNmXeX0ZMsezK2Q= Received: from mx-prod-mc-05.mail-002.prod.us-west-2.aws.redhat.com (ec2-54-186-198-63.us-west-2.compute.amazonaws.com [54.186.198.63]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-17-uFU5hYCoPmynEhkpGN88FQ-1; Mon, 21 Oct 2024 09:37:45 -0400 X-MC-Unique: uFU5hYCoPmynEhkpGN88FQ-1 Received: from mx-prod-int-04.mail-002.prod.us-west-2.aws.redhat.com (mx-prod-int-04.mail-002.prod.us-west-2.aws.redhat.com [10.30.177.40]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mx-prod-mc-05.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTPS id 7B29E1955F56; Mon, 21 Oct 2024 13:37:44 +0000 (UTC) Received: from nuthatch.redhat.com (unknown [10.45.225.218]) by mx-prod-int-04.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTP id 056EA19560AA; Mon, 21 Oct 2024 13:37:41 +0000 (UTC) From: Milan Zamazal To: libcamera-devel@lists.libcamera.org Cc: Milan Zamazal , robert.mader@posteo.de, kieran.bingham@ideasonboard.com, laurent.pinchart@ideasonboard.com Subject: [PATCH v5 3/3] libcamera: software_isp: Clean up pending requests on stop Date: Mon, 21 Oct 2024 15:37:18 +0200 Message-ID: <20241021133718.894374-4-mzamazal@redhat.com> In-Reply-To: <20241021133718.894374-1-mzamazal@redhat.com> References: <20241021133718.894374-1-mzamazal@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.0 on 10.30.177.40 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com 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" PipelineHandler::stop() calls stopDevice() method to perform pipeline specific cleanup and then completes waiting requests. If any queued requests remain, an assertion error is raised. Software ISP stores request buffers in SimpleCameraData::conversionQueue_ and queues them as V4L2 signals bufferReady. stopDevice() cleanup forgets to clean up the buffers and their requests from conversionQueue_, possibly resulting in the assertion error. This patch fixes the omission. The problem wasn't very visible when SimplePipelineHandler::kNumInternalBuffers (the number of buffers allocated in V4L2) was equal to the number of buffers exported from software ISP. But when the number of the exported buffers was increased by one in commit abe2ec64f9e4e97bbdfe3a50372611bd7b5315c2, the assertion error started pop up in some environments. Increasing the number of the buffers much more, e.g. to 9, makes the problem very reproducible. Each pipeline uses its own mechanism to track the requests to clean up and it can't be excluded that similar omissions are present in other places. But there is no obvious way to make a common cleanup for all the pipelines (except for doing it instead of raising the assertion error, which is probably undesirable, in order not to hide incomplete pipeline specific cleanups). Bug: https://bugs.libcamera.org/show_bug.cgi?id=234 Signed-off-by: Milan Zamazal Reviewed-by: Kieran Bingham Tested-by: Kieran Bingham Reviewed-by: Laurent Pinchart --- src/libcamera/pipeline/simple/simple.cpp | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp index f3a6b1635..6b1121c34 100644 --- a/src/libcamera/pipeline/simple/simple.cpp +++ b/src/libcamera/pipeline/simple/simple.cpp @@ -288,6 +288,7 @@ public: }; std::queue conversionQueue_; bool useConversion_; + void clearIncompleteRequests(); std::unique_ptr converter_; std::unique_ptr swIsp_; @@ -897,6 +898,14 @@ void SimpleCameraData::conversionOutputDone(FrameBuffer *buffer) pipe->completeRequest(request); } +void SimpleCameraData::clearIncompleteRequests() +{ + while (!conversionQueue_.empty()) { + pipe()->cancelRequest(conversionQueue_.front().request); + conversionQueue_.pop(); + } +} + void SimpleCameraData::ispStatsReady(uint32_t frame, uint32_t bufferId) { swIsp_->processStats(frame, bufferId, @@ -1406,6 +1415,7 @@ void SimplePipelineHandler::stopDevice(Camera *camera) video->bufferReady.disconnect(data, &SimpleCameraData::bufferReady); + data->clearIncompleteRequests(); data->conversionBuffers_.clear(); releasePipeline(data);