Bug Fix: New DJI config file for consistent M3M tags#75
Conversation
zthorson
left a comment
There was a problem hiding this comment.
I agree this is much cleaner than his solution (given we couldn't fully figure out why his was working). Just a few questions to see if we can clean it up more.
| #------------------------------------------------------------------------------ | ||
| # File: exiftool.cfg | ||
| # | ||
| # Description: This config file contains tag definitions needed to be able | ||
| # to write Pix4D and Sentera XMP-Camera tags | ||
| # | ||
| # Usage: exiftool -config exiftool.cfg -XMP-camera:TAG=VAL ... | ||
| # | ||
| # Requires: ExifTool version 7.00 or later | ||
| # | ||
| # References: 1) https://support.pix4d.com/hc/en-us/articles/360016450032-Specifications-of-xmpcamera-tags | ||
| #------------------------------------------------------------------------------ |
There was a problem hiding this comment.
Description should be updated to mention dji tags as well.
Is there a reason a new file was created rather than just updating the existing exiftool.cfg? Unless there were conflicts, you should be able to extend the existing file without affecting the other entries, making maintenance easier.
There was a problem hiding this comment.
Yes, I was hoping it would be as simple as adding the drone-dji section but we write CaptureUUID in XMP-Camera and DJI writes it in drone-dji. This was one of the main issues with processing corrected images in pix4d fields and likely ODM.
There was a problem hiding this comment.
I think you should be able to have both, as they are in different namespaces (XMP-Camera vs XMP-DJI), but maybe there is something I'm missing. If you've already tried it and it doesn't work then I'm good with the workaround. Otherwise I can try it on my end this afternoon as well.
There was a problem hiding this comment.
I'll try it to double check, I think I had some of the syntax wrong the first time I tried and this method worked.
| make = parser.make() | ||
| if make == "DJI": | ||
| command += ["-config", "cfg/dji.cfg"] | ||
| elif make == "Sentera": | ||
| command += ["-config", "cfg/exiftool.cfg"] | ||
| command += [ |
There was a problem hiding this comment.
Same question as above on whether we need to distinguish the two modes.
| pathex=['.'], | ||
| binaries=[], | ||
| datas=[('exiftool/exiftool.exe', '.'), ('cfg/exiftool.cfg', 'cfg'), ('cfg/reg_config.ini', 'cfg')] + metadata_datas, | ||
| datas=[('exiftool/exiftool.exe', '.'), ('cfg/exiftool.cfg', 'cfg'), ('cfg/dji.cfg', 'cfg'), ('cfg/reg_config.ini', 'cfg')] + metadata_datas, |
There was a problem hiding this comment.
get_corrections_*.spec also had the exiftool.cfg, but it doesn't look like they actually use it. If we decide to keep the dji.cfg/exiftool.cfg split, maybe we update those spec files to remove exiftool.cfg to make it clearer that it isn't used for those exes. Or since very few people use get_correction, we could just remove it.
There was a problem hiding this comment.
Thats a good catch, I will remove those from the get_corrections files
zthorson
left a comment
There was a problem hiding this comment.
Further testing reveals differences in where tags are copied if we try to merge it into 1 file, so we will have to maintain the 2 separate files. See Joey's notes for details.
Bug Fix: New DJI config file for consistent M3M tags
What?
Why?
PR Checklist
Breaking Changes