[v1,04/35] libcamera: delayed_controls: Queue noop when needed, not before
diff mbox series

Message ID 20251024085130.995967-5-stefan.klug@ideasonboard.com
State New
Headers show
Series
  • rkisp1: pipeline rework for PFC
Related show

Commit Message

Stefan Klug Oct. 24, 2025, 8:50 a.m. UTC
A no-op is queued when DelayedControls runs out of controls after a
successful applyControls(). But after applyControls() there is still
some time left for new controls to be pushed until the next call to
applyControls(). To fix that, move the no-op push to the beginning of
applyContols().

Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
---
 src/libcamera/delayed_controls.cpp | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

Comments

Barnabás Pőcze Oct. 24, 2025, 11:39 a.m. UTC | #1
2025. 10. 24. 10:50 keltezéssel, Stefan Klug írta:
> A no-op is queued when DelayedControls runs out of controls after a
> successful applyControls(). But after applyControls() there is still
> some time left for new controls to be pushed until the next call to
> applyControls(). To fix that, move the no-op push to the beginning of
> applyContols().
> 
> Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
> ---
>   src/libcamera/delayed_controls.cpp | 14 ++++++++------
>   1 file changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/src/libcamera/delayed_controls.cpp b/src/libcamera/delayed_controls.cpp
> index 044c6c7325e7..67213fe87d53 100644
> --- a/src/libcamera/delayed_controls.cpp
> +++ b/src/libcamera/delayed_controls.cpp
> @@ -272,6 +272,14 @@ void DelayedControls::applyControls(uint32_t sequence)
>   {
>   	LOG(DelayedControls, Debug) << "frame " << sequence << " started";
> 
> +	while (queueCount_ - 1 < sequence) {
> +		LOG(DelayedControls, Warning)
> +			<< "Queue is empty, auto queue no-op." << queueCount_;

Missing space or some kind of separator in the log message.


> +		push(queueCount_, {});
> +	}
> +
> +	writeCount_ = sequence;

I must be missing something. The commit message says

   there is still some time left for new controls to be pushed until the next call

but the loop above does the same number of `push()` that the removed loop did.
So I don't understand what the commit message refers to, how could the user do
any "valid" `push()` that wasn't previously possible?


> +
>   	/*
>   	 * Create control list peeking ahead in the value queue to ensure
>   	 * values are set in time to satisfy the sensor delay.
> @@ -312,12 +320,6 @@ void DelayedControls::applyControls(uint32_t sequence)
> 
>   	writeCount_ = sequence + 1;
> 
> -	while (writeCount_ > queueCount_) {
> -		LOG(DelayedControls, Warning)
> -			<< "Queue is empty, auto queue no-op.";
> -		push(queueCount_, {});
> -	}
> -
>   	device_->setControls(&out);
>   }
> 
> --
> 2.48.1
>

Patch
diff mbox series

diff --git a/src/libcamera/delayed_controls.cpp b/src/libcamera/delayed_controls.cpp
index 044c6c7325e7..67213fe87d53 100644
--- a/src/libcamera/delayed_controls.cpp
+++ b/src/libcamera/delayed_controls.cpp
@@ -272,6 +272,14 @@  void DelayedControls::applyControls(uint32_t sequence)
 {
 	LOG(DelayedControls, Debug) << "frame " << sequence << " started";
 
+	while (queueCount_ - 1 < sequence) {
+		LOG(DelayedControls, Warning)
+			<< "Queue is empty, auto queue no-op." << queueCount_;
+		push(queueCount_, {});
+	}
+
+	writeCount_ = sequence;
+
 	/*
 	 * Create control list peeking ahead in the value queue to ensure
 	 * values are set in time to satisfy the sensor delay.
@@ -312,12 +320,6 @@  void DelayedControls::applyControls(uint32_t sequence)
 
 	writeCount_ = sequence + 1;
 
-	while (writeCount_ > queueCount_) {
-		LOG(DelayedControls, Warning)
-			<< "Queue is empty, auto queue no-op.";
-		push(queueCount_, {});
-	}
-
 	device_->setControls(&out);
 }