[libcamera-devel] [PATCH 1/3] utils: raspberrypi: ctt: load_image: Ignore JPEG files with no raw data

[David Plowman]

Hi Laurent

Thanks for the review!

On Wed, 6 Jul 2022 at 18:36, Laurent Pinchart
<laurent.pinchart at ideasonboard.com> wrote:
>
> Hi David and William,
>
> Thank you for the patch.
>
> On Wed, Jul 06, 2022 at 11:18:34AM +0100, David Plowman via libcamera-devel wrote:
> > From: William Vinnicombe <william.vinnicombe at raspberrypi.com>
> >
> > The load_image function would throw errors with JPEG or JPG files containing
> > no raw data.
> >
> > Prevent throwing these errors by returning 0 if an error has occurred.
> >
> > Signed-off-by: William Vinnicombe <william.vinnicombe at raspberrypi.com>
> > ---
> >  utils/raspberrypi/ctt/ctt_image_load.py | 5 +++++
> >  1 file changed, 5 insertions(+)
> >
> > diff --git a/utils/raspberrypi/ctt/ctt_image_load.py b/utils/raspberrypi/ctt/ctt_image_load.py
> > index 66adb237..934db123 100644
> > --- a/utils/raspberrypi/ctt/ctt_image_load.py
> > +++ b/utils/raspberrypi/ctt/ctt_image_load.py
> > @@ -358,6 +358,11 @@ def load_image(Cam, im_str, mac_config=None, show=False, mac=True, show_meta=Fal
> >              Img = dng_load_image(Cam, im_str)
> >          else:
> >              Img = brcm_load_image(Cam, im_str)
>
> David, are there open tools that produce BRCM JPEG files, or is this
> legacy code that could be removed ?

It's only the legacy camera stack on the Pi that produces such files.
But there are still folks using it, of course, as well as quite a lot
of stored files in this format (indeed I have quite a few myself from
pre-libcamera days!).

Thanks
David

>
> > +        """
> > +        handle errors smoothly if loading image failed
> > +        """
>
> One day I'll likely replace these comment blocks with real comments, but
> that's a candidate for a patch that will go through the whole code base
> in one go.
>
> > +        if Img == 0:
>
> Returning 0 on error is a peculiar pattern, a more pythonic way would be
> to return None, or raise an exception. That's also a candidate for a
> separate cleanup patch.
>
> Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
>
> > +            return 0
> >          if show_meta:
> >              Img.print_meta()
> >
>
> --
> Regards,
>
> Laurent Pinchart