Reduced memory usage to avoid any OOM.

[RobertoMorelos]

Implements

#91

Description

Reduced memory usage to avoid any OOM.

Before

Library could lead to OOM due to the huge amount of memory allocated by the bitmap.

(Java memory increases up to 60 MB, sometimes up to 100 MB!)

After

Memory usage reduced and released the bitmap memory allocated.

(Java memory increases from 17 MB to 25 MB only)

Reviewers

See reviewer section.

[natario1]

This is a bit confusing. I think that crop.recycle() is needed and should help.

But in CameraUtils by using halfWidth and halfHeight instead of width and height, you are loading a bigger image, not smaller. Also with || instead of &&, you are just making it bigger. This can only increase the memory, right?

I think all the benefit you are seeing is just due to crop.recycle().

[RobertoMorelos]

I tried it just with crop.recycle() and I did not see any difference in performance. What I changed is based on the developers page (https://developer.android.com/topic/performance/graphics/load-bitmap.html). So yes, we are getting a bigger sample size, but not a bigger image. Basically the bigger the sample size, the smallest the image. ("For example, an image with resolution 2048x1536 that is decoded with an inSampleSize of 4 produces a bitmap of approximately 512x384").
Basically you need the largest sample size value, but you need the && to keep width and height, otherwise you'll get the first largest image found with either width or height.

[natario1]

I understand what you say, but the developer page is wrong in this, it's the opposite. You can try yourself, see CameraUtilsTest#testDecodeDownscaledBitmap. In your case the final bitmap is bigger than the given maxWidth and maxHeight, so the test fails (see travis logs).

I know this because I have worked on this recently. I honestly don't understand the developer page example...

I'd say, let's merge just the recycle call for now

[RobertoMorelos]

I disagree, with just the crop.recycle() we get this result:

(You can see is up to 70 MB)

Maybe the test is only measuring the width and height, but no the memory allocated (the actual bitmap size).

[natario1]

I don't how to explain your results, different shoot orientations, the code you are running, some obscure native cache, I don't know. I'm getting good results in the demo app so I think you should just pass a lower value of maxWidth and maxHeight when loading the bitmap in your own activity view.

Since we are trying to reduce memory here, loading bigger bitmaps in the library code is not a good idea :) recycle() will not solve the main issue, but it's needed. If you can go back to just that, we can merge this

[RobertoMorelos]

Ok, I understand what you are saying. I always did the same test case: take a picture with cameraCropOutput enabled in portrait mode. I always called the garbage collector before doing my tests so no any other memory allocation could change the results. If you want me to just add the crop.recycle() it's fine, but I hope no memory issues come to the near feature.

[natario1]

The real issue is here and it is not affected by your changes because it uses Integer.MAX_VALUE. I'll see if I can find a workaround. Meanwhile, yes, I think we should merge the recycle call! I really can't explain your profiles right now...

[RobertoMorelos]

@natario1 Done! Thank you!

[codecov]

Codecov Report

Merging #93 into master will increase coverage by <.1%.
The diff coverage is 100%.

Impacted Files	Coverage Δ	Complexity Δ
...utils/com/otaliastudios/cameraview/CropHelper.java	95.8% <100%> (+0.1%)	4 <0> (ø)	⬇️

[natario1]

@RobertoMorelos thanks a lot for your time.