[libcamera-devel] [PATCH v4 3/9] libcamera: pipeline: raspberrypi: Add some debug logging

[Laurent Pinchart]

Hi Naush,

Thank you for the patch.

On Mon, Jul 20, 2020 at 10:13:05AM +0100, Naushir Patuck wrote:
> No functional changes, only added some more trace points.
> 
> Signed-off-by: Naushir Patuck <naush at raspberrypi.com>
> Reviewed-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> ---
>  src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> index d3639ce1..35f14dda 100644
> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> @@ -1160,6 +1160,11 @@ void RPiCameraData::ispInputDequeue(FrameBuffer *buffer)
>  	if (state_ == State::Stopped)
>  		return;
>  
> +	LOG(RPI, Debug) << "Stream ISP Input buffer complete"

Did you mean s/Stream ISP/ISP/ ?

> +			<< ", buffer id " << buffer->cookie()

Maybe s/id/cookie/ ?

> +			<< ", timestamp: " << buffer->metadata().timestamp;
> +
> +	/* The ISP input buffer gets re-queued into Unicam. */
>  	handleStreamBuffer(buffer, &unicam_[Unicam::Image]);
>  	handleState();
>  }
> @@ -1324,6 +1329,8 @@ void RPiCameraData::checkRequestCompleted()
>  		pipe_->completeRequest(camera_, request);
>  		requestQueue_.pop_front();
>  		requestCompleted = true;
> +
> +		LOG(RPI, Debug) << "Request is complete";

Would it make sense to add this to Request::complete() instead ? And
maybe print the request cookie ?

>  	}
>  
>  	/*
> @@ -1462,7 +1469,7 @@ FrameBuffer *RPiCameraData::updateQueue(std::queue<FrameBuffer *> &q, uint64_t t
>  		if (b->metadata().timestamp < timestamp) {
>  			q.pop();
>  			dev->queueBuffer(b);
> -			LOG(RPI, Error) << "Dropping input frame!";
> +			LOG(RPI, Warning) << "Dropping input frame!";
>  		} else if (b->metadata().timestamp == timestamp) {
>  			/* The calling function will pop the item from the queue. */
>  			return b;

-- 
Regards,

Laurent Pinchart