[libcamera-devel] [PATCH] lc-compliance: simple_capture: Handle unsupported roles

[Niklas Söderlund]

Hi Kieran,

On 2021-04-14 15:19:54 +0100, Kieran Bingham wrote:
> Hi Niklas,
> 
> On 14/04/2021 15:10, Niklas Söderlund wrote:
> > It's valid for a camera to return a nullptr if the requested set of
> > roles can not be satisfied. This is not correctly handled by
> > lc-compliance which instead crashes, fix this.
> > 
> > Signed-off-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> 
> Ohhh I bet we haven't considered this in all the places it's used.

Yea I think it's bordering on a flaw in our organic API growth. It's not 
super clear at a first look what the difference errors signaled between

    !conifg

and

    config->validate() != CameraConfiguration::Valid

are. I think we somehow should consolidate this in the Camera so that 
applications always get a configuration but if the pipeline can't 
satisfy the roles it could maybe also be signaled thru 
config->validate()?

> 
> But indeed this is true.
> 
> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> 
> > ---
> >  src/lc-compliance/simple_capture.cpp | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/src/lc-compliance/simple_capture.cpp b/src/lc-compliance/simple_capture.cpp
> > index 811a6220009654be..64e862a08e3a8221 100644
> > --- a/src/lc-compliance/simple_capture.cpp
> > +++ b/src/lc-compliance/simple_capture.cpp
> > @@ -23,6 +23,9 @@ Results::Result SimpleCapture::configure(StreamRole role)
> >  {
> >  	config_ = camera_->generateConfiguration({ role });
> >  
> > +	if (!config_)
> > +		return { Results::Skip, "Role not supported by camera" };
> > +
> >  	if (config_->validate() != CameraConfiguration::Valid) {
> >  		config_.reset();
> >  		return { Results::Fail, "Configuration not valid" };
> > 
> 
> -- 
> Regards
> --
> Kieran

-- 
Regards,
Niklas Söderlund