Test all Canon lenses (backport to 0.27-maintenance) #1429
Conversation
The mathematical calculation of fnumbers does not always match the expected values: For example for f/3.5 the precise mathematical value is 3.564..., which gets rounded to 3.6. Fix this special case by returning a value closer to the expected value.
When searching for the Tamron lens, only the string "300mm" is searched in the lens description, which also happens to be present for the Canon lens. Since the Canon lens comes first in the list, it wins. Fix this issue by prefixing the search string with a single space so it always has to match the full focal length specification.
….5-5.6 When searching for the Sigma lens, only the string "5" is searched in the lens description, which also happens to be present for the Canon lens. Since the Canon lens comes first in the list, it wins. Fix this issue by prefixing the search with f/ so it always has to match the beginning of the aperture specification. This will still allow mismatches for example when searching for f/4 that also matches f/4.5, but no such issues occur at the moment, so the simple fix is sufficient for now.
Lenses with and without a TC may share the same lens ID. Prefer entries that explicitly mention the TC.
Fixes some small inconsistencies, so that all lenses use the same format, that is also shared with other lens databases such as lensfun: * Always prefix aperture with f/ * Always put L suffix directly after aperture
Lenses that have the exact same ID, focal length and aperture as some other lens that comes earlier in the list (and thus always wins): * 103, "Rokinon AF 14mm f/2.8 EF" * 137, "Tamron SP 17-50mm f/2.8 XR Di II VC" * 137, "Tamron SP 24-70mm f/2.8 Di VC USD" * 161, "Sigma 28-70mm f/2.8 EX" * 161, "Tokina AT-X 24-70mm f/2.8 PRO FX (IF)" * 173, "Sigma 180mm EX HSM Macro f/3.5" * 174, "Sigma 120-300mm f/2.8 DG OS HSM S013" * 180, "Zeiss Milvus 50mm f/1.4" * 180, "Tokina Opera 50mm f/1.4 FF" * 183, "Sigma 150-600mm f/5-6.3 DG OS HSM | S" * 239, "Rokinon SP 85mm f/1.2" * 251, "Canon EF 70-200mm f/2.8L IS III USM" * 252, "Canon EF 70-200mm f/2.8L IS III USM + 1.4x" * 253, "Canon EF 70-200mm f/2.8L IS III USM + 2x" * 368, "Sigma 14-24mm f/2.8 DG HSM" * 495, "Sigma 24-70mm f/2.8 DG OS HSM | A" * 748, "Tamron 100-400mm f/4.5-6.3 Di VC USD + 1.4x" Lenses that have the exact same ID and focal length as some other lens that comes earlier in the list, and an unsupported aperture that cannot be used to distinguish the lenses: * 103, "Rokinon SP 14mm f/2.4" * 169, "Sigma 35mm f/1.5 FF High-Speed Prime | 017" * 180, "Sigma 24mm f/1.5 FF High-Speed Prime | 017" * 180, "Sigma 50mm f/1.5 FF High-Speed Prime | 017" * 180, "Sigma 85mm f/1.5 FF High-Speed Prime | 017" * 250, "Sigma 20mm f/1.5 FF High-Speed Prime | 017" Lenses that share their IDs with other lenses, but have no or an unsupported focal length: * 33, "Voigtlander or Carl Zeiss Lens" * 131, "Sigma 4.5mm f/2.8 EX DC HSM Circular Fisheye"
There is no need to handle tests on Windows and Unix differently here. Always using a shell allows for more flexibility when writing tests.
Generates a test case for every known lens from canonCsLensType, that first sets the corresponding lens metadata and then verifies that exiv2 maps it to the expected lens description. Only metadata fields that are relevant for lens identification are modified.
webmeister
changed the title
There was a problem hiding this comment.
@webmeister This looks amazing. Excellent work. Thank You very much. And you've delivered very quickly indeed. Great Job.
I've had another long day, so I'm not going to push 'Approve' when I am tired. I'd like to step through your changes and leave it half a day to see something occurs to me later. So I expect to approve this on Thursday afternoon.
A couple of questions spring instantly to mind:
- My question earlier about the dependencies and order of submitting your PRs.
- It's a pity to have all that code about the lenses duplicated in the C++ and python code. There's a program samples/taglist which prints all the TagInfo structures and that's then used to generate web-pages for exiv2.org. What do think about having samples/lenslist.cpp to do something similar. At first, it only need to reveal this Canon magic and you can call it from python. We don't need to generate web-pages yet - however we're preparing for that possibility.
Thank You again for a very impressive contribution. Speak Tomorrow.
There was a problem hiding this comment.
This looks really good. And like all good work, there are ways to make it even better.
1 The Fnumber
Good catch, both the rounding and presentation in the "pretty printer".
I'm nervous about the pretty printer. When we change anything in the presentation, somebody always complains!
The rounding code is married to 3.5. I should round to every fnumber to 1 decimal place. It's something like f = ((int)(f*10.0+0.5))/10.0); https://www.geeksforgeeks.org/rounding-floating-point-number-two-decimal-places-c-c/
We should look at how the Fnumber is converted to rational when it's parsed.
705 rmills@rmillsmm-local:~/temp $ exiv2 -M'set Exif.Photo.FNumber f4.2' ~/Stonehenge.jpg
706 rmills@rmillsmm-local:~/temp $ exiv2 --grep FNumber ~/Stonehenge.jpg
Exif.Photo.FNumber Rational 1 F4.1
707 rmills@rmillsmm-local:~/temp $ For sure, there's a rounding presentation error. However perhaps we should be rounding the Fnumber when it arrives and storing something slightly different.
More to the point is that if we present the data as f/3.5, we should parse f/3.5 and that will require changing the man page.
I think we should retain the F3.5 presentation (parsing and man page) and fix the rounding (both in the pretty printer and when parsing).
I think the f/3.5 should go into 'master' (and you'll have to update exiv2.1 and the input parser).
- template.exv
Ingeneous. Can you name that test_canon_lenses.exv to make it obvious where this is being used.
Backport of #1428 to the 0.27-maintenance branch.
The general structure is the same, but two new fixes were necessary on this branch, see 5ff03d6 and bf8dfce.