[libcamera-devel] [RFCv2 2/7] libcamera: pipeline: rkisp1: Queue parameters even if they are not ready

[Laurent Pinchart]

Hi Niklas,

Thank you for the patch.

On Thu, Mar 26, 2020 at 05:08:14PM +0100, Niklas Söderlund wrote:
> If the IPA have not filled in the parameters buffer still queue it to

s/have/has/

> hardware. Not queuing the buffer results in the pipeline and hardware
> going out of sync.
> 
> This is not a permanent fix of the problem and a todo is added to fix it
> properly. This change do not make the situation worse as the state of

s/do/does/

> the pipeline is just as unknown as if no param buffer is queued as if one
> with old content in it.
> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> ---
>  src/libcamera/pipeline/rkisp1/rkisp1.cpp | 18 ++++++++++++++----
>  1 file changed, 14 insertions(+), 4 deletions(-)
> 
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> index 2f909cef7c75ba0f..4ec74c5aa8286ffb 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> @@ -351,13 +351,23 @@ protected:
>  		if (!info)
>  			LOG(RkISP1, Fatal) << "Frame not known";
>  
> -		if (info->paramFilled)
> -			pipe_->param_->queueBuffer(info->paramBuffer);
> -		else
> +		/*
> +		 * \todo: If parameters are not filled a better method to handle
> +		 * the situation then queing a buffer with unkown content should

s/then/than/
s/unkown/unknown/

> +		 * be used.
> +		 *
> +		 * It seems obsessive to keep an internal zeroed scratch

obsessive ? Did you mean excessive ?

> +		 * parameters buffer around as this should not happen uless the

s/uless/unless/

> +		 * devices is under too much load. Perhaps failing the request
> +		 * and returning it to the applicaiton with an error code is better

s/applicaiton/application/

> +		 * then to queue it to hardware?

s/then to queue/than queuing/

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

> +		 */
> +		if (!info->paramFilled)
>  			LOG(RkISP1, Error)
>  				<< "Parameters not ready on time for frame "
> -				<< frame() << ", ignore parameters.";
> +				<< frame();
>  
> +		pipe_->param_->queueBuffer(info->paramBuffer);
>  		pipe_->stat_->queueBuffer(info->statBuffer);
>  		pipe_->video_->queueBuffer(info->videoBuffer);
>  	}

-- 
Regards,

Laurent Pinchart