handling wrong type tag values

jernejaMislej commented 6 years ago

About

if one wants to edit an existing exif, where some tag values are written in the wrong type and those tags are not edited, piexif will crash when the edited dictionary is attempted to be dumped back into bytes, to be written back to the images, since some tag values are in the wrong type(when packing to struct in _dump)

Notes

piexif is great to edit or create new exif in an image, but we have issues where we often get images that have valuable information in exif tags we don't edit, but the information is stored in the wrong type, in cases like this, just loading exif from image and trying to _dump back will fail.

Please give us a suggestion on how to solve this rather than looping through all the tags and either removing the tags which have the value in the wrong type or trying to force the value in the right type.

I agree that the exif standard should be enforced and the type of each tag value is well specified, but still many cameras or people processing images will write the tag values in the wrong type and your _dump script can not seem to handle that, did you happen to experience similar issues yourself?

hMatoba commented 6 years ago

I'm thinking to add option "ignore_wrong_type" for dump in few days. Insert type check for given values into function "_dict_to_bytes" at first line, and remove values that has wrong type.

It's quite right that "many cameras or people processing images will write the tag values in the wrong type".

hMatoba commented 6 years ago

I want few samples that have wrong type.

hMatoba commented 6 years ago

https://github.com/hMatoba/Piexif/tree/dev

jernejaMislej commented 6 years ago

thank you for looking into this so quickly, here is one example where an unsigned 8bit tag has a rational value example_rational_altituderef

i ll have a look and try the dev branch, thanks

hMatoba commented 6 years ago

Fixed a problem. Can you try again?

jernejaMislej commented 6 years ago

hi, thank you so much for fixing this, works great on some images, but for some images the bad format tag is not removed, below image for example, if I load it and write it back by dumping with ignore_wrong_type=True, I will not get any error from piexif lib, but the resulting exif is still not valid example_bad_altituderef

jernejaMislej commented 6 years ago

hi, if you are done with the fixes, it would be great if you could merge with the master and make a new release or something, again thank you very much for the fixes

hMatoba commented 6 years ago

I don't have time to do this matter. Welcome to contribute.

jernejaMislej commented 6 years ago

hi, I understand that you don't have time, we r ok with the fix as it is, the issue with the resulting invalid exif is not that problematic, since we can successfully edit the tags we need, but we would need the new fixes to be public, will you be able to merge and make a new release, or you don't have time for that neither?

hMatoba commented 6 years ago

There still is a bug, so I can't release it. If you want to use, fork this library and release it by yourself.

jernejaMislej commented 6 years ago

leaving this opened for now, i m working on it with help from @hinxx at https://github.com/hinxx/Piexif we would like to have an option to either:

dump exif as it is, so for each tag, the value is dumped along with the type, count and offset to next as it was read from file, even if it is not according to standard
validate what was loaded in the dictionary, by saving the type read from file and comparing it to the type in your TAGS and remove those tags that don't have appropriate tag, so resulting dictionary can be written as standard

i am trying to have it done as you have originally planned, by having a flag for dump, i will do pull request when its finished

jernejaMislej commented 6 years ago

closing since we have forked Piexif for our use and made some changes, in case you would like to improve original version, these are the changes :

since you save and dump the data in the type it should be according to standard, then load needs to be more careful if encountering tags which dont have the type value correct, in our fork, these tags are ignored with a warning, otherwise strange things will happen when dumping, often leading to failed process
if jfif present then exif is looked for in the following segment, but we have encountered cases where this is not so, so we added a more robust search for exif
your tests have a bug, you are missing variable segment on this line

hMatoba commented 6 years ago

Thanks!

hMatoba / Piexif

handling wrong type tag values #45

About

Notes