Initial support for OM System MakerNote #2167
Conversation
Codecov Report
@@ Coverage Diff @@
## 0.27-maintenance #2167 +/- ##
====================================================
+ Coverage 58.73% 58.81% +0.07%
====================================================
Files 148 148
Lines 23154 23161 +7
Branches 12689 12696 +7
====================================================
+ Hits 13600 13621 +21
+ Misses 6720 6703 -17
- Partials 2834 2837 +3
Continue to review full report at Codecov.
|
kmilos
added
makerNote
Will do when I find some time again @piponazo |
Glad to provide any .exv files, but would need to know if any other switches beside -ex should be used. |
|
I think we had a discussion about how to manually create the .EXV when a segment (XMP or Exif) > 64k. #2126 (comment) In the first instance, use |
OM100015.exv is here: |
|
Gosh, I don't want to get sucked into this. Where's the "original" file OM100015.ORF? Sometimes, when you're up to your neck in alligators, it's hard to remember that this mission was to drain the swamp! All we want is a small file that contains the metadata in which you are interested. We prefer to add small files to the test suite. Can you get ExifTool to create a .EXV from OM100015.ORF? |
|
I thought it might convenient to bring here the topic about the usage of Git LFS. If we move on with the proposal I presented in #2159 we could eventually upload large binary files for testing. |
|
Here is the original RAW OM100015.ORF (19MB), from which the .exv was created: I have also created .exv from the JPEG sample attached in #Support for files from OM System cameras #2126: I do have a local version of ExifTool, modified to read OM System MakerNote. Here is an .EXV created with |
|
Good to go I think, unless @clanmills or @hassec figure out a more compact/elegant way of implementing this. |
There was a problem hiding this comment.
Thanks for working on this, @kmilos.
I'm going to approve this because it works and fits the existing architecture. It's amazing that we need 100+ lines of additional boiler plate code when tvisitor does the same job in 2 lines (which I suspect I can reduce to 1 line!).
The inflexibility of the __MnHeader::__Signature() methods is because they are baked into the code. I feel we have failed to discover how this can be detected and fixed by read(). Having created the class with confidence and proceeded to read the metadata, we should not need rigid static strings and size_t variables in different classes.
The other architectural issue is the makernote classes. tvisitor.cpp has no maker note classes at all. None. I never discovered a need for any. However the effort and risk involved in removing the makernote classes from Exiv2 cannot be justified. So, let's leave alone. If anybody ever rewrites Exiv2, I hope they will appreciate the simpler (and more robust and consistent) approach used by tvisitor to deal with makernotes, ICC, IPTC and format specific metadata such as PNG/textual-information. Less can be more!
None-the-less, the code is correct and does its job. Thank You to @kmilos for putting in the effort to add this to Exiv2.
|
Should we maybe just create a small follow-up issue to look into cleaning up the duplication? Just so that it doesn't get forgotten? Otherwise, I'm happy for this to go in 👍 |
|
@hassec The duplication? Are you referring to the boiler plate class code being mostly duplicated in the _MnHeader classes? If that's what you're thinking, my inclination is to allow it as the effort/risk to re-engineer these classes is considerable. |
Sure, feel free! |
kmilos
added
the
forward-to-main
Initial support for OM System MakerNote (backport #2167)
Addresses #2126 in a "dumb" way by duplicating the header class.