[libcamera-devel] [PATCH 24/30] cam: Move printing of camera information to CameraSession class

[Kieran Bingham]

On 07/07/2021 03:19, Laurent Pinchart wrote:
> The three CamApp functions listControls(), listProperties() and
> infoConfiguration() operate on a camera. They would thus be better
> placed in the CameraSession class. Move them there. As they now have no
> error to return anymore, make them void functions.

Makes me wonder if the arg parsing for those shouldn't be handled by the
CameraSession - but I don't know if that's practical, and this is fine.

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

> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> ---
>  src/cam/camera_session.cpp |  45 +++++++++++++++++
>  src/cam/camera_session.h   |   4 ++
>  src/cam/main.cpp           | 101 ++++++++-----------------------------
>  3 files changed, 70 insertions(+), 80 deletions(-)
> 
> diff --git a/src/cam/camera_session.cpp b/src/cam/camera_session.cpp
> index 10e66446ee67..ceb2c3ab3a92 100644
> --- a/src/cam/camera_session.cpp
> +++ b/src/cam/camera_session.cpp
> @@ -11,6 +11,7 @@
>  #include <sstream>
>  
>  #include <libcamera/control_ids.h>
> +#include <libcamera/property_ids.h>
>  
>  #include "camera_session.h"
>  #include "event_loop.h"
> @@ -89,6 +90,50 @@ CameraSession::~CameraSession()
>  		camera_->release();
>  }
>  
> +void CameraSession::listControls() const
> +{
> +	for (const auto &ctrl : camera_->controls()) {
> +		const ControlId *id = ctrl.first;
> +		const ControlInfo &info = ctrl.second;
> +
> +		std::cout << "Control: " << id->name() << ": "
> +			  << info.toString() << std::endl;
> +	}
> +}
> +
> +void CameraSession::listProperties() const
> +{
> +	for (const auto &prop : camera_->properties()) {
> +		const ControlId *id = properties::properties.at(prop.first);
> +		const ControlValue &value = prop.second;
> +
> +		std::cout << "Property: " << id->name() << " = "
> +			  << value.toString() << std::endl;
> +	}
> +}
> +
> +void CameraSession::infoConfiguration() const
> +{
> +	unsigned int index = 0;
> +	for (const StreamConfiguration &cfg : *config_) {
> +		std::cout << index << ": " << cfg.toString() << std::endl;
> +
> +		const StreamFormats &formats = cfg.formats();
> +		for (PixelFormat pixelformat : formats.pixelformats()) {
> +			std::cout << " * Pixelformat: "
> +				  << pixelformat.toString() << " "
> +				  << formats.range(pixelformat).toString()
> +				  << std::endl;
> +
> +			for (const Size &size : formats.sizes(pixelformat))
> +				std::cout << "  - " << size.toString()
> +					  << std::endl;
> +		}
> +
> +		index++;
> +	}
> +}
> +
>  int CameraSession::start(const OptionsParser::Options &options)
>  {
>  	int ret;
> diff --git a/src/cam/camera_session.h b/src/cam/camera_session.h
> index 88baf9061629..6221aadadf90 100644
> --- a/src/cam/camera_session.h
> +++ b/src/cam/camera_session.h
> @@ -35,6 +35,10 @@ public:
>  	libcamera::Camera *camera() { return camera_.get(); }
>  	libcamera::CameraConfiguration *config() { return config_.get(); }
>  
> +	void listControls() const;
> +	void listProperties() const;
> +	void infoConfiguration() const;
> +
>  	int start(const OptionsParser::Options &options);
>  	void stop();
>  
> diff --git a/src/cam/main.cpp b/src/cam/main.cpp
> index cba0793ac39b..4fe0c4c368d1 100644
> --- a/src/cam/main.cpp
> +++ b/src/cam/main.cpp
> @@ -39,9 +39,6 @@ private:
>  	void cameraRemoved(std::shared_ptr<Camera> cam);
>  	void captureDone();
>  	int parseOptions(int argc, char *argv[]);
> -	int listControls();
> -	int listProperties();
> -	int infoConfiguration();
>  	int run();
>  
>  	static std::string cameraName(const Camera *camera);
> @@ -158,74 +155,6 @@ int CamApp::parseOptions(int argc, char *argv[])
>  	return 0;
>  }
>  
> -int CamApp::listControls()
> -{
> -	if (!session_) {
> -		std::cout << "Cannot list controls without a camera"
> -			  << std::endl;
> -		return -EINVAL;
> -	}
> -
> -	for (const auto &ctrl : session_->camera()->controls()) {
> -		const ControlId *id = ctrl.first;
> -		const ControlInfo &info = ctrl.second;
> -
> -		std::cout << "Control: " << id->name() << ": "
> -			  << info.toString() << std::endl;
> -	}
> -
> -	return 0;
> -}
> -
> -int CamApp::listProperties()
> -{
> -	if (!session_) {
> -		std::cout << "Cannot list properties without a camera"
> -			  << std::endl;
> -		return -EINVAL;
> -	}
> -
> -	for (const auto &prop : session_->camera()->properties()) {
> -		const ControlId *id = properties::properties.at(prop.first);
> -		const ControlValue &value = prop.second;
> -
> -		std::cout << "Property: " << id->name() << " = "
> -			  << value.toString() << std::endl;
> -	}
> -
> -	return 0;
> -}
> -
> -int CamApp::infoConfiguration()
> -{
> -	if (!session_) {
> -		std::cout << "Cannot print stream information without a camera"
> -			  << std::endl;
> -		return -EINVAL;
> -	}
> -
> -	unsigned int index = 0;
> -	for (const StreamConfiguration &cfg : *session_->config()) {
> -		std::cout << index << ": " << cfg.toString() << std::endl;
> -
> -		const StreamFormats &formats = cfg.formats();
> -		for (PixelFormat pixelformat : formats.pixelformats()) {
> -			std::cout << " * Pixelformat: "
> -				  << pixelformat.toString() << " "
> -				  << formats.range(pixelformat).toString()
> -				  << std::endl;
> -
> -			for (const Size &size : formats.sizes(pixelformat))
> -				std::cout << "  - " << size.toString()
> -					  << std::endl;
> -		}
> -
> -		index++;
> -	}
> -
> -	return 0;
> -}
> -
>  void CamApp::cameraAdded(std::shared_ptr<Camera> cam)
>  {
>  	std::cout << "Camera Added: " << cam->id() << std::endl;
> @@ -269,21 +198,33 @@ int CamApp::run()
>  	}
>  
>  	if (options_.isSet(OptListControls)) {
> -		ret = listControls();
> -		if (ret)
> -			return ret;
> +		if (!session_) {
> +			std::cout << "Cannot list controls without a camera"
> +				  << std::endl;
> +			return -EINVAL;
> +		}
> +
> +		session_->listControls();
>  	}
>  
>  	if (options_.isSet(OptListProperties)) {
> -		ret = listProperties();
> -		if (ret)
> -			return ret;
> +		if (!session_) {
> +			std::cout << "Cannot list properties without a camera"
> +				  << std::endl;
> +			return -EINVAL;
> +		}
> +
> +		session_->listProperties();
>  	}
>  
>  	if (options_.isSet(OptInfo)) {
> -		ret = infoConfiguration();
> -		if (ret)
> -			return ret;
> +		if (!session_) {
> +			std::cout << "Cannot print stream information without a camera"
> +				  << std::endl;
> +			return -EINVAL;
> +		}
> +
> +		session_->infoConfiguration();
>  	}
>  
>  	if (options_.isSet(OptCapture)) {
>