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

[Jacopo Mondi]

Hi Tomi

On Mon, Sep 04, 2023 at 08:18:11PM +0300, Tomi Valkeinen wrote:
> On 01/09/2023 18:02, Jacopo Mondi via libcamera-devel wrote:
> > Add a '--orientation|-o' option to the Python version of the cam test
> > application to set an orientation to the image stream.
> >
> > Supported values are:
> > - Rot180: Rotate 180 degrees
> > - Flip: vertical flip
> > - Mirror: horizontal flip
> >
> > Signed-off-by: Jacopo Mondi <jacopo.mondi at ideasonboard.com>
> > ---
> >   src/py/cam/cam.py | 20 ++++++++++++++++++++
> >   1 file changed, 20 insertions(+)
> >
> > diff --git a/src/py/cam/cam.py b/src/py/cam/cam.py
> > index a2a115c164b4..84d8ca1716fd 100755
> > --- a/src/py/cam/cam.py
> > +++ b/src/py/cam/cam.py
> > @@ -23,6 +23,7 @@ class CameraContext:
> >       opt_metadata: bool
> >       opt_save_frames: bool
> >       opt_capture: int
> > +    opt_orientation: str
> >       stream_names: dict[libcam.Stream, str]
> >       streams: list[libcam.Stream]
> > @@ -146,6 +147,20 @@ class CameraContext:
> >               if 'pixelformat' in stream_opts:
> >                   stream_config.pixel_format = libcam.PixelFormat(stream_opts['pixelformat'])
> > +        if self.opt_orientation is not None:
> > +            orientation_map = {
> > +                'Rot180': libcam.Orientation.Rotate180,
> > +                'Mirror': libcam.Orientation.Rotate0Flip,
> > +                'Flip': libcam.Orientation.Rotate180Flip,
> > +            }
>
> Any reason to not support all orientations? In python you can get the name
> of the enum as a string, and you could just compare that directly to the
> string from the user. Also, you could lower-case both before comparison, so
> that the user could say "-o rotate270".
>

The three supported options are the ones supporte by the C++ version
of cam, and I like the two to behave the same. The reason why the C++
version of cam only supports the three above options is because
they're usually the most common features as user expects. The list
could indeed be expanded, but for now I would like the two versions to
behave the same. Is this ok with you ?

> > +
> > +            orient = orientation_map.get(self.opt_orientation, None)
> > +            if orient is None:
> > +                print('Bad orientation: ', self.opt_orientation)
> > +                sys.exit(-1)
> > +
> > +            camconfig.orientation = orient
> > +
> >           stat = camconfig.validate()
> >           if stat == libcam.CameraConfiguration.Status.Invalid:
> > @@ -385,6 +400,7 @@ def main():
> >       parser.add_argument('--metadata', nargs=0, type=bool, action=CustomAction, help='Print the metadata for completed requests')
> >       parser.add_argument('--strict-formats', type=bool, nargs=0, action=CustomAction, help='Do not allow requested stream format(s) to be adjusted')
> >       parser.add_argument('-s', '--stream', nargs='+', action=CustomAction)
> > +    parser.add_argument('-o', '--orientation', help='Desired image orientation (Rot180, Mirror, Flip)')
>
> Hmm, you need to use the action=CustomAction to get the option to apply to a
> camera context.

Uh right, orientation is indeed per camera
>
> And then I think you can do this below:
>
> ctx.opt_orientation = args.orientation.get(cam_idx, None)
>

Thanks
   j

> >       args = parser.parse_args()
> >       cm = libcam.CameraManager.singleton()
> > @@ -408,6 +424,10 @@ def main():
> >           ctx.opt_metadata = args.metadata.get(cam_idx, False)
> >           ctx.opt_strict_formats = args.strict_formats.get(cam_idx, False)
> >           ctx.opt_stream = args.stream.get(cam_idx, ['role=viewfinder'])
> > +        if args.orientation is not None:
> > +            ctx.opt_orientation = args.orientation
> > +        else:
> > +            ctx.opt_orientation = None
> >           contexts.append(ctx)
> >       for ctx in contexts:
>