[libcamera-devel,1/2] simple-cam: Early return if no cameras found on the system

9787 diff mbox series

Early return if no cameras are found on the system.
Failing to do so, the codepath will segfault while trying to
acquire a non-existent camera.

Signed-off-by: Umang Jain <email@uajain.com>
---
 simple-cam.cpp | 7 +++++++
 1 file changed, 7 insertions(+)

Hi Umang,

On 24/09/2020 15:18, Umang Jain wrote:
> Early return if no cameras are found on the system.
> Failing to do so, the codepath will segfault while trying to
> acquire a non-existent camera.
> 

Ah, yes this sounds like a reasonably good idea to have here.

> Signed-off-by: Umang Jain <email@uajain.com>
> ---
>  simple-cam.cpp | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/simple-cam.cpp b/simple-cam.cpp
> index 3aa975e..d51000c 100644
> --- a/simple-cam.cpp
> +++ b/simple-cam.cpp
> @@ -95,6 +95,13 @@ int main()
>  	CameraManager *cm = new CameraManager();
>  	cm->start();
>  
> +	if(!cm->cameras().size()) {
> +		std::cout << "Please connect atleast one camera to the system."

s/atleast/at least/

Though I'd probably simply say "No cameras were identified on the
system", as it might not always be feasible to 'add' a camera, and it
might also be that the cameras are connected, but actually libcamera
might not have the required pipelines enabled.

So as the action to take is ambiguous, I'd leave it out and just state
the facts.

Otherwise,

Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

> +			  << std::endl;
> +		cm->stop();
> +		return -1;
> +	}
> +
>  	/*
>  	 * Just as a test, list all id's of the Camera registered in the
>  	 * system. They are indexed by name by the CameraManager.
>

Hi Kieran

On 9/24/20 8:11 PM, Kieran Bingham wrote:
> Hi Umang,
>
> On 24/09/2020 15:18, Umang Jain wrote:
>> Early return if no cameras are found on the system.
>> Failing to do so, the codepath will segfault while trying to
>> acquire a non-existent camera.
>>
> Ah, yes this sounds like a reasonably good idea to have here.
Great. I thought it was an overkill initially, but sent the patch anyway.
>> Signed-off-by: Umang Jain <email@uajain.com>
>> ---
>>   simple-cam.cpp | 7 +++++++
>>   1 file changed, 7 insertions(+)
>>
>> diff --git a/simple-cam.cpp b/simple-cam.cpp
>> index 3aa975e..d51000c 100644
>> --- a/simple-cam.cpp
>> +++ b/simple-cam.cpp
>> @@ -95,6 +95,13 @@ int main()
>>   	CameraManager *cm = new CameraManager();
>>   	cm->start();
>>   
>> +	if(!cm->cameras().size()) {
>> +		std::cout << "Please connect atleast one camera to the system."
> s/atleast/at least/
>
> Though I'd probably simply say "No cameras were identified on the
> system", as it might not always be feasible to 'add' a camera, and it
> might also be that the cameras are connected, but actually libcamera
> might not have the required pipelines enabled.
>
> So as the action to take is ambiguous, I'd leave it out and just state
> the facts.
Ah, indeed. I didn't put a lot of thought in writing the log message.
>
> Otherwise,
>
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
>
>> +			  << std::endl;
>> +		cm->stop();
>> +		return -1;
>> +	}
>> +
>>   	/*
>>   	 * Just as a test, list all id's of the Camera registered in the
>>   	 * system. They are indexed by name by the CameraManager.
>>