[libcamera-devel] [PATCH] qcam: main: Cache lookup of role property

[Niklas Söderlund]

Hi Laurent,

On 2020-03-23 12:40:25 +0200, Laurent Pinchart wrote:
> This should obviously have s/qcam/cam/ in the subject line.

With this fixed and regardless on how you choose to act on Kieran's 
suggestion,

Reviewed-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>

> 
> On Mon, Mar 23, 2020 at 12:39:38PM +0200, Laurent Pinchart wrote:
> > The code handling the stream role option retrieves the role property and
> > converts it to a string in every branch. Cache it and use the cached
> > value.
> > 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > ---
> >  src/cam/main.cpp | 16 ++++++++++------
> >  1 file changed, 10 insertions(+), 6 deletions(-)
> > 
> > diff --git a/src/cam/main.cpp b/src/cam/main.cpp
> > index 2a0a830ff371..56059ab6c047 100644
> > --- a/src/cam/main.cpp
> > +++ b/src/cam/main.cpp
> > @@ -209,17 +209,21 @@ int CamApp::prepareConfig()
> >  		for (auto const &value : streamOptions) {
> >  			KeyValueParser::Options opt = value.toKeyValues();
> >  
> > -			if (!opt.isSet("role")) {
> > -				roles.push_back(StreamRole::VideoRecording);
> > -			} else if (opt["role"].toString() == "viewfinder") {
> > +			std::string role;
> > +			if (opt.isSet("role"))
> > +				role = opt["role"].toString();
> > +			else
> > +				role = "viewfinder";
> > +
> > +			if (role == "viewfinder") {
> >  				roles.push_back(StreamRole::Viewfinder);
> > -			} else if (opt["role"].toString() == "video") {
> > +			} else if (role == "video") {
> >  				roles.push_back(StreamRole::VideoRecording);
> > -			} else if (opt["role"].toString() == "still") {
> > +			} else if (role == "still") {
> >  				roles.push_back(StreamRole::StillCapture);
> >  			} else {
> >  				std::cerr << "Unknown stream role "
> > -					  << opt["role"].toString() << std::endl;
> > +					  << role << std::endl;
> >  				return -EINVAL;
> >  			}
> >  		}
> 
> -- 
> Regards,
> 
> Laurent Pinchart
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel

-- 
Regards,
Niklas Söderlund