[libcamera-devel] [PATCH v3 10/13] android: camera_device: Report configuration changes from validate()

[Laurent Pinchart]

Hi Kieran,

Thank you for the patch.

On Tue, Aug 04, 2020 at 10:47:08PM +0100, Kieran Bingham wrote:
> When we call validate on a configuration, if there are any adjustments
> on the configuration, we fail without showing why.
> 
> Display the stream configuration after the validate stage to aid
> debugging stream startup failures.
> 
> Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>
> Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> ---
>  src/android/camera_device.cpp | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> index 5899154b3e78..4178db952846 100644
> --- a/src/android/camera_device.cpp
> +++ b/src/android/camera_device.cpp
> @@ -1047,7 +1047,11 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)
>  	case CameraConfiguration::Valid:
>  		break;
>  	case CameraConfiguration::Adjusted:
> -		LOG(HAL, Info) << "Camera configuration adjusted";
> +		LOG(HAL, Info) << "Camera configuration adjusted:";
> +
> +		for (const StreamConfiguration &cfg : *config_)
> +			LOG(HAL, Info) << " : " << cfg.toString();

This would output

Camera configuration ajusted:
 : 1920x1080-NV12
 : 800x600-YUYV

The leading colons seem a bit weird to me, are they intended ? Would

Camera configuration ajusted:
- 1920x1080-NV12
- 800x600-YUYV

be better ?

You could also use the Debug level to log the stream configurations (but
as we're not supposed to fail, Info could be fine), in which case I'd
remove the colon as the end of the first line.

With or without any of those changes,

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

> +
>  		config_.reset();
>  		return -EINVAL;
>  	case CameraConfiguration::Invalid:

-- 
Regards,

Laurent Pinchart