Fix: Preserve ICC color profile during lossless JPEG rotation#6776
Fix: Preserve ICC color profile during lossless JPEG rotation#6776shankarpriyank wants to merge 17 commits intocommons-app:mainfrom
Conversation
…ayout adjustments
# Conflicts: # app/src/main/java/fr/free/nrw/commons/edit/EditActivity.kt # app/src/main/java/fr/free/nrw/commons/edit/EditViewModel.kt # app/src/main/java/fr/free/nrw/commons/edit/TransformImage.kt # app/src/main/res/layout/activity_edit.xml
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…p related code" This reverts commit cc4e150.
Replace Android's ExifInterface.saveAttributes() with LLJTran's native Exif API to set orientation after rotation. ExifInterface repackages the metadata block and strips the APP2 segment containing the ICC color profile, causing a visible brightness/color shift. Changes: - Use LLJTran's Exif.getTagValue() + refreshAppx() to update orientation without touching the ICC profile (APP2 segment) - Remove manual EXIF tag collection and copyExifData() from EditActivity since LLJTran preserves all metadata natively with OPT_WRITE_ALL - Add ICC profile preservation unit test that verifies raw ICC bytes are identical after 90/180/270 degree rotations - Add ICC_test.jpg test resource with embedded ICC color profile - Re-enable 360 degree rotation cycle test (was disabled due to this bug) - Switch assertImagesAreIdentical to pixel-level comparison Fixes commons-app#6659
Replace LLJTran's refreshAppx() with a direct binary patch of the orientation byte in the saved JPEG file. refreshAppx() re-serializes the entire EXIF structure from LLJTran's internal representation, which drops ExifIFD sub-tags (FNumber, ExposureTime, GPS, Make, Model, WhiteBalance, etc.). The new patchExifOrientation() locates the orientation entry in IFD0 and overwrites only its 2-byte value to NORMAL (1), leaving all other metadata completely untouched. This preserves: - ICC color profile (APP2 segment) - All EXIF tags (FNumber, DateTime, GPS coordinates, Make, Model, etc.) - Embedded thumbnails Add unit test verifying all standard EXIF tags survive 90/180/270 degree rotations.
Instead of shipping a separate test image, inject a minimal sRGB ICC profile into Landscape_0.jpg at runtime during the test. This avoids adding a new binary to the repository while still verifying that ICC profiles survive lossless rotation.
- Remove unused app1Start variable in patchExifOrientation - Replace app1Len read with skipBytes(2) since value was discarded - Guard patchExifOrientation to only run for 90/180/270 rotations - Fix typo "thishould" → "this should" in test comment
|
✅ Generated APK variants! |
|
Hi @shankarpriyank, we suggest contributors to comment on the issue if they wish to get it assigned to prevent redundant effort. @Kota-Jagadeesh had volunteered to work on it. Even if there's no visible activity, we can always check with the assignee if they have any updates to share :) @Kota-Jagadeesh - hope you'd not started working on it. |
Sorry @RitikaPahwa4444 for not updating my work. I’m working on it and trying to find the best approach. Since it’s also a GSoC task, I’m exploring the best ways to proceed. I didn’t update on the issue earlier because I’ve been busy writing my GSOC proposal and working on other issues, including #6743, #6775, and a few issues. 🙂 |
|
Hi @RitikaPahwa4444, You're right, I should have commented on the issue first. I came across this issue after Nicolas suggested it in a comment on my proposal, and while investigating it I found a fix, so I went ahead and opened a draft PR. My plan was still to comment on the issue before moving the PR out of draft, but I understand the concern. I'm happy to close this PR since @Kota-Jagadeesh is already assigned to it. |
4 participants
Summary
Fixes #6659 — Rotating images strips the ICC Color Profile and EXIF metadata, causing a brightness/color shift and loss of camera information.
Root causes:
ExifInterface.saveAttributes()physically restructures the metadata block and strips the APP2 segment containing the ICC color profile.refreshAppx()re-serializes the EXIF structure from its internal representation, dropping ExifIFD sub-tags (FNumber, ExposureTime, GPS, Make, Model, etc.).Fix: Skip both
ExifInterface.saveAttributes()andrefreshAppx(). Instead, save withOPT_WRITE_ALL(which writes raw APP segment bytes untouched), then patch only the 2-byte EXIF orientation value directly in the output file usingRandomAccessFile. This preserves:Changes
refreshAppx()withpatchExifOrientation()that does a binary patch of the orientation tag in IFD0. RemoveExifimport.sourceExifAttributeList) andcopyExifData()— LLJTran preserves all metadata natively withOPT_WRITE_ALL.Verification
Unit tests (all pass)
Manual verification with exiftool
exiftool -b -ICC_Profileraw bytes: original vs rotated22585b96114de9db971ff4ea6c9938b6, 220 bytes)jpegtran -copy allreference: ICC preserved after 90°jpegtran360° cycle: pixel data viadjpegcomparisonOn-device verification (Pixel 9 Pro Fold, Android 16)