[libcamera-devel] [PATCH] delayed_controls: Remove reduandant firstSequence_

[Laurent Pinchart]

Hi Kieran,

Thank you for the patch.

On Mon, Jul 04, 2022 at 02:37:51PM +0100, Kieran Bingham via libcamera-devel wrote:
> The DelayedControls implementation tracked the sequence numbers to
> determine the offset if a device did not commence with a sequence number
> of 0.
> 
> This guarantee is now handled by the V4L2VideoDevice.
> 
> Remove the firstSequence_ offset and the corresponding running_ flag
> which was used to track setting firstSequence_ from the DelayedControls.
> 
> Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>

With the typo in the commit message fixed,

Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

> ---
> 
> This will conflict with Jacopo's series, but is still applicable
> conceptually, whichever gets to go first.
> 
>  include/libcamera/internal/delayed_controls.h |  3 ---
>  src/libcamera/delayed_controls.cpp            | 12 ++----------
>  2 files changed, 2 insertions(+), 13 deletions(-)
> 
> diff --git a/include/libcamera/internal/delayed_controls.h b/include/libcamera/internal/delayed_controls.h
> index 703fdb66bc27..f560c823d4e9 100644
> --- a/include/libcamera/internal/delayed_controls.h
> +++ b/include/libcamera/internal/delayed_controls.h
> @@ -72,9 +72,6 @@ private:
>  	std::unordered_map<const ControlId *, ControlParams> controlParams_;
>  	unsigned int maxDelay_;
>  
> -	bool running_;
> -	uint32_t firstSequence_;
> -
>  	uint32_t queueCount_;
>  	uint32_t writeCount_;
>  	/* \todo Evaluate if we should index on ControlId * or unsigned int */
> diff --git a/src/libcamera/delayed_controls.cpp b/src/libcamera/delayed_controls.cpp
> index 9667187e9a4a..527c319113ac 100644
> --- a/src/libcamera/delayed_controls.cpp
> +++ b/src/libcamera/delayed_controls.cpp
> @@ -115,8 +115,6 @@ DelayedControls::DelayedControls(V4L2Device *device,
>   */
>  void DelayedControls::reset()
>  {
> -	running_ = false;
> -	firstSequence_ = 0;
>  	queueCount_ = 1;
>  	writeCount_ = 0;
>  
> @@ -204,8 +202,7 @@ bool DelayedControls::push(const ControlList &controls)
>   */
>  ControlList DelayedControls::get(uint32_t sequence)
>  {
> -	uint32_t adjustedSeq = sequence - firstSequence_;
> -	unsigned int index = std::max<int>(0, adjustedSeq - maxDelay_);
> +	unsigned int index = std::max<int>(0, sequence - maxDelay_);
>  
>  	ControlList out(device_->controls());
>  	for (const auto &ctrl : values_) {
> @@ -236,11 +233,6 @@ void DelayedControls::applyControls(uint32_t sequence)
>  {
>  	LOG(DelayedControls, Debug) << "frame " << sequence << " started";
>  
> -	if (!running_) {
> -		firstSequence_ = sequence;
> -		running_ = true;
> -	}
> -
>  	/*
>  	 * Create control list peeking ahead in the value queue to ensure
>  	 * values are set in time to satisfy the sensor delay.
> @@ -279,7 +271,7 @@ void DelayedControls::applyControls(uint32_t sequence)
>  		}
>  	}
>  
> -	writeCount_ = sequence - firstSequence_ + 1;
> +	writeCount_ = sequence + 1;
>  
>  	while (writeCount_ > queueCount_) {
>  		LOG(DelayedControls, Debug)

-- 
Regards,

Laurent Pinchart