[libcamera-devel,v3,10/10] apps: cam: Add option to set stream orientation
diff mbox series

Message ID 20230718105210.83558-11-jacopo.mondi@ideasonboard.com
State Superseded
Headers show
Series
  • libcamera: Replace CameraConfiguration::transform
Related show

Commit Message

Jacopo Mondi July 18, 2023, 10:52 a.m. UTC 
Add a '--orientation|-o' option to the cam test application to set
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@ideasonboard.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(+)

Comments

David Plowman July 20, 2023, 10:16 a.m. UTC | #1 
Hi Jacopo

Thanks for the patch.

On Tue, 18 Jul 2023 at 11:52, Jacopo Mondi
<jacopo.mondi@ideasonboard.com> wrote:
>
> Add a '--orientation|-o' option to the cam test application to set
> 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

I guess it offends my mathematical sense of symmetry and completeness
ever so slightly not to have an explicit parameter for the fourth and
final supported orientation... namely "identity" (or "rot0" or
something)!! But as a non-cam user, please feel free to ignore me.

>
> Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.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") {

Do we care about case in option parameters? I guess not. In which case
it all LGTM.

Reviewed-by: David Plowman <david.plowman@raspberrypi.com>

Thanks!
David

> +                       config->orientation =
> +                               libcamera::Orientation::rotate180;
> +               } else if (orient == "mirror") {
> +                       config->orientation =
> +                               libcamera::Orientation::rotate0Flip;
> +               } else if (orient == "flip") {
> +                       config->orientation =
> +                               libcamera::Orientation::rotate180Flip;
> +               } else {
> +                       std::cerr << "Invalid orientation " << orient << std::endl;
> +                       return;
> +               }
> +       }
> +
>         /* 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..8698d5eedcf8 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,
> +                        "The camera stream 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,
> --
> 2.40.1
>
Jacopo Mondi July 24, 2023, 7 a.m. UTC | #2 
Hi David

On Thu, Jul 20, 2023 at 11:16:40AM +0100, David Plowman via libcamera-devel wrote:
> Hi Jacopo
>
> Thanks for the patch.
>
> On Tue, 18 Jul 2023 at 11:52, Jacopo Mondi
> <jacopo.mondi@ideasonboard.com> wrote:
> >
> > Add a '--orientation|-o' option to the cam test application to set
> > 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
>
> I guess it offends my mathematical sense of symmetry and completeness
> ever so slightly not to have an explicit parameter for the fourth and
> final supported orientation... namely "identity" (or "rot0" or
> something)!! But as a non-cam user, please feel free to ignore me.
>

Isn't "Identity" the default ?

> >
> > Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.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") {
>
> Do we care about case in option parameters? I guess not. In which case

I don't think we do for any of the other ?

> it all LGTM.
>
> Reviewed-by: David Plowman <david.plowman@raspberrypi.com>

Thanks
  j
>
> Thanks!
> David
>
> > +                       config->orientation =
> > +                               libcamera::Orientation::rotate180;
> > +               } else if (orient == "mirror") {
> > +                       config->orientation =
> > +                               libcamera::Orientation::rotate0Flip;
> > +               } else if (orient == "flip") {
> > +                       config->orientation =
> > +                               libcamera::Orientation::rotate180Flip;
> > +               } else {
> > +                       std::cerr << "Invalid orientation " << orient << std::endl;
> > +                       return;
> > +               }
> > +       }
> > +
> >         /* 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..8698d5eedcf8 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,
> > +                        "The camera stream 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,
> > --
> > 2.40.1
> >

Patch
diff mbox series 

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;
+		} else if (orient == "flip") {
+			config->orientation =
+				libcamera::Orientation::rotate180Flip;
+		} else {
+			std::cerr << "Invalid orientation " << orient << std::endl;
+			return;
+		}
+	}
+
 	/* 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..8698d5eedcf8 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,
+			 "The camera stream 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,