feat: improve lens recognition of canon makernote

If multiple choices are possible they are now all reported. This
behaviour is now the same as it is in exiftool.

All lenses are tested in the new test_canon_lenses.py test

tests/lens_tests/test_canon_lenses.py

@@ -1,10 +1,15 @@
 # -*- coding: utf-8 -*-
-
 import re
 import os
 import system_tests
-import math
+from lens_tests.utils import extract_lenses_from_cpp, make_test_cases, aperture_to_raw_exif
-from lens_tests.utils import extract_lenses_from_cpp, make_test_cases
+
+# NOTE
+# Normally the canon maker note holds the max aperture of the lens at the focal length
+# the picture was taken at. Thus for a f/4-6.3 lens, this value could be anywhere in that range.
+# For the below tests we only test the scenario where the lens was used at it's shortest focal length.
+# Thus we always pick the 'aperture_max_short' of a lens as the value to write into the
+# Exif.CanonCs.MaxAperture field.
 
 # get directory of the current file
 file_dir = os.path.dirname(os.path.realpath(__file__))
@@ -17,31 +22,6 @@ lenses = extract_lenses_from_cpp(canon_lens_file, startpattern)
 # use utils function to define test case data
 test_cases = make_test_cases(lenses)
 
-# see https://github.com/exiftool/exiftool/blob/master/lib/Image/ExifTool/Canon.pm#L9678
-def aperture_to_raw_exif(aperture):
-    # for apertures < 1 the below is negative
-    num = math.log(aperture) * 2 / math.log(2)
-
-    # temporarily make the number positive
-    if num < 0:
-        num = -num
-        sign = -1
-    else:
-        sign = 1
-
-    val = int(num)
-    frac = num - val
-
-    if abs(frac - 0.33) < 0.05:
-        frac = 0x0C
-    elif abs(frac - 0.67) < 0.05:
-        frac = 0x14
-    else:
-        frac = int(frac * 0x20 + 0.5)
-
-    return sign * (val * 0x20 + frac)
-
-
 for lens_tc in test_cases:
 
     testname = lens_tc["id"] + "_" + lens_tc["desc"]
@@ -59,7 +39,7 @@ for lens_tc in test_cases:
             "retval": [0],
             "lens_id": lens_tc["id"],
             "lens_description": lens_tc["target"],
-            "aperture_max": aperture_to_raw_exif(lens_tc["aperture_max_short"]),
+            "aperture_max": aperture_to_raw_exif(lens_tc["aperture_max_short"] * lens_tc["tc"]),
             "focal_length_min": int(lens_tc["focal_length_min"] * lens_tc["tc"]),
             "focal_length_max": int(lens_tc["focal_length_max"] * lens_tc["tc"]),
         },

tests/lens_tests/utils.py

@@ -1,6 +1,7 @@
 import re
 import os
 import logging
+import math
 from itertools import groupby
 
 log = logging.getLogger(__name__)
