[libcamera-devel] [PATCH] libcamera: pipeline: uvcvideo: add warning if no default video device is found

[Kieran Bingham]

Heya,

On 27/01/2019 20:21, Laurent Pinchart wrote:
> Hi Niklas,
> 
> On Sun, Jan 27, 2019 at 05:14:52PM +0100, Niklas Söderlund wrote:
>> On 2019-01-27 18:04:26 +0200, Laurent Pinchart wrote:
>>> On Sun, Jan 27, 2019 at 01:52:06AM +0100, Niklas Söderlund wrote:
>>>> If for any reason a default video device is not found in the media graph
>>>> the creation of the UVC pipeline silently failed. This is not optimal
>>>> when debugging problems with the pipeline, add a warning to notify the
>>>> user of the issue.
>>>>
>>>> Signed-off-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
>>>> ---
>>>>  src/libcamera/pipeline/uvcvideo.cpp | 6 ++++++
>>>>  1 file changed, 6 insertions(+)
>>>>
>>>> diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp
>>>> index c51e8bc1f2c2bf25..4ae179d24709c856 100644
>>>> --- a/src/libcamera/pipeline/uvcvideo.cpp
>>>> +++ b/src/libcamera/pipeline/uvcvideo.cpp
>>>> @@ -8,12 +8,15 @@
>>>>  #include <libcamera/camera.h>
>>>>  
>>>>  #include "device_enumerator.h"
>>>> +#include "log.h"
>>>>  #include "media_device.h"
>>>>  #include "pipeline_handler.h"
>>>>  #include "v4l2_device.h"
>>>>  
>>>>  namespace libcamera {
>>>>  
>>>> +LOG_DEFINE_CATEGORY(UVC)
>>>> +
>>>>  class PipelineHandlerUVC : public PipelineHandler
>>>>  {
>>>>  public:
>>>> @@ -60,6 +63,9 @@ bool PipelineHandlerUVC::match(DeviceEnumerator *enumerator)
>>>>  	}
>>>>  
>>>>  	if (!video_ || video_->open()) {
>>>> +		if (!video_)
>>>> +			LOG(UVC, Warning) << "Could not find a default video device";
>>>
>>> I'd make it an error, as it's quite fatal. I think we should also log a
>>> message in the video_->open() failure case, as that's equally fatal (the
>>> permissions denied case is especially important).
>>
>> I will make it an error.
>>
>> Regarding logging the failure of video_->open() this already happens in 
>> V4L2Device::open(). I'm open to a discussion of adding logging in each 
>> pipeline handler for the open call, my initial position is however that 
>> it would add little of value as such an error is already logged. What is 
>> the rest of the groups view?
> 
> I agree with you, it's best to handle as much as possible in the
> libcamera core to minimize the potential issues in the pipeline
> handlers.


Yes, failures and errors should be reported as early as possible, and as
close to the root component as possible.

In this case - at the point of the Open call.

It does however present an possible issue with filtering.
If an error occurs - and someone sets something like:

LIBCAMERA_LOG_LEVELS=pipeline:Debug

- (and nothing else) Would that then /hide/ the V4L2 errors? (or does
each category stay at it's default severity level unless explicitly changed)

As long as the V4L2 category remains unchanged, and will still print
then that's fine.


>>>> +
>>>>  		media_->release();
>>>>  		return false;
>>>>  	}
> 

-- 
Regards
--
Kieran