[{"id":32875,"web_url":"https://patchwork.libcamera.org/comment/32875/","msgid":"","date":"2024-12-18T14:29:07","subject":"Re: [PATCH v2] pipeline: simple: Use proper device for frame start\n\tevents","submitter":{"id":102,"url":"https://patchwork.libcamera.org/api/people/102/","name":"Hans de Goede","email":"hdegoede@redhat.com"},"content":"Hi,\n\nOn 18-Dec-24 3:22 PM, Stanislaw Gruszka wrote:\n> Currently we use frame start event from video capture device to\n> apply controls. But the capture device might not generate the events.\n> Usually CSI-2 receiver is proper device to subscribe for start\n> frame events.\n> \n> Without DelayedConntrols:applyControls() is possible that we can get\n> call to DelayedControls::get() with frame number that exceed number\n> of saved entries and get below assertion failure:\n> \n> ../src/libcamera/delayed_controls.cpp:227:\n> libcamera::ControlList libcamera::DelayedControls::get(uint32_t):\n> Assertion `info.type() != ControlTypeNone' failed\n> \n> Assertion failure can happen at the beginning of streaming when\n> ControlRingBuffer is not yet filled and there are errors on CSI-2.\n> \n> To fix, loop over devices in the pipeline (starting from the last one),\n> find one that emits start frame events and connect applyControls()\n> to it. Additionally remove direct call to sensor_->setControls() if\n> the emitter device was found.\n> \n> Bug: https://bugs.libcamera.org/show_bug.cgi?id=241\n> Co-developed-by: Hans de Goede \n> Signed-off-by: Hans de Goede \n> Signed-off-by: Stanislaw Gruszka \n\nThanks, patch looks good to me:\n\nReviewed-by: Hans de Goede \n\nwhich is a bit weird since Stanislaw gave me Co-developed-by credits,\nbut still...\n\nAlso:\n\nTested-by: Hans de Goede # Lenovo X1 Yoga IPU6 + ov2740\n\nRegards,\n\nHans\n\n\n\n\n> ---\n> v1 -> v2:\n> - make eventEmitter_ subdevice part of SimpleCameraData\n> - add debug log when found event emitter device\n> - nullify eventEmitter_ on stop\n> - remove direct sensor_->setControls()\n> - add delayedCtrls_->reset() on start\n> \n> src/libcamera/pipeline/simple/simple.cpp | 39 +++++++++++++++++++++---\n> 1 file changed, 35 insertions(+), 4 deletions(-)\n> \n> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n> index 8ac24e6e..a7594c2c 100644\n> --- a/src/libcamera/pipeline/simple/simple.cpp\n> +++ b/src/libcamera/pipeline/simple/simple.cpp\n> @@ -277,6 +277,7 @@ public:\n> \tstd::list entities_;\n> \tstd::unique_ptr sensor_;\n> \tV4L2VideoDevice *video_;\n> +\tV4L2Subdevice *eventEmitter_;\n> \n> \tstd::vector configs_;\n> \tstd::map> formats_;\n> @@ -911,8 +912,11 @@ void SimpleCameraData::ispStatsReady(uint32_t frame, uint32_t bufferId)\n> void SimpleCameraData::setSensorControls(const ControlList &sensorControls)\n> {\n> \tdelayedCtrls_->push(sensorControls);\n> -\tControlList ctrls(sensorControls);\n> -\tsensor_->setControls(&ctrls);\n> +\t/* Directly apply controls now if there is no frameStart signal */\n> +\tif (!eventEmitter_) {\n> +\t\tControlList ctrls(sensorControls);\n> +\t\tsensor_->setControls(&ctrls);\n> +\t}\n> }\n> \n> /* Retrieve all source pads connected to a sink pad through active routes. */\n> @@ -1299,8 +1303,6 @@ int SimplePipelineHandler::configure(Camera *camera, CameraConfiguration *c)\n> \tdata->delayedCtrls_ =\n> \t\tstd::make_unique(data->sensor_->device(),\n> \t\t\t\t\t\t params);\n> -\tdata->video_->frameStart.connect(data->delayedCtrls_.get(),\n> -\t\t\t\t\t &DelayedControls::applyControls);\n> \n> \tStreamConfiguration inputCfg;\n> \tinputCfg.pixelFormat = pipeConfig->captureFormat;\n> @@ -1368,6 +1370,28 @@ int SimplePipelineHandler::start(Camera *camera, [[maybe_unused]] const ControlL\n> \n> \tvideo->bufferReady.connect(data, &SimpleCameraData::imageBufferReady);\n> \n> +\t/*\n> +\t * Enable frame start event on last device in the pipeline\n> +\t * that provides the events.\n> +\t */\n> +\tfor (auto it = data->entities_.rbegin(); it != data->entities_.rend(); ++it) {\n> +\t\tV4L2Subdevice *sd = subdev(it->entity);\n> +\t\tif (!sd)\n> +\t\t\tcontinue;\n> +\t\tif (sd->setFrameStartEnabled(true) < 0)\n> +\t\t\tcontinue;\n> +\n> +\t\tLOG(SimplePipeline, Debug)\n> +\t\t\t<< \"Using \" << it->entity->name() << \" frameStart signal\";\n> +\n> +\t\tsd->frameStart.connect(data->delayedCtrls_.get(),\n> +\t\t\t\t &DelayedControls::applyControls);\n> +\t\tdata->eventEmitter_ = sd;\n> +\t\tbreak;\n> +\t}\n> +\n> +\tdata->delayedCtrls_->reset();\n> +\n> \tret = video->streamOn();\n> \tif (ret < 0) {\n> \t\tstop(camera);\n> @@ -1400,6 +1424,13 @@ void SimplePipelineHandler::stopDevice(Camera *camera)\n> \tSimpleCameraData *data = cameraData(camera);\n> \tV4L2VideoDevice *video = data->video_;\n> \n> +\tif (data->eventEmitter_) {\n> +\t\tdata->eventEmitter_->setFrameStartEnabled(false);\n> +\t\tdata->eventEmitter_->frameStart.disconnect(data->delayedCtrls_.get(),\n> +\t\t\t\t\t\t\t &DelayedControls::applyControls);\n> +\t\tdata->eventEmitter_ = NULL;\n> +\t}\n> +\n> \tif (data->useConversion_) {\n> \t\tif (data->converter_)\n> \t\t\tdata->converter_->stop();\n> --\n> 2.43.0\n>","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 ACB12C32FE\n\tfor ;\n\tWed, 18 Dec 2024 14:29:15 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B3B066809F;\n\tWed, 18 Dec 2024 15:29:14 +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 C2ACB67F59\n\tfor ;\n\tWed, 18 Dec 2024 15:29:12 +0100 (CET)","from mail-ej1-f72.google.com (mail-ej1-f72.google.com\n\t[209.85.218.72]) by relay.mimecast.com with ESMTP with STARTTLS\n\t(version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id\n\tus-mta-689-7BjG26EAPh-TVCGutT9BZQ-1; Wed, 18 Dec 2024 09:29:10 -0500","by mail-ej1-f72.google.com with SMTP id\n\ta640c23a62f3a-aa6732a1af5so590488266b.3\n\tfor ;\n\tWed, 18 Dec 2024 06:29:10 -0800 (PST)","from ?IPV6:2001:1c00:c32:7800:5bfa:a036:83f0:f9ec?\n\t(2001-1c00-0c32-7800-5bfa-a036-83f0-f9ec.cable.dynamic.v6.ziggo.nl.\n\t[2001:1c00:c32:7800:5bfa:a036:83f0:f9ec])\n\tby smtp.gmail.com with ESMTPSA id\n\ta640c23a62f3a-aab963595e7sm562780266b.115.2024.12.18.06.29.07\n\t(version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128);\n\tWed, 18 Dec 2024 06:29:08 -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=\"VL8D0SlA\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1734532151;\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=bE+/s1DHMxc7yzMbFwPQHpana+qJE1SdcFOg0S5XNOI=;\n\tb=VL8D0SlAUb/AGx33PsNcIVSVBvCSa+jN5gtkCfNUFHkvxtPMlpZ8SSDwFZgvFq0dEH1I55\n\tIHRWznqz9N7iHojyoxRhjoPr+lsF3D/yjLmNoLS5CWCiNQOqA1pPknOan9syWdjibtmgiu\n\t7IgkGk77U+rwmbUt23cHyicUzDVJiuw=","X-MC-Unique":"7BjG26EAPh-TVCGutT9BZQ-1","X-Mimecast-MFC-AGG-ID":"7BjG26EAPh-TVCGutT9BZQ","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1734532149; x=1735136949;\n\th=content-transfer-encoding:in-reply-to:from:content-language\n\t:references:cc:to:subject:user-agent:mime-version:date:message-id\n\t:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to;\n\tbh=bE+/s1DHMxc7yzMbFwPQHpana+qJE1SdcFOg0S5XNOI=;\n\tb=pVRG2F84m+MvPcYlc+q+p2svAIwI/RS8KZoU27jB5NROXEhle7ThbCaLhQjPcpPAnu\n\tHd3dnFBC2oaI3K+i9097CoK6sFSjeDRpj+75XkCmsXRt/Iru5Xip6vnCCUt+68I6T0LD\n\tt1FTF5Jctll2m8iokpvOEWILq0yanCSpoA/40DTUxv9U2+ucKbTq17fEHLmnUysxVx9U\n\taPvCYWJknPEfpQyC9/iCITfc1rRJvNG8hzUVeJq353wGiUjw4EPJSJgfv1UCqNhfmU8z\n\tXXBY+dab1UOjbm5p5YPwp/rgX3Z3H96TwViXw1QnJSGXE1/XL+A92E71hXvXPXDLNDJy\n\tGT4g==","X-Forwarded-Encrypted":"i=1;\n\tAJvYcCU6sy8erzMf853l1/ykE+xCbxWT16U/22h2q6SkBImJ9saQmdgSBKa/8YB6v/IZwNIsamcW0/JKbB8k4Df5fBk=@lists.libcamera.org","X-Gm-Message-State":"AOJu0YwUWV5X6GuyS+jt80nl6vTzkL9ftAUbOnJKUsbJ2i4yFqEP4I0D\n\t+whKwMwPxOHhkMYQGcUBcWDSpfwAH6h1fmwkkkqpVLHTW3y8d2qNtyjiHzX62jJNyHtuaLj+RQU\n\tC2PW3xNXGBNqnuE0T2pO51M2+dWlhg7ayJC1lS82c2riWqdQVaREYN41v1XQkUCRZMkYHoFs=","X-Gm-Gg":"ASbGncvloe1SQgasGAS3qoHhtzHvF/2Kl1cCuwfl73GmB7S0itoGD2RkFNH+JyC2fj4\n\trL8TP4W3UCoVmoYhd4AIuckU+SWM3yTUkTv/nzT6HLiCy3NOMhzEjl/wB7bRJpuW+B7VuegfZF2\n\t/dcUB+c+AfmE9eCTM60UzhSzqCefMZEGEmfLC24Z+WNTWSmK5sK5r0JGzCyjLo2JjhJfWcfhEhj\n\tzWrqAvVwzTUqC9JVCXUdFm/06ylxA9+zNnkrftHCLSBFi7wHz0YEikN4U3+rr4iOeWBgbXV6rjI\n\tdq0yx032JkaHmdbV6NPCRDpjWaSIDHPai5SnhyDUOP8B2mRoExoJAlP4XGMh6qISz7nRbhwkMfO\n\tczs2F22IraH3zQCbtkac2xGF99dwWb0s=","X-Received":["by 2002:a17:907:961a:b0:aa6:98c9:aadc with SMTP id\n\ta640c23a62f3a-aabf47a7572mr237590166b.31.1734532149087; \n\tWed, 18 Dec 2024 06:29:09 -0800 (PST)","by 2002:a17:907:961a:b0:aa6:98c9:aadc with SMTP id\n\ta640c23a62f3a-aabf47a7572mr237587566b.31.1734532148533; \n\tWed, 18 Dec 2024 06:29:08 -0800 (PST)"],"X-Google-Smtp-Source":"AGHT+IF7pVayGFQAnWAQVcwL41R/bk7TwxvI/xtxLUUARoODx7bY0s1/rHdr+pzyEgaXih7l+Q6u8w==","Message-ID":"","Date":"Wed, 18 Dec 2024 15:29:07 +0100","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v2] pipeline: simple: Use proper device for frame start\n\tevents","To":"Stanislaw Gruszka ,\n\tlibcamera-devel@lists.libcamera.org","Cc":"Milan Zamazal ,\n\tLaurent Pinchart ,\n\tKieran Bingham ,\n\tNaushir Patuck ,\n\tSakari Ailus ","References":"<20241218142217.437842-1-stanislaw.gruszka@linux.intel.com>","From":"Hans de Goede ","In-Reply-To":"<20241218142217.437842-1-stanislaw.gruszka@linux.intel.com>","X-Mimecast-Spam-Score":"0","X-Mimecast-MFC-PROC-ID":"nLeT_8gwyBqZOcNZPYUvrtlQ5M08owM4rqiHCPKV0RI_1734532149","X-Mimecast-Originator":"redhat.com","Content-Language":"en-US, nl","Content-Type":"text/plain; charset=UTF-8","Content-Transfer-Encoding":"7bit","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":32930,"web_url":"https://patchwork.libcamera.org/comment/32930/","msgid":"<85ttafycuz.fsf@mzamazal-thinkpadp1gen3.tpbc.csb>","date":"2025-01-03T18:24:36","subject":"Re: [PATCH v2] pipeline: simple: Use proper device for frame start\n\tevents","submitter":{"id":177,"url":"https://patchwork.libcamera.org/api/people/177/","name":"Milan Zamazal","email":"mzamazal@redhat.com"},"content":"Hi Stanislaw,\n\nthank you for the fix.\n\nStanislaw Gruszka writes:\n\n> Currently we use frame start event from video capture device to\n> apply controls. But the capture device might not generate the events.\n> Usually CSI-2 receiver is proper device to subscribe for start\n> frame events.\n>\n> Without DelayedConntrols:applyControls() is possible that we can get\n> call to DelayedControls::get() with frame number that exceed number\n> of saved entries and get below assertion failure:\n>\n> ../src/libcamera/delayed_controls.cpp:227:\n> libcamera::ControlList libcamera::DelayedControls::get(uint32_t):\n> Assertion `info.type() != ControlTypeNone' failed\n>\n> Assertion failure can happen at the beginning of streaming when\n> ControlRingBuffer is not yet filled and there are errors on CSI-2.\n>\n> To fix, loop over devices in the pipeline (starting from the last one),\n> find one that emits start frame events and connect applyControls()\n> to it. Additionally remove direct call to sensor_->setControls() if\n> the emitter device was found.\n>\n> Bug: https://bugs.libcamera.org/show_bug.cgi?id=241\n> Co-developed-by: Hans de Goede \n> Signed-off-by: Hans de Goede \n> Signed-off-by: Stanislaw Gruszka \n\nAFAICT:\n\nReviewed-by: Milan Zamazal \n\n> ---\n> v1 -> v2:\n> - make eventEmitter_ subdevice part of SimpleCameraData\n> - add debug log when found event emitter device\n> - nullify eventEmitter_ on stop\n> - remove direct sensor_->setControls()\n> - add delayedCtrls_->reset() on start\n>\n> src/libcamera/pipeline/simple/simple.cpp | 39 +++++++++++++++++++++---\n> 1 file changed, 35 insertions(+), 4 deletions(-)\n>\n> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n> index 8ac24e6e..a7594c2c 100644\n> --- a/src/libcamera/pipeline/simple/simple.cpp\n> +++ b/src/libcamera/pipeline/simple/simple.cpp\n> @@ -277,6 +277,7 @@ public:\n> \tstd::list entities_;\n> \tstd::unique_ptr sensor_;\n> \tV4L2VideoDevice *video_;\n> +\tV4L2Subdevice *eventEmitter_;\n>\n> \tstd::vector configs_;\n> \tstd::map> formats_;\n> @@ -911,8 +912,11 @@ void SimpleCameraData::ispStatsReady(uint32_t frame, uint32_t bufferId)\n> void SimpleCameraData::setSensorControls(const ControlList &sensorControls)\n> {\n> \tdelayedCtrls_->push(sensorControls);\n> -\tControlList ctrls(sensorControls);\n> -\tsensor_->setControls(&ctrls);\n> +\t/* Directly apply controls now if there is no frameStart signal */\n> +\tif (!eventEmitter_) {\n> +\t\tControlList ctrls(sensorControls);\n> +\t\tsensor_->setControls(&ctrls);\n> +\t}\n> }\n>\n> /* Retrieve all source pads connected to a sink pad through active routes. */\n> @@ -1299,8 +1303,6 @@ int SimplePipelineHandler::configure(Camera *camera, CameraConfiguration *c)\n> \tdata->delayedCtrls_ =\n> \t\tstd::make_unique(data->sensor_->device(),\n> \t\t\t\t\t\t params);\n> -\tdata->video_->frameStart.connect(data->delayedCtrls_.get(),\n> -\t\t\t\t\t &DelayedControls::applyControls);\n>\n> \tStreamConfiguration inputCfg;\n> \tinputCfg.pixelFormat = pipeConfig->captureFormat;\n> @@ -1368,6 +1370,28 @@ int SimplePipelineHandler::start(Camera *camera, [[maybe_unused]] const ControlL\n>\n> \tvideo->bufferReady.connect(data, &SimpleCameraData::imageBufferReady);\n>\n> +\t/*\n> +\t * Enable frame start event on last device in the pipeline\n> +\t * that provides the events.\n> +\t */\n> +\tfor (auto it = data->entities_.rbegin(); it != data->entities_.rend(); ++it) {\n> +\t\tV4L2Subdevice *sd = subdev(it->entity);\n> +\t\tif (!sd)\n> +\t\t\tcontinue;\n> +\t\tif (sd->setFrameStartEnabled(true) < 0)\n> +\t\t\tcontinue;\n> +\n> +\t\tLOG(SimplePipeline, Debug)\n> +\t\t\t<< \"Using \" << it->entity->name() << \" frameStart signal\";\n> +\n> +\t\tsd->frameStart.connect(data->delayedCtrls_.get(),\n> +\t\t\t\t &DelayedControls::applyControls);\n> +\t\tdata->eventEmitter_ = sd;\n> +\t\tbreak;\n> +\t}\n> +\n> +\tdata->delayedCtrls_->reset();\n> +\n> \tret = video->streamOn();\n> \tif (ret < 0) {\n> \t\tstop(camera);\n> @@ -1400,6 +1424,13 @@ void SimplePipelineHandler::stopDevice(Camera *camera)\n> \tSimpleCameraData *data = cameraData(camera);\n> \tV4L2VideoDevice *video = data->video_;\n>\n> +\tif (data->eventEmitter_) {\n> +\t\tdata->eventEmitter_->setFrameStartEnabled(false);\n> +\t\tdata->eventEmitter_->frameStart.disconnect(data->delayedCtrls_.get(),\n> +\t\t\t\t\t\t\t &DelayedControls::applyControls);\n> +\t\tdata->eventEmitter_ = NULL;\n> +\t}\n> +\n> \tif (data->useConversion_) {\n> \t\tif (data->converter_)\n> \t\t\tdata->converter_->stop();\n> --\n> 2.43.0","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 3DE66C0F1B\n\tfor ;\n\tFri, 3 Jan 2025 18:24:46 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5682E61886;\n\tFri, 3 Jan 2025 19:24:45 +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 2AFEE61886\n\tfor ;\n\tFri, 3 Jan 2025 19:24:44 +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-6-qhPSY428N0WUa8bM4ASH7w-1; Fri, 03 Jan 2025 13:24:41 -0500","by mail-wm1-f71.google.com with SMTP id\n\t5b1f17b1804b1-4361fc2b2d6so31717485e9.3\n\tfor ;\n\tFri, 03 Jan 2025 10:24:41 -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-4364a3766dcsm393747115e9.0.2025.01.03.10.24.37\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tFri, 03 Jan 2025 10:24:38 -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=\"BHAisi7L\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1735928682;\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=IzqSJ9Qr2ggk7GhEdmMNS70J5vMfXY6K4dhLnbhDlCw=;\n\tb=BHAisi7L26k4gppB5ObsmhdvdxIRGokp9b3J0o0tTB0iTg7YQDkz/KRgoah5v6KcHAUW1m\n\tSgKawn9wA6ODvOQZIdlExP805nn5Ia1lbKhxz52gsmtCqy+92AE+WgwVTfJtO8h0Upx7F4\n\t8MmTcbxngfGh7fVtg+wbrEV8/4Dk/pU=","X-MC-Unique":"qhPSY428N0WUa8bM4ASH7w-1","X-Mimecast-MFC-AGG-ID":"qhPSY428N0WUa8bM4ASH7w","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1735928680; x=1736533480;\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=IzqSJ9Qr2ggk7GhEdmMNS70J5vMfXY6K4dhLnbhDlCw=;\n\tb=NEqq8/v0ZHJKgWrpWwfWzZ23TvqaWtGgGByuSCITBfmwt7AyR4A3Iu5Pvv1dnSaiQQ\n\td64YpcReHt1JhNOD1A92t8PpnstdPRNISNV5YvH2FYZt8yJPKG2Rjg8BLbHdoTWIhV03\n\t4O20kEoUiCwI79EVCzr4wyKqCbXxubwD8Dv63EZZrnElGY3dY5gUXcCQW6iZ0aEI8XvJ\n\t5BoVh1GAC0NdzGGD2klbij4Ds9YTuhaqveC20oHAFN1UB5oUVyRAZDH4xqWgVkG6e9cg\n\tJxzl/ujyERxQmoPSKoY0zrlZDm3xJdFLxRFHGczGtjh7nY7aQB3FsDYzQheHBOe7HiKp\n\t4Wiw==","X-Gm-Message-State":"AOJu0YzmmvmCw6EssarBqShcinRzJ5ONTdVGmi4dmSURQQhqPjDIe2ns\n\tO/pHZ2059qI68KEuVuAhrCzSuGy4KUFNgizk1iSk9viY7nYNzKzsNu43AAl37NJxF8AaK9WSSfG\n\trN54vRQB6YxHFSpoqNHiWQhIZFQkJ8gmRno/x7nHTmOq9EL5zGYMeKVUDv4ttK1RZDWuHZBE=","X-Gm-Gg":"ASbGnctUOKhXgvH/EDzw1LJxWZC1VM3+ltOXb3PaFKuayRrbCIwoBnRWnLIFmg3dRFO\n\tqQJ4ygYJMl5gsisVnw+fchxGFk98/YaZp+8dFhNO/rp2AhKOtZQJUMVY8fz3uRU8HAkkwOK+7eW\n\tW3ZPY1F+v9uPBDkk1D6J7lnGTb8lmx0iIHD5yuoDkPpxEH71p82p859PSXwg0B3ELmo92e5qTG5\n\tSdcTbgQg4IVE0+Zx+e+zTD0y/luHHfaBIteqi7tzer5HY9KqGsoxB3JSs0fQ0Nzuojv09cJvv1h\n\t4p2EnBUREkZF54V+IkSX17vxu2A2iYZf9w==","X-Received":["by 2002:a05:600c:b4c:b0:434:f1d5:1453 with SMTP id\n\t5b1f17b1804b1-43673f02a1amr426749355e9.0.1735928680165; \n\tFri, 03 Jan 2025 10:24:40 -0800 (PST)","by 2002:a05:600c:b4c:b0:434:f1d5:1453 with SMTP id\n\t5b1f17b1804b1-43673f02a1amr426749145e9.0.1735928679750; \n\tFri, 03 Jan 2025 10:24:39 -0800 (PST)"],"X-Google-Smtp-Source":"AGHT+IHOcyustx/S4loXq/ZRiXLkEkPAOF3sTdjMXZaY9UXid3tZXUfPnTLfX2+sMxcvS2W6e42dJA==","From":"Milan Zamazal ","To":"Stanislaw Gruszka ","Cc":"libcamera-devel@lists.libcamera.org, Laurent Pinchart\n\t, Kieran Bingham\n\t, Naushir Patuck\n\t, Sakari Ailus , \n\tHans de Goede ","Subject":"Re: [PATCH v2] pipeline: simple: Use proper device for frame start\n\tevents","In-Reply-To":"<20241218142217.437842-1-stanislaw.gruszka@linux.intel.com>\n\t(Stanislaw Gruszka's message of \"Wed, 18 Dec 2024 15:22:17 +0100\")","References":"<20241218142217.437842-1-stanislaw.gruszka@linux.intel.com>","Date":"Fri, 03 Jan 2025 19:24:36 +0100","Message-ID":"<85ttafycuz.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":"iwk2G1V3vwET6U9mQpI9vs38wtyG5GUD9eocleE342w_1735928680","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":"","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":33241,"web_url":"https://patchwork.libcamera.org/comment/33241/","msgid":"<173827488121.2362053.2810990589873058843@ping.linuxembedded.co.uk>","date":"2025-01-30T22:08:01","subject":"Re: [PATCH v2] pipeline: simple: Use proper device for frame start\n\tevents","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Stanislaw,\n\nQuoting Stanislaw Gruszka (2024-12-18 14:22:17)\n> Currently we use frame start event from video capture device to\n> apply controls. But the capture device might not generate the events.\n> Usually CSI-2 receiver is proper device to subscribe for start\n> frame events.\n> \n> Without DelayedConntrols:applyControls() is possible that we can get\n> call to DelayedControls::get() with frame number that exceed number\n> of saved entries and get below assertion failure:\n> \n> ../src/libcamera/delayed_controls.cpp:227:\n> libcamera::ControlList libcamera::DelayedControls::get(uint32_t):\n> Assertion `info.type() != ControlTypeNone' failed\n> \n> Assertion failure can happen at the beginning of streaming when\n> ControlRingBuffer is not yet filled and there are errors on CSI-2.\n> \n> To fix, loop over devices in the pipeline (starting from the last one),\n> find one that emits start frame events and connect applyControls()\n> to it. Additionally remove direct call to sensor_->setControls() if\n> the emitter device was found.\n> \n> Bug: https://bugs.libcamera.org/show_bug.cgi?id=241\n> Co-developed-by: Hans de Goede \n> Signed-off-by: Hans de Goede \n> Signed-off-by: Stanislaw Gruszka \n> ---\n> v1 -> v2:\n> - make eventEmitter_ subdevice part of SimpleCameraData\n> - add debug log when found event emitter device\n> - nullify eventEmitter_ on stop\n> - remove direct sensor_->setControls()\n> - add delayedCtrls_->reset() on start\n> \n> src/libcamera/pipeline/simple/simple.cpp | 39 +++++++++++++++++++++---\n> 1 file changed, 35 insertions(+), 4 deletions(-)\n> \n> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n> index 8ac24e6e..a7594c2c 100644\n> --- a/src/libcamera/pipeline/simple/simple.cpp\n> +++ b/src/libcamera/pipeline/simple/simple.cpp\n> @@ -277,6 +277,7 @@ public:\n> std::list entities_;\n> std::unique_ptr sensor_;\n> V4L2VideoDevice *video_;\n> + V4L2Subdevice *eventEmitter_;\n> \n> std::vector configs_;\n> std::map> formats_;\n> @@ -911,8 +912,11 @@ void SimpleCameraData::ispStatsReady(uint32_t frame, uint32_t bufferId)\n> void SimpleCameraData::setSensorControls(const ControlList &sensorControls)\n> {\n> delayedCtrls_->push(sensorControls);\n> - ControlList ctrls(sensorControls);\n> - sensor_->setControls(&ctrls);\n> + /* Directly apply controls now if there is no frameStart signal */\n> + if (!eventEmitter_) {\n> + ControlList ctrls(sensorControls);\n> + sensor_->setControls(&ctrls);\n> + }\n> }\n> \n> /* Retrieve all source pads connected to a sink pad through active routes. */\n> @@ -1299,8 +1303,6 @@ int SimplePipelineHandler::configure(Camera *camera, CameraConfiguration *c)\n> data->delayedCtrls_ =\n> std::make_unique(data->sensor_->device(),\n> params);\n> - data->video_->frameStart.connect(data->delayedCtrls_.get(),\n> - &DelayedControls::applyControls);\n> \n> StreamConfiguration inputCfg;\n> inputCfg.pixelFormat = pipeConfig->captureFormat;\n> @@ -1368,6 +1370,28 @@ int SimplePipelineHandler::start(Camera *camera, [[maybe_unused]] const ControlL\n> \n> video->bufferReady.connect(data, &SimpleCameraData::imageBufferReady);\n> \n> + /*\n> + * Enable frame start event on last device in the pipeline\n> + * that provides the events.\n> + */\n> + for (auto it = data->entities_.rbegin(); it != data->entities_.rend(); ++it) {\n> + V4L2Subdevice *sd = subdev(it->entity);\n> + if (!sd)\n> + continue;\n> + if (sd->setFrameStartEnabled(true) < 0)\n> + continue;\n> +\n> + LOG(SimplePipeline, Debug)\n> + << \"Using \" << it->entity->name() << \" frameStart signal\";\n> +\n> + sd->frameStart.connect(data->delayedCtrls_.get(),\n> + &DelayedControls::applyControls);\n> + data->eventEmitter_ = sd;\n> + break;\n> + }\n> +\n> + data->delayedCtrls_->reset();\n> +\n> ret = video->streamOn();\n> if (ret < 0) {\n> stop(camera);\n> @@ -1400,6 +1424,13 @@ void SimplePipelineHandler::stopDevice(Camera *camera)\n> SimpleCameraData *data = cameraData(camera);\n> V4L2VideoDevice *video = data->video_;\n> \n> + if (data->eventEmitter_) {\n> + data->eventEmitter_->setFrameStartEnabled(false);\n\nThere's a segfault here when stopping the pipeline/shutting down.\nRunning qcam and closing the window exhbits the issue very reproducibly:\n\n\nThread 10 \"qcam\" received signal SIGSEGV, Segmentation fault.\n[Switching to Thread 0xffffd97aec80 (LWP 36600)]\n0x0000fffff7cb8554 in libcamera::V4L2Device::setFrameStartEnabled (this=0x40, enable=false) at ../../src/libcamera/v4l2_device.cpp:463\n463\t\tif (frameStartEnabled_ == enable)\nMissing rpms, try: dnf --enablerepo='*debug*' install libtiff-debuginfo-4.7.0-3.fc42.aarch64 qt6-qtbase-debuginfo-6.8.1-11.fc42.aarch64 qt6-qtbase-gui-debuginfo-6.8.1-11.fc42.aarch64 libstdc++-debuginfo-15.0.1-0.4.fc42.aarch64 libgcc-debuginfo-15.0.1-0.4.fc42.aarch64 glibc-debuginfo-2.40.9000-35.fc42.aarch64 libjpeg-turbo-debuginfo-3.1.0-2.fc42.aarch64 gnutls-debuginfo-3.8.8-2.fc42.aarch64 systemd-libs-debuginfo-257.2-17.fc42.aarch64 libyaml-debuginfo-0.2.5-16.fc42.aarch64 elfutils-libs-debuginfo-0.192-8.fc42.aarch64 libunwind-debuginfo-1.8.0-5.fc42.aarch64 libwebp-debuginfo-1.5.0-2.fc42.aarch64 libzstd-debuginfo-1.5.6-3.fc42.aarch64 liblerc-debuginfo-4.0.0-8.fc42.aarch64 jbigkit-libs-debuginfo-2.1-31.fc42.aarch64 zlib-ng-compat-debuginfo-2.2.3-2.fc42.aarch64 libicu-debuginfo-76.1-4.fc42.aarch64 glib2-debuginfo-2.83.2-6.fc42.aarch64 double-conversion-debuginfo-3.3.0-5.fc42.aarch64 libb2-debuginfo-0.98.1-13.fc42.aarch64 pcre2-utf16-debuginfo-10.44-1.fc42.2.aarch64 openssl-libs-debuginfo-3.2.2-13.fc42.aarch64 libglvnd-egl-debuginfo-1.7.0-7.fc42.aarch64 fontconfig-debuginfo-2.16.0-2.fc42.aarch64 libX11-debuginfo-1.8.10-3.fc42.aarch64 libxkbcommon-debuginfo-1.7.0-6.fc42.aarch64 libglvnd-glx-debuginfo-1.7.0-7.fc42.aarch64 libglvnd-opengl-debuginfo-1.7.0-7.fc42.aarch64 libpng-debuginfo-1.6.44-2.fc42.aarch64 harfbuzz-debuginfo-10.2.0-2.fc42.aarch64 freetype-debuginfo-2.13.3-2.fc42.aarch64 p11-kit-debuginfo-0.25.5-5.fc42.aarch64 libidn2-debuginfo-2.3.7-3.fc42.aarch64 libunistring-debuginfo-1.1-9.fc42.aarch64 libtasn1-debuginfo-4.19.0-11.fc42.aarch64 nettle-debuginfo-3.10-6.fc42.aarch64 gmp-debuginfo-6.3.0-2.fc41.aarch64 libcap-debuginfo-2.73-2.fc42.aarch64 elfutils-libelf-debuginfo-0.192-8.fc42.aarch64 xz-libs-debuginfo-5.6.3-3.fc42.aarch64 bzip2-libs-debuginfo-1.0.8-20.fc42.aarch64 pcre2-debuginfo-10.44-1.fc42.2.aarch64 libgomp-debuginfo-15.0.1-0.4.fc42.aarch64 libglvnd-debuginfo-1.7.0-7.fc42.aarch64 libxml2-debuginfo-2.12.9-2.fc42.aarch64 libxcb-debuginfo-1.17.0-5.fc42.aarch64 dbus-libs-debuginfo-1.16.0-3.fc42.aarch64 libXext-debuginfo-1.3.6-3.fc42.aarch64 graphite2-debuginfo-1.3.14-18.fc42.aarch64 libbrotli-debuginfo-1.1.0-6.fc42.aarch64 libffi-debuginfo-3.4.6-5.fc42.aarch64 libXau-debuginfo-1.0.12-2.fc42.aarch64 qt6-qtwayland-debuginfo-6.8.1-3.fc42.aarch64 libwayland-client-debuginfo-1.23.0-3.fc42.aarch64 libwayland-cursor-debuginfo-1.23.0-3.fc42.aarch64 gtk3-debuginfo-3.24.43-3.fc42.aarch64 pango-debuginfo-1.56.1-1.fc42.aarch64 gdk-pixbuf2-debuginfo-2.42.12-10.fc42.aarch64 cairo-debuginfo-1.18.2-3.fc42.aarch64 fribidi-debuginfo-1.0.16-2.fc42.aarch64 cairo-gobject-debuginfo-1.18.2-3.fc42.aarch64 atk-debuginfo-2.55.0.1-2.fc42.aarch64 libepoxy-debuginfo-1.5.10-9.fc42.aarch64 libXi-debuginfo-1.8.2-2.fc42.aarch64 at-spi2-atk-debuginfo-2.55.0.1-2.fc42.aarch64 libcloudproviders-debuginfo-0.3.5-6.fc42.aarch64 libtracker-sparql-debuginfo-3.7.3-5.fc42.aarch64 libXfixes-debuginfo-6.0.1-5.fc42.aarch64 libwayland-egl-debuginfo-1.23.0-3.fc42.aarch64 libXcursor-debuginfo-1.2.3-2.fc42.aarch64 libXdamage-debuginfo-1.1.6-5.fc42.aarch64 libXcomposite-debuginfo-0.4.6-5.fc42.aarch64 libXrandr-debuginfo-1.5.4-5.fc42.aarch64 libXinerama-debuginfo-1.1.5-8.fc42.aarch64 libthai-debuginfo-0.1.29-10.fc42.aarch64 libmount-debuginfo-2.40.4-2.fc42.aarch64 libselinux-debuginfo-3.8-0.rc3.1.fc42.3.aarch64 libXrender-debuginfo-0.9.12-2.fc42.aarch64 pixman-debuginfo-0.44.2-2.fc42.aarch64 at-spi2-core-debuginfo-2.55.0.1-2.fc42.aarch64 json-glib-debuginfo-1.10.6-2.fc42.aarch64 sqlite-libs-debuginfo-3.47.2-2.fc42.aarch64 libdatrie-debuginfo-0.2.13-11.fc42.aarch64 libblkid-debuginfo-2.40.4-2.fc42.aarch64 gvfs-client-debuginfo-1.56.1-3.fc42.aarch64 libcanberra-gtk3-debuginfo-0.30-37.fc42.aarch64 libcanberra-debuginfo-0.30-37.fc42.aarch64 libvorbis-debuginfo-1.3.7-12.fc42.aarch64 libtdb-debuginfo-1.4.12-5.fc42.aarch64 libtool-ltdl-debuginfo-2.5.4-4.fc42.aarch64 libogg-debuginfo-1.3.5-11.fc42.aarch64 PackageKit-gtk3-module-debuginfo-1.2.8-9.fc42.aarch64 qt6-qtsvg-debuginfo-6.8.1-3.fc42.aarch64 qt6-qtpdf-debuginfo-6.8.1-4.fc42.aarch64 krb5-libs-debuginfo-1.21.3-4.fc42.aarch64 libproxy-debuginfo-0.5.8-2.fc42.aarch64 libcom_err-debuginfo-1.47.2-3.fc42.aarch64 keyutils-libs-debuginfo-1.6.3-5.fc42.aarch64 libcurl-minimal-debuginfo-8.11.1-3.fc42.aarch64 duktape-debuginfo-2.7.0-9.fc42.aarch64 libnghttp2-debuginfo-1.64.0-3.fc42.aarch64\n(gdb) bt\n#0 0x0000fffff7cb8554 in libcamera::V4L2Device::setFrameStartEnabled (this=0x40, enable=false) at ../../src/libcamera/v4l2_device.cpp:463\n#1 0x0000fffff7da1d24 in libcamera::SimplePipelineHandler::stopDevice (this=0xffffbc00cc40, camera=0xffffbc002bc0) at ../../src/libcamera/pipeline/simple/simple.cpp:1531\n#2 0x0000fffff7cab650 in libcamera::PipelineHandler::stop (this=0xffffbc00cc40, camera=0xffffbc002bc0) at ../../src/libcamera/pipeline_handler.cpp:364\n#3 0x0000fffff7c41d7c in libcamera::BoundMethodMember::invoke (this=0x700170, args#0=0xffffbc002bc0) at ../../include/libcamera/base/bound_method.h:191\n#4 0x0000fffff7c42124 in libcamera::BoundMethodArgs::invokePack<0ul, void> (this=0x700170, pack=0x617ff0) at ../../include/libcamera/base/bound_method.h:115\n#5 0x0000fffff7c41d00 in libcamera::BoundMethodArgs::invokePack (this=0x700170, pack=0x617ff0) at ../../include/libcamera/base/bound_method.h:124\n#6 0x0000fffff7718154 in libcamera::InvokeMessage::invoke (this=0x78dc00) at ../../src/libcamera/base/message.cpp:153\n#7 0x0000fffff7700014 in libcamera::Object::message (this=0xffffbc00cc40, msg=0x78dc00) at ../../src/libcamera/base/object.cpp:211\n#8 0x0000fffff77197dc in libcamera::Thread::dispatchMessages (this=0x6e9090, type=libcamera::Message::None) at ../../src/libcamera/base/thread.cpp:649\n#9 0x0000fffff7709078 in libcamera::EventDispatcherPoll::processEvents (this=0xffffbc0162b0) at ../../src/libcamera/base/event_dispatcher_poll.cpp:146\n#10 0x0000fffff7718a84 in libcamera::Thread::exec (this=0x6e9090) at ../../src/libcamera/base/thread.cpp:311\n#11 0x0000fffff7c42a58 in libcamera::CameraManager::Private::run (this=0x6e9080) at ../../src/libcamera/camera_manager.cpp:89\n#12 0x0000fffff7718a24 in libcamera::Thread::startThread (this=0x6e9090) at ../../src/libcamera/base/thread.cpp:289\n#13 0x0000fffff771d58c in std::__invoke_impl\n (__f=@0x6aec80: (void (libcamera::Thread::*)(libcamera::Thread * const)) 0xfffff7718924 , __t=@0x6aec78: 0x6e9090) at /usr/include/c++/15/bits/invoke.h:76\n#14 0x0000fffff771d4d8 in std::__invoke\n (__fn=@0x6aec80: (void (libcamera::Thread::*)(libcamera::Thread * const)) 0xfffff7718924 ) at /usr/include/c++/15/bits/invoke.h:98\n#15 0x0000fffff771d448 in std::thread::_Invoker >::_M_invoke<0ul, 1ul> (this=0x6aec78) at /usr/include/c++/15/bits/std_thread.h:303\n#16 0x0000fffff771d3f8 in std::thread::_Invoker >::operator() (this=0x6aec78) at /usr/include/c++/15/bits/std_thread.h:310\n#17 0x0000fffff771d3d8 in std::thread::_State_impl > >::_M_run (this=0x6aec70) at /usr/include/c++/15/bits/std_thread.h:255\n#18 0x0000fffff5bb2980 in execute_native_thread_routine () at /lib64/libstdc++.so.6\n#19 0x0000fffff594d364 in start_thread () at /lib64/libc.so.6\n#20 0x0000fffff59b868c in thread_start () at /lib64/libc.so.6\n\n\n\nSo I think storing data->eventEmitter_ has not been done safely.\n\n--\nKieran\n\n\n> + data->eventEmitter_->frameStart.disconnect(data->delayedCtrls_.get(),\n> + &DelayedControls::applyControls);\n> + data->eventEmitter_ = NULL;\n> + }\n> +\n> if (data->useConversion_) {\n> if (data->converter_)\n> data->converter_->stop();\n> --\n> 2.43.0\n>","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 D7A23C3259\n\tfor ;\n\tThu, 30 Jan 2025 22:08:06 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 79A3B68563;\n\tThu, 30 Jan 2025 23:08:05 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 588D16034C\n\tfor ;\n\tThu, 30 Jan 2025 23:08:04 +0100 (CET)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust6594.18-1.cable.virginm.net [86.31.185.195])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 34F6C316;\n\tThu, 30 Jan 2025 23:06: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=\"uqMp9qd0\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1738274815;\n\tbh=b8WZSNzM3B2cRQ76w850t5K4+AYPF0f1XuJU0wb3Z4U=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=uqMp9qd0d3YEWoXChLbHntsBnrP3q2tG0eLnbEsiU84etP08ZFl8yfQtDRl6TNaMr\n\tVDgMw7Fph/dn7CZJ4N7PxNT+XVfIjwKiCB5c4SQ4W/Ent7CKHd9+usDbBbq+e6GZRV\n\tuauCot4xyVuBagMSWVqFmova17PqhKCKY1kNTrgk=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20241218142217.437842-1-stanislaw.gruszka@linux.intel.com>","References":"<20241218142217.437842-1-stanislaw.gruszka@linux.intel.com>","Subject":"Re: [PATCH v2] pipeline: simple: Use proper device for frame start\n\tevents","From":"Kieran Bingham ","Cc":"Milan Zamazal ,\n\tLaurent Pinchart ,\n\tNaushir Patuck ,\n\tSakari Ailus ,\n\tHans de Goede ","To":"Stanislaw Gruszka ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Thu, 30 Jan 2025 22:08:01 +0000","Message-ID":"<173827488121.2362053.2810990589873058843@ping.linuxembedded.co.uk>","User-Agent":"alot/0.10","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":33242,"web_url":"https://patchwork.libcamera.org/comment/33242/","msgid":"<20250130225443.GE25569@pendragon.ideasonboard.com>","date":"2025-01-30T22:54:43","subject":"Re: [PATCH v2] pipeline: simple: Use proper device for frame start\n\tevents","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Stanislaw,\n\nThank you for the patch, and sorry this got out of my radar.\n\nOn Wed, Dec 18, 2024 at 03:22:17PM +0100, Stanislaw Gruszka wrote:\n> Currently we use frame start event from video capture device to\n> apply controls. But the capture device might not generate the events.\n> Usually CSI-2 receiver is proper device to subscribe for start\n> frame events.\n> \n> Without DelayedConntrols:applyControls() is possible that we can get\n> call to DelayedControls::get() with frame number that exceed number\n> of saved entries and get below assertion failure:\n> \n> ../src/libcamera/delayed_controls.cpp:227:\n> libcamera::ControlList libcamera::DelayedControls::get(uint32_t):\n> Assertion `info.type() != ControlTypeNone' failed\n> \n> Assertion failure can happen at the beginning of streaming when\n> ControlRingBuffer is not yet filled and there are errors on CSI-2.\n> \n> To fix, loop over devices in the pipeline (starting from the last one),\n> find one that emits start frame events and connect applyControls()\n> to it. Additionally remove direct call to sensor_->setControls() if\n> the emitter device was found.\n> \n> Bug: https://bugs.libcamera.org/show_bug.cgi?id=241\n> Co-developed-by: Hans de Goede \n> Signed-off-by: Hans de Goede \n> Signed-off-by: Stanislaw Gruszka \n> ---\n> v1 -> v2:\n> - make eventEmitter_ subdevice part of SimpleCameraData\n> - add debug log when found event emitter device\n> - nullify eventEmitter_ on stop\n> - remove direct sensor_->setControls()\n> - add delayedCtrls_->reset() on start\n> \n> src/libcamera/pipeline/simple/simple.cpp | 39 +++++++++++++++++++++---\n> 1 file changed, 35 insertions(+), 4 deletions(-)\n> \n> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n> index 8ac24e6e..a7594c2c 100644\n> --- a/src/libcamera/pipeline/simple/simple.cpp\n> +++ b/src/libcamera/pipeline/simple/simple.cpp\n> @@ -277,6 +277,7 @@ public:\n> \tstd::list entities_;\n> \tstd::unique_ptr sensor_;\n> \tV4L2VideoDevice *video_;\n> +\tV4L2Subdevice *eventEmitter_;\n> \n> \tstd::vector configs_;\n> \tstd::map> formats_;\n> @@ -911,8 +912,11 @@ void SimpleCameraData::ispStatsReady(uint32_t frame, uint32_t bufferId)\n> void SimpleCameraData::setSensorControls(const ControlList &sensorControls)\n> {\n> \tdelayedCtrls_->push(sensorControls);\n> -\tControlList ctrls(sensorControls);\n> -\tsensor_->setControls(&ctrls);\n> +\t/* Directly apply controls now if there is no frameStart signal */\n> +\tif (!eventEmitter_) {\n> +\t\tControlList ctrls(sensorControls);\n> +\t\tsensor_->setControls(&ctrls);\n> +\t}\n> }\n> \n> /* Retrieve all source pads connected to a sink pad through active routes. */\n> @@ -1299,8 +1303,6 @@ int SimplePipelineHandler::configure(Camera *camera, CameraConfiguration *c)\n> \tdata->delayedCtrls_ =\n> \t\tstd::make_unique(data->sensor_->device(),\n> \t\t\t\t\t\t params);\n> -\tdata->video_->frameStart.connect(data->delayedCtrls_.get(),\n> -\t\t\t\t\t &DelayedControls::applyControls);\n> \n> \tStreamConfiguration inputCfg;\n> \tinputCfg.pixelFormat = pipeConfig->captureFormat;\n> @@ -1368,6 +1370,28 @@ int SimplePipelineHandler::start(Camera *camera, [[maybe_unused]] const ControlL\n> \n> \tvideo->bufferReady.connect(data, &SimpleCameraData::imageBufferReady);\n> \n> +\t/*\n> +\t * Enable frame start event on last device in the pipeline\n> +\t * that provides the events.\n> +\t */\n> +\tfor (auto it = data->entities_.rbegin(); it != data->entities_.rend(); ++it) {\n> +\t\tV4L2Subdevice *sd = subdev(it->entity);\n> +\t\tif (!sd)\n> +\t\t\tcontinue;\n> +\t\tif (sd->setFrameStartEnabled(true) < 0)\n> +\t\t\tcontinue;\n> +\n> +\t\tLOG(SimplePipeline, Debug)\n> +\t\t\t<< \"Using \" << it->entity->name() << \" frameStart signal\";\n> +\n> +\t\tsd->frameStart.connect(data->delayedCtrls_.get(),\n> +\t\t\t\t &DelayedControls::applyControls);\n> +\t\tdata->eventEmitter_ = sd;\n> +\t\tbreak;\n> +\t}\n\nCould we do this at init time instead ? V4L2 doesn't offer an API to\nlist what events are supported by a subdev (that should probably be\nadded to the kernel, but that's a separate problem), so we need to\nsubscribe to an event in order to know if it's supported. This could be\nimplemented as a V4L2Subdevice::supportsEvent() helper function that\nwould subscribe (and unsubscribe is the subscription is successful).\n\n> +\n> +\tdata->delayedCtrls_->reset();\n> +\n> \tret = video->streamOn();\n> \tif (ret < 0) {\n> \t\tstop(camera);\n> @@ -1400,6 +1424,13 @@ void SimplePipelineHandler::stopDevice(Camera *camera)\n> \tSimpleCameraData *data = cameraData(camera);\n> \tV4L2VideoDevice *video = data->video_;\n> \n> +\tif (data->eventEmitter_) {\n> +\t\tdata->eventEmitter_->setFrameStartEnabled(false);\n> +\t\tdata->eventEmitter_->frameStart.disconnect(data->delayedCtrls_.get(),\n> +\t\t\t\t\t\t\t &DelayedControls::applyControls);\n> +\t\tdata->eventEmitter_ = NULL;\n> +\t}\n> +\n> \tif (data->useConversion_) {\n> \t\tif (data->converter_)\n> \t\t\tdata->converter_->stop();","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 2E0D4BD808\n\tfor ;\n\tThu, 30 Jan 2025 22:54:49 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 606CD68563;\n\tThu, 30 Jan 2025 23:54:48 +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 4D4176034C\n\tfor ;\n\tThu, 30 Jan 2025 23:54:46 +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 1BE5F886;\n\tThu, 30 Jan 2025 23:53:37 +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=\"DpvnnwSG\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1738277617;\n\tbh=BTzoqQA2eDm1HquUoIOajwcZDGD28PLOm7Spronsi8w=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=DpvnnwSGTR3gkwL2ONHaL1OdtRynG2Dp5PAzJOXsOnkPbqf25iFpqal09E0Ximk9y\n\t24VDWgE1YnSQ+QkFpeepOrD4eOzy+G1UaMP8zW6xWiK8rnjnyEU2TJIcXj13nIXhDt\n\tC3mzmAX2TRJKrk4RQJLqqiukHI2TyFyyf1azpAAw=","Date":"Fri, 31 Jan 2025 00:54:43 +0200","From":"Laurent Pinchart ","To":"Stanislaw Gruszka ","Cc":"libcamera-devel@lists.libcamera.org, Milan Zamazal ,\n\tKieran Bingham ,\n\tNaushir Patuck ,\n\tSakari Ailus ,\n\tHans de Goede ","Subject":"Re: [PATCH v2] pipeline: simple: Use proper device for frame start\n\tevents","Message-ID":"<20250130225443.GE25569@pendragon.ideasonboard.com>","References":"<20241218142217.437842-1-stanislaw.gruszka@linux.intel.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20241218142217.437842-1-stanislaw.gruszka@linux.intel.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\" "}}]