Status Update
Comments
ap...@google.com <ap...@google.com> #2
Branch: androidx-master-dev
commit b90079595f33f58fece04026a97faa0d243acdb1
Author: Yuichi Araki <yaraki@google.com>
Date: Wed Sep 18 16:55:49 2019
Change the way to detect mismatch between POJO and query
This fixes cursor mismatch warnings with expandProjection.
Bug: 140759491
Test: QueryMethodProcessorTest
Change-Id: I7659002e5e0d1ef60fc1af2a625c0c36da0664d8
M room/compiler/src/main/kotlin/androidx/room/processor/QueryMethodProcessor.kt
M room/compiler/src/main/kotlin/androidx/room/solver/TypeAdapterStore.kt
M room/compiler/src/main/kotlin/androidx/room/solver/query/result/PojoRowAdapter.kt
M room/compiler/src/test/kotlin/androidx/room/processor/QueryMethodProcessorTest.kt
M room/compiler/src/test/kotlin/androidx/room/testing/TestProcessor.kt
ib...@google.com <ib...@google.com> #3
ib...@google.com <ib...@google.com> #4
Branch: androidx-master-dev
commit bdde5a1a970ddc9007b28de4aa29d60ffa588f08
Author: Yigit Boyar <yboyar@google.com>
Date: Thu Apr 16 16:47:05 2020
Re-factor how errors are dismissed when query is re-written
This CL changes how we handle errors/warnings if query is
re-written.
There was a bug in expandProjection where we would report warnings
for things that Room already fixes automatically (
The solution to that problem (I7659002e5e0d1ef60fc1af2a625c0c36da0664d8)
solved it by deferring validating of columns until after re-write
decision is made. Unfortunately, this required changing PojoRowAdapter
to have a dummy mapping until it is validating, make it hard to use
as it does have a non-null mapping which is not useful.
This CL partially reverts that change and instead rely on the log
deferring logic we have in Context. This way, we don't need to break
the stability of PojoRowAdapter while still having the ability to
drop warnings that room fixes. This will also play nicer when we
have different query re-writing options that can use more information
about the query results.
Bug: 153387066
Bug: 140759491
Test: existing tests pass
Change-Id: I2ec967c763d33d7a3ff02c1a13c6953b460d1e5f
M room/compiler/src/main/kotlin/androidx/room/log/RLog.kt
M room/compiler/src/main/kotlin/androidx/room/processor/QueryMethodProcessor.kt
M room/compiler/src/main/kotlin/androidx/room/solver/TypeAdapterStore.kt
M room/compiler/src/main/kotlin/androidx/room/solver/query/result/PojoRowAdapter.kt
ib...@google.com <ib...@google.com> #5
Actually thinking about it more, it's not obvious to me why the APP1
segment of the original image is so large either - looking in exiftool
it doesn't seem to contain any particularly enormous fields, so I can't really see where 65kB comes from.
ib...@google.com <ib...@google.com> #6
re APP1
segment is so large, I missed the thumbnail the first time, which takes up a significant chunk (64kB):
+ [IFD1 directory with 15 entries]
| 0) ImageHeight = 384
| - Tag 0x0101 (4 bytes, int32u[1]):
| 0323: 80 01 00 00 [....]
| 1) Orientation = 6
| - Tag 0x0112 (2 bytes, int16u[1]):
| 032f: 06 00 [..]
| Warning = Tag ID 0x0103 Compression out of sequence in IFD1
| 2) Compression = 6
| - Tag 0x0103 (2 bytes, int16u[1]):
| 033b: 06 00 [..]
| 3) ThumbnailOffset = 1273
| - Tag 0x0201 (4 bytes, int32u[1]):
| 0347: f9 04 00 00 [....]
| 4) ThumbnailLength = 64000
| - Tag 0x0202 (4 bytes, int32u[1]):
| 0353: 00 fa 00 00 [....]
| Warning = Tag ID 0x010f Make out of sequence in IFD1
ib...@google.com <ib...@google.com> #7
APP1
:
Exif metadata are restricted in size to 64 kB in JPEG images because according to the specification this information must be contained within a single JPEG APP1 segment.
Charcoal: Do you know if CameraX is deciding how large to make the thumbnail in this case, or is this happening lower down in the device? Taking up 64,000 bytes doesn't leave much space for the rest of the Exif data.
Related thread I found (also complaining about a Samsung device, but from 2013...):
I experimented to see what exiftool
does now when the APP1
segment gets too large, and it looks like they just add a second APP1
segment and call it 'multi-segment EXIF` (I have no idea how well supported this is):
$ exiftool -UserComment="abcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyz" b-263747161_original.jpg
Warning: [minor] File contains multi-segment EXIF - b-263747161_original.jpg
1 image files updated
$ exiftool -v3 b-263747161_original.jpg
ExifToolVersion = 12.52
FileName = b-263747161_original.jpg
Directory = .
FileSize = 434075
FileModifyDate = 1678121141
FileAccessDate = 1678121141
FileInodeChangeDate = 1678121141
FilePermissions = 33184
FileType = JPEG
FileTypeExtension = JPG
MIMEType = image/jpeg
JPEG APP1 (65533 bytes):
0006: 45 78 69 66 00 00 49 49 2a 00 08 00 00 00 0d 00 [Exif..II*.......]
0016: 00 01 04 00 01 00 00 00 a0 05 00 00 01 01 04 00 [................]
0026: 01 00 00 00 38 04 00 00 0e 01 02 00 01 00 00 00 [....8...........]
0036: 00 00 00 00 0f 01 02 00 08 00 00 00 aa 00 00 00 [................]
0046: 10 01 02 00 09 00 00 00 b2 00 00 00 12 01 03 00 [................]
0056: 01 00 00 00 06 00 00 00 1a 01 05 00 01 00 00 00 [................]
0066: bc 00 00 00 1b 01 05 00 01 00 00 00 c4 00 00 00 [................]
[snip 65421 bytes]
Warning = [minor] File contains multi-segment EXIF
JPEG APP1 (71 bytes):
10007: 45 78 69 66 00 00 00 00 00 00 00 00 00 00 00 00 [Exif............]
10017: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 [................]
10027: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 [................]
10037: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 [................]
10047: 00 00 00 00 00 00 00 [.......]
ExifByteOrder = II
Obviously the current ExifInterface
behaviour is very problematic (corrupting the image completely), but I'm not actually sure the best way to improve it. Some options I came up with:
- If the size of the
APP1
segment is larger than2^16
then just write the first2^16
bytes and hope. - Implement the same 'split segment' support that
exiftool
has. - Add logic to
ExifInterface
to resize a thumbnail to free up space in theAPP1
segment if it runs out (this seems too magical and could be very confusing for users). - Refuse to save a file in
ExifInterface.saveAttributes
if it results inAPP1
being larger than2^16
, either by silently not writing out (confusing) or throwing an exception (disruptive). - Add prioritisation of Exif tags, so when we run out of space we drop tags lowest-priority-first (very unlikely we can predict the 'correct' prioritisation such that users aren't annoyed/confused by the tags we decide to drop).
ch...@google.com <ch...@google.com> #8
Hi Ian,
Thanks for looking into the issue and provide the detailed analysis result.
About the question: Charcoal: Do you know if CameraX is deciding how large to make the thumbnail in this case, or is this happening lower down in the device? Taking up 64,000 bytes doesn't leave much space for the rest of the Exif data.
CameraX doesn't specify any thumbnail related capture request settings (
I'm curious about why this issue only happens when setting a FUSED location data but does not happen when setting a GPS location data? Does the FUSED location data in Exif actually occupy a little bit more size than GPS location data? So it causes the different result?
ib...@google.com <ib...@google.com> #9
I'm curious about why this issue only happens when setting a FUSED location data but does not happen when setting a GPS location data? Does the FUSED location data in Exif actually occupy a little bit more size than GPS location data? So it causes the different result?
Good question - I actually only got as far as playing with the image provided inside MyApplication-ExifLocationIssue.zip
, I didn't look at CameraXBasic-WriteFusedLocation.zip
, and as you noted in
ib...@google.com <ib...@google.com> #10
I've decided to go with option 4 from saveAttributes
in order to avoid creating a corrupted image. I've filed a new issue to track the possibility of adding multi-segment support in the future:
ap...@google.com <ap...@google.com> #11
Branch: androidx-main
commit c6bbc03f39f811ac546882880be46380cb31f5d5
Author: Ian Baker <ibaker@google.com>
Date: Tue Mar 07 11:09:08 2023
Throw an exception if trying to write a JPEG APP1 segment that's too large
jpeg_with_full_app_segment.jpg is an image taken with a Samsung A32
with a nearly-full APP1 segment due to a 64,000 byte thumbnail:
$ exiftool -v3 jpeg_with_exif_full_app1_segment.jpg
<snip>
JPEG APP1 (65072 bytes):
<snip>
| 5) ThumbnailLength = 64000
Bug: 263747161
Test: ./gradlew :exifinterface:exifinterface:connectedAndroidTest
Change-Id: Id421cd36d974ad4f29a2c6d6c730d99d8986d917
M exifinterface/exifinterface/build.gradle
M exifinterface/exifinterface/src/androidTest/java/androidx/exifinterface/media/ExifInterfaceTest.java
A exifinterface/exifinterface/src/androidTest/res/raw/jpeg_with_exif_full_app1_segment.jpg
M exifinterface/exifinterface/src/main/java/androidx/exifinterface/media/ExifInterface.java
ib...@google.com <ib...@google.com>
mw...@gmail.com <mw...@gmail.com> #12
Are you sure it's an issue with the APP1 segment being too large with the FUSED location? I was thinking perhaps there's some issue with the length calculation that's happening as we were able to fix these images by just detecting the start of the jpeg image data and re-writing the length field based on that detection.
ib...@google.com <ib...@google.com> #13
Are you sure it's an issue with the APP1 segment being too large with the FUSED location? I was thinking perhaps there's some issue with the length calculation that's happening as we were able to fix these images by just detecting the start of the jpeg image data and re-writing the length field based on that detection.
This fix is consistent with the current failure mode of ExifInterface
when the APP1
segment gets 'too large' (before my change linked above in 0x10030
bytes to write to the APP1 segment, and ExifInterface
does two things:
- When writing the length of the APP1 segment it truncates this to
0x0030
- It writes all those
0x10030
bytes out, and then it writes the rest of the image.
A JPEG parser (I assume) comes along and reads the length of the APP1
segment (0x30
) and skips that many bytes, then expects to find JPEG data - but it's actually in the middle of the Exif and so it fails.
If you manually find the start of the JPEG data then you obviate both of the problems - and the image is 'fixed'.
na...@google.com <na...@google.com> #14
The following release(s) address this bug.It is possible this bug has only been partially addressed:
androidx.exifinterface:exifinterface:1.3.7
na...@google.com <na...@google.com> #15
The following release(s) address this bug.It is possible this bug has only been partially addressed:
androidx.exifinterface:exifinterface:1.4.0-alpha01
Description
Component used: Version used: Devices/Android versions reproduced on: Samsung A32 (sm-a325f) Android 12 (also A12/A13 mentioned in b/263289024 )
When reporting bugs, please always include:
Where possible, please also provide: