Revert "Fix Canon lens detection (Exiv2#2057)"

This reverts commit d8d5a3a.

Revert to fix the lens detection in a different way.

src/canonmn_int.cpp

@@ -2527,13 +2527,6 @@ 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);
@@ -2547,14 +2540,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.MinAperture");
+  ExifKey aperKey("Exif.CanonCs.MaxAperture");
   pos = metadata->findKey(aperKey);
   if (pos == metadata->end() || pos->value().count() != 1 || pos->value().typeId() != unsignedShort) {
     os << "Unknown Lens (" << lensType << ")";
     return os;
   }
 
-  auto exifAperMin = fnumber(canonEv(static_cast<int16_t>(pos->value().toInt64(0))));
+  auto exifAperMax = 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
@@ -2595,8 +2588,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 || exifAperMin < (aperMaxShort - .1 * tc) ||
-        exifAperMin < (aperMaxTele + .1 * tc)) {
+    if (flMin != exifFlMin || flMax != exifFlMax || exifAperMax < (aperMaxShort - .1 * tc) ||
+        exifAperMax > (aperMaxTele + .1 * tc)) {
       continue;
     }
 

tests/lens_tests/test_canon_lenses.py

@@ -5,9 +5,11 @@
 from lens_tests.utils import extract_lenses_from_cpp, make_test_cases, aperture_to_raw_exif
 
 # NOTE
-# 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.
+# 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__))
@@ -30,14 +32,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.MinAperture $aperture_min" $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.MaxAperture $aperture_max" $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_min": aperture_to_raw_exif(lens_tc["aperture_min_short"] * lens_tc["tc"]),
+            "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

@@ -119,9 +119,10 @@ 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_min_short' value
-    as the minimum aperture value to write into exif.
-    Normally the canon makernote holds the minimum aperture of the camera.
+    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.
     """
     # 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
@@ -131,9 +132,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_min_short"] - 0.1) * l2["tc"]
-            >= reconstructed_aperture
-            <= (l2["aperture_min_tele"] + 0.1) * l2["tc"],
+            (l2["aperture_max_short"] - 0.1) * l2["tc"]
+            <= reconstructed_aperture
+            <= (l2["aperture_max_tele"] + 0.1) * l2["tc"],
         ]
     )