Skip to content
This repository has been archived by the owner on Feb 22, 2023. It is now read-only.

[camera] Re-enable ability to concurrently record and stream video #6808

Merged

Conversation

adam-harwood
Copy link
Contributor

This re-enables the functionality from #6290, which was reverted in #6796 due to a broken test. This PR will attempt to fix the integration test.

Closes flutter/flutter#83634

Pre-launch Checklist

  • I read the [Contributor Guide] and followed the process outlined there for submitting PRs.
  • I read the [Tree Hygiene] wiki page, which explains my responsibilities.
  • I read and followed the [relevant style guides] and ran [the auto-formatter]. (Unlike the flutter/flutter repo, the flutter/plugins repo does use dart format.)
  • I signed the [CLA].
  • The title of the PR starts with the name of the plugin surrounded by square brackets, e.g. [shared_preferences]
  • I listed at least one issue that this PR fixes in the description above.
  • I updated pubspec.yaml with an appropriate new version according to the [pub versioning philosophy], or this PR is [exempt from version changes].
  • I updated CHANGELOG.md to add a description of the change, [following repository CHANGELOG style].
  • I updated/added relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making, or this PR is [test-exempt].
  • All existing and new tests are passing.

Adam Harwood added 2 commits December 8, 2022 10:18
This re-commits the content from flutter#6290. Will make a subsequent commit to try and fix the broken integ tests.
The `whenComplete` call was sometimes causing a race condition. It is also isn't needed for the test (to reset the value of isDetecting, given it's local), so removing it makes the test reliable.
@adam-harwood
Copy link
Contributor Author

See this comment for more context on this PR. CC @stuartmorgan

Copy link
Contributor

@hellohuanlin hellohuanlin left a comment

Choose a reason for hiding this comment

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

LGTM since it's a re-land.

@stuartmorgan
Copy link
Contributor

There was a camera_avfoundation streaming test failure, but I suspect it's flake rather than actually related since I wouldn't think this PR affects the codepath it's testing. Re-running to see if it's flake.

Looks like the Android tests are passing!

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.

Everything is green, so this should go more smoothly this time. Thanks for the quick update!

@stuartmorgan stuartmorgan added the autosubmit Merge PR when tree becomes green via auto submit App label Dec 8, 2022
@auto-submit auto-submit bot merged commit 929c9a6 into flutter:main Dec 8, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Dec 9, 2022
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Dec 9, 2022
* 929c9a632 [camera] Re-enable ability to concurrently record and stream video (flutter/plugins#6808)

* f31a438f3 [video_player] Fix file URI construction (flutter/plugins#6803)

* 6ab7d710d Roll Flutter from a570fd2 to 521028c (11 revisions) (flutter/plugins#6810)
gspencergoog pushed a commit to gspencergoog/flutter that referenced this pull request Jan 19, 2023
…#116781)

* 929c9a632 [camera] Re-enable ability to concurrently record and stream video (flutter/plugins#6808)

* f31a438f3 [video_player] Fix file URI construction (flutter/plugins#6803)

* 6ab7d710d Roll Flutter from a570fd2 to 521028c (11 revisions) (flutter/plugins#6810)
mauricioluz pushed a commit to mauricioluz/plugins that referenced this pull request Jan 26, 2023
…lutter#6808)

* Re-enable stream and record

This re-commits the content from flutter#6290. Will make a subsequent commit to try and fix the broken integ tests.

* Fix broken integration test for streaming

The `whenComplete` call was sometimes causing a race condition. It is also isn't needed for the test (to reset the value of isDetecting, given it's local), so removing it makes the test reliable.

* Trivial CHANGELOG change to trigger full CI tests

Co-authored-by: stuartmorgan <stuartmorgan@google.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
4 participants