[libcamera-devel] [PATCH 1/5] src: ipa: raspberrypi: Distinguish the first camera start from others

[Laurent Pinchart]

Hi David,

Thank you for the patch.

On Wed, Dec 02, 2020 at 11:52:49AM +0000, David Plowman wrote:
> This makes it possible to tell whether we're starting the sensor for
> the first time, or whether it's happening because of a mode switch or
> because the camera has been paused and re-started. Depending on this,
> some sensors may require us to drop different numbers of frames.
> 
> Signed-off-by: David Plowman <david.plowman at raspberrypi.com>

This looks good to me, but I'd squash it with patch 2/5 as the chance is
very small and it's easier to review it when also seeing how the new
variable is used.

Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

On a side note, how does mode change and initial start differ ? Don't we
stop the sensor when reconfiguring the camera ?

> ---
>  src/ipa/raspberrypi/raspberrypi.cpp | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp
> index 69be5e4e..b8298768 100644
> --- a/src/ipa/raspberrypi/raspberrypi.cpp
> +++ b/src/ipa/raspberrypi/raspberrypi.cpp
> @@ -67,7 +67,7 @@ public:
>  	IPARPi()
>  		: lastMode_({}), controller_(), controllerInit_(false),
>  		  frameCount_(0), checkCount_(0), mistrustCount_(0),
> -		  lsTable_(nullptr)
> +		  lsTable_(nullptr), firstStart_(true)
>  	{
>  	}
>  
> @@ -145,6 +145,9 @@ private:
>  	/* LS table allocation passed in from the pipeline handler. */
>  	FileDescriptor lsTableHandle_;
>  	void *lsTable_;
> +
> +	/* Distinguish the first camera start from others. */
> +	bool firstStart_;
>  };
>  
>  int IPARPi::init(const IPASettings &settings)
> @@ -179,6 +182,8 @@ int IPARPi::start(const IPAOperationData &ipaConfig, IPAOperationData *result)
>  		result->operation |= RPi::IPA_CONFIG_SENSOR;
>  	}
>  
> +	firstStart_ = false;
> +
>  	return 0;
>  }
>  

-- 
Regards,

Laurent Pinchart