Add NikonFl7 makernotes group #1963
Conversation
postscript-dev
added
documentation
enhancement
Codecov Report
@@ Coverage Diff @@
## main #1963 +/- ##
==========================================
- Coverage 61.29% 61.19% -0.11%
==========================================
Files 96 96
Lines 19110 19269 +159
Branches 9760 9862 +102
==========================================
+ Hits 11713 11791 +78
- Misses 5076 5135 +59
- Partials 2321 2343 +22
Continue to review full report at Codecov.
|
postscript-dev
force-pushed
the
add_FlashInfo0107_makernote_group
branch
from
cbcd5c1
to
ca379de
Compare
src/nikonmn_int.cpp
Outdated
| std::ostringstream oss; | ||
| oss.copyfmt(os); | ||
| float temp = ( value.toFloat()/float(-6.0) ); | ||
| temp *= float(1.00001); // Avoid round-off errors |
There was a problem hiding this comment.
Could you just use std::round here?
There was a problem hiding this comment.
I think that using std::round() would only round to the nearest integer which would loose accuracy. I copied the formula from ExifTool (found in exiftool/Nikon.pm and exiftool/Exif.pm).
I am happy to make the change if you know more about this?
There was a problem hiding this comment.
@postscript-dev: I did a bit of experimenting and I don't think this code is working correctly. For example, if temp == 1.5 then it prints 2/2. Similarly when temp == 0.666666 it prints 0/3. I think the perl code in Exif.pm must have different rounding behavior than C++ does. Regardless, that perl code in Exif.pm looks numerically clumsy to me. My suggestion is to use a condition like this to check whether the number is suitable for printing as a fraction:
std::abs(std::fmod(temp*3, 1)) < 0.001I have attached the three files that I used for testing: files.tar.gz
print_fraction.pmis based on the code in Exif.pmprint_fraction1.cppis based on your codeprint_fraction2.cppis my version that usesstd::absandstd::fmod.
There was a problem hiding this comment.
I haven't had much time recently and realised that I needed to look at this again. Thanks for helping.
There was a problem hiding this comment.
I made a mistake before. We need to use std::abs(std::remainderf(...)), not std::abs(std::fmod(...)). With that change, this multiplication is unnecessary. (The goal is that numbers like 0.4999 or 0.5001 are printed as a fraction.)
| temp *= float(1.00001); // Avoid round-off errors |
|
This pull request introduces 1 alert when merging 5a0f93f into bbfbcb6 - view on LGTM.com new alerts:
|
This differs from other `NikonFl` groups as it uses `bigEndian` format. This is because `Exif.NikonFl7.ExternalFlashFirmware` is a short and was incorrectly output when using `invalidByteOrder` or `littleEndian`. The other groups use `invalidByteOrder` which may be wrong. Sadly all of Exiv2s
+ Fix fuzzing CI errors of `exp2` and `round` not being in `std` by adding `#include <cmath>` + Fix Windows CI warning C4305 by explicitly defining value as a float
postscript-dev
force-pushed
the
add_FlashInfo0107_makernote_group
branch
from
9bc87fd
to
277b187
Compare
src/nikonmn_int.cpp
Outdated
| std::ostringstream oss; | ||
| oss.copyfmt(os); | ||
| float temp = ( value.toFloat()/float(-6.0) ); | ||
| temp *= float(1.00001); // Avoid round-off errors |
There was a problem hiding this comment.
I made a mistake before. We need to use std::abs(std::remainderf(...)), not std::abs(std::fmod(...)). With that change, this multiplication is unnecessary. (The goal is that numbers like 0.4999 or 0.5001 are printed as a fraction.)
| temp *= float(1.00001); // Avoid round-off errors |
postscript-dev
requested review from
kevinbackhouse
and removed request for
kmilos
NikonFl7is enabled for camera models where theExif.NikonFl7.versiontag is0107or0108. The tags are taken from ExifTool's Nikon FlashInfo0107 tags.This is part of resolving #1941.
NikonFl7differs from existingNikonFlgroups as it usesbigEndianformat. This is becauseExif.NikonFl7.ExternalFlashFirmwareis a short and was incorrectly output when usinginvalidByteOrderorlittleEndian. The other groups useinvalidByteOrderwhich may be wrong. Sadly all of Exiv2's test images for those groups don't use external flashes and the tag is set to zero.Changes:
NikonFl7groupSource: Exiftool