[libcamera-devel] [PATCH v5 11/12] apps: cam: Add option to set stream orientation

[Laurent Pinchart]

Hi Jacopo,

Thank you for the patch.

On Fri, Sep 01, 2023 at 05:02:14PM +0200, Jacopo Mondi via libcamera-devel wrote:
> Add a '--orientation|-o' option to the cam test application to set

Hmmmm... '-o' is usually used as a short form of '--output'. Do we want
to keep it free for that purpose in case we add an output argument later
? I don't mind either way, I suppose we'll still b e able to change it.

> an orientation to the image stream.
> 
> Supported values are the ones obtained by applying flips to the camera
> sensor:
> - rot180: Rotate 180 degrees
> - flip: vertical flip
> - mirror: horizontal flip
> 
> Signed-off-by: Jacopo Mondi <jacopo.mondi at ideasonboard.com>
> Reviewed-by: David Plowman <david.plowman at raspberrypi.com>
> ---
>  src/apps/cam/camera_session.cpp | 17 +++++++++++++++++
>  src/apps/cam/main.cpp           |  5 +++++
>  src/apps/cam/main.h             |  1 +
>  3 files changed, 23 insertions(+)
> 
> diff --git a/src/apps/cam/camera_session.cpp b/src/apps/cam/camera_session.cpp
> index 066e397b5f47..1cf7a245aaa7 100644
> --- a/src/apps/cam/camera_session.cpp
> +++ b/src/apps/cam/camera_session.cpp
> @@ -65,6 +65,23 @@ CameraSession::CameraSession(CameraManager *cm,
>  		return;
>  	}
>  
> +	if (options_.isSet(OptOrientation)) {
> +		std::string orient = options_[OptOrientation].toString();
> +		if (orient == "rot180") {
> +			config->orientation =
> +				libcamera::Orientation::rotate180;
> +		} else if (orient == "mirror") {
> +			config->orientation =
> +				libcamera::Orientation::rotate0Flip;

This makes me wonder if we shouldn't call the orientation rotate0Mirror.
What do you think ?

> +		} else if (orient == "flip") {
> +			config->orientation =
> +				libcamera::Orientation::rotate180Flip;
> +		} else {
> +			std::cerr << "Invalid orientation " << orient << std::endl;
> +			return;
> +		}

This would be more C++, and easier to extend:

		static const std::map<std::string, libcamera::Orientation> orientations{
			{ "rot180", libcamera::Orientation::rotate180 },
			{ "mirror", libcamera::Orientation::rotate0Flip },
			{ "flip", libcamera::Orientation::rotate180Flip },
		};

		auto orientation = orientations.find(options_[OptOrientation].toString());
		if (orientation == orientations.end()) {
			std::cerr << "Invalid orientation " << orient << std::endl;
			return;
		}

		config->orientation = orientation.second;

Entirely up to you.

> +	}
> +
>  	/* Apply configuration if explicitly requested. */
>  	if (StreamKeyValueParser::updateConfiguration(config.get(),
>  						      options_[OptStream])) {
> diff --git a/src/apps/cam/main.cpp b/src/apps/cam/main.cpp
> index 5d8a57bc14e5..d68243edac3f 100644
> --- a/src/apps/cam/main.cpp
> +++ b/src/apps/cam/main.cpp
> @@ -133,6 +133,11 @@ int CamApp::parseOptions(int argc, char *argv[])
>  			 "Capture until interrupted by user or until <count> frames captured",
>  			 "capture", ArgumentOptional, "count", false,
>  			 OptCamera);
> +
> +	parser.addOption(OptOrientation, OptionString,
> +			 "Desired image orientation (rot180, mirror, flip)",
> +			 "orientation", ArgumentRequired, "orientation", false,
> +			 OptCamera);
>  #ifdef HAVE_KMS
>  	parser.addOption(OptDisplay, OptionString,
>  			 "Display viewfinder through DRM/KMS on specified connector",
> diff --git a/src/apps/cam/main.h b/src/apps/cam/main.h
> index 526aecece3f3..4aa959b32e13 100644
> --- a/src/apps/cam/main.h
> +++ b/src/apps/cam/main.h
> @@ -17,6 +17,7 @@ enum {
>  	OptList = 'l',
>  	OptListProperties = 'p',
>  	OptMonitor = 'm',
> +	OptOrientation = 'o',
>  	OptSDL = 'S',
>  	OptStream = 's',
>  	OptListControls = 256,

-- 
Regards,

Laurent Pinchart