{"id":21537,"url":"https://patchwork.libcamera.org/api/patches/21537/?format=json","web_url":"https://patchwork.libcamera.org/patch/21537/","project":{"id":1,"url":"https://patchwork.libcamera.org/api/projects/1/?format=json","name":"libcamera","link_name":"libcamera","list_id":"libcamera_core","list_email":"libcamera-devel@lists.libcamera.org","web_url":"","scm_url":"","webscm_url":""},"msgid":"<20241008150916.1041703-1-mzamazal@redhat.com>","date":"2024-10-08T15:09:16","name":"libcamera: software_isp: Clean up pending requests on stop","commit_ref":null,"pull_url":null,"state":"superseded","archived":false,"hash":"45bcf775d09189e40e96af98c95b61924fc1ade6","submitter":{"id":177,"url":"https://patchwork.libcamera.org/api/people/177/?format=json","name":"Milan Zamazal","email":"mzamazal@redhat.com"},"delegate":null,"mbox":"https://patchwork.libcamera.org/patch/21537/mbox/","series":[{"id":4666,"url":"https://patchwork.libcamera.org/api/series/4666/?format=json","web_url":"https://patchwork.libcamera.org/project/libcamera/list/?series=4666","date":"2024-10-08T15:09:16","name":"libcamera: software_isp: Clean up pending requests on stop","version":1,"mbox":"https://patchwork.libcamera.org/series/4666/mbox/"}],"comments":"https://patchwork.libcamera.org/api/patches/21537/comments/","check":"pending","checks":"https://patchwork.libcamera.org/api/patches/21537/checks/","tags":{},"headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id DCD73BD80A\n\tfor ;\n\tTue, 8 Oct 2024 15:09:56 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 1C90B6352E;\n\tTue, 8 Oct 2024 17:09:56 +0200 (CEST)","from us-smtp-delivery-124.mimecast.com\n\t(us-smtp-delivery-124.mimecast.com [170.10.133.124])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 867BA618C9\n\tfor ;\n\tTue, 8 Oct 2024 17:09:53 +0200 (CEST)","from mx-prod-mc-04.mail-002.prod.us-west-2.aws.redhat.com\n\t(ec2-54-186-198-63.us-west-2.compute.amazonaws.com [54.186.198.63])\n\tby relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3,\n\tcipher=TLS_AES_256_GCM_SHA384) id us-mta-2-05_gFAnoNX62Cbi6blTT1g-1;\n\tTue, 08 Oct 2024 11:09:50 -0400","from mx-prod-int-01.mail-002.prod.us-west-2.aws.redhat.com\n\t(mx-prod-int-01.mail-002.prod.us-west-2.aws.redhat.com [10.30.177.4])\n\t(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)\n\tkey-exchange X25519 server-signature RSA-PSS (2048 bits)\n\tserver-digest SHA256) (No client certificate requested)\n\tby mx-prod-mc-04.mail-002.prod.us-west-2.aws.redhat.com (Postfix)\n\twith ESMTPS id 719E51955D62; Tue, 8 Oct 2024 15:09:48 +0000 (UTC)","from nuthatch.redhat.com (unknown [10.45.224.67])\n\tby mx-prod-int-01.mail-002.prod.us-west-2.aws.redhat.com (Postfix)\n\twith ESMTP id 972D9300019E; Tue, 8 Oct 2024 15:09:44 +0000 (UTC)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=redhat.com header.i=@redhat.com\n\theader.b=\"N4XvQ+2O\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1728400192;\n\th=from:from:reply-to:subject:subject:date:date:message-id:message-id:\n\tto:to:cc:cc:mime-version:mime-version:content-type:content-type:\n\tcontent-transfer-encoding:content-transfer-encoding;\n\tbh=pl2ByBp+CwQcGvNucSw7RJ3XsRGc7zOmjZu2W8bnX3M=;\n\tb=N4XvQ+2O3RA2kkLn+mquhBLgZzIqlYBUe8AVNRKWWsZIEDMU7HAABcuzlFk6pI9uBqWrpI\n\tYlbjAj1GmX863qFIJ9ojZD39GaNC2W6S01lqTLHbJBHX72/9S4ukPfyZhrhDTfVZrg4tUZ\n\tgmM2PlNc5rPErIjmKo4uAG+7uWoZVvE=","X-MC-Unique":"05_gFAnoNX62Cbi6blTT1g-1","From":"Milan Zamazal ","To":"libcamera-devel@lists.libcamera.org","Cc":"Milan Zamazal , robert.mader@posteo.de,\n\tkieran.bingham@ideasonboard.com","Subject":"[PATCH] libcamera: software_isp: Clean up pending requests on stop","Date":"Tue, 8 Oct 2024 17:09:16 +0200","Message-ID":"<20241008150916.1041703-1-mzamazal@redhat.com>","MIME-Version":"1.0","X-Scanned-By":"MIMEDefang 3.4.1 on 10.30.177.4","X-Mimecast-Spam-Score":"0","X-Mimecast-Originator":"redhat.com","Content-Transfer-Encoding":"8bit","Content-Type":"text/plain; charset=\"US-ASCII\"; x-default=true","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "},"content":"PipelineHandler::stop() calls stopDevice() method to perform pipeline\nspecific cleanup and then completes waiting requests. If any queued\nrequests remain, an assertion error is raised.\n\nSoftware ISP stores request buffers in\nSimpleCameraData::conversionQueue_ and queues them as V4L2 signals\nbufferReady. stopDevice() cleanup forgets to clean up the buffers and\ntheir requests from conversionQueue_, possibly resulting in the\nassertion error. This patch fixes the omission.\n\nThe problem wasn't very visible when\nSimplePipelineHandler::kNumInternalBuffers (the number of buffers\nallocated in V4L2) was equal to the number of buffers exported from\nsoftware ISP. But when the number of the exported buffers was increased\nby one in commit abe2ec64f9e4e97bbdfe3a50372611bd7b5315c2, the assertion\nerror started pop up in some environments. Increasing the number of the\nbuffers much more, e.g. to 9, makes the problem very reproducible.\n\nEach pipeline uses its own mechanism to track the requests to clean up\nand it can't be excluded that similar omissions are present in other\nplaces. But there is no obvious way to make a common cleanup for all\nthe pipelines (except for doing it instead of raising the assertion\nerror, which is probably undesirable, in order not to hide incomplete\npipeline specific cleanups).\n\nBug: https://bugs.libcamera.org/show_bug.cgi?id=234\nSigned-off-by: Milan Zamazal \n---\n src/libcamera/pipeline/simple/simple.cpp | 15 +++++++++++++++\n 1 file changed, 15 insertions(+)","diff":"diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\nindex 3ddce71d3..4e8504922 100644\n--- a/src/libcamera/pipeline/simple/simple.cpp\n+++ b/src/libcamera/pipeline/simple/simple.cpp\n@@ -284,6 +284,7 @@ public:\n \tstd::vector> conversionBuffers_;\n \tstd::queue> conversionQueue_;\n \tbool useConversion_;\n+\tvoid clearIncompleteRequests();\n \n \tstd::unique_ptr converter_;\n \tstd::unique_ptr swIsp_;\n@@ -897,6 +898,19 @@ void SimpleCameraData::conversionOutputDone(FrameBuffer *buffer)\n \t\tpipe->completeRequest(request);\n }\n \n+void SimpleCameraData::clearIncompleteRequests()\n+{\n+\twhile (!conversionQueue_.empty()) {\n+\t\tfor (auto &item : conversionQueue_.front()) {\n+\t\t\tFrameBuffer *outputBuffer = item.second;\n+\t\t\tRequest *request = outputBuffer->request();\n+\t\t\tpipe()->completeBuffer(request, outputBuffer);\n+\t\t\tpipe()->completeRequest(request);\n+\t\t}\n+\t\tconversionQueue_.pop();\n+\t}\n+}\n+\n void SimpleCameraData::ispStatsReady(uint32_t frame, uint32_t bufferId)\n {\n \tswIsp_->processStats(frame, bufferId,\n@@ -1407,6 +1421,7 @@ void SimplePipelineHandler::stopDevice(Camera *camera)\n \tvideo->bufferReady.disconnect(data, &SimpleCameraData::bufferReady);\n \n \tdata->conversionBuffers_.clear();\n+\tdata->clearIncompleteRequests();\n \n \treleasePipeline(data);\n }\n","prefixes":[]}