[libcamera-devel] [PATCH 5/5] ipa: ipu3: agc: Saturate the averages when computing relative luminance

[Kieran Bingham]

Quoting Kieran Bingham (2021-11-18 09:53:22)
> Quoting Jean-Michel Hautbois (2021-11-17 10:26:59)
> > Hi Laurent,
> > 
> > Thanks for the patch !
> > 
> > On 16/11/2021 17:26, Laurent Pinchart wrote:
> > > The relative luminance is calculated using an iterative process to
> > > account for saturation in the sensor, as multiplying pixels by a gain
> > > doesn't increase the relative luminance by the same factor if some
> > > regions are saturated. Relative luminance estimation doesn't apply a
> > > saturation, which produces a value that doesn't match what the sensor
> > > will output, and defeats the point of the iterative process. Fix it.
> > > 
> > > Fixes: f8f07f9468c6 ("ipa: ipu3: agc: Improve gain calculation")
> > > Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > > ---
> > >   src/ipa/ipu3/algorithms/agc.cpp | 25 ++++++++++++++++++-------
> > >   1 file changed, 18 insertions(+), 7 deletions(-)
> > > 
> > > diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp
> > > index 71398fdd96a6..46aa1b14a100 100644
> > > --- a/src/ipa/ipu3/algorithms/agc.cpp
> > > +++ b/src/ipa/ipu3/algorithms/agc.cpp
> > > @@ -252,10 +252,19 @@ void Agc::computeExposure(IPAFrameContext &frameContext, double yGain,
> > >    * \param[in] gain The analogue gain to apply to the frame
> > >    * \return The relative luminance
> > >    *
> > > - * Luma is the weighted sum of gamma-compressed R′G′B′ components of a color
> > > - * video. The luma values are normalized as 0.0 to 1.0, with 1.0 being a
> > > - * theoretical perfect reflector of 100% reference white. We use the Rec. 601
> > > - * luma here.
> > > + * This function estimates the average relative luminance of the frame that
> > > + * would be output by the sensor if an additional analogue \a gain was applied.
> > 
> > s/additional analogue \a gain/additional \a gain/
> > 
> > > + *
> > > + * The estimation is based on the AWB statistics for the current frame. Red,
> > > + * green and blue averages for all cells are first multiplied by the gain, and
> > > + * then saturated to approximate the sensor behaviour at high brightness
> > > + * values. The approximation is quitte rough, as it doesn't take into account
> 
> /quitte/quite/
> 
> > > + * non-linearities when approaching saturation.
> > > + *
> > > + * The relative luminance (Y) is computed from the linear RGB components using
> > > + * the Rec. 601 formula. The values is normalized to the [0.0, 1.0] range,
> 
> /values is/values are/
> 
> > > + * where 1.0 corresponds to a theoretical perfect reflector of 100% reference
> > > + * white.
> > >    *
> > >    * More detailed information can be found in:
> > >    * https://en.wikipedia.org/wiki/Relative_luminance
> > > @@ -267,6 +276,7 @@ double Agc::estimateLuminance(IPAFrameContext &frameContext,
> > >   {
> > >       double redSum = 0, greenSum = 0, blueSum = 0;
> > >   
> > > +     /* Sum the per-channel averages, saturated to 255. */
> > >       for (unsigned int cellY = 0; cellY < grid.height; cellY++) {
> > >               for (unsigned int cellX = 0; cellX < grid.width; cellX++) {
> > >                       uint32_t cellPosition = cellY * stride_ + cellX;
> > > @@ -275,10 +285,11 @@ double Agc::estimateLuminance(IPAFrameContext &frameContext,
> > >                               reinterpret_cast<const ipu3_uapi_awb_set_item *>(
> > >                                       &stats->awb_raw_buffer.meta_data[cellPosition]
> > >                               );
> > > +                     const uint8_t G_avg = (cell->Gr_avg + cell->Gb_avg) / 2;
> > >   
> > > -                     redSum += cell->R_avg * gain;
> > > -                     greenSum += (cell->Gr_avg + cell->Gb_avg) / 2 * gain;
> > > -                     blueSum += cell->B_avg * gain;
> > > +                     redSum += std::min(cell->R_avg * gain, 255.0);
> > > +                     greenSum += std::min(G_avg * gain, 255.0);
> > > +                     blueSum += std::min(cell->B_avg * gain, 255.0);

Seeing this constant in the new RKISP series again, wouldn't it make
sense to keep the kMaximumLuminance constant that you remove at the
beginning of the series and use it here so that it's self-documented
that the min operation is clamping to the maximum luminance value?

Or is 255 really not a luminance value here?


> > >               }
> > >       }
> > >   
> > > 
> > 
> 
> Still works well in my harsh backlight office conditions too.
> 
> Tested-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> 
> > Tested-by: Jean-Michel Hautbois <jeanmichel.hautbois at ideasonboard.com>
> > Reviewed-by: Jean-Michel Hautbois <jeanmichel.hautbois at ideasonboard.com>