[libcamera-devel,v3] utils: raspberrypi: ctt: dng_load_image: Work with DNG files from Picamera2

16924 diff mbox series

From: William Vinnicombe <william.vinnicombe@raspberrypi.com>

The DNG specification is based on the TIFF file format and recommends
storing the raw image data in a SubIFD and the Exif tags in an Exif IFD.
Other options are allowed, even if not recommended, such as storing both
the raw image data and the Exif data in IFD0, as done by the TIFF/EP
specification.

libcamera-apps use pyexiv2 to produce DNG files, following the DNG
recommendation, while applications based on picamera2 use PiDNG, which
adopts the TIFF/EP structure. Why it does so is not currently clear (see
https://github.com/schoolpost/PiDNG/issues/65 for discussions on this
topic), but as files based on the DNG and TIFF/EP variants exist in the
wild, both need to be supported by ctt.

Add code to identify which tags are being used, and then load the
metadata from the correct tags.

Signed-off-by: William Vinnicombe <william.vinnicombe@raspberrypi.com>
Reviewed-by: David Plowman <david.plowman@raspberrypi.com>
Reviewed-by: Naushir Patuck <naush@raspberrypi.com>
---
 utils/raspberrypi/ctt/ctt_image_load.py | 32 +++++++++++++++++++------
 1 file changed, 25 insertions(+), 7 deletions(-)

Hi William,

Thank you for the patch.

On Wed, Aug 03, 2022 at 09:25:39AM +0100, William Vinnicombe via libcamera-devel wrote:
> From: William Vinnicombe <william.vinnicombe@raspberrypi.com>
> 
> The DNG specification is based on the TIFF file format and recommends
> storing the raw image data in a SubIFD and the Exif tags in an Exif IFD.
> Other options are allowed, even if not recommended, such as storing both
> the raw image data and the Exif data in IFD0, as done by the TIFF/EP
> specification.
> 
> libcamera-apps use pyexiv2 to produce DNG files, following the DNG
> recommendation, while applications based on picamera2 use PiDNG, which
> adopts the TIFF/EP structure. Why it does so is not currently clear (see
> https://github.com/schoolpost/PiDNG/issues/65 for discussions on this
> topic), but as files based on the DNG and TIFF/EP variants exist in the
> wild, both need to be supported by ctt.
> 
> Add code to identify which tags are being used, and then load the
> metadata from the correct tags.
> 
> Signed-off-by: William Vinnicombe <william.vinnicombe@raspberrypi.com>
> Reviewed-by: David Plowman <david.plowman@raspberrypi.com>
> Reviewed-by: Naushir Patuck <naush@raspberrypi.com>

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

