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

[Niklas Söderlund]

Hi Laurent,

Thanks for your feedback.

On 2019-01-27 18:04:26 +0200, Laurent Pinchart wrote:
> Hi Niklas,
> 
> Thank you for the patch.
> 
> 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?

> 
> > +
> >  		media_->release();
> >  		return false;
> >  	}
> 
> -- 
> Regards,
> 
> Laurent Pinchart

-- 
Regards,
Niklas Söderlund