Skip to content

Conversation

@czgnp
Copy link
Contributor

@czgnp czgnp commented Jan 16, 2021

Added Sigma lenses with code 368: 50/1.4, 35/1.4 40/1.4, 28/1.4, 14-24/2.8 and 60-600/4.5-6.3.
Configured code 368 to be resolved using focal length and max aperture.

Updated the canonEv algorithm since it didn't handle Sigma f/6.3 aperture (the lens reports f/6.2 to camera)

Verified all lenses added to 368: My own 35 and 50mm + downloaded Canon cr2 images from dpreview for the other lenses and they all resolve correctly.

Added Sigma lenses with code 368: 50/1.4, 35/1.4 40/1.4, 28/1.4, 14-24/2.8 and 60-600/4.5-6.3.
Configured code 368 to be resolved using focal length and max aperture.
    
Updated the canonEv algorithm since it didn't handle Sigma f/6.3 aperture (the lens reports f/6.2 to camera)
    
Verified all lenses added to 368: My own 35 and 50mm + downloaded Canon cr2 images from dpreview for the other lenses and they all resolve correctly.
@codecov
Copy link

codecov bot commented Jan 16, 2021

Codecov Report

Merging #1454 (147b5bb) into master (0dd82dd) will decrease coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1454      +/-   ##
==========================================
- Coverage   71.78%   71.77%   -0.01%     
==========================================
  Files         156      156              
  Lines       17389    17390       +1     
==========================================
  Hits        12482    12482              
- Misses       4907     4908       +1     
Impacted Files Coverage Δ
src/canonmn_int.cpp 88.69% <100.00%> (-0.32%) ⬇️

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 0dd82dd...c58ec0c. Read the comment docs.

@clanmills
Copy link
Collaborator

Thank You very much for this PR. I assume this resolves the issue that you raised in #1446. Thank You very much for working on this. I've been contributing to Exiv2 for 12 years and you are one of a handful or engineers who have modified the C++ lens code.

Can I ask you to submit this PR to 0.27-maintenance and you will be credited for your good work. Exiv2 v0.27.4 will ship on 2021-05-22: #1018 (comment) There is no schedule for the release of 'master' as 0.28

When making changes to the lens code, it's important to update the test suite to ensure future changes will no "forget" your lenses. There is a wiki page about this and includes an example of a python lens test. https://github.com/Exiv2/exiv2/wiki/Adding-a-new-lens

@czgnp
Copy link
Contributor Author

czgnp commented Jan 17, 2021

Hi, thanks for encouragement. But I messed up the github PR and created yet another one. Can you delete both this one and the 1455 and I will add both the canonmn_int file and testcases again on the 0.27 maintenance branch.
Sorry for the mess, but I can't tame github to clean up after me.

@clanmills
Copy link
Collaborator

Thank You for your contribution. Please don't call your work a "mess". Together we'll make everything good.

@czgnp
Copy link
Contributor Author

czgnp commented Jan 18, 2021

new to github I finally found the close PR and will reopen a new PR on the 0.27-maitenance branch.

@czgnp czgnp closed this Jan 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants