-
Notifications
You must be signed in to change notification settings - Fork 2.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[camerax] Fixes unawaited_futures
violations
#4337
Conversation
processCameraProvider?.unbindAll(); | ||
imageAnalysis?.clearAnalyzer(); | ||
unawaited(imageAnalysis?.clearAnalyzer()); |
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.
@bparrishMines this makes me wonder if I did some wrapping wrong. Should all of the methods I wrapped that were void
been wrapped as Future<void>
?
@@ -526,7 +526,7 @@ class CameraController extends ValueNotifier<CameraValue> { | |||
} | |||
|
|||
if (value.isStreamingImages) { | |||
stopImageStream(); | |||
await stopImageStream(); |
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.
Adding the await
here will make it take longer to actually begin the process of stopping recording on the native side; do we want that? An alternative would be to store the future in a local variable, and then await it just before the return
.
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.
I don't think we want this...I'll remove the await.
@@ -423,7 +423,7 @@ class AndroidCameraCameraX extends CameraPlatform { | |||
/// [cameraId] not used. | |||
@override | |||
Future<void> pausePreview(int cameraId) async { | |||
_unbindUseCaseFromLifecycle(preview!); | |||
await _unbindUseCaseFromLifecycle(preview!); | |||
_previewIsPaused = true; |
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.
This looks like the mistake I made in maps: you're making a field update asynchronously instead of synchronously. You probably want to reverse the statement order.
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.
Ah okay yes I'll reverse the order!
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.
This was also the case in resumePreview
, so I modified it there, too.
…ges into unawaited_futures_camerax
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.
LGTM
flutter/packages@7042079...771ec9b 2023-07-06 stuartmorgan@google.com [ci] Enable LUCI Dart unit tests (flutter/packages#4378) 2023-07-06 stuartmorgan@google.com [ci] Bring up LUCI Linux custom package tests (flutter/packages#4382) 2023-07-06 tarrinneal@gmail.com [pigeon] adds generate option for example pigeons (flutter/packages#4370) 2023-07-06 47866232+chunhtai@users.noreply.github.com [go_router] Allows redirect only GoRoute to be part of RouteMatchList (flutter/packages#4315) 2023-07-06 49699333+dependabot[bot]@users.noreply.github.com [sign_in]: Bump com.google.android.gms:play-services-auth from 20.5.0 to 20.6.0 in /packages/google_sign_in/google_sign_in_android/android (flutter/packages#4359) 2023-07-06 41930132+hellohuanlin@users.noreply.github.com [pigeon]fix a crash when casting NSNull to an Array (flutter/packages#4289) 2023-07-06 hello@kamil.id [google_sign_in_web] Fixes force unwrap on values that can be null (flutter/packages#4374) 2023-07-05 engine-flutter-autoroll@skia.org Roll Flutter from 590ef2d to 35085c3 (3 revisions) (flutter/packages#4379) 2023-07-05 43054281+camsim99@users.noreply.github.com [camerax] Fixes `unawaited_futures` violations (flutter/packages#4337) 2023-07-05 stuartmorgan@google.com [ci] Remove `starqlteue` from FTL tests (flutter/packages#4375) 2023-07-05 stuartmorgan@google.com [tools] Switch to `flutter test` (flutter/packages#4348) 2023-07-05 joshuapetitma@yahoo.com [flutter_markdown] Add TableCellVerticalAlignment property in markdown stylesheet (flutter/packages#3880) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages-flutter-autoroll Please CC flutter-ecosystem@google.com,rmistry@google.com on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
Fixes
unawaited_futures
violations incamera_android_camerax
plugin. The onlyawait
s that I did not add are those related to closing/disposing native objects that shouldn't require us to wait for completion.Part of flutter/flutter#127323.
Pre-launch Checklist
dart format
.)[shared_preferences]
pubspec.yaml
with an appropriate new version according to the pub versioning philosophy, or this PR is exempt from version changes.CHANGELOG.md
to add a description of the change, following repository CHANGELOG style.///
).