[libcamera-devel] [PATCH v3 4/4] pipeline: raspberrypi: Allow either strict or non-strict buffer matching

[David Plowman]

Another interesting case that we're looking at is a sensor with
on-board HDR. Because the sensor tonemaps the image before outputting
it, it means that the statistics we get from the ISP are no good for
the AGC/AEC algorithm - they're not linear with exposure/gain. Because
of this the sensor actually outputs some statistics of its own that we
can collect into the "embedded data" buffers in our usual way. From
there, the plan would be to pass them to our AGC/AEC.

But anyway, this is another case where "strict" matching is important.
(Also, there will likely be changes in our pipeline handler and IPAs
in the short-to-medium term to support this kind of use-case more
straightforwardly!).

David

On Mon, 22 Feb 2021 at 13:52, Naushir Patuck <naush at raspberrypi.com> wrote:
>
> Hi Laurent
>
>
> On Sun, 21 Feb 2021 at 23:09, Laurent Pinchart <laurent.pinchart at ideasonboard.com> wrote:
>>
>> Hi Naush,
>>
>> Thank you for the patch.
>>
>> On Thu, Feb 18, 2021 at 05:01:26PM +0000, Naushir Patuck wrote:
>> > A new flag, StrictBufferMatching, is used to control the behavior of
>> > the embedded and bayer buffer matching routine.
>> >
>> > If set to true (default), we reject and drop all bayer frames that do
>> > not have a matching embedded data buffer and vice-versa. This guarantees
>> > the IPA will always have the correct frame exposure and gain values to
>> > use.
>> >
>> > If set to false, we use bayer frames that do not have a matching
>> > embedded data buffer. In this case, IPA will use use the ControlList
>> > passed to it for gain and exposure values.
>> >
>> > Additionally, allow external stream buffers to behave as if
>> > StrictBufferMatching = false since we are not allowed to drop them.
>>
>> Could you explain the use case for both options ? Assuming an API for
>> applications to set this, how would an application decide ?
>
>
> As an example, consider a sensor that generates some kind of computational
> metadata (e.g. face/object detection) in the sensor silicon and sends it out
> via embedded data.  In such cases, the image data alone is not very helpful
> for the application.  An application would need both embedded data and image
> data buffers to do its magic, and crucially they must be in sync.  This is where
> the StrictBufferMatching = true would be used.  We are actually talking to a
> vendor about this exact use case. Of course, today we do not have the ability to
> get to that embedded data buffer in the application, but we will in the future, so
> right now this hint is more for the IPA having a guarantee that both buffers are in
> sync.
>
> For more simpler use cases, e.g. high framerate video, we do not necessarily care
> about strict matching, as the embedded buffer will only be used for AWB/AE loops,
> and avoiding frame drops is more desirable.  These cases would use StrictBufferMatching = false.
> Perhaps a better strategy would be to set this to the default...?
>
> Hope that makes sense.
>
> Naush
>
>
>>
>>
>> > Signed-off-by: Naushir Patuck <naush at raspberrypi.com>
>> > ---
>> >  .../pipeline/raspberrypi/raspberrypi.cpp      | 31 ++++++++++++++++---
>> >  1 file changed, 26 insertions(+), 5 deletions(-)
>> >
>> > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
>> > index d969c77993eb..7f66d6995176 100644
>> > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
>> > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
>> > @@ -46,6 +46,22 @@ namespace libcamera {
>> >
>> >  LOG_DEFINE_CATEGORY(RPI)
>> >
>> > +/*
>> > + * Set this to true to reject and drop all bayer frames that do not have a
>> > + * matching embedded data buffer and vice-versa. This guarantees the IPA will
>> > + * always have the correct frame exposure and gain values to use.
>> > + *
>> > + * Set this to false to use bayer frames that do not have a matching embedded
>> > + * data buffer. In this case, IPA will use use our local history for gain and
>> > + * exposure values, occasional frame drops may cause these number to be out of
>> > + * sync for a short period.
>> > + *
>> > + * \todo Ideally, this property should be set by the application, but we don't
>> > + * have any mechanism to pass generic properties into a pipeline handler at
>> > + * present.
>> > + */
>> > +static const bool StrictBufferMatching = true;
>> > +
>> >  namespace {
>> >
>> >  bool isRaw(PixelFormat &pixFmt)
>> > @@ -1724,13 +1740,13 @@ bool RPiCameraData::findMatchingBuffers(BayerFrame &bayerFrame, FrameBuffer *&em
>> >               embeddedBuffer = nullptr;
>> >               while (!embeddedQueue_.empty()) {
>> >                       FrameBuffer *b = embeddedQueue_.front();
>> > -                     if (!unicam_[Unicam::Embedded].isExternal() && b->metadata().timestamp < ts) {
>> > +                     if (b->metadata().timestamp < ts) {
>> >                               embeddedQueue_.pop();
>> >                               unicam_[Unicam::Embedded].queueBuffer(b);
>> >                               embeddedRequeueCount++;
>> >                               LOG(RPI, Warning) << "Dropping unmatched input frame in stream "
>> >                                                 << unicam_[Unicam::Embedded].name();
>> > -                     } else if (unicam_[Unicam::Embedded].isExternal() || b->metadata().timestamp == ts) {
>> > +                     } else if (b->metadata().timestamp == ts) {
>> >                               /* We pop the item from the queue lower down. */
>> >                               embeddedBuffer = b;
>> >                               break;
>> > @@ -1744,10 +1760,15 @@ bool RPiCameraData::findMatchingBuffers(BayerFrame &bayerFrame, FrameBuffer *&em
>> >
>> >                       LOG(RPI, Debug) << "Could not find matching embedded buffer";
>> >
>> > -                     if (!sensorMetadata_) {
>> > +                     if (unicam_[Unicam::Embedded].isExternal() ||
>> > +                         !StrictBufferMatching || !sensorMetadata_) {
>> >                               /*
>> > -                              * If there is no sensor metadata, simply return the
>> > -                              * first bayer frame in the queue.
>> > +                              * If any of the follwing is true:
>> > +                              * - This is an external stream buffer (so cannot be dropped).
>> > +                              * - We do not care about strict buffer matching.
>> > +                              * - There is no sensor metadata present.
>> > +                              * we can simply return the first bayer frame in the queue
>> > +                              * without a matching embedded buffer.
>> >                                */
>> >                               LOG(RPI, Debug) << "Returning bayer frame without a match";
>> >                               bayerQueue_.pop();
>>
>> --
>> Regards,
>>
>> Laurent Pinchart
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel