[libcamera-devel] apply methods in IPARPi

[Naushir Patuck]

Hi Sebastian,

On Wed, 5 May 2021 at 19:08, Sebastian Fricke <sebastian.fricke at posteo.net>
wrote:

> Hello Naushir and David,
>
> I am currently performing a lot of investigation and preparatory work
> for a few generalizations in form of an IPAFramework class, that reuses
> some of the great concepts you introduced to libcamera. Along the way I
> noticed, that you have implemented the apply methods (applyAGC, applyAWB,
> etc.) as part of the IPARPi class.
>
> I was wondering if it would be possible to move those to the controller
> and specific algorithm implementation, as they mostly get the status from
> the algorithm, perform a few calculations with it and insert the results
> of those calculations into the ISP controls.
>
> The upside of doing so, from my point of view, would be that we could
> call `apply` similar to the way we call `initialize`, `process` and
> `prepare`.
> That way, I would be able to generalize the whole step of applying the
> algorithm status to a set of controls, which can then be used for setting
> the ISP controls.
> Now, the reason why I ask you is that I want to be sure, that I didn't
> miss a key concept.
>

This was indeed something I did consider doing some time back, but never
got to it.  There are a couple of considerations with this proposal that we
need to address:

1) My original intention with keeping apply* out of the Controllers was to
*somewhat* decouple the BCM2835 ISP parameters from the Controller
algorithms.  I say somewhat because some algorithms do have to know the
statistics structure defined in the bcm2835-isp.h header, so there is
already a hardware dependency there.  Secondly, I wanted to try and not
include any libcamera dependency in the Controller code.  We do now have
libcamera logging in the controller source files, so again, this is also
not strictly true.

2) applyAGC() is a special one in that it runs after Controller::Process()
whereas all the others run after Controller::Prepare.  applyAGC() also runs
on IPA::start() so this must be treated differently somehow.

In a way, having a Controller::Apply() might be quite nice, if we are able
to somehow add a HW abstraction layer (through templated
functions perhaps?).  Or does it even matter? David, what are your thoughts
on this?


>
> --
>
> Additionally, off-topic to the topic mentioned above. While
> investigating the RPiStream abstraction, I noticed the following comment
> within: `rpi_stream.cpp`:116
> ```
>   * A nullptr buffer implies an external stream, but no external
>   * buffer has been supplied in the Request. So, pick one from the
>   * availableBuffers_ queue.
>   ```
>
>   And within `rpi_steam.h`:139
>   ```
>   * A nullptr indicates any internal buffer can be used (from
> availableBuffers_),
>   * whereas a valid pointer indicates an external buffer to be queued.
>   ```
>
>   Which confused me a little bit, could you enlighten me in this matter?
>   Does a nullptr buffer imply an external stream and therefore an
>   external buffer? Or does a nullptr buffer imply an internal buffer and
>   therefore an internal stream?
>

I can see why this caused some confusion :-)

Firstly, my definitions of external and internal streams may need
clarification.  The former is a stream that *may* be exported to the
application, and the latter is a stream that can never be exported to the
application.  For external streams, the buffers used may be provided by the
application, and these buffers will be returned back to the application
once they have been processed.  However, we can also use internally
allocated buffers for external streams that will never go back to the
application.  This is needed when:

1) The application has not provided an external buffer to use for that
stream in a Request.  Example of this is when we have a RAW stream set to
external, but that application will only request a RAW image once every N
frames.
2) If we need to drop frames as instructed by the IPA, we cannot use
external buffers as they will eventually be dropped and not returned to the
application.  In this case, we queue an internal buffer that can be dropped
safely.  The alternative is to requeue external buffers once dropped, but
will add a ton of latency to the capture cycle.

Hope that helps clarify things a bit.

Regards,
Naush



>
>   Greetings,
>   Sebastian
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20210506/7624e7ec/attachment.htm>