Lightweight EXIF orientation for network JPG/TIFF #1031
Conversation
| import java.io.IOException; | ||
| import java.io.InputStream; | ||
|
|
||
| public class ExifStreamReader { |
|
Lovely test cases! |
|
The tests works locally but not with Travis for some reason. |
|
I can try locally later tonight. |
| @@ -0,0 +1,41 @@ | |||
| /* | |||
| * Copyright (C) 2013 Square, Inc. | |||
|
The test fails locally for me. I'll add some logging to the parsing and see what's wrong at lunch. |
|
The res folder in the test folder should be resources. I accidentally pushed a commit with the unrenamed version. |
|
squash to one commit. |
|
Holding on on this one for perhaps the next release. 2.5.3 is supposed to be a small release and its turning out to be pretty big. |
|
@JohnWowUs I just tried your branch to test a similar issue I found, but it didn't work in that images are broken and don't load at all. I saw the following messages in the logs: After investigating it, it seems like it's due to the InputStream already being consumed by |
| if (stream == null) { | ||
| return 0; | ||
| } | ||
| MarkableInputStream markStream = new MarkableInputStream(stream); |
There was a problem hiding this comment.
Isn't using a MarkableInputStream here irrelevant, since it's local to ExifStreamReader only and will be discarded at the end of the call. The InputStream provided by the caller will still have been consumed and is independent of MarkableInputStream. Instead, I think you want to find a way to maintain the original InputStream (perhaps best done by the caller?)
I believe this is the root cause of the issue I was having using your branch, where images don't load up.
There was a problem hiding this comment.
The MarkableInputStream is used to allow us to reset the Inputstream i.e. not have it consumed.
There was a problem hiding this comment.
I tried to build JohnWowUs:IFStream-EXIF branch, and have the same issue. It would be great, if you can fix it and merge into picasso.
There was a problem hiding this comment.
@JohnWowUs I see what you mean; I was under the impression that the MarkableInputStream maintains its own marker and resetting it just resets MarkableInputStream, but after looking at the code, I see that it also tries to reset the original InputStream.
However, it seems that it does not work (hence, why I'm seeing failures in decoding the images). I believe it happens when the downloader is UrlConnectionDownloader, since that downloader uses the InputStream provided by openConnection(), which doesn't seem resettable.
Can you also try it on your end to confirm?
There was a problem hiding this comment.
I use OkHttpDownloader. Also I use NetworkInterceptor, wich replaces original InputStream with new one, because original stream is encrypted and I need to decrypt it.
|
Great!, It works for me now, thank you @JohnWowUs |
|
@JohnWowUs it also works for me now. Thanks for fixing it! |
|
Is this going to be included in 2.5.3 and good to merge? |
Lightweight EXIF orientation for network JPG/TIFF square#1031
|
Any update on timeline for merging this? |
|
Any chance this is going to be merged soon? |
I took some of the comments in PR #692 into account and came with a new solution light weight (no dependencies) for EXIF support for network JPG/TIFF files. These integrate the changes made in PR #1021 to handle all EXIF orientations.
I had to add some new resources for the tests hopefully that won't cause any problems with Travis.