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