> ---
>  utils/raspberrypi/ctt/ctt_image_load.py | 32 +++++++++++++++++++------
>  1 file changed, 25 insertions(+), 7 deletions(-)
> 
> diff --git a/utils/raspberrypi/ctt/ctt_image_load.py b/utils/raspberrypi/ctt/ctt_image_load.py
> index 934db123..7f8ba72a 100644
> --- a/utils/raspberrypi/ctt/ctt_image_load.py
> +++ b/utils/raspberrypi/ctt/ctt_image_load.py
> @@ -301,17 +301,35 @@ def dng_load_image(Cam, im_str):
>          metadata.read()
>  
>          Img.ver = 100  # random value
> -        Img.w = metadata['Exif.SubImage1.ImageWidth'].value
> +        """
> +        The DNG and TIFF/EP specifications use different IFDs to store the raw
> +        image data and the Exif tags. DNG stores them in a SubIFD and in an Exif
> +        IFD respectively (named "SubImage1" and "Photo" by pyexiv2), while
> +        TIFF/EP stores them both in IFD0 (name "Image"). Both are used in "DNG"
> +        files, with libcamera-apps following the DNG recommendation and
> +        applications based on picamera2 following TIFF/EP.
> +
> +        This code detects which tags are being used, and therefore extracts the
> +        correct values.
> +        """
> +        try:
> +            Img.w = metadata['Exif.SubImage1.ImageWidth'].value
> +            subimage = "SubImage1"
> +            photo = "Photo"
> +        except KeyError:
> +            Img.w = metadata['Exif.Image.ImageWidth'].value
> +            subimage = "Image"
> +            photo = "Image"
>          Img.pad = 0
> -        Img.h = metadata['Exif.SubImage1.ImageLength'].value
> -        white = metadata['Exif.SubImage1.WhiteLevel'].value
> +        Img.h = metadata[f'Exif.{subimage}.ImageLength'].value
> +        white = metadata[f'Exif.{subimage}.WhiteLevel'].value
>          Img.sigbits = int(white).bit_length()
>          Img.fmt = (Img.sigbits - 4) // 2
> -        Img.exposure = int(metadata['Exif.Photo.ExposureTime'].value*1000000)
> -        Img.againQ8 = metadata['Exif.Photo.ISOSpeedRatings'].value*256/100
> +        Img.exposure = int(metadata[f'Exif.{photo}.ExposureTime'].value*1000000)
> +        Img.againQ8 = metadata[f'Exif.{photo}.ISOSpeedRatings'].value*256/100
>          Img.againQ8_norm = Img.againQ8 / 256
>          Img.camName = metadata['Exif.Image.Model'].value
> -        Img.blacklevel = int(metadata['Exif.SubImage1.BlackLevel'].value[0])
> +        Img.blacklevel = int(metadata[f'Exif.{subimage}.BlackLevel'].value[0])
>          Img.blacklevel_16 = Img.blacklevel << (16 - Img.sigbits)
>          bayer_case = {
>              '0 1 1 2': (0, (0, 1, 2, 3)),
> @@ -319,7 +337,7 @@ def dng_load_image(Cam, im_str):
>              '2 1 1 0': (2, (3, 2, 1, 0)),
>              '1 0 2 1': (3, (1, 0, 3, 2))
>          }
> -        cfa_pattern = metadata['Exif.SubImage1.CFAPattern'].value
> +        cfa_pattern = metadata[f'Exif.{subimage}.CFAPattern'].value
>          Img.pattern = bayer_case[cfa_pattern][0]
>          Img.order = bayer_case[cfa_pattern][1]
>

On Wed, Aug 03, 2022 at 02:30:38PM +0300, Laurent Pinchart wrote:
> Hi William,
> 
> Thank you for the patch.
> 
> On Wed, Aug 03, 2022 at 09:25:39AM +0100, William Vinnicombe via libcamera-devel wrote:
> > From: William Vinnicombe <william.vinnicombe@raspberrypi.com>
> > 
> > The DNG specification is based on the TIFF file format and recommends
> > storing the raw image data in a SubIFD and the Exif tags in an Exif IFD.
> > Other options are allowed, even if not recommended, such as storing both
> > the raw image data and the Exif data in IFD0, as done by the TIFF/EP
> > specification.
> > 
> > libcamera-apps use pyexiv2 to produce DNG files, following the DNG
> > recommendation, while applications based on picamera2 use PiDNG, which
> > adopts the TIFF/EP structure. Why it does so is not currently clear (see
> > https://github.com/schoolpost/PiDNG/issues/65 for discussions on this
> > topic), but as files based on the DNG and TIFF/EP variants exist in the
> > wild, both need to be supported by ctt.
> > 
> > Add code to identify which tags are being used, and then load the
> > metadata from the correct tags.
> > 
> > Signed-off-by: William Vinnicombe <william.vinnicombe@raspberrypi.com>
> > Reviewed-by: David Plowman <david.plowman@raspberrypi.com>
> > Reviewed-by: Naushir Patuck <naush@raspberrypi.com>
> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
> > ---
> >  utils/raspberrypi/ctt/ctt_image_load.py | 32 +++++++++++++++++++------
> >  1 file changed, 25 insertions(+), 7 deletions(-)
> > 
> > diff --git a/utils/raspberrypi/ctt/ctt_image_load.py b/utils/raspberrypi/ctt/ctt_image_load.py
> > index 934db123..7f8ba72a 100644
> > --- a/utils/raspberrypi/ctt/ctt_image_load.py
> > +++ b/utils/raspberrypi/ctt/ctt_image_load.py
> > @@ -301,17 +301,35 @@ def dng_load_image(Cam, im_str):
> >          metadata.read()
> >  
> >          Img.ver = 100  # random value
> > -        Img.w = metadata['Exif.SubImage1.ImageWidth'].value
> > +        """
> > +        The DNG and TIFF/EP specifications use different IFDs to store the raw
> > +        image data and the Exif tags. DNG stores them in a SubIFD and in an Exif
> > +        IFD respectively (named "SubImage1" and "Photo" by pyexiv2), while
> > +        TIFF/EP stores them both in IFD0 (name "Image"). Both are used in "DNG"
> > +        files, with libcamera-apps following the DNG recommendation and
> > +        applications based on picamera2 following TIFF/EP.
> > +
> > +        This code detects which tags are being used, and therefore extracts the
> > +        correct values.
> > +        """
> > +        try:
> > +            Img.w = metadata['Exif.SubImage1.ImageWidth'].value
> > +            subimage = "SubImage1"
> > +            photo = "Photo"
> > +        except KeyError:
> > +            Img.w = metadata['Exif.Image.ImageWidth'].value
> > +            subimage = "Image"
> > +            photo = "Image"
> >          Img.pad = 0
> > -        Img.h = metadata['Exif.SubImage1.ImageLength'].value
> > -        white = metadata['Exif.SubImage1.WhiteLevel'].value
> > +        Img.h = metadata[f'Exif.{subimage}.ImageLength'].value
> > +        white = metadata[f'Exif.{subimage}.WhiteLevel'].value
> >          Img.sigbits = int(white).bit_length()
> >          Img.fmt = (Img.sigbits - 4) // 2
> > -        Img.exposure = int(metadata['Exif.Photo.ExposureTime'].value*1000000)
> > -        Img.againQ8 = metadata['Exif.Photo.ISOSpeedRatings'].value*256/100
> > +        Img.exposure = int(metadata[f'Exif.{photo}.ExposureTime'].value*1000000)
> > +        Img.againQ8 = metadata[f'Exif.{photo}.ISOSpeedRatings'].value*256/100

