[libcamera-devel] [RFC 4/4] apps: cam: Add option to set stream orientation

[Jacopo Mondi]

Hi Umang

On Wed, Jun 21, 2023 at 11:04:48AM +0530, Umang Jain wrote:
> Hi Jacopo,
>
> Thank you for the patch.
>
> On 6/20/23 7:59 PM, Jacopo Mondi via libcamera-devel 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
>
> From an application PoV, it should be able to set every possible rotation
> defined in enum Orientation, I think.
> >
> > Signed-off-by: Jacopo Mondi <jacopo.mondi at 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 8fcec6304d66..2d6a1440cde2 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::CameraConfiguration::rotate180;
> > +		} else if (orient == "mirror") {
> > +			config->orientation =
> > +				libcamera::CameraConfiguration::flipRotate0;
> > +		} else if (orient == "flip") {
> > +			config->orientation =
> > +				libcamera::CameraConfiguration::flipRotate180;
> > +		} else {
> > +			std::cerr << "Invalid orientation " << orient << std::endl;
> > +			return;
>
> If we want to restrict the orientation not supported by libcamera, we should
> restrict them at libcamera level itself.
>
> Restricting them at application level doesn't make sense to me.

Remember 'cam' is a test tool, not a full application.

I had the same thought you had, but then I wondered what was the
purpose of exposing a value we know cannot be used. The same reasoning
could be made for 'rotation0': does it make sense to have as an option
something we know it's the 'default' ?

> > +		}
> > +	}
> > +
> >   	/* 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
> >
>