[{"id":15560,"web_url":"https://patchwork.libcamera.org/comment/15560/","msgid":"<82c5f6c6-e6ea-80d9-8ac3-3a403a707b75@ideasonboard.com>","date":"2021-03-09T09:54:41","subject":"Re: [libcamera-devel] [PATCH v4 1/7] libcamera: delayed_controls:\n\tAdd notion of priority write","submitter":{"id":75,"url":"https://patchwork.libcamera.org/api/people/75/","name":"Jean-Michel Hautbois","email":"jeanmichel.hautbois@ideasonboard.com"},"content":"Hi Naushir,\n\nThanks for the patch !\n\nOn 04/03/2021 09:17, Naushir Patuck wrote:\n> If an exposure time change adjusts the vblanking limits, and we set both\n> VBLANK and EXPOSURE controls through the VIDIOC_S_EXT_CTRLS ioctl, the\n> latter may fail if the value is outside of the limits calculated by the\n> old VBLANK value. This is a limitation in V4L2 and cannot be fixed by\n> setting VBLANK before EXPOSURE in a single VIDIOC_S_EXT_CTRLS ioctl.\n> \n> The workaround here is to have the DelayedControls object mark the\n> VBLANK control as \"priority write\", which then write VBLANK separately\n> from (and ahead of) any other controls. This way, the sensor driver will\n> update the EXPOSURE control with new limits before the new values is\n> presented, and will thus be seen as valid.\n> \n> To support this, a new struct DelayedControls::ControlParams is used in\n> the constructor to provide the control delay value as well as the\n> priority write flag.\n> \n> Signed-off-by: Naushir Patuck \n> Tested-by: David Plowman \n> Reviewed-by: Paul Elder \nTested-by: Jean-Michel Hautbois \n\n> ---\n> include/libcamera/internal/delayed_controls.h | 9 +++-\n> src/libcamera/delayed_controls.cpp | 54 +++++++++++++------\n> src/libcamera/pipeline/ipu3/ipu3.cpp | 8 +--\n> .../pipeline/raspberrypi/raspberrypi.cpp | 13 +++--\n> src/libcamera/pipeline/rkisp1/rkisp1.cpp | 8 +--\n> test/delayed_contols.cpp | 20 +++----\n> 6 files changed, 68 insertions(+), 44 deletions(-)\n> \n> diff --git a/include/libcamera/internal/delayed_controls.h b/include/libcamera/internal/delayed_controls.h\n> index dc447a882514..564d9f2e2440 100644\n> --- a/include/libcamera/internal/delayed_controls.h\n> +++ b/include/libcamera/internal/delayed_controls.h\n> @@ -19,8 +19,13 @@ class V4L2Device;\n> class DelayedControls\n> {\n> public:\n> +\tstruct ControlParams {\n> +\t\tunsigned int delay;\n> +\t\tbool priorityWrite;\n> +\t};\n> +\n> \tDelayedControls(V4L2Device *device,\n> -\t\t\tconst std::unordered_map &delays);\n> +\t\t\tconst std::unordered_map &controlParams);\n> \n> \tvoid reset();\n> \n> @@ -64,7 +69,7 @@ private:\n> \n> \tV4L2Device *device_;\n> \t/* \\todo Evaluate if we should index on ControlId * or unsigned int */\n> -\tstd::unordered_map delays_;\n> +\tstd::unordered_map controlParams_;\n> \tunsigned int maxDelay_;\n> \n> \tbool running_;\n> diff --git a/src/libcamera/delayed_controls.cpp b/src/libcamera/delayed_controls.cpp\n> index ab1d40057c5f..3ed1dfebd035 100644\n> --- a/src/libcamera/delayed_controls.cpp\n> +++ b/src/libcamera/delayed_controls.cpp\n> @@ -40,15 +40,19 @@ LOG_DEFINE_CATEGORY(DelayedControls)\n> /**\n> * \\brief Construct a DelayedControls instance\n> * \\param[in] device The V4L2 device the controls have to be applied to\n> - * \\param[in] delays Map of the numerical V4L2 control ids to their associated\n> - * delays (in frames)\n> + * \\param[in] controlParams Map of the numerical V4L2 control ids to their\n> + * associated control parameters.\n> *\n> - * Only controls specified in \\a delays are handled. If it's desired to mix\n> - * delayed controls and controls that take effect immediately the immediate\n> - * controls must be listed in the \\a delays map with a delay value of 0.\n> + * The control parameters comprise of delays (in frames) and a priority write\n> + * flag. If this flag is set, the relevant control is written separately from,\n> + * ahead of the reset of the batched controls.\n> + *\n> + * Only controls specified in \\a controlParams are handled. If it's desired to\n> + * mix delayed controls and controls that take effect immediately the immediate\n> + * controls must be listed in the \\a controlParams map with a delay value of 0.\n> */\n> DelayedControls::DelayedControls(V4L2Device *device,\n> -\t\t\t\t const std::unordered_map &delays)\n> +\t\t\t\t const std::unordered_map &controlParams)\n> \t: device_(device), maxDelay_(0)\n> {\n> \tconst ControlInfoMap &controls = device_->controls();\n> @@ -57,12 +61,12 @@ DelayedControls::DelayedControls(V4L2Device *device,\n> \t * Create a map of control ids to delays for controls exposed by the\n> \t * device.\n> \t */\n> -\tfor (auto const &delay : delays) {\n> -\t\tauto it = controls.find(delay.first);\n> +\tfor (auto const ¶m : controlParams) {\n> +\t\tauto it = controls.find(param.first);\n> \t\tif (it == controls.end()) {\n> \t\t\tLOG(DelayedControls, Error)\n> \t\t\t\t<< \"Delay request for control id \"\n> -\t\t\t\t<< utils::hex(delay.first)\n> +\t\t\t\t<< utils::hex(param.first)\n> \t\t\t\t<< \" but control is not exposed by device \"\n> \t\t\t\t<< device_->deviceNode();\n> \t\t\tcontinue;\n> @@ -70,13 +74,14 @@ DelayedControls::DelayedControls(V4L2Device *device,\n> \n> \t\tconst ControlId *id = it->first;\n> \n> -\t\tdelays_[id] = delay.second;\n> +\t\tcontrolParams_[id] = param.second;\n> \n> \t\tLOG(DelayedControls, Debug)\n> -\t\t\t<< \"Set a delay of \" << delays_[id]\n> +\t\t\t<< \"Set a delay of \" << controlParams_[id].delay\n> +\t\t\t<< \" and priority write flag \" << controlParams_[id].priorityWrite\n> \t\t\t<< \" for \" << id->name();\n> \n> -\t\tmaxDelay_ = std::max(maxDelay_, delays_[id]);\n> +\t\tmaxDelay_ = std::max(maxDelay_, controlParams_[id].delay);\n> \t}\n> \n> \treset();\n> @@ -97,8 +102,8 @@ void DelayedControls::reset()\n> \n> \t/* Retrieve control as reported by the device. */\n> \tstd::vector ids;\n> -\tfor (auto const &delay : delays_)\n> -\t\tids.push_back(delay.first->id());\n> +\tfor (auto const ¶m : controlParams_)\n> +\t\tids.push_back(param.first->id());\n> \n> \tControlList controls = device_->getControls(ids);\n> \n> @@ -140,7 +145,7 @@ bool DelayedControls::push(const ControlList &controls)\n> \n> \t\tconst ControlId *id = it->second;\n> \n> -\t\tif (delays_.find(id) == delays_.end())\n> +\t\tif (controlParams_.find(id) == controlParams_.end())\n> \t\t\treturn false;\n> \n> \t\tInfo &info = values_[id][queueCount_];\n> @@ -220,12 +225,27 @@ void DelayedControls::applyControls(uint32_t sequence)\n> \tControlList out(device_->controls());\n> \tfor (const auto &ctrl : values_) {\n> \t\tconst ControlId *id = ctrl.first;\n> -\t\tunsigned int delayDiff = maxDelay_ - delays_[id];\n> +\t\tunsigned int delayDiff = maxDelay_ - controlParams_[id].delay;\n> \t\tunsigned int index = std::max(0, writeCount_ - delayDiff);\n> \t\tconst Info &info = ctrl.second[index];\n> \n> \t\tif (info.updated) {\n> -\t\t\tout.set(id->id(), info);\n> +\t\t\tif (controlParams_[id].priorityWrite) {\n> +\t\t\t\t/*\n> +\t\t\t\t * This control must be written now, it could\n> +\t\t\t\t * affect validity of the other controls.\n> +\t\t\t\t */\n> +\t\t\t\tControlList priority(device_->controls());\n> +\t\t\t\tpriority.set(id->id(), info);\n> +\t\t\t\tdevice_->setControls(&priority);\n> +\t\t\t} else {\n> +\t\t\t\t/*\n> +\t\t\t\t * Batch up the list of controls and write them\n> +\t\t\t\t * at the end of the function.\n> +\t\t\t\t */\n> +\t\t\t\tout.set(id->id(), info);\n> +\t\t\t}\n> +\n> \t\t\tLOG(DelayedControls, Debug)\n> \t\t\t\t<< \"Setting \" << id->name()\n> \t\t\t\t<< \" to \" << info.toString()\n> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> index 3e6b88af4188..ac92f066a07e 100644\n> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> @@ -963,14 +963,14 @@ int PipelineHandlerIPU3::registerCameras()\n> \t\t * a sensor database. For now use generic values taken from\n> \t\t * the Raspberry Pi and listed as 'generic values'.\n> \t\t */\n> -\t\tstd::unordered_map delays = {\n> -\t\t\t{ V4L2_CID_ANALOGUE_GAIN, 1 },\n> -\t\t\t{ V4L2_CID_EXPOSURE, 2 },\n> +\t\tstd::unordered_map params = {\n> +\t\t\t{ V4L2_CID_ANALOGUE_GAIN, { 1, false } },\n> +\t\t\t{ V4L2_CID_EXPOSURE, { 2, false } },\n> \t\t};\n> \n> \t\tdata->delayedCtrls_ =\n> \t\t\tstd::make_unique(cio2->sensor()->device(),\n> -\t\t\t\t\t\t\t delays);\n> +\t\t\t\t\t\t\t params);\n> \t\tdata->cio2_.frameStart().connect(data->delayedCtrls_.get(),\n> \t\t\t\t\t\t &DelayedControls::applyControls);\n> \n> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> index a60415d93705..ba74ace183db 100644\n> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> @@ -1244,16 +1244,15 @@ int RPiCameraData::configureIPA(const CameraConfiguration *config)\n> \tif (result.params & ipa::RPi::ConfigSensorParams) {\n> \t\t/*\n> \t\t * Setup our delayed control writer with the sensor default\n> -\t\t * gain and exposure delays.\n> +\t\t * gain and exposure delays. Mark VBLANK for priority write.\n> \t\t */\n> -\t\tstd::unordered_map delays = {\n> -\t\t\t{ V4L2_CID_ANALOGUE_GAIN, result.sensorConfig.gainDelay },\n> -\t\t\t{ V4L2_CID_EXPOSURE, result.sensorConfig.exposureDelay },\n> -\t\t\t{ V4L2_CID_VBLANK, result.sensorConfig.vblank }\n> +\t\tstd::unordered_map params = {\n> +\t\t\t{ V4L2_CID_ANALOGUE_GAIN, { result.sensorConfig.gainDelay, false } },\n> +\t\t\t{ V4L2_CID_EXPOSURE, { result.sensorConfig.exposureDelay, false } },\n> +\t\t\t{ V4L2_CID_VBLANK, { result.sensorConfig.vblank, true } }\n> \t\t};\n> \n> -\t\tdelayedCtrls_ = std::make_unique(unicam_[Unicam::Image].dev(), delays);\n> -\n> +\t\tdelayedCtrls_ = std::make_unique(unicam_[Unicam::Image].dev(), params);\n> \t\tsensorMetadata_ = result.sensorConfig.sensorMetadata;\n> \t}\n> \n> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> index a794501a9c8d..17c0f9751cd3 100644\n> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> @@ -938,14 +938,14 @@ int PipelineHandlerRkISP1::createCamera(MediaEntity *sensor)\n> \t * a sensor database. For now use generic values taken from\n> \t * the Raspberry Pi and listed as generic values.\n> \t */\n> -\tstd::unordered_map delays = {\n> -\t\t{ V4L2_CID_ANALOGUE_GAIN, 1 },\n> -\t\t{ V4L2_CID_EXPOSURE, 2 },\n> +\tstd::unordered_map params = {\n> +\t\t{ V4L2_CID_ANALOGUE_GAIN, { 1, false } },\n> +\t\t{ V4L2_CID_EXPOSURE, { 2, false } },\n> \t};\n> \n> \tdata->delayedCtrls_ =\n> \t\tstd::make_unique(data->sensor_->device(),\n> -\t\t\t\t\t\t delays);\n> +\t\t\t\t\t\t params);\n> \tisp_->frameStart.connect(data->delayedCtrls_.get(),\n> \t\t\t\t &DelayedControls::applyControls);\n> \n> diff --git a/test/delayed_contols.cpp b/test/delayed_contols.cpp\n> index 50169b12e566..3855eb18ecd4 100644\n> --- a/test/delayed_contols.cpp\n> +++ b/test/delayed_contols.cpp\n> @@ -72,8 +72,8 @@ protected:\n> \n> \tint singleControlNoDelay()\n> \t{\n> -\t\tstd::unordered_map delays = {\n> -\t\t\t{ V4L2_CID_BRIGHTNESS, 0 },\n> +\t\tstd::unordered_map delays = {\n> +\t\t\t{ V4L2_CID_BRIGHTNESS, { 0, false } },\n> \t\t};\n> \t\tstd::unique_ptr delayed =\n> \t\t\tstd::make_unique(dev_.get(), delays);\n> @@ -109,8 +109,8 @@ protected:\n> \n> \tint singleControlWithDelay()\n> \t{\n> -\t\tstd::unordered_map delays = {\n> -\t\t\t{ V4L2_CID_BRIGHTNESS, 1 },\n> +\t\tstd::unordered_map delays = {\n> +\t\t\t{ V4L2_CID_BRIGHTNESS, { 1, false } },\n> \t\t};\n> \t\tstd::unique_ptr delayed =\n> \t\t\tstd::make_unique(dev_.get(), delays);\n> @@ -150,9 +150,9 @@ protected:\n> \n> \tint dualControlsWithDelay(uint32_t startOffset)\n> \t{\n> -\t\tstd::unordered_map delays = {\n> -\t\t\t{ V4L2_CID_BRIGHTNESS, 1 },\n> -\t\t\t{ V4L2_CID_CONTRAST, 2 },\n> +\t\tstd::unordered_map delays = {\n> +\t\t\t{ V4L2_CID_BRIGHTNESS, { 1, false } },\n> +\t\t\t{ V4L2_CID_CONTRAST, { 2, false } },\n> \t\t};\n> \t\tstd::unique_ptr delayed =\n> \t\t\tstd::make_unique(dev_.get(), delays);\n> @@ -197,9 +197,9 @@ protected:\n> \n> \tint dualControlsMultiQueue()\n> \t{\n> -\t\tstd::unordered_map delays = {\n> -\t\t\t{ V4L2_CID_BRIGHTNESS, 1 },\n> -\t\t\t{ V4L2_CID_CONTRAST, 2 },\n> +\t\tstd::unordered_map delays = {\n> +\t\t\t{ V4L2_CID_BRIGHTNESS, { 1, false } },\n> +\t\t\t{ V4L2_CID_CONTRAST, { 2, false } }\n> \t\t};\n> \t\tstd::unique_ptr delayed =\n> \t\t\tstd::make_unique(dev_.get(), delays);\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 E6801BD80C\n\tfor ;\n\tTue, 9 Mar 2021 09:54:44 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 43E0D68AA3;\n\tTue, 9 Mar 2021 10:54:44 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id B04C96051F\n\tfor ;\n\tTue, 9 Mar 2021 10:54:42 +0100 (CET)","from [IPv6:2a01:e0a:169:7140:eb55:7706:22b:1e57] (unknown\n\t[IPv6:2a01:e0a:169:7140:eb55:7706:22b:1e57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 4F262E9;\n\tTue, 9 Mar 2021 10:54:42 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"PH2yS2Md\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1615283682;\n\tbh=M5HKujWZkERuBkh3cvTuwWOSx0yNdBJwTcaty+ZYEoE=;\n\th=Subject:To:References:From:Date:In-Reply-To:From;\n\tb=PH2yS2MdwtVgFFRNgo8IBorZeKHx5kCG7Ar9kqdO8MsD+JXuaWdn7OCLbISJhgFEM\n\t4dHJKyMXBLPbTFRt7hM+G+jYPnU/R1ZB5cI8Ik/52UI124E07KGhjb4+AcrGxmZMn1\n\trLbJI0zFjxwaw+vPsL57yNDtcTsSf6yErqKLbr0I=","To":"Naushir Patuck ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20210304081728.1058394-1-naush@raspberrypi.com>\n\t<20210304081728.1058394-2-naush@raspberrypi.com>","From":"Jean-Michel Hautbois ","Message-ID":"<82c5f6c6-e6ea-80d9-8ac3-3a403a707b75@ideasonboard.com>","Date":"Tue, 9 Mar 2021 10:54:41 +0100","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101\n\tThunderbird/78.7.1","MIME-Version":"1.0","In-Reply-To":"<20210304081728.1058394-2-naush@raspberrypi.com>","Content-Language":"en-US","Subject":"Re: [libcamera-devel] [PATCH v4 1/7] libcamera: delayed_controls:\n\tAdd notion of priority write","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","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":15613,"web_url":"https://patchwork.libcamera.org/comment/15613/","msgid":"<7532d1e4-d01c-4b1f-9c82-28e070120a5a@ideasonboard.com>","date":"2021-03-12T13:25:13","subject":"Re: [libcamera-devel] [PATCH v4 1/7] libcamera: delayed_controls:\n\tAdd notion of priority write","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Naush,\n\nOn 09/03/2021 09:54, Jean-Michel Hautbois wrote:\n> Hi Naushir,\n> \n> Thanks for the patch !\n> \n> On 04/03/2021 09:17, Naushir Patuck wrote:\n>> If an exposure time change adjusts the vblanking limits, and we set both\n>> VBLANK and EXPOSURE controls through the VIDIOC_S_EXT_CTRLS ioctl, the\n>> latter may fail if the value is outside of the limits calculated by the\n>> old VBLANK value. This is a limitation in V4L2 and cannot be fixed by\n>> setting VBLANK before EXPOSURE in a single VIDIOC_S_EXT_CTRLS ioctl.\n>>\n>> The workaround here is to have the DelayedControls object mark the\n>> VBLANK control as \"priority write\", which then write VBLANK separately\n>> from (and ahead of) any other controls. This way, the sensor driver will\n>> update the EXPOSURE control with new limits before the new values is\n>> presented, and will thus be seen as valid.\n>>\n>> To support this, a new struct DelayedControls::ControlParams is used in\n>> the constructor to provide the control delay value as well as the\n>> priority write flag.\n>>\n>> Signed-off-by: Naushir Patuck \n>> Tested-by: David Plowman \n>> Reviewed-by: Paul Elder \n> Tested-by: Jean-Michel Hautbois \n> \n>> ---\n>> include/libcamera/internal/delayed_controls.h | 9 +++-\n>> src/libcamera/delayed_controls.cpp | 54 +++++++++++++------\n>> src/libcamera/pipeline/ipu3/ipu3.cpp | 8 +--\n>> .../pipeline/raspberrypi/raspberrypi.cpp | 13 +++--\n>> src/libcamera/pipeline/rkisp1/rkisp1.cpp | 8 +--\n>> test/delayed_contols.cpp | 20 +++----\n>> 6 files changed, 68 insertions(+), 44 deletions(-)\n>>\n>> diff --git a/include/libcamera/internal/delayed_controls.h b/include/libcamera/internal/delayed_controls.h\n>> index dc447a882514..564d9f2e2440 100644\n>> --- a/include/libcamera/internal/delayed_controls.h\n>> +++ b/include/libcamera/internal/delayed_controls.h\n>> @@ -19,8 +19,13 @@ class V4L2Device;\n>> class DelayedControls\n>> {\n>> public:\n>> +\tstruct ControlParams {\n>> +\t\tunsigned int delay;\n>> +\t\tbool priorityWrite;\n>> +\t};\n>> +\n>> \tDelayedControls(V4L2Device *device,\n>> -\t\t\tconst std::unordered_map &delays);\n>> +\t\t\tconst std::unordered_map &controlParams);\n>> \n>> \tvoid reset();\n>> \n>> @@ -64,7 +69,7 @@ private:\n>> \n>> \tV4L2Device *device_;\n>> \t/* \\todo Evaluate if we should index on ControlId * or unsigned int */\n>> -\tstd::unordered_map delays_;\n>> +\tstd::unordered_map controlParams_;\n>> \tunsigned int maxDelay_;\n>> \n>> \tbool running_;\n>> diff --git a/src/libcamera/delayed_controls.cpp b/src/libcamera/delayed_controls.cpp\n>> index ab1d40057c5f..3ed1dfebd035 100644\n>> --- a/src/libcamera/delayed_controls.cpp\n>> +++ b/src/libcamera/delayed_controls.cpp\n>> @@ -40,15 +40,19 @@ LOG_DEFINE_CATEGORY(DelayedControls)\n>> /**\n>> * \\brief Construct a DelayedControls instance\n>> * \\param[in] device The V4L2 device the controls have to be applied to\n>> - * \\param[in] delays Map of the numerical V4L2 control ids to their associated\n>> - * delays (in frames)\n>> + * \\param[in] controlParams Map of the numerical V4L2 control ids to their\n>> + * associated control parameters.\n>> *\n>> - * Only controls specified in \\a delays are handled. If it's desired to mix\n>> - * delayed controls and controls that take effect immediately the immediate\n>> - * controls must be listed in the \\a delays map with a delay value of 0.\n>> + * The control parameters comprise of delays (in frames) and a priority write\n>> + * flag. If this flag is set, the relevant control is written separately from,\n>> + * ahead of the reset of the batched controls.\n\nminor:\n\n'and ahead of' ?\n\ncould be fixed while applying if there's nothing else major:\n\nReviewed-by: Kieran Bingham \n\n\n\n>> + *\n>> + * Only controls specified in \\a controlParams are handled. If it's desired to\n>> + * mix delayed controls and controls that take effect immediately the immediate\n>> + * controls must be listed in the \\a controlParams map with a delay value of 0.\n>> */\n>> DelayedControls::DelayedControls(V4L2Device *device,\n>> -\t\t\t\t const std::unordered_map &delays)\n>> +\t\t\t\t const std::unordered_map &controlParams)\n>> \t: device_(device), maxDelay_(0)\n>> {\n>> \tconst ControlInfoMap &controls = device_->controls();\n>> @@ -57,12 +61,12 @@ DelayedControls::DelayedControls(V4L2Device *device,\n>> \t * Create a map of control ids to delays for controls exposed by the\n>> \t * device.\n>> \t */\n>> -\tfor (auto const &delay : delays) {\n>> -\t\tauto it = controls.find(delay.first);\n>> +\tfor (auto const ¶m : controlParams) {\n>> +\t\tauto it = controls.find(param.first);\n>> \t\tif (it == controls.end()) {\n>> \t\t\tLOG(DelayedControls, Error)\n>> \t\t\t\t<< \"Delay request for control id \"\n>> -\t\t\t\t<< utils::hex(delay.first)\n>> +\t\t\t\t<< utils::hex(param.first)\n>> \t\t\t\t<< \" but control is not exposed by device \"\n>> \t\t\t\t<< device_->deviceNode();\n>> \t\t\tcontinue;\n>> @@ -70,13 +74,14 @@ DelayedControls::DelayedControls(V4L2Device *device,\n>> \n>> \t\tconst ControlId *id = it->first;\n>> \n>> -\t\tdelays_[id] = delay.second;\n>> +\t\tcontrolParams_[id] = param.second;\n>> \n>> \t\tLOG(DelayedControls, Debug)\n>> -\t\t\t<< \"Set a delay of \" << delays_[id]\n>> +\t\t\t<< \"Set a delay of \" << controlParams_[id].delay\n>> +\t\t\t<< \" and priority write flag \" << controlParams_[id].priorityWrite\n>> \t\t\t<< \" for \" << id->name();\n>> \n>> -\t\tmaxDelay_ = std::max(maxDelay_, delays_[id]);\n>> +\t\tmaxDelay_ = std::max(maxDelay_, controlParams_[id].delay);\n>> \t}\n>> \n>> \treset();\n>> @@ -97,8 +102,8 @@ void DelayedControls::reset()\n>> \n>> \t/* Retrieve control as reported by the device. */\n>> \tstd::vector ids;\n>> -\tfor (auto const &delay : delays_)\n>> -\t\tids.push_back(delay.first->id());\n>> +\tfor (auto const ¶m : controlParams_)\n>> +\t\tids.push_back(param.first->id());\n>> \n>> \tControlList controls = device_->getControls(ids);\n>> \n>> @@ -140,7 +145,7 @@ bool DelayedControls::push(const ControlList &controls)\n>> \n>> \t\tconst ControlId *id = it->second;\n>> \n>> -\t\tif (delays_.find(id) == delays_.end())\n>> +\t\tif (controlParams_.find(id) == controlParams_.end())\n>> \t\t\treturn false;\n>> \n>> \t\tInfo &info = values_[id][queueCount_];\n>> @@ -220,12 +225,27 @@ void DelayedControls::applyControls(uint32_t sequence)\n>> \tControlList out(device_->controls());\n>> \tfor (const auto &ctrl : values_) {\n>> \t\tconst ControlId *id = ctrl.first;\n>> -\t\tunsigned int delayDiff = maxDelay_ - delays_[id];\n>> +\t\tunsigned int delayDiff = maxDelay_ - controlParams_[id].delay;\n>> \t\tunsigned int index = std::max(0, writeCount_ - delayDiff);\n>> \t\tconst Info &info = ctrl.second[index];\n>> \n>> \t\tif (info.updated) {\n>> -\t\t\tout.set(id->id(), info);\n>> +\t\t\tif (controlParams_[id].priorityWrite) {\n>> +\t\t\t\t/*\n>> +\t\t\t\t * This control must be written now, it could\n>> +\t\t\t\t * affect validity of the other controls.\n>> +\t\t\t\t */\n>> +\t\t\t\tControlList priority(device_->controls());\n>> +\t\t\t\tpriority.set(id->id(), info);\n>> +\t\t\t\tdevice_->setControls(&priority);\n>> +\t\t\t} else {\n>> +\t\t\t\t/*\n>> +\t\t\t\t * Batch up the list of controls and write them\n>> +\t\t\t\t * at the end of the function.\n>> +\t\t\t\t */\n>> +\t\t\t\tout.set(id->id(), info);\n>> +\t\t\t}\n>> +\n>> \t\t\tLOG(DelayedControls, Debug)\n>> \t\t\t\t<< \"Setting \" << id->name()\n>> \t\t\t\t<< \" to \" << info.toString()\n>> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n>> index 3e6b88af4188..ac92f066a07e 100644\n>> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n>> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n>> @@ -963,14 +963,14 @@ int PipelineHandlerIPU3::registerCameras()\n>> \t\t * a sensor database. For now use generic values taken from\n>> \t\t * the Raspberry Pi and listed as 'generic values'.\n>> \t\t */\n>> -\t\tstd::unordered_map delays = {\n>> -\t\t\t{ V4L2_CID_ANALOGUE_GAIN, 1 },\n>> -\t\t\t{ V4L2_CID_EXPOSURE, 2 },\n>> +\t\tstd::unordered_map params = {\n>> +\t\t\t{ V4L2_CID_ANALOGUE_GAIN, { 1, false } },\n>> +\t\t\t{ V4L2_CID_EXPOSURE, { 2, false } },\n>> \t\t};\n>> \n>> \t\tdata->delayedCtrls_ =\n>> \t\t\tstd::make_unique(cio2->sensor()->device(),\n>> -\t\t\t\t\t\t\t delays);\n>> +\t\t\t\t\t\t\t params);\n>> \t\tdata->cio2_.frameStart().connect(data->delayedCtrls_.get(),\n>> \t\t\t\t\t\t &DelayedControls::applyControls);\n>> \n>> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n>> index a60415d93705..ba74ace183db 100644\n>> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n>> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n>> @@ -1244,16 +1244,15 @@ int RPiCameraData::configureIPA(const CameraConfiguration *config)\n>> \tif (result.params & ipa::RPi::ConfigSensorParams) {\n>> \t\t/*\n>> \t\t * Setup our delayed control writer with the sensor default\n>> -\t\t * gain and exposure delays.\n>> +\t\t * gain and exposure delays. Mark VBLANK for priority write.\n>> \t\t */\n>> -\t\tstd::unordered_map delays = {\n>> -\t\t\t{ V4L2_CID_ANALOGUE_GAIN, result.sensorConfig.gainDelay },\n>> -\t\t\t{ V4L2_CID_EXPOSURE, result.sensorConfig.exposureDelay },\n>> -\t\t\t{ V4L2_CID_VBLANK, result.sensorConfig.vblank }\n>> +\t\tstd::unordered_map params = {\n>> +\t\t\t{ V4L2_CID_ANALOGUE_GAIN, { result.sensorConfig.gainDelay, false } },\n>> +\t\t\t{ V4L2_CID_EXPOSURE, { result.sensorConfig.exposureDelay, false } },\n>> +\t\t\t{ V4L2_CID_VBLANK, { result.sensorConfig.vblank, true } }\n>> \t\t};\n>> \n>> -\t\tdelayedCtrls_ = std::make_unique(unicam_[Unicam::Image].dev(), delays);\n>> -\n>> +\t\tdelayedCtrls_ = std::make_unique(unicam_[Unicam::Image].dev(), params);\n>> \t\tsensorMetadata_ = result.sensorConfig.sensorMetadata;\n>> \t}\n>> \n>> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n>> index a794501a9c8d..17c0f9751cd3 100644\n>> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n>> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n>> @@ -938,14 +938,14 @@ int PipelineHandlerRkISP1::createCamera(MediaEntity *sensor)\n>> \t * a sensor database. For now use generic values taken from\n>> \t * the Raspberry Pi and listed as generic values.\n>> \t */\n>> -\tstd::unordered_map delays = {\n>> -\t\t{ V4L2_CID_ANALOGUE_GAIN, 1 },\n>> -\t\t{ V4L2_CID_EXPOSURE, 2 },\n>> +\tstd::unordered_map params = {\n>> +\t\t{ V4L2_CID_ANALOGUE_GAIN, { 1, false } },\n>> +\t\t{ V4L2_CID_EXPOSURE, { 2, false } },\n>> \t};\n>> \n>> \tdata->delayedCtrls_ =\n>> \t\tstd::make_unique(data->sensor_->device(),\n>> -\t\t\t\t\t\t delays);\n>> +\t\t\t\t\t\t params);\n>> \tisp_->frameStart.connect(data->delayedCtrls_.get(),\n>> \t\t\t\t &DelayedControls::applyControls);\n>> \n>> diff --git a/test/delayed_contols.cpp b/test/delayed_contols.cpp\n>> index 50169b12e566..3855eb18ecd4 100644\n>> --- a/test/delayed_contols.cpp\n>> +++ b/test/delayed_contols.cpp\n>> @@ -72,8 +72,8 @@ protected:\n>> \n>> \tint singleControlNoDelay()\n>> \t{\n>> -\t\tstd::unordered_map delays = {\n>> -\t\t\t{ V4L2_CID_BRIGHTNESS, 0 },\n>> +\t\tstd::unordered_map delays = {\n>> +\t\t\t{ V4L2_CID_BRIGHTNESS, { 0, false } },\n>> \t\t};\n>> \t\tstd::unique_ptr delayed =\n>> \t\t\tstd::make_unique(dev_.get(), delays);\n>> @@ -109,8 +109,8 @@ protected:\n>> \n>> \tint singleControlWithDelay()\n>> \t{\n>> -\t\tstd::unordered_map delays = {\n>> -\t\t\t{ V4L2_CID_BRIGHTNESS, 1 },\n>> +\t\tstd::unordered_map delays = {\n>> +\t\t\t{ V4L2_CID_BRIGHTNESS, { 1, false } },\n>> \t\t};\n>> \t\tstd::unique_ptr delayed =\n>> \t\t\tstd::make_unique(dev_.get(), delays);\n>> @@ -150,9 +150,9 @@ protected:\n>> \n>> \tint dualControlsWithDelay(uint32_t startOffset)\n>> \t{\n>> -\t\tstd::unordered_map delays = {\n>> -\t\t\t{ V4L2_CID_BRIGHTNESS, 1 },\n>> -\t\t\t{ V4L2_CID_CONTRAST, 2 },\n>> +\t\tstd::unordered_map delays = {\n>> +\t\t\t{ V4L2_CID_BRIGHTNESS, { 1, false } },\n>> +\t\t\t{ V4L2_CID_CONTRAST, { 2, false } },\n>> \t\t};\n>> \t\tstd::unique_ptr delayed =\n>> \t\t\tstd::make_unique(dev_.get(), delays);\n>> @@ -197,9 +197,9 @@ protected:\n>> \n>> \tint dualControlsMultiQueue()\n>> \t{\n>> -\t\tstd::unordered_map delays = {\n>> -\t\t\t{ V4L2_CID_BRIGHTNESS, 1 },\n>> -\t\t\t{ V4L2_CID_CONTRAST, 2 },\n>> +\t\tstd::unordered_map delays = {\n>> +\t\t\t{ V4L2_CID_BRIGHTNESS, { 1, false } },\n>> +\t\t\t{ V4L2_CID_CONTRAST, { 2, false } }\n>> \t\t};\n>> \t\tstd::unique_ptr delayed =\n>> \t\t\tstd::make_unique(dev_.get(), delays);\n>>\n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel\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 E9053BD1F1\n\tfor ;\n\tFri, 12 Mar 2021 13:25:17 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5F46568C67;\n\tFri, 12 Mar 2021 14:25:17 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 7AF1368AA1\n\tfor ;\n\tFri, 12 Mar 2021 14:25:16 +0100 (CET)","from [192.168.0.20]\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 E51E4A2A;\n\tFri, 12 Mar 2021 14:25:15 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"PvUHK17r\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1615555516;\n\tbh=ZOoGLHKN2IRZZneb17QrSDra3UKgfrnSHj/pcVxHqFE=;\n\th=Reply-To:Subject:To:References:From:Date:In-Reply-To:From;\n\tb=PvUHK17rMvwPYYsnldZ6KK9SlLI9hUJVK071Pujk5MdFKAOVbXiginhdIgQ3GpF3K\n\tq8RlFJJsrq5/OAty0FeGsfyH0epHw0Ac91fCd08Y+6lJsf9Gd6Xog6/sblZ2oaBx8A\n\tZS/NC9nnhhnllVuoTgbncNopEolCroYIgoIgX1eA=","To":"Jean-Michel Hautbois ,\n\tNaushir Patuck ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20210304081728.1058394-1-naush@raspberrypi.com>\n\t<20210304081728.1058394-2-naush@raspberrypi.com>\n\t<82c5f6c6-e6ea-80d9-8ac3-3a403a707b75@ideasonboard.com>","From":"Kieran Bingham ","Autocrypt":"addr=kieran.bingham@ideasonboard.com; keydata=\n\tmQINBFYE/WYBEACs1PwjMD9rgCu1hlIiUA1AXR4rv2v+BCLUq//vrX5S5bjzxKAryRf0uHat\n\tV/zwz6hiDrZuHUACDB7X8OaQcwhLaVlq6byfoBr25+hbZG7G3+5EUl9cQ7dQEdvNj6V6y/SC\n\trRanWfelwQThCHckbobWiQJfK9n7rYNcPMq9B8e9F020LFH7Kj6YmO95ewJGgLm+idg1Kb3C\n\tpotzWkXc1xmPzcQ1fvQMOfMwdS+4SNw4rY9f07Xb2K99rjMwZVDgESKIzhsDB5GY465sCsiQ\n\tcSAZRxqE49RTBq2+EQsbrQpIc8XiffAB8qexh5/QPzCmR4kJgCGeHIXBtgRj+nIkCJPZvZtf\n\tKr2EAbc6tgg6DkAEHJb+1okosV09+0+TXywYvtEop/WUOWQ+zo+Y/OBd+8Ptgt1pDRyOBzL8\n\tRXa8ZqRf0Mwg75D+dKntZeJHzPRJyrlfQokngAAs4PaFt6UfS+ypMAF37T6CeDArQC41V3ko\n\tlPn1yMsVD0p+6i3DPvA/GPIksDC4owjnzVX9kM8Zc5Cx+XoAN0w5Eqo4t6qEVbuettxx55gq\n\t8K8FieAjgjMSxngo/HST8TpFeqI5nVeq0/lqtBRQKumuIqDg+Bkr4L1V/PSB6XgQcOdhtd36\n\tOe9X9dXB8YSNt7VjOcO7BTmFn/Z8r92mSAfHXpb07YJWJosQOQARAQABtDBLaWVyYW4gQmlu\n\tZ2hhbSA8a2llcmFuLmJpbmdoYW1AaWRlYXNvbmJvYXJkLmNvbT6JAlcEEwEKAEECGwMFCwkI\n\tBwIGFQgJCgsCBBYCAwECHgECF4ACGQEWIQSQLdeYP70o/eNy1HqhHkZyEKRh/QUCXWTtygUJ\n\tCyJXZAAKCRChHkZyEKRh/f8dEACTDsbLN2nioNZMwyLuQRUAFcXNolDX48xcUXsWS2QjxaPm\n\tVsJx8Uy8aYkS85mdPBh0C83OovQR/OVbr8AxhGvYqBs3nQvbWuTl/+4od7DfK2VZOoKBAu5S\n\tQK2FYuUcikDqYcFWJ8DQnubxfE8dvzojHEkXw0sA4igINHDDFX3HJGZtLio+WpEFQtCbfTAG\n\tYZslasz1YZRbwEdSsmO3/kqy5eMnczlm8a21A3fKUo3g8oAZEFM+f4DUNzqIltg31OAB/kZS\n\tenKZQ/SWC8PmLg/ZXBrReYakxXtkP6w3FwMlzOlhGxqhIRNiAJfXJBaRhuUWzPOpEDE9q5YJ\n\tBmqQL2WJm1VSNNVxbXJHpaWMH1sA2R00vmvRrPXGwyIO0IPYeUYQa3gsy6k+En/aMQJd27dp\n\taScf9am9PFICPY5T4ppneeJLif2lyLojo0mcHOV+uyrds9XkLpp14GfTkeKPdPMrLLTsHRfH\n\tfA4I4OBpRrEPiGIZB/0im98MkGY/Mu6qxeZmYLCcgD6qz4idOvfgVOrNh+aA8HzIVR+RMW8H\n\tQGBN9f0E3kfwxuhl3omo6V7lDw8XOdmuWZNC9zPq1UfryVHANYbLGz9KJ4Aw6M+OgBC2JpkD\n\thXMdHUkC+d20dwXrwHTlrJi1YNp6rBc+xald3wsUPOZ5z8moTHUX/uPA/qhGsbkCDQRWBP1m\n\tARAAzijkb+Sau4hAncr1JjOY+KyFEdUNxRy+hqTJdJfaYihxyaj0Ee0P0zEi35CbE6lgU0Uz\n\ttih9fiUbSV3wfsWqg1Ut3/5rTKu7kLFp15kF7eqvV4uezXRD3Qu4yjv/rMmEJbbD4cTvGCYI\n\td6MDC417f7vK3hCbCVIZSp3GXxyC1LU+UQr3fFcOyCwmP9vDUR9JV0BSqHHxRDdpUXE26Dk6\n\tmhf0V1YkspE5St814ETXpEus2urZE5yJIUROlWPIL+hm3NEWfAP06vsQUyLvr/GtbOT79vXl\n\tEn1aulcYyu20dRRxhkQ6iILaURcxIAVJJKPi8dsoMnS8pB0QW12AHWuirPF0g6DiuUfPmrA5\n\tPKe56IGlpkjc8cO51lIxHkWTpCMWigRdPDexKX+Sb+W9QWK/0JjIc4t3KBaiG8O4yRX8ml2R\n\t+rxfAVKM6V769P/hWoRGdgUMgYHFpHGSgEt80OKK5HeUPy2cngDUXzwrqiM5Sz6Od0qw5pCk\n\tNlXqI0W/who0iSVM+8+RmyY0OEkxEcci7rRLsGnM15B5PjLJjh1f2ULYkv8s4SnDwMZ/kE04\n\t/UqCMK/KnX8pwXEMCjz0h6qWNpGwJ0/tYIgQJZh6bqkvBrDogAvuhf60Sogw+mH8b+PBlx1L\n\toeTK396wc+4c3BfiC6pNtUS5GpsPMMjYMk7kVvEAEQEAAYkCPAQYAQoAJgIbDBYhBJAt15g/\n\tvSj943LUeqEeRnIQpGH9BQJdizzIBQkLSKZiAAoJEKEeRnIQpGH9eYgQAJpjaWNgqNOnMTmD\n\tMJggbwjIotypzIXfhHNCeTkG7+qCDlSaBPclcPGYrTwCt0YWPU2TgGgJrVhYT20ierN8LUvj\n\t6qOPTd+Uk7NFzL65qkh80ZKNBFddx1AabQpSVQKbdcLb8OFs85kuSvFdgqZwgxA1vl4TFhNz\n\tPZ79NAmXLackAx3sOVFhk4WQaKRshCB7cSl+RIng5S/ThOBlwNlcKG7j7W2MC06BlTbdEkUp\n\tECzuuRBv8wX4OQl+hbWbB/VKIx5HKlLu1eypen/5lNVzSqMMIYkkZcjV2SWQyUGxSwq0O/sx\n\tS0A8/atCHUXOboUsn54qdxrVDaK+6jIAuo8JiRWctP16KjzUM7MO0/+4zllM8EY57rXrj48j\n\tsbEYX0YQnzaj+jO6kJtoZsIaYR7rMMq9aUAjyiaEZpmP1qF/2sYenDx0Fg2BSlLvLvXM0vU8\n\tpQk3kgDu7kb/7PRYrZvBsr21EIQoIjXbZxDz/o7z95frkP71EaICttZ6k9q5oxxA5WC6sTXc\n\tMW8zs8avFNuA9VpXt0YupJd2ijtZy2mpZNG02fFVXhIn4G807G7+9mhuC4XG5rKlBBUXTvPU\n\tAfYnB4JBDLmLzBFavQfvonSfbitgXwCG3vS+9HEwAjU30Bar1PEOmIbiAoMzuKeRm2LVpmq4\n\tWZw01QYHU/GUV/zHJSFk","Organization":"Ideas on Board","Message-ID":"<7532d1e4-d01c-4b1f-9c82-28e070120a5a@ideasonboard.com>","Date":"Fri, 12 Mar 2021 13:25:13 +0000","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101\n\tThunderbird/68.10.0","MIME-Version":"1.0","In-Reply-To":"<82c5f6c6-e6ea-80d9-8ac3-3a403a707b75@ideasonboard.com>","Content-Language":"en-GB","Subject":"Re: [libcamera-devel] [PATCH v4 1/7] libcamera: delayed_controls:\n\tAdd notion of priority write","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","Reply-To":"kieran.bingham@ideasonboard.com","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":15621,"web_url":"https://patchwork.libcamera.org/comment/15621/","msgid":"","date":"2021-03-12T14:00:02","subject":"Re: [libcamera-devel] [PATCH v4 1/7] libcamera: delayed_controls:\n\tAdd notion of priority write","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"On 12/03/2021 13:25, Kieran Bingham wrote:\n> Hi Naush,\n> \n> On 09/03/2021 09:54, Jean-Michel Hautbois wrote:\n>> Hi Naushir,\n>>\n>> Thanks for the patch !\n>>\n>> On 04/03/2021 09:17, Naushir Patuck wrote:\n>>> If an exposure time change adjusts the vblanking limits, and we set both\n>>> VBLANK and EXPOSURE controls through the VIDIOC_S_EXT_CTRLS ioctl, the\n>>> latter may fail if the value is outside of the limits calculated by the\n>>> old VBLANK value. This is a limitation in V4L2 and cannot be fixed by\n>>> setting VBLANK before EXPOSURE in a single VIDIOC_S_EXT_CTRLS ioctl.\n>>>\n>>> The workaround here is to have the DelayedControls object mark the\n>>> VBLANK control as \"priority write\", which then write VBLANK separately\n>>> from (and ahead of) any other controls. This way, the sensor driver will\n>>> update the EXPOSURE control with new limits before the new values is\n>>> presented, and will thus be seen as valid.\n>>>\n>>> To support this, a new struct DelayedControls::ControlParams is used in\n>>> the constructor to provide the control delay value as well as the\n>>> priority write flag.\n>>>\n>>> Signed-off-by: Naushir Patuck \n>>> Tested-by: David Plowman \n>>> Reviewed-by: Paul Elder \n>> Tested-by: Jean-Michel Hautbois \n>>\n>>> ---\n>>> include/libcamera/internal/delayed_controls.h | 9 +++-\n>>> src/libcamera/delayed_controls.cpp | 54 +++++++++++++------\n>>> src/libcamera/pipeline/ipu3/ipu3.cpp | 8 +--\n>>> .../pipeline/raspberrypi/raspberrypi.cpp | 13 +++--\n>>> src/libcamera/pipeline/rkisp1/rkisp1.cpp | 8 +--\n>>> test/delayed_contols.cpp | 20 +++----\n>>> 6 files changed, 68 insertions(+), 44 deletions(-)\n>>>\n>>> diff --git a/include/libcamera/internal/delayed_controls.h b/include/libcamera/internal/delayed_controls.h\n>>> index dc447a882514..564d9f2e2440 100644\n>>> --- a/include/libcamera/internal/delayed_controls.h\n>>> +++ b/include/libcamera/internal/delayed_controls.h\n>>> @@ -19,8 +19,13 @@ class V4L2Device;\n>>> class DelayedControls\n>>> {\n>>> public:\n>>> +\tstruct ControlParams {\n>>> +\t\tunsigned int delay;\n>>> +\t\tbool priorityWrite;\n>>> +\t};\n>>> +\n>>> \tDelayedControls(V4L2Device *device,\n>>> -\t\t\tconst std::unordered_map &delays);\n>>> +\t\t\tconst std::unordered_map &controlParams);\n>>> \n>>> \tvoid reset();\n>>> \n>>> @@ -64,7 +69,7 @@ private:\n>>> \n>>> \tV4L2Device *device_;\n>>> \t/* \\todo Evaluate if we should index on ControlId * or unsigned int */\n>>> -\tstd::unordered_map delays_;\n>>> +\tstd::unordered_map controlParams_;\n>>> \tunsigned int maxDelay_;\n>>> \n>>> \tbool running_;\n>>> diff --git a/src/libcamera/delayed_controls.cpp b/src/libcamera/delayed_controls.cpp\n>>> index ab1d40057c5f..3ed1dfebd035 100644\n>>> --- a/src/libcamera/delayed_controls.cpp\n>>> +++ b/src/libcamera/delayed_controls.cpp\n>>> @@ -40,15 +40,19 @@ LOG_DEFINE_CATEGORY(DelayedControls)\n>>> /**\n>>> * \\brief Construct a DelayedControls instance\n>>> * \\param[in] device The V4L2 device the controls have to be applied to\n>>> - * \\param[in] delays Map of the numerical V4L2 control ids to their associated\n>>> - * delays (in frames)\n>>> + * \\param[in] controlParams Map of the numerical V4L2 control ids to their\n>>> + * associated control parameters.\n>>> *\n>>> - * Only controls specified in \\a delays are handled. If it's desired to mix\n>>> - * delayed controls and controls that take effect immediately the immediate\n>>> - * controls must be listed in the \\a delays map with a delay value of 0.\n>>> + * The control parameters comprise of delays (in frames) and a priority write\n>>> + * flag. If this flag is set, the relevant control is written separately from,\n>>> + * ahead of the reset of the batched controls.\n> \n> minor:\n> \n> 'and ahead of' ?\n> \n> could be fixed while applying if there's nothing else major:\n\n\nCould you confirm if this is correct to say 'and ahead of the reset of'\nor if it was supposed to be 'and ahead of the rest of' ...\n\n\n> \n> Reviewed-by: Kieran Bingham \n> \n> \n> \n>>> + *\n>>> + * Only controls specified in \\a controlParams are handled. If it's desired to\n>>> + * mix delayed controls and controls that take effect immediately the immediate\n>>> + * controls must be listed in the \\a controlParams map with a delay value of 0.\n>>> */\n>>> DelayedControls::DelayedControls(V4L2Device *device,\n>>> -\t\t\t\t const std::unordered_map &delays)\n>>> +\t\t\t\t const std::unordered_map &controlParams)\n>>> \t: device_(device), maxDelay_(0)\n>>> {\n>>> \tconst ControlInfoMap &controls = device_->controls();\n>>> @@ -57,12 +61,12 @@ DelayedControls::DelayedControls(V4L2Device *device,\n>>> \t * Create a map of control ids to delays for controls exposed by the\n>>> \t * device.\n>>> \t */\n>>> -\tfor (auto const &delay : delays) {\n>>> -\t\tauto it = controls.find(delay.first);\n>>> +\tfor (auto const ¶m : controlParams) {\n>>> +\t\tauto it = controls.find(param.first);\n>>> \t\tif (it == controls.end()) {\n>>> \t\t\tLOG(DelayedControls, Error)\n>>> \t\t\t\t<< \"Delay request for control id \"\n>>> -\t\t\t\t<< utils::hex(delay.first)\n>>> +\t\t\t\t<< utils::hex(param.first)\n>>> \t\t\t\t<< \" but control is not exposed by device \"\n>>> \t\t\t\t<< device_->deviceNode();\n>>> \t\t\tcontinue;\n>>> @@ -70,13 +74,14 @@ DelayedControls::DelayedControls(V4L2Device *device,\n>>> \n>>> \t\tconst ControlId *id = it->first;\n>>> \n>>> -\t\tdelays_[id] = delay.second;\n>>> +\t\tcontrolParams_[id] = param.second;\n>>> \n>>> \t\tLOG(DelayedControls, Debug)\n>>> -\t\t\t<< \"Set a delay of \" << delays_[id]\n>>> +\t\t\t<< \"Set a delay of \" << controlParams_[id].delay\n>>> +\t\t\t<< \" and priority write flag \" << controlParams_[id].priorityWrite\n>>> \t\t\t<< \" for \" << id->name();\n>>> \n>>> -\t\tmaxDelay_ = std::max(maxDelay_, delays_[id]);\n>>> +\t\tmaxDelay_ = std::max(maxDelay_, controlParams_[id].delay);\n>>> \t}\n>>> \n>>> \treset();\n>>> @@ -97,8 +102,8 @@ void DelayedControls::reset()\n>>> \n>>> \t/* Retrieve control as reported by the device. */\n>>> \tstd::vector ids;\n>>> -\tfor (auto const &delay : delays_)\n>>> -\t\tids.push_back(delay.first->id());\n>>> +\tfor (auto const ¶m : controlParams_)\n>>> +\t\tids.push_back(param.first->id());\n>>> \n>>> \tControlList controls = device_->getControls(ids);\n>>> \n>>> @@ -140,7 +145,7 @@ bool DelayedControls::push(const ControlList &controls)\n>>> \n>>> \t\tconst ControlId *id = it->second;\n>>> \n>>> -\t\tif (delays_.find(id) == delays_.end())\n>>> +\t\tif (controlParams_.find(id) == controlParams_.end())\n>>> \t\t\treturn false;\n>>> \n>>> \t\tInfo &info = values_[id][queueCount_];\n>>> @@ -220,12 +225,27 @@ void DelayedControls::applyControls(uint32_t sequence)\n>>> \tControlList out(device_->controls());\n>>> \tfor (const auto &ctrl : values_) {\n>>> \t\tconst ControlId *id = ctrl.first;\n>>> -\t\tunsigned int delayDiff = maxDelay_ - delays_[id];\n>>> +\t\tunsigned int delayDiff = maxDelay_ - controlParams_[id].delay;\n>>> \t\tunsigned int index = std::max(0, writeCount_ - delayDiff);\n>>> \t\tconst Info &info = ctrl.second[index];\n>>> \n>>> \t\tif (info.updated) {\n>>> -\t\t\tout.set(id->id(), info);\n>>> +\t\t\tif (controlParams_[id].priorityWrite) {\n>>> +\t\t\t\t/*\n>>> +\t\t\t\t * This control must be written now, it could\n>>> +\t\t\t\t * affect validity of the other controls.\n>>> +\t\t\t\t */\n>>> +\t\t\t\tControlList priority(device_->controls());\n>>> +\t\t\t\tpriority.set(id->id(), info);\n>>> +\t\t\t\tdevice_->setControls(&priority);\n>>> +\t\t\t} else {\n>>> +\t\t\t\t/*\n>>> +\t\t\t\t * Batch up the list of controls and write them\n>>> +\t\t\t\t * at the end of the function.\n>>> +\t\t\t\t */\n>>> +\t\t\t\tout.set(id->id(), info);\n>>> +\t\t\t}\n>>> +\n>>> \t\t\tLOG(DelayedControls, Debug)\n>>> \t\t\t\t<< \"Setting \" << id->name()\n>>> \t\t\t\t<< \" to \" << info.toString()\n>>> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n>>> index 3e6b88af4188..ac92f066a07e 100644\n>>> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n>>> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n>>> @@ -963,14 +963,14 @@ int PipelineHandlerIPU3::registerCameras()\n>>> \t\t * a sensor database. For now use generic values taken from\n>>> \t\t * the Raspberry Pi and listed as 'generic values'.\n>>> \t\t */\n>>> -\t\tstd::unordered_map delays = {\n>>> -\t\t\t{ V4L2_CID_ANALOGUE_GAIN, 1 },\n>>> -\t\t\t{ V4L2_CID_EXPOSURE, 2 },\n>>> +\t\tstd::unordered_map params = {\n>>> +\t\t\t{ V4L2_CID_ANALOGUE_GAIN, { 1, false } },\n>>> +\t\t\t{ V4L2_CID_EXPOSURE, { 2, false } },\n>>> \t\t};\n>>> \n>>> \t\tdata->delayedCtrls_ =\n>>> \t\t\tstd::make_unique(cio2->sensor()->device(),\n>>> -\t\t\t\t\t\t\t delays);\n>>> +\t\t\t\t\t\t\t params);\n>>> \t\tdata->cio2_.frameStart().connect(data->delayedCtrls_.get(),\n>>> \t\t\t\t\t\t &DelayedControls::applyControls);\n>>> \n>>> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n>>> index a60415d93705..ba74ace183db 100644\n>>> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n>>> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n>>> @@ -1244,16 +1244,15 @@ int RPiCameraData::configureIPA(const CameraConfiguration *config)\n>>> \tif (result.params & ipa::RPi::ConfigSensorParams) {\n>>> \t\t/*\n>>> \t\t * Setup our delayed control writer with the sensor default\n>>> -\t\t * gain and exposure delays.\n>>> +\t\t * gain and exposure delays. Mark VBLANK for priority write.\n>>> \t\t */\n>>> -\t\tstd::unordered_map delays = {\n>>> -\t\t\t{ V4L2_CID_ANALOGUE_GAIN, result.sensorConfig.gainDelay },\n>>> -\t\t\t{ V4L2_CID_EXPOSURE, result.sensorConfig.exposureDelay },\n>>> -\t\t\t{ V4L2_CID_VBLANK, result.sensorConfig.vblank }\n>>> +\t\tstd::unordered_map params = {\n>>> +\t\t\t{ V4L2_CID_ANALOGUE_GAIN, { result.sensorConfig.gainDelay, false } },\n>>> +\t\t\t{ V4L2_CID_EXPOSURE, { result.sensorConfig.exposureDelay, false } },\n>>> +\t\t\t{ V4L2_CID_VBLANK, { result.sensorConfig.vblank, true } }\n\nApplying has a merge conflict on master here.\n\nSetting this as\n { V4L2_CID_VBLANK, { result.sensorConfig.vblankDelay, true } }\n\n\n\n>>> \t\t};\n>>> \n>>> -\t\tdelayedCtrls_ = std::make_unique(unicam_[Unicam::Image].dev(), delays);\n>>> -\n>>> +\t\tdelayedCtrls_ = std::make_unique(unicam_[Unicam::Image].dev(), params);\n>>> \t\tsensorMetadata_ = result.sensorConfig.sensorMetadata;\n>>> \t}\n>>> \n>>> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n>>> index a794501a9c8d..17c0f9751cd3 100644\n>>> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n>>> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n>>> @@ -938,14 +938,14 @@ int PipelineHandlerRkISP1::createCamera(MediaEntity *sensor)\n>>> \t * a sensor database. For now use generic values taken from\n>>> \t * the Raspberry Pi and listed as generic values.\n>>> \t */\n>>> -\tstd::unordered_map delays = {\n>>> -\t\t{ V4L2_CID_ANALOGUE_GAIN, 1 },\n>>> -\t\t{ V4L2_CID_EXPOSURE, 2 },\n>>> +\tstd::unordered_map params = {\n>>> +\t\t{ V4L2_CID_ANALOGUE_GAIN, { 1, false } },\n>>> +\t\t{ V4L2_CID_EXPOSURE, { 2, false } },\n>>> \t};\n>>> \n>>> \tdata->delayedCtrls_ =\n>>> \t\tstd::make_unique(data->sensor_->device(),\n>>> -\t\t\t\t\t\t delays);\n>>> +\t\t\t\t\t\t params);\n>>> \tisp_->frameStart.connect(data->delayedCtrls_.get(),\n>>> \t\t\t\t &DelayedControls::applyControls);\n>>> \n>>> diff --git a/test/delayed_contols.cpp b/test/delayed_contols.cpp\n>>> index 50169b12e566..3855eb18ecd4 100644\n>>> --- a/test/delayed_contols.cpp\n>>> +++ b/test/delayed_contols.cpp\n>>> @@ -72,8 +72,8 @@ protected:\n>>> \n>>> \tint singleControlNoDelay()\n>>> \t{\n>>> -\t\tstd::unordered_map delays = {\n>>> -\t\t\t{ V4L2_CID_BRIGHTNESS, 0 },\n>>> +\t\tstd::unordered_map delays = {\n>>> +\t\t\t{ V4L2_CID_BRIGHTNESS, { 0, false } },\n>>> \t\t};\n>>> \t\tstd::unique_ptr delayed =\n>>> \t\t\tstd::make_unique(dev_.get(), delays);\n>>> @@ -109,8 +109,8 @@ protected:\n>>> \n>>> \tint singleControlWithDelay()\n>>> \t{\n>>> -\t\tstd::unordered_map delays = {\n>>> -\t\t\t{ V4L2_CID_BRIGHTNESS, 1 },\n>>> +\t\tstd::unordered_map delays = {\n>>> +\t\t\t{ V4L2_CID_BRIGHTNESS, { 1, false } },\n>>> \t\t};\n>>> \t\tstd::unique_ptr delayed =\n>>> \t\t\tstd::make_unique(dev_.get(), delays);\n>>> @@ -150,9 +150,9 @@ protected:\n>>> \n>>> \tint dualControlsWithDelay(uint32_t startOffset)\n>>> \t{\n>>> -\t\tstd::unordered_map delays = {\n>>> -\t\t\t{ V4L2_CID_BRIGHTNESS, 1 },\n>>> -\t\t\t{ V4L2_CID_CONTRAST, 2 },\n>>> +\t\tstd::unordered_map delays = {\n>>> +\t\t\t{ V4L2_CID_BRIGHTNESS, { 1, false } },\n>>> +\t\t\t{ V4L2_CID_CONTRAST, { 2, false } },\n>>> \t\t};\n>>> \t\tstd::unique_ptr delayed =\n>>> \t\t\tstd::make_unique(dev_.get(), delays);\n>>> @@ -197,9 +197,9 @@ protected:\n>>> \n>>> \tint dualControlsMultiQueue()\n>>> \t{\n>>> -\t\tstd::unordered_map delays = {\n>>> -\t\t\t{ V4L2_CID_BRIGHTNESS, 1 },\n>>> -\t\t\t{ V4L2_CID_CONTRAST, 2 },\n>>> +\t\tstd::unordered_map delays = {\n>>> +\t\t\t{ V4L2_CID_BRIGHTNESS, { 1, false } },\n>>> +\t\t\t{ V4L2_CID_CONTRAST, { 2, false } }\n>>> \t\t};\n>>> \t\tstd::unique_ptr delayed =\n>>> \t\t\tstd::make_unique(dev_.get(), delays);\n>>>\n>> _______________________________________________\n>> libcamera-devel mailing list\n>> libcamera-devel@lists.libcamera.org\n>> https://lists.libcamera.org/listinfo/libcamera-devel\n>>\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 050F9BD1F1\n\tfor ;\n\tFri, 12 Mar 2021 14:00:08 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 4BA7768C73;\n\tFri, 12 Mar 2021 15:00:07 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 6112F68C68\n\tfor ;\n\tFri, 12 Mar 2021 15:00:05 +0100 (CET)","from [192.168.0.20]\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 D2D93A2A;\n\tFri, 12 Mar 2021 15:00:04 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"cqVS6wEe\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1615557605;\n\tbh=ktpM2CM+xFptPvp/QbOrBxk3IiEdJQ7F/Sd3/vinEt0=;\n\th=Reply-To:Subject:From:To:References:Date:In-Reply-To:From;\n\tb=cqVS6wEe4DpdDscUP01xttWjHauLudDe3lmywCyC4MkyMeBBLRv6oNwj/6F+q3ZGK\n\tf0MYzpWO+zckw2ntFLYEH/+MT17tIYrEfNbaJ88zHz4na/eQpiyVBWChVr+rbIBC6q\n\tynGsYqwYt3z4LFlOjAuvenpQpummA8beKeWhJDxs=","From":"Kieran Bingham ","To":"Jean-Michel Hautbois ,\n\tNaushir Patuck ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20210304081728.1058394-1-naush@raspberrypi.com>\n\t<20210304081728.1058394-2-naush@raspberrypi.com>\n\t<82c5f6c6-e6ea-80d9-8ac3-3a403a707b75@ideasonboard.com>\n\t<7532d1e4-d01c-4b1f-9c82-28e070120a5a@ideasonboard.com>","Autocrypt":"addr=kieran.bingham@ideasonboard.com; keydata=\n\tmQINBFYE/WYBEACs1PwjMD9rgCu1hlIiUA1AXR4rv2v+BCLUq//vrX5S5bjzxKAryRf0uHat\n\tV/zwz6hiDrZuHUACDB7X8OaQcwhLaVlq6byfoBr25+hbZG7G3+5EUl9cQ7dQEdvNj6V6y/SC\n\trRanWfelwQThCHckbobWiQJfK9n7rYNcPMq9B8e9F020LFH7Kj6YmO95ewJGgLm+idg1Kb3C\n\tpotzWkXc1xmPzcQ1fvQMOfMwdS+4SNw4rY9f07Xb2K99rjMwZVDgESKIzhsDB5GY465sCsiQ\n\tcSAZRxqE49RTBq2+EQsbrQpIc8XiffAB8qexh5/QPzCmR4kJgCGeHIXBtgRj+nIkCJPZvZtf\n\tKr2EAbc6tgg6DkAEHJb+1okosV09+0+TXywYvtEop/WUOWQ+zo+Y/OBd+8Ptgt1pDRyOBzL8\n\tRXa8ZqRf0Mwg75D+dKntZeJHzPRJyrlfQokngAAs4PaFt6UfS+ypMAF37T6CeDArQC41V3ko\n\tlPn1yMsVD0p+6i3DPvA/GPIksDC4owjnzVX9kM8Zc5Cx+XoAN0w5Eqo4t6qEVbuettxx55gq\n\t8K8FieAjgjMSxngo/HST8TpFeqI5nVeq0/lqtBRQKumuIqDg+Bkr4L1V/PSB6XgQcOdhtd36\n\tOe9X9dXB8YSNt7VjOcO7BTmFn/Z8r92mSAfHXpb07YJWJosQOQARAQABtDBLaWVyYW4gQmlu\n\tZ2hhbSA8a2llcmFuLmJpbmdoYW1AaWRlYXNvbmJvYXJkLmNvbT6JAlcEEwEKAEECGwMFCwkI\n\tBwIGFQgJCgsCBBYCAwECHgECF4ACGQEWIQSQLdeYP70o/eNy1HqhHkZyEKRh/QUCXWTtygUJ\n\tCyJXZAAKCRChHkZyEKRh/f8dEACTDsbLN2nioNZMwyLuQRUAFcXNolDX48xcUXsWS2QjxaPm\n\tVsJx8Uy8aYkS85mdPBh0C83OovQR/OVbr8AxhGvYqBs3nQvbWuTl/+4od7DfK2VZOoKBAu5S\n\tQK2FYuUcikDqYcFWJ8DQnubxfE8dvzojHEkXw0sA4igINHDDFX3HJGZtLio+WpEFQtCbfTAG\n\tYZslasz1YZRbwEdSsmO3/kqy5eMnczlm8a21A3fKUo3g8oAZEFM+f4DUNzqIltg31OAB/kZS\n\tenKZQ/SWC8PmLg/ZXBrReYakxXtkP6w3FwMlzOlhGxqhIRNiAJfXJBaRhuUWzPOpEDE9q5YJ\n\tBmqQL2WJm1VSNNVxbXJHpaWMH1sA2R00vmvRrPXGwyIO0IPYeUYQa3gsy6k+En/aMQJd27dp\n\taScf9am9PFICPY5T4ppneeJLif2lyLojo0mcHOV+uyrds9XkLpp14GfTkeKPdPMrLLTsHRfH\n\tfA4I4OBpRrEPiGIZB/0im98MkGY/Mu6qxeZmYLCcgD6qz4idOvfgVOrNh+aA8HzIVR+RMW8H\n\tQGBN9f0E3kfwxuhl3omo6V7lDw8XOdmuWZNC9zPq1UfryVHANYbLGz9KJ4Aw6M+OgBC2JpkD\n\thXMdHUkC+d20dwXrwHTlrJi1YNp6rBc+xald3wsUPOZ5z8moTHUX/uPA/qhGsbkCDQRWBP1m\n\tARAAzijkb+Sau4hAncr1JjOY+KyFEdUNxRy+hqTJdJfaYihxyaj0Ee0P0zEi35CbE6lgU0Uz\n\ttih9fiUbSV3wfsWqg1Ut3/5rTKu7kLFp15kF7eqvV4uezXRD3Qu4yjv/rMmEJbbD4cTvGCYI\n\td6MDC417f7vK3hCbCVIZSp3GXxyC1LU+UQr3fFcOyCwmP9vDUR9JV0BSqHHxRDdpUXE26Dk6\n\tmhf0V1YkspE5St814ETXpEus2urZE5yJIUROlWPIL+hm3NEWfAP06vsQUyLvr/GtbOT79vXl\n\tEn1aulcYyu20dRRxhkQ6iILaURcxIAVJJKPi8dsoMnS8pB0QW12AHWuirPF0g6DiuUfPmrA5\n\tPKe56IGlpkjc8cO51lIxHkWTpCMWigRdPDexKX+Sb+W9QWK/0JjIc4t3KBaiG8O4yRX8ml2R\n\t+rxfAVKM6V769P/hWoRGdgUMgYHFpHGSgEt80OKK5HeUPy2cngDUXzwrqiM5Sz6Od0qw5pCk\n\tNlXqI0W/who0iSVM+8+RmyY0OEkxEcci7rRLsGnM15B5PjLJjh1f2ULYkv8s4SnDwMZ/kE04\n\t/UqCMK/KnX8pwXEMCjz0h6qWNpGwJ0/tYIgQJZh6bqkvBrDogAvuhf60Sogw+mH8b+PBlx1L\n\toeTK396wc+4c3BfiC6pNtUS5GpsPMMjYMk7kVvEAEQEAAYkCPAQYAQoAJgIbDBYhBJAt15g/\n\tvSj943LUeqEeRnIQpGH9BQJdizzIBQkLSKZiAAoJEKEeRnIQpGH9eYgQAJpjaWNgqNOnMTmD\n\tMJggbwjIotypzIXfhHNCeTkG7+qCDlSaBPclcPGYrTwCt0YWPU2TgGgJrVhYT20ierN8LUvj\n\t6qOPTd+Uk7NFzL65qkh80ZKNBFddx1AabQpSVQKbdcLb8OFs85kuSvFdgqZwgxA1vl4TFhNz\n\tPZ79NAmXLackAx3sOVFhk4WQaKRshCB7cSl+RIng5S/ThOBlwNlcKG7j7W2MC06BlTbdEkUp\n\tECzuuRBv8wX4OQl+hbWbB/VKIx5HKlLu1eypen/5lNVzSqMMIYkkZcjV2SWQyUGxSwq0O/sx\n\tS0A8/atCHUXOboUsn54qdxrVDaK+6jIAuo8JiRWctP16KjzUM7MO0/+4zllM8EY57rXrj48j\n\tsbEYX0YQnzaj+jO6kJtoZsIaYR7rMMq9aUAjyiaEZpmP1qF/2sYenDx0Fg2BSlLvLvXM0vU8\n\tpQk3kgDu7kb/7PRYrZvBsr21EIQoIjXbZxDz/o7z95frkP71EaICttZ6k9q5oxxA5WC6sTXc\n\tMW8zs8avFNuA9VpXt0YupJd2ijtZy2mpZNG02fFVXhIn4G807G7+9mhuC4XG5rKlBBUXTvPU\n\tAfYnB4JBDLmLzBFavQfvonSfbitgXwCG3vS+9HEwAjU30Bar1PEOmIbiAoMzuKeRm2LVpmq4\n\tWZw01QYHU/GUV/zHJSFk","Organization":"Ideas on Board","Message-ID":"","Date":"Fri, 12 Mar 2021 14:00:02 +0000","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101\n\tThunderbird/68.10.0","MIME-Version":"1.0","In-Reply-To":"<7532d1e4-d01c-4b1f-9c82-28e070120a5a@ideasonboard.com>","Content-Language":"en-GB","Subject":"Re: [libcamera-devel] [PATCH v4 1/7] libcamera: delayed_controls:\n\tAdd notion of priority write","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","Reply-To":"kieran.bingham@ideasonboard.com","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":15622,"web_url":"https://patchwork.libcamera.org/comment/15622/","msgid":"","date":"2021-03-12T14:04:27","subject":"Re: [libcamera-devel] [PATCH v4 1/7] libcamera: delayed_controls:\n\tAdd notion of priority write","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hi Kieran,\n\nThank you for your review feedback.\n\nOn Fri, 12 Mar 2021 at 14:00, Kieran Bingham <\nkieran.bingham@ideasonboard.com> wrote:\n\n>\n>\n> On 12/03/2021 13:25, Kieran Bingham wrote:\n> > Hi Naush,\n> >\n> > On 09/03/2021 09:54, Jean-Michel Hautbois wrote:\n> >> Hi Naushir,\n> >>\n> >> Thanks for the patch !\n> >>\n> >> On 04/03/2021 09:17, Naushir Patuck wrote:\n> >>> If an exposure time change adjusts the vblanking limits, and we set\n> both\n> >>> VBLANK and EXPOSURE controls through the VIDIOC_S_EXT_CTRLS ioctl, the\n> >>> latter may fail if the value is outside of the limits calculated by the\n> >>> old VBLANK value. This is a limitation in V4L2 and cannot be fixed by\n> >>> setting VBLANK before EXPOSURE in a single VIDIOC_S_EXT_CTRLS ioctl.\n> >>>\n> >>> The workaround here is to have the DelayedControls object mark the\n> >>> VBLANK control as \"priority write\", which then write VBLANK separately\n> >>> from (and ahead of) any other controls. This way, the sensor driver\n> will\n> >>> update the EXPOSURE control with new limits before the new values is\n> >>> presented, and will thus be seen as valid.\n> >>>\n> >>> To support this, a new struct DelayedControls::ControlParams is used in\n> >>> the constructor to provide the control delay value as well as the\n> >>> priority write flag.\n> >>>\n> >>> Signed-off-by: Naushir Patuck \n> >>> Tested-by: David Plowman \n> >>> Reviewed-by: Paul Elder \n> >> Tested-by: Jean-Michel Hautbois \n> >>\n> >>> ---\n> >>> include/libcamera/internal/delayed_controls.h | 9 +++-\n> >>> src/libcamera/delayed_controls.cpp | 54 +++++++++++++------\n> >>> src/libcamera/pipeline/ipu3/ipu3.cpp | 8 +--\n> >>> .../pipeline/raspberrypi/raspberrypi.cpp | 13 +++--\n> >>> src/libcamera/pipeline/rkisp1/rkisp1.cpp | 8 +--\n> >>> test/delayed_contols.cpp | 20 +++----\n> >>> 6 files changed, 68 insertions(+), 44 deletions(-)\n> >>>\n> >>> diff --git a/include/libcamera/internal/delayed_controls.h\n> b/include/libcamera/internal/delayed_controls.h\n> >>> index dc447a882514..564d9f2e2440 100644\n> >>> --- a/include/libcamera/internal/delayed_controls.h\n> >>> +++ b/include/libcamera/internal/delayed_controls.h\n> >>> @@ -19,8 +19,13 @@ class V4L2Device;\n> >>> class DelayedControls\n> >>> {\n> >>> public:\n> >>> + struct ControlParams {\n> >>> + unsigned int delay;\n> >>> + bool priorityWrite;\n> >>> + };\n> >>> +\n> >>> DelayedControls(V4L2Device *device,\n> >>> - const std::unordered_map\n> &delays);\n> >>> + const std::unordered_map\n> &controlParams);\n> >>>\n> >>> void reset();\n> >>>\n> >>> @@ -64,7 +69,7 @@ private:\n> >>>\n> >>> V4L2Device *device_;\n> >>> /* \\todo Evaluate if we should index on ControlId * or unsigned\n> int */\n> >>> - std::unordered_map delays_;\n> >>> + std::unordered_map\n> controlParams_;\n> >>> unsigned int maxDelay_;\n> >>>\n> >>> bool running_;\n> >>> diff --git a/src/libcamera/delayed_controls.cpp\n> b/src/libcamera/delayed_controls.cpp\n> >>> index ab1d40057c5f..3ed1dfebd035 100644\n> >>> --- a/src/libcamera/delayed_controls.cpp\n> >>> +++ b/src/libcamera/delayed_controls.cpp\n> >>> @@ -40,15 +40,19 @@ LOG_DEFINE_CATEGORY(DelayedControls)\n> >>> /**\n> >>> * \\brief Construct a DelayedControls instance\n> >>> * \\param[in] device The V4L2 device the controls have to be applied\n> to\n> >>> - * \\param[in] delays Map of the numerical V4L2 control ids to their\n> associated\n> >>> - * delays (in frames)\n> >>> + * \\param[in] controlParams Map of the numerical V4L2 control ids to\n> their\n> >>> + * associated control parameters.\n> >>> *\n> >>> - * Only controls specified in \\a delays are handled. If it's desired\n> to mix\n> >>> - * delayed controls and controls that take effect immediately the\n> immediate\n> >>> - * controls must be listed in the \\a delays map with a delay value of\n> 0.\n> >>> + * The control parameters comprise of delays (in frames) and a\n> priority write\n> >>> + * flag. If this flag is set, the relevant control is written\n> separately from,\n> >>> + * ahead of the reset of the batched controls.\n> >\n> > minor:\n> >\n> > 'and ahead of' ?\n> >\n> > could be fixed while applying if there's nothing else major:\n>\n>\n> Could you confirm if this is correct to say 'and ahead of the reset of'\n> or if it was supposed to be 'and ahead of the rest of' ...\n>\n\n'separately from, and ahead of the rest of` should be the correct term here\n:)\n\nHappy for you to fix while applying if it is convenient for you.\n\nRegards,\nNaush\n\n\n\n>\n>\n> >\n> > Reviewed-by: Kieran Bingham \n> >\n> >\n> >\n> >>> + *\n> >>> + * Only controls specified in \\a controlParams are handled. If it's\n> desired to\n> >>> + * mix delayed controls and controls that take effect immediately the\n> immediate\n> >>> + * controls must be listed in the \\a controlParams map with a delay\n> value of 0.\n> >>> */\n> >>> DelayedControls::DelayedControls(V4L2Device *device,\n> >>> - const std::unordered_map unsigned int> &delays)\n> >>> + const std::unordered_map ControlParams> &controlParams)\n> >>> : device_(device), maxDelay_(0)\n> >>> {\n> >>> const ControlInfoMap &controls = device_->controls();\n> >>> @@ -57,12 +61,12 @@ DelayedControls::DelayedControls(V4L2Device\n> *device,\n> >>> * Create a map of control ids to delays for controls exposed by\n> the\n> >>> * device.\n> >>> */\n> >>> - for (auto const &delay : delays) {\n> >>> - auto it = controls.find(delay.first);\n> >>> + for (auto const ¶m : controlParams) {\n> >>> + auto it = controls.find(param.first);\n> >>> if (it == controls.end()) {\n> >>> LOG(DelayedControls, Error)\n> >>> << \"Delay request for control id \"\n> >>> - << utils::hex(delay.first)\n> >>> + << utils::hex(param.first)\n> >>> << \" but control is not exposed by device \"\n> >>> << device_->deviceNode();\n> >>> continue;\n> >>> @@ -70,13 +74,14 @@ DelayedControls::DelayedControls(V4L2Device\n> *device,\n> >>>\n> >>> const ControlId *id = it->first;\n> >>>\n> >>> - delays_[id] = delay.second;\n> >>> + controlParams_[id] = param.second;\n> >>>\n> >>> LOG(DelayedControls, Debug)\n> >>> - << \"Set a delay of \" << delays_[id]\n> >>> + << \"Set a delay of \" << controlParams_[id].delay\n> >>> + << \" and priority write flag \" <<\n> controlParams_[id].priorityWrite\n> >>> << \" for \" << id->name();\n> >>>\n> >>> - maxDelay_ = std::max(maxDelay_, delays_[id]);\n> >>> + maxDelay_ = std::max(maxDelay_, controlParams_[id].delay);\n> >>> }\n> >>>\n> >>> reset();\n> >>> @@ -97,8 +102,8 @@ void DelayedControls::reset()\n> >>>\n> >>> /* Retrieve control as reported by the device. */\n> >>> std::vector ids;\n> >>> - for (auto const &delay : delays_)\n> >>> - ids.push_back(delay.first->id());\n> >>> + for (auto const ¶m : controlParams_)\n> >>> + ids.push_back(param.first->id());\n> >>>\n> >>> ControlList controls = device_->getControls(ids);\n> >>>\n> >>> @@ -140,7 +145,7 @@ bool DelayedControls::push(const ControlList\n> &controls)\n> >>>\n> >>> const ControlId *id = it->second;\n> >>>\n> >>> - if (delays_.find(id) == delays_.end())\n> >>> + if (controlParams_.find(id) == controlParams_.end())\n> >>> return false;\n> >>>\n> >>> Info &info = values_[id][queueCount_];\n> >>> @@ -220,12 +225,27 @@ void DelayedControls::applyControls(uint32_t\n> sequence)\n> >>> ControlList out(device_->controls());\n> >>> for (const auto &ctrl : values_) {\n> >>> const ControlId *id = ctrl.first;\n> >>> - unsigned int delayDiff = maxDelay_ - delays_[id];\n> >>> + unsigned int delayDiff = maxDelay_ -\n> controlParams_[id].delay;\n> >>> unsigned int index = std::max(0, writeCount_ -\n> delayDiff);\n> >>> const Info &info = ctrl.second[index];\n> >>>\n> >>> if (info.updated) {\n> >>> - out.set(id->id(), info);\n> >>> + if (controlParams_[id].priorityWrite) {\n> >>> + /*\n> >>> + * This control must be written now, it\n> could\n> >>> + * affect validity of the other controls.\n> >>> + */\n> >>> + ControlList priority(device_->controls());\n> >>> + priority.set(id->id(), info);\n> >>> + device_->setControls(&priority);\n> >>> + } else {\n> >>> + /*\n> >>> + * Batch up the list of controls and write\n> them\n> >>> + * at the end of the function.\n> >>> + */\n> >>> + out.set(id->id(), info);\n> >>> + }\n> >>> +\n> >>> LOG(DelayedControls, Debug)\n> >>> << \"Setting \" << id->name()\n> >>> << \" to \" << info.toString()\n> >>> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> >>> index 3e6b88af4188..ac92f066a07e 100644\n> >>> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> >>> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> >>> @@ -963,14 +963,14 @@ int PipelineHandlerIPU3::registerCameras()\n> >>> * a sensor database. For now use generic values taken from\n> >>> * the Raspberry Pi and listed as 'generic values'.\n> >>> */\n> >>> - std::unordered_map delays = {\n> >>> - { V4L2_CID_ANALOGUE_GAIN, 1 },\n> >>> - { V4L2_CID_EXPOSURE, 2 },\n> >>> + std::unordered_map DelayedControls::ControlParams> params = {\n> >>> + { V4L2_CID_ANALOGUE_GAIN, { 1, false } },\n> >>> + { V4L2_CID_EXPOSURE, { 2, false } },\n> >>> };\n> >>>\n> >>> data->delayedCtrls_ =\n> >>>\n> std::make_unique(cio2->sensor()->device(),\n> >>> - delays);\n> >>> + params);\n> >>> data->cio2_.frameStart().connect(data->delayedCtrls_.get(),\n> >>>\n> &DelayedControls::applyControls);\n> >>>\n> >>> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> >>> index a60415d93705..ba74ace183db 100644\n> >>> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> >>> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> >>> @@ -1244,16 +1244,15 @@ int RPiCameraData::configureIPA(const\n> CameraConfiguration *config)\n> >>> if (result.params & ipa::RPi::ConfigSensorParams) {\n> >>> /*\n> >>> * Setup our delayed control writer with the sensor default\n> >>> - * gain and exposure delays.\n> >>> + * gain and exposure delays. Mark VBLANK for priority\n> write.\n> >>> */\n> >>> - std::unordered_map delays = {\n> >>> - { V4L2_CID_ANALOGUE_GAIN,\n> result.sensorConfig.gainDelay },\n> >>> - { V4L2_CID_EXPOSURE,\n> result.sensorConfig.exposureDelay },\n> >>> - { V4L2_CID_VBLANK, result.sensorConfig.vblank }\n> >>> + std::unordered_map DelayedControls::ControlParams> params = {\n> >>> + { V4L2_CID_ANALOGUE_GAIN, {\n> result.sensorConfig.gainDelay, false } },\n> >>> + { V4L2_CID_EXPOSURE, {\n> result.sensorConfig.exposureDelay, false } },\n> >>> + { V4L2_CID_VBLANK, { result.sensorConfig.vblank,\n> true } }\n>\n> Applying has a merge conflict on master here.\n>\n> Setting this as\n> { V4L2_CID_VBLANK, { result.sensorConfig.vblankDelay, true } }\n>\n>\n>\n> >>> };\n> >>>\n> >>> - delayedCtrls_ =\n> std::make_unique(unicam_[Unicam::Image].dev(), delays);\n> >>> -\n> >>> + delayedCtrls_ =\n> std::make_unique(unicam_[Unicam::Image].dev(), params);\n> >>> sensorMetadata_ = result.sensorConfig.sensorMetadata;\n> >>> }\n> >>>\n> >>> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> >>> index a794501a9c8d..17c0f9751cd3 100644\n> >>> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> >>> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> >>> @@ -938,14 +938,14 @@ int\n> PipelineHandlerRkISP1::createCamera(MediaEntity *sensor)\n> >>> * a sensor database. For now use generic values taken from\n> >>> * the Raspberry Pi and listed as generic values.\n> >>> */\n> >>> - std::unordered_map delays = {\n> >>> - { V4L2_CID_ANALOGUE_GAIN, 1 },\n> >>> - { V4L2_CID_EXPOSURE, 2 },\n> >>> + std::unordered_map\n> params = {\n> >>> + { V4L2_CID_ANALOGUE_GAIN, { 1, false } },\n> >>> + { V4L2_CID_EXPOSURE, { 2, false } },\n> >>> };\n> >>>\n> >>> data->delayedCtrls_ =\n> >>> std::make_unique(data->sensor_->device(),\n> >>> - delays);\n> >>> + params);\n> >>> isp_->frameStart.connect(data->delayedCtrls_.get(),\n> >>> &DelayedControls::applyControls);\n> >>>\n> >>> diff --git a/test/delayed_contols.cpp b/test/delayed_contols.cpp\n> >>> index 50169b12e566..3855eb18ecd4 100644\n> >>> --- a/test/delayed_contols.cpp\n> >>> +++ b/test/delayed_contols.cpp\n> >>> @@ -72,8 +72,8 @@ protected:\n> >>>\n> >>> int singleControlNoDelay()\n> >>> {\n> >>> - std::unordered_map delays = {\n> >>> - { V4L2_CID_BRIGHTNESS, 0 },\n> >>> + std::unordered_map DelayedControls::ControlParams> delays = {\n> >>> + { V4L2_CID_BRIGHTNESS, { 0, false } },\n> >>> };\n> >>> std::unique_ptr delayed =\n> >>> std::make_unique(dev_.get(),\n> delays);\n> >>> @@ -109,8 +109,8 @@ protected:\n> >>>\n> >>> int singleControlWithDelay()\n> >>> {\n> >>> - std::unordered_map delays = {\n> >>> - { V4L2_CID_BRIGHTNESS, 1 },\n> >>> + std::unordered_map DelayedControls::ControlParams> delays = {\n> >>> + { V4L2_CID_BRIGHTNESS, { 1, false } },\n> >>> };\n> >>> std::unique_ptr delayed =\n> >>> std::make_unique(dev_.get(),\n> delays);\n> >>> @@ -150,9 +150,9 @@ protected:\n> >>>\n> >>> int dualControlsWithDelay(uint32_t startOffset)\n> >>> {\n> >>> - std::unordered_map delays = {\n> >>> - { V4L2_CID_BRIGHTNESS, 1 },\n> >>> - { V4L2_CID_CONTRAST, 2 },\n> >>> + std::unordered_map DelayedControls::ControlParams> delays = {\n> >>> + { V4L2_CID_BRIGHTNESS, { 1, false } },\n> >>> + { V4L2_CID_CONTRAST, { 2, false } },\n> >>> };\n> >>> std::unique_ptr delayed =\n> >>> std::make_unique(dev_.get(),\n> delays);\n> >>> @@ -197,9 +197,9 @@ protected:\n> >>>\n> >>> int dualControlsMultiQueue()\n> >>> {\n> >>> - std::unordered_map delays = {\n> >>> - { V4L2_CID_BRIGHTNESS, 1 },\n> >>> - { V4L2_CID_CONTRAST, 2 },\n> >>> + std::unordered_map DelayedControls::ControlParams> delays = {\n> >>> + { V4L2_CID_BRIGHTNESS, { 1, false } },\n> >>> + { V4L2_CID_CONTRAST, { 2, false } }\n> >>> };\n> >>> std::unique_ptr delayed =\n> >>> std::make_unique(dev_.get(),\n> delays);\n> >>>\n> >> _______________________________________________\n> >> libcamera-devel mailing list\n> >> libcamera-devel@lists.libcamera.org\n> >> https://lists.libcamera.org/listinfo/libcamera-devel\n> >>\n> >\n>\n> --\n> Regards\n> --\n> Kieran\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 A37E2BD80C\n\tfor ;\n\tFri, 12 Mar 2021 14:04:48 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id DFF6168C73;\n\tFri, 12 Mar 2021 15:04:47 +0100 (CET)","from mail-lf1-x135.google.com (mail-lf1-x135.google.com\n\t[IPv6:2a00:1450:4864:20::135])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 0E4E168C68\n\tfor ;\n\tFri, 12 Mar 2021 15:04:45 +0100 (CET)","by mail-lf1-x135.google.com with SMTP id u4so45974868lfs.0\n\tfor ;\n\tFri, 12 Mar 2021 06:04:44 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"fQwEtWY8\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=MuaGu2tg1lOxK4nvl6IQf2gn3WnoMBpvL7cjOw2Bb7U=;\n\tb=fQwEtWY8bTMFEDjY4f7pL4ux7XlrWdwF+Ke+I7Eu5HN2pIC7GIuC8eBX5ch/wGaNJz\n\tm4Do/v0Q6eXdLmS3hIvYy6W3fmdW/v5fN8r+NcuRidZHDZOTREzfVZ4fWRAkuJ/RBBtn\n\tAFvufMegJlWoMpH0Ja6VRyTgiYz+enKxW42XSSGBfw+CO7BlehukYH8s0Q13LyBhCDj+\n\t8s/qcnSlxgTCgvNEm+CtCua4De0sOwS5XKfOC9zFdsth6eVcsllo1LKdeIy06lHwtVU2\n\tMr8AqNRqMHtWRCMN5i2gEjkexXuV+i1txGnsj+wIfubeUfZ9NQlG5xBtMqf7WnMkcE1D\n\twEZg==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=MuaGu2tg1lOxK4nvl6IQf2gn3WnoMBpvL7cjOw2Bb7U=;\n\tb=rTPAkjzJ3vsN1lg6CjVbxMfU5tpgEYT8GSTEJAf5w3U0YyPEzGc/1+LCbeQ4mJ06IC\n\taEYyPr+992XwinPrOWmwkaPQQFnBxUTdKaQUCKpGOqcM4aO/9iHlfW1UqJ4bnA2dMkx8\n\thVbo/Z056rRiGj/a3WwLtBQIiAE7al+TP4xwdb6cjvJ8q+e3Q83q4ikNeQ1wTtIGuPR9\n\trYihdfBZgjm6YqnqpHqq6MaoO0An0s1jWIDvTcpOSk2bbiqOUPcCgHgrSFt6inQdN7lM\n\tR780wADdV7iWNtSuIUG+VkbigjRN4WNTnVzOX6wRHJ842Ej6vfLAncv8AlxzvSVLNO1t\n\t4JtA==","X-Gm-Message-State":"AOAM533SdWZgunrX8E4tmqj3UQ+VXyU28q5C3NpE9N73AvBuZlzeHzs6\n\tOs/cNhNLybZx8jzg8nQVfwdh1ma81THuOyqB6/rNnA==","X-Google-Smtp-Source":"ABdhPJwCRdZsEA8gJcx0YcydFRiZfOLGtt+GKQo9BWXWTnvnTJnD9qcRqrtUOYSsQx8EJ/Mu5oRbLXcN28qTU3MzDCg=","X-Received":"by 2002:a19:3f58:: with SMTP id\n\tm85mr5345505lfa.617.1615557884044; \n\tFri, 12 Mar 2021 06:04:44 -0800 (PST)","MIME-Version":"1.0","References":"<20210304081728.1058394-1-naush@raspberrypi.com>\n\t<20210304081728.1058394-2-naush@raspberrypi.com>\n\t<82c5f6c6-e6ea-80d9-8ac3-3a403a707b75@ideasonboard.com>\n\t<7532d1e4-d01c-4b1f-9c82-28e070120a5a@ideasonboard.com>\n\t","In-Reply-To":"","From":"Naushir Patuck ","Date":"Fri, 12 Mar 2021 14:04:27 +0000","Message-ID":"","To":"Kieran Bingham ","Subject":"Re: [libcamera-devel] [PATCH v4 1/7] libcamera: delayed_controls:\n\tAdd notion of priority write","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","Cc":"libcamera devel ","Content-Type":"multipart/mixed;\n\tboundary=\"===============4437518731109981432==\"","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":15625,"web_url":"https://patchwork.libcamera.org/comment/15625/","msgid":"<87945c10-8c1f-18ee-5e14-cabe0fcca1da@ideasonboard.com>","date":"2021-03-12T14:22:41","subject":"Re: [libcamera-devel] [PATCH v4 1/7] libcamera: delayed_controls:\n\tAdd notion of priority write","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"On 12/03/2021 14:04, Naushir Patuck wrote:\n> Hi Kieran,\n> \n> Thank you for your review feedback.\n> \n> b/src/libcamera/delayed_controls.cpp\n> >>> index ab1d40057c5f..3ed1dfebd035 100644\n> >>> --- a/src/libcamera/delayed_controls.cpp\n> >>> +++ b/src/libcamera/delayed_controls.cpp\n> >>> @@ -40,15 +40,19 @@ LOG_DEFINE_CATEGORY(DelayedControls)\n> >>>  /**\n> >>>   * \\brief Construct a DelayedControls instance\n> >>>   * \\param[in] device The V4L2 device the controls have to be\n> applied to\n> >>> - * \\param[in] delays Map of the numerical V4L2 control ids to\n> their associated\n> >>> - * delays (in frames)\n> >>> + * \\param[in] controlParams Map of the numerical V4L2 control\n> ids to their\n> >>> + * associated control parameters.\n> >>>   *\n> >>> - * Only controls specified in \\a delays are handled. If it's\n> desired to mix\n> >>> - * delayed controls and controls that take effect immediately\n> the immediate\n> >>> - * controls must be listed in the \\a delays map with a delay\n> value of 0.\n> >>> + * The control parameters comprise of delays (in frames) and a\n> priority write\n> >>> + * flag. If this flag is set, the relevant control is written\n> separately from,\n> >>> + * ahead of the reset of the batched controls.\n> >\n> > minor:\n> >\n> > 'and ahead of' ?\n> >\n> > could be fixed while applying if there's nothing else major:\n> \n> \n> Could you confirm if this is correct to say 'and ahead of the reset of'\n> or if it was supposed to be 'and ahead of the rest of' ...\n> \n> \n> 'separately from, and ahead of the rest of` should be the correct term\n> here :)\n> \n\nAha great.\n\n> Happy for you to fix while applying if it is convenient for you.\n\nDone and pushed.\n\n--\nThanks\n\nKieran","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 A6D80BD80C\n\tfor ;\n\tFri, 12 Mar 2021 14:22:46 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 2188068C69;\n\tFri, 12 Mar 2021 15:22:46 +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 E15B268C68\n\tfor ;\n\tFri, 12 Mar 2021 15:22:44 +0100 (CET)","from [192.168.0.20]\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 5A54CA2A;\n\tFri, 12 Mar 2021 15:22:44 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"NngyxLo2\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1615558964;\n\tbh=PBGAeq/MoXDhppUmanxNazXawWTsf8u++vLj8gS/dB8=;\n\th=Reply-To:Subject:To:Cc:References:From:Date:In-Reply-To:From;\n\tb=NngyxLo2UgtkClbe1TmcJNcymM+33UJHWsSEi/kWM1VL2iaKOETHEy71cQECwJ1/o\n\ts8RVwk+2RSVS13/gfX2WLNVuko2mVetP2kpAmGWb1kHoXBbclcRkRy2ActcjzQsOdj\n\tk+3pP8H39fV7H6fuFKR7Zi4+hC7xJGGNxdBRRaek=","To":"Naushir Patuck ","References":"<20210304081728.1058394-1-naush@raspberrypi.com>\n\t<20210304081728.1058394-2-naush@raspberrypi.com>\n\t<82c5f6c6-e6ea-80d9-8ac3-3a403a707b75@ideasonboard.com>\n\t<7532d1e4-d01c-4b1f-9c82-28e070120a5a@ideasonboard.com>\n\t\n\t","From":"Kieran Bingham ","Autocrypt":"addr=kieran.bingham@ideasonboard.com; keydata=\n\tmQINBFYE/WYBEACs1PwjMD9rgCu1hlIiUA1AXR4rv2v+BCLUq//vrX5S5bjzxKAryRf0uHat\n\tV/zwz6hiDrZuHUACDB7X8OaQcwhLaVlq6byfoBr25+hbZG7G3+5EUl9cQ7dQEdvNj6V6y/SC\n\trRanWfelwQThCHckbobWiQJfK9n7rYNcPMq9B8e9F020LFH7Kj6YmO95ewJGgLm+idg1Kb3C\n\tpotzWkXc1xmPzcQ1fvQMOfMwdS+4SNw4rY9f07Xb2K99rjMwZVDgESKIzhsDB5GY465sCsiQ\n\tcSAZRxqE49RTBq2+EQsbrQpIc8XiffAB8qexh5/QPzCmR4kJgCGeHIXBtgRj+nIkCJPZvZtf\n\tKr2EAbc6tgg6DkAEHJb+1okosV09+0+TXywYvtEop/WUOWQ+zo+Y/OBd+8Ptgt1pDRyOBzL8\n\tRXa8ZqRf0Mwg75D+dKntZeJHzPRJyrlfQokngAAs4PaFt6UfS+ypMAF37T6CeDArQC41V3ko\n\tlPn1yMsVD0p+6i3DPvA/GPIksDC4owjnzVX9kM8Zc5Cx+XoAN0w5Eqo4t6qEVbuettxx55gq\n\t8K8FieAjgjMSxngo/HST8TpFeqI5nVeq0/lqtBRQKumuIqDg+Bkr4L1V/PSB6XgQcOdhtd36\n\tOe9X9dXB8YSNt7VjOcO7BTmFn/Z8r92mSAfHXpb07YJWJosQOQARAQABtDBLaWVyYW4gQmlu\n\tZ2hhbSA8a2llcmFuLmJpbmdoYW1AaWRlYXNvbmJvYXJkLmNvbT6JAlcEEwEKAEECGwMFCwkI\n\tBwIGFQgJCgsCBBYCAwECHgECF4ACGQEWIQSQLdeYP70o/eNy1HqhHkZyEKRh/QUCXWTtygUJ\n\tCyJXZAAKCRChHkZyEKRh/f8dEACTDsbLN2nioNZMwyLuQRUAFcXNolDX48xcUXsWS2QjxaPm\n\tVsJx8Uy8aYkS85mdPBh0C83OovQR/OVbr8AxhGvYqBs3nQvbWuTl/+4od7DfK2VZOoKBAu5S\n\tQK2FYuUcikDqYcFWJ8DQnubxfE8dvzojHEkXw0sA4igINHDDFX3HJGZtLio+WpEFQtCbfTAG\n\tYZslasz1YZRbwEdSsmO3/kqy5eMnczlm8a21A3fKUo3g8oAZEFM+f4DUNzqIltg31OAB/kZS\n\tenKZQ/SWC8PmLg/ZXBrReYakxXtkP6w3FwMlzOlhGxqhIRNiAJfXJBaRhuUWzPOpEDE9q5YJ\n\tBmqQL2WJm1VSNNVxbXJHpaWMH1sA2R00vmvRrPXGwyIO0IPYeUYQa3gsy6k+En/aMQJd27dp\n\taScf9am9PFICPY5T4ppneeJLif2lyLojo0mcHOV+uyrds9XkLpp14GfTkeKPdPMrLLTsHRfH\n\tfA4I4OBpRrEPiGIZB/0im98MkGY/Mu6qxeZmYLCcgD6qz4idOvfgVOrNh+aA8HzIVR+RMW8H\n\tQGBN9f0E3kfwxuhl3omo6V7lDw8XOdmuWZNC9zPq1UfryVHANYbLGz9KJ4Aw6M+OgBC2JpkD\n\thXMdHUkC+d20dwXrwHTlrJi1YNp6rBc+xald3wsUPOZ5z8moTHUX/uPA/qhGsbkCDQRWBP1m\n\tARAAzijkb+Sau4hAncr1JjOY+KyFEdUNxRy+hqTJdJfaYihxyaj0Ee0P0zEi35CbE6lgU0Uz\n\ttih9fiUbSV3wfsWqg1Ut3/5rTKu7kLFp15kF7eqvV4uezXRD3Qu4yjv/rMmEJbbD4cTvGCYI\n\td6MDC417f7vK3hCbCVIZSp3GXxyC1LU+UQr3fFcOyCwmP9vDUR9JV0BSqHHxRDdpUXE26Dk6\n\tmhf0V1YkspE5St814ETXpEus2urZE5yJIUROlWPIL+hm3NEWfAP06vsQUyLvr/GtbOT79vXl\n\tEn1aulcYyu20dRRxhkQ6iILaURcxIAVJJKPi8dsoMnS8pB0QW12AHWuirPF0g6DiuUfPmrA5\n\tPKe56IGlpkjc8cO51lIxHkWTpCMWigRdPDexKX+Sb+W9QWK/0JjIc4t3KBaiG8O4yRX8ml2R\n\t+rxfAVKM6V769P/hWoRGdgUMgYHFpHGSgEt80OKK5HeUPy2cngDUXzwrqiM5Sz6Od0qw5pCk\n\tNlXqI0W/who0iSVM+8+RmyY0OEkxEcci7rRLsGnM15B5PjLJjh1f2ULYkv8s4SnDwMZ/kE04\n\t/UqCMK/KnX8pwXEMCjz0h6qWNpGwJ0/tYIgQJZh6bqkvBrDogAvuhf60Sogw+mH8b+PBlx1L\n\toeTK396wc+4c3BfiC6pNtUS5GpsPMMjYMk7kVvEAEQEAAYkCPAQYAQoAJgIbDBYhBJAt15g/\n\tvSj943LUeqEeRnIQpGH9BQJdizzIBQkLSKZiAAoJEKEeRnIQpGH9eYgQAJpjaWNgqNOnMTmD\n\tMJggbwjIotypzIXfhHNCeTkG7+qCDlSaBPclcPGYrTwCt0YWPU2TgGgJrVhYT20ierN8LUvj\n\t6qOPTd+Uk7NFzL65qkh80ZKNBFddx1AabQpSVQKbdcLb8OFs85kuSvFdgqZwgxA1vl4TFhNz\n\tPZ79NAmXLackAx3sOVFhk4WQaKRshCB7cSl+RIng5S/ThOBlwNlcKG7j7W2MC06BlTbdEkUp\n\tECzuuRBv8wX4OQl+hbWbB/VKIx5HKlLu1eypen/5lNVzSqMMIYkkZcjV2SWQyUGxSwq0O/sx\n\tS0A8/atCHUXOboUsn54qdxrVDaK+6jIAuo8JiRWctP16KjzUM7MO0/+4zllM8EY57rXrj48j\n\tsbEYX0YQnzaj+jO6kJtoZsIaYR7rMMq9aUAjyiaEZpmP1qF/2sYenDx0Fg2BSlLvLvXM0vU8\n\tpQk3kgDu7kb/7PRYrZvBsr21EIQoIjXbZxDz/o7z95frkP71EaICttZ6k9q5oxxA5WC6sTXc\n\tMW8zs8avFNuA9VpXt0YupJd2ijtZy2mpZNG02fFVXhIn4G807G7+9mhuC4XG5rKlBBUXTvPU\n\tAfYnB4JBDLmLzBFavQfvonSfbitgXwCG3vS+9HEwAjU30Bar1PEOmIbiAoMzuKeRm2LVpmq4\n\tWZw01QYHU/GUV/zHJSFk","Organization":"Ideas on Board","Message-ID":"<87945c10-8c1f-18ee-5e14-cabe0fcca1da@ideasonboard.com>","Date":"Fri, 12 Mar 2021 14:22:41 +0000","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101\n\tThunderbird/68.10.0","MIME-Version":"1.0","In-Reply-To":"","Content-Language":"en-GB","Subject":"Re: [libcamera-devel] [PATCH v4 1/7] libcamera: delayed_controls:\n\tAdd notion of priority write","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","Reply-To":"kieran.bingham@ideasonboard.com","Cc":"libcamera devel ","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":15631,"web_url":"https://patchwork.libcamera.org/comment/15631/","msgid":"","date":"2021-03-12T20:56:16","subject":"Re: [libcamera-devel] [PATCH v4 1/7] libcamera: delayed_controls:\n\tAdd notion of priority write","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Naush,\n\nThank you for the patch.\n\nOn Thu, Mar 04, 2021 at 08:17:22AM +0000, Naushir Patuck wrote:\n> If an exposure time change adjusts the vblanking limits, and we set both\n> VBLANK and EXPOSURE controls through the VIDIOC_S_EXT_CTRLS ioctl, the\n> latter may fail if the value is outside of the limits calculated by the\n> old VBLANK value. This is a limitation in V4L2 and cannot be fixed by\n> setting VBLANK before EXPOSURE in a single VIDIOC_S_EXT_CTRLS ioctl.\n> \n> The workaround here is to have the DelayedControls object mark the\n> VBLANK control as \"priority write\", which then write VBLANK separately\n> from (and ahead of) any other controls. This way, the sensor driver will\n> update the EXPOSURE control with new limits before the new values is\n> presented, and will thus be seen as valid.\n> \n> To support this, a new struct DelayedControls::ControlParams is used in\n> the constructor to provide the control delay value as well as the\n> priority write flag.\n> \n> Signed-off-by: Naushir Patuck \n> Tested-by: David Plowman \n> Reviewed-by: Paul Elder \n> ---\n> include/libcamera/internal/delayed_controls.h | 9 +++-\n> src/libcamera/delayed_controls.cpp | 54 +++++++++++++------\n> src/libcamera/pipeline/ipu3/ipu3.cpp | 8 +--\n> .../pipeline/raspberrypi/raspberrypi.cpp | 13 +++--\n> src/libcamera/pipeline/rkisp1/rkisp1.cpp | 8 +--\n> test/delayed_contols.cpp | 20 +++----\n> 6 files changed, 68 insertions(+), 44 deletions(-)\n> \n> diff --git a/include/libcamera/internal/delayed_controls.h b/include/libcamera/internal/delayed_controls.h\n> index dc447a882514..564d9f2e2440 100644\n> --- a/include/libcamera/internal/delayed_controls.h\n> +++ b/include/libcamera/internal/delayed_controls.h\n> @@ -19,8 +19,13 @@ class V4L2Device;\n> class DelayedControls\n> {\n> public:\n> +\tstruct ControlParams {\n> +\t\tunsigned int delay;\n> +\t\tbool priorityWrite;\n> +\t};\n\nI've only noticed now that the series has been merged, this structure\nisn't documented:\n\n[333/464] Generating doxygen with a custom command\ninclude/libcamera/internal/delayed_controls.h:22: warning: Compound libcamera::DelayedControls::ControlParams is not documented.\ninclude/libcamera/internal/delayed_controls.h:23: warning: Member delay (variable) of struct libcamera::DelayedControls::ControlParams is not documented.\ninclude/libcamera/internal/delayed_controls.h:24: warning: Member priorityWrite (variable) of struct libcamera::DelayedControls::ControlParams is not documented.\n\nCould you send a follow-up patch to address this ?\n\nI'm also curious if I'm the only one compiling the documentation :-)\n\n> +\n> \tDelayedControls(V4L2Device *device,\n> -\t\t\tconst std::unordered_map &delays);\n> +\t\t\tconst std::unordered_map &controlParams);\n> \n> \tvoid reset();\n> \n> @@ -64,7 +69,7 @@ private:\n> \n> \tV4L2Device *device_;\n> \t/* \\todo Evaluate if we should index on ControlId * or unsigned int */\n> -\tstd::unordered_map delays_;\n> +\tstd::unordered_map controlParams_;\n> \tunsigned int maxDelay_;\n> \n> \tbool running_;\n> diff --git a/src/libcamera/delayed_controls.cpp b/src/libcamera/delayed_controls.cpp\n> index ab1d40057c5f..3ed1dfebd035 100644\n> --- a/src/libcamera/delayed_controls.cpp\n> +++ b/src/libcamera/delayed_controls.cpp\n> @@ -40,15 +40,19 @@ LOG_DEFINE_CATEGORY(DelayedControls)\n> /**\n> * \\brief Construct a DelayedControls instance\n> * \\param[in] device The V4L2 device the controls have to be applied to\n> - * \\param[in] delays Map of the numerical V4L2 control ids to their associated\n> - * delays (in frames)\n> + * \\param[in] controlParams Map of the numerical V4L2 control ids to their\n> + * associated control parameters.\n> *\n> - * Only controls specified in \\a delays are handled. If it's desired to mix\n> - * delayed controls and controls that take effect immediately the immediate\n> - * controls must be listed in the \\a delays map with a delay value of 0.\n> + * The control parameters comprise of delays (in frames) and a priority write\n> + * flag. If this flag is set, the relevant control is written separately from,\n> + * ahead of the reset of the batched controls.\n> + *\n> + * Only controls specified in \\a controlParams are handled. If it's desired to\n> + * mix delayed controls and controls that take effect immediately the immediate\n> + * controls must be listed in the \\a controlParams map with a delay value of 0.\n> */\n> DelayedControls::DelayedControls(V4L2Device *device,\n> -\t\t\t\t const std::unordered_map &delays)\n> +\t\t\t\t const std::unordered_map &controlParams)\n> \t: device_(device), maxDelay_(0)\n> {\n> \tconst ControlInfoMap &controls = device_->controls();\n> @@ -57,12 +61,12 @@ DelayedControls::DelayedControls(V4L2Device *device,\n> \t * Create a map of control ids to delays for controls exposed by the\n> \t * device.\n> \t */\n> -\tfor (auto const &delay : delays) {\n> -\t\tauto it = controls.find(delay.first);\n> +\tfor (auto const ¶m : controlParams) {\n> +\t\tauto it = controls.find(param.first);\n> \t\tif (it == controls.end()) {\n> \t\t\tLOG(DelayedControls, Error)\n> \t\t\t\t<< \"Delay request for control id \"\n> -\t\t\t\t<< utils::hex(delay.first)\n> +\t\t\t\t<< utils::hex(param.first)\n> \t\t\t\t<< \" but control is not exposed by device \"\n> \t\t\t\t<< device_->deviceNode();\n> \t\t\tcontinue;\n> @@ -70,13 +74,14 @@ DelayedControls::DelayedControls(V4L2Device *device,\n> \n> \t\tconst ControlId *id = it->first;\n> \n> -\t\tdelays_[id] = delay.second;\n> +\t\tcontrolParams_[id] = param.second;\n> \n> \t\tLOG(DelayedControls, Debug)\n> -\t\t\t<< \"Set a delay of \" << delays_[id]\n> +\t\t\t<< \"Set a delay of \" << controlParams_[id].delay\n> +\t\t\t<< \" and priority write flag \" << controlParams_[id].priorityWrite\n> \t\t\t<< \" for \" << id->name();\n> \n> -\t\tmaxDelay_ = std::max(maxDelay_, delays_[id]);\n> +\t\tmaxDelay_ = std::max(maxDelay_, controlParams_[id].delay);\n> \t}\n> \n> \treset();\n> @@ -97,8 +102,8 @@ void DelayedControls::reset()\n> \n> \t/* Retrieve control as reported by the device. */\n> \tstd::vector ids;\n> -\tfor (auto const &delay : delays_)\n> -\t\tids.push_back(delay.first->id());\n> +\tfor (auto const ¶m : controlParams_)\n> +\t\tids.push_back(param.first->id());\n> \n> \tControlList controls = device_->getControls(ids);\n> \n> @@ -140,7 +145,7 @@ bool DelayedControls::push(const ControlList &controls)\n> \n> \t\tconst ControlId *id = it->second;\n> \n> -\t\tif (delays_.find(id) == delays_.end())\n> +\t\tif (controlParams_.find(id) == controlParams_.end())\n> \t\t\treturn false;\n> \n> \t\tInfo &info = values_[id][queueCount_];\n> @@ -220,12 +225,27 @@ void DelayedControls::applyControls(uint32_t sequence)\n> \tControlList out(device_->controls());\n> \tfor (const auto &ctrl : values_) {\n> \t\tconst ControlId *id = ctrl.first;\n> -\t\tunsigned int delayDiff = maxDelay_ - delays_[id];\n> +\t\tunsigned int delayDiff = maxDelay_ - controlParams_[id].delay;\n> \t\tunsigned int index = std::max(0, writeCount_ - delayDiff);\n> \t\tconst Info &info = ctrl.second[index];\n> \n> \t\tif (info.updated) {\n> -\t\t\tout.set(id->id(), info);\n> +\t\t\tif (controlParams_[id].priorityWrite) {\n> +\t\t\t\t/*\n> +\t\t\t\t * This control must be written now, it could\n> +\t\t\t\t * affect validity of the other controls.\n> +\t\t\t\t */\n> +\t\t\t\tControlList priority(device_->controls());\n> +\t\t\t\tpriority.set(id->id(), info);\n> +\t\t\t\tdevice_->setControls(&priority);\n> +\t\t\t} else {\n> +\t\t\t\t/*\n> +\t\t\t\t * Batch up the list of controls and write them\n> +\t\t\t\t * at the end of the function.\n> +\t\t\t\t */\n> +\t\t\t\tout.set(id->id(), info);\n> +\t\t\t}\n> +\n> \t\t\tLOG(DelayedControls, Debug)\n> \t\t\t\t<< \"Setting \" << id->name()\n> \t\t\t\t<< \" to \" << info.toString()\n> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> index 3e6b88af4188..ac92f066a07e 100644\n> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> @@ -963,14 +963,14 @@ int PipelineHandlerIPU3::registerCameras()\n> \t\t * a sensor database. For now use generic values taken from\n> \t\t * the Raspberry Pi and listed as 'generic values'.\n> \t\t */\n> -\t\tstd::unordered_map delays = {\n> -\t\t\t{ V4L2_CID_ANALOGUE_GAIN, 1 },\n> -\t\t\t{ V4L2_CID_EXPOSURE, 2 },\n> +\t\tstd::unordered_map params = {\n> +\t\t\t{ V4L2_CID_ANALOGUE_GAIN, { 1, false } },\n> +\t\t\t{ V4L2_CID_EXPOSURE, { 2, false } },\n> \t\t};\n> \n> \t\tdata->delayedCtrls_ =\n> \t\t\tstd::make_unique(cio2->sensor()->device(),\n> -\t\t\t\t\t\t\t delays);\n> +\t\t\t\t\t\t\t params);\n> \t\tdata->cio2_.frameStart().connect(data->delayedCtrls_.get(),\n> \t\t\t\t\t\t &DelayedControls::applyControls);\n> \n> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> index a60415d93705..ba74ace183db 100644\n> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> @@ -1244,16 +1244,15 @@ int RPiCameraData::configureIPA(const CameraConfiguration *config)\n> \tif (result.params & ipa::RPi::ConfigSensorParams) {\n> \t\t/*\n> \t\t * Setup our delayed control writer with the sensor default\n> -\t\t * gain and exposure delays.\n> +\t\t * gain and exposure delays. Mark VBLANK for priority write.\n> \t\t */\n> -\t\tstd::unordered_map delays = {\n> -\t\t\t{ V4L2_CID_ANALOGUE_GAIN, result.sensorConfig.gainDelay },\n> -\t\t\t{ V4L2_CID_EXPOSURE, result.sensorConfig.exposureDelay },\n> -\t\t\t{ V4L2_CID_VBLANK, result.sensorConfig.vblank }\n> +\t\tstd::unordered_map params = {\n> +\t\t\t{ V4L2_CID_ANALOGUE_GAIN, { result.sensorConfig.gainDelay, false } },\n> +\t\t\t{ V4L2_CID_EXPOSURE, { result.sensorConfig.exposureDelay, false } },\n> +\t\t\t{ V4L2_CID_VBLANK, { result.sensorConfig.vblank, true } }\n> \t\t};\n> \n> -\t\tdelayedCtrls_ = std::make_unique(unicam_[Unicam::Image].dev(), delays);\n> -\n> +\t\tdelayedCtrls_ = std::make_unique(unicam_[Unicam::Image].dev(), params);\n> \t\tsensorMetadata_ = result.sensorConfig.sensorMetadata;\n> \t}\n> \n> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> index a794501a9c8d..17c0f9751cd3 100644\n> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> @@ -938,14 +938,14 @@ int PipelineHandlerRkISP1::createCamera(MediaEntity *sensor)\n> \t * a sensor database. For now use generic values taken from\n> \t * the Raspberry Pi and listed as generic values.\n> \t */\n> -\tstd::unordered_map delays = {\n> -\t\t{ V4L2_CID_ANALOGUE_GAIN, 1 },\n> -\t\t{ V4L2_CID_EXPOSURE, 2 },\n> +\tstd::unordered_map params = {\n> +\t\t{ V4L2_CID_ANALOGUE_GAIN, { 1, false } },\n> +\t\t{ V4L2_CID_EXPOSURE, { 2, false } },\n> \t};\n> \n> \tdata->delayedCtrls_ =\n> \t\tstd::make_unique(data->sensor_->device(),\n> -\t\t\t\t\t\t delays);\n> +\t\t\t\t\t\t params);\n> \tisp_->frameStart.connect(data->delayedCtrls_.get(),\n> \t\t\t\t &DelayedControls::applyControls);\n> \n> diff --git a/test/delayed_contols.cpp b/test/delayed_contols.cpp\n> index 50169b12e566..3855eb18ecd4 100644\n> --- a/test/delayed_contols.cpp\n> +++ b/test/delayed_contols.cpp\n> @@ -72,8 +72,8 @@ protected:\n> \n> \tint singleControlNoDelay()\n> \t{\n> -\t\tstd::unordered_map delays = {\n> -\t\t\t{ V4L2_CID_BRIGHTNESS, 0 },\n> +\t\tstd::unordered_map delays = {\n> +\t\t\t{ V4L2_CID_BRIGHTNESS, { 0, false } },\n> \t\t};\n> \t\tstd::unique_ptr delayed =\n> \t\t\tstd::make_unique(dev_.get(), delays);\n> @@ -109,8 +109,8 @@ protected:\n> \n> \tint singleControlWithDelay()\n> \t{\n> -\t\tstd::unordered_map delays = {\n> -\t\t\t{ V4L2_CID_BRIGHTNESS, 1 },\n> +\t\tstd::unordered_map delays = {\n> +\t\t\t{ V4L2_CID_BRIGHTNESS, { 1, false } },\n> \t\t};\n> \t\tstd::unique_ptr delayed =\n> \t\t\tstd::make_unique(dev_.get(), delays);\n> @@ -150,9 +150,9 @@ protected:\n> \n> \tint dualControlsWithDelay(uint32_t startOffset)\n> \t{\n> -\t\tstd::unordered_map delays = {\n> -\t\t\t{ V4L2_CID_BRIGHTNESS, 1 },\n> -\t\t\t{ V4L2_CID_CONTRAST, 2 },\n> +\t\tstd::unordered_map delays = {\n> +\t\t\t{ V4L2_CID_BRIGHTNESS, { 1, false } },\n> +\t\t\t{ V4L2_CID_CONTRAST, { 2, false } },\n> \t\t};\n> \t\tstd::unique_ptr delayed =\n> \t\t\tstd::make_unique(dev_.get(), delays);\n> @@ -197,9 +197,9 @@ protected:\n> \n> \tint dualControlsMultiQueue()\n> \t{\n> -\t\tstd::unordered_map delays = {\n> -\t\t\t{ V4L2_CID_BRIGHTNESS, 1 },\n> -\t\t\t{ V4L2_CID_CONTRAST, 2 },\n> +\t\tstd::unordered_map delays = {\n> +\t\t\t{ V4L2_CID_BRIGHTNESS, { 1, false } },\n> +\t\t\t{ V4L2_CID_CONTRAST, { 2, false } }\n> \t\t};\n> \t\tstd::unique_ptr delayed =\n> \t\t\tstd::make_unique(dev_.get(), delays);","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 6843FBD80C\n\tfor ;\n\tFri, 12 Mar 2021 20:57:02 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7CA8D68C68;\n\tFri, 12 Mar 2021 21:57:01 +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 DC7D268AA1\n\tfor ;\n\tFri, 12 Mar 2021 21:56:59 +0100 (CET)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 445DE88F;\n\tFri, 12 Mar 2021 21:56:56 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"egl+tcID\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1615582619;\n\tbh=Fvx54ZbnpMaJ+JZCBXQrB72WEu0gkYv9Tv2Ey5Y97Mc=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=egl+tcID2olN5UZbQLFslpEoYEfn3b5BEZsyM6F11AaDdIhO+V9CibL/soKPtNPzO\n\t2M5k+4DmOsDh89wsVMc4vYkdN99TCoAMIQHCOSogqnCfNSZFi3qAdLyrdvSaZv67EG\n\tHQy34rxtL0IvjX7B75AxkY0+kiWoIaR8+M0bhffk=","Date":"Fri, 12 Mar 2021 22:56:16 +0200","From":"Laurent Pinchart ","To":"Naushir Patuck ","Message-ID":"","References":"<20210304081728.1058394-1-naush@raspberrypi.com>\n\t<20210304081728.1058394-2-naush@raspberrypi.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20210304081728.1058394-2-naush@raspberrypi.com>","Subject":"Re: [libcamera-devel] [PATCH v4 1/7] libcamera: delayed_controls:\n\tAdd notion of priority write","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","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":15640,"web_url":"https://patchwork.libcamera.org/comment/15640/","msgid":"","date":"2021-03-13T07:33:43","subject":"Re: [libcamera-devel] [PATCH v4 1/7] libcamera: delayed_controls:\n\tAdd notion of priority write","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hi Laurent,\n\nOn Fri, 12 Mar 2021, 8:57 pm Laurent Pinchart, <\nlaurent.pinchart@ideasonboard.com> wrote:\n\n> Hi Naush,\n>\n> Thank you for the patch.\n>\n> On Thu, Mar 04, 2021 at 08:17:22AM +0000, Naushir Patuck wrote:\n> > If an exposure time change adjusts the vblanking limits, and we set both\n> > VBLANK and EXPOSURE controls through the VIDIOC_S_EXT_CTRLS ioctl, the\n> > latter may fail if the value is outside of the limits calculated by the\n> > old VBLANK value. This is a limitation in V4L2 and cannot be fixed by\n> > setting VBLANK before EXPOSURE in a single VIDIOC_S_EXT_CTRLS ioctl.\n> >\n> > The workaround here is to have the DelayedControls object mark the\n> > VBLANK control as \"priority write\", which then write VBLANK separately\n> > from (and ahead of) any other controls. This way, the sensor driver will\n> > update the EXPOSURE control with new limits before the new values is\n> > presented, and will thus be seen as valid.\n> >\n> > To support this, a new struct DelayedControls::ControlParams is used in\n> > the constructor to provide the control delay value as well as the\n> > priority write flag.\n> >\n> > Signed-off-by: Naushir Patuck \n> > Tested-by: David Plowman \n> > Reviewed-by: Paul Elder \n> > ---\n> > include/libcamera/internal/delayed_controls.h | 9 +++-\n> > src/libcamera/delayed_controls.cpp | 54 +++++++++++++------\n> > src/libcamera/pipeline/ipu3/ipu3.cpp | 8 +--\n> > .../pipeline/raspberrypi/raspberrypi.cpp | 13 +++--\n> > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 8 +--\n> > test/delayed_contols.cpp | 20 +++----\n> > 6 files changed, 68 insertions(+), 44 deletions(-)\n> >\n> > diff --git a/include/libcamera/internal/delayed_controls.h\n> b/include/libcamera/internal/delayed_controls.h\n> > index dc447a882514..564d9f2e2440 100644\n> > --- a/include/libcamera/internal/delayed_controls.h\n> > +++ b/include/libcamera/internal/delayed_controls.h\n> > @@ -19,8 +19,13 @@ class V4L2Device;\n> > class DelayedControls\n> > {\n> > public:\n> > + struct ControlParams {\n> > + unsigned int delay;\n> > + bool priorityWrite;\n> > + };\n>\n> I've only noticed now that the series has been merged, this structure\n> isn't documented:\n>\n> [333/464] Generating doxygen with a custom command\n> include/libcamera/internal/delayed_controls.h:22: warning: Compound\n> libcamera::DelayedControls::ControlParams is not documented.\n> include/libcamera/internal/delayed_controls.h:23: warning: Member delay\n> (variable) of struct libcamera::DelayedControls::ControlParams is not\n> documented.\n> include/libcamera/internal/delayed_controls.h:24: warning: Member\n> priorityWrite (variable) of struct\n> libcamera::DelayedControls::ControlParams is not documented.\n>\n> Could you send a follow-up patch to address this ?\n>\n\nApologies for missing the doc update. I will post a patch with it soon.\n\n\n> I'm also curious if I'm the only one compiling the documentation :-)\n>\n\nMy documentation compilation is likely disabled, but I will switch it back\non now :-)\n\nRegards,\nNaush\n\n\n> > +\n> > DelayedControls(V4L2Device *device,\n> > - const std::unordered_map\n> &delays);\n> > + const std::unordered_map\n> &controlParams);\n> >\n> > void reset();\n> >\n> > @@ -64,7 +69,7 @@ private:\n> >\n> > V4L2Device *device_;\n> > /* \\todo Evaluate if we should index on ControlId * or unsigned\n> int */\n> > - std::unordered_map delays_;\n> > + std::unordered_map\n> controlParams_;\n> > unsigned int maxDelay_;\n> >\n> > bool running_;\n> > diff --git a/src/libcamera/delayed_controls.cpp\n> b/src/libcamera/delayed_controls.cpp\n> > index ab1d40057c5f..3ed1dfebd035 100644\n> > --- a/src/libcamera/delayed_controls.cpp\n> > +++ b/src/libcamera/delayed_controls.cpp\n> > @@ -40,15 +40,19 @@ LOG_DEFINE_CATEGORY(DelayedControls)\n> > /**\n> > * \\brief Construct a DelayedControls instance\n> > * \\param[in] device The V4L2 device the controls have to be applied to\n> > - * \\param[in] delays Map of the numerical V4L2 control ids to their\n> associated\n> > - * delays (in frames)\n> > + * \\param[in] controlParams Map of the numerical V4L2 control ids to\n> their\n> > + * associated control parameters.\n> > *\n> > - * Only controls specified in \\a delays are handled. If it's desired to\n> mix\n> > - * delayed controls and controls that take effect immediately the\n> immediate\n> > - * controls must be listed in the \\a delays map with a delay value of 0.\n> > + * The control parameters comprise of delays (in frames) and a priority\n> write\n> > + * flag. If this flag is set, the relevant control is written\n> separately from,\n> > + * ahead of the reset of the batched controls.\n> > + *\n> > + * Only controls specified in \\a controlParams are handled. If it's\n> desired to\n> > + * mix delayed controls and controls that take effect immediately the\n> immediate\n> > + * controls must be listed in the \\a controlParams map with a delay\n> value of 0.\n> > */\n> > DelayedControls::DelayedControls(V4L2Device *device,\n> > - const std::unordered_map unsigned int> &delays)\n> > + const std::unordered_map ControlParams> &controlParams)\n> > : device_(device), maxDelay_(0)\n> > {\n> > const ControlInfoMap &controls = device_->controls();\n> > @@ -57,12 +61,12 @@ DelayedControls::DelayedControls(V4L2Device *device,\n> > * Create a map of control ids to delays for controls exposed by\n> the\n> > * device.\n> > */\n> > - for (auto const &delay : delays) {\n> > - auto it = controls.find(delay.first);\n> > + for (auto const ¶m : controlParams) {\n> > + auto it = controls.find(param.first);\n> > if (it == controls.end()) {\n> > LOG(DelayedControls, Error)\n> > << \"Delay request for control id \"\n> > - << utils::hex(delay.first)\n> > + << utils::hex(param.first)\n> > << \" but control is not exposed by device \"\n> > << device_->deviceNode();\n> > continue;\n> > @@ -70,13 +74,14 @@ DelayedControls::DelayedControls(V4L2Device *device,\n> >\n> > const ControlId *id = it->first;\n> >\n> > - delays_[id] = delay.second;\n> > + controlParams_[id] = param.second;\n> >\n> > LOG(DelayedControls, Debug)\n> > - << \"Set a delay of \" << delays_[id]\n> > + << \"Set a delay of \" << controlParams_[id].delay\n> > + << \" and priority write flag \" <<\n> controlParams_[id].priorityWrite\n> > << \" for \" << id->name();\n> >\n> > - maxDelay_ = std::max(maxDelay_, delays_[id]);\n> > + maxDelay_ = std::max(maxDelay_, controlParams_[id].delay);\n> > }\n> >\n> > reset();\n> > @@ -97,8 +102,8 @@ void DelayedControls::reset()\n> >\n> > /* Retrieve control as reported by the device. */\n> > std::vector ids;\n> > - for (auto const &delay : delays_)\n> > - ids.push_back(delay.first->id());\n> > + for (auto const ¶m : controlParams_)\n> > + ids.push_back(param.first->id());\n> >\n> > ControlList controls = device_->getControls(ids);\n> >\n> > @@ -140,7 +145,7 @@ bool DelayedControls::push(const ControlList\n> &controls)\n> >\n> > const ControlId *id = it->second;\n> >\n> > - if (delays_.find(id) == delays_.end())\n> > + if (controlParams_.find(id) == controlParams_.end())\n> > return false;\n> >\n> > Info &info = values_[id][queueCount_];\n> > @@ -220,12 +225,27 @@ void DelayedControls::applyControls(uint32_t\n> sequence)\n> > ControlList out(device_->controls());\n> > for (const auto &ctrl : values_) {\n> > const ControlId *id = ctrl.first;\n> > - unsigned int delayDiff = maxDelay_ - delays_[id];\n> > + unsigned int delayDiff = maxDelay_ -\n> controlParams_[id].delay;\n> > unsigned int index = std::max(0, writeCount_ -\n> delayDiff);\n> > const Info &info = ctrl.second[index];\n> >\n> > if (info.updated) {\n> > - out.set(id->id(), info);\n> > + if (controlParams_[id].priorityWrite) {\n> > + /*\n> > + * This control must be written now, it\n> could\n> > + * affect validity of the other controls.\n> > + */\n> > + ControlList priority(device_->controls());\n> > + priority.set(id->id(), info);\n> > + device_->setControls(&priority);\n> > + } else {\n> > + /*\n> > + * Batch up the list of controls and write\n> them\n> > + * at the end of the function.\n> > + */\n> > + out.set(id->id(), info);\n> > + }\n> > +\n> > LOG(DelayedControls, Debug)\n> > << \"Setting \" << id->name()\n> > << \" to \" << info.toString()\n> > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > index 3e6b88af4188..ac92f066a07e 100644\n> > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > @@ -963,14 +963,14 @@ int PipelineHandlerIPU3::registerCameras()\n> > * a sensor database. For now use generic values taken from\n> > * the Raspberry Pi and listed as 'generic values'.\n> > */\n> > - std::unordered_map delays = {\n> > - { V4L2_CID_ANALOGUE_GAIN, 1 },\n> > - { V4L2_CID_EXPOSURE, 2 },\n> > + std::unordered_map DelayedControls::ControlParams> params = {\n> > + { V4L2_CID_ANALOGUE_GAIN, { 1, false } },\n> > + { V4L2_CID_EXPOSURE, { 2, false } },\n> > };\n> >\n> > data->delayedCtrls_ =\n> >\n> std::make_unique(cio2->sensor()->device(),\n> > - delays);\n> > + params);\n> > data->cio2_.frameStart().connect(data->delayedCtrls_.get(),\n> >\n> &DelayedControls::applyControls);\n> >\n> > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > index a60415d93705..ba74ace183db 100644\n> > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > @@ -1244,16 +1244,15 @@ int RPiCameraData::configureIPA(const\n> CameraConfiguration *config)\n> > if (result.params & ipa::RPi::ConfigSensorParams) {\n> > /*\n> > * Setup our delayed control writer with the sensor default\n> > - * gain and exposure delays.\n> > + * gain and exposure delays. Mark VBLANK for priority\n> write.\n> > */\n> > - std::unordered_map delays = {\n> > - { V4L2_CID_ANALOGUE_GAIN,\n> result.sensorConfig.gainDelay },\n> > - { V4L2_CID_EXPOSURE,\n> result.sensorConfig.exposureDelay },\n> > - { V4L2_CID_VBLANK, result.sensorConfig.vblank }\n> > + std::unordered_map DelayedControls::ControlParams> params = {\n> > + { V4L2_CID_ANALOGUE_GAIN, {\n> result.sensorConfig.gainDelay, false } },\n> > + { V4L2_CID_EXPOSURE, {\n> result.sensorConfig.exposureDelay, false } },\n> > + { V4L2_CID_VBLANK, { result.sensorConfig.vblank,\n> true } }\n> > };\n> >\n> > - delayedCtrls_ =\n> std::make_unique(unicam_[Unicam::Image].dev(), delays);\n> > -\n> > + delayedCtrls_ =\n> std::make_unique(unicam_[Unicam::Image].dev(), params);\n> > sensorMetadata_ = result.sensorConfig.sensorMetadata;\n> > }\n> >\n> > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > index a794501a9c8d..17c0f9751cd3 100644\n> > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > @@ -938,14 +938,14 @@ int\n> PipelineHandlerRkISP1::createCamera(MediaEntity *sensor)\n> > * a sensor database. For now use generic values taken from\n> > * the Raspberry Pi and listed as generic values.\n> > */\n> > - std::unordered_map delays = {\n> > - { V4L2_CID_ANALOGUE_GAIN, 1 },\n> > - { V4L2_CID_EXPOSURE, 2 },\n> > + std::unordered_map\n> params = {\n> > + { V4L2_CID_ANALOGUE_GAIN, { 1, false } },\n> > + { V4L2_CID_EXPOSURE, { 2, false } },\n> > };\n> >\n> > data->delayedCtrls_ =\n> > std::make_unique(data->sensor_->device(),\n> > - delays);\n> > + params);\n> > isp_->frameStart.connect(data->delayedCtrls_.get(),\n> > &DelayedControls::applyControls);\n> >\n> > diff --git a/test/delayed_contols.cpp b/test/delayed_contols.cpp\n> > index 50169b12e566..3855eb18ecd4 100644\n> > --- a/test/delayed_contols.cpp\n> > +++ b/test/delayed_contols.cpp\n> > @@ -72,8 +72,8 @@ protected:\n> >\n> > int singleControlNoDelay()\n> > {\n> > - std::unordered_map delays = {\n> > - { V4L2_CID_BRIGHTNESS, 0 },\n> > + std::unordered_map DelayedControls::ControlParams> delays = {\n> > + { V4L2_CID_BRIGHTNESS, { 0, false } },\n> > };\n> > std::unique_ptr delayed =\n> > std::make_unique(dev_.get(),\n> delays);\n> > @@ -109,8 +109,8 @@ protected:\n> >\n> > int singleControlWithDelay()\n> > {\n> > - std::unordered_map delays = {\n> > - { V4L2_CID_BRIGHTNESS, 1 },\n> > + std::unordered_map DelayedControls::ControlParams> delays = {\n> > + { V4L2_CID_BRIGHTNESS, { 1, false } },\n> > };\n> > std::unique_ptr delayed =\n> > std::make_unique(dev_.get(),\n> delays);\n> > @@ -150,9 +150,9 @@ protected:\n> >\n> > int dualControlsWithDelay(uint32_t startOffset)\n> > {\n> > - std::unordered_map delays = {\n> > - { V4L2_CID_BRIGHTNESS, 1 },\n> > - { V4L2_CID_CONTRAST, 2 },\n> > + std::unordered_map DelayedControls::ControlParams> delays = {\n> > + { V4L2_CID_BRIGHTNESS, { 1, false } },\n> > + { V4L2_CID_CONTRAST, { 2, false } },\n> > };\n> > std::unique_ptr delayed =\n> > std::make_unique(dev_.get(),\n> delays);\n> > @@ -197,9 +197,9 @@ protected:\n> >\n> > int dualControlsMultiQueue()\n> > {\n> > - std::unordered_map delays = {\n> > - { V4L2_CID_BRIGHTNESS, 1 },\n> > - { V4L2_CID_CONTRAST, 2 },\n> > + std::unordered_map DelayedControls::ControlParams> delays = {\n> > + { V4L2_CID_BRIGHTNESS, { 1, false } },\n> > + { V4L2_CID_CONTRAST, { 2, false } }\n> > };\n> > std::unique_ptr delayed =\n> > std::make_unique(dev_.get(),\n> delays);\n>\n> --\n> Regards,\n>\n> Laurent Pinchart\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 931E1BD80C\n\tfor ;\n\tSat, 13 Mar 2021 07:33:56 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C04CD68C69;\n\tSat, 13 Mar 2021 08:33:55 +0100 (CET)","from mail-lf1-x130.google.com (mail-lf1-x130.google.com\n\t[IPv6:2a00:1450:4864:20::130])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 067B3602E8\n\tfor ;\n\tSat, 13 Mar 2021 08:33:54 +0100 (CET)","by mail-lf1-x130.google.com with SMTP id p21so48910967lfu.11\n\tfor ;\n\tFri, 12 Mar 2021 23:33:54 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"XOOR+KHn\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=ojbLStB2iln7TXw/h2cUvTTIYCL74Xh+ywnGoG93Mq8=;\n\tb=XOOR+KHnXkcS0ORQoOLTpN75dhzy76KPlHgSQowtQNaeY3jk3HGOQzVvf2ZyPgw+d0\n\ty0/BcYUuKiczOd8GTJdCI9QHMDdQf98cSAvEEt3bUYc/ftbfvOg1/mnA5bq2pXU412rN\n\tABK1Am1wguQF0xd0AuMtAtemTLrhbSG11qaPpJ2ytpUISzqrUKTAryldyov/fInU+SLe\n\t6HGnwkaCWqQYNp84Vd+x7KP0abpwV7KBR747g3zWULnf0KI2M55/Qjp5YflCSKG9978J\n\tFDv2ro3zNZkO9YGSiRT01MzNepinChcI1dw/rMsGwCIpNliHDnE5h8zh9LGCKw2q/nAf\n\tWrvA==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=ojbLStB2iln7TXw/h2cUvTTIYCL74Xh+ywnGoG93Mq8=;\n\tb=U3jBwvDqxbN0jXh/QArZrROaGQNOichK+Pu8c2Kh8D5zKY1Z0dFhLiCVehONCKnnvQ\n\tDoyaggLfDHRASn4zFx2Pu29ZkUkL83LK8kdWSGyhHYUcW8zuZSUYLhh6FdhV5CPsdNuC\n\tzYi6pfrUWhM4dYQGZBQJiteUnhyQBBGK2DZSQqP3H/vsL36NkYwzxA3VM3sWDdQVbKI7\n\tT6O95Fd5zQMa420YWqQVY5OgWc4KZrUmoSYM69qcvyRSi6tPLufEMkBvHZZGgq+zRCEF\n\tThyoj8i/xSosWfUwZkLpJaosb3PGvPJnaditIYtlBkCBZ5itAHbt2M6obX1ilNhO3MLR\n\td4zQ==","X-Gm-Message-State":"AOAM530LVu9elTg/3KT4zee/8E9ro/8S/LDE/VGqh+pkkG49f0ICMInA\n\tz0bP1LjdaRf2q3kpg8BrrcvpobcZjQMLrbAkRbk6U7I1jhY=","X-Google-Smtp-Source":"ABdhPJwOarEMQE7ai27lwoP2iSS4dQcJwmCUEO0vPnskXtQcBKo+eegonV2AjBOm6tjNf6SF5HCDEpaLsMvZ981F+cU=","X-Received":"by 2002:a19:3f58:: with SMTP id\n\tm85mr1811587lfa.617.1615620833958; \n\tFri, 12 Mar 2021 23:33:53 -0800 (PST)","MIME-Version":"1.0","References":"<20210304081728.1058394-1-naush@raspberrypi.com>\n\t<20210304081728.1058394-2-naush@raspberrypi.com>\n\t","In-Reply-To":"","From":"Naushir Patuck ","Date":"Sat, 13 Mar 2021 07:33:43 +0000","Message-ID":"","To":"Laurent Pinchart ","Subject":"Re: [libcamera-devel] [PATCH v4 1/7] libcamera: delayed_controls:\n\tAdd notion of priority write","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","Cc":"libcamera devel ","Content-Type":"multipart/mixed;\n\tboundary=\"===============0109919251672568215==\"","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]