StefanOltmann commented on PR #367: URL: https://github.com/apache/commons-imaging/pull/367#issuecomment-1967682494
> And from a quick read at the code I think you might be correct 🙂 By the time I was porting Commons Imaging to Kotlin I created my own exif writer roundtrip test to make sure no data get lost. And to have a broad set of test files I used the [Unsplash lite](https://unsplash.com/data) set, which contains 25k images from all kinds of cameras - with all kinds of oddities, too. For a good chunk of these files I found that there was a data corruption and not knowing what it caused I wanted to open up a issue first, but I saw on the tracker that someone else also encountered this problem and @gwlucastrig luckily already found the cause and provided a fix. :) That's why I can be so certain that the bugfix is correct. ;) So all credit for #359 goes to him. > We can wait a bit longer for other to review the legal issue. Yes, we can. There is no hurry. I would just recommend to not release a M1 milestone release with that bug in the interest of the project. > Which is why we try to enforce the policy to always include tests with the changes. I agree that this is in general a good policy. Especially for new features that you want to be covered. But I don't feel that it's right to block a critical bugfix that can be manually verified because the lack of an test. > from what I understood, you wrote the fix and a test. Yes, the fix was @gwlucastrig work, but the correction of the exif roundtrip test was my own. > You used an image with a license that could be blocked by ASF's legal. At first I did not include it, because I can't decide such a thing for Apache. Gary wanted me to include it without checking with legal first. That's one of the things that went wrong in our communication. > But IMHO, you went the extra mile already by writing the test and trying to sort out the issue. Yes, even if I'm using a fork without this bug and personally have no issue anymore, I felt I wanted to contribute something important back in my free time to thank Apache. Writing my lib from scratch would be much harder, because on porting the code to Kotlin I first grasped how image metadata actually works thanks to this project. I was paid for my time figuring out how the exif roundtrip test actually must be crafted. Porting my findings from Kotlin back to Java was just doing and I somehow owed it. > You don't have to do everything yourself Right :) > You can leave the PR as-is, and one of us can take a look to see if we can find other images, or even craft one using some editor like GIMP, or searching in other archives, etc. That's right. You can drop in images from allowed sources and see where the test fails on. Having access to Unsplash images, which should be totally possible according to the lincese, would still be a benefit, because it's really a large collection. And a lot of metadata there is partly broken, because apparently many cameras write fault metadata and a lot of tools corrupt it. So Apache Commons Imaging is by far not the only tool corrupting metadata. Sadly a lot of tools do it. And I try very hard that my app won't become one of them. :D -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
