Uh oh!
There was an error while loading. Please reload this page.
[camera] Reland implementations of flip/change camera while recording - #3272
Conversation
hellohuanlin
left a comment
There was a problem hiding this comment.
Re-stamp the iOS part from the previous PR.
BradenBagby
commented
Feb 22, 2023
@bparrishMines and @stuartmorgan Here is the migrated PR that was merged and then reverted containing the fixes for the old Android devices |
stuartmorgan-g
commented
Feb 23, 2023
Capturing from the other issue for ease of reviewing: this is a reland with changes of a previously reviewed/landed/reverted PR. The diffs relative to the original reverted PR are at https://github.com/flutter/plugins/pull/7185/files/69a8c567d474c0a24ee1a7c21e6c06cb61ccd837..6fc7df4c89f435b4e890bd39f49dd9f46e1cf04d @camsim99 Could you take a look at the diffs since you reviewed the original PR? |
stuartmorgan-g
commented
Feb 23, 2023
Also, should remember this time to have one of us push a commit here to trigger the FTL tests in presubmit. |
stuartmorgan-g
commented
Feb 23, 2023
Or I can just do it myself right now so we don't forget :) |
BradenBagby
commented
Feb 23, 2023
Same failure for starqlteue-26-en-portrait. Im not sure why because eglSwapBuffers() is supposed to be available for >= 26, and in my separate tests it worked on >= 26. Possibly device specific? I will look more into checking availability of eglSwapBuffers manually instead of just checking Android version. I will have to make the tests pass with either successful flip, or failure/error on flip but no crash since this is likely device specific |
camsim99
left a comment
There was a problem hiding this comment.
Reviewed the changes, just left two nits!
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
camsim99
commented
Feb 27, 2023
@BradenBagby On the failure, I'm not sure what it is offhand, but looks like you may need to bump the example app compileSdkVersion to 33 https://cirrus-ci.com/task/4622324217937920?logs=firebase_test_lab#L274. |
…r/plugins/camera/Camera.java Co-authored-by: Camille Simon <43054281+camsim99@users.noreply.github.com>
That is due to device_info_plus package. Its a dev dependency I added for Android integration tests. We need to check SDK version to expect failures for devices < 26. Would you recommend using the old/discontinued device_info package so that warning does not appear? Device_info_plus seems to work I have tested it on sdk 25, 26, and 33 in TestLab. Im thinking the part of the package we are using is supported and we can ignore the warning, but I will change it if needed. |
stuartmorgan-g
commented
Feb 28, 2023
Third-party package dependencies are a liability for us in the longer term, so we avoid them unless there's really no other option. In this case, since the SDK switch logic being tested is on the native side, we should just test that on the native side. On the Dart side you can have the code allow for either outcome (without switching) with a comment explaining and pointing to the relevant native test. |
@stuartmorgan Tests failed on TestLab. Which I had ran myself on my personal TestLab on the same device and had them succeed. Sorry but will need revert again and Ill run through everything again. Maybe I re-broke it |
stuartmorgan-g
commented
Mar 7, 2023
Ah, it's flaky, which is why it passed presubmit this time. It's a different failure though, so hopefully that's progress? |
…ecording (flutter#3272)" This reverts commit d311478.
stuartmorgan-g
commented
Mar 7, 2023
#3405 will revert Android. I left the iOS part since there's no reason to keep reverting and re-landing that. |
This reverts commit 036cb1e.
BradenBagby
commented
Mar 13, 2023
@stuartmorgan Are you able to send me or make public the logcat.txt from the failing test here: https://console.developers.google.com/storage/browser/flutter_cirrus_testlab/plugins_android_test/camera_android/5094810416054272/92dcbd69-1247-4c79-8ee6-865a4935458c/example/0/ I cannot reproduce this crash on testlab, it passes every time on the same device. Im using the plugin tools firebase-test-lab command and running it on my personal testlab. Trying to get some good logs so I can see whats going on |
stuartmorgan-g
commented
Mar 14, 2023
Unfortunately I don't think there's any way for me to change visibility (we're working on resolving the general access issue here), but here's the specific file: https://drive.google.com/file/d/1lwaHd8CpmS7GV_TdnUno9n-qwmsQ4Sjo/view?usp=share_link |
…flutter#3272) [camera] Reland implementations of flip/change camera while recording
Reverts the Android part of flutter#3272 (commit d311478), which introduced significant crash flake in the tests.
…flutter#3272) [camera] Reland implementations of flip/change camera while recording
Reverts the Android part of flutter#3272 (commit d311478), which introduced significant crash flake in the tests.
…lutter#5930) `DEVELOPMENT_TEAM` should not be set in the example Xcode app. Introduced extraneously in flutter#3272
…flutter#3272) [camera] Reland implementations of flip/change camera while recording
Reverts the Android part of flutter#3272 (commit a97aab1), which introduced significant crash flake in the tests.
…lutter#5930) `DEVELOPMENT_TEAM` should not be set in the example Xcode app. Introduced extraneously in flutter#3272
this PR recreates the PR here flutter-team-archive/plugins#7185 that was the second PR for the federated plugin out of 3 that will be needed.
The first was merged here: flutter-team-archive/plugins#7011
The 3rd PR exists here flutter-team-archive/plugins#6478 (comment) and will need to be updated as well