[libcamera-devel] [PATCH v3 4/4] pipeline: raspberrypi: Allow either strict or non-strict buffer matching
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Thu Mar 4 03:12:17 CET 2021
Hello,
On Thu, Feb 25, 2021 at 11:17:37AM +0000, David Plowman wrote:
> 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!).
>
> On Mon, 22 Feb 2021 at 13:52, Naushir Patuck <naush at raspberrypi.com> wrote:
> > On Sun, 21 Feb 2021 at 23:09, Laurent Pinchart <laurent.pinchart at ideasonboard.com> wrote:
> >> 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.
So I understand why strict matching is important, and that's the default
in this patch. Non-strict matching relaxes the requirements, but I'm not
sure what impact it will have on algorithms.
Furthermore, to make this really useful, we would need a way to
configure the policy at runtime, and I think it may be a hard choice for
applications. I wonder if there could be a way to handle the buffer
matching policy automagically ? The code hear deals with the case were
no matching buffer is available, which I assume would be caused by a
frame drop (either on the image stream or the embedded data stream).
Would it be possible to detect such situations and handle them in an
automatic way ? I'm not sure what to propose exactly, but going in the
direction of exposing to applications controls that tune heuristics in
ill-defined ways worries me. We could easily end up with controls that
applications would have no meaningful way to set. If you ask me if I
need to avoid frame drops or avoid bad images, I'll tell you I need both
:-) I could possibly make a decision if I knew the exact risk of frame
drop, and exactly how "bad" images could be, but I don't think that's
something we'll be able to express in an API. I'd really like to avoid
going in this direction.
> >> > 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
More information about the libcamera-devel
mailing list