-
Notifications
You must be signed in to change notification settings - Fork 1k
Fixed memory leaks (cleaner version) #135
base: master
Are you sure you want to change the base?
Changes from all commits
e9a2186
08ca696
69652bb
bdfbb23
8c3fef8
9f1dd0e
12a7e91
c43c49f
4e2cac1
6afd807
3c5956a
ea5812c
f729f79
ebfac8d
a6f7a35
33498eb
efb88f3
4cc984a
885a6f8
c0ccf80
172205e
18e862e
c83e06f
a403531
bb6be2b
593868a
21aa401
55a9b66
e32ffde
4a6a866
f3ba066
b59ad49
47f4808
a65dfaa
c50efd9
e9935e4
e56b55e
7420796
3bc2368
e096d96
00e8800
17eb039
16f05e2
8214fb1
89f4ad4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -33,6 +33,7 @@ | |
|
|
||
| import java.lang.annotation.Retention; | ||
| import java.lang.annotation.RetentionPolicy; | ||
| import java.lang.ref.WeakReference; | ||
| import java.util.ArrayList; | ||
| import java.util.Set; | ||
|
|
||
|
|
@@ -96,13 +97,13 @@ public CameraView(Context context, AttributeSet attrs, int defStyleAttr) { | |
| } | ||
| // Internal setup | ||
| final PreviewImpl preview = createPreviewImpl(context); | ||
| mCallbacks = new CallbackBridge(); | ||
| mCallbacks = new CallbackBridge(this); | ||
| if (Build.VERSION.SDK_INT < 21) { | ||
| mImpl = new Camera1(mCallbacks, preview); | ||
| } else if (Build.VERSION.SDK_INT < 23) { | ||
| mImpl = new Camera2(mCallbacks, preview, context); | ||
| mImpl = new Camera2(mCallbacks, preview, context.getApplicationContext()); | ||
|
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Important for memory leak fix is to pass the application context here. |
||
| } else { | ||
| mImpl = new Camera2Api23(mCallbacks, preview, context); | ||
| mImpl = new Camera2Api23(mCallbacks, preview, context.getApplicationContext()); | ||
|
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Important for memory leak fix is to pass the application context here. |
||
| } | ||
| // Attributes | ||
| TypedArray a = context.obtainStyledAttributes(attrs, R.styleable.CameraView, defStyleAttr, | ||
|
|
@@ -152,6 +153,9 @@ protected void onDetachedFromWindow() { | |
| mDisplayOrientationDetector.disable(); | ||
| } | ||
| super.onDetachedFromWindow(); | ||
| if (mCallbacks != null && mCallbacks.mCallbacks != null) { | ||
| mCallbacks.mCallbacks.clear(); | ||
|
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Important for memory leak: clear out any callbacks to ensure no extra references are held |
||
| } | ||
| } | ||
|
|
||
| @Override | ||
|
|
@@ -407,13 +411,16 @@ public void takePicture() { | |
| mImpl.takePicture(); | ||
| } | ||
|
|
||
| private class CallbackBridge implements CameraViewImpl.Callback { | ||
| private static class CallbackBridge implements CameraViewImpl.Callback { | ||
|
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Important for memory leak: non-static inner classes implicitly hold references to their parent. This was causing a big leak. Making it static fixes that.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do you actually experience memory leaks due to this?
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. My team uses LeakCanary to detect memory leaks. Unfortunately, the warnings were not false-positives. When I implemented these simple changes, the warnings went away. |
||
|
|
||
| private final ArrayList<Callback> mCallbacks = new ArrayList<>(); | ||
|
|
||
| private boolean mRequestLayoutOnOpen; | ||
|
|
||
| CallbackBridge() { | ||
| private WeakReference<CameraView> cameraView; | ||
|
|
||
| CallbackBridge(CameraView cameraView) { | ||
| this.cameraView = new WeakReference<>(cameraView); | ||
|
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Important for memory leak: Since CallbackBridge is now static, we must pass the CameraView. Within CallbackBridge only hold a weak reference to it. |
||
| } | ||
|
|
||
| public void add(Callback callback) { | ||
|
|
@@ -428,24 +435,26 @@ public void remove(Callback callback) { | |
| public void onCameraOpened() { | ||
| if (mRequestLayoutOnOpen) { | ||
| mRequestLayoutOnOpen = false; | ||
| requestLayout(); | ||
| if (cameraView.get() != null) { | ||
| cameraView.get().requestLayout(); | ||
| } | ||
| } | ||
| for (Callback callback : mCallbacks) { | ||
| callback.onCameraOpened(CameraView.this); | ||
| callback.onCameraOpened(cameraView.get()); | ||
| } | ||
| } | ||
|
|
||
| @Override | ||
| public void onCameraClosed() { | ||
| for (Callback callback : mCallbacks) { | ||
| callback.onCameraClosed(CameraView.this); | ||
| callback.onCameraClosed(cameraView.get()); | ||
| } | ||
| } | ||
|
|
||
| @Override | ||
| public void onPictureTaken(byte[] data) { | ||
| for (Callback callback : mCallbacks) { | ||
| callback.onPictureTaken(CameraView.this, data); | ||
| callback.onPictureTaken(cameraView.get(), data); | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -488,7 +497,7 @@ public void writeToParcel(Parcel out, int flags) { | |
| out.writeInt(flash); | ||
| } | ||
|
|
||
| public static final Parcelable.Creator<SavedState> CREATOR | ||
| public static final Creator<SavedState> CREATOR | ||
| = ParcelableCompat.newCreator(new ParcelableCompatCreatorCallbacks<SavedState>() { | ||
|
|
||
| @Override | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you suspend these changes in build scripts?