Actually, checkstyle.py reports

--------------------------------------------------------------------------------------------------------------------
31444bed873fbafa1bfa66d76f34044ace09f7d5 utils: raspberrypi: ctt: dng_load_image: Work with DNG files from Picamera2
--------------------------------------------------------------------------------------------------------------------
--- utils/raspberrypi/ctt/ctt_image_load.py
+++ utils/raspberrypi/ctt/ctt_image_load.py
#328: : E226 missing whitespace around arithmetic operator
+        Img.exposure = int(metadata[f'Exif.{photo}.ExposureTime'].value*1000000)
#329: : E226 missing whitespace around arithmetic operator
+        Img.againQ8 = metadata[f'Exif.{photo}.ISOSpeedRatings'].value*256/100
#329: : E226 missing whitespace around arithmetic operator
+        Img.againQ8 = metadata[f'Exif.{photo}.ISOSpeedRatings'].value*256/100
---
3 potential issues detected, please review

I'll fix this by adding the missing spaces.

Could you install the libcamera git post-commit hook that runs
checkstyle.py automatically ?

$ cp utils/hooks/post-commit .git/hooks/

> >          Img.againQ8_norm = Img.againQ8 / 256
> >          Img.camName = metadata['Exif.Image.Model'].value
> > -        Img.blacklevel = int(metadata['Exif.SubImage1.BlackLevel'].value[0])
> > +        Img.blacklevel = int(metadata[f'Exif.{subimage}.BlackLevel'].value[0])
> >          Img.blacklevel_16 = Img.blacklevel << (16 - Img.sigbits)
> >          bayer_case = {
> >              '0 1 1 2': (0, (0, 1, 2, 3)),
> > @@ -319,7 +337,7 @@ def dng_load_image(Cam, im_str):
> >              '2 1 1 0': (2, (3, 2, 1, 0)),
> >              '1 0 2 1': (3, (1, 0, 3, 2))
> >          }
> > -        cfa_pattern = metadata['Exif.SubImage1.CFAPattern'].value
> > +        cfa_pattern = metadata[f'Exif.{subimage}.CFAPattern'].value
> >          Img.pattern = bayer_case[cfa_pattern][0]
> >          Img.order = bayer_case[cfa_pattern][1]
> >