| Message ID | 20220704133751.1582959-1-kieran.bingham@ideasonboard.com |
|---|---|
| State | Accepted |
| Commit | 6f539a6d2fa992c90782a7b7077cd009fce4457b |
| Headers | show |
| Series |
|
| Related | show |
Hi Kieran 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@ideasonboard.com> It seems correct to me! Reviewed-by: Jacopo Mondi <jacopo@jmondi.org> Thanks j > --- > > 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) > -- > 2.34.1 >
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@ideasonboard.com> With the typo in the commit message fixed, Reviewed-by: Laurent Pinchart <laurent.pinchart@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)
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)
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@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(-)