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

16562 diff mbox series

From: William Vinnicombe <william.vinnicombe@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@raspberrypi.com>
---
 utils/raspberrypi/ctt/ctt_image_load.py | 5 +++++
 1 file changed, 5 insertions(+)

Hi William

Thanks for this patch.

On Wed, 6 Jul 2022 at 11:18, David Plowman
<david.plowman@raspberrypi.com> wrote:
>
> From: William Vinnicombe <william.vinnicombe@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@raspberrypi.com>

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

Thanks!
David

> ---
>  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)
> +        """
> +        handle errors smoothly if loading image failed
> +        """
> +        if Img == 0:
> +            return 0
>          if show_meta:
>              Img.print_meta()
>
> --
> 2.30.2
>

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@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@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 ?

> +        """
> +        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@ideasonboard.com>

> +            return 0
>          if show_meta:
>              Img.print_meta()
>

Hi Laurent

Thanks for the review!

On Wed, 6 Jul 2022 at 18:36, Laurent Pinchart
<laurent.pinchart@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@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@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@ideasonboard.com>
>
> > +            return 0
> >          if show_meta:
> >              Img.print_meta()
> >
>
> --
> Regards,
>
> Laurent Pinchart