[libcamera-devel,v2,9/9] android: camera_device: Report an error in notifyError()

8564 diff mbox series

If an error is generated, try to be verbose about it in the libcamera logs.

Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
---
 src/android/camera_device.cpp | 3 +++
 1 file changed, 3 insertions(+)

Hi Kieran,

Thanks for your work.

On 2020-07-02 22:36:54 +0100, Kieran Bingham wrote:
> If an error is generated, try to be verbose about it in the libcamera logs.
> 
> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>

> ---
>  src/android/camera_device.cpp | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> index 4681fd8af90b..5a3b4dfae6a0 100644
> --- a/src/android/camera_device.cpp
> +++ b/src/android/camera_device.cpp
> @@ -1192,6 +1192,9 @@ void CameraDevice::notifyError(uint32_t frameNumber, camera3_stream_t *stream)
>  {
>  	camera3_notify_msg_t notify = {};
>  
> +	LOG(HAL, Error) << "Error occured on frame " << frameNumber
> +			<< toPixelFormat(stream->format);
> +
>  	notify.type = CAMERA3_MSG_ERROR;
>  	notify.message.error.error_stream = stream;
>  	notify.message.error.frame_number = frameNumber;
> -- 
> 2.25.1
> 
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel

Hi Kieran,

Thank you for the patch.

On Thu, Jul 02, 2020 at 10:36:54PM +0100, Kieran Bingham wrote:
> If an error is generated, try to be verbose about it in the libcamera logs.
> 
> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> ---
>  src/android/camera_device.cpp | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> index 4681fd8af90b..5a3b4dfae6a0 100644
> --- a/src/android/camera_device.cpp
> +++ b/src/android/camera_device.cpp
> @@ -1192,6 +1192,9 @@ void CameraDevice::notifyError(uint32_t frameNumber, camera3_stream_t *stream)
>  {
>  	camera3_notify_msg_t notify = {};
>  
> +	LOG(HAL, Error) << "Error occured on frame " << frameNumber
> +			<< toPixelFormat(stream->format);

The frame number and the pixel format will be concatenated without a
space. Wouldn't it also make sense to print the stream number instead of
the format ? There could be multiple streams using the same format.

> +
>  	notify.type = CAMERA3_MSG_ERROR;
>  	notify.message.error.error_stream = stream;
>  	notify.message.error.frame_number = frameNumber;

Hi Laurent,

On 03/07/2020 01:38, Laurent Pinchart wrote:
> Hi Kieran,
> 
> Thank you for the patch.
> 
> On Thu, Jul 02, 2020 at 10:36:54PM +0100, Kieran Bingham wrote:
>> If an error is generated, try to be verbose about it in the libcamera logs.
>>
>> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
>> ---
>>  src/android/camera_device.cpp | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
>> index 4681fd8af90b..5a3b4dfae6a0 100644
>> --- a/src/android/camera_device.cpp
>> +++ b/src/android/camera_device.cpp
>> @@ -1192,6 +1192,9 @@ void CameraDevice::notifyError(uint32_t frameNumber, camera3_stream_t *stream)
>>  {
>>  	camera3_notify_msg_t notify = {};
>>  
>> +	LOG(HAL, Error) << "Error occured on frame " << frameNumber

s/occured/occurred/

>> +			<< toPixelFormat(stream->format);

Hrm ... this should be toPixelFormat(stream->format).toString() surely ;-)

> 
> The frame number and the pixel format will be concatenated without a
> space. Wouldn't it also make sense to print the stream number instead of
> the format ? There could be multiple streams using the same format.

Unfortunately I can't easily get the stream number currently.

I'd rather get 'something' in to report the errors (especially as these
'do' occur), and leave adding any further information to a \todo or for
whoever actually needs it.

I think it's better to report something here, rather than nothing so
this is at least a first step.

I'll add:

  \todo: Report and identify the stream number or configuration to
clarify the stream that failed.


>> +
>>  	notify.type = CAMERA3_MSG_ERROR;
>>  	notify.message.error.error_stream = stream;
>>  	notify.message.error.frame_number = frameNumber;
>