[libcamera-devel] [RFC v1 1/7] py: cam.py: Fix multi camera capture without -C

[Laurent Pinchart]

On Fri, Jun 24, 2022 at 09:13:53AM +0100, Kieran Bingham wrote:
> Quoting Tomi Valkeinen (2022-06-23 15:47:30)
> > -C flag is supposed to affect only the camera that was previously
> > defined in the arguments. That's not the case, and, e.g.:
> > 
> > cam.py -c2 -C -c3
> > 
> > causes camera 3 to start capturing, but it stops after the initial
> > Requests have been completed.
> > 
> > Fix the issue by filtering out camera contexts that do not have -C
> > defined.
> > 
> > Signed-off-by: Tomi Valkeinen <tomi.valkeinen at ideasonboard.com>
> > ---
> >  src/py/cam/cam.py | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/src/py/cam/cam.py b/src/py/cam/cam.py
> > index 2ae89fa8..733e9ae5 100755
> > --- a/src/py/cam/cam.py
> > +++ b/src/py/cam/cam.py
> > @@ -434,6 +434,8 @@ def main():
> >          if args.info:
> >              ctx.do_cmd_info()
> 
> As the tools are often used for reference or guidance to newcomers it
> might be helpful to clarify this with a comment. Maybe the filtering is
> more clear to someone who writes a lot more python ... but from a C
> perspective it takes me a minute to identify what this line really
> does.
> 
>        # Filter out contexts which don't need to run a capture
> > +    contexts = [ctx for ctx in contexts if ctx.opt_capture > 0]
> > +
> >      if args.capture:

Maybe

    if contexts:

would be clearer ?

> Should this be done on a per-context basis? - Can cam.py already capture
> from multiple cameras at the same time?
> Maybe the rest is in the code below that I haven't read yet.
> 
> But the filtering itself sounds fine independently:
> 
> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

> >          state = CaptureState(cm, contexts)
> >  

-- 
Regards,

Laurent Pinchart