From patchwork Wed Nov 6 20:17:19 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Milan Zamazal X-Patchwork-Id: 21823 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 19B8ABDB13 for ; Wed, 6 Nov 2024 20:17:39 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id B55A66545A; Wed, 6 Nov 2024 21:17:38 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.b="affYuEiv"; 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 0A84C65446 for ; Wed, 6 Nov 2024 21:17:35 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1730924255; 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=/PveyiXvNvVrexH9JTJH0bWLYLUXWwRuLPQ6SwEwjFM=; b=affYuEivLVl5dUdQHxEpXyYzh/DHFSImbV9eBLphEu4wcTOMLmiW4QVecoXbqSdU8Uu6zr e/S9QSVQUywFF23Glsu+pHRiW6zoqJOpQqOXR4vDWm3Kh6mk++yyU3qy5Vvb+5t7cowzcl eaJ5FHhIwbwSfCNuyAV2mw1gtHd6C6w= Received: from mx-prod-mc-04.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-681-w5kOqdlYN3GjOHRv80C_9Q-1; Wed, 06 Nov 2024 15:17:33 -0500 X-MC-Unique: w5kOqdlYN3GjOHRv80C_9Q-1 X-Mimecast-MFC-AGG-ID: w5kOqdlYN3GjOHRv80C_9Q Received: from mx-prod-int-02.mail-002.prod.us-west-2.aws.redhat.com (mx-prod-int-02.mail-002.prod.us-west-2.aws.redhat.com [10.30.177.15]) (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-04.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTPS id 9493419560A3; Wed, 6 Nov 2024 20:17:32 +0000 (UTC) Received: from nuthatch.redhat.com (unknown [10.45.224.4]) by mx-prod-int-02.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTP id 1E2CD1955F42; Wed, 6 Nov 2024 20:17:29 +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, hdegoede@redhat.com Subject: [PATCH v6 1/3] libcamera: pipeline_handler: Provide cancelRequest Date: Wed, 6 Nov 2024 21:17:19 +0100 Message-ID: <20241106201721.1624461-2-mzamazal@redhat.com> In-Reply-To: <20241106201721.1624461-1-mzamazal@redhat.com> References: <20241106201721.1624461-1-mzamazal@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.0 on 10.30.177.15 X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: TBzYGZ2PWPf_fEW_5gdmVauiWHeaQC7uwxQrw5OhJAI_1730924253 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: Hans de Goede Reviewed-by: Laurent Pinchart Tested-by: Stanislaw Gruszka --- 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..4905863c4 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 and completes the request. 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 Wed Nov 6 20:17:20 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Milan Zamazal X-Patchwork-Id: 21824 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 B5F43BDB13 for ; Wed, 6 Nov 2024 20:17:42 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 63DA365461; Wed, 6 Nov 2024 21:17:42 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.b="c4bAgc9X"; 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 DFB3065456 for ; Wed, 6 Nov 2024 21:17:40 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1730924259; 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=gToRVLvkzVjHKyRR7W5wHInMORe1zLI30hDb4yfyaFk=; b=c4bAgc9X66koI0cngCt3/xUsxUOewOX7E1V8sPCOTGw2QSKkf0eNd2MAgz1abeLD0pQmN3 fWhcgaPhMw38gH8Xsnjvx4UvghbEjQFKM6t/PO5Fq3Q2sbBBZKK4zUWBOllQ43lG/NYuGS PvpE3y5wRnVcu9TkkKh5wfemE6c+wdo= 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-489-isQeNduvPbKLUEeiQhYAnA-1; Wed, 06 Nov 2024 15:17:36 -0500 X-MC-Unique: isQeNduvPbKLUEeiQhYAnA-1 X-Mimecast-MFC-AGG-ID: isQeNduvPbKLUEeiQhYAnA Received: from mx-prod-int-02.mail-002.prod.us-west-2.aws.redhat.com (mx-prod-int-02.mail-002.prod.us-west-2.aws.redhat.com [10.30.177.15]) (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 7505D1955EE9; Wed, 6 Nov 2024 20:17:35 +0000 (UTC) Received: from nuthatch.redhat.com (unknown [10.45.224.4]) by mx-prod-int-02.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTP id 16F081956088; Wed, 6 Nov 2024 20:17:32 +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, hdegoede@redhat.com Subject: [PATCH v6 2/3] libcamera: simple: Track requests in conversionQueue_ Date: Wed, 6 Nov 2024 21:17:20 +0100 Message-ID: <20241106201721.1624461-3-mzamazal@redhat.com> In-Reply-To: <20241106201721.1624461-1-mzamazal@redhat.com> References: <20241106201721.1624461-1-mzamazal@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.0 on 10.30.177.15 X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: E92kG_6oBJuoW3E42RsuiDiS8mqSECnsP514dfWjx60_1730924255 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 Tested-by: Stanislaw Gruszka --- 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 67f583b8a..13c0a1891 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_; @@ -808,16 +812,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); - } + const RequestOutputs &outputs = conversionQueue_.front(); + for (auto &[stream, buf] : outputs.outputs) + pipe->completeBuffer(outputs.request, buf); + pipe->completeRequest(outputs.request); conversionQueue_.pop(); - if (request) - pipe->completeRequest(request); return; } @@ -833,7 +833,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) @@ -857,7 +857,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 @@ -865,7 +865,7 @@ void SimpleCameraData::bufferReady(FrameBuffer *buffer) * already here. */ swIsp_->queueBuffers(request->sequence(), buffer, - conversionQueue_.front()); + conversionQueue_.front().outputs); conversionQueue_.pop(); return; @@ -1429,7 +1429,7 @@ int SimplePipelineHandler::queueRequestDevice(Camera *camera, Request *request) } if (data->useConversion_) { - data->conversionQueue_.push(std::move(buffers)); + data->conversionQueue_.push({ request, std::move(buffers) }); if (data->swIsp_) data->swIsp_->queueRequest(request->sequence(), request->controls()); } From patchwork Wed Nov 6 20:17:21 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Milan Zamazal X-Patchwork-Id: 21825 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 8AF6DBDB13 for ; Wed, 6 Nov 2024 20:17:47 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 38AB165467; Wed, 6 Nov 2024 21:17:47 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.b="guDCJqyU"; 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 34B836545A for ; Wed, 6 Nov 2024 21:17:46 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1730924265; 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=dw1rSXsZj0CiLBB+4rarMmUmuy8CjjFtsacMneVOvUo=; b=guDCJqyUg7RzG2vueHRawsS5GKpc5ZV2MYtLaJktKLI+YkHcE0Bo5o2L++lewxqskiMxlc TLGSVJW6YhQEsquJQ0q08ev6nTz+l8zK8HtgYSSeoCPOq4PryKFEf8mlJfVpX9RI8qMk8V RZA6FiVfF4PXpIfoDseQogGMxa1MZqk= Received: from mx-prod-mc-03.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-445-aDKZP5YuM0W5-GWF_CW4pg-1; Wed, 06 Nov 2024 15:17:40 -0500 X-MC-Unique: aDKZP5YuM0W5-GWF_CW4pg-1 X-Mimecast-MFC-AGG-ID: aDKZP5YuM0W5-GWF_CW4pg Received: from mx-prod-int-02.mail-002.prod.us-west-2.aws.redhat.com (mx-prod-int-02.mail-002.prod.us-west-2.aws.redhat.com [10.30.177.15]) (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-03.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTPS id 0DE5019560BD; Wed, 6 Nov 2024 20:17:39 +0000 (UTC) Received: from nuthatch.redhat.com (unknown [10.45.224.4]) by mx-prod-int-02.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTP id 0FAED1956088; Wed, 6 Nov 2024 20:17:35 +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, hdegoede@redhat.com, Stanislaw Gruszka Subject: [PATCH v6 3/3] libcamera: software_isp: Clean up pending requests on stop Date: Wed, 6 Nov 2024 21:17:21 +0100 Message-ID: <20241106201721.1624461-4-mzamazal@redhat.com> In-Reply-To: <20241106201721.1624461-1-mzamazal@redhat.com> References: <20241106201721.1624461-1-mzamazal@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.0 on 10.30.177.15 X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: b006d7PVPI579MdHh1AVMhxbusOLwQIU2yKBAVIXBNA_1730924259 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 Reviewed-by: Hans de Goede Reviewed-by: Laurent Pinchart Tested-by: Kieran Bingham Tested-by: Stanislaw Gruszka --- 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 13c0a1891..b425bc0de 100644 --- a/src/libcamera/pipeline/simple/simple.cpp +++ b/src/libcamera/pipeline/simple/simple.cpp @@ -226,6 +226,7 @@ public: V4L2Subdevice::Whence whence, Transform transform = Transform::Identity); void bufferReady(FrameBuffer *buffer); + void clearIncompleteRequests(); unsigned int streamIndex(const Stream *stream) const { @@ -876,6 +877,14 @@ void SimpleCameraData::bufferReady(FrameBuffer *buffer) pipe->completeRequest(request); } +void SimpleCameraData::clearIncompleteRequests() +{ + while (!conversionQueue_.empty()) { + pipe()->cancelRequest(conversionQueue_.front().request); + conversionQueue_.pop(); + } +} + void SimpleCameraData::conversionInputDone(FrameBuffer *buffer) { /* Queue the input buffer back for capture. */ @@ -1401,6 +1410,7 @@ void SimplePipelineHandler::stopDevice(Camera *camera) video->bufferReady.disconnect(data, &SimpleCameraData::bufferReady); + data->clearIncompleteRequests(); data->conversionBuffers_.clear(); releasePipeline(data);