[nikon] Add instructions for adding new Nikon F mount lenses #2071
Conversation
Sturmflut
commented
Feb 4, 2022
Sturmflut
commented
- Also cleans up a dead link
src/nikonmn_int.cpp
Outdated
| // lid: LensIDNumber | ||
| // stps: LensFStops | ||
| // focs: MinFocalLength | ||
| // focl: MaxFocalLength | ||
| // aps: MaxApertureAtMinFocal | ||
| // apl: MaxApertureAtMaxFocal | ||
| // lfw: MCUVersion | ||
| // ltype: LensType | ||
| // | ||
| // The tcinfo, dblid and mid fields are being ignored. | ||
| // | ||
| // Please note that all fields except LensType have to be looked up in the | ||
| // Exif.NikonLd* prefix and not other Exif.Nikon* prefixes. For example: for modern | ||
| // Nikon bodies with modern lenses, there will be both a Exif.Nikon3.LensFStops and a | ||
| // Exif.NikonLd3.LensFStops entry in the EXIF data. You are looking for | ||
| // Exif.NikonLd3.LensFStops. | ||
| // | ||
| // In most cases the necessary hex values should be extracted from a test image using | ||
| // the following command: | ||
| // | ||
| // exiv2 -ph -g NikonLd3.LensIDNumber -g NikonLd3.LensFStops \ | ||
| // -g NikonLd3.MinFocalLength -g NikonLd3.MaxFocalLength \ | ||
| // -g NikonLd3.MaxApertureAtMinFocal -g NikonLd3.MaxApertureAtMaxFocal\ | ||
| // -g NikonLd3.MCUVersion -g Nikon3.LensType test.NEF | ||
| // | ||
| //------------------------------------------------------------------------------ | ||
| // Nikkor lenses by their LensID | ||
| //------------------------------------------------------------------------------ |
There was a problem hiding this comment.
There already is a wiki entry that explains adding a lens.
So maybe just point to that one from here? (And potentially add the above command to improve the wiki)
There was a problem hiding this comment.
Right, I did not notice the wiki entry. Probably we can use this information to improve the wiki and then just add here a link to the wiki.
There was a problem hiding this comment.
I think all documentation that is necessary to continue developing the code should be part of the the release tarball. The wiki is not part of the release. So I think it should rather be the other way around: The Wiki should have a link to this file, or there should be a document in docs/ that has the information.
There was a problem hiding this comment.
You've made a good point @Sturmflut . On the other hand, nowadays, anyone contributing to the project must have an internet connection and therefore access to the Wiki.
I have a slight preference for moving these notes to the wiki. The lenses topic is quite involved and not unique to the file src/nikonmn_int.cpp. What are your thoughts @hassec ? I can see you were the last one editing the wiki page.
There was a problem hiding this comment.
Right now the documentation situation is admittedly not that great as it is a bit spread across md files in the root and doc/ dir as well as the wiki.
I'd prefer to have it in the wiki too, as that's where we already have info about "adding a lens".
Having the doc in the source file, IMHO, kind of defeats the purpose a bit because the user would have to already know a lot of implementation details to ever find this documentation
There was a problem hiding this comment.
I took the freedom to extend the wiki page with the information provided here (I hope this is fine for you @Sturmflut ):
https://github.com/Exiv2/exiv2/wiki/Adding-a-new-lens
I would suggest to keep lines 1911-1920 which provided relevant information in the context of understanding the fields of the structure. And replace replace the other lines for a reference to the wiki page.
Would that work for you guys?
- Also cleans up a dead link
Sturmflut
force-pushed
the
nikon-improve-lens-docs
branch
from
a5c61b5
to
50f34f4
Compare
Codecov Report
@@ Coverage Diff @@
## main #2071 +/- ##
==========================================
- Coverage 62.08% 61.87% -0.22%
==========================================
Files 96 96
Lines 19153 19059 -94
Branches 9798 9782 -16
==========================================
- Hits 11891 11792 -99
- Misses 4967 4984 +17
+ Partials 2295 2283 -12
Continue to review full report at Codecov.
|
Ups I can't. @hassec do you have a strong position about not merging this until we chop part of the doc? I can do it afterwards in another PR. I just want to unblock some of the open PRs (which are blocked for small details) |
