[{"id":25803,"web_url":"https://patchwork.libcamera.org/comment/25803/","msgid":"","date":"2022-11-15T14:01:01","subject":"Re: [libcamera-devel] [PATCH v6 4/8] pipeline: raspberrypi:\n\tdelayed_controls: Add user cookie to DelayedControls","submitter":{"id":42,"url":"https://patchwork.libcamera.org/api/people/42/","name":"David Plowman","email":"david.plowman@raspberrypi.com"},"content":"Hi Naush\n\nThanks for this patch.\n\nOn Tue, 15 Nov 2022 at 09:08, Naushir Patuck via libcamera-devel\n wrote:\n>\n> Allow the caller to provide a cookie value to DelayedControls::reset and\n> DelayedControls::push. This cookie value is returned from DelayedControls::get\n> for the frame that has the control values applied to it.\n>\n> The cookie value is useful in tracking when a set of controls has been applied\n> to a frame. In a subsequent commit, it will be used by the Raspberry Pi IPA to\n> track the IPA context used when setting the control values.\n>\n> Signed-off-by: Naushir Patuck \n\nI guess the only thing I wonder very slightly, given that these\nchances no longer affect the world at large, is whether we should call\n\"cookies\" something more meaningful. But that's just me thinking out\nloud, and I would honestly not suggest doing anything at this point!\n\nReviewed-by: David Plowman \n\nThanks!\nDavid\n\n> ---\n> .../pipeline/raspberrypi/delayed_controls.cpp | 14 ++++++++------\n> .../pipeline/raspberrypi/delayed_controls.h | 8 +++++---\n> src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 6 +++---\n> 3 files changed, 16 insertions(+), 12 deletions(-)\n>\n> diff --git a/src/libcamera/pipeline/raspberrypi/delayed_controls.cpp b/src/libcamera/pipeline/raspberrypi/delayed_controls.cpp\n> index 867e3866cc46..3db92e7d24fb 100644\n> --- a/src/libcamera/pipeline/raspberrypi/delayed_controls.cpp\n> +++ b/src/libcamera/pipeline/raspberrypi/delayed_controls.cpp\n> @@ -108,7 +108,7 @@ DelayedControls::DelayedControls(V4L2Device *device,\n> maxDelay_ = std::max(maxDelay_, controlParams_[id].delay);\n> }\n>\n> - reset();\n> + reset(0);\n> }\n>\n> /**\n> @@ -117,10 +117,11 @@ DelayedControls::DelayedControls(V4L2Device *device,\n> * Resets the state machine to a starting position based on control values\n> * retrieved from the device.\n> */\n> -void DelayedControls::reset()\n> +void DelayedControls::reset(unsigned int cookie)\n> {\n> queueCount_ = 1;\n> writeCount_ = 0;\n> + cookies_[0] = cookie;\n>\n> /* Retrieve control as reported by the device. */\n> std::vector ids;\n> @@ -150,7 +151,7 @@ void DelayedControls::reset()\n> *\n> * \\returns true if \\a controls are accepted, or false otherwise\n> */\n> -bool DelayedControls::push(const ControlList &controls)\n> +bool DelayedControls::push(const ControlList &controls, const unsigned int cookie)\n> {\n> /* Copy state from previous frame. */\n> for (auto &ctrl : values_) {\n> @@ -184,6 +185,7 @@ bool DelayedControls::push(const ControlList &controls)\n> << \" at index \" << queueCount_;\n> }\n>\n> + cookies_[queueCount_] = cookie;\n> queueCount_++;\n>\n> return true;\n> @@ -204,7 +206,7 @@ bool DelayedControls::push(const ControlList &controls)\n> *\n> * \\return The controls at \\a sequence number\n> */\n> -ControlList DelayedControls::get(uint32_t sequence)\n> +std::pair DelayedControls::get(uint32_t sequence)\n> {\n> unsigned int index = std::max(0, sequence - maxDelay_);\n>\n> @@ -221,7 +223,7 @@ ControlList DelayedControls::get(uint32_t sequence)\n> << \" at index \" << index;\n> }\n>\n> - return out;\n> + return { out, cookies_[index] };\n> }\n>\n> /**\n> @@ -280,7 +282,7 @@ void DelayedControls::applyControls(uint32_t sequence)\n> while (writeCount_ > queueCount_) {\n> LOG(RPiDelayedControls, Debug)\n> << \"Queue is empty, auto queue no-op.\";\n> - push({});\n> + push({}, cookies_[queueCount_ - 1]);\n> }\n>\n> device_->setControls(&out);\n> diff --git a/src/libcamera/pipeline/raspberrypi/delayed_controls.h b/src/libcamera/pipeline/raspberrypi/delayed_controls.h\n> index 238b86ab6cb4..61f755f0fddd 100644\n> --- a/src/libcamera/pipeline/raspberrypi/delayed_controls.h\n> +++ b/src/libcamera/pipeline/raspberrypi/delayed_controls.h\n> @@ -11,6 +11,7 @@\n>\n> #include \n> #include \n> +#include \n>\n> #include \n>\n> @@ -31,10 +32,10 @@ public:\n> DelayedControls(V4L2Device *device,\n> const std::unordered_map &controlParams);\n>\n> - void reset();\n> + void reset(unsigned int cookie);\n>\n> - bool push(const ControlList &controls);\n> - ControlList get(uint32_t sequence);\n> + bool push(const ControlList &controls, unsigned int cookie);\n> + std::pair get(uint32_t sequence);\n>\n> void applyControls(uint32_t sequence);\n>\n> @@ -78,6 +79,7 @@ private:\n> uint32_t queueCount_;\n> uint32_t writeCount_;\n> std::unordered_map> values_;\n> + RingBuffer cookies_;\n> };\n>\n> } /* namespace RPi */\n> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> index f3be4ee3b730..2c12ed1fba95 100644\n> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> @@ -1064,7 +1064,7 @@ int PipelineHandlerRPi::start(Camera *camera, const ControlList *controls)\n> * Reset the delayed controls with the gain and exposure values set by\n> * the IPA.\n> */\n> - data->delayedCtrls_->reset();\n> + data->delayedCtrls_->reset(0);\n>\n> data->state_ = RPiCameraData::State::Idle;\n>\n> @@ -1794,7 +1794,7 @@ void RPiCameraData::setIspControls(const ControlList &controls)\n>\n> void RPiCameraData::setDelayedControls(const ControlList &controls)\n> {\n> - if (!delayedCtrls_->push(controls))\n> + if (!delayedCtrls_->push(controls, 0))\n> LOG(RPI, Error) << \"V4L2 DelayedControl set failed\";\n> handleState();\n> }\n> @@ -1867,7 +1867,7 @@ void RPiCameraData::unicamBufferDequeue(FrameBuffer *buffer)\n> * Lookup the sensor controls used for this frame sequence from\n> * DelayedControl and queue them along with the frame buffer.\n> */\n> - ControlList ctrl = delayedCtrls_->get(buffer->metadata().sequence);\n> + auto [ctrl, cookie] = delayedCtrls_->get(buffer->metadata().sequence);\n> /*\n> * Add the frame timestamp to the ControlList for the IPA to use\n> * as it does not receive the FrameBuffer object.\n> --\n> 2.25.1\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 403C8BE08B\n\tfor ;\n\tTue, 15 Nov 2022 14:01:15 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 9C22963088;\n\tTue, 15 Nov 2022 15:01:14 +0100 (CET)","from mail-pf1-x42f.google.com (mail-pf1-x42f.google.com\n\t[IPv6:2607:f8b0:4864:20::42f])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 7EAB363079\n\tfor ;\n\tTue, 15 Nov 2022 15:01:13 +0100 (CET)","by mail-pf1-x42f.google.com with SMTP id v28so14167710pfi.12\n\tfor ;\n\tTue, 15 Nov 2022 06:01:13 -0800 (PST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1668520874;\n\tbh=OT7cjrKq4r31MkkWUxWcey2Gts+F/GfNI0jLnrwQfKA=;\n\th=References:In-Reply-To:Date:To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=jqIN9P4ia7TFG4NfsZHOmFFB1n41xUDwDSg2U9bGzRJKg3qKc09zdzAe2rXkYIhOU\n\tVEbdy1eQaAYc/4BiOs9bzTzsuAebrULS66mmI0Q13kobYzdzlAdVYvvKpnUSkiaSVT\n\t/wXNcg8XY9FmI30DlDiyV9KbLkR1EJ6aGsBwNrINxiT/30zxKWe2C0IA4mRklGidDu\n\tgLgIFvKoapIEAOyM2QVU4tFvj/xs7T03YBFEi2tds3tAdZwV2vBv0ngZzmc0zoKgdv\n\tNe1TT8mwSaDuLoFsJ3qRAi052Pf9wOkK7OX0wOUN6E7RKUiSoZvJnaSXtKa+ISWg1a\n\t5Ej4kZakunADg==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:from:to:cc:subject:date:message-id:reply-to;\n\tbh=SrtGdXijq3sdlfyLBwP4Vadj0Y+ryejyaGZHG1k69ow=;\n\tb=rvPglqMv3GepLwgfvjzvltT4x/DGBNUfyXzgXW8gQZ74p8B0GoAXB+CGXJYtXcbnoF\n\tEYDiRH1PLNNKlNnjKRp+7B/8ywrwm4OV8hFbM8ezgNkB6UP/VYX2CMVYg1R3i3D+V4Gj\n\tJjrD9E4BU2LFbB6jYuFqatsp5Ajl+mEmbCHrkHBasK1OBjzE8FuVzKkfEtKF1pdorn62\n\t9i/hfmUZU10yf/SBhcumz4/uNlcWSqvyPvli5JipLxRwC/BD/zdc+DkA/AxFmZmKk1DG\n\tmVRwD0eorPuajdtQu3G3pZyT9D6fvZjgJwgV8HsXCUdHyjTxbL/LeG/Va68L+1qx8ptR\n\tvCag=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=raspberrypi.com\n\theader.i=@raspberrypi.com\n\theader.b=\"rvPglqMv\"; dkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:x-gm-message-state:from:to:cc:subject:date:message-id\n\t:reply-to;\n\tbh=SrtGdXijq3sdlfyLBwP4Vadj0Y+ryejyaGZHG1k69ow=;\n\tb=zQuKxpsL8fxrgoqU9xj6uvZKMIwTQLb1IHxVN0tYunJhprpuhvhKfzDoFiWi4t/5wN\n\tgRkpO/nUha7L2TVXiiDgwf+2hiNtfW8fx/2BAAWK1USgYP7wMG11OCEkW0LE/aKfMi8J\n\t57TC5t+1sgZm9mcdtph2s2BJbixsZ7TgpIeqt2z5VuQtoSO3Ljyghpo9JTvmVBKm1hFr\n\thhymSHQGkN3UWLmtiUcntFHedNJP+cQOd9+1w68Wm1ItA73vY+MzI84Yu4wiHeVCnYt3\n\tm+TRE8K3ErhKkZCuXTrRJHT1tJiHyBIUKNqNX4PDrNZhus6P5FVHiqPRrUpGuu9QJQvG\n\tgjaw==","X-Gm-Message-State":"ANoB5pmdhBmr0t5HumyZ4sJOPanebkbZFLhHDT+51FeRNjW+HV0jOGIt\n\tZbdgf+b9wxvHD4aUUVyW2eXCBoiZxYQ7CtU5SrK/iQab/+s=","X-Google-Smtp-Source":"AA0mqf6PRjO2JIEnfJBtufdZfyHU3rh3WCKRr+15FhYCbkrmT+frPQStQcwVTpC99c/a4XqHUSc2pVGv1Dvc0mUdu1w=","X-Received":"by 2002:a63:5164:0:b0:46f:ec9f:dcb0 with SMTP id\n\tr36-20020a635164000000b0046fec9fdcb0mr16088674pgl.202.1668520871904;\n\tTue, 15 Nov 2022 06:01:11 -0800 (PST)","MIME-Version":"1.0","References":"<20221115090755.2921-1-naush@raspberrypi.com>\n\t<20221115090755.2921-5-naush@raspberrypi.com>","In-Reply-To":"<20221115090755.2921-5-naush@raspberrypi.com>","Date":"Tue, 15 Nov 2022 14:01:01 +0000","Message-ID":"","To":"Naushir Patuck ","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH v6 4/8] pipeline: raspberrypi:\n\tdelayed_controls: Add user cookie to DelayedControls","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"David Plowman via libcamera-devel ","Reply-To":"David Plowman ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]