[libcamera-devel] [RFC PATCH 2/6] android: camera_device: Report configuration changes from validate()

[Kieran Bingham]

On 22/07/2020 11:36, Jacopo Mondi wrote:
> Hi Kieran,
> 
> On Tue, Jul 21, 2020 at 11:01:22PM +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.
>>
>> Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
>> ---
>>  src/android/camera_device.cpp | 8 ++++++++
>>  1 file changed, 8 insertions(+)
>>
>> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
>> index f51542b282d5..3f3d7857f0ab 100644
>> --- a/src/android/camera_device.cpp
>> +++ b/src/android/camera_device.cpp
>> @@ -1032,6 +1032,14 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)
>>  		break;
>>  	case CameraConfiguration::Adjusted:
>>  		LOG(HAL, Info) << "Camera configuration adjusted";
>> +
>> +		for (unsigned int i = 0; i < stream_list->num_streams; ++i) {
>> +			CameraStream *cameraStream = &streams_[i];
>> +			StreamConfiguration &cfg = config_->at(cameraStream->index);
>> +
>> +			LOG(HAL, Info) << i << " : " << cfg.toString();
> 
> Debug maybe ?

This is on the fail path, so it's highly relevant to seeing why the
stream was not constructed. The validation phase has failed. I chose
'Info' to match the warning statement above.

This is a continuation of that statement IMO, It could be reduced to
Debug ... but I would expect /any/ time this has printed, that it would
be relevant to know... I'd almost put it as a Warning....

Unless Android uses this path as a means to test the capabilities of the
device? If it's a 'probing' path and failures are expected then yes,
this could be a Debug print ... but I sort of thought this code path
expects success... so a validation failure is a fairly critical event.

If that is true, I'd even put the "Camera configuration adjusted" up to
warning/error though.


> 
>> +		}
>> +
>>  		config_.reset();
>>  		return -EINVAL;
>>  	case CameraConfiguration::Invalid:
>> --
>> 2.25.1
>>
>> _______________________________________________
>> libcamera-devel mailing list
>> libcamera-devel at lists.libcamera.org
>> https://lists.libcamera.org/listinfo/libcamera-devel

-- 
Regards
--
Kieran