exif: separate entries into raw vs human-readable #2518
Conversation
|
I had a thought about this --- how about leaving the current exif outpuf as is for compatibility, and (as fredprobidi suggests) adding a new set of fields using JSON? Something like: Now we get compatibility, plus easy and reliable structured data handling in all language bindings. We could maybe hide the old exif tags in vipsheader output. We'd need to detect user edits to the JSON on save. We'd need to add a dependency for a JSON read-write library though :-( An obvious choice might be: https://gitlab.gnome.org/GNOME/json-glib It's mature, well-maintained, integrates with glib, it's by ebassi (who is great), etc. |
|
If we're thinking of providing EXIF as JSON then perhaps this should be rolled up at a higher level as a single property, so the dirs and keys are also part of the JSON? {
"ifd2": {
"ShutterSpeed": {
"value": "253/50",
"humanValue": "5.06 EV (1/33 sec.)",
"dataType": "SRational",
"components": 1,
"length": 8
}
}
}Alternatively, for backwards compatibility, we could provide the non-human values in new |
|
That's true, though you wonder what value we are adding then --- the user could simply parse the Off-topic, but I just reread
Maybe this is the best compromise. We keep back compat, and we don't need to add a json library. I'm not sure how useful the extra info in the json output would be anyway. |
|
A possible problem with the alternative At the moment you can e.g. call |
|
Yes, write is harder. We could maybe do: ie. we add a |
|
Thinking about this again, perhaps we could fix this another way? Here's a tiny ruby program to show the issue: #!/usr/bin/ruby
require "vips"
x = Vips::Image.new_from_file ARGV[0]
x = x.mutate do |mutable|
mutable.set_type! GObject::GSTR_TYPE, "exif-ifd0-XPComment", "hello ( there"
end
x.write_to_file ARGV[1]I can run like this and it works: So that's the correct When we later copy the image, it fails: What's happened is:
So for a fix, how about just changing steps 3 and 4. Don't try to transform the new image metadata value to match the original exif value (ie. reversing the stringification from step 2, a difficult problem), instead just re-stringify the exifdata block and look for differences in the coded value. If we find they are different, we simply attach the new value to the image, and assume the user didn't painstakingly create the trailing This removes the need to reverse the stringification and hopefully fixes the bug. What do you think? It means we keep this stupid and broken interface. Perhaps we should have a separate issue for replacing it with something better. |
|
I did a quick hack: 8195b67 It was easier than I thought -- you only need to add a change detector near the start. With this I see: ie. it's correctly seeing that the item hasn't changed and shouldn't be regenerated. What do you think? |
|
Looks good, and much simpler. Is it possible to remove an EXIF property with this approach? |
|
Looks good to me as well. I tried to cover this by a unit test, see commit kleisauke@008fe2d. |
|
Yes, remove should work, I expanded the test prog: #!/usr/bin/ruby
require "vips"
# set the bad comment
x = Vips::Image.new_from_file ARGV[0]
x = x.mutate do |mutable|
mutable.set_type! GObject::GSTR_TYPE, "exif-ifd0-XPComment", "hello ( there"
end
x.write_to_file "t1.jpg"
# copy the image with the bad comment
x = Vips::Image.new_from_file "t1.jpg"
comment = x.get("exif-ifd0-XPComment")
puts "comment after save, load:"
puts "#{comment}"
x.write_to_file "t2.jpg"
# read back again and check the comment is OK
x = Vips::Image.new_from_file "t2.jpg"
comment = x.get("exif-ifd0-XPComment")
puts "comment after save, load, save, load:"
puts "#{comment}"
# delete and save
x = x.mutate do |mutable|
mutable.remove! "exif-ifd0-XPComment"
end
x.write_to_file "t3.jpg"
# read back again and check the comment is OK
x = Vips::Image.new_from_file "t3.jpg"
puts "try to get after remove, save, load:"
typeof = x.get_typeof("exif-ifd0-XPComment")
puts "typeof = #{typeof}" I see: So it's successfully saved and loaded again unchanged, and you can remove. |
|
Ah, @kleisauke 's unit test tested remove as well, that's easy. OK, I've merged the change to master, I think we're done! Let's kill off the last few tiny things and make 8.12-rc1. |
|
Excellent, thanks John. |
Here's a work-in-progress PR to fix #2388, which takes the approach suggested in kleisauke/net-vips#74 (comment)
It still needs unit tests (and more testing).