[camerax] Implement onCameraClosing #3878
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -451,8 +451,9 @@ class AndroidCameraCameraX extends CameraPlatform { | |
| /// Binds [preview] instance to the camera lifecycle controlled by the | ||
| /// [processCameraProvider]. | ||
| /// | ||
| /// [cameraId] used to build [CameraEvent]s should you wish to filter | ||
| /// these based on a reference to a cameraId received from calling | ||
| /// `createCamera(...)`. | ||
| Future<void> _bindPreviewToLifecycle(int cameraId) async { | ||
| final bool previewIsBound = await processCameraProvider!.isBound(preview!); | ||
| if (previewIsBound || _previewIsPaused) { | ||
|
|
@@ -590,6 +591,9 @@ class AndroidCameraCameraX extends CameraPlatform { | |
|
|
||
| // Callback method used to implement the behavior described above: | ||
| void onChanged(Object stateAsObject) { | ||
|
Contributor
There was a problem hiding this comment. This should use a weak reference to the AndroidCameraCameraX instance. Something like: final WeakReference<AndroidCameraCameraX> weakThis = WeakReference<AndroidCameraCameraX>(this);The reason being that callback methods in the Native Wrapper API can cause leaking if they end up referencing themselves. Which there is a good chance to happen if you reference the complex class I forgot to mention in the image streaming PR, so the code for that may need to be updated as well.
Contributor
Author
There was a problem hiding this comment. Updated this callback and the image streaming callback!
Contributor
There was a problem hiding this comment. If it is important to use a weak reference to make sure something does not leak then we should add a test that covers that behavior.
Contributor
Author
There was a problem hiding this comment. To clarify, test that a weak reference is used in these callbacks and/or that using a weak reference avoids any leaking?
Contributor
There was a problem hiding this comment. Garbage collection for Dart is non-deterministic, so testing for it is a little difficult. I do have a test for leaking in the I don't think the implementation of the test should block this PR though. Leaks only prevent the release of the strong reference to the object in the Java I can also help with writing the test. |
||
| // This cast is safe because the Observer implemntation ensures | ||
camsim99 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| // the type of stateAsObject is the same as the observer this callback | ||
| // is attached to. | ||
| final CameraState state = stateAsObject as CameraState; | ||
|
Contributor
There was a problem hiding this comment. Should this have fallback behavior if stateAsObject is not a CameraState?
Contributor
Author
There was a problem hiding this comment. This is handled in the
Contributor
There was a problem hiding this comment. Consider adding a comment to that effect since one class is relying on the known implementation of another class and not on its api surface. |
||
| if (state.type == CameraStateType.closing) { | ||
| weakThis.target!.cameraEventStreamController | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.