Fix Canon lens detection (Exiv2#2057)

Previously, when translating `Exif.CanonCs.LensType`,
`Exif.CanonCs.MaxAperture` was used as the upper bound to find a
potential match. In Exiv2#2057, the test file produced a value outside of
that range and the lens was not identified.

The fix uses `Exif.CanonCs.MinAperture` instead.

src/canonmn_int.cpp

@@ -2527,6 +2527,13 @@ std::ostream& printCsLensTypeByMetadata(std::ostream& os, const Value& value, co
     return printCsLensFFFF(os, value, metadata);
   }
 
+  // TODO: The lens identification could be improved further:
+  // 1. RF lenses also set Exif.CanonFi.RFLensType. If a lens cannot be found here then
+  //    the RF mechanism could be used instead.
+  // 2. Exif.Photo.LensModel and Exif.Canon.LensModel provide a text description of the lens
+  //    (e.g., "RF24-105mm F4-7.1 IS STM" - Note: no manufacturer or space after "RF").
+  //    After parsing, the values could be used to search in the lens array.
+
   // get the values we need from the metadata container
   ExifKey lensKey("Exif.CanonCs.Lens");
   auto pos = metadata->findKey(lensKey);
@@ -2540,14 +2547,14 @@ std::ostream& printCsLensTypeByMetadata(std::ostream& os, const Value& value, co
   auto const exifFlMin = static_cast<int>(static_cast<float>(pos->value().toInt64(1)) / pos->value().toFloat(2));
   auto const exifFlMax = static_cast<int>(static_cast<float>(pos->value().toInt64(0)) / pos->value().toFloat(2));
 
-  ExifKey aperKey("Exif.CanonCs.MaxAperture");
+  ExifKey aperKey("Exif.CanonCs.MinAperture");
   pos = metadata->findKey(aperKey);
   if (pos == metadata->end() || pos->value().count() != 1 || pos->value().typeId() != unsignedShort) {
     os << "Unknown Lens (" << lensType << ")";
     return os;
   }
 
-  auto exifAperMax = fnumber(canonEv(static_cast<int16_t>(pos->value().toInt64(0))));
+  auto exifAperMin = fnumber(canonEv(static_cast<int16_t>(pos->value().toInt64(0))));
 
   // regex to extract short and tele focal length, max aperture at short and tele position
   // and the teleconverter factor from the lens label
@@ -2588,8 +2595,8 @@ std::ostream& printCsLensTypeByMetadata(std::ostream& os, const Value& value, co
     auto aperMaxTele = std::stof(base_match[4].str()) * tc;
     auto aperMaxShort = base_match[3].length() > 0 ? std::stof(base_match[3].str()) * tc : aperMaxTele;
 
-    if (flMin != exifFlMin || flMax != exifFlMax || exifAperMax < (aperMaxShort - .1 * tc) ||
-        exifAperMax > (aperMaxTele + .1 * tc)) {
+    if (flMin != exifFlMin || flMax != exifFlMax || exifAperMin < (aperMaxShort - .1 * tc) ||
+        exifAperMin < (aperMaxTele + .1 * tc)) {
       continue;
     }
 

tests/bugfixes/github/test_issue_2057.py

@@ -0,0 +1,17 @@
+# -*- coding: utf-8 -*-
+
+from system_tests import CaseMeta, path
+
+class PanasonicMakerPrintAccelerometerIntOverflow(metaclass=CaseMeta):
+    """
+    Regression test for the bug described in:
+    https://github.com/Exiv2/exiv2/issues/2057
+    """
+    url = "https://github.com/Exiv2/exiv2/issues/2057"
+
+    filename = path("$data_path/issue_2057_poc1.exv")
+    commands = ["$exiv2 --Print kyyvt --key Exif.CanonCs.LensType $filename"]
+    stderr = [""]
+    stdout = ["""Exif.CanonCs.LensType                        Short      61182  Canon RF 24-105mm F4L IS USM *OR* Canon RF 24-105mm F4-7.1 IS STM
+"""]
+    retval = [0]

tests/lens_tests/test_canon_lenses.py

@@ -5,11 +5,9 @@
 from lens_tests.utils import extract_lenses_from_cpp, make_test_cases, aperture_to_raw_exif
 
 # 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.
+# Normally the canon makernote holds the minimum aperture used by the camera (in 
+# Exif.CanonCs.MinAperture). For the tests below, we set this tag to the minimum 
+# aperture of the lens.
 
 # get directory of the current file
 file_dir = os.path.dirname(os.path.realpath(__file__))
@@ -32,14 +30,14 @@
         {
             "filename": "$data_path/template.exv",
             "commands": [
-                '$exiv2 -M"set Exif.CanonCs.LensType $lens_id" -M"set Exif.CanonCs.Lens $focal_length_max $focal_length_min 1" -M"set Exif.CanonCs.MaxAperture $aperture_max" $filename && $exiv2 -pa -K Exif.CanonCs.LensType $filename'
+                '$exiv2 -M"set Exif.CanonCs.LensType $lens_id" -M"set Exif.CanonCs.Lens $focal_length_max $focal_length_min 1" -M"set Exif.CanonCs.MinAperture $aperture_min" $filename && $exiv2 -pa -K Exif.CanonCs.LensType $filename'
             ],
             "stderr": [""],
             "stdout": ["Exif.CanonCs.LensType                        Short       1  $lens_description\n"],
             "retval": [0],
             "lens_id": lens_tc["id"],
             "lens_description": lens_tc["target"],
-            "aperture_max": aperture_to_raw_exif(lens_tc["aperture_max_short"] * lens_tc["tc"]),
+            "aperture_min": aperture_to_raw_exif(lens_tc["aperture_min_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

@@ -119,10 +119,9 @@ def lens_is_match(l1, l2):
     """
     Test if lens l2 is compatible with lens l1
 
-    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.
+    This assumes we write l1's metadata and pick its 'aperture_min_short' value
+    as the minimum aperture value to write into exif.
+    Normally the canon makernote holds the minimum aperture of the camera.
     """
     # the problem is that the round trip transformation isn't exact
     # so we need to account for this here as well to not define a target
@@ -132,9 +131,9 @@ def lens_is_match(l1, l2):
         [
             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"] - 0.1) * l2["tc"]
-            <= reconstructed_aperture
-            <= (l2["aperture_max_tele"] + 0.1) * l2["tc"],
+            (l2["aperture_min_short"] - 0.1) * l2["tc"]
+            >= reconstructed_aperture
+            <= (l2["aperture_min_tele"] + 0.1) * l2["tc"],
         ]
     )