[PATCH 3/3] apps: cam: Support full orientation options

[Jacopo Mondi]

Hi Kieran

On Thu, May 01, 2025 at 03:16:09PM +0100, Kieran Bingham wrote:
> The Orientation control was implemented in commit 7e5f1e1cedf5 "apps:

nit: could you use the canonical commit reference style ?

The Orientation control was implemented in commit 7e5f1e1cedf5 ("apps:
 cam: Add option to set stream orientation") but did not fully add all of

> cam: Add option to set stream orientation" but did not fully add all of
> the supported Orientation types.
>
> The upcoming dewarp support on RkISP1 will provide full rotation
> implementation options, including previously difficult '90' degree
> rotations.
>
> Extend the cam application to support setting any of the valid options.
>
> The existing 'mirror' and 'flip' options are not consistent with the

Where are these existing already ?

> rest of the option definitions, but maintain them as useful aliases to
> their corresponding type.
>
> Fixes: 7e5f1e1cedf5 ("apps: cam: Add option to set stream orientation")

Not really a fixes..

> Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
>
> ---
> It might be debatable if this really is a 'Fixes' as the existing commit
> wasn't broken - just not fully implemented - however the full set of
> Orientation types were there at the time the patch was merged.

Ah yes. Doesn't matter at all to me

>
>  src/apps/cam/camera_session.cpp | 9 +++++++++
>  src/apps/cam/main.cpp           | 5 ++++-
>  2 files changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/src/apps/cam/camera_session.cpp b/src/apps/cam/camera_session.cpp
> index f63fcb228519..43188133d9cb 100644
> --- a/src/apps/cam/camera_session.cpp
> +++ b/src/apps/cam/camera_session.cpp
> @@ -97,7 +97,16 @@ CameraSession::CameraSession(CameraManager *cm,
>  		std::string orientOpt = options_[OptOrientation].toString();
>  		static const std::map<std::string, libcamera::Orientation> orientations{
>  			{ "rot0", libcamera::Orientation::Rotate0 },
> +			{ "rot90", libcamera::Orientation::Rotate90 },
>  			{ "rot180", libcamera::Orientation::Rotate180 },
> +			{ "rot270", libcamera::Orientation::Rotate270 },
> +
> +			{ "rot0mirror", libcamera::Orientation::Rotate0Mirror },
> +			{ "rot90mirror", libcamera::Orientation::Rotate90Mirror },
> +			{ "rot180mirror", libcamera::Orientation::Rotate180Mirror },
> +			{ "rot270mirror", libcamera::Orientation::Rotate270Mirror },
> +
> +			/* Helpful aliases */
>  			{ "mirror", libcamera::Orientation::Rotate0Mirror },
>  			{ "flip", libcamera::Orientation::Rotate180Mirror },
>  		};
> diff --git a/src/apps/cam/main.cpp b/src/apps/cam/main.cpp
> index fa266eca6d30..f1495a2db465 100644
> --- a/src/apps/cam/main.cpp
> +++ b/src/apps/cam/main.cpp
> @@ -136,7 +136,10 @@ int CamApp::parseOptions(int argc, char *argv[])
>  			 OptCamera);
>
>  	parser.addOption(OptOrientation, OptionString,
> -			 "Desired image orientation (rot0, rot180, mirror, flip)",
> +			 "Desired image orientation. Supported values:\n"
> +			 "- rot0, rot90, rot180, rot270,\n"
> +			 "- rot0mirror, rot90mirror, rot180mirror, rot270mirror,\n"
> +			 "- mirror (alias for rot0mirror), flip (alias for rot180mirror)",
>  			 "orientation", ArgumentRequired, "orientation", false,
>  			 OptCamera);

Have you tested this on a platform that doesn't support
transpositions?

With the above confirmed
Reviewed-by: Jacopo Mondi <jacopo.mondi at ideasonboard.com>

Thanks
  j


>  #ifdef HAVE_KMS
> --
> 2.48.1
>