Skip to content

Commit 315ae6d

Browse files
Revert "Fix Canon lens detection (#2057)"
This reverts commit d8d5a3a. Revert to fix the lens detection in a different way.
1 parent d8d5a3a commit 315ae6d

File tree

6 files changed

+18
-529
lines changed

6 files changed

+18
-529
lines changed

src/canonmn_int.cpp

+4-11
Original file line numberDiff line numberDiff line change
@@ -2527,13 +2527,6 @@ std::ostream& printCsLensTypeByMetadata(std::ostream& os, const Value& value, co
25272527
return printCsLensFFFF(os, value, metadata);
25282528
}
25292529

2530-
// TODO: The lens identification could be improved further:
2531-
// 1. RF lenses also set Exif.CanonFi.RFLensType. If a lens cannot be found here then
2532-
// the RF mechanism could be used instead.
2533-
// 2. Exif.Photo.LensModel and Exif.Canon.LensModel provide a text description of the lens
2534-
// (e.g., "RF24-105mm F4-7.1 IS STM" - Note: no manufacturer or space after "RF").
2535-
// After parsing, the values could be used to search in the lens array.
2536-
25372530
// get the values we need from the metadata container
25382531
ExifKey lensKey("Exif.CanonCs.Lens");
25392532
auto pos = metadata->findKey(lensKey);
@@ -2547,14 +2540,14 @@ std::ostream& printCsLensTypeByMetadata(std::ostream& os, const Value& value, co
25472540
auto const exifFlMin = static_cast<int>(static_cast<float>(pos->value().toInt64(1)) / pos->value().toFloat(2));
25482541
auto const exifFlMax = static_cast<int>(static_cast<float>(pos->value().toInt64(0)) / pos->value().toFloat(2));
25492542

2550-
ExifKey aperKey("Exif.CanonCs.MinAperture");
2543+
ExifKey aperKey("Exif.CanonCs.MaxAperture");
25512544
pos = metadata->findKey(aperKey);
25522545
if (pos == metadata->end() || pos->value().count() != 1 || pos->value().typeId() != unsignedShort) {
25532546
os << "Unknown Lens (" << lensType << ")";
25542547
return os;
25552548
}
25562549

2557-
auto exifAperMin = fnumber(canonEv(static_cast<int16_t>(pos->value().toInt64(0))));
2550+
auto exifAperMax = fnumber(canonEv(static_cast<int16_t>(pos->value().toInt64(0))));
25582551

25592552
// regex to extract short and tele focal length, max aperture at short and tele position
25602553
// and the teleconverter factor from the lens label
@@ -2595,8 +2588,8 @@ std::ostream& printCsLensTypeByMetadata(std::ostream& os, const Value& value, co
25952588
auto aperMaxTele = std::stof(base_match[4].str()) * tc;
25962589
auto aperMaxShort = base_match[3].length() > 0 ? std::stof(base_match[3].str()) * tc : aperMaxTele;
25972590

2598-
if (flMin != exifFlMin || flMax != exifFlMax || exifAperMin < (aperMaxShort - .1 * tc) ||
2599-
exifAperMin < (aperMaxTele + .1 * tc)) {
2591+
if (flMin != exifFlMin || flMax != exifFlMax || exifAperMax < (aperMaxShort - .1 * tc) ||
2592+
exifAperMax > (aperMaxTele + .1 * tc)) {
26002593
continue;
26012594
}
26022595

test/data/issue_2057_poc1.exv

-21.7 KB
Binary file not shown.

test/data/test_reference_files/issue_2057_poc1.exv.out

-490
This file was deleted.

tests/bugfixes/github/test_issue_2057.py

-17
This file was deleted.

tests/lens_tests/test_canon_lenses.py

+7-5
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,11 @@
55
from lens_tests.utils import extract_lenses_from_cpp, make_test_cases, aperture_to_raw_exif
66

77
# NOTE
8-
# Normally the canon makernote holds the minimum aperture used by the camera (in
9-
# Exif.CanonCs.MinAperture). For the tests below, we set this tag to the minimum
10-
# aperture of the lens.
8+
# Normally the canon maker note holds the max aperture of the lens at the focal length
9+
# the picture was taken at. Thus for a f/4-6.3 lens, this value could be anywhere in that range.
10+
# For the below tests we only test the scenario where the lens was used at it's shortest focal length.
11+
# Thus we always pick the 'aperture_max_short' of a lens as the value to write into the
12+
# Exif.CanonCs.MaxAperture field.
1113

1214
# get directory of the current file
1315
file_dir = os.path.dirname(os.path.realpath(__file__))
@@ -30,14 +32,14 @@
3032
{
3133
"filename": "$data_path/template.exv",
3234
"commands": [
33-
'$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'
35+
'$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'
3436
],
3537
"stderr": [""],
3638
"stdout": ["Exif.CanonCs.LensType Short 1 $lens_description\n"],
3739
"retval": [0],
3840
"lens_id": lens_tc["id"],
3941
"lens_description": lens_tc["target"],
40-
"aperture_min": aperture_to_raw_exif(lens_tc["aperture_min_short"] * lens_tc["tc"]),
42+
"aperture_max": aperture_to_raw_exif(lens_tc["aperture_max_short"] * lens_tc["tc"]),
4143
"focal_length_min": int(lens_tc["focal_length_min"] * lens_tc["tc"]),
4244
"focal_length_max": int(lens_tc["focal_length_max"] * lens_tc["tc"]),
4345
},

tests/lens_tests/utils.py

+7-6
Original file line numberDiff line numberDiff line change
@@ -119,9 +119,10 @@ def lens_is_match(l1, l2):
119119
"""
120120
Test if lens l2 is compatible with lens l1
121121
122-
This assumes we write l1's metadata and pick its 'aperture_min_short' value
123-
as the minimum aperture value to write into exif.
124-
Normally the canon makernote holds the minimum aperture of the camera.
122+
This assumes we write l1's metadata and pick its 'aperture_max_short' value
123+
as the maximum aperture value to write into exif.
124+
Normally the canon maker note holds the max aperture of the lens at the focal length
125+
the picture was taken at. Thus for a f/4-6.3 lens, this value could be anywhere in that range.
125126
"""
126127
# the problem is that the round trip transformation isn't exact
127128
# 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):
131132
[
132133
l1["focal_length_min"] * l1["tc"] == l2["focal_length_min"] * l2["tc"],
133134
l1["focal_length_max"] * l1["tc"] == l2["focal_length_max"] * l2["tc"],
134-
(l2["aperture_min_short"] - 0.1) * l2["tc"]
135-
>= reconstructed_aperture
136-
<= (l2["aperture_min_tele"] + 0.1) * l2["tc"],
135+
(l2["aperture_max_short"] - 0.1) * l2["tc"]
136+
<= reconstructed_aperture
137+
<= (l2["aperture_max_tele"] + 0.1) * l2["tc"],
137138
]
138139
)
139140

0 commit comments

Comments
 (0)