subsampling-scale-image-view icon indicating copy to clipboard operation
subsampling-scale-image-view copied to clipboard

OutOfMemoryError

Open tibbi opened this issue 7 years ago • 26 comments

Hey, since version 3.8.0 Im getting some OutOfMemory errors, like at the attached screenshot. I also just started using GlideDecoder in the new version, not sure if that can be causing the issues. Im always using Subsampling view just for the currently visible image, it is being used at http://bit.ly/2jelMzx .

GlideDecoder is at http://bit.ly/2zpO1GH . Any ideas if there's anything I can do to improve the image loading? You are catching Exceptions around that place, maybe you could add OutOfMemory catching too? Thanks snimka obrazovky 22 11 2017 00 06 57

tibbi avatar Nov 21 '17 23:11 tibbi

Have you tried commenting out the Glide decoder to see if it's causing the problem? On what phones is this happening, with which images?

You are only overriding the ImageDecoder, and not the ImageRegionDecoder. Normally you should override both, because unless you explicitly disable tiling, you don't know for sure which one will be used. It depends on view size and screen density. Also it looks like the decoder is forcing the bitmap to be up/downscaled to match the screen size, which may use more memory for some images on high density screens.

These don't explain the OOME from the ExifInterface. With 3.8.0, I changed from the core library's version, which was deprecated and insecure, to the new support library. Perhaps this works differently.

davemorrissey avatar Nov 22 '17 11:11 davemorrissey

I will most likely remove the GlideDecoder in the next version released in a couple days, as it doesnt work properly. I also didn't manage implementing an ImageRegionDecoder yet, so the zoom is somewhat broken as it doesn't refocus on the zoomed region and it is blurry. I should have more info then if it was related to the GlideDecoder. As I checked the crash statistics now, it seems to be happening on Android 7.0 and Android 7.1 only. Thanks snimka obrazovky 22 11 2017 12 20 01

tibbi avatar Nov 22 '17 11:11 tibbi

I don't have a Nougat phone to test this on. It's worth trying without the Glide decoder but as I mentioned the crash seems more related to the new ExifInterface library. I can't find any mention of problems with it, but the old version shouldn't be used any more so I'm not sure what's the best course of action if it's broken.

davemorrissey avatar Nov 22 '17 11:11 davemorrissey

Great app, I hadn't tried it before!

There are a couple of things I'm curious about in the code where you seem to have reproduced features the view provides, though I don't think they're anything to do with this bug.

One is that you set the double tap zoom scale by decoding the bounds again. When onReady is called you can call getSWidth() and getSHeight() to find these values. The other is you can use the view's other ORIENTATION options to manually rotate the image, by first calling getAppliedOrientation (after onReady) to find out the orientation determined by EXIF, then applying the manual rotation on top (e.g EXIF = 90 + manual = 90 = ORIENTATION_180). Then you wouldn't need to decode the bitmap yourself.

What was the reason for trying Glide, was there a particular issue you needed to solve?

I'd be happy to have a play with this myself and submit a PR.

davemorrissey avatar Nov 22 '17 14:11 davemorrissey

The initial reason for using Glide was being able to display images in ARGB_8888 for better quality at some gradient areas. If I can achieve it somehow with SubsamplingScaleImageView, I think I could remove Glide. Thanks :)

tibbi avatar Nov 22 '17 14:11 tibbi

Yes, since 3.7.0 this has been possible:

    imageView.setBitmapDecoderFactory(new CompatDecoderFactory<ImageDecoder>(SkiaImageDecoder.class, Bitmap.Config.ARGB_8888));
    imageView.setRegionDecoderFactory(new CompatDecoderFactory<ImageRegionDecoder>(SkiaImageRegionDecoder.class, Bitmap.Config.ARGB_8888));

davemorrissey avatar Nov 22 '17 15:11 davemorrissey

Okay, sadly the performance at higher resolution images like 12MP+ is quite bad :/ Only at some images tho, not always. Not sure yet what does it depend on, my displaymetrics are DisplayMetrics{density=2.0, width=720, height=1184, scaledDensity=2.0, xdpi=326.571, ydpi=325.119} . I replaced GlideDecoder with those 2 lines you sent me. Im testig in on my Android 6, both Subsampling 3.7.2 and 3.8.0 work the same. Ill be playing around with it more once I get some time, thanks.

tibbi avatar Nov 22 '17 16:11 tibbi

There's a discussion on ARGB_8888 performance on #366.

davemorrissey avatar Nov 22 '17 16:11 davemorrissey

I see, so it looks like it wont have an easy fix anytime soon. I could help with testing some possible improvements or publishing stable versions out to people, nothing too risky though. 85% of my users have Android 6+, so solving it for them would be perfectly enough. I dont have any Android 8 myself yet, I could test just on Android 4, 6, 7. gallery

tibbi avatar Nov 22 '17 17:11 tibbi

