[libcamera-devel] [PATCH v4 24/32] ipa: rkisp1: Document the active state and frame context

[Kieran Bingham]

Quoting Laurent Pinchart via libcamera-devel (2022-09-08 02:41:52)
> Now that data used by algorithms has been partitioned between the active
> state and frame context, we have a better view of the role of each of
> those structures. Document them appropriately.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> ---
>  src/ipa/rkisp1/ipa_context.cpp | 55 ++++++++++++++++++++++++++++------
>  1 file changed, 46 insertions(+), 9 deletions(-)
> 
> diff --git a/src/ipa/rkisp1/ipa_context.cpp b/src/ipa/rkisp1/ipa_context.cpp
> index b2628ef73d49..335cb32c538d 100644
> --- a/src/ipa/rkisp1/ipa_context.cpp
> +++ b/src/ipa/rkisp1/ipa_context.cpp
> @@ -88,16 +88,28 @@ namespace libcamera::ipa::rkisp1 {
>   * \struct IPAActiveState
>   * \brief Active state for algorithms
>   *
> - * The active state stores algorithm-specific data that needs to be shared
> - * between multiple algorithms and the IPA module. It is accessible through the
> - * IPAContext structure.
> + * The active state contains all algorithm-specific data that need to be

/need/needs/

> + * maintained by algorithms across frames. Unlike the session configuration,
> + * the active state is mutable and constantly updated by algorithms. The active
> + * state is accessible through the IPAContext structure.
>   *
> - * \todo Split the data contained in this structure between the active state
> - * and the frame contexts.
> + * The active state stores two distinct categories of information:
>   *
> - * Each of the fields in the active state belongs to either a specific
> - * algorithm, or to the top-level IPA module. A field may be read by any
> - * algorithm, but should only be written by its owner.
> + *  - The consolidated value of all algorithm controls. Requests passed to
> + *    the queueRequest() function store values for controls that the
> + *    application wants to modify for that particular frame, and the
> + *    queueRequest() function updates the active state with those values.
> + *    The active state thus contains a consolidated view of the value of all
> + *    controls handled by the algorithm.
> + *
> + *  - The value of parameters computed by the algorithm when running in auto
> + *    mode. Algorithms running in auto mode compute new parameters every
> + *    time statistics buffers are received (either synchronously, or
> + *    possibly in a background thread). The latest computed value of those
> + *    parameters is stored in the active state in the process() function.
> + *
> + * Each of the members in the active state belongs to a specific algorithm. A
> + * member may be read by any algorithm, but shall only be written by its owner.
>   */
>  
>  /**
> @@ -185,7 +197,32 @@ namespace libcamera::ipa::rkisp1 {
>   * \struct RkISP1FrameContext
>   * \brief Per-frame context for algorithms
>   *
> - * \todo Populate the frame context for all algorithms
> + * The frame context stores two distinct categories of information:
> + *
> + * - The value of the controls to be applied to the frame. These values are
> + *   typically set in the queueRequest() function, from the consolidated
> + *   control values stored in the active state. The frame context thus stores
> + *   values for all controls related to the algorithm, not limited to the
> + *   controls specified in the corresponding request, but consolidated from all
> + *   requests that have been queued so far.
> + *
> + *   For controls that can be set manually or computed by an algorithm
> + *   (depending on the algorithm operation mode), such as for instance the
> + *   colour gains for the AWB algorithm, the control value will be stored in
> + *   the frame context in the queueRequest() function only when operating in
> + *   manual mode. When operating in auto mode, the values are computed by the
> + *   algorithm in process(), stored in the active state, and copied to the
> + *   frame context in prepare(), just before being stored in the ISP parameters
> + *   buffer.
> + *
> + *   The queueRequest() function can also store ancillary data in the frame
> + *   context, such as flags to indicate if (and what) control values have
> + *   changed compared to the previous request.
> + *
> + * - Status information computed by the algorithm for a frame. For instance,
> + *   the colour temperature estimated by the AWB algorithm from ISP statistics
> + *   calculated on a frame is stored in the frame context for that frame in
> + *   the process() function.

That all sounds fine. I wonder if some of the examples might need
tweaking later, as I think for instance the colour temperature for a
frame might go directlty into the metadata, as mentioned earlier. The
FrameContext is really only storing information about a frame that is
required at multiple processing steps/calls. And the ColourTemperature
is likely not needed (for a specific frame) after it's calculated,
except to put it into the metadata for that frame.


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

>   */
>  
>  /**
> -- 
> Regards,
> 
> Laurent Pinchart
>