[libcamera-devel] [PATCH v4 04/32] ipa: libipa: Introduce FrameContextQueue

Kieran Bingham kieran.bingham at ideasonboard.com
Tue Sep 20 15:38:05 CEST 2022


Quoting Laurent Pinchart via libcamera-devel (2022-09-08 02:41:32)
> From: Umang Jain <umang.jain at ideasonboard.com>
> 
> Introduce a common implementation in libipa to represent the queue of
> frame contexts.
> 
> Signed-off-by: Umang Jain <umang.jain at ideasonboard.com>
> Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> ---
> Changes since v3:
> 
> - Split the IPU3 changes to a separate patch
> - Use composition instead of inheritance
> - Use vector instead of array for backend storage
> - Make the queue size dynamic
> - Rename initialise() to init()
> ---
>  src/ipa/libipa/fc_queue.cpp | 117 ++++++++++++++++++++++++++++++++++++
>  src/ipa/libipa/fc_queue.h   | 108 +++++++++++++++++++++++++++++++++
>  src/ipa/libipa/meson.build  |   2 +
>  3 files changed, 227 insertions(+)
>  create mode 100644 src/ipa/libipa/fc_queue.cpp
>  create mode 100644 src/ipa/libipa/fc_queue.h
> 
> diff --git a/src/ipa/libipa/fc_queue.cpp b/src/ipa/libipa/fc_queue.cpp
> new file mode 100644
> index 000000000000..b81d497e255a
> --- /dev/null
> +++ b/src/ipa/libipa/fc_queue.cpp
> @@ -0,0 +1,117 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2022, Google Inc.
> + *
> + * fc_queue.cpp - IPA Frame context queue
> + */
> +
> +#include "fc_queue.h"
> +
> +#include <libcamera/base/log.h>
> +
> +namespace libcamera {
> +
> +LOG_DEFINE_CATEGORY(FCQueue)
> +
> +namespace ipa {
> +
> +/**
> + * \file fc_queue.h
> + * \brief Queue to access per-frame Context
> + */
> +
> +/**
> + * \class FCQueue
> + * \brief A support class for queueing FrameContext instances through the IPA
> + * \tparam FrameContext The IPA specific FrameContext derived class type

; or , after FrameContext?

> + *
> + * The frame context queue provides a simple circular buffer implementation to
> + * store IPA specific context for each frame through its lifetime within the
> + * IPA.
> + *
> + * FrameContext instances are expected to be referenced by a monotonically
> + * increasing sequence count corresponding to a frame sequence number.
> + *
> + * A FrameContext is initialised for a given frame when the corresponding
> + * Request is first queued into the IPA. From that point on the FrameContext can
> + * be obtained by the IPA and its algorithms by referencing it from the frame
> + * sequence number.
> + *
> + * A frame sequence number from the image source must correspond to the request
> + * sequence number for this implementation to be supported in an IPA. It is
> + * expected that the same sequence number will be used to reference the context
> + * of the frame from the point of queueing the request, specifying controls for
> + * a given frame, and processing of any ISP related controls and statistics for
> + * the same corresponding image.
> + *
> + * IPA specific frame context implementations shall inherit from the
> + * IPAFrameContext base class to support the minimum required features for a
> + * FrameContext.
> + *
> + * FrameContexts are overwritten when they are recycled and re-initialised by
> + * the first access made on them by either init(frame) or get(frame). It is
> + * required that the number of available slots in the frame context queue is
> + * larger or equal to the maximum number of in-flight requests a pipeline can
> + * handle to avoid overwriting frame context instances that still have to be
> + * processed.
> + *
> + * In the case an application does not queue requests to the camera fast
> + * enough, frames can be produced and processed by the IPA without a
> + * corresponding Request being queued. In this case the IPA algorithm
> + * will try to access the FrameContext with a call to get() before it
> + * had been initialized at queueRequest() time. In this case, there
> + * is no way the controls associated with the late Request could be
> + * applied in time, as the frame as already been processed by the IPA.
> + *
> + * \todo Set an error flag for per-frame control errors.
> + */
> +
> +/**
> + * \fn FCQueue::FCQueue(unsigned int size)
> + * \brief Construct a frame context queue of a specified size
> + * \param[in] size The number of contexts in the queue
> + */
> +
> +/**
> + * \fn FCQueue::clear()
> + * \brief Clear the context queue
> + *
> + * Reset the queue and ensure all FrameContext slots are re-initialised.
> + * IPA modules are expected to clear the frame context queue at the beginning of
> + * a new streaming session, in IPAModule::start().
> + */
> +
> +/**
> + * \fn FCQueue::init(uint32_t frame)

I saw you've questioned the name here.

I don't mind if this is init(), alloc() new() ?

We don't (yet?) have an explicit free/delete/release of the FCQueue, as
they stay in memory until something overwrites them like an LRU....


> + * \brief Initialize and return the Frame Context at slot specified by \a frame
> + * \param[in] frame The frame context sequence number
> + *
> + * The first call to obtain a FrameContext from the FCQueue should be handled
> + * through this call. The FrameContext will be initialised for the frame and
> + * returned to the caller if it was not already initialised.
> + *
> + * If the FrameContext was already initialized for this sequence, a warning will
> + * be reported and the previously initialized FrameContext is returned.
> + *
> + * Frame contexts are expected to be initialised when a Request is first passed
> + * to the IPA module in IPAModule::queueRequest().
> + *
> + * \return A reference to the FrameContext for sequence \a frame
> + */
> +
> +/**
> + * \fn FCQueue::get()
> + * \brief Obtain the Frame Context at slot specified by \a frame
> + * \param[in] frame The frame context sequence number
> + *
> + * Obtains an existing FrameContext from the queue and returns it to the caller.
> + *
> + * If the FrameContext is not correctly initialised for the \a frame, it will be
> + * initialised.
> + *
> + * \return A reference to the FrameContext for sequence \a frame
> + */
> +
> +} /* namespace ipa */
> +
> +} /* namespace libcamera */
> diff --git a/src/ipa/libipa/fc_queue.h b/src/ipa/libipa/fc_queue.h
> new file mode 100644
> index 000000000000..0f3af0f3a189
> --- /dev/null
> +++ b/src/ipa/libipa/fc_queue.h
> @@ -0,0 +1,108 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2022, Google Inc.
> + *
> + * fc_queue.h - IPA Frame context queue
> + */
> +
> +#pragma once
> +
> +#include <algorithm>
> +#include <vector>
> +
> +#include <libcamera/base/log.h>
> +
> +namespace libcamera {
> +
> +LOG_DECLARE_CATEGORY(FCQueue)
> +
> +namespace ipa {
> +
> +template<typename FrameContext>
> +class FCQueue
> +{
> +public:
> +       FCQueue(unsigned int size)
> +               : contexts_(size)
> +       {
> +       }
> +
> +       void clear()
> +       {
> +               std::fill(contexts_.begin(), contexts_.end(), FrameContext{});
> +       }
> +
> +       FrameContext &init(const uint32_t frame)
> +       {
> +               FrameContext &frameContext = contexts_[frame % contexts_.size()];
> +
> +               /*
> +                * Do not re-initialise if a get() call has already fetched this
> +                * frame context to preseve the context.
> +                *
> +                * \todo If the the sequence number of the context to initialise
> +                * is smaller than the sequence number of the queue slot to use,
> +                * it means that we had a serious request underrun and more
> +                * frames than the queue size has been produced since the last
> +                * time the application has queued a request. Does this deserve
> +                * an error condition ?
> +                */
> +               if (frame != 0 && frame <= frameContext.frame)
> +                       LOG(FCQueue, Warning)
> +                               << "Frame " << frame << " already initialised";
> +               else
> +                       init(frameContext, frame);
> +
> +               return frameContext;
> +       }
> +
> +       FrameContext &get(uint32_t frame)
> +       {
> +               FrameContext &frameContext = contexts_[frame % contexts_.size()];
> +
> +               /*
> +                * If the IPA algorithms try to access a frame context slot which
> +                * has been already overwritten by a newer context, it means the
> +                * frame context queue has overflowed and the desired context
> +                * has been forever lost. The pipeline handler shall avoid
> +                * queueing more requests to the IPA than the frame context
> +                * queue size.
> +                */
> +               if (frame < frameContext.frame)
> +                       LOG(FCQueue, Fatal) << "Frame Context for " << frame
> +                                           << " has been overwritten by "
> +                                           << frameContext.frame;
> +
> +               if (frame == frameContext.frame)
> +                       return frameContext;
> +
> +               /*
> +                * The frame context has been retrieved before it was
> +                * initialised through the initialise() call. This indicates an
> +                * algorithm attempted to access a Frame context before it was
> +                * queued to the IPA.  Controls applied for this request may be

double space between 'IPA.  Controls'

> +                * left unhandled.
> +                *
> +                * \todo Set an error flag for per-frame control errors.
> +                */
> +               LOG(FCQueue, Warning)
> +                       << "Obtained an uninitialised FrameContext for " << frame;
> +
> +               init(frameContext, frame);
> +
> +               return frameContext;
> +       }
> +
> +private:
> +       void init(FrameContext &frameContext, const uint32_t frame)
> +       {
> +               frameContext = {};
> +               frameContext.frame = frame;

Aha this is better.


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

> +       }
> +
> +       std::vector<FrameContext> contexts_;
> +};
> +
> +} /* namespace ipa */
> +
> +} /* namespace libcamera */
> diff --git a/src/ipa/libipa/meson.build b/src/ipa/libipa/meson.build
> index fb894bc614af..016b8e0ec9be 100644
> --- a/src/ipa/libipa/meson.build
> +++ b/src/ipa/libipa/meson.build
> @@ -3,6 +3,7 @@
>  libipa_headers = files([
>      'algorithm.h',
>      'camera_sensor_helper.h',
> +    'fc_queue.h',
>      'histogram.h',
>      'module.h',
>  ])
> @@ -10,6 +11,7 @@ libipa_headers = files([
>  libipa_sources = files([
>      'algorithm.cpp',
>      'camera_sensor_helper.cpp',
> +    'fc_queue.cpp',
>      'histogram.cpp',
>      'module.cpp',
>  ])
> -- 
> Regards,
> 
> Laurent Pinchart
>


More information about the libcamera-devel mailing list