[{"id":33772,"web_url":"https://patchwork.libcamera.org/comment/33772/","msgid":"<yjb2gmh3e2h7xbiksairvkq5slyyjeuwthyqnejet5plokgnqh@nu4dlunepeky>","date":"2025-03-31T15:52:16","subject":"Re: [PATCH v6 3/5] pipeline: simple: Enable frame start events","submitter":{"id":184,"url":"https://patchwork.libcamera.org/api/people/184/","name":"Stefan Klug","email":"stefan.klug@ideasonboard.com"},"content":"Hi Stanislaw,\n\nThank you for the patch.\n\nOn Wed, Mar 05, 2025 at 02:52:54PM +0100, Stanislaw Gruszka wrote:\n> The simple pipeline handler uses frame start events to apply sensor\n> controls through the DelayedControls class. The setSensorControls()\n> function applies the controls directly, which would result in controls\n> being applied twice, if it wasn't for the fact that the pipeline handler\n> forgot to enable the frame start events in the first place. Those two\n> issues cancel each other, but cause controls to always be applied\n> directly.\n> \n> Fix the issue by only applying controls directly in setSensorControls()\n> if no frame start event emitter is available, and by enabling the frame\n> start events in startDevice() otherwise. Disable them in stopDevice()\n> for symmetry.\n> \n> Co-developed-by: Hans de Goede <hdegoede@redhat.com>\n> Signed-off-by: Hans de Goede <hdegoede@redhat.com>\n> Co-developed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> Signed-off-by: Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com>\n> ---\n>  src/libcamera/pipeline/simple/simple.cpp | 49 +++++++++++++++++++++---\n>  1 file changed, 43 insertions(+), 6 deletions(-)\n> \n> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n> index 8345a771..7be49017 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<Entity> entities_;\n>  \tstd::unique_ptr<CameraSensor> sensor_;\n>  \tV4L2VideoDevice *video_;\n> +\tV4L2Subdevice *frameStartEmitter_;\n>  \n>  \tstd::vector<Configuration> configs_;\n>  \tstd::map<PixelFormat, std::vector<const Configuration *>> formats_;\n> @@ -579,6 +580,20 @@ int SimpleCameraData::init()\n>  \n>  \tproperties_ = sensor_->properties();\n>  \n> +\t/* Find the first subdev that can generate a frame start signal, if any. */\n> +\tframeStartEmitter_ = nullptr;\n> +\tfor (const Entity &entity : entities_) {\n> +\t\tV4L2Subdevice *sd = pipe->subdev(entity.entity);\n> +\t\tif (!sd || !sd->supportsFrameStartEvent())\n> +\t\t\tcontinue;\n> +\n> +\t\tLOG(SimplePipeline, Debug)\n> +\t\t\t<< \"Using frameStart signal from '\"\n> +\t\t\t<< entity.entity->name() << \"'\";\n> +\t\tframeStartEmitter_ = sd;\n> +\t\tbreak;\n> +\t}\n> +\n>  \treturn 0;\n>  }\n>  \n> @@ -897,8 +912,18 @@ 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/*\n> +         * Directly apply controls now if there is no frameStart signal.\n> +\t *\n> +\t * \\todo Applying controls directly not only increases the risk of\n> +\t * applying them to the wrong frame (or across a frame boundary),\n> +\t * but it also bypasses delayedCtrls_, creating AGC regulation issues.\n> +\t * Both problems should be fixed.\n> +\t */\n> +\tif (!frameStartEmitter_) {\n> +\t\tControlList ctrls(sensorControls);\n> +\t\tsensor_->setControls(&ctrls);\n> +\t}\n\nIf I get it right, we don't use delayedControls::applyControls() here\nbecause we don't have the sequence number available. Wouldn't it be\nbetter to reduce this to delayedControls_->push()\n\nand to add a\n\nif(!frameStartEmitter_) {\n\tdelayedControls_::applyControls(buffer->metadata().sequence);\n}\n\nto SimpleCameraData::imageBufferReady().\n\nthen we get the \"per control delays\" from delayed controls. Timing might\nstill be off, but it would be the best we can do for now.\n\nBest regards,\nStefan\n\n\n>  }\n>  \n>  /* Retrieve all source pads connected to a sink pad through active routes. */\n> @@ -1323,6 +1348,7 @@ int SimplePipelineHandler::start(Camera *camera, [[maybe_unused]] const ControlL\n>  {\n>  \tSimpleCameraData *data = cameraData(camera);\n>  \tV4L2VideoDevice *video = data->video_;\n> +\tV4L2Subdevice *frameStartEmitter = data->frameStartEmitter_;\n>  \tint ret;\n>  \n>  \tconst MediaPad *pad = acquirePipeline(data);\n> @@ -1352,8 +1378,15 @@ int SimplePipelineHandler::start(Camera *camera, [[maybe_unused]] const ControlL\n>  \n>  \tvideo->bufferReady.connect(data, &SimpleCameraData::imageBufferReady);\n>  \n> -\tdata->video_->frameStart.connect(data->delayedCtrls_.get(),\n> -\t\t\t\t\t &DelayedControls::applyControls);\n> +\tif (frameStartEmitter) {\n> +\t\tret = frameStartEmitter->setFrameStartEnabled(true);\n> +\t\tif (ret) {\n> +\t\t\tstop(camera);\n> +\t\t\treturn ret;\n> +\t\t}\n> +\t\tframeStartEmitter->frameStart.connect(data->delayedCtrls_.get(),\n> +\t\t\t\t\t\t      &DelayedControls::applyControls);\n> +\t}\n>  \n>  \tret = video->streamOn();\n>  \tif (ret < 0) {\n> @@ -1386,9 +1419,13 @@ void SimplePipelineHandler::stopDevice(Camera *camera)\n>  {\n>  \tSimpleCameraData *data = cameraData(camera);\n>  \tV4L2VideoDevice *video = data->video_;\n> +\tV4L2Subdevice *frameStartEmitter = data->frameStartEmitter_;\n>  \n> -\tdata->video_->frameStart.disconnect(data->delayedCtrls_.get(),\n> -\t\t\t\t\t    &DelayedControls::applyControls);\n> +\tif (frameStartEmitter) {\n> +\t\tframeStartEmitter->setFrameStartEnabled(false);\n> +\t\tframeStartEmitter->frameStart.disconnect(data->delayedCtrls_.get(),\n> +\t\t\t\t\t\t\t &DelayedControls::applyControls);\n> +\t}\n>  \n>  \tif (data->useConversion_) {\n>  \t\tif (data->converter_)\n> -- \n> 2.43.0\n>","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 8D4B8C323E\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 31 Mar 2025 15:52:21 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C5E2C68981;\n\tMon, 31 Mar 2025 17:52:20 +0200 (CEST)","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 CBCE068967\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 31 Mar 2025 17:52:19 +0200 (CEST)","from ideasonboard.com (unknown\n\t[IPv6:2a00:6020:448c:6c00:823a:c275:e8b5:b937])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 066FB703;\n\tMon, 31 Mar 2025 17:50:27 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"Z2R8nJ0q\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1743436228;\n\tbh=xLPsejNIGkfAE3UhdsiYnHZ45T72OZaGt9QFM1qPaMY=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=Z2R8nJ0qKRpoDNEE5xXYB3mopMaMbed57OeHTrKJvsxSJMMNP0xyfsHccrMbZ5NO2\n\t9gtxuduOgV1C/dY4I7EOz6JFOxhEr6Uu/Hu3ngbKXbxtVg8UhuahMjXUsA1MqumtAJ\n\thD30x3Sa/wYjmzDvAyHLUdSBRBqClPtrKUcacT9s=","Date":"Mon, 31 Mar 2025 17:52:16 +0200","From":"Stefan Klug <stefan.klug@ideasonboard.com>","To":"Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com>","Cc":"libcamera-devel@lists.libcamera.org, Milan Zamazal <mzamazal@redhat.com>,\n\tLaurent Pinchart <laurent.pinchart@ideasonboard.com>, \n\tKieran Bingham <kieran.bingham@ideasonboard.com>,\n\tNaushir Patuck <naush@raspberrypi.com>, \n\tSakari Ailus <sakari.ailus@linux.intel.com>,\n\tHans de Goede <hdegoede@redhat.com>","Subject":"Re: [PATCH v6 3/5] pipeline: simple: Enable frame start events","Message-ID":"<yjb2gmh3e2h7xbiksairvkq5slyyjeuwthyqnejet5plokgnqh@nu4dlunepeky>","References":"<20250305135256.801351-1-stanislaw.gruszka@linux.intel.com>\n\t<20250305135256.801351-4-stanislaw.gruszka@linux.intel.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20250305135256.801351-4-stanislaw.gruszka@linux.intel.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":33883,"web_url":"https://patchwork.libcamera.org/comment/33883/","msgid":"<Z+z+iGQ8FmYoluzq@linux.intel.com>","date":"2025-04-02T09:08:24","subject":"Re: [PATCH v6 3/5] pipeline: simple: Enable frame start events","submitter":{"id":211,"url":"https://patchwork.libcamera.org/api/people/211/","name":"Stanislaw Gruszka","email":"stanislaw.gruszka@linux.intel.com"},"content":"Hi Stefan,\n\nthanks for the review.\n\nOn Mon, Mar 31, 2025 at 05:52:16PM +0200, Stefan Klug wrote:\n> > @@ -897,8 +912,18 @@ 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/*\n> > +         * Directly apply controls now if there is no frameStart signal.\n> > +\t *\n> > +\t * \\todo Applying controls directly not only increases the risk of\n> > +\t * applying them to the wrong frame (or across a frame boundary),\n> > +\t * but it also bypasses delayedCtrls_, creating AGC regulation issues.\n> > +\t * Both problems should be fixed.\n> > +\t */\n> > +\tif (!frameStartEmitter_) {\n> > +\t\tControlList ctrls(sensorControls);\n> > +\t\tsensor_->setControls(&ctrls);\n> > +\t}\n> \n> If I get it right, we don't use delayedControls::applyControls() here\n> because we don't have the sequence number available. Wouldn't it be\n> better to reduce this to delayedControls_->push()\n> \n> and to add a\n> \n> if(!frameStartEmitter_) {\n> \tdelayedControls_::applyControls(buffer->metadata().sequence);\n> }\n> \n> to SimpleCameraData::imageBufferReady().\n> \n> then we get the \"per control delays\" from delayed controls. Timing might\n> still be off, but it would be the best we can do for now.\n\nI'm testing below change and sometimes get strange low frequency flickering.\n\ndiff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\nindex 659b1123a717..c71bacc90bfb 100644\n--- a/src/libcamera/pipeline/simple/simple.cpp\n+++ b/src/libcamera/pipeline/simple/simple.cpp\n@@ -597,7 +597,7 @@ int SimpleCameraData::init()\n \t\tLOG(SimplePipeline, Debug)\n \t\t\t<< \"Using frameStart signal from '\"\n \t\t\t<< entity.entity->name() << \"'\";\n-\t\tframeStartEmitter_ = sd;\n+\t\t// frameStartEmitter_ = sd;\n \t\tbreak;\n \t}\n \n@@ -851,6 +851,10 @@ void SimpleCameraData::imageBufferReady(FrameBuffer *buffer)\n \t\t}\n \t}\n \n+\tif (!frameStartEmitter_) {\n+\t\tdelayedCtrls_->applyControls(buffer->metadata().sequence);\n+\t}\n+\n \tif (request)\n \t\trequest->metadata().set(controls::SensorTimestamp,\n \t\t\t\t\tbuffer->metadata().timestamp);\n@@ -927,10 +931,10 @@ void SimpleCameraData::setSensorControls(const ControlList &sensorControls)\n \t * but it also bypasses delayedCtrls_, creating AGC regulation issues.\n \t * Both problems should be fixed.\n \t */\n-\tif (!frameStartEmitter_) {\n-\t\tControlList ctrls(sensorControls);\n-\t\tsensor_->setControls(&ctrls);\n-\t}\n+\t// if (!frameStartEmitter_) {\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\nCould we apply this one for now with \\todo and do\napplyControls(buffer->metadata().sequence) change later on top,\nonce I figure out why I'm getting the flickering ? \n\nI think that should be fine as this patch does not change existing\nbehaviour for !frameStartEmitter case.\n\nRegards\nStanislaw","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 395DBC3213\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed,  2 Apr 2025 09:08:33 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8D7D36897A;\n\tWed,  2 Apr 2025 11:08:32 +0200 (CEST)","from mgamail.intel.com (mgamail.intel.com [198.175.65.16])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 5E8796897A\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed,  2 Apr 2025 11:08:30 +0200 (CEST)","from fmviesa003.fm.intel.com ([10.60.135.143])\n\tby orvoesa108.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384;\n\t02 Apr 2025 02:08:29 -0700","from sgruszka-mobl.ger.corp.intel.com (HELO localhost)\n\t([10.246.8.237]) by fmviesa003-auth.fm.intel.com with\n\tESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 02 Apr 2025 02:08:26 -0700"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=intel.com header.i=@intel.com\n\theader.b=\"C08iwFvz\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple;\n\td=intel.com; i=@intel.com; q=dns/txt; s=Intel;\n\tt=1743584911; x=1775120911;\n\th=date:from:to:cc:subject:message-id:references:\n\tmime-version:in-reply-to;\n\tbh=1xr7bzkS4R/B8AnVqZcOBDF7yMLe+ZsnE3M6AdO+VV8=;\n\tb=C08iwFvzFamyFygsT7uI/xZdEC/P0cSasBSM579hnJxHj5x9c2UyMu1Q\n\taqv40lugfAIKDbpcBs9xjBjV2mR4zpQ7BwAatzEqyCg+PVFor8uIDsuiZ\n\tsRe6xmpDWMCZyUQSbP/HZ+BXLEnjT5XNdwLDVw+mTHkBp+w0L8ajNYgen\n\tE6ke1lNsbYw7c5N0HkC9e+5+G7FaxAF+jCsPPcWvgm9SyDbfMVens67nF\n\tmBuFVB8C+J94i1eyrRO7DLWQs3hDa/Jz/0LVik1qwNgOqr3zwtWt1b9FJ\n\tkTXTh6zcmP6MCPAmKO0JXW/Ldokf9hqyyrrgpoE8/Gn0z29ho8E6PK7Km w==;","X-CSE-ConnectionGUID":["NFJkHHi/TFCAnidIqViDTQ==","f5BUbWeLTqGKowdu/BYnxQ=="],"X-CSE-MsgGUID":["mYR2d/UnS+qs6uFvsAeR9A==","eAPwimRyRbixryNWTTyudA=="],"X-IronPort-AV":["E=McAfee;i=\"6700,10204,11391\"; a=\"45036675\"","E=Sophos;i=\"6.14,295,1736841600\"; d=\"scan'208\";a=\"45036675\"","E=Sophos;i=\"6.14,295,1736841600\"; d=\"scan'208\";a=\"130761577\""],"X-ExtLoop1":"1","Date":"Wed, 2 Apr 2025 11:08:24 +0200","From":"Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com>","To":"Stefan Klug <stefan.klug@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org, Milan Zamazal <mzamazal@redhat.com>,\n\tLaurent Pinchart <laurent.pinchart@ideasonboard.com>,\n\tKieran Bingham <kieran.bingham@ideasonboard.com>,\n\tNaushir Patuck <naush@raspberrypi.com>,\n\tSakari Ailus <sakari.ailus@linux.intel.com>,\n\tHans de Goede <hdegoede@redhat.com>","Subject":"Re: [PATCH v6 3/5] pipeline: simple: Enable frame start events","Message-ID":"<Z+z+iGQ8FmYoluzq@linux.intel.com>","References":"<20250305135256.801351-1-stanislaw.gruszka@linux.intel.com>\n\t<20250305135256.801351-4-stanislaw.gruszka@linux.intel.com>\n\t<yjb2gmh3e2h7xbiksairvkq5slyyjeuwthyqnejet5plokgnqh@nu4dlunepeky>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<yjb2gmh3e2h7xbiksairvkq5slyyjeuwthyqnejet5plokgnqh@nu4dlunepeky>","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":33888,"web_url":"https://patchwork.libcamera.org/comment/33888/","msgid":"<vs6aofgw3b3estvnm6ctr5hhafydjjkp4g6ncgsypgcziwr2eb@s6733cv7tuss>","date":"2025-04-02T14:00:59","subject":"Re: [PATCH v6 3/5] pipeline: simple: Enable frame start events","submitter":{"id":184,"url":"https://patchwork.libcamera.org/api/people/184/","name":"Stefan Klug","email":"stefan.klug@ideasonboard.com"},"content":"Hi Stanislaw,\n\nOn Wed, Apr 02, 2025 at 11:08:24AM +0200, Stanislaw Gruszka wrote:\n> Hi Stefan,\n> \n> thanks for the review.\n> \n> On Mon, Mar 31, 2025 at 05:52:16PM +0200, Stefan Klug wrote:\n> > > @@ -897,8 +912,18 @@ 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/*\n> > > +         * Directly apply controls now if there is no frameStart signal.\n> > > +\t *\n> > > +\t * \\todo Applying controls directly not only increases the risk of\n> > > +\t * applying them to the wrong frame (or across a frame boundary),\n> > > +\t * but it also bypasses delayedCtrls_, creating AGC regulation issues.\n> > > +\t * Both problems should be fixed.\n> > > +\t */\n> > > +\tif (!frameStartEmitter_) {\n> > > +\t\tControlList ctrls(sensorControls);\n> > > +\t\tsensor_->setControls(&ctrls);\n> > > +\t}\n> > \n> > If I get it right, we don't use delayedControls::applyControls() here\n> > because we don't have the sequence number available. Wouldn't it be\n> > better to reduce this to delayedControls_->push()\n> > \n> > and to add a\n> > \n> > if(!frameStartEmitter_) {\n> > \tdelayedControls_::applyControls(buffer->metadata().sequence);\n> > }\n> > \n> > to SimpleCameraData::imageBufferReady().\n> > \n> > then we get the \"per control delays\" from delayed controls. Timing might\n> > still be off, but it would be the best we can do for now.\n> \n> I'm testing below change and sometimes get strange low frequency flickering.\n> \n> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n> index 659b1123a717..c71bacc90bfb 100644\n> --- a/src/libcamera/pipeline/simple/simple.cpp\n> +++ b/src/libcamera/pipeline/simple/simple.cpp\n> @@ -597,7 +597,7 @@ int SimpleCameraData::init()\n>  \t\tLOG(SimplePipeline, Debug)\n>  \t\t\t<< \"Using frameStart signal from '\"\n>  \t\t\t<< entity.entity->name() << \"'\";\n> -\t\tframeStartEmitter_ = sd;\n> +\t\t// frameStartEmitter_ = sd;\n>  \t\tbreak;\n>  \t}\n>  \n> @@ -851,6 +851,10 @@ void SimpleCameraData::imageBufferReady(FrameBuffer *buffer)\n>  \t\t}\n>  \t}\n>  \n> +\tif (!frameStartEmitter_) {\n> +\t\tdelayedCtrls_->applyControls(buffer->metadata().sequence);\n> +\t}\n> +\n>  \tif (request)\n>  \t\trequest->metadata().set(controls::SensorTimestamp,\n>  \t\t\t\t\tbuffer->metadata().timestamp);\n> @@ -927,10 +931,10 @@ void SimpleCameraData::setSensorControls(const ControlList &sensorControls)\n>  \t * but it also bypasses delayedCtrls_, creating AGC regulation issues.\n>  \t * Both problems should be fixed.\n>  \t */\n> -\tif (!frameStartEmitter_) {\n> -\t\tControlList ctrls(sensorControls);\n> -\t\tsensor_->setControls(&ctrls);\n> -\t}\n> +\t// if (!frameStartEmitter_) {\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> \n> Could we apply this one for now with \\todo and do\n> applyControls(buffer->metadata().sequence) change later on top,\n> once I figure out why I'm getting the flickering ? \n\nThanks for testing that out. I'm fine with merging this and doing the\nrest later (There was a bit of hope it could \"just work\"). There are\nalso some changes and fixes to delayed controls in my pipeline that I\nhope to post as an RFC in the not too far future.\n\nSo\n\nReviewed-by: Stefan Klug <stefan.klug@ideasonboard.com> \n\nBest regards,\nStefan\n\n> \n> I think that should be fine as this patch does not change existing\n> behaviour for !frameStartEmitter case.\n> \n> Regards\n> Stanislaw\n>","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 17A11C3213\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed,  2 Apr 2025 14:01:04 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3C4966898E;\n\tWed,  2 Apr 2025 16:01:03 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 9D70E68979\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed,  2 Apr 2025 16:01:01 +0200 (CEST)","from ideasonboard.com (unknown\n\t[IPv6:2a00:6020:448c:6c00:3e5a:57a6:9731:f378])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id AFA43415;\n\tWed,  2 Apr 2025 15:59:08 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"HcqIdRVb\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1743602348;\n\tbh=PN5oynkY4ey/ci/9Z6bzDRXXknpBYqo3LD/i+Ru60so=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=HcqIdRVbjzFp7nwkyI6xRkv5AkBmBnGqcxf2DqW+tIQ4XCkhypLO+KdFirokZvR6w\n\t512fK7whfzpo/coi0k+AtNDu8GqvuaF54cKW1nbN+D3aWSDkU2LQe/shd/OY+sR8vi\n\tdZdCBFmvQ14AJ37aP8Ufs8qKRsUDadGlTja7L8rU=","Date":"Wed, 2 Apr 2025 16:00:59 +0200","From":"Stefan Klug <stefan.klug@ideasonboard.com>","To":"Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com>","Cc":"libcamera-devel@lists.libcamera.org, Milan Zamazal <mzamazal@redhat.com>,\n\tLaurent Pinchart <laurent.pinchart@ideasonboard.com>, \n\tKieran Bingham <kieran.bingham@ideasonboard.com>,\n\tNaushir Patuck <naush@raspberrypi.com>, \n\tSakari Ailus <sakari.ailus@linux.intel.com>,\n\tHans de Goede <hdegoede@redhat.com>","Subject":"Re: [PATCH v6 3/5] pipeline: simple: Enable frame start events","Message-ID":"<vs6aofgw3b3estvnm6ctr5hhafydjjkp4g6ncgsypgcziwr2eb@s6733cv7tuss>","References":"<20250305135256.801351-1-stanislaw.gruszka@linux.intel.com>\n\t<20250305135256.801351-4-stanislaw.gruszka@linux.intel.com>\n\t<yjb2gmh3e2h7xbiksairvkq5slyyjeuwthyqnejet5plokgnqh@nu4dlunepeky>\n\t<Z+z+iGQ8FmYoluzq@linux.intel.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<Z+z+iGQ8FmYoluzq@linux.intel.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>"}}]