[libcamera-devel] cam: Rename conf variable referring to command line option to opt
diff mbox series

Message ID 20190522221210.19257-1-laurent.pinchart@ideasonboard.com
State Accepted
Commit 8d846ed8a7d9d7c9783472a618a7f369f0843e67
Headers show
Series
  • [libcamera-devel] cam: Rename conf variable referring to command line option to opt
Related show

Commit Message

Laurent Pinchart May 22, 2019, 10:12 p.m. UTC 
Naming a variable that refers to command line options 'conf' is
confusing as we using 'config' and 'cfg' to refer to camera and stream
configurations. Rename it to 'opt'.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 src/cam/main.cpp | 26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)

Comments

Jacopo Mondi May 23, 2019, 7:21 a.m. UTC | #1 
Hi Laurent,
  I welcome this change

Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>

On Thu, May 23, 2019 at 01:12:10AM +0300, Laurent Pinchart wrote:
> Naming a variable that refers to command line options 'conf' is
> confusing as we using 'config' and 'cfg' to refer to camera and stream
> configurations. Rename it to 'opt'.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  src/cam/main.cpp | 26 +++++++++++++-------------
>  1 file changed, 13 insertions(+), 13 deletions(-)
>
> diff --git a/src/cam/main.cpp b/src/cam/main.cpp
> index 5ecd7e0e38d7..4155889678ce 100644
> --- a/src/cam/main.cpp
> +++ b/src/cam/main.cpp
> @@ -98,19 +98,19 @@ static std::unique_ptr<CameraConfiguration> prepareCameraConfig()
>
>  	/* Use roles and get a default configuration. */
>  	for (auto const &value : streamOptions) {
> -		KeyValueParser::Options conf = value.toKeyValues();
> +		KeyValueParser::Options opt = value.toKeyValues();
>
> -		if (!conf.isSet("role")) {
> +		if (!opt.isSet("role")) {
>  			roles.push_back(StreamRole::VideoRecording);
> -		} else if (conf["role"].toString() == "viewfinder") {
> +		} else if (opt["role"].toString() == "viewfinder") {
>  			roles.push_back(StreamRole::Viewfinder);
> -		} else if (conf["role"].toString() == "video") {
> +		} else if (opt["role"].toString() == "video") {
>  			roles.push_back(StreamRole::VideoRecording);
> -		} else if (conf["role"].toString() == "still") {
> +		} else if (opt["role"].toString() == "still") {
>  			roles.push_back(StreamRole::StillCapture);
>  		} else {
>  			std::cerr << "Unknown stream role "
> -				  << conf["role"].toString() << std::endl;
> +				  << opt["role"].toString() << std::endl;
>  			return nullptr;
>  		}
>  	}
> @@ -125,18 +125,18 @@ static std::unique_ptr<CameraConfiguration> prepareCameraConfig()
>  	/* Apply configuration explicitly requested. */
>  	unsigned int i = 0;
>  	for (auto const &value : streamOptions) {
> -		KeyValueParser::Options conf = value.toKeyValues();
> +		KeyValueParser::Options opt = value.toKeyValues();
>  		StreamConfiguration &cfg = config->at(i++);
>
> -		if (conf.isSet("width"))
> -			cfg.size.width = conf["width"];
> +		if (opt.isSet("width"))
> +			cfg.size.width = opt["width"];
>
> -		if (conf.isSet("height"))
> -			cfg.size.height = conf["height"];
> +		if (opt.isSet("height"))
> +			cfg.size.height = opt["height"];
>
>  		/* TODO: Translate 4CC string to ID. */
> -		if (conf.isSet("pixelformat"))
> -			cfg.pixelFormat = conf["pixelformat"];
> +		if (opt.isSet("pixelformat"))
> +			cfg.pixelFormat = opt["pixelformat"];
>  	}
>
>  	return config;
> --
> Regards,
>
> Laurent Pinchart
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
Kieran Bingham May 23, 2019, 10:15 a.m. UTC | #2 
Hi Laurent,

On 22/05/2019 23:12, Laurent Pinchart wrote:
> Naming a variable that refers to command line options 'conf' is
> confusing as we using 'config' and 'cfg' to refer to camera and stream
> configurations. Rename it to 'opt'.
> 

Sounds fine to me, but you're going to be racing with Niklas on his
series...

Who's going to rebase on top of the other ...

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

But you should probably poke neg for an ack due to the patch conflicts


> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  src/cam/main.cpp | 26 +++++++++++++-------------
>  1 file changed, 13 insertions(+), 13 deletions(-)
> 
> diff --git a/src/cam/main.cpp b/src/cam/main.cpp
> index 5ecd7e0e38d7..4155889678ce 100644
> --- a/src/cam/main.cpp
> +++ b/src/cam/main.cpp
> @@ -98,19 +98,19 @@ static std::unique_ptr<CameraConfiguration> prepareCameraConfig()
>  
>  	/* Use roles and get a default configuration. */
>  	for (auto const &value : streamOptions) {
> -		KeyValueParser::Options conf = value.toKeyValues();
> +		KeyValueParser::Options opt = value.toKeyValues();
>  
> -		if (!conf.isSet("role")) {
> +		if (!opt.isSet("role")) {
>  			roles.push_back(StreamRole::VideoRecording);
> -		} else if (conf["role"].toString() == "viewfinder") {
> +		} else if (opt["role"].toString() == "viewfinder") {
>  			roles.push_back(StreamRole::Viewfinder);
> -		} else if (conf["role"].toString() == "video") {
> +		} else if (opt["role"].toString() == "video") {
>  			roles.push_back(StreamRole::VideoRecording);
> -		} else if (conf["role"].toString() == "still") {
> +		} else if (opt["role"].toString() == "still") {
>  			roles.push_back(StreamRole::StillCapture);
>  		} else {
>  			std::cerr << "Unknown stream role "
> -				  << conf["role"].toString() << std::endl;
> +				  << opt["role"].toString() << std::endl;
>  			return nullptr;
>  		}
>  	}
> @@ -125,18 +125,18 @@ static std::unique_ptr<CameraConfiguration> prepareCameraConfig()
>  	/* Apply configuration explicitly requested. */
>  	unsigned int i = 0;
>  	for (auto const &value : streamOptions) {
> -		KeyValueParser::Options conf = value.toKeyValues();
> +		KeyValueParser::Options opt = value.toKeyValues();
>  		StreamConfiguration &cfg = config->at(i++);
>  
> -		if (conf.isSet("width"))
> -			cfg.size.width = conf["width"];
> +		if (opt.isSet("width"))
> +			cfg.size.width = opt["width"];
>  
> -		if (conf.isSet("height"))
> -			cfg.size.height = conf["height"];
> +		if (opt.isSet("height"))
> +			cfg.size.height = opt["height"];
>  
>  		/* TODO: Translate 4CC string to ID. */
> -		if (conf.isSet("pixelformat"))
> -			cfg.pixelFormat = conf["pixelformat"];
> +		if (opt.isSet("pixelformat"))
> +			cfg.pixelFormat = opt["pixelformat"];
>  	}
>  
>  	return config;
>
Laurent Pinchart May 23, 2019, 10:42 a.m. UTC | #3 
On Thu, May 23, 2019 at 11:15:09AM +0100, Kieran Bingham wrote:
> Hi Laurent,
> 
> On 22/05/2019 23:12, Laurent Pinchart wrote:
> > Naming a variable that refers to command line options 'conf' is
> > confusing as we using 'config' and 'cfg' to refer to camera and stream
> > configurations. Rename it to 'opt'.
> 
> Sounds fine to me, but you're going to be racing with Niklas on his
> series...
> 
> Who's going to rebase on top of the other ...
> 
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> 
> But you should probably poke neg for an ack due to the patch conflicts

I think Niklas will have to rebase, as I've pushed this already :-$

> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> >  src/cam/main.cpp | 26 +++++++++++++-------------
> >  1 file changed, 13 insertions(+), 13 deletions(-)
> > 
> > diff --git a/src/cam/main.cpp b/src/cam/main.cpp
> > index 5ecd7e0e38d7..4155889678ce 100644
> > --- a/src/cam/main.cpp
> > +++ b/src/cam/main.cpp
> > @@ -98,19 +98,19 @@ static std::unique_ptr<CameraConfiguration> prepareCameraConfig()
> >  
> >  	/* Use roles and get a default configuration. */
> >  	for (auto const &value : streamOptions) {
> > -		KeyValueParser::Options conf = value.toKeyValues();
> > +		KeyValueParser::Options opt = value.toKeyValues();
> >  
> > -		if (!conf.isSet("role")) {
> > +		if (!opt.isSet("role")) {
> >  			roles.push_back(StreamRole::VideoRecording);
> > -		} else if (conf["role"].toString() == "viewfinder") {
> > +		} else if (opt["role"].toString() == "viewfinder") {
> >  			roles.push_back(StreamRole::Viewfinder);
> > -		} else if (conf["role"].toString() == "video") {
> > +		} else if (opt["role"].toString() == "video") {
> >  			roles.push_back(StreamRole::VideoRecording);
> > -		} else if (conf["role"].toString() == "still") {
> > +		} else if (opt["role"].toString() == "still") {
> >  			roles.push_back(StreamRole::StillCapture);
> >  		} else {
> >  			std::cerr << "Unknown stream role "
> > -				  << conf["role"].toString() << std::endl;
> > +				  << opt["role"].toString() << std::endl;
> >  			return nullptr;
> >  		}
> >  	}
> > @@ -125,18 +125,18 @@ static std::unique_ptr<CameraConfiguration> prepareCameraConfig()
> >  	/* Apply configuration explicitly requested. */
> >  	unsigned int i = 0;
> >  	for (auto const &value : streamOptions) {
> > -		KeyValueParser::Options conf = value.toKeyValues();
> > +		KeyValueParser::Options opt = value.toKeyValues();
> >  		StreamConfiguration &cfg = config->at(i++);
> >  
> > -		if (conf.isSet("width"))
> > -			cfg.size.width = conf["width"];
> > +		if (opt.isSet("width"))
> > +			cfg.size.width = opt["width"];
> >  
> > -		if (conf.isSet("height"))
> > -			cfg.size.height = conf["height"];
> > +		if (opt.isSet("height"))
> > +			cfg.size.height = opt["height"];
> >  
> >  		/* TODO: Translate 4CC string to ID. */
> > -		if (conf.isSet("pixelformat"))
> > -			cfg.pixelFormat = conf["pixelformat"];
> > +		if (opt.isSet("pixelformat"))
> > +			cfg.pixelFormat = opt["pixelformat"];
> >  	}
> >  
> >  	return config;

Patch
diff mbox series 

diff --git a/src/cam/main.cpp b/src/cam/main.cpp
index 5ecd7e0e38d7..4155889678ce 100644
--- a/src/cam/main.cpp
+++ b/src/cam/main.cpp
@@ -98,19 +98,19 @@  static std::unique_ptr<CameraConfiguration> prepareCameraConfig()
 
 	/* Use roles and get a default configuration. */
 	for (auto const &value : streamOptions) {
-		KeyValueParser::Options conf = value.toKeyValues();
+		KeyValueParser::Options opt = value.toKeyValues();
 
-		if (!conf.isSet("role")) {
+		if (!opt.isSet("role")) {
 			roles.push_back(StreamRole::VideoRecording);
-		} else if (conf["role"].toString() == "viewfinder") {
+		} else if (opt["role"].toString() == "viewfinder") {
 			roles.push_back(StreamRole::Viewfinder);
-		} else if (conf["role"].toString() == "video") {
+		} else if (opt["role"].toString() == "video") {
 			roles.push_back(StreamRole::VideoRecording);
-		} else if (conf["role"].toString() == "still") {
+		} else if (opt["role"].toString() == "still") {
 			roles.push_back(StreamRole::StillCapture);
 		} else {
 			std::cerr << "Unknown stream role "
-				  << conf["role"].toString() << std::endl;
+				  << opt["role"].toString() << std::endl;
 			return nullptr;
 		}
 	}
@@ -125,18 +125,18 @@  static std::unique_ptr<CameraConfiguration> prepareCameraConfig()
 	/* Apply configuration explicitly requested. */
 	unsigned int i = 0;
 	for (auto const &value : streamOptions) {
-		KeyValueParser::Options conf = value.toKeyValues();
+		KeyValueParser::Options opt = value.toKeyValues();
 		StreamConfiguration &cfg = config->at(i++);
 
-		if (conf.isSet("width"))
-			cfg.size.width = conf["width"];
+		if (opt.isSet("width"))
+			cfg.size.width = opt["width"];
 
-		if (conf.isSet("height"))
-			cfg.size.height = conf["height"];
+		if (opt.isSet("height"))
+			cfg.size.height = opt["height"];
 
 		/* TODO: Translate 4CC string to ID. */
-		if (conf.isSet("pixelformat"))
-			cfg.pixelFormat = conf["pixelformat"];
+		if (opt.isSet("pixelformat"))
+			cfg.pixelFormat = opt["pixelformat"];
 	}
 
 	return config;