Status Update
Comments
ku...@google.com <ku...@google.com> #2
Looking at the exiftool data, it reports Orientation = 6, which is rotated 90 degrees counter-clockwise and needs 90 degrees clockwise rotation to appear correct. Of course, it's unusual that a Google format would be correct in exiftool and not in Google's own ExifInterface library. I see it's also not handling the rotation as I would expect in Chrome.
exiftool -V ~/Downloads/676x380.webp
ExifToolVersion = 12.60
FileName = 676x380.webp Dog\ Yule\ Log.mp4 Lil\ Bub\ Video\ Yule\ Log.mp4 android-studio-2022.2.1.19-mac_arm.dmg gradle.properties simpsons_background.png
Directory = /Users/clee/Downloads
FileSize = 116812
FileModifyDate = 1683650274
FileAccessDate = 1683655195
FileInodeChangeDate = 1683654924
FilePermissions = 33188
FileType = WEBP
FileTypeExtension = WEBP
MIMEType = image/webp
FileType [override] = Extended WEBP
FileTypeExtension [override] = WEBP
RIFF 'VP8X' chunk (10 bytes of data):
VP8X (SubDirectory) -->
+ [BinaryData directory, 10 bytes]
| WebP_Flags = 8
| ImageWidth = 2063598243
| ImageHeight = 97024
RIFF 'VP8 ' chunk (53444 bytes of data):
VP8Bitstream (SubDirectory) -->
+ [BinaryData directory, 53444 bytes]
| VP8Version = 0
| ImageWidth = 676
| HorizontalScale = 0
| ImageHeight = 380
| VerticalScale = 0
RIFF 'EXIF' chunk (63321 bytes of data):
Warning = [minor] Improper EXIF header
EXIF (SubDirectory) -->
+ [TIFF directory]
| ExifByteOrder = II
| + [IFD0 directory with 12 entries]
| | 0) ImageWidth = 4000
| | 1) ImageHeight = 3000
| | 2) Make = samsung
| | 3) Model = SM-S908U1
| | 4) Orientation = 6
| | 5) XResolution = 72 (72/1)
| | 6) YResolution = 72 (72/1)
| | 7) ResolutionUnit = 2
| | 8) Software = S908U1UES2CWCC
| | 9) ModifyDate = 2023:04:22 09:15:37
| | 10) YCbCrPositioning = 1
| | 11) ExifOffset (SubDirectory) -->
| | + [ExifIFD directory with 30 entries]
| | | 0) ExposureTime = 0.03333333333 (1/30)
| | | 1) FNumber = 2.2 (220/100)
| | | 2) ExposureProgram = 2
| | | 3) ISO = 400
| | | 4) ExifVersion = 0220
| | | 5) DateTimeOriginal = 2023:04:22 09:15:37
| | | 6) CreateDate = 2023:04:22 09:15:37
| | | 7) OffsetTime = -05:00
| | | 8) OffsetTimeOriginal = -05:00
| | | 9) ShutterSpeedValue = 0.03333333333 (1/30)
| | | 10) ApertureValue = 2.27 (227/100)
| | | 11) BrightnessValue = 0.21 (21/100)
| | | 12) ExposureCompensation = 0 (0/100)
| | | 13) MaxApertureValue = 2.27 (227/100)
| | | 14) MeteringMode = 2
| | | 15) Flash = 0
| | | 16) FocalLength = 2.2 (220/100)
| | | 17) SubSecTime = 960
| | | 18) SubSecTimeOriginal = 960
| | | 19) SubSecTimeDigitized = 960
| | | 20) FlashpixVersion = 0100
| | | 21) ColorSpace = 1
| | | 22) ExifImageWidth = 676
| | | 23) ExifImageHeight = 380
| | | 24) ExposureMode = 0
| | | 25) WhiteBalance = 0
| | | 26) DigitalZoomRatio = 1 (100/100)
| | | 27) FocalLengthIn35mmFormat = 13
| | | 28) SceneCaptureType = 0
| | | 29) ImageUniqueID = DA8XLOD01SM
| + [IFD1 directory with 8 entries]
| | 0) ImageWidth = 512
| | 1) ImageHeight = 384
| | 2) Compression = 6
| | 3) XResolution = 72 (72/1)
| | 4) YResolution = 72 (72/1)
| | 5) ResolutionUnit = 2
| | 6) ThumbnailOffset = 852
| | 7) ThumbnailLength = 62463
ku...@google.com <ku...@google.com> #3
For context, I found this because I got a bug report about a similar issue with an upside down photo on a Samsung Galaxy S10 and managed to reproduce the problem as a sideways WEBP on an S22 Ultra. I can supply more pictures from different Samsung models exhibiting this same problem if that would be useful.
ku...@google.com <ku...@google.com> #4
Will anyone please take a look at this bug? It appears on many, very common devices and results in mis-rotated images.
ko...@jetbrains.com <ko...@jetbrains.com> #5
Hi,
I can confirm there is an issue with the WebP exif reading implementation. I ran into the same issue when retrieving WebP images from the Amazon's Serverless Image Handler. Initially I thought it was a bug in the library I'm using (Coil) but found out it wasn't the case.
I have a bugfix available for the issue and am preparing a PR (that will be submitted via Gerrit).
Validated with my own images (a yellow sticky note captured at all 4 orientations) + the one from the google repo + the one from the topic starter.
Note: The bottom left image is a google webp without exif, the bottom right image is a google webp with exif
Note 2: It's also broken in Chrome & Firefox but works correctly on Safari/Finder on MacOS. I might submit bugfixes for those too.
ap...@google.com <ap...@google.com> #8
Thanks for the detailed investigation! I've replied on the PR with more details - but I'm not sure this image is a valid WebP.
ap...@google.com <ap...@google.com> #9
Branch: androidx-main
commit 161d43d41357f5d34fe7907c82ed9fca0b04d57a
Author: Laurence Muller <laurence.muller@gmail.com>
Date: Fri Dec 22 14:38:22 2023
Fix attribute reader for WebP images with Exif App1 Sections
The current attribute reader for WebP images fails to read the attributes if the Exif metadata contains an Exif App1 Section. The reason is that the byte-order is specified after the Exif App1 Section marker in the Exif payload. This patch will check if the Exif App1 section is present and correct the payload accordingly. This will allow it to read the Exif metadata (such as image orientation) correctly.
Test: Added a new image to cover this use case with a test. Run the instrument test using: ./gradlew :exifinterface:exifinterface:connectedAndroidTest
Fixes:
Change-Id: Idbb14e9fbcb038616ce650da03ed1d7eb9f997a5
M exifinterface/exifinterface/src/androidTest/java/androidx/exifinterface/media/ExifInterfaceTest.java
A exifinterface/exifinterface/src/androidTest/res/raw/invalid_webp_with_jpeg_app1_marker.webp
M exifinterface/exifinterface/src/androidTest/res/values/arrays.xml
M exifinterface/exifinterface/src/main/java/androidx/exifinterface/media/ExifInterface.java
ap...@google.com <ap...@google.com> #10
Hi, since this issue is now fixed in the codebase, is there any timeline on when this will be rolled out via a minor update (e.g v1.3.8)?
Would love to be able to use Exifinterface without having to use local workarounds in my apps.
ap...@google.com <ap...@google.com> #11
This will be included in 1.4.0, likely released sometime in Q2 - I'm afraid we won't be doing a 1.3.8 release as the current exifinterface
release branch is too old and we need to snap it to a more recent version from androidx-main
.
ap...@google.com <ap...@google.com> #12
Branch: androidx-main
commit fd3fee5a3ef4d40a3a35a869b7bfb2399e63e814
Author: Ian Baker <ibaker@google.com>
Date: Tue Feb 20 18:16:04 2024
Add comment for test added in
Test: ExifInterfaceTest
Bug: 281638358
Change-Id: Ied3650ee5d2bf290556c86ad546c3e28bd2f8d8a
M exifinterface/exifinterface/src/androidTest/java/androidx/exifinterface/media/ExifInterfaceTest.java
ap...@google.com <ap...@google.com> #13
The following release(s) address this bug.It is possible this bug has only been partially addressed:
androidx.exifinterface:exifinterface:1.4.0-alpha01
ap...@google.com <ap...@google.com> #14
Android API Change
This bug was referenced by a recent CL that changed the Android API surface area.
The
We'll wait until you mark this bug as 'Fixed' before starting our review, but please reach out if you'd like us to review it sooner.
Changes to savedstate/savedstate/api/current.txt
- androidx.savedstate
- androidx.savedstate.SavedStateReader
- androidx.savedstate.SavedStateWriter
[Gerrit:https://android-review.googlesource.com/3523153]
[API-Approvers:
[Branch:androidx-main]
[LIBRARY_API_REVIEW_TAG:savedstate/savedstate/api/current.txt]
ap...@google.com <ap...@google.com> #15
Android API Change
This bug was referenced by a recent CL that changed the Android API surface area.
The
We'll wait until you mark this bug as 'Fixed' before starting our review, but please reach out if you'd like us to review it sooner.
Changes to savedstate/savedstate/api/current.txt
- androidx.savedstate
- androidx.savedstate.SavedStateReader
[Gerrit:https://android-review.googlesource.com/3523152]
[API-Approvers:
[Branch:androidx-main]
[LIBRARY_API_REVIEW_TAG:savedstate/savedstate/api/current.txt]
ap...@google.com <ap...@google.com> #16
Android API Change
This bug was referenced by a recent CL that changed the Android API surface area.
The
We'll wait until you mark this bug as 'Fixed' before starting our review, but please reach out if you'd like us to review it sooner.
Changes to savedstate/savedstate/api/current.txt
- androidx.savedstate
- androidx.savedstate.SavedStateReader
[Gerrit:https://android-review.googlesource.com/3523151]
[API-Approvers:
[Branch:androidx-main]
[LIBRARY_API_REVIEW_TAG:savedstate/savedstate/api/current.txt]
mg...@google.com <mg...@google.com>
na...@google.com <na...@google.com> #17
The following release(s) address this bug.It is possible this bug has only been partially addressed:
androidx.lifecycle:lifecycle-viewmodel-savedstate:2.9.0-alpha12
androidx.lifecycle:lifecycle-viewmodel-savedstate-android:2.9.0-alpha12
androidx.lifecycle:lifecycle-viewmodel-savedstate-desktop:2.9.0-alpha12
androidx.lifecycle:lifecycle-viewmodel-savedstate-iosarm64:2.9.0-alpha12
androidx.lifecycle:lifecycle-viewmodel-savedstate-iossimulatorarm64:2.9.0-alpha12
androidx.lifecycle:lifecycle-viewmodel-savedstate-iosx64:2.9.0-alpha12
androidx.lifecycle:lifecycle-viewmodel-savedstate-linuxarm64:2.9.0-alpha12
androidx.lifecycle:lifecycle-viewmodel-savedstate-linuxx64:2.9.0-alpha12
androidx.lifecycle:lifecycle-viewmodel-savedstate-macosarm64:2.9.0-alpha12
androidx.lifecycle:lifecycle-viewmodel-savedstate-macosx64:2.9.0-alpha12
androidx.navigation:navigation-common:2.9.0-alpha08
androidx.navigation:navigation-common-android:2.9.0-alpha08
androidx.navigation:navigation-common-desktop:2.9.0-alpha08
androidx.navigation:navigation-common-iosarm64:2.9.0-alpha08
androidx.navigation:navigation-common-iossimulatorarm64:2.9.0-alpha08
androidx.navigation:navigation-common-iosx64:2.9.0-alpha08
androidx.navigation:navigation-common-linuxarm64:2.9.0-alpha08
androidx.navigation:navigation-common-linuxx64:2.9.0-alpha08
androidx.navigation:navigation-common-macosarm64:2.9.0-alpha08
androidx.navigation:navigation-common-macosx64:2.9.0-alpha08
androidx.navigation:navigation-runtime:2.9.0-alpha08
androidx.navigation:navigation-runtime-android:2.9.0-alpha08
androidx.navigation:navigation-runtime-desktop:2.9.0-alpha08
androidx.navigation:navigation-runtime-iosarm64:2.9.0-alpha08
androidx.navigation:navigation-runtime-iossimulatorarm64:2.9.0-alpha08
androidx.navigation:navigation-runtime-iosx64:2.9.0-alpha08
androidx.navigation:navigation-runtime-linuxarm64:2.9.0-alpha08
androidx.navigation:navigation-runtime-linuxx64:2.9.0-alpha08
androidx.navigation:navigation-runtime-macosarm64:2.9.0-alpha08
androidx.navigation:navigation-runtime-macosx64:2.9.0-alpha08
androidx.savedstate:savedstate:1.3.0-alpha10
androidx.savedstate:savedstate-android:1.3.0-alpha10
androidx.savedstate:savedstate-desktop:1.3.0-alpha10
androidx.savedstate:savedstate-iosx64:1.3.0-alpha10
androidx.savedstate:savedstate-linuxarm64:1.3.0-alpha10
androidx.savedstate:savedstate-linuxx64:1.3.0-alpha10
androidx.savedstate:savedstate-macosarm64:1.3.0-alpha10
androidx.savedstate:savedstate-macosx64:1.3.0-alpha10
Description
JetBrains has asked if we could support a
get*OrNull()
variant for our methods. The currentget*OrElse()
does not allow returning null, and checking isNull beforehand is both unnecessary and inefficient.With AOSP/3500556 change to reduce excessive inlining, we plan to move from inline to non-inline functions, as benchmarks show minimal performance impact. This works well for
get*
functions, butget*OrElse()
would allocate lambdas if we don't use inline, which we don't want to.Here are the options before the beta:
Option 1: Do not support
get*OrNull()
.isNull
andcontains
before aget
runs at 60ns instead of 20ns and (2) wouldn’t be able to remove inline fromget*OrElse()
due to lambda allocations.Option 2a: Support
get*OrNull()
, which also allows migratingget*OrElse()
out of inline functions.get*OrElse()
since users can simply callgetOrNull() ?: else()
.get*OrElse()
, so this would require extra work on their side.getOrNull()
seems useful, andBundle
always boxes primitives anyway, since everything is stored in aMap<K, V>
.Edit 03.03.25:
get*()
, and change to return a@Nullable
value.get
function (Map
). (2) Reduces API surface by half.SavedState
is a specializedMap
, not a general-purpose one, so if anACTION_FOO
expectsEXTRA_FOO_ARG1
andEXTRA_FOO_ARG2
, their absence means execution can't proceed. Throwing an error for a missing key is (arguably) reasonable, whileget*OrNull()
/get*OrElse()
provide an explicit opt-out mechanism for cases where a null or default value is acceptable.