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

11465 diff mbox series

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.

Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
Tested-by: David Plowman <david.plowman@raspberrypi.com>
---
 .../pipeline/raspberrypi/raspberrypi.cpp      | 31 ++++++++++++++++---
 1 file changed, 26 insertions(+), 5 deletions(-)

Hi Naush,

Thank you for the patch.

On Tue, Mar 02, 2021 at 03:11:11PM +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.
> 
> Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
> Tested-by: David Plowman <david.plowman@raspberrypi.com>

I've skipped this one for now, as there's ongoing discussion in v3. I'll
push the first three patches once my compile tests complete.

> ---
>  .../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 5ae2551957f8..549d6147cd60 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)
> @@ -1725,13 +1741,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;
> @@ -1745,10 +1761,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 following 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";
>  				bayerFrame = std::move(bayerQueue_.front());