search

rwcarlsen / goexif

Decode embedded EXIF meta data from image files.
BSD 2-Clause "Simplified" License
627 stars 134 forks source link

Improve Nikon maker note parsing. #18

Open gsadams opened 10 years ago

gsadams commented 10 years ago

Implement parsing Nikon type 1 maker notes, and don't fail on unfamiliar Nikon maker note formats.

The rename of ImageAdjustment to Nikon_ImageAdjustment is a breaking change for anyone who may have been using that field. But the field value really has to be interpreted in a Nikon-specific way, so it seems to make sense.

(There still seems to be a problem with the Canon maker note parser failing on some images in my library, by which I mean it causes the entire exif parser to return failure, even when it's just failing to recognize the particular format inside the maker note. I'll track that down and send a separate pull request.)

rwcarlsen commented 10 years ago

Thanks! It would probably be a good idea to write a couple of tests for the makernote parsing that check for the problem fixed by this PR and the other problem you mention. I can write tests if you don't want to. Regardless, I will be enjoying my Christmas vacation for the next several days and so won't do anything till I'm back (including this merge).

On Tue, Dec 24, 2013 at 1:20 AM, Geoff Adams notifications@github.comwrote:

Implement parsing Nikon type 1 maker notes, and don't fail on unfamiliar Nikon maker note formats.

The rename of ImageAdjustment to Nikon_ImageAdjustment is a breaking change for anyone who may have been using that field. But the field value really has to be interpreted in a Nikon-specific way, so it seems to make sense.

(There still seems to be a problem with the Canon maker note parser failing on some images in my library, by which I mean it causes the entire exif parser to return failure, even when it's just failing to recognize the particular format inside the maker note. I'll track that down and send a

separate pull request.)

You can merge this Pull Request by running

git pull https://github.com/gsadams/goexif improve-mknote

Or view, comment on, or merge it at:

https://github.com/rwcarlsen/goexif/pull/18 Commit Summary

  • Improve Nikon maker note parsing.

File Changes

  • M mknote/fields.gohttps://github.com/rwcarlsen/goexif/pull/18/files#diff-0(73)
  • M mknote/mknote.gohttps://github.com/rwcarlsen/goexif/pull/18/files#diff-1(35)

Patch Links:

gsadams commented 10 years ago

I agree about the tests, and it bothered me to send this pull request without them. But I'm not sure the best way to write such tests.

Would you recommend constructing trivial test images that trigger the problems, or doing some programmatic construction of just enough data structures to test, or doing a more function-level unit test? Or some other method?

Enjoy your vacation!

rwcarlsen commented 10 years ago

Test images are what I've used for testing so far. What I have done is truncate the images after the app1 section leaving approximately tens of bytes. And then my tests use the truncated file. My truncated test files are in the exif/samples directory. You are certainly welcome to manually construct images for testing if you really want to, but I wouldn't if it were me. Let me know if you are up for making a few tests.

Also, I have been thinking of changing the tiff and exif packages' error-handling paradigm to do a best-effort - basically to have the decoding return as much decoded data as possible in addition to any errors rather than just nil and the error.