@@ -12,11 +13,11 @@ LENS_META_DEFAULT_RE = re.compile(
     (
         # anything at the start
         ".*?"
-        # maybe min focal length and hyhpen, surely max focal length e.g.: 24-70mm
+        # maybe min focal length and hyphen, surely max focal length e.g.: 24-70mm
         "(?:(?P<focal_length_min>[0-9]+)-)?(?P<focal_length_max>[0-9]+)mm"
-        # anything inbetween
+        # anything in-between
         ".*?"
-        # maybe short focal length max aperture and hyhpen, surely at least single max aperture e.g.: f/4.5-5.6
+        # maybe short focal length max aperture and hyphen, surely at least single max aperture e.g.: f/4.5-5.6
         # short and tele indicate apertures at the short (focal_length_min) and tele (focal_length_max) position of the lens
         "(?:(?:f\/)|T)(?:(?P<aperture_max_short>[0-9]+(?:\.[0-9]+)?)-)?(?P<aperture_max_tele>[0-9]+(?:\.[0-9])?)"
         # check if there is a teleconverter pattern e.g. + 1.4x
@@ -25,9 +26,58 @@ LENS_META_DEFAULT_RE = re.compile(
 )
 
 
+def aperture_to_raw_exif(aperture):
+    # see https://github.com/exiftool/exiftool/blob/master/lib/Image/ExifTool/Canon.pm#L9678
+    """Transform aperture value to Canon maker note style hex format."""
+    # for apertures < 1 the below is negative
+    num = math.log(aperture) * 2 / math.log(2)
+
+    # temporarily make the number positive
+    if num < 0:
+        num = -num
+        sign = -1
+    else:
+        sign = 1
+
+    val = int(num)
+    frac = num - val
+
+    if abs(frac - 0.33) < 0.05:
+        frac = 0x0C
+    elif abs(frac - 0.67) < 0.05:
+        frac = 0x14
+    else:
+        frac = int(frac * 0x20 + 0.5)
+
+    return sign * (val * 0x20 + frac)
+
+
+def raw_exif_to_aperture(raw):
+    """The inverse operation of aperture_to_raw_exif"""
+    val = raw
+    if val < 0:
+        val = -val
+        sign = -1
+    else:
+        sign = 1
+
+    frac = val & 0x1F
+    val -= frac
+    # Convert 1/3 and 2/3 codes
+    if frac == 0x0C:
+        frac = 0x20 / 3
+    elif frac == 0x14:
+        frac = 0x40 / 3
+
+    ev = sign * (val + frac) / 0x20
+    return math.exp(ev * math.log(2) / 2)
+
+
 def parse_lens_entry(text, pattern=LENS_ENTRY_DEFAULT_RE):
-    """get the ID, and description from a lens entry field
+    """
-    Expexted input format:
+    get the ID, and description from a lens entry field
+
+    Expected input format:
     { 748, "Canon EF 100-400mm f/4.5-5.6L IS II USM + 1.4x" }
     We return a dict of:
     lens_id = 748
@@ -40,6 +90,7 @@ def parse_lens_entry(text, pattern=LENS_ENTRY_DEFAULT_RE):
 def extract_meta(text, pattern=LENS_META_DEFAULT_RE):
     """
     Extract metadata from lens description.
+
     Input expected in the form of e.g. "Canon EF 100-400mm f/4.5-5.6L IS II USM + 1.4x"
     We return a dict of:
     focal_length_min = 100
@@ -64,16 +115,27 @@ def extract_meta(text, pattern=LENS_META_DEFAULT_RE):
     return ret
 
 
-# FIXME explain somwhere that lens_is_match(l1,l2) does not imply lens_is_match(l2,l1)
-# becuse we don't have short and tele aperture values in exif
 def lens_is_match(l1, l2):
     """
-    Test if lens l2 is compatible with lens l1,
+    Test if lens l2 is compatible with lens l1
-    assuming we write l1's metadata and apeture_max_short into exif
+
+    This assumes we write l1's metadata and pick its 'aperture_max_short' value
+    as the maximum aperture value to write into exif.
+    Normally the canon maker note holds the max aperture of the lens at the focal length
+    the picture was taken at. Thus for a f/4-6.3 lens, this value could be anywhere in that range.
     """
-    return (
+    # the problem is that the round trip transformation isn't exact
-        all([l1[k] == l2[k] for k in ["tc", "focal_length_min", "focal_length_max"]])
+    # so we need to account for this here as well to not define a target
-        and l2["aperture_max_short"] <= l1["aperture_max_short"] <= l2["aperture_max_tele"]
+    # which isn't achievable for exiv2
+    reconstructed_aperture = raw_exif_to_aperture(aperture_to_raw_exif(l1["aperture_max_short"] * l1["tc"]))
+    return all(
+        [
+            l1["focal_length_min"] * l1["tc"] == l2["focal_length_min"] * l2["tc"],
+            l1["focal_length_max"] * l1["tc"] == l2["focal_length_max"] * l2["tc"],
+            (l2["aperture_max_short"] * l2["tc"]) - 0.1
+            <= reconstructed_aperture
+            <= (l2["aperture_max_tele"] * l2["tc"]) + 0.1,
+        ]
     )
 
 
@@ -100,7 +162,7 @@ def make_test_cases(lenses):
 
 def extract_lenses_from_cpp(filename, start_pattern):
     """
-    Extract lens information from the lens descritpions array in a maker note cpp file
+    Extract lens information from the lens descriptions array in a maker note cpp file
     filename: path to cpp file
     start_pattern: start_pattern == line.strip() should return True for
                    the starting line of the array containing the lenses.
@@ -134,7 +196,7 @@ def extract_lenses_from_cpp(filename, start_pattern):
 
                 meta = extract_meta(lens_entry[1])
                 if not meta:
-                    log.error(f"Failure extracing metadata from lens description:  {lens_entry[0]}: {lens_entry[1]}.")
+                    log.error(f"Failure extracting metadata from lens description:  {lens_entry[0]}: {lens_entry[1]}.")
                     continue
 
                 lenses.append({"id": lens_entry[0], "desc": lens_entry[1], "meta": meta})