Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

correct identification for Tamron SP 24-70mm G2 A032 #1691

Merged
merged 3 commits into from
Jun 10, 2021
Merged

Conversation

nulllinie
Copy link
Contributor

@nulllinie nulllinie commented Jun 6, 2021

SP 24-70mm F/2.8 Di VC USD G2 has a different identcode
from exiftool: Composite:LensID CE 47 37 5C 25 25 DF 0E
example image (Nikon raw) with this lens:
exampleImage.zip

SP 24-70mm F/2.8 Di VC USD G2 has a different identcode
from exiftool: Composite:LensID CE 47 37 5C 25 25 DF 0E
@codecov
Copy link

codecov bot commented Jun 6, 2021

Codecov Report

Merging #1691 (c8f6c56) into main (a6b1126) will increase coverage by 0.00%.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #1691   +/-   ##
=======================================
  Coverage   66.90%   66.90%           
=======================================
  Files         151      151           
  Lines       20762    20764    +2     
=======================================
+ Hits        13890    13892    +2     
  Misses       6872     6872           
Impacted Files Coverage Δ
src/nikonmn_int.cpp 53.60% <ø> (ø)
src/makernote_int.cpp 79.22% <0.00%> (+0.10%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a6b1126...c8f6c56. Read the comment docs.

@hassec
Copy link
Member

hassec commented Jun 6, 2021

Hi @nulllinie, thanks a lot for the report! 👍

If you look at the supported lenses in exiftool here, you'll actually see that exiftool has the following line on that site:

'CE 47 37 5C 25 25 DF 4E'  = Tamron SP 24-70mm f/2.8 Di VC USD G2 (A032)

So exiv2 and exiftool both expect a "4E" instead of a "0E".
This value corresponds to the raw hex value for the LensType.

I've found a sample picture from a review of the Lens shot with a nikon camera where exiftool and exiv2 report a LensType of "4E".

So something must have changed, because your picture definitely has a value of "0E" for LensType.
Could you post the model of your camera, and also maybe the firmware version of camera and lens?

But the above means, that we can't just change the old entry to reflect the new value because it would break exiv2 for existing pictures.
You can see in the list you changed, that many lenses already have duplicated entries with slightly different values.
Could you modify your PR, and just duplicate the existing line and make your change to the new entry?

Edit:
Sorry forgot to mention, that we would also need a test for this of course.
The procedure of how to write a short python test is quite easy and documented in the wiki under Adding a new lens which also shows you how to create a very small *.exv file from you *.nef file.

@hassec hassec added lens Issue related to lens detection makerNote Anything related to one of the various supported MakerNote formats labels Jun 6, 2021
@nulllinie
Copy link
Contributor Author

Hi @hassec ,
thanks for your quick response. i will do the duplicate entry and testcase, but this will take some days, weekend is over ;-)
The info you asked for:
Nikon D7000
Firmware: A 1.04, B 1.05, L 2.016
Lens: No info about firmeware, but bought 3 weeks ago, according Tamron website the lens firmware should be the current one - unless the lense is older than three years ...

@nulllinie
Copy link
Contributor Author

Hi @hassec
can u send me / port an link to the sample picture from a review you found? Then i will try to make a testcase for this too, then both lenses are covered.

@hassec
Copy link
Member

hassec commented Jun 9, 2021

Hi @nulllinie for now don't worry about the other test image I mentioned, as I'm not sure if we can simply include it in the test images. (Copyright and all...)

stay: *4E, new: 0E
and added testcase for additional lens ID 0E for Tamron_SP_24-70mm_F2.8_Di_VC_USD_G2
Copy link
Member

@hassec hassec left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

Thanks a lot @nulllinie for taking the time to provide the fix + testcase 👍

@hassec hassec merged commit 23dbf4f into Exiv2:main Jun 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lens Issue related to lens detection makerNote Anything related to one of the various supported MakerNote formats
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants