[libcamera-devel] [PATCH 1/3] libcamera: Add sequence value observer

[Umang Jain]

Hi Kieran,

Thank you for the patch

On 4/30/22 01:04, Kieran Bingham via libcamera-devel wrote:
> Components will often need to track and store a sequence number.
> Sequence numbers should be monotonically incrementing, but may require
> features such as validating that an external source matches this one, or
> ensuring that there are no gaps.
>
> Support the operations needed on sequence numbers in a dedicated class
> to reduce code duplication across components.
>
> Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> ---
>   include/libcamera/meson.build |  1 +
>   include/libcamera/sequence.h  | 20 +++++++++++
>   src/libcamera/meson.build     |  1 +
>   src/libcamera/sequence.cpp    | 66 +++++++++++++++++++++++++++++++++++
>   4 files changed, 88 insertions(+)
>   create mode 100644 include/libcamera/sequence.h
>   create mode 100644 src/libcamera/sequence.cpp
>
> diff --git a/include/libcamera/meson.build b/include/libcamera/meson.build
> index 408b7acf152c..2911d4bf4ec7 100644
> --- a/include/libcamera/meson.build
> +++ b/include/libcamera/meson.build
> @@ -14,6 +14,7 @@ libcamera_public_headers = files([
>       'logging.h',
>       'pixel_format.h',
>       'request.h',
> +    'sequence.h',
>       'stream.h',
>       'transform.h',
>   ])
> diff --git a/include/libcamera/sequence.h b/include/libcamera/sequence.h
> new file mode 100644
> index 000000000000..ad6e99726a4e
> --- /dev/null
> +++ b/include/libcamera/sequence.h
> @@ -0,0 +1,20 @@
> +#pragma once
> +
> +#include <optional>
> +
> +#include <libcamera/base/compiler.h>
> +
> +namespace libcamera {
> +
> +class Sequence
> +{
> +public:
> +	__nodiscard int update(unsigned int seq);
> +	void reset() { sequence_.reset(); }
> +
> +private:
> +	std::optional<unsigned int> sequence_;
> +};
> +
> +}; // namespace libcamera
> +
> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build
> index 26912ca17819..ffdffeb4bfdc 100644
> --- a/src/libcamera/meson.build
> +++ b/src/libcamera/meson.build
> @@ -38,6 +38,7 @@ libcamera_sources = files([
>       'process.cpp',
>       'pub_key.cpp',
>       'request.cpp',
> +    'sequence.cpp',
>       'source_paths.cpp',
>       'stream.cpp',
>       'sysfs.cpp',
> diff --git a/src/libcamera/sequence.cpp b/src/libcamera/sequence.cpp
> new file mode 100644
> index 000000000000..374b4f04d0a3
> --- /dev/null
> +++ b/src/libcamera/sequence.cpp
> @@ -0,0 +1,66 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2022, Ideas on Board Oy.
> + *
> + * sequence.cpp Sequence Number Observer
> + */
> +
> +#include <libcamera/base/log.h>
> +
> +#include <libcamera/sequence.h>
> +
> +/**
> + * \file sequence.h
> + * \brief Sequence number observer
> + */
> +
> +namespace libcamera {
> +
> +/**
> + * \class Sequence
> + * \brief Sequence number tracking which expects monotonically incrementing
> + *	  numbers


Indentation


Looks good to me,

Reviewed-by: Umang Jain <umang.jain at ideasonboard.com>

> + *
> + * The Sequence number observer is initialised with the first value it is given.
> + * It will return a difference of the expected update value, against the newly
> + * provided value - allowing the consumer to identify if a break in a sequence
> + * has occured.
> + */
> +
> +/**
> + * \brief Update the sequence observer with the latest value
> + * \param seq The latest value for the sequence
> + *
> + * This function will update the state of the Sequence observer and identify any
> + * non-monotonic increment or change that may occur and return the difference
> + * from the expected update value.
> + *
> + * The sequence is initialised to the first value passed into \a update.
> + *
> + * \return The number of drops in the sequence that were detected
> + */
> +__nodiscard int Sequence::update(unsigned int seq)
> +{
> +	if (!sequence_)
> +		sequence_ = seq - 1;
> +
> +	/*
> +	 * Any update expects a single integer difference from
> +	 * the previous value.
> +	 */
> +	int diff = seq - sequence_.value() - 1;
> +
> +	sequence_ = seq;
> +
> +	return diff;
> +};
> +
> +/**
> + * \fn Sequence::reset
> + * \brief Resets the sequence observer
> + *
> + * Re-initialises the sequence observer so that any known break in the monotonic
> + * sequence is not reported.
> + */
> +
> +} /* namespace libcamera */