From patchwork Wed Oct 9 17:21:08 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Milan Zamazal X-Patchwork-Id: 21565 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 08B60C32DE for ; Wed, 9 Oct 2024 17:21:29 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id BAB826536C; Wed, 9 Oct 2024 19:21:28 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.b="E9cW5//a"; 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 AE22A63527 for ; Wed, 9 Oct 2024 19:21:23 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1728494482; 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=812ruoLKpxT3+QEGPORyZO2YnCfLt+3fxr0fgL0W9qg=; b=E9cW5//a2wUufvcU65l/UBMAo1zu/DnGSHWDjeVNbkkH9yeuZhLa3JVTHwTShibMitUm+a HB5GwS9wfbX7ujQZnhU8eNK/JPuZcbK3+B1wGsBd6Goc4MqSSBrpFF4PHRARRfccKGF2c5 PM65wi1M0WGScCZCRUKSLt9icqcm7VQ= 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-575--e8JjZO3PcqovBkS94_L8w-1; Wed, 09 Oct 2024 13:21:19 -0400 X-MC-Unique: -e8JjZO3PcqovBkS94_L8w-1 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-01.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTPS id 8E07719560AF; Wed, 9 Oct 2024 17:21:18 +0000 (UTC) Received: from nuthatch.redhat.com (unknown [10.45.224.78]) by mx-prod-int-02.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTP id CA6A5195607C; Wed, 9 Oct 2024 17:21:16 +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 v3 2/2] libcamera: software_isp: Clean up pending requests on stop Date: Wed, 9 Oct 2024 19:21:08 +0200 Message-ID: <20241009172108.2058320-3-mzamazal@redhat.com> In-Reply-To: <20241009172108.2058320-1-mzamazal@redhat.com> References: <20241009172108.2058320-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-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 --- src/libcamera/pipeline/simple/simple.cpp | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp index 3ddce71d3..ff3859f18 100644 --- a/src/libcamera/pipeline/simple/simple.cpp +++ b/src/libcamera/pipeline/simple/simple.cpp @@ -284,6 +284,7 @@ public: std::vector> conversionBuffers_; std::queue> conversionQueue_; bool useConversion_; + void clearIncompleteRequests(); std::unique_ptr converter_; std::unique_ptr swIsp_; @@ -897,6 +898,19 @@ void SimpleCameraData::conversionOutputDone(FrameBuffer *buffer) pipe->completeRequest(request); } +void SimpleCameraData::clearIncompleteRequests() +{ + while (!conversionQueue_.empty()) { + for (auto &item : conversionQueue_.front()) { + FrameBuffer *outputBuffer = item.second; + Request *request = outputBuffer->request(); + if (request->status() == Request::RequestPending) + pipe()->cancelRequest(request); + } + conversionQueue_.pop(); + } +} + void SimpleCameraData::ispStatsReady(uint32_t frame, uint32_t bufferId) { swIsp_->processStats(frame, bufferId, @@ -1406,6 +1420,7 @@ void SimplePipelineHandler::stopDevice(Camera *camera) video->bufferReady.disconnect(data, &SimpleCameraData::bufferReady); + data->clearIncompleteRequests(); data->conversionBuffers_.clear(); releasePipeline(data);