[{"id":23701,"web_url":"https://patchwork.libcamera.org/comment/23701/","msgid":"<165663080219.2049236.6130607312605590800@Monstersaurus>","date":"2022-06-30T23:13:22","subject":"Re: [libcamera-devel] [PATCH v3 11/23] libcamera: camera_sensor:\n\tExpose DelayedControls interface","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Jacopo Mondi via libcamera-devel (2022-06-30 14:38:50)\n> Now that the CameraSensor class has an instance of delayed controls\n> expose its interface for pipeline handlers to use it.\n> \n> The interface towards DelayedControls still operates on lists of\n> V4L2 controls.\n> \n> Signed-off-by: Jacopo Mondi \n> ---\n> include/libcamera/internal/camera_sensor.h | 4 +++\n> src/libcamera/camera_sensor/camera_sensor.cpp | 31 +++++++++++++++++++\n> 2 files changed, 35 insertions(+)\n> \n> diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h\n> index bd5aa0dbc27d..a606bc5d1cb5 100644\n> --- a/include/libcamera/internal/camera_sensor.h\n> +++ b/include/libcamera/internal/camera_sensor.h\n> @@ -71,6 +71,10 @@ public:\n> \n> CameraLens *focusLens() { return focusLens_.get(); }\n> \n> + void frameStart(uint32_t sequence);\n> + void pushControls(const ControlList &ctrls);\n> + ControlList getControls(uint32_t sequence);\n> +\n> protected:\n> std::string logPrefix() const override;\n> \n> diff --git a/src/libcamera/camera_sensor/camera_sensor.cpp b/src/libcamera/camera_sensor/camera_sensor.cpp\n> index 211c7461f5c6..e1770e8fa130 100644\n> --- a/src/libcamera/camera_sensor/camera_sensor.cpp\n> +++ b/src/libcamera/camera_sensor/camera_sensor.cpp\n> @@ -1043,6 +1043,37 @@ void CameraSensor::updateControlInfo()\n> updateControls();\n> }\n> \n> +/**\n> + * \\brief Signal to the camera sensor class that a new frame has start exposing\n> + * \\param[in] sequence The frame sequence number\n> + */\n> +void CameraSensor::frameStart(uint32_t sequence)\n> +{\n> + /* A new capture session requires to start in a clean state. */\n> + if (sequence == 0)\n> + delayedCtrls_->reset();\n\nWe want to have queued up some controls *before* we start the device,\nand so here this races with anything that might have been queued up\nbefore the first frame SoF is called.\n\nI don't know 'how much' of a race it is ... but I don't think it's good\nto have this reset here. I think it should be an explict reset tied in\nelsewhere.\n\n\n> +\n> + delayedCtrls_->applyControls(sequence);\n> +}\n> +\n> +/**\n> + * \\brief Push a new set of controls to the CameraSensor\n> + * \\param[in] ctrls The list of controls to push to the sensor\n> + */\n> +void CameraSensor::pushControls(const ControlList &ctrls)\n\nI think we're going to want to pass in the sequence number here, and\ncall it setControls probably. But while this is currently push in the\nDelayedControls, keeping the existing implementation during the move is\ngood I think.\n\n\n\n> +{\n> + delayedCtrls_->push(ctrls);\n> +}\n> +\n> +/**\n> + * \\brief Get the list of controls applied at frame \\a sequence\n> + * \\param[in] sequence The frame sequence number\n> + */\n> +ControlList CameraSensor::getControls(uint32_t sequence)\n> +{\n> + return delayedCtrls_->get(sequence);\n> +}\n> +\n> /**\n> * \\fn CameraSensor::focusLens()\n> * \\brief Retrieve the focus lens controller\n> -- \n> 2.36.1\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 E4FAEBE173\n\tfor ;\n\tThu, 30 Jun 2022 23:13:25 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 395D56564E;\n\tFri, 1 Jul 2022 01:13:25 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id AA80660412\n\tfor ;\n\tFri, 1 Jul 2022 01:13:24 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 3E30825C;\n\tFri, 1 Jul 2022 01:13:24 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1656630805;\n\tbh=NYp+uDAAjTia6l/yGLY+r2I3KU7CWiM9We9HUg22YbE=;\n\th=In-Reply-To:References:To:Date:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:\n\tFrom;\n\tb=N0EJ2zrLfFfpQN77ehOdmTNfqb0e1P+u6YONyxVnuqaOjMTUb46g2Yaz4Nxzi62im\n\tNYp4PEX1H1NkNQGttypE/rkcgh9+LOgZIx5miS3N0dhIwBE+vZo+591ZnHuGNZNzc0\n\tvBCXKQgz6/neA8ERct8CvajfTbmeI9k1BZI8xrEwn642U+eTfOUjqFoWumNOY8M3gx\n\tN3vaG/DQIAUiFhSFryL0NCrJWsKr1n+kULOq5dQfXXxi+C0fp4kOKh/wkySTdEgcxM\n\tpjLdBUke1ttPs6R72GpVZAesm9bco5FCLUFi0fEg4uLiseVw5hSy7HlIjC7l5hME9q\n\tnPjGbXehZYXBg==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1656630804;\n\tbh=NYp+uDAAjTia6l/yGLY+r2I3KU7CWiM9We9HUg22YbE=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=v+IYlYrpnvve5ntnNgHGJsH+xsrAzVuz8H7NXCqezE5hTAplAFma7Lz2rdYFMAVCI\n\tSUtNdFJyBkaADXVsGJ15Ra+hElaTq2qYQohRZki0TLVL39qYq7S5QSxVRHVlPNjhtz\n\tuVjU2BLjVbF36d3iSrGbE1AT5PDB9sp3FpqvRLUY="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"v+IYlYrp\"; dkim-atps=neutral","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20220630133902.321099-12-jacopo@jmondi.org>","References":"<20220630133902.321099-1-jacopo@jmondi.org>\n\t<20220630133902.321099-12-jacopo@jmondi.org>","To":"Jacopo Mondi , libcamera-devel@lists.libcamera.org","Date":"Fri, 01 Jul 2022 00:13:22 +0100","Message-ID":"<165663080219.2049236.6130607312605590800@Monstersaurus>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH v3 11/23] libcamera: camera_sensor:\n\tExpose DelayedControls interface","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","From":"Kieran Bingham via libcamera-devel\n\t","Reply-To":"Kieran Bingham ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":23709,"web_url":"https://patchwork.libcamera.org/comment/23709/","msgid":"<20220701092028.iis6jovwqgvaoe7u@uno.localdomain>","date":"2022-07-01T09:20:28","subject":"Re: [libcamera-devel] [PATCH v3 11/23] libcamera: camera_sensor:\n\tExpose DelayedControls interface","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Kieran,\n\nOn Fri, Jul 01, 2022 at 12:13:22AM +0100, Kieran Bingham wrote:\n> Quoting Jacopo Mondi via libcamera-devel (2022-06-30 14:38:50)\n> > Now that the CameraSensor class has an instance of delayed controls\n> > expose its interface for pipeline handlers to use it.\n> >\n> > The interface towards DelayedControls still operates on lists of\n> > V4L2 controls.\n> >\n> > Signed-off-by: Jacopo Mondi \n> > ---\n> > include/libcamera/internal/camera_sensor.h | 4 +++\n> > src/libcamera/camera_sensor/camera_sensor.cpp | 31 +++++++++++++++++++\n> > 2 files changed, 35 insertions(+)\n> >\n> > diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h\n> > index bd5aa0dbc27d..a606bc5d1cb5 100644\n> > --- a/include/libcamera/internal/camera_sensor.h\n> > +++ b/include/libcamera/internal/camera_sensor.h\n> > @@ -71,6 +71,10 @@ public:\n> >\n> > CameraLens *focusLens() { return focusLens_.get(); }\n> >\n> > + void frameStart(uint32_t sequence);\n> > + void pushControls(const ControlList &ctrls);\n> > + ControlList getControls(uint32_t sequence);\n> > +\n> > protected:\n> > std::string logPrefix() const override;\n> >\n> > diff --git a/src/libcamera/camera_sensor/camera_sensor.cpp b/src/libcamera/camera_sensor/camera_sensor.cpp\n> > index 211c7461f5c6..e1770e8fa130 100644\n> > --- a/src/libcamera/camera_sensor/camera_sensor.cpp\n> > +++ b/src/libcamera/camera_sensor/camera_sensor.cpp\n> > @@ -1043,6 +1043,37 @@ void CameraSensor::updateControlInfo()\n> > updateControls();\n> > }\n> >\n> > +/**\n> > + * \\brief Signal to the camera sensor class that a new frame has start exposing\n> > + * \\param[in] sequence The frame sequence number\n> > + */\n> > +void CameraSensor::frameStart(uint32_t sequence)\n> > +{\n> > + /* A new capture session requires to start in a clean state. */\n> > + if (sequence == 0)\n> > + delayedCtrls_->reset();\n>\n> We want to have queued up some controls *before* we start the device,\n> and so here this races with anything that might have been queued up\n> before the first frame SoF is called.\n\nAH! You're right, there could be controls queued up at Camera::start()\nthat would be overridden by this.\n\nI tried to be smart and avoid having to add a 'start()' function to\nCameraSensor, but that won't work well..\n\nGood catch, we need an explicit reset to be issued at\nCamera::configure() time.\n\n>\n> I don't know 'how much' of a race it is ... but I don't think it's good\n> to have this reset here. I think it should be an explict reset tied in\n> elsewhere.\n>\n>\n> > +\n> > + delayedCtrls_->applyControls(sequence);\n> > +}\n> > +\n> > +/**\n> > + * \\brief Push a new set of controls to the CameraSensor\n> > + * \\param[in] ctrls The list of controls to push to the sensor\n> > + */\n> > +void CameraSensor::pushControls(const ControlList &ctrls)\n>\n> I think we're going to want to pass in the sequence number here, and\n> call it setControls probably. But while this is currently push in the\n> DelayedControls, keeping the existing implementation during the move is\n> good I think.\n>\n>\n>\n> > +{\n> > + delayedCtrls_->push(ctrls);\n> > +}\n> > +\n> > +/**\n> > + * \\brief Get the list of controls applied at frame \\a sequence\n> > + * \\param[in] sequence The frame sequence number\n> > + */\n> > +ControlList CameraSensor::getControls(uint32_t sequence)\n> > +{\n> > + return delayedCtrls_->get(sequence);\n> > +}\n> > +\n> > /**\n> > * \\fn CameraSensor::focusLens()\n> > * \\brief Retrieve the focus lens controller\n> > --\n> > 2.36.1\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 8748CBE173\n\tfor ;\n\tFri, 1 Jul 2022 09:20:32 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id F3E3A6564B;\n\tFri, 1 Jul 2022 11:20:31 +0200 (CEST)","from relay6-d.mail.gandi.net (relay6-d.mail.gandi.net\n\t[IPv6:2001:4b98:dc4:8::226])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 5010D6054A\n\tfor ;\n\tFri, 1 Jul 2022 11:20:30 +0200 (CEST)","(Authenticated sender: jacopo@jmondi.org)\n\tby mail.gandi.net (Postfix) with ESMTPSA id CDE69C000D;\n\tFri, 1 Jul 2022 09:20:29 +0000 (UTC)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1656667232;\n\tbh=4hsYFqKv9P0xC3Tc1RTNNv3hRd5wox2aTXesv5+d0Xs=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=T0ziUDpdwNhVWeM0VIjxmQEEuzYEUn0Dxf+iSzJR0jvHlJvgco3TELWAjqAy0g3BZ\n\tXvEzdLawXAMOPFQvu/ahwX6Vf54mZ/vAeY5kXBX3AA+/qrUtrqkl+SF/dzOdrxDDsi\n\tUJJSvEHd7vA/+ia5NX34Tmg8mrqx1IuKeCPlcUmxtbxa1f/Ts/eU2LHvWxFasyAadX\n\tgD6w6wB5zfg7zw/xjESn8hPiAkV/9/LfapIwVDxXNO4jBz0ZRFO8Iblgmh4ETK7ka3\n\tlbH+lj5LqL9s6qBFN3FPB08+pfvkqNfca7KvqtiG3QKKWH1+xPyTJQLmRh3Wr8jg02\n\tmYYTfMFhnUMEA==","Date":"Fri, 1 Jul 2022 11:20:28 +0200","To":"Kieran Bingham ","Message-ID":"<20220701092028.iis6jovwqgvaoe7u@uno.localdomain>","References":"<20220630133902.321099-1-jacopo@jmondi.org>\n\t<20220630133902.321099-12-jacopo@jmondi.org>\n\t<165663080219.2049236.6130607312605590800@Monstersaurus>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<165663080219.2049236.6130607312605590800@Monstersaurus>","Subject":"Re: [libcamera-devel] [PATCH v3 11/23] libcamera: camera_sensor:\n\tExpose DelayedControls interface","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","From":"Jacopo Mondi via libcamera-devel ","Reply-To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]