Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[camera] Reland android flip/change camera while recording #3460

Merged
merged 3 commits into from Mar 17, 2023
Merged

[camera] Reland android flip/change camera while recording #3460

merged 3 commits into from Mar 17, 2023

Conversation

BradenBagby
Copy link
Contributor

This is the revert to the revert here #3272 that was merged because a flaky crash on an old device

@BradenBagby
Copy link
Contributor Author

BradenBagby commented Mar 14, 2023 •
edited

A couple comments @stuartmorgan @camsim99:

Ive tried reproducing the crash that was seen here #3272 for the old Galaxy device on APK 26. I cannot reproduce it in TestLab.

Reading through the logs from the official testlab crash it looks like the camera was started, but by the time a captureRequest was made, the camera was already closed. This is probably due to the older device erroring somewhere early on. From the logs I can tell it is happening when the camera is flipped mid recording, so it happens on the second camera. This error was never caught and causes the application to crash.

I check to make sure flipping while recording is compatible by looking at the APK version. If its not compatible, it just throws an error and doesnt flip. But this Galaxy APK 26 device doesnt indicate that it is incompatible. It just fails sometimes.

This PR does not stop the error from happening. It just catches and logs it. Im not sure if that is enough, but at least the application will not crash. Wondering if @stuartmorgan could trigger another TestLab run to see if it happens again?

@stuartmorgan
Copy link
Contributor

That should trigger a run; if it passes I can restart it a couple of times to try to catch flake.

@BradenBagby
Copy link
Contributor Author

I think something might be wrong with the test run Failed to start an instance! Failed to pull null image! Repository does not exist or may require authentication.

@stuartmorgan
Copy link
Contributor

Looks like some kind of Cirrus-wide issue; trying again to see if it's resolved.

@BradenBagby
Copy link
Contributor Author

It looks to be fixed but the full tests did not run

@stuartmorgan
Copy link
Contributor

the full tests did not run

How so? https://cirrus-ci.com/task/4946608610082816?logs=firebase_test_lab looks like a full run to me, with all five integration tests passing.

@BradenBagby
Copy link
Contributor Author

the full tests did not run

How so? https://cirrus-ci.com/task/4946608610082816?logs=firebase_test_lab looks like a full run to me, with all five integration tests passing.

I guess I was looking at the wrong thing

@stuartmorgan
Copy link
Contributor

It's passed 5/5 runs (not counting the Cirrus meltdown), so it's looking good!

@BradenBagby
Copy link
Contributor Author

So Im assuming we're waiting on a @camsim99 review now?

camsim99
camsim99 approved these changes Mar 17, 2023
View reviewed changes
Copy link
Contributor

@camsim99 camsim99 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This sounds good to me!

@camsim99 camsim99 added the autosubmit Merge PR when tree becomes green via auto submit App label Mar 17, 2023
@auto-submit auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Mar 17, 2023
@auto-submit
Copy link
Contributor

auto-submit bot commented Mar 17, 2023

auto label is removed for flutter/packages, pr: 3460, due to This PR has not met approval requirements for merging. You have project association CONTRIBUTOR and need 1 more review(s) in order to merge this PR.

  • Merge guidelines: You need at least one approved review if you are already a MEMBER or two member reviews if you are not a MEMBER before re-applying the autosubmit label. Reviewers: If you left a comment approving, please use the "approve" review action instead.

@auto-submit
Copy link
Contributor

auto-submit bot commented Mar 17, 2023

auto label is removed for flutter/packages, pr: 3460, due to Validations Fail.

stuartmorgan
stuartmorgan approved these changes Mar 17, 2023
View reviewed changes
Copy link
Contributor

@stuartmorgan stuartmorgan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@stuartmorgan stuartmorgan changed the title [camera] Revert of revert android flip/change camera while recording [camera] Reland android flip/change camera while recording Mar 17, 2023
@stuartmorgan stuartmorgan added the autosubmit Merge PR when tree becomes green via auto submit App label Mar 17, 2023
@auto-submit auto-submit bot merged commit d0de136 into flutter:main Mar 17, 2023
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Mar 20, 2023
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Mar 20, 2023
@engine-flutter-autoroll
d0de136 [camera] Reland android flip/change camera while recording (f…
f4b9220 
…lutter/packages#3460)
tarrinneal pushed a commit to flutter/flutter that referenced this pull request Mar 20, 2023
@engine-flutter-autoroll
Roll Packages from 44ddf2c to bbf86a7 (19 revisions) (#123015)
95256c0 
* 7636eb7 [go_router_builder] Support default value for `Set`, `List` and `Iterable` route parameters (flutter/packages#3391)

* 26c95da [image_picker] Move HashSet construction within if-statement (flutter/packages#3448)

* f5687b2 [image_picker] fix typos in comments (flutter/packages#3413)

* 84afba7 [image_picker] Migrate Android to Pigeon (flutter/packages#3476)

* 42927fc [image_picker]: Bump androidx.exifinterface:exifinterface from 1.3.3 to 1.3.6 in /packages/image_picker/image_picker_android/android (flutter/packages#3238)

* 9a44bdf Require Dart SDK 2.14, because of using APIs. (flutter/packages#3468)

* 12609a2 [ci] Clean up iOS simulators (flutter/packages#3458)

* 9b136a9 [ci/tool] Add external dependency validation (flutter/packages#3466)

* 11aab47 Manual roll Flutter from fb7e828 to 67e5f66 (8 revisions) (flutter/packages#3482)

* 784291b Update goldens (flutter/packages#3442)

* 43a42d1 [webview_flutter_android] Updates Dart and Java InstanceManagers (flutter/packages#3282)

* d0de136 [camera] Reland android flip/change camera while recording (flutter/packages#3460)

* 74fd094 [image_picker_android] Adjust file extension in FileUtils when it does not match its mime type (flutter/packages#3409)

* d2f1d2d [flutter_adaptive_scaffold] : 🐛 [FIX] : Issue: 121135. (flutter/packages#3253)

* 3d078b5 Roll Flutter from 67e5f66 to 53dfd23 (39 revisions) (flutter/packages#3484)

* 3b3a09d [url_launcher] Convert iOS to Pigeon (flutter/packages#3481)

* 80cd50a Roll Flutter from 53dfd23 to 6bd2b3c (17 revisions) (flutter/packages#3486)

* 998bb29 [webview_flutter] Updates the README with the migration of `WebView.initialCookies` and Hybrid Composition on (flutter/packages#3470)

* bbf86a7 Roll Flutter from 6bd2b3c to 3e4e092 (7 revisions) (flutter/packages#3490)
@BradenBagby BradenBagby mentioned this pull request Mar 20, 2023
Merged
nploi pushed a commit to nploi/packages that referenced this pull request Jul 16, 2023
@BradenBagby @nploi
[camera] Reland android flip/change camera while recording (flutter#3460
45b9517 
)

[camera] Reland android flip/change camera while recording
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@stuartmorgan stuartmorgan stuartmorgan approved these changes

@camsim99 camsim99 camsim99 approved these changes

Assignees
No one assigned
Labels
autosubmit Merge PR when tree becomes green via auto submit App p: camera platform-android
Projects
None yet
Milestone
No milestone
3 participants
@BradenBagby @stuartmorgan @camsim99