[{"id":32885,"web_url":"https://patchwork.libcamera.org/comment/32885/","msgid":"","date":"2024-12-19T15:05:38","subject":"Re: [PATCH] libcamera: software_isp: Handle signals in the proper\n\tthread","submitter":{"id":133,"url":"https://patchwork.libcamera.org/api/people/133/","name":"Pőcze Barnabás","email":"pobrn@protonmail.com"},"content":"Hi\n\n\n2024. december 19., csütörtök 11:31 keltezéssel, Milan Zamazal írta:\n\n> inputBufferReady ready signal in the simple pipeline is handled in the\n> pipeline handler thread. Similarly, outputBufferReady and ispStatsReady\n> signals should be handled there too, everything should be called from\n> the right thread and not block the callers.\n> \n> Signed-off-by: Milan Zamazal \n> ---\n> src/libcamera/pipeline/simple/simple.cpp | 9 +++++++--\n> 1 file changed, 7 insertions(+), 2 deletions(-)\n> \n> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n> index 8ac24e6e..280112f4 100644\n> --- a/src/libcamera/pipeline/simple/simple.cpp\n> +++ b/src/libcamera/pipeline/simple/simple.cpp\n> @@ -548,12 +548,17 @@ int SimpleCameraData::init()\n> \t\t\t * bound explicitly to the pipe, which is bound to the pipeline\n> \t\t\t * handler thread. The function then simply forwards the call to\n> \t\t\t * conversionInputDone().\n> +\t\t\t * Similarly for outputBufferReady and ispStatsReady signals.\n> \t\t\t */\n> \t\t\tswIsp_->inputBufferReady.connect(pipe, [this](FrameBuffer *buffer) {\n> \t\t\t\tthis->conversionInputDone(buffer);\n> \t\t\t});\n> -\t\t\tswIsp_->outputBufferReady.connect(this, &SimpleCameraData::conversionOutputDone);\n> -\t\t\tswIsp_->ispStatsReady.connect(this, &SimpleCameraData::ispStatsReady);\n> +\t\t\tswIsp_->outputBufferReady.connect(this, [this](FrameBuffer *buffer) {\n> +\t\t\t\tthis->conversionOutputDone(buffer);\n> +\t\t\t});\n> +\t\t\tswIsp_->ispStatsReady.connect(this, [this](uint32_t frame, uint32_t bufferId) {\n> +\t\t\t\tthis->ispStatsReady(frame, bufferId);\n> +\t\t\t});\n> [...]\n\nThe object is still `this`, so I wouldn't expect any difference in behaviour. Is there?\n\n\nRegards,\nBarnabás Pőcze","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 550F7C3272\n\tfor ;\n\tThu, 19 Dec 2024 15:05:49 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 603ED68474;\n\tThu, 19 Dec 2024 16:05:48 +0100 (CET)","from mail-40133.protonmail.ch (mail-40133.protonmail.ch\n\t[185.70.40.133])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id BB1AB61899\n\tfor ;\n\tThu, 19 Dec 2024 16:05:46 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=protonmail.com header.i=@protonmail.com\n\theader.b=\"NQyxP1Be\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=protonmail.com;\n\ts=protonmail3; t=1734620746; x=1734879946;\n\tbh=1ru6gzoNaBX3hOkys583Mk2lMO4zqVeLnjEF7bp9auc=;\n\th=Date:To:From:Cc:Subject:Message-ID:In-Reply-To:References:\n\tFeedback-ID:From:To:Cc:Date:Subject:Reply-To:Feedback-ID:\n\tMessage-ID:BIMI-Selector:List-Unsubscribe:List-Unsubscribe-Post;\n\tb=NQyxP1Be5FZyV3uD6sIzMqrfqkpOK/O4WFjGFwxhERWGcP/ZMMTQKcs2bPTjzxI3a\n\tXpfah9J9W+vvpsYjxu0b2WPVvhxL+3CKEpc1yF6UAxvDGUzSh7bKxYhqr19Ll2P6ia\n\tln6bw7jbqaUetXAq1d/7y5VJNXfBMiYpbOnrSHWVno9kAukxGw3hCi8dq95vGe7J1A\n\t3ChQYoeA4l+eMnsVF92m0dd8ZvjBWZr7WEIjpEaBUR58x41vSJ7a9mrbwOqnRndIxd\n\tR79716v9OKNDYr5ds2zpx8CL6dNikzt8em0ho/f5suSsoZB8ZkJG4HsFzJaXrTee9W\n\tRLP5wRZjL7X5Q==","Date":"Thu, 19 Dec 2024 15:05:38 +0000","To":"Milan Zamazal ","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Cc":"libcamera-devel@lists.libcamera.org,\n\tLaurent Pinchart ,\n\tStanislaw Gruszka ","Subject":"Re: [PATCH] libcamera: software_isp: Handle signals in the proper\n\tthread","Message-ID":"","In-Reply-To":"<20241219103108.244243-1-mzamazal@redhat.com>","References":"<20241219103108.244243-1-mzamazal@redhat.com>","Feedback-ID":"20568564:user:proton","X-Pm-Message-ID":"210c0941c06cca5fc8efb25af286fb6ffb830293","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Transfer-Encoding":"quoted-printable","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\" "}},{"id":32886,"web_url":"https://patchwork.libcamera.org/comment/32886/","msgid":"<878qsb8jgy.fsf@redhat.com>","date":"2024-12-19T21:13:17","subject":"Re: [PATCH] libcamera: software_isp: Handle signals in the proper\n\tthread","submitter":{"id":177,"url":"https://patchwork.libcamera.org/api/people/177/","name":"Milan Zamazal","email":"mzamazal@redhat.com"},"content":"Hi Barnabás,\n\nthank you for review.\n\nBarnabás Pőcze writes:\n\n> Hi\n>\n>\n> 2024. december 19., csütörtök 11:31 keltezéssel, Milan Zamazal írta:\n>\n>> inputBufferReady ready signal in the simple pipeline is handled in the\n>> pipeline handler thread. Similarly, outputBufferReady and ispStatsReady\n>> signals should be handled there too, everything should be called from\n>> the right thread and not block the callers.\n>> \n>> Signed-off-by: Milan Zamazal \n>> ---\n>> src/libcamera/pipeline/simple/simple.cpp | 9 +++++++--\n>> 1 file changed, 7 insertions(+), 2 deletions(-)\n>> \n>> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n>> index 8ac24e6e..280112f4 100644\n>> --- a/src/libcamera/pipeline/simple/simple.cpp\n>> +++ b/src/libcamera/pipeline/simple/simple.cpp\n>> @@ -548,12 +548,17 @@ int SimpleCameraData::init()\n>> \t\t\t * bound explicitly to the pipe, which is bound to the pipeline\n>> \t\t\t * handler thread. The function then simply forwards the call to\n>> \t\t\t * conversionInputDone().\n>> +\t\t\t * Similarly for outputBufferReady and ispStatsReady signals.\n>> \t\t\t */\n>> \t\t\tswIsp_->inputBufferReady.connect(pipe, [this](FrameBuffer *buffer) {\n>> \t\t\t\tthis->conversionInputDone(buffer);\n>> \t\t\t});\n>> -\t\t\tswIsp_->outputBufferReady.connect(this, &SimpleCameraData::conversionOutputDone);\n>> -\t\t\tswIsp_->ispStatsReady.connect(this, &SimpleCameraData::ispStatsReady);\n>> +\t\t\tswIsp_->outputBufferReady.connect(this, [this](FrameBuffer *buffer) {\n>> +\t\t\t\tthis->conversionOutputDone(buffer);\n>> +\t\t\t});\n>> +\t\t\tswIsp_->ispStatsReady.connect(this, [this](uint32_t frame, uint32_t bufferId) {\n>> +\t\t\t\tthis->ispStatsReady(frame, bufferId);\n>> +\t\t\t});\n>> [...]\n>\n> The object is still `this`, so I wouldn't expect any difference in behaviour. Is there?\n\nAh, right, should be `pipe', will fix it in v2.\n\n> Regards,\n> Barnabás Pőcze","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 55FA1C3272\n\tfor ;\n\tThu, 19 Dec 2024 21:13:28 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id ACC7068482;\n\tThu, 19 Dec 2024 22:13:27 +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 E255A68471\n\tfor ;\n\tThu, 19 Dec 2024 22:13:24 +0100 (CET)","from mail-wm1-f71.google.com (mail-wm1-f71.google.com\n\t[209.85.128.71]) by relay.mimecast.com with ESMTP with STARTTLS\n\t(version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id\n\tus-mta-649-whnS1XZGNim_Dd53sda9sg-1; Thu, 19 Dec 2024 16:13:22 -0500","by mail-wm1-f71.google.com with SMTP id\n\t5b1f17b1804b1-4362552ce62so7179855e9.0\n\tfor ;\n\tThu, 19 Dec 2024 13:13:22 -0800 (PST)","from nuthatch (ip-77-48-47-2.net.vodafone.cz. [77.48.47.2])\n\tby smtp.gmail.com with ESMTPSA id\n\t5b1f17b1804b1-43656b42757sm62842415e9.39.2024.12.19.13.13.18\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tThu, 19 Dec 2024 13:13:19 -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=\"Nmktyhs0\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1734642803;\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\tin-reply-to:in-reply-to:references:references;\n\tbh=VoQI+ktTVQwGHgWw9IfTqqck6TmccTWhDbJcPNQNPto=;\n\tb=Nmktyhs0vnRQ0HHrhOtgFR02EGL6spTRwr+YH0GTKBVySAQ6xngpzNMkBFsOkJ7OUsaFv4\n\tFCdl8yuX6YYu5i1/MKMC/i6o41UpMgjwdbXjoID+N+4MVfzecfyHwgDPcVReYaPvKXahEC\n\tUpubWp2xF/JJZkFvPyKrpUMchU9za0M=","X-MC-Unique":"whnS1XZGNim_Dd53sda9sg-1","X-Mimecast-MFC-AGG-ID":"whnS1XZGNim_Dd53sda9sg","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1734642801; x=1735247601;\n\th=content-transfer-encoding:mime-version:user-agent:message-id:date\n\t:references:in-reply-to:subject:cc:to:from:x-gm-message-state:from\n\t:to:cc:subject:date:message-id:reply-to;\n\tbh=UhY5NdlUTVSTzVfK5HsAjAHl/UIxMCYZ3OMMaMt15NI=;\n\tb=lIq12KKwBQNuPyKeTNJIiqpOjDaiuogz7uorMsk6AgBLXj4D1Liqb44ZatME6Ovtm2\n\tE0Z6del40nQRWOc3ZF5+2Tj7vNytoM/e6GqtWeYmtsVZBFeGVv//lBS7zVpBAvhVizEl\n\tDxoO11Y/AsBCtNtm8zpYMNdI2Qp8nJkHgcwyXtVwFOoGo5fPWOBoqo5xg7RAz9Ctm7YA\n\tSxyzqleVcW+OTibHKqBrHTUGNpGKj1iIedPAgamd/PiPqCtdr0vfs44z+QJEyPlbliWh\n\tt0l4tWXzIMea7pIEIwqMcSw1BRD9owHWuOyI0iBwdZigNk4KLb3PUZskLkZF9nJEPd0i\n\tEang==","X-Gm-Message-State":"AOJu0YyiTVjJkqFUuKFl0hykFeIZEzMGNby8iDk0wOjxbLZfr6CMeAPf\n\tG6yyaSVoYccZKLPSi6ZqYt6nElmpCUuQskUE8jCie6MiW41jimkGn+t6826V+rXHUvw6un0YcR0\n\t03rC2w/iQ9wFgkE2k49dlX0eOtaAf1hopjkKuc9AH4JvWn5ohNjfhLHnFfR6+VgYvdZiZTcyALS\n\ts1bPs=","X-Gm-Gg":"ASbGnct3rAQ6btpA1lb6NPjwzbt0tttJDF4EKJtdr7gqecYKo/Ndv9TobW3N9rRdi3c\n\tcxbR82LhF4LwhmXWvr33GyDXMkiqrOzqI51ffJ/BD9T6HFxpxVGIGOFDt9tGVUgxM6JxBQ3Il2W\n\trwbBFW5pxTA9wyizEILnVTk4vFOpfIcwULzThn/ZeHv0OIs/EjmSwNbu2zW463r22DErKm4NgW+\n\tRFxg9lGZyhr2z3Mb7PC5yBfjkq060TbEn9aSyBQx6hXHbIcxgCPgtRr+NIkaau0u63RP+fXg0g1\n\tGw==","X-Received":["by 2002:a05:600c:350c:b0:434:a1d3:a30f with SMTP id\n\t5b1f17b1804b1-43668547554mr2865255e9.6.1734642800773; \n\tThu, 19 Dec 2024 13:13:20 -0800 (PST)","by 2002:a05:600c:350c:b0:434:a1d3:a30f with SMTP id\n\t5b1f17b1804b1-43668547554mr2865175e9.6.1734642800423; \n\tThu, 19 Dec 2024 13:13:20 -0800 (PST)"],"X-Google-Smtp-Source":"AGHT+IGrQF9DksrhkioN1ZG5pbHVVFzIhZyFxg5ZL3REYbcmOAJSTMohFbiinE5mCjcNozy62HJUMA==","From":"Milan Zamazal ","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Cc":"libcamera-devel@lists.libcamera.org, Laurent Pinchart\n\t, Stanislaw Gruszka\n\t","Subject":"Re: [PATCH] libcamera: software_isp: Handle signals in the proper\n\tthread","In-Reply-To":"\n\t( =?utf-8?b?IkJhcm5hYsOhcyBQxZFjemUiJ3M=?= message of \"Thu,\n\t19 Dec 2024 15:05:38 +0000\")","References":"<20241219103108.244243-1-mzamazal@redhat.com>\n\t","Date":"Thu, 19 Dec 2024 22:13:17 +0100","Message-ID":"<878qsb8jgy.fsf@redhat.com>","User-Agent":"Gnus/5.13 (Gnus v5.13)","MIME-Version":"1.0","X-Mimecast-Spam-Score":"0","X-Mimecast-MFC-PROC-ID":"hO_CHxFUMIOaGpFE8I2rC-lbcMxr6CXyN_HOS10O99Y_1734642801","X-Mimecast-Originator":"redhat.com","Content-Type":"text/plain; charset=utf-8","Content-Transfer-Encoding":"quoted-printable","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\" "}},{"id":32888,"web_url":"https://patchwork.libcamera.org/comment/32888/","msgid":"<20241219232219.GK19884@pendragon.ideasonboard.com>","date":"2024-12-19T23:22:19","subject":"Re: [PATCH] libcamera: software_isp: Handle signals in the proper\n\tthread","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Milan,\n\nOn Thu, Dec 19, 2024 at 10:13:17PM +0100, Milan Zamazal wrote:\n> Barnabás Pőcze writes:\n> > 2024. december 19., csütörtök 11:31 keltezéssel, Milan Zamazal írta:\n> >\n> >> inputBufferReady ready signal in the simple pipeline is handled in the\n> >> pipeline handler thread. Similarly, outputBufferReady and ispStatsReady\n> >> signals should be handled there too, everything should be called from\n> >> the right thread and not block the callers.\n> >> \n> >> Signed-off-by: Milan Zamazal \n> >> ---\n> >> src/libcamera/pipeline/simple/simple.cpp | 9 +++++++--\n> >> 1 file changed, 7 insertions(+), 2 deletions(-)\n> >> \n> >> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n> >> index 8ac24e6e..280112f4 100644\n> >> --- a/src/libcamera/pipeline/simple/simple.cpp\n> >> +++ b/src/libcamera/pipeline/simple/simple.cpp\n> >> @@ -548,12 +548,17 @@ int SimpleCameraData::init()\n> >> \t\t\t * bound explicitly to the pipe, which is bound to the pipeline\n> >> \t\t\t * handler thread. The function then simply forwards the call to\n> >> \t\t\t * conversionInputDone().\n> >> +\t\t\t * Similarly for outputBufferReady and ispStatsReady signals.\n> >> \t\t\t */\n> >> \t\t\tswIsp_->inputBufferReady.connect(pipe, [this](FrameBuffer *buffer) {\n> >> \t\t\t\tthis->conversionInputDone(buffer);\n> >> \t\t\t});\n> >> -\t\t\tswIsp_->outputBufferReady.connect(this, &SimpleCameraData::conversionOutputDone);\n> >> -\t\t\tswIsp_->ispStatsReady.connect(this, &SimpleCameraData::ispStatsReady);\n> >> +\t\t\tswIsp_->outputBufferReady.connect(this, [this](FrameBuffer *buffer) {\n> >> +\t\t\t\tthis->conversionOutputDone(buffer);\n> >> +\t\t\t});\n> >> +\t\t\tswIsp_->ispStatsReady.connect(this, [this](uint32_t frame, uint32_t bufferId) {\n> >> +\t\t\t\tthis->ispStatsReady(frame, bufferId);\n> >> +\t\t\t});\n> >> [...]\n> >\n> > The object is still `this`, so I wouldn't expect any difference in behaviour. Is there?\n> \n> Ah, right, should be `pipe', will fix it in v2.\n\nI think it would be better to emit the signals from the camera manager\nthread, instead of relying on the user of the soft ISP to perform\ncross-thread synchronization. That will be less error-prone.","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 71EA7BD7D8\n\tfor ;\n\tThu, 19 Dec 2024 23:22:27 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 553DD68485;\n\tFri, 20 Dec 2024 00:22:26 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 6CA6D680AE\n\tfor ;\n\tFri, 20 Dec 2024 00:22:24 +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 23DBA526;\n\tFri, 20 Dec 2024 00:21:45 +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=\"kFMsMfi3\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1734650505;\n\tbh=5aqpUWFiBipQN6LGzr3qAruD3IMWybRA1OTN7ePm1Ew=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=kFMsMfi36IExmK44VzVAx6Kj0Zxq94b8329fHacvs7M6ILY/tQzdDFaWFaR46x1FG\n\tVhWA7Fkp0oIiHeR62LPip9UtnqSUHUUIo/nHr3v0W9G6S4Tj4rQsuzo9ZJOsDsePCw\n\tb/ziaF85qrgogESNrGUDaTALiIoCkkB4hlLUwX7M=","Date":"Fri, 20 Dec 2024 01:22:19 +0200","From":"Laurent Pinchart ","To":"Milan Zamazal ","Cc":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ,\n\tlibcamera-devel@lists.libcamera.org, Stanislaw Gruszka\n\t","Subject":"Re: [PATCH] libcamera: software_isp: Handle signals in the proper\n\tthread","Message-ID":"<20241219232219.GK19884@pendragon.ideasonboard.com>","References":"<20241219103108.244243-1-mzamazal@redhat.com>\n\t\n\t<878qsb8jgy.fsf@redhat.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<878qsb8jgy.fsf@redhat.com>","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\" "}},{"id":32931,"web_url":"https://patchwork.libcamera.org/comment/32931/","msgid":"<85pll3ycit.fsf@mzamazal-thinkpadp1gen3.tpbc.csb>","date":"2025-01-03T18:31:54","subject":"Re: [PATCH] libcamera: software_isp: Handle signals in the proper\n\tthread","submitter":{"id":177,"url":"https://patchwork.libcamera.org/api/people/177/","name":"Milan Zamazal","email":"mzamazal@redhat.com"},"content":"Hi Laurent,\n\nLaurent Pinchart writes:\n\n> Hi Milan,\n>\n> On Thu, Dec 19, 2024 at 10:13:17PM +0100, Milan Zamazal wrote:\n>> Barnabás Pőcze writes:\n>> > 2024. december 19., csütörtök 11:31 keltezéssel, Milan Zamazal írta:\n>> >\n>> >> inputBufferReady ready signal in the simple pipeline is handled in the\n>> >> pipeline handler thread. Similarly, outputBufferReady and ispStatsReady\n>> >> signals should be handled there too, everything should be called from\n>> >> the right thread and not block the callers.\n>> >> \n>> >> Signed-off-by: Milan Zamazal \n>> >> ---\n>> >> src/libcamera/pipeline/simple/simple.cpp | 9 +++++++--\n>> >> 1 file changed, 7 insertions(+), 2 deletions(-)\n>> >> \n>> >> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n>> >> index 8ac24e6e..280112f4 100644\n>> >> --- a/src/libcamera/pipeline/simple/simple.cpp\n>> >> +++ b/src/libcamera/pipeline/simple/simple.cpp\n>> >> @@ -548,12 +548,17 @@ int SimpleCameraData::init()\n>> >> \t\t\t * bound explicitly to the pipe, which is bound to the pipeline\n>> >> \t\t\t * handler thread. The function then simply forwards the call to\n>> >> \t\t\t * conversionInputDone().\n>> >> +\t\t\t * Similarly for outputBufferReady and ispStatsReady signals.\n>> >> \t\t\t */\n>> >> \t\t\tswIsp_->inputBufferReady.connect(pipe, [this](FrameBuffer *buffer) {\n>> >> \t\t\t\tthis->conversionInputDone(buffer);\n>> >> \t\t\t});\n>> >> -\t\t\tswIsp_->outputBufferReady.connect(this, &SimpleCameraData::conversionOutputDone);\n>> >> -\t\t\tswIsp_->ispStatsReady.connect(this, &SimpleCameraData::ispStatsReady);\n>> >> +\t\t\tswIsp_->outputBufferReady.connect(this, [this](FrameBuffer *buffer) {\n>> >> +\t\t\t\tthis->conversionOutputDone(buffer);\n>> >> +\t\t\t});\n>> >> +\t\t\tswIsp_->ispStatsReady.connect(this, [this](uint32_t frame, uint32_t bufferId) {\n>> >> +\t\t\t\tthis->ispStatsReady(frame, bufferId);\n>> >> +\t\t\t});\n>> >> [...]\n>> >\n>> > The object is still `this`, so I wouldn't expect any difference in behaviour. Is there?\n>> \n>> Ah, right, should be `pipe', will fix it in v2.\n>\n> I think it would be better to emit the signals from the camera manager\n> thread, instead of relying on the user of the soft ISP to perform\n> cross-thread synchronization. That will be less error-prone.\n\nMaybe a stupid question, but how to do it? The signals are emitted from\nthe corresponding place/thread asynchronously and there is little choice\nof the thread to use. At least without passing around some Object bound\nto the camera manager thread, which wouldn't be nice either I suppose.\nWhat do I miss?","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 8F465C324C\n\tfor ;\n\tFri, 3 Jan 2025 18:32:02 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 77950684CC;\n\tFri, 3 Jan 2025 19:32:01 +0100 (CET)","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 C0C0D61886\n\tfor ;\n\tFri, 3 Jan 2025 19:31:59 +0100 (CET)","from mail-wm1-f70.google.com (mail-wm1-f70.google.com\n\t[209.85.128.70]) by relay.mimecast.com with ESMTP with STARTTLS\n\t(version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id\n\tus-mta-98-FEOVKlQ5MxSgg_3X_9kvqw-1; Fri, 03 Jan 2025 13:31:57 -0500","by mail-wm1-f70.google.com with SMTP id\n\t5b1f17b1804b1-4359eb032c9so99254065e9.2\n\tfor ;\n\tFri, 03 Jan 2025 10:31:57 -0800 (PST)","from mzamazal-thinkpadp1gen3.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-43656b3b2afsm536515275e9.35.2025.01.03.10.31.54\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tFri, 03 Jan 2025 10:31: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=\"FlaxRmRF\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1735929118;\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\tin-reply-to:in-reply-to:references:references;\n\tbh=sEckL0KagZayQ0SxBarpU8ibpZuGCF1Amlt4m+tB3WU=;\n\tb=FlaxRmRFRRBJWA3n/UGN6IIZKCFXqYLwHwNTa290jVG2dmmMxpRTaoO7/DsD8g7HkRYvYf\n\t7nlJwb9yrh6gah9+FOQTCnwpumfuBjsYMO69kxEOTS89Ila6H5PITdZg9PX1iT9T2fAk2z\n\tV6wUhcnOfHitQY+1r8l0NZ+pVR2hoD4=","X-MC-Unique":"FEOVKlQ5MxSgg_3X_9kvqw-1","X-Mimecast-MFC-AGG-ID":"FEOVKlQ5MxSgg_3X_9kvqw","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1735929116; x=1736533916;\n\th=content-transfer-encoding:mime-version:user-agent:message-id:date\n\t:references:in-reply-to:subject:cc:to:from:x-gm-message-state:from\n\t:to:cc:subject:date:message-id:reply-to;\n\tbh=ZfsQTRUgFKGYmRWdJ5nV6s+bQX10lSNnlGk4Yq9YuLg=;\n\tb=MZcKn/F5JHEZJ4KYcOIQujpK6WVoeWaIDd7IGXFde1c2oRpakPsXS3IsEtyiJj8HFa\n\tJcluKo9HG4kxZgBDkKVvjlJmYldDe30/injJdCFrnEyBcqmwrgcM61EDlmRNWZatJ9T5\n\tADZBG6pyiiz10ISoDiPofGQfaubschtM8dxBNmmnRE7E7uP91AqY010ex26VRSKrc6pC\n\tvHWt6ZZ/yZAFFBOKHSMdUds1jwDxeARtraJFAcagnaVMRSnKFW33R3c+FqoybplZwr5r\n\tySlPIQt7b/y/Oj4VgLGsO21uBvw8hxcUJUCq7EGaXWXHW0hxyy2QWmxvVhUXXWtgcv+0\n\tAxTg==","X-Forwarded-Encrypted":"i=1;\n\tAJvYcCXNyIDgGo3bQ6rdzDznQKNd7NqLU59qa0cV/B7e0rZtVrDBIBJ0pzePsbJ1nNixQq6e9bplJT0ItfSBgMxvHJQ=@lists.libcamera.org","X-Gm-Message-State":"AOJu0YxQXWh1E3/A+K0CON+PrFueJge4XkE6NfBSySJF3/kxWMhIM3o5\n\tvmqvyn/kuG8dBFfRzv/ydfslG47MLH8TgwnDzB+G/VQvLYSVmj5PiVvwGhjB0+IQRSAHokGwq1L\n\t17QHyEU60qX5FjGtmcFI2/45v2H/L6XvtxyrtEEF4Oe0pR4xzhG2beEBUks6G0IbkwdxipPA=","X-Gm-Gg":"ASbGnctnbqM5yrShduhFKwlP3eIKawK7iOxvrqOvKaGgER7Je7l2H/t0VeK3XkxvV4P\n\tRQvDhYq37L4Uv7z+sIupvMDFP6ePzNooDvyJ8fV98F3JeX0rvxEMWEEZyek9FB1IAF1ABcEpbgY\n\t9foohgDTzt+VN2lW+P+/uclPJyG0sPhho5F68QJZocQskESchfMsZhhfGDWqkCfYcO5ftxASieK\n\t4/V7xtFXC4Bz+Qw3shVi07tEhJ+JawSJKB+jNbv3DRFUhD0OcWY8zrlaYttbfKf0NcDiOtpPxFi\n\tYLqunfPGwT431rSVQRgNqgEuSv4WEOOQCg==","X-Received":["by 2002:a05:600c:4f03:b0:435:1a2:2633 with SMTP id\n\t5b1f17b1804b1-43668645cf5mr468130575e9.15.1735929116242; \n\tFri, 03 Jan 2025 10:31:56 -0800 (PST)","by 2002:a05:600c:4f03:b0:435:1a2:2633 with SMTP id\n\t5b1f17b1804b1-43668645cf5mr468130395e9.15.1735929115886; \n\tFri, 03 Jan 2025 10:31:55 -0800 (PST)"],"X-Google-Smtp-Source":"AGHT+IG9KxtP/+OjO85qlJal7rmdTXeY8D32gLQr83h9E9LtkT8xMRf/7yynApx84gle3zY1A6MTUQ==","From":"Milan Zamazal ","To":"Laurent Pinchart ","Cc":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ,\n\tlibcamera-devel@lists.libcamera.org, Stanislaw Gruszka\n\t","Subject":"Re: [PATCH] libcamera: software_isp: Handle signals in the proper\n\tthread","In-Reply-To":"<20241219232219.GK19884@pendragon.ideasonboard.com> (Laurent\n\tPinchart's message of \"Fri, 20 Dec 2024 01:22:19 +0200\")","References":"<20241219103108.244243-1-mzamazal@redhat.com>\n\t\n\t<878qsb8jgy.fsf@redhat.com>\n\t<20241219232219.GK19884@pendragon.ideasonboard.com>","Date":"Fri, 03 Jan 2025 19:31:54 +0100","Message-ID":"<85pll3ycit.fsf@mzamazal-thinkpadp1gen3.tpbc.csb>","User-Agent":"Gnus/5.13 (Gnus v5.13)","MIME-Version":"1.0","X-Mimecast-Spam-Score":"0","X-Mimecast-MFC-PROC-ID":"d3ZnKjykdmsQPjMvJarTljUndeodvtuPjYYw-rYZGpU_1735929116","X-Mimecast-Originator":"redhat.com","Content-Type":"text/plain; charset=utf-8","Content-Transfer-Encoding":"quoted-printable","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\" "}},{"id":33197,"web_url":"https://patchwork.libcamera.org/comment/33197/","msgid":"<20250127074319.GC11289@pendragon.ideasonboard.com>","date":"2025-01-27T07:43:19","subject":"Re: [PATCH] libcamera: software_isp: Handle signals in the proper\n\tthread","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Milan,\n\nOn Fri, Jan 03, 2025 at 07:31:54PM +0100, Milan Zamazal wrote:\n> Laurent Pinchart writes:\n> > On Thu, Dec 19, 2024 at 10:13:17PM +0100, Milan Zamazal wrote:\n> >> Barnabás Pőcze writes:\n> >> > 2024. december 19., csütörtök 11:31 keltezéssel, Milan Zamazal írta:\n> >> >\n> >> >> inputBufferReady ready signal in the simple pipeline is handled in the\n> >> >> pipeline handler thread. Similarly, outputBufferReady and ispStatsReady\n> >> >> signals should be handled there too, everything should be called from\n> >> >> the right thread and not block the callers.\n> >> >> \n> >> >> Signed-off-by: Milan Zamazal \n> >> >> ---\n> >> >> src/libcamera/pipeline/simple/simple.cpp | 9 +++++++--\n> >> >> 1 file changed, 7 insertions(+), 2 deletions(-)\n> >> >> \n> >> >> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n> >> >> index 8ac24e6e..280112f4 100644\n> >> >> --- a/src/libcamera/pipeline/simple/simple.cpp\n> >> >> +++ b/src/libcamera/pipeline/simple/simple.cpp\n> >> >> @@ -548,12 +548,17 @@ int SimpleCameraData::init()\n> >> >> \t\t\t * bound explicitly to the pipe, which is bound to the pipeline\n> >> >> \t\t\t * handler thread. The function then simply forwards the call to\n> >> >> \t\t\t * conversionInputDone().\n> >> >> +\t\t\t * Similarly for outputBufferReady and ispStatsReady signals.\n> >> >> \t\t\t */\n> >> >> \t\t\tswIsp_->inputBufferReady.connect(pipe, [this](FrameBuffer *buffer) {\n> >> >> \t\t\t\tthis->conversionInputDone(buffer);\n> >> >> \t\t\t});\n> >> >> -\t\t\tswIsp_->outputBufferReady.connect(this, &SimpleCameraData::conversionOutputDone);\n> >> >> -\t\t\tswIsp_->ispStatsReady.connect(this, &SimpleCameraData::ispStatsReady);\n> >> >> +\t\t\tswIsp_->outputBufferReady.connect(this, [this](FrameBuffer *buffer) {\n> >> >> +\t\t\t\tthis->conversionOutputDone(buffer);\n> >> >> +\t\t\t});\n> >> >> +\t\t\tswIsp_->ispStatsReady.connect(this, [this](uint32_t frame, uint32_t bufferId) {\n> >> >> +\t\t\t\tthis->ispStatsReady(frame, bufferId);\n> >> >> +\t\t\t});\n> >> >> [...]\n> >> >\n> >> > The object is still `this`, so I wouldn't expect any difference\n> >> > in behaviour. Is there?\n> >> \n> >> Ah, right, should be `pipe', will fix it in v2.\n> >\n> > I think it would be better to emit the signals from the camera manager\n> > thread, instead of relying on the user of the soft ISP to perform\n> > cross-thread synchronization. That will be less error-prone.\n> \n> Maybe a stupid question, but how to do it? The signals are emitted from\n> the corresponding place/thread asynchronously and there is little choice\n> of the thread to use. At least without passing around some Object bound\n> to the camera manager thread, which wouldn't be nice either I suppose.\n> What do I miss?\n\nNot a stupid question at all, I should have made this clearer.\n\nSignals are emitted from the thread running when the emit() function is\ncalled. If a signal is connected to an object that does not inherit from\nthe Object class, the connected slot will be called synchronously, in\nthe same thread.\n\nIf the connected object inherits from the Object class, the behaviour\ndiffers and depends on the connection type, specified when calling the\nconnect() function on the signal:\n\n- If the connection type is ConnectionTypeDirect, the slot is called\n synchronously, in the same thread as the emit() calls.\n\n- If the connection type is ConnectionTypeQueued, the slot is called\n asynchronously, in the thread of the connected object. The emit() call\n returns immediately, before the slot is called.\n\n- If the connection type is ConnectionTypeBlocking, the slot is called\n synchronously, in the thread of the connected object. The emit() call\n waits until the synchronous call completes in the other thread before\n returning. If the emitter and receiver are in the same thread, this\n behaves like ConnectionTypeDirect.\n\n- If the connection type is ConnectionTypeAuto (the default),\n ConnectionTypeDirect is used if the emitter and receiver are in the\n same thread, and ConnectionTypeQueued is used otherwise.\n\nThe SimpleCameraData class does not inherit from Object, so slots are\ncalled synchronously, in the thread of the emitter. This is why the\ninputBufferReady signal is handled with a lambda function:\n\n\tswIsp_->inputBufferReady.connect(pipe, [this](FrameBuffer *buffer) {\n\t\tthis->conversionInputDone(buffer);\n\t});\n\nThe first argument to the connect() function is the receiver object,\nwhich here inherits from the Object class. As a result, the lambda\nfunction is called asynchronusly in the pipeline handler thread, not the\nsoft ISP worker thread.\n\nThis construct is fragile, as shown by this patch: the outputBufferReady\nand ispStatsReady signals were mistakenly connected to the camera data\nobject, resulting in the slots being called from the wrong thread. I\nthink it would be better to handle this issue in the soft ISP class, to\navoid depending on the implementation of the receiver. One easy way to\ndo so is to make the SoftwareIsp class inherit from the Object class.\nThat way, SoftwareIsp::inputReady() will be called in the thread in\nwhich the SoftwareIsp instance lives, which is the pipeline handler\nthread. The inputBufferReady signal that is emitted from there will\nalways come from the pipeline handler thread.","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 39531BDCC1\n\tfor ;\n\tMon, 27 Jan 2025 07:43:32 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3F6A06855D;\n\tMon, 27 Jan 2025 08:43:31 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 3092461875\n\tfor ;\n\tMon, 27 Jan 2025 08:43:30 +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 9B382352;\n\tMon, 27 Jan 2025 08:42:23 +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=\"ja1B2aXM\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1737963743;\n\tbh=HzadwBB6lx6PyQeevaGhnqRULjpQQAl1q5Uzj6muCVU=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=ja1B2aXMw2MPy6/Ym5xFnvdPHE1tfGS8KE+OloYNWL8G2EfnNx9Y7ysx7zambrf5y\n\tFaos/MNOYrM3t5yNPPuRE7UWXodSx7VTOXD/Hwo5sEhdWXw5gIevU8kKoYsbGFd7zT\n\t4Wgiv/edbf7PeJYHXIoOz4F7lwvjU+1PQRM7vH/s=","Date":"Mon, 27 Jan 2025 09:43:19 +0200","From":"Laurent Pinchart ","To":"Milan Zamazal ","Cc":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ,\n\tlibcamera-devel@lists.libcamera.org, Stanislaw Gruszka\n\t","Subject":"Re: [PATCH] libcamera: software_isp: Handle signals in the proper\n\tthread","Message-ID":"<20250127074319.GC11289@pendragon.ideasonboard.com>","References":"<20241219103108.244243-1-mzamazal@redhat.com>\n\t\n\t<878qsb8jgy.fsf@redhat.com>\n\t<20241219232219.GK19884@pendragon.ideasonboard.com>\n\t<85pll3ycit.fsf@mzamazal-thinkpadp1gen3.tpbc.csb>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<85pll3ycit.fsf@mzamazal-thinkpadp1gen3.tpbc.csb>","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\" "}},{"id":33243,"web_url":"https://patchwork.libcamera.org/comment/33243/","msgid":"<85plk2x0cw.fsf@mzamazal-thinkpadp1gen3.tpbc.csb>","date":"2025-01-31T19:13:51","subject":"Re: [PATCH] libcamera: software_isp: Handle signals in the proper\n\tthread","submitter":{"id":177,"url":"https://patchwork.libcamera.org/api/people/177/","name":"Milan Zamazal","email":"mzamazal@redhat.com"},"content":"Hi Laurent,\n\nLaurent Pinchart writes:\n\n> Hi Milan,\n>\n> On Fri, Jan 03, 2025 at 07:31:54PM +0100, Milan Zamazal wrote:\n>> Laurent Pinchart writes:\n>> > On Thu, Dec 19, 2024 at 10:13:17PM +0100, Milan Zamazal wrote:\n>> >> Barnabás Pőcze writes:\n>> >> > 2024. december 19., csütörtök 11:31 keltezéssel, Milan Zamazal írta:\n>> >> >\n>> >> >> inputBufferReady ready signal in the simple pipeline is handled in the\n>> >> >> pipeline handler thread. Similarly, outputBufferReady and ispStatsReady\n>> >> >> signals should be handled there too, everything should be called from\n>> >> >> the right thread and not block the callers.\n>> >> >> \n>> >> >> Signed-off-by: Milan Zamazal \n>> >> >> ---\n>> >> >> src/libcamera/pipeline/simple/simple.cpp | 9 +++++++--\n>> >> >> 1 file changed, 7 insertions(+), 2 deletions(-)\n>> >> >> \n>> >> >> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n>> >> >> index 8ac24e6e..280112f4 100644\n>> >> >> --- a/src/libcamera/pipeline/simple/simple.cpp\n>> >> >> +++ b/src/libcamera/pipeline/simple/simple.cpp\n>> >> >> @@ -548,12 +548,17 @@ int SimpleCameraData::init()\n>> >> >> \t\t\t * bound explicitly to the pipe, which is bound to the pipeline\n>> >> >> \t\t\t * handler thread. The function then simply forwards the call to\n>> >> >> \t\t\t * conversionInputDone().\n>> >> >> +\t\t\t * Similarly for outputBufferReady and ispStatsReady signals.\n>> >> >> \t\t\t */\n>> >> >> \t\t\tswIsp_->inputBufferReady.connect(pipe, [this](FrameBuffer *buffer) {\n>> >> >> \t\t\t\tthis->conversionInputDone(buffer);\n>> >> >> \t\t\t});\n>> >> >> -\t\t\tswIsp_->outputBufferReady.connect(this, &SimpleCameraData::conversionOutputDone);\n>> >> >> -\t\t\tswIsp_->ispStatsReady.connect(this, &SimpleCameraData::ispStatsReady);\n>> >> >> +\t\t\tswIsp_->outputBufferReady.connect(this, [this](FrameBuffer *buffer) {\n>> >> >> +\t\t\t\tthis->conversionOutputDone(buffer);\n>> >> >> +\t\t\t});\n>> >> >> +\t\t\tswIsp_->ispStatsReady.connect(this, [this](uint32_t frame, uint32_t bufferId) {\n>> >> >> +\t\t\t\tthis->ispStatsReady(frame, bufferId);\n>> >> >> +\t\t\t});\n>> >> >> [...]\n>> >> >\n>> >> > The object is still `this`, so I wouldn't expect any difference\n>> >> > in behaviour. Is there?\n>> >> \n>> >> Ah, right, should be `pipe', will fix it in v2.\n>> >\n>> > I think it would be better to emit the signals from the camera manager\n>> > thread, instead of relying on the user of the soft ISP to perform\n>> > cross-thread synchronization. That will be less error-prone.\n>> \n>> Maybe a stupid question, but how to do it? The signals are emitted from\n>> the corresponding place/thread asynchronously and there is little choice\n>> of the thread to use. At least without passing around some Object bound\n>> to the camera manager thread, which wouldn't be nice either I suppose.\n>> What do I miss?\n>\n> Not a stupid question at all, I should have made this clearer.\n>\n> Signals are emitted from the thread running when the emit() function is\n> called. If a signal is connected to an object that does not inherit from\n> the Object class, the connected slot will be called synchronously, in\n> the same thread.\n>\n> If the connected object inherits from the Object class, the behaviour\n> differs and depends on the connection type, specified when calling the\n> connect() function on the signal:\n>\n> - If the connection type is ConnectionTypeDirect, the slot is called\n> synchronously, in the same thread as the emit() calls.\n>\n> - If the connection type is ConnectionTypeQueued, the slot is called\n> asynchronously, in the thread of the connected object. The emit() call\n> returns immediately, before the slot is called.\n>\n> - If the connection type is ConnectionTypeBlocking, the slot is called\n> synchronously, in the thread of the connected object. The emit() call\n> waits until the synchronous call completes in the other thread before\n> returning. If the emitter and receiver are in the same thread, this\n> behaves like ConnectionTypeDirect.\n>\n> - If the connection type is ConnectionTypeAuto (the default),\n> ConnectionTypeDirect is used if the emitter and receiver are in the\n> same thread, and ConnectionTypeQueued is used otherwise.\n>\n> The SimpleCameraData class does not inherit from Object, so slots are\n> called synchronously, in the thread of the emitter. This is why the\n> inputBufferReady signal is handled with a lambda function:\n>\n> \tswIsp_->inputBufferReady.connect(pipe, [this](FrameBuffer *buffer) {\n> \t\tthis->conversionInputDone(buffer);\n> \t});\n>\n> The first argument to the connect() function is the receiver object,\n> which here inherits from the Object class. As a result, the lambda\n> function is called asynchronusly in the pipeline handler thread, not the\n> soft ISP worker thread.\n>\n> This construct is fragile, as shown by this patch: the outputBufferReady\n> and ispStatsReady signals were mistakenly connected to the camera data\n> object, resulting in the slots being called from the wrong thread. I\n> think it would be better to handle this issue in the soft ISP class, to\n> avoid depending on the implementation of the receiver. One easy way to\n> do so is to make the SoftwareIsp class inherit from the Object class.\n> That way, SoftwareIsp::inputReady() will be called in the thread in\n> which the SoftwareIsp instance lives, which is the pipeline handler\n> thread. The inputBufferReady signal that is emitted from there will\n> always come from the pipeline handler thread.\n\nThank you for thorough explanation. The idea of making SoftwareIsp to\ninherit Object looks fine and working, I'll post v2 with it.","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 2F20BC3263\n\tfor ;\n\tFri, 31 Jan 2025 19:14:01 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 57D4668568;\n\tFri, 31 Jan 2025 20:14:00 +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 56CB060353\n\tfor ;\n\tFri, 31 Jan 2025 20:13:58 +0100 (CET)","from mail-ej1-f71.google.com (mail-ej1-f71.google.com\n\t[209.85.218.71]) by relay.mimecast.com with ESMTP with STARTTLS\n\t(version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id\n\tus-mta-33-cX0YsGLIOnqKuLICk4Nb4Q-1; Fri, 31 Jan 2025 14:13:55 -0500","by mail-ej1-f71.google.com with SMTP id\n\ta640c23a62f3a-ab6930f94b7so278633966b.1\n\tfor ;\n\tFri, 31 Jan 2025 11:13:55 -0800 (PST)","from mzamazal-thinkpadp1gen3.tpbc.csb ([77.48.47.2])\n\tby smtp.gmail.com with ESMTPSA id\n\ta640c23a62f3a-ab6e47f1d82sm334211866b.84.2025.01.31.11.13.52\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tFri, 31 Jan 2025 11:13:52 -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=\"bNIqwekm\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1738350837;\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\tin-reply-to:in-reply-to:references:references;\n\tbh=Qn5Du0PBUJascsKHi58c3PnVIvTICHAg+8bK8NQZc9o=;\n\tb=bNIqwekm7fm0SKY/MxXmLWSOp5vpLG4tgAzgpRWE4lhpGVr7wU/mrclcjGkqH3zPpTK1N6\n\tdtdBYC2/qr1M+vce4ZAeBrXt2992msXbwk9urbm/VITbeWkSTIIDNjAUmoP05FwYAGf8jO\n\t1Z7xiRS5/y6jZ01iMwtvPA4U+Bdf/dw=","X-MC-Unique":"cX0YsGLIOnqKuLICk4Nb4Q-1","X-Mimecast-MFC-AGG-ID":"cX0YsGLIOnqKuLICk4Nb4Q","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1738350834; x=1738955634;\n\th=content-transfer-encoding:mime-version:user-agent:message-id:date\n\t:references:in-reply-to:subject:cc:to:from:x-gm-message-state:from\n\t:to:cc:subject:date:message-id:reply-to;\n\tbh=mY8zrCzum2y4Fe0ggg8T8clwv8FPdnQtAknor9lRUM8=;\n\tb=gTfJrBXqGIiNuRtEdFGh9jRIKKaIj7KkV5OZtinpf/a6TMS0xMLZDlfEnChhoZX3T2\n\t1Bdb+A7EABfKnf5ZMeB5eBa07r6Dtzx/Wc8kh7D7HhzOt+hsUi5RIZpIZqd3sk0tE1Cw\n\tLrgIDBVvxb/bIA/eAo8InslQkKQ0RvDNQ7hh3tB3YsAhlw9DmSnMTmvBQ4NoShrMTLIb\n\tJmOZbqijDnu8YOIwLyU1qdCMwXceSR9yLqgLUk6AOw+2VYE4JfRYzbhBH8/mjoqHsQB+\n\tDEsTVMxEMZYyYmOq9La5ZLfWWaBEZfgfkdq33thdn1AQbpoHgWv5mfFcoiCHPdei9615\n\t6RgQ==","X-Forwarded-Encrypted":"i=1;\n\tAJvYcCWY8Z07Qo7uXT0Q/IYBXoMOILFoRXawKT5AorLxe5Gx2PQmRnapjMb25bXYAhWPRs/6eBF+civJL9y2nVz6NPg=@lists.libcamera.org","X-Gm-Message-State":"AOJu0Yz4ih6c7KEg94QG2wEnX8gP89ngUSdSd/2vp7XmHBjlP0FyfrI4\n\tGGxT94zIDcS51Ghyb2bL58F6l6z1MtwK657UtaNacmvnE735HvuJxkC1TKzHRGKIhhyOErmRRHU\n\tHtSG65MVv77ZuiEiAdozVxrQXU/WA+fUXTzqVYK39NYLffiaP7cwvnIPTT0zzIuqwri9FM/sszm\n\thb9/U=","X-Gm-Gg":"ASbGncuNXc9huX88TszMxyk01NBfl54J5Q77a1ZIh9h+jjVyuAztyd+rxngJPr8D3Fb\n\tcN6CDRSD0YQI0Yy5xC6fPZmj8OCF2SVrkH4WxUXun9i8eJXWODA5xOzJ+JIt1wT88oFi2FbvtAv\n\t2liXXgUKXd8c2ibr+IbMU0Rph8jiJQ7XgoZm24XIWB+dgw8/OpFuYGA47GQJF+gpriElt6iaqTb\n\t2rdt7o12wNX0Sey8d1k5wAYMVq9N5bjSjE0iEQGlUnkvTEQgEaDfftF07j3KJNJISbXcKynXA+m\n\tRI/x+qVDLNgu9qbibcE7isSuRGU=","X-Received":["by 2002:a17:907:60ce:b0:ab6:efc5:5d73 with SMTP id\n\ta640c23a62f3a-ab6efc579a5mr798499466b.24.1738350834127; \n\tFri, 31 Jan 2025 11:13:54 -0800 (PST)","by 2002:a17:907:60ce:b0:ab6:efc5:5d73 with SMTP id\n\ta640c23a62f3a-ab6efc579a5mr798495666b.24.1738350833633; \n\tFri, 31 Jan 2025 11:13:53 -0800 (PST)"],"X-Google-Smtp-Source":"AGHT+IFbOtE0I7erHlGfKg0DjXLCmmXGrjIZqvgkoA6dxsKUPLuKXhsBXCqt3iXiRQ6VdUq+VttTDg==","From":"Milan Zamazal ","To":"Laurent Pinchart ","Cc":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ,\n\tlibcamera-devel@lists.libcamera.org, Stanislaw Gruszka\n\t","Subject":"Re: [PATCH] libcamera: software_isp: Handle signals in the proper\n\tthread","In-Reply-To":"<20250127074319.GC11289@pendragon.ideasonboard.com> (Laurent\n\tPinchart's message of \"Mon, 27 Jan 2025 09:43:19 +0200\")","References":"<20241219103108.244243-1-mzamazal@redhat.com>\n\t\n\t<878qsb8jgy.fsf@redhat.com>\n\t<20241219232219.GK19884@pendragon.ideasonboard.com>\n\t<85pll3ycit.fsf@mzamazal-thinkpadp1gen3.tpbc.csb>\n\t<20250127074319.GC11289@pendragon.ideasonboard.com>","Date":"Fri, 31 Jan 2025 20:13:51 +0100","Message-ID":"<85plk2x0cw.fsf@mzamazal-thinkpadp1gen3.tpbc.csb>","User-Agent":"Gnus/5.13 (Gnus v5.13)","MIME-Version":"1.0","X-Mimecast-Spam-Score":"0","X-Mimecast-MFC-PROC-ID":"i1cElkFU0fIb1Xk59h3XHefsT_zdJxfb0ClF8OKLeV8_1738350835","X-Mimecast-Originator":"redhat.com","Content-Type":"text/plain; charset=utf-8","Content-Transfer-Encoding":"quoted-printable","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\" "}}]