[libcamera-devel] [RFC PATCH] libcamera: Request: Add setHalfLife

[Kieran Bingham]

Hi Paul,

On 01/04/2021 04:46, Paul Elder wrote:
> From the perspective of a Buffer, a Request can suddenly become nullptr
> in the middle of a stream. As Requests are supposed to be atomic, and
> they are the nucleus of the libcamera camera streaming API, it would be
> convenient if we could set the rate of decay of Requests.
> 
> Add a half life member variable to Request, and a setHalfLife() member
> function to set this. This will affect whether the Request is reusable
> or not, so Request::reuse() can now fail.
> 
> Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
> 
> ---
> TODO: change the applications to reuse the request if reuse() fails
> ---
>  include/libcamera/request.h |  8 +++++++-
>  src/libcamera/request.cpp   | 36 ++++++++++++++++++++++++++++++++++--
>  2 files changed, 41 insertions(+), 3 deletions(-)
> 
> diff --git a/include/libcamera/request.h b/include/libcamera/request.h
> index 4cf5ff3f..45c707f3 100644
> --- a/include/libcamera/request.h
> +++ b/include/libcamera/request.h
> @@ -7,6 +7,7 @@
>  #ifndef __LIBCAMERA_REQUEST_H__
>  #define __LIBCAMERA_REQUEST_H__
>  
> +#include <chrono>
>  #include <map>
>  #include <memory>
>  #include <stdint.h>
> @@ -43,7 +44,7 @@ public:
>  	Request(Camera *camera, uint64_t cookie = 0);
>  	~Request();
>  
> -	void reuse(ReuseFlag flags = Default);
> +	bool reuse(ReuseFlag flags = Default);
>  
>  	ControlList &controls() { return *controls_; }
>  	ControlList &metadata() { return *metadata_; }
> @@ -57,6 +58,8 @@ public:
>  
>  	bool hasPendingBuffers() const { return !pending_.empty(); }
>  
> +	void setHalfLife(std::chrono::duration<double> hl);
> +
>  	std::string toString() const;
>  
>  private:
> @@ -79,6 +82,9 @@ private:
>  	const uint64_t cookie_;
>  	Status status_;
>  	bool cancelled_;
> +
> +	std::chrono::steady_clock::time_point birthTime_;
> +	std::chrono::duration<double> halfLife_;
>  };
>  
>  } /* namespace libcamera */
> diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp
> index ce2dd7b1..0543abaf 100644
> --- a/src/libcamera/request.cpp
> +++ b/src/libcamera/request.cpp
> @@ -7,7 +7,9 @@
>  
>  #include <libcamera/request.h>
>  
> +#include <chrono>
>  #include <map>
> +#include <random>
>  #include <sstream>
>  
>  #include <libcamera/buffer.h>
> @@ -74,7 +76,9 @@ LOG_DEFINE_CATEGORY(Request)
>   */
>  Request::Request(Camera *camera, uint64_t cookie)
>  	: camera_(camera), sequence_(0), cookie_(cookie),
> -	  status_(RequestPending), cancelled_(false)
> +	  status_(RequestPending), cancelled_(false),
> +	  birthTime_(std::chrono::steady_clock::now()),
> +	  halfLife_(std::chrono::seconds(10))

I think we should reduce this to just one second in four. or four in
one, it depends on your location I think.

Other than that, looks like a great debug addition, lets do it.

Completely-Entirely-Reviewed-by: Kieran Bingham
<kieran.bingham at ideasonboard.com>


>  {
>  	/**
>  	 * \todo Should the Camera expose a validator instance, to avoid
> @@ -111,11 +115,28 @@ Request::~Request()
>   * prior to queueing the request to the camera, in lieu of constructing a new
>   * request. The application can reuse the buffers that were previously added
>   * to the request via addBuffer() by setting \a flags to ReuseBuffers.
> + *
> + * The request for reuse may fail, as Requests can decay based on the half
> + * life.
> + *
> + * \return True on success, false otherwise
>   */
> -void Request::reuse(ReuseFlag flags)
> +bool Request::reuse(ReuseFlag flags)
>  {
>  	LIBCAMERA_TRACEPOINT(request_reuse, this);
>  
> +	std::chrono::steady_clock::time_point end =
> +		std::chrono::steady_clock::now();
> +	std::chrono::duration<double> diff = end - birthTime_;
> +
> +	std::random_device rd{};
> +	std::mt19937 gen{ rd() };
> +	double hlns =
> +		std::chrono::duration_cast<std::chrono::nanoseconds>(halfLife_).count();
> +	std::normal_distribution<> dist{ hlns, hlns / 2 };
> +	if (dist(gen) > hlns)
> +		return false;
> +
>  	pending_.clear();
>  	if (flags & ReuseBuffers) {
>  		for (auto pair : bufferMap_) {
> @@ -133,6 +154,8 @@ void Request::reuse(ReuseFlag flags)
>  
>  	controls_->clear();
>  	metadata_->clear();
> +
> +	return true;
>  }
>  
>  /**
> @@ -320,6 +343,15 @@ bool Request::completeBuffer(FrameBuffer *buffer)
>  	return !hasPendingBuffers();
>  }
>  
> +/**
> + * \brief Set the half life of the request
> + * \param[in] hl Half-life of the request
> + */
> +void Request::setHalfLife(std::chrono::duration<double> hl)
> +{
> +	halfLife_ = hl;
> +}
> +
>  /**
>   * \brief Generate a string representation of the Request internals
>   *
> 

-- 
Regards
--
Kieran