With the code I saw, I don't think you were using Glide to load the really large images, because you only provided an ImageDecoder and not an ImageRegionDecoder. This meant they were being loaded as RGB_565, and that's why the performance was better. Only smaller images would have used Glide and been decoded as ARGB_8888, and they were probably small enough for good performance.

If you're happy to sacrifice a bit of resolution for better performance with ARGB_8888, try setMinimumTileDpi(240) or lower. The default as of 3.8.0 is 320.

You could also try setLayerType(LAYER_TYPE_SOFTWARE, null), but as you've seen from the other #366, results are unpredictable.

davemorrissey avatar Nov 22 '17 19:11 davemorrissey

minimum 240dpi didnt help. Layer_type_software helped a bit at there were no freezes during zooming, but the zoom was generally jerky. Ill roll back to the default 565 way for now, that seems like the best compromise.

tibbi avatar Nov 23 '17 07:11 tibbi

so the new version with using the default image and region decoder is published, and Im still getting a few out of memories from time to time. Im not using the GlideDecoder, so the issue is apparently with the new ExifInterface. The crashdump is the same as on the first screenshot in this issue.

tibbi avatar Nov 25 '17 18:11 tibbi

If ExifInterface is causing the problem then I'm not sure what to do. The old version isn't supposed to be used any more. I need to wait a bit longer and see if other developers are encountering the same issue, but if you need a fix you could turn off ORIENTATION_USE_EXIF and get the orientation yourself using the deprecated core library class.

davemorrissey avatar Nov 25 '17 18:11 davemorrissey

I upgrated to 3.9.0 and Im setting the orientation manually, but it is still crashing on Android 7+ because no matter what I set, you try retrieving the exif orientation at https://github.com/davemorrissey/subsampling-scale-image-view/blob/master/library/src/main/java/com/davemorrissey/labs/subscaleview/SubsamplingScaleImageView.java#L1556 . Thanks, here is a crashdump crashdump

tibbi avatar Dec 18 '17 08:12 tibbi

Still no reports of this problem from anyone else. Also nothing coming up in Google results. Are you loading a layout with many view instances? If so all the instances might be trying to load image EXIF data simultaneously.

Lazy loading the orientation when required is an option. However I don't have a reason to believe this is a proper fix, it will just move the bug, so I'd like to understand what's going on first. I have no idea how I'm going to do that unless other developers start reporting the same problem.

I think at the moment your best bet is forking the project and deleting that line.

davemorrissey avatar Dec 26 '17 23:12 davemorrissey

Im loading the main image + 2 images per side with Glide, Subsampling is used only for the currently visible fragment. Ill try recycling Subsampling after swiping away, think Im not doing it now. It is the most frequent crash for me, but still not a critical one so I dont really want to maintain a fork. Thanks

tibbi avatar Dec 27 '17 08:12 tibbi

so I forked your library and with this commit I indeed fixed hundreds of crashes per day ;) https://github.com/tibbi/subsampling-scale-image-view/commit/6d880a617e8818d4d0372482978163c8005c26bf

tibbi avatar Jan 10 '18 11:01 tibbi

Okay, so I need to choose between #282 and this. Fixing this would win if I could find any evidence of it being a widespread problem.

davemorrissey avatar Jan 10 '18 12:01 davemorrissey

well ye, you have to decide. I guess the only benefit of using the support media library is that vulnerability fix, which affects some old devices. Since 90% of my users have Android 6+, it's simply not worth for using by me. It still looks strange that nobody else reported those crashes.

tibbi avatar Jan 10 '18 12:01 tibbi

I don't seem to have this crash anywhere and I use the support Exif support lib standalone in several projects. Something smells fishy with your code. Can you post it for us?

franciscofranco avatar Jan 16 '18 00:01 franciscofranco

sure, it is an opensource gallery, subsampling is used at https://github.com/SimpleMobileTools/Simple-Gallery/blob/master/app/src/main/kotlin/com/simplemobiletools/gallery/fragments/PhotoFragment.kt#L246

tibbi avatar Jan 16 '18 08:01 tibbi

also the crash is related to the support library added in version 3.8.0 2 months ago, so you dont have it included in https://play.google.com/store/apps/details?id=com.franco.focus&rdid=com.franco.focus yet, if you mean that

tibbi avatar Jan 16 '18 08:01 tibbi

No, that's not the app. Anyway your code seems alright, so I dunno. Works fine on my end so that's definitely funky.

franciscofranco avatar Jan 16 '18 15:01 franciscofranco

I had the same problem,can you tell me how to solve this problem.

Kangzhengwei avatar Jul 10 '18 09:07 Kangzhengwei

I solved it by forking the project, doing this one commit https://github.com/tibbi/subsampling-scale-image-view/commit/6d880a617e8818d4d0372482978163c8005c26bf and including my fork in the app. Didnt have any issues since then.

tibbi avatar Jul 10 '18 09:07 tibbi

I solved this problem in other ways, but I still want to thank you.

Kangzhengwei avatar Jul 13 '18 05:07 Kangzhengwei