[{"id":33457,"web_url":"https://patchwork.libcamera.org/comment/33457/","msgid":"<20250224195209.GI6778@pendragon.ideasonboard.com>","date":"2025-02-24T19:52:09","subject":"Re: [PATCH v2 3/5] libcamera: software_isp: Handle queued input\n\tbuffers on stop","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Milan,\n\nThank you for the patch.\n\nOn Mon, Feb 24, 2025 at 07:52:33PM +0100, Milan Zamazal wrote:\n> When SoftwareIsp stops, input and output buffers queued to it may not\n> yet be fully processed.  They will be eventually returned but stop means\n> stop, there should be no processing related actions invoked afterwards.\n> \n> Let's stop forwarding processed input buffers from SoftwareIsp slots\n> when SoftwareIsp is stopped.  Let's track the queued input buffers and\n> return them back for capture in SoftwareIsp::stop().\n> \n> Signed-off-by: Milan Zamazal <mzamazal@redhat.com>\n> ---\n>  .../internal/software_isp/software_isp.h         |  1 +\n>  src/libcamera/software_isp/software_isp.cpp      | 16 +++++++++++++++-\n>  2 files changed, 16 insertions(+), 1 deletion(-)\n> \n> diff --git a/include/libcamera/internal/software_isp/software_isp.h b/include/libcamera/internal/software_isp/software_isp.h\n> index f2344355..bfe34725 100644\n> --- a/include/libcamera/internal/software_isp/software_isp.h\n> +++ b/include/libcamera/internal/software_isp/software_isp.h\n> @@ -102,6 +102,7 @@ private:\n>  \tstd::unique_ptr<ipa::soft::IPAProxySoft> ipa_;\n>  \tbool running_;\n>  \tstd::deque<FrameBuffer *> queuedOutputBuffers_;\n> +\tstd::deque<FrameBuffer *> queuedInputBuffers_;\n\nI'd move input before output.\n\n>  };\n>  \n>  } /* namespace libcamera */\n> diff --git a/src/libcamera/software_isp/software_isp.cpp b/src/libcamera/software_isp/software_isp.cpp\n> index 4339e547..3a605ab2 100644\n> --- a/src/libcamera/software_isp/software_isp.cpp\n> +++ b/src/libcamera/software_isp/software_isp.cpp\n> @@ -301,6 +301,8 @@ int SoftwareIsp::queueBuffers(uint32_t frame, FrameBuffer *input,\n>  \t\t\treturn -EINVAL;\n>  \t}\n>  \n> +\tqueuedInputBuffers_.push_back(input);\n> +\n>  \tfor (auto iter = outputs.begin(); iter != outputs.end(); iter++) {\n>  \t\tFrameBuffer *const buffer = iter->second;\n>  \t\tqueuedOutputBuffers_.push_back(buffer);\n> @@ -327,6 +329,9 @@ int SoftwareIsp::start()\n>  \n>  /**\n>   * \\brief Stops the Software ISP streaming operation\n> + *\n> + * All pending buffers are returned back (output buffers as canceled) before\n> + * this method finishes.\n\nShouldn't the input buffer be cancelled too ?\n\n>   */\n>  void SoftwareIsp::stop()\n>  {\n> @@ -342,6 +347,10 @@ void SoftwareIsp::stop()\n>  \t\toutputBufferReady.emit(buffer);\n>  \t}\n>  \tqueuedOutputBuffers_.clear();\n> +\n> +\tfor (auto buffer : queuedInputBuffers_)\n> +\t\tinputBufferReady.emit(buffer);\n> +\tqueuedInputBuffers_.clear();\n>  }\n>  \n>  /**\n> @@ -375,7 +384,12 @@ void SoftwareIsp::statsReady(uint32_t frame, uint32_t bufferId)\n>  \n>  void SoftwareIsp::inputReady(FrameBuffer *input)\n>  {\n> -\tinputBufferReady.emit(input);\n> +\tif (running_) {\n> +\t\tqueuedInputBuffers_.erase(find(queuedInputBuffers_.begin(),\n> +\t\t\t\t\t       queuedInputBuffers_.end(),\n> +\t\t\t\t\t       input));\n\nSame comment as for 2/5.\n\n> +\t\tinputBufferReady.emit(input);\n> +\t}\n>  }\n>  \n>  void SoftwareIsp::outputReady(FrameBuffer *output)","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 4B7A2C32A9\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 24 Feb 2025 19:52:30 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 9E62068701;\n\tMon, 24 Feb 2025 20:52:29 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 91A6E686F6\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 24 Feb 2025 20:52:27 +0100 (CET)","from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi\n\t[81.175.209.231])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id A0B81220;\n\tMon, 24 Feb 2025 20:51:00 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"PhR8SwhQ\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1740426660;\n\tbh=Z5kzjk7xCKThZC6RQY1VCFU/7fmbTus5Hu46hzPl3C8=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=PhR8SwhQVz3CfytCvXt3E49IIa8rrPnfpQkIZV2bjmbMhT86PAdzMCU6kJ8PR1dO4\n\tvKREVZgG1jOXI1KUKYf0RK+57uS2lne4yxzPuwN0qfWwv1s/P32Yh/tf5uRmw5CiNU\n\tyVsON3/vdRrk6EQKC4TtBdJAGYFm0FLHJsD+mBlY=","Date":"Mon, 24 Feb 2025 21:52:09 +0200","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Milan Zamazal <mzamazal@redhat.com>","Cc":"libcamera-devel@lists.libcamera.org,\n\tStanislaw Gruszka <stanislaw.gruszka@linux.intel.com>,\n\tKieran Bingham <kieran.bingham@ideasonboard.com>","Subject":"Re: [PATCH v2 3/5] libcamera: software_isp: Handle queued input\n\tbuffers on stop","Message-ID":"<20250224195209.GI6778@pendragon.ideasonboard.com>","References":"<20250224185235.43381-1-mzamazal@redhat.com>\n\t<20250224185235.43381-4-mzamazal@redhat.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20250224185235.43381-4-mzamazal@redhat.com>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":33459,"web_url":"https://patchwork.libcamera.org/comment/33459/","msgid":"<85ecznm6xi.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","date":"2025-02-24T20:19:53","subject":"Re: [PATCH v2 3/5] libcamera: software_isp: Handle queued input\n\tbuffers on stop","submitter":{"id":177,"url":"https://patchwork.libcamera.org/api/people/177/","name":"Milan Zamazal","email":"mzamazal@redhat.com"},"content":"Hi Laurent,\n\nthank you for review.\n\nLaurent Pinchart <laurent.pinchart@ideasonboard.com> writes:\n\n> Hi Milan,\n>\n> Thank you for the patch.\n>\n> On Mon, Feb 24, 2025 at 07:52:33PM +0100, Milan Zamazal wrote:\n>> When SoftwareIsp stops, input and output buffers queued to it may not\n>> yet be fully processed.  They will be eventually returned but stop means\n>> stop, there should be no processing related actions invoked afterwards.\n>> \n>> Let's stop forwarding processed input buffers from SoftwareIsp slots\n>> when SoftwareIsp is stopped.  Let's track the queued input buffers and\n>> return them back for capture in SoftwareIsp::stop().\n>> \n>> Signed-off-by: Milan Zamazal <mzamazal@redhat.com>\n>> ---\n>>  .../internal/software_isp/software_isp.h         |  1 +\n>>  src/libcamera/software_isp/software_isp.cpp      | 16 +++++++++++++++-\n>>  2 files changed, 16 insertions(+), 1 deletion(-)\n>> \n>> diff --git a/include/libcamera/internal/software_isp/software_isp.h b/include/libcamera/internal/software_isp/software_isp.h\n>> index f2344355..bfe34725 100644\n>> --- a/include/libcamera/internal/software_isp/software_isp.h\n>> +++ b/include/libcamera/internal/software_isp/software_isp.h\n>> @@ -102,6 +102,7 @@ private:\n>>  \tstd::unique_ptr<ipa::soft::IPAProxySoft> ipa_;\n>>  \tbool running_;\n>>  \tstd::deque<FrameBuffer *> queuedOutputBuffers_;\n>> +\tstd::deque<FrameBuffer *> queuedInputBuffers_;\n>\n> I'd move input before output.\n>\n>>  };\n>>  \n>>  } /* namespace libcamera */\n>> diff --git a/src/libcamera/software_isp/software_isp.cpp b/src/libcamera/software_isp/software_isp.cpp\n>> index 4339e547..3a605ab2 100644\n>> --- a/src/libcamera/software_isp/software_isp.cpp\n>> +++ b/src/libcamera/software_isp/software_isp.cpp\n>> @@ -301,6 +301,8 @@ int SoftwareIsp::queueBuffers(uint32_t frame, FrameBuffer *input,\n>>  \t\t\treturn -EINVAL;\n>>  \t}\n>>  \n>> +\tqueuedInputBuffers_.push_back(input);\n>> +\n>>  \tfor (auto iter = outputs.begin(); iter != outputs.end(); iter++) {\n>>  \t\tFrameBuffer *const buffer = iter->second;\n>>  \t\tqueuedOutputBuffers_.push_back(buffer);\n>> @@ -327,6 +329,9 @@ int SoftwareIsp::start()\n>>  \n>>  /**\n>>   * \\brief Stops the Software ISP streaming operation\n>> + *\n>> + * All pending buffers are returned back (output buffers as canceled) before\n>> + * this method finishes.\n>\n> Shouldn't the input buffer be cancelled too ?\n\nIs it necessary to cancel input buffers?  Aren't they just returned for\nre-use regardless of their contents (unlike the output buffers where the\nvalidity of the contents matters)?  The input buffer lands in\nV4L2VideoDevice::queueBuffer() and I can't see any use of the status\nthere.\n\n>>   */\n>>  void SoftwareIsp::stop()\n>>  {\n>> @@ -342,6 +347,10 @@ void SoftwareIsp::stop()\n>>  \t\toutputBufferReady.emit(buffer);\n>>  \t}\n>>  \tqueuedOutputBuffers_.clear();\n>> +\n>> +\tfor (auto buffer : queuedInputBuffers_)\n>> +\t\tinputBufferReady.emit(buffer);\n>> +\tqueuedInputBuffers_.clear();\n>>  }\n>>  \n>>  /**\n>> @@ -375,7 +384,12 @@ void SoftwareIsp::statsReady(uint32_t frame, uint32_t bufferId)\n>>  \n>>  void SoftwareIsp::inputReady(FrameBuffer *input)\n>>  {\n>> -\tinputBufferReady.emit(input);\n>> +\tif (running_) {\n>> +\t\tqueuedInputBuffers_.erase(find(queuedInputBuffers_.begin(),\n>> +\t\t\t\t\t       queuedInputBuffers_.end(),\n>> +\t\t\t\t\t       input));\n>\n> Same comment as for 2/5.\n>\n>> +\t\tinputBufferReady.emit(input);\n>> +\t}\n>>  }\n>>  \n>>  void SoftwareIsp::outputReady(FrameBuffer *output)","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 84762C32A9\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 24 Feb 2025 20:20:03 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 832B4686FF;\n\tMon, 24 Feb 2025 21:20:02 +0100 (CET)","from us-smtp-delivery-124.mimecast.com\n\t(us-smtp-delivery-124.mimecast.com [170.10.129.124])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id A411361856\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 24 Feb 2025 21:20:00 +0100 (CET)","from mail-wm1-f72.google.com (mail-wm1-f72.google.com\n\t[209.85.128.72]) by relay.mimecast.com with ESMTP with STARTTLS\n\t(version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id\n\tus-mta-397-Aofb7EoxMm6T0pzsywVq9A-1; Mon, 24 Feb 2025 15:19:57 -0500","by mail-wm1-f72.google.com with SMTP id\n\t5b1f17b1804b1-438e4e9a53fso40219415e9.1\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 24 Feb 2025 12:19:57 -0800 (PST)","from mzamazal-thinkpadp1gen7.tpbc.csb\n\t(ip-77-48-47-2.net.vodafone.cz. [77.48.47.2])\n\tby smtp.gmail.com with ESMTPSA id\n\t5b1f17b1804b1-43ab1532f20sm2046915e9.8.2025.02.24.12.19.54\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tMon, 24 Feb 2025 12:19:54 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=redhat.com header.i=@redhat.com\n\theader.b=\"QMKrhRgw\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1740428399;\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\tin-reply-to:in-reply-to:references:references;\n\tbh=/wqo7+1fYkOsu1AzcPukz3wseYw7/V/N2Vg9wF5bauc=;\n\tb=QMKrhRgwTBkflha0MzKs2e693Qe363gvtiiwUAyOdIGHh1cJmDM0YdYydhjgWuwIqRfxIj\n\tSrY7vDpsC3hI2zwbCyURVLQSPpO+nbGMaDsj623Mmly7miHLHBDx4ed8vIgsuywmyIIfMm\n\tlSuUYjU5cBZFM/McQuxZo+EctpmcpFk=","X-MC-Unique":"Aofb7EoxMm6T0pzsywVq9A-1","X-Mimecast-MFC-AGG-ID":"Aofb7EoxMm6T0pzsywVq9A_1740428397","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1740428396; x=1741033196;\n\th=mime-version:user-agent:message-id:date:references:in-reply-to\n\t:subject:cc:to:from:x-gm-message-state:from:to:cc:subject:date\n\t:message-id:reply-to;\n\tbh=/wqo7+1fYkOsu1AzcPukz3wseYw7/V/N2Vg9wF5bauc=;\n\tb=K1wLMrjpkVog7h+RU7H+I3tNI5Kifqot7b+iI6pZd8e+2N4O+HB7JqiTzJr2KMs/zh\n\tjRwNYHtKFOb9mhhvoUPIYElZgTUciMVtCXuchQOz0NgD86R3zNE6Y/6eY9gFFTmqoC3p\n\tOxdg4iaW+EBVbQGeRxiS8XznlZ+zYUFuvUHW44FrPoOKPXSaPguqC08jd2b7cI4nD814\n\t1DSrJbdBIGS1JgIvQp/kmdYeNGddoNU13bRd5me/nT6qq7P3OdqcxTqN/sI6wu7gsNHB\n\tR/ytjZu1ckjnCR0s9mkPIypqJIF058sDMe0qGeW+TV4ifLF7fdbbLqjgJsPFtoBkjZc3\n\t9XsQ==","X-Gm-Message-State":"AOJu0YzNNfqGeHFu9A4zVVGXMsFc/yWVJWvgPP6fhDJW7SkQbR46GpJm\n\tdFWUks9W23E7L2yQW1wJ0sz+Wa02zz/sDanSrdVMGg9GutHmdV7BN5e1Ahf52X0ru3ukDnIGirP\n\tDDl0IVNkoZrYAfhRqsFmImfJhQrL1co9FMN7HtRciuFfwrlUCeEiHFUPWIew6ejLPp7oG2Qo=","X-Gm-Gg":"ASbGnctZzpI3oX2SBThK1frV9Lz5sj5lE8dgthUDNKqvzrIR/R7iB0yfIV//KJ1/iRe\n\tN+XbEcPbC+LairzOR0FyA2VJGJK1CFcgAUK4LlZC2H2keOMA2hzanKdIgk4qpZMqDgvPY6YHWNI\n\tSvCs87gJb0ofoO2O9sSPBc88geUoqRUBh3n+VSDtMvwhAubmcDXlpAyOGbGt0co+fYLQ6VgZXPy\n\txJzyH6MCaqOLLvsTdFyMsWsxi1emV39bHerB3cI3+/0pIPVgoHGOijv1S2aPnvWQlWTVbKRwpcB\n\tP4aJJlZjUQg58Usf26pDxKLIskreozKnxIOgXKnw/sd1LyBNoYm0ZB3edUyRxt92y9fe","X-Received":["by 2002:a05:600c:1e15:b0:439:9cbc:7753 with SMTP id\n\t5b1f17b1804b1-439b03246d2mr123415145e9.10.1740428396605; \n\tMon, 24 Feb 2025 12:19:56 -0800 (PST)","by 2002:a05:600c:1e15:b0:439:9cbc:7753 with SMTP id\n\t5b1f17b1804b1-439b03246d2mr123414975e9.10.1740428396205; \n\tMon, 24 Feb 2025 12:19:56 -0800 (PST)"],"X-Google-Smtp-Source":"AGHT+IE0tt9Ph7RyuDctDpdIn05ATe2QzRgJzH0RtAT8rKWN4boqUrRXS56FMH+QHbeZAisGSJs4aA==","From":"Milan Zamazal <mzamazal@redhat.com>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org,  Stanislaw Gruszka\n\t<stanislaw.gruszka@linux.intel.com>,  Kieran Bingham\n\t<kieran.bingham@ideasonboard.com>","Subject":"Re: [PATCH v2 3/5] libcamera: software_isp: Handle queued input\n\tbuffers on stop","In-Reply-To":"<20250224195209.GI6778@pendragon.ideasonboard.com> (Laurent\n\tPinchart's message of \"Mon, 24 Feb 2025 21:52:09 +0200\")","References":"<20250224185235.43381-1-mzamazal@redhat.com>\n\t<20250224185235.43381-4-mzamazal@redhat.com>\n\t<20250224195209.GI6778@pendragon.ideasonboard.com>","Date":"Mon, 24 Feb 2025 21:19:53 +0100","Message-ID":"<85ecznm6xi.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","User-Agent":"Gnus/5.13 (Gnus v5.13)","MIME-Version":"1.0","X-Mimecast-Spam-Score":"0","X-Mimecast-MFC-PROC-ID":"ZDIzJpYV9q2a7rf2ZUuXyxyEkXRYW4pkHJEHQASfubk_1740428397","X-Mimecast-Originator":"redhat.com","Content-Type":"text/plain","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":33461,"web_url":"https://patchwork.libcamera.org/comment/33461/","msgid":"<20250224204855.GA7352@pendragon.ideasonboard.com>","date":"2025-02-24T20:48:55","subject":"Re: [PATCH v2 3/5] libcamera: software_isp: Handle queued input\n\tbuffers on stop","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Mon, Feb 24, 2025 at 09:19:53PM +0100, Milan Zamazal wrote:\n> Laurent Pinchart writes:\n> > On Mon, Feb 24, 2025 at 07:52:33PM +0100, Milan Zamazal wrote:\n> >> When SoftwareIsp stops, input and output buffers queued to it may not\n> >> yet be fully processed.  They will be eventually returned but stop means\n> >> stop, there should be no processing related actions invoked afterwards.\n> >> \n> >> Let's stop forwarding processed input buffers from SoftwareIsp slots\n> >> when SoftwareIsp is stopped.  Let's track the queued input buffers and\n> >> return them back for capture in SoftwareIsp::stop().\n> >> \n> >> Signed-off-by: Milan Zamazal <mzamazal@redhat.com>\n> >> ---\n> >>  .../internal/software_isp/software_isp.h         |  1 +\n> >>  src/libcamera/software_isp/software_isp.cpp      | 16 +++++++++++++++-\n> >>  2 files changed, 16 insertions(+), 1 deletion(-)\n> >> \n> >> diff --git a/include/libcamera/internal/software_isp/software_isp.h b/include/libcamera/internal/software_isp/software_isp.h\n> >> index f2344355..bfe34725 100644\n> >> --- a/include/libcamera/internal/software_isp/software_isp.h\n> >> +++ b/include/libcamera/internal/software_isp/software_isp.h\n> >> @@ -102,6 +102,7 @@ private:\n> >>  \tstd::unique_ptr<ipa::soft::IPAProxySoft> ipa_;\n> >>  \tbool running_;\n> >>  \tstd::deque<FrameBuffer *> queuedOutputBuffers_;\n> >> +\tstd::deque<FrameBuffer *> queuedInputBuffers_;\n> >\n> > I'd move input before output.\n> >\n> >>  };\n> >>  \n> >>  } /* namespace libcamera */\n> >> diff --git a/src/libcamera/software_isp/software_isp.cpp b/src/libcamera/software_isp/software_isp.cpp\n> >> index 4339e547..3a605ab2 100644\n> >> --- a/src/libcamera/software_isp/software_isp.cpp\n> >> +++ b/src/libcamera/software_isp/software_isp.cpp\n> >> @@ -301,6 +301,8 @@ int SoftwareIsp::queueBuffers(uint32_t frame, FrameBuffer *input,\n> >>  \t\t\treturn -EINVAL;\n> >>  \t}\n> >>  \n> >> +\tqueuedInputBuffers_.push_back(input);\n> >> +\n> >>  \tfor (auto iter = outputs.begin(); iter != outputs.end(); iter++) {\n> >>  \t\tFrameBuffer *const buffer = iter->second;\n> >>  \t\tqueuedOutputBuffers_.push_back(buffer);\n> >> @@ -327,6 +329,9 @@ int SoftwareIsp::start()\n> >>  \n> >>  /**\n> >>   * \\brief Stops the Software ISP streaming operation\n> >> + *\n> >> + * All pending buffers are returned back (output buffers as canceled) before\n> >> + * this method finishes.\n> >\n> > Shouldn't the input buffer be cancelled too ?\n> \n> Is it necessary to cancel input buffers?  Aren't they just returned for\n> re-use regardless of their contents (unlike the output buffers where the\n> validity of the contents matters)?  The input buffer lands in\n> V4L2VideoDevice::queueBuffer() and I can't see any use of the status\n> there.\n\nAt the moment we don't use the status, but I think it would be nicer to\nstill mark it appropriately. If the input buffer hasn't been fully\nconsumed, marking it as cancelled will let pipeline handlers deal with\nthis if they need to.\n\nIf marking it as cancelled causes other issues then we can reconsider\nthat, but if it's easy I'd do so for completeness.\n\n> >>   */\n> >>  void SoftwareIsp::stop()\n> >>  {\n> >> @@ -342,6 +347,10 @@ void SoftwareIsp::stop()\n> >>  \t\toutputBufferReady.emit(buffer);\n> >>  \t}\n> >>  \tqueuedOutputBuffers_.clear();\n> >> +\n> >> +\tfor (auto buffer : queuedInputBuffers_)\n> >> +\t\tinputBufferReady.emit(buffer);\n> >> +\tqueuedInputBuffers_.clear();\n> >>  }\n> >>  \n> >>  /**\n> >> @@ -375,7 +384,12 @@ void SoftwareIsp::statsReady(uint32_t frame, uint32_t bufferId)\n> >>  \n> >>  void SoftwareIsp::inputReady(FrameBuffer *input)\n> >>  {\n> >> -\tinputBufferReady.emit(input);\n> >> +\tif (running_) {\n> >> +\t\tqueuedInputBuffers_.erase(find(queuedInputBuffers_.begin(),\n> >> +\t\t\t\t\t       queuedInputBuffers_.end(),\n> >> +\t\t\t\t\t       input));\n> >\n> > Same comment as for 2/5.\n> >\n> >> +\t\tinputBufferReady.emit(input);\n> >> +\t}\n> >>  }\n> >>  \n> >>  void SoftwareIsp::outputReady(FrameBuffer *output)","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 E8F2DC32A9\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 24 Feb 2025 20:49:16 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 1E69861856;\n\tMon, 24 Feb 2025 21:49:16 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id CF74C61856\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 24 Feb 2025 21:49:13 +0100 (CET)","from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi\n\t[81.175.209.231])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id D999D581;\n\tMon, 24 Feb 2025 21:47:46 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"m7gDvDod\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1740430067;\n\tbh=PgG93TIraY4HTFR8m7xHutPQKILbqGjzUrVUSnWaKPE=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=m7gDvDodlmDpQn3EBxNaxGxQClmTaEvcGE9KNy0I9OU4hTiCDEMy3ioAKSKmsH6J5\n\td6fQ+9O636PFPaYKSfaCtXzQPrsKm2xVhJhVavtS99gNYKRYHw79Q2iJ9K7dbt8JEo\n\tn8cqioOqFmcAliIVUMQpdoGCwa1dTKU39UL/V2jA=","Date":"Mon, 24 Feb 2025 22:48:55 +0200","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Milan Zamazal <mzamazal@redhat.com>","Cc":"libcamera-devel@lists.libcamera.org,\n\tStanislaw Gruszka <stanislaw.gruszka@linux.intel.com>,\n\tKieran Bingham <kieran.bingham@ideasonboard.com>","Subject":"Re: [PATCH v2 3/5] libcamera: software_isp: Handle queued input\n\tbuffers on stop","Message-ID":"<20250224204855.GA7352@pendragon.ideasonboard.com>","References":"<20250224185235.43381-1-mzamazal@redhat.com>\n\t<20250224185235.43381-4-mzamazal@redhat.com>\n\t<20250224195209.GI6778@pendragon.ideasonboard.com>\n\t<85ecznm6xi.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<85ecznm6xi.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":33466,"web_url":"https://patchwork.libcamera.org/comment/33466/","msgid":"<85zfibkq5c.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","date":"2025-02-24T21:07:43","subject":"Re: [PATCH v2 3/5] libcamera: software_isp: Handle queued input\n\tbuffers on stop","submitter":{"id":177,"url":"https://patchwork.libcamera.org/api/people/177/","name":"Milan Zamazal","email":"mzamazal@redhat.com"},"content":"Laurent Pinchart <laurent.pinchart@ideasonboard.com> writes:\n\n> On Mon, Feb 24, 2025 at 09:19:53PM +0100, Milan Zamazal wrote:\n>> Laurent Pinchart writes:\n>> > On Mon, Feb 24, 2025 at 07:52:33PM +0100, Milan Zamazal wrote:\n>\n>> >> When SoftwareIsp stops, input and output buffers queued to it may not\n>> >> yet be fully processed.  They will be eventually returned but stop means\n>> >> stop, there should be no processing related actions invoked afterwards.\n>> >> \n>> >> Let's stop forwarding processed input buffers from SoftwareIsp slots\n>> >> when SoftwareIsp is stopped.  Let's track the queued input buffers and\n>> >> return them back for capture in SoftwareIsp::stop().\n>> >> \n>> >> Signed-off-by: Milan Zamazal <mzamazal@redhat.com>\n>> >> ---\n>> >>  .../internal/software_isp/software_isp.h         |  1 +\n>> >>  src/libcamera/software_isp/software_isp.cpp      | 16 +++++++++++++++-\n>> >>  2 files changed, 16 insertions(+), 1 deletion(-)\n>> >> \n>> >> diff --git a/include/libcamera/internal/software_isp/software_isp.h b/include/libcamera/internal/software_isp/software_isp.h\n>> >> index f2344355..bfe34725 100644\n>> >> --- a/include/libcamera/internal/software_isp/software_isp.h\n>> >> +++ b/include/libcamera/internal/software_isp/software_isp.h\n>> >> @@ -102,6 +102,7 @@ private:\n>> >>  \tstd::unique_ptr<ipa::soft::IPAProxySoft> ipa_;\n>> >>  \tbool running_;\n>> >>  \tstd::deque<FrameBuffer *> queuedOutputBuffers_;\n>> >> +\tstd::deque<FrameBuffer *> queuedInputBuffers_;\n>> >\n>> > I'd move input before output.\n>> >\n>> >>  };\n>> >>  \n>> >>  } /* namespace libcamera */\n>> >> diff --git a/src/libcamera/software_isp/software_isp.cpp b/src/libcamera/software_isp/software_isp.cpp\n>> >> index 4339e547..3a605ab2 100644\n>> >> --- a/src/libcamera/software_isp/software_isp.cpp\n>> >> +++ b/src/libcamera/software_isp/software_isp.cpp\n>> >> @@ -301,6 +301,8 @@ int SoftwareIsp::queueBuffers(uint32_t frame, FrameBuffer *input,\n>> >>  \t\t\treturn -EINVAL;\n>> >>  \t}\n>> >>  \n>> >> +\tqueuedInputBuffers_.push_back(input);\n>> >> +\n>> >>  \tfor (auto iter = outputs.begin(); iter != outputs.end(); iter++) {\n>> >>  \t\tFrameBuffer *const buffer = iter->second;\n>> >>  \t\tqueuedOutputBuffers_.push_back(buffer);\n>> >> @@ -327,6 +329,9 @@ int SoftwareIsp::start()\n>> >>  \n>> >>  /**\n>> >>   * \\brief Stops the Software ISP streaming operation\n>> >> + *\n>> >> + * All pending buffers are returned back (output buffers as canceled) before\n>> >> + * this method finishes.\n>> >\n>> > Shouldn't the input buffer be cancelled too ?\n>> \n>> Is it necessary to cancel input buffers?  Aren't they just returned for\n>> re-use regardless of their contents (unlike the output buffers where the\n>> validity of the contents matters)?  The input buffer lands in\n>> V4L2VideoDevice::queueBuffer() and I can't see any use of the status\n>> there.\n>\n> At the moment we don't use the status, but I think it would be nicer to\n> still mark it appropriately. If the input buffer hasn't been fully\n> consumed, marking it as cancelled will let pipeline handlers deal with\n> this if they need to.\n\nOK.\n\n> If marking it as cancelled causes other issues then we can reconsider\n> that, but if it's easy I'd do so for completeness.\n\nWorks for me.  But I'm not much comfortable with the fact that then the\nbuffer may keep FrameCancelled status forever and float with it around.\nShould SimpleCameraData::conversionInputDone() or something else reset\nthe status?\n\n>> >>   */\n>> >>  void SoftwareIsp::stop()\n>> >>  {\n>> >> @@ -342,6 +347,10 @@ void SoftwareIsp::stop()\n>> >>  \t\toutputBufferReady.emit(buffer);\n>> >>  \t}\n>> >>  \tqueuedOutputBuffers_.clear();\n>> >> +\n>> >> +\tfor (auto buffer : queuedInputBuffers_)\n>> >> +\t\tinputBufferReady.emit(buffer);\n>> >> +\tqueuedInputBuffers_.clear();\n>> >>  }\n>> >>  \n>> >>  /**\n>> >> @@ -375,7 +384,12 @@ void SoftwareIsp::statsReady(uint32_t frame, uint32_t bufferId)\n>> >>  \n>> >>  void SoftwareIsp::inputReady(FrameBuffer *input)\n>> >>  {\n>> >> -\tinputBufferReady.emit(input);\n>> >> +\tif (running_) {\n>> >> +\t\tqueuedInputBuffers_.erase(find(queuedInputBuffers_.begin(),\n>> >> +\t\t\t\t\t       queuedInputBuffers_.end(),\n>> >> +\t\t\t\t\t       input));\n>> >\n>> > Same comment as for 2/5.\n>> >\n>> >> +\t\tinputBufferReady.emit(input);\n>> >> +\t}\n>> >>  }\n>> >>  \n>> >>  void SoftwareIsp::outputReady(FrameBuffer *output)","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 2FC12C324E\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 24 Feb 2025 21:07:53 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 832ED6870B;\n\tMon, 24 Feb 2025 22:07:52 +0100 (CET)","from us-smtp-delivery-124.mimecast.com\n\t(us-smtp-delivery-124.mimecast.com [170.10.129.124])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 5FCC561856\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 24 Feb 2025 22:07:50 +0100 (CET)","from mail-wm1-f72.google.com (mail-wm1-f72.google.com\n\t[209.85.128.72]) by relay.mimecast.com with ESMTP with STARTTLS\n\t(version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id\n\tus-mta-685-EDfvx4uVOUG6ef2fQXJjCQ-1; Mon, 24 Feb 2025 16:07:47 -0500","by mail-wm1-f72.google.com with SMTP id\n\t5b1f17b1804b1-43998ec3733so24577085e9.2\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 24 Feb 2025 13:07:47 -0800 (PST)","from mzamazal-thinkpadp1gen7.tpbc.csb\n\t(ip-77-48-47-2.net.vodafone.cz. [77.48.47.2])\n\tby smtp.gmail.com with ESMTPSA id\n\t5b1f17b1804b1-439b02f5b38sm118207125e9.24.2025.02.24.13.07.43\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tMon, 24 Feb 2025 13:07:44 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=redhat.com header.i=@redhat.com\n\theader.b=\"I4xCZ/HS\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1740431269;\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\tin-reply-to:in-reply-to:references:references;\n\tbh=a5gNZGvczDurkbhXPD+gCECFElGXBQM9x8PKLPGlPqs=;\n\tb=I4xCZ/HSPUIDV7p9Tu/X2BKUVtA25uRc7JyshFB+VJ8/MDVf6jO1KsuC/B4SkDFpDJ74rj\n\t3MJOicH6n0UDhaq/pyLE9/PZMIsz6YY4UQLf7IpnEekxDl6O9Dq8jW5ycTQ1T8Ofbx8GYo\n\tmeHwBBF7cFkpAyTBP81Ggh4kLup3mt8=","X-MC-Unique":"EDfvx4uVOUG6ef2fQXJjCQ-1","X-Mimecast-MFC-AGG-ID":"EDfvx4uVOUG6ef2fQXJjCQ_1740431266","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1740431266; x=1741036066;\n\th=mime-version:user-agent:message-id:date:references:in-reply-to\n\t:subject:cc:to:from:x-gm-message-state:from:to:cc:subject:date\n\t:message-id:reply-to;\n\tbh=a5gNZGvczDurkbhXPD+gCECFElGXBQM9x8PKLPGlPqs=;\n\tb=NBynLPAyDuR+TiQuhgT/MmHs48+PnEepMropKrWx/vdmkLeKdx9cNVk4G/Bo3MznK1\n\t0ba2gnlRnIbhYM0wCcOua6y2hUfRjRjky+RoJ5KnknP52Cz31GFLWzckU5lZlzn03gZ6\n\t17BReO4FC8h25Uphd3yMr4UReSKXOQ7+iJA5a84pw8bs/D8qrENFvMLznZdONdXKuzVV\n\teAyr72lYFmoTsXYWbGtw7UsonPiOLWyMO9QQZF1hhqdThDdRP178QX7odnZDPQK1LnHx\n\t9L5ax4j/A8F8aIDsurlGBosN5tkdzzDQA2xyZpXip+xIupAo9VAuS2abKT5ejsXll8jh\n\tvNuQ==","X-Gm-Message-State":"AOJu0YycU7I/KlamgGjrLpgdWoH/3RJ9gdbYo4jarPlb71v1PNlKMw1Y\n\tUpoVWseV0GPDSL3tlMHlf45wH6ZrsSTE/5lg8U++6m2E7nGOMztt3wSOPcsWOhzfqy8gPwdcch8\n\tO51FVDeNIBiijyOasggUpTj19WHpxPN5jLlwVIcdb05wta+r9JqrthgD7MKwk+M8nKLwqRh0=","X-Gm-Gg":"ASbGncs3+0PCkEoyT9WUMn8HLO1fzBGSah0IAFH0uHoNvk983E9jVvNSGj8DJewe/bU\n\t+0Bg/Er3cGiMMc8gAyiHS3H5Ia+hnCUYpJyo3Mj1hXkWZy2zCKSwmW7QdG7Q3GwHsRC6daxSu5R\n\tYR+RXPVaNjze/hBmv1chPvfmaT7eHly1QjGsC9Zri81nEGG+xR/OGXDfE41iby6dWmDrzwIe2on\n\t6grcRaqylKXsQDVRs91/3nh9MaKEzxn5Ljk8Feoc0whPweu6wUYAOCyAFPRnosuhJ1au3NDu4J3\n\twikKAoP3bppvwhkFxpQJBmXqgrhZZMjYSizC8JGwmmHaugN+OiM1JOcxBWsXzUjmokXJ","X-Received":["by 2002:a05:600c:198e:b0:439:9e53:49d8 with SMTP id\n\t5b1f17b1804b1-43ab0f979cdmr7932895e9.30.1740431266247; \n\tMon, 24 Feb 2025 13:07:46 -0800 (PST)","by 2002:a05:600c:198e:b0:439:9e53:49d8 with SMTP id\n\t5b1f17b1804b1-43ab0f979cdmr7932315e9.30.1740431265746; \n\tMon, 24 Feb 2025 13:07:45 -0800 (PST)"],"X-Google-Smtp-Source":"AGHT+IG3qPKxHKsx8P+n2MPJ/SC+nbtOa17JaoWt7sFXXZrJ1zfwLB0yOEUYhrqmIJ7qfNB0YMiOgg==","From":"Milan Zamazal <mzamazal@redhat.com>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org,  Stanislaw Gruszka\n\t<stanislaw.gruszka@linux.intel.com>,  Kieran Bingham\n\t<kieran.bingham@ideasonboard.com>","Subject":"Re: [PATCH v2 3/5] libcamera: software_isp: Handle queued input\n\tbuffers on stop","In-Reply-To":"<20250224204855.GA7352@pendragon.ideasonboard.com> (Laurent\n\tPinchart's message of \"Mon, 24 Feb 2025 22:48:55 +0200\")","References":"<20250224185235.43381-1-mzamazal@redhat.com>\n\t<20250224185235.43381-4-mzamazal@redhat.com>\n\t<20250224195209.GI6778@pendragon.ideasonboard.com>\n\t<85ecznm6xi.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>\n\t<20250224204855.GA7352@pendragon.ideasonboard.com>","Date":"Mon, 24 Feb 2025 22:07:43 +0100","Message-ID":"<85zfibkq5c.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","User-Agent":"Gnus/5.13 (Gnus v5.13)","MIME-Version":"1.0","X-Mimecast-Spam-Score":"0","X-Mimecast-MFC-PROC-ID":"hq4medyY9aUQdhn1r81VFruqDd3AOJ1gu30ps8xMDPk_1740431266","X-Mimecast-Originator":"redhat.com","Content-Type":"text/plain","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":33467,"web_url":"https://patchwork.libcamera.org/comment/33467/","msgid":"<20250224211203.GM6778@pendragon.ideasonboard.com>","date":"2025-02-24T21:12:03","subject":"Re: [PATCH v2 3/5] libcamera: software_isp: Handle queued input\n\tbuffers on stop","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Mon, Feb 24, 2025 at 10:07:43PM +0100, Milan Zamazal wrote:\n> Laurent Pinchart <laurent.pinchart@ideasonboard.com> writes:\n> \n> > On Mon, Feb 24, 2025 at 09:19:53PM +0100, Milan Zamazal wrote:\n> >> Laurent Pinchart writes:\n> >> > On Mon, Feb 24, 2025 at 07:52:33PM +0100, Milan Zamazal wrote:\n> >\n> >> >> When SoftwareIsp stops, input and output buffers queued to it may not\n> >> >> yet be fully processed.  They will be eventually returned but stop means\n> >> >> stop, there should be no processing related actions invoked afterwards.\n> >> >> \n> >> >> Let's stop forwarding processed input buffers from SoftwareIsp slots\n> >> >> when SoftwareIsp is stopped.  Let's track the queued input buffers and\n> >> >> return them back for capture in SoftwareIsp::stop().\n> >> >> \n> >> >> Signed-off-by: Milan Zamazal <mzamazal@redhat.com>\n> >> >> ---\n> >> >>  .../internal/software_isp/software_isp.h         |  1 +\n> >> >>  src/libcamera/software_isp/software_isp.cpp      | 16 +++++++++++++++-\n> >> >>  2 files changed, 16 insertions(+), 1 deletion(-)\n> >> >> \n> >> >> diff --git a/include/libcamera/internal/software_isp/software_isp.h b/include/libcamera/internal/software_isp/software_isp.h\n> >> >> index f2344355..bfe34725 100644\n> >> >> --- a/include/libcamera/internal/software_isp/software_isp.h\n> >> >> +++ b/include/libcamera/internal/software_isp/software_isp.h\n> >> >> @@ -102,6 +102,7 @@ private:\n> >> >>  \tstd::unique_ptr<ipa::soft::IPAProxySoft> ipa_;\n> >> >>  \tbool running_;\n> >> >>  \tstd::deque<FrameBuffer *> queuedOutputBuffers_;\n> >> >> +\tstd::deque<FrameBuffer *> queuedInputBuffers_;\n> >> >\n> >> > I'd move input before output.\n> >> >\n> >> >>  };\n> >> >>  \n> >> >>  } /* namespace libcamera */\n> >> >> diff --git a/src/libcamera/software_isp/software_isp.cpp b/src/libcamera/software_isp/software_isp.cpp\n> >> >> index 4339e547..3a605ab2 100644\n> >> >> --- a/src/libcamera/software_isp/software_isp.cpp\n> >> >> +++ b/src/libcamera/software_isp/software_isp.cpp\n> >> >> @@ -301,6 +301,8 @@ int SoftwareIsp::queueBuffers(uint32_t frame, FrameBuffer *input,\n> >> >>  \t\t\treturn -EINVAL;\n> >> >>  \t}\n> >> >>  \n> >> >> +\tqueuedInputBuffers_.push_back(input);\n> >> >> +\n> >> >>  \tfor (auto iter = outputs.begin(); iter != outputs.end(); iter++) {\n> >> >>  \t\tFrameBuffer *const buffer = iter->second;\n> >> >>  \t\tqueuedOutputBuffers_.push_back(buffer);\n> >> >> @@ -327,6 +329,9 @@ int SoftwareIsp::start()\n> >> >>  \n> >> >>  /**\n> >> >>   * \\brief Stops the Software ISP streaming operation\n> >> >> + *\n> >> >> + * All pending buffers are returned back (output buffers as canceled) before\n> >> >> + * this method finishes.\n> >> >\n> >> > Shouldn't the input buffer be cancelled too ?\n> >> \n> >> Is it necessary to cancel input buffers?  Aren't they just returned for\n> >> re-use regardless of their contents (unlike the output buffers where the\n> >> validity of the contents matters)?  The input buffer lands in\n> >> V4L2VideoDevice::queueBuffer() and I can't see any use of the status\n> >> there.\n> >\n> > At the moment we don't use the status, but I think it would be nicer to\n> > still mark it appropriately. If the input buffer hasn't been fully\n> > consumed, marking it as cancelled will let pipeline handlers deal with\n> > this if they need to.\n> \n> OK.\n> \n> > If marking it as cancelled causes other issues then we can reconsider\n> > that, but if it's easy I'd do so for completeness.\n> \n> Works for me.  But I'm not much comfortable with the fact that then the\n> buffer may keep FrameCancelled status forever and float with it around.\n> Should SimpleCameraData::conversionInputDone() or something else reset\n> the status?\n\nAren't the buffers freed in SimplePipelineHandler::stopDevice() by\n\n\tdata->conversionBuffers_.clear();\n\n?\n\n> >> >>   */\n> >> >>  void SoftwareIsp::stop()\n> >> >>  {\n> >> >> @@ -342,6 +347,10 @@ void SoftwareIsp::stop()\n> >> >>  \t\toutputBufferReady.emit(buffer);\n> >> >>  \t}\n> >> >>  \tqueuedOutputBuffers_.clear();\n> >> >> +\n> >> >> +\tfor (auto buffer : queuedInputBuffers_)\n> >> >> +\t\tinputBufferReady.emit(buffer);\n> >> >> +\tqueuedInputBuffers_.clear();\n> >> >>  }\n> >> >>  \n> >> >>  /**\n> >> >> @@ -375,7 +384,12 @@ void SoftwareIsp::statsReady(uint32_t frame, uint32_t bufferId)\n> >> >>  \n> >> >>  void SoftwareIsp::inputReady(FrameBuffer *input)\n> >> >>  {\n> >> >> -\tinputBufferReady.emit(input);\n> >> >> +\tif (running_) {\n> >> >> +\t\tqueuedInputBuffers_.erase(find(queuedInputBuffers_.begin(),\n> >> >> +\t\t\t\t\t       queuedInputBuffers_.end(),\n> >> >> +\t\t\t\t\t       input));\n> >> >\n> >> > Same comment as for 2/5.\n> >> >\n> >> >> +\t\tinputBufferReady.emit(input);\n> >> >> +\t}\n> >> >>  }\n> >> >>  \n> >> >>  void SoftwareIsp::outputReady(FrameBuffer *output)","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 C254BC32A9\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 24 Feb 2025 21:12:24 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id CF4966870E;\n\tMon, 24 Feb 2025 22:12:23 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 1E9F2686FF\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 24 Feb 2025 22:12:22 +0100 (CET)","from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi\n\t[81.175.209.231])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 3ACC5220;\n\tMon, 24 Feb 2025 22:10:55 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"FPXYMrTy\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1740431455;\n\tbh=WwXiT/SEf8YR/+VsrwIpCFCCDs94h+re21ci8KN3MqY=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=FPXYMrTyhyIS+G81KZZL81I8Hg+qpIWI2vxRlm+NwVBUrRsHY4owLFvLUUta93LXW\n\tc2puDnD3AErLkr2V278nFCZRdAQUtDhsu9JQXFS4JiWROKhiyQ4s5ZdpPQEMkLazbD\n\tTVtOxLb9pzqFb1R4VaI8zMCBTlCqSvOAsgmywJIo=","Date":"Mon, 24 Feb 2025 23:12:03 +0200","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Milan Zamazal <mzamazal@redhat.com>","Cc":"libcamera-devel@lists.libcamera.org,\n\tStanislaw Gruszka <stanislaw.gruszka@linux.intel.com>,\n\tKieran Bingham <kieran.bingham@ideasonboard.com>","Subject":"Re: [PATCH v2 3/5] libcamera: software_isp: Handle queued input\n\tbuffers on stop","Message-ID":"<20250224211203.GM6778@pendragon.ideasonboard.com>","References":"<20250224185235.43381-1-mzamazal@redhat.com>\n\t<20250224185235.43381-4-mzamazal@redhat.com>\n\t<20250224195209.GI6778@pendragon.ideasonboard.com>\n\t<85ecznm6xi.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>\n\t<20250224204855.GA7352@pendragon.ideasonboard.com>\n\t<85zfibkq5c.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<85zfibkq5c.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]