[camera] Add ability to concurrently record and stream video

[adam-harwood]

This PR adds the ability to optionally also enable streaming when starting recording of a video. This feature is useful, for example, in applications that use image recognition. The customer will want some immediate feedback as the video is being recorded (where an ML model might be run on the captured images), as well as to upload the video to a server at the end for final server-side processing.

Functionally, the new optional named parameter on startVideoRecording will combine the existing implementation with that of startImageStream. Besides this, all functionality remains the same. For existing code, that does not use the new parameter, there is no change to the code path.

I have added unit tests to the higher-level dart abstractions, added integration tests for the underlying android and iOS implementations, and tested manually on Android and iOS devices. I have updated camera_web to throw an exception if the user attempts to enable streaming when recording (given that streaming is not yet supported).

Closes https://github.com/flutter/flutter/issues/83634

Pre-launch Checklist

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

[google-cla[bot]]

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

[adam-harwood]

Looks like I need to update camera_windows as well. Will do.

[adam-harwood]

One of the checks is failing because it doesn't want the PR to include both changes to camera_platform_interface and the implementations. Can you confirm if that's the case (and that you're happy with the changes in camera_platform_interface)? If so then I'll split this out into 2 PRs.

[adam-harwood]

Thanks for the feedback @hellohuanlin , I'll wait for some feedback on the other modules as well and then address everything you raised.

[hellohuanlin]

Saw a few updates in my email. Just double check - let me know when it's ready for a second review.

[adam-harwood]

Saw a few updates in my email. Just double check - let me know when it's ready for a second review.

Yep, it's ready @hellohuanlin , thanks :)

[adam-harwood]

@hellohuanlin this should be ready for review again.

[jmagman]

@hellohuanlin can you take another look at this, if your changes have all been addressed?

[jmagman]

@loic-sharma this still needs Windows review.

@ditman you were marked as the camera_web codeowner, did you remove yourself as a reviewer? Not sure how that disappeared.

[stuartmorgan]

@bparrishMines Looks like the implementations are approved, and this needs an app-facing-package API review.

[jmagman]

last line of pub output: "Because camera_web depends on camera_platform_interface ^2.3.1 which doesn't match any versions, version solving failed."

@stuartmorgan this might need some guidance landing the last few changes, but it looks pretty much ready?

[adam-harwood]

last line of pub output: "Because camera_web depends on camera_platform_interface ^2.3.1 which doesn't match any versions, version solving failed."

@stuartmorgan this might need some guidance landing the last few changes, but it looks pretty much ready?

Thanks @jmagman , currently I'm waiting on https://github.com/flutter/plugins/pull/6666 to get merged so that I can fix those errors you pointed out. Just some sequencing to be done.

[adam-harwood]

I've updated this now with the changes from #6550 and #6666 . I have removed the changes to the camera package, since they are dependent on the changes in the various implementation packages (e.g. camera_android). Therefore, once this PR is merged I'll open another one to include the changes to camera.

This is ready for re-review @stuartmorgan .

[faisalmushtaq007]

has this part been merged?

[adam-harwood]

has this part been merged?

This PR? No. It's awaiting final review but should be ready to merge.

[faisalmushtaq007]

@adam-harwood is their any set date for this merge and package update? Actually we have an app launch in coming days

[adam-harwood]

@adam-harwood is their any set date for this merge and package update? Actually we have an app launch in coming days

I don't know mate, I'm not on the Flutter team (I'm just an open source contributor) so I have no idea on what their merge and release schedule is. To be honest, this PR has been open since August 18 so I don't like your changes of having it merged and released in the next few days. I'd suggest coming up with a Plan B if I was you.

[jmagman]

@stuartmorgan merging is blocked on your change requested re: breaking changes. Can you take another look?

[faisalmushtaq007]

@stuartmorgan @jmagman why is the merging blocked??

[faisalmushtaq007]

@tarrinneal @gaaclarke @cyanglaz @GaryQian @jmagman @stuartmorgan Review was requested, please approve the changes!

[faisalmushtaq007]

@adam-harwood cant we do anything about this blockage??

[adam-harwood]

@adam-harwood cant we do anything about this blockage??

I can't sorry, like I said I don't work on the Flutter team and have no power here. I've been trying to get it merged for 4 months. I suggest you like for a plan B, possibly forking and distributing the code yourself.

[stuartmorgan]

@faisalmushtaq007 Please don't spam people with pings.

[flutter-dashboard[bot]]

This pull request is not mergeable in its current state, likely because of a merge conflict. Pre-submit CI jobs were not triggered. Pushing a new commit to this branch that resolves the issue will result in pre-submit jobs being scheduled.

[adam-harwood]

@stuartmorgan this is ready for re-review.

[stuartmorgan]

We'll need to revert this; the new Android integration test is failing. See https://github.com/flutter/plugins/pull/6796 for details. I should have pushed a trivial change to this PR to trigger the Android tests, but forgot.

@adam-harwood Are you able to reproduce that failure locally to debug it for an updated version of the PR?

[adam-harwood]

@adam-harwood Are you able to reproduce that failure locally to debug it for an updated version of the PR?

I ran them just now on an Android phone and cannot replicate the error you posted. I did get a different error though:

══╡ EXCEPTION CAUGHT BY FLUTTER TEST FRAMEWORK ╞════════════════════════════════════════════════════
The following assertion was thrown running a test:
Guarded function conflict.
You must use "await" with all Future-returning test APIs.
The guarded method "pump" from class LiveTestWidgetsFlutterBinding was called from
package:flutter_test/src/binding.dart on line 841.
Then, the "expectLater" function was called from
file:///home/adam/dev/flutter_plugins/packages/camera/camera_android/example/integration_test/camera_test.dart
on line 275.
The first method (LiveTestWidgetsFlutterBinding.pump) had not yet finished executing at the time
that the second function (expectLater) was called. Since both are guarded, and the second was not a
nested call inside the first, the first must complete its execution before the second can be called.
Typically, this is achieved by putting an "await" statement in front of the call to the first.

When the first method (LiveTestWidgetsFlutterBinding.pump) was called, this was the stack:
#0      TestAsyncUtils.guard (package:flutter_test/src/test_async_utils.dart:69:54)
#1      LiveTestWidgetsFlutterBinding.pump (package:flutter_test/src/binding.dart:1653:27)
#2      TestWidgetsFlutterBinding._runTestBody (package:flutter_test/src/binding.dart:841:13)
<asynchronous suspension>

When the exception was thrown, this was the stack:
#0      TestAsyncUtils.guardSync (package:flutter_test/src/test_async_utils.dart:267:5)
#1      expectLater (package:flutter_test/src/widget_tester.dart:491:18)
#2      main.<anonymous closure>.<anonymous closure> (file:///home/adam/dev/flutter_plugins/packages/camera/camera_android/example/integration_test/camera_test.dart:275:9)
(elided 24 frames from dart:async and package:stack_trace)

The test description was:
  recording with image stream
════════════════════════════════════════════════════════════════════════════════════════════════════
01:46 +2 ~2 -1: recording with image stream [E]                                                                                                                            
  Test failed. See exception logs above.
  The test description was: recording with image stream
  
  'package:flutter_test/src/binding.dart': Failed assertion: line 1736 pos 12: '!_expectingFrame': is not true.
  dart:core                                                                  _AssertionError._throwNew
  package:flutter_test/src/binding.dart 1736:12                              LiveTestWidgetsFlutterBinding.postTest
  ===== asynchronous gap ===========================
  package:stream_channel                                                     _GuaranteeSink.add
  tmp/flutter_tools.TJHMNR/flutter_test_listener.FNKBUV/listener.dart 53:22  main.<fn>
  tmp/flutter_tools.TJHMNR/flutter_test_listener.FNKBUV/listener.dart 52:20  main.<fn>

which I can fix, but I think it's unlikely it will solve the problem you posted. Applying the fix locally, all tests are then green. I'm testing this on an Android device, what device do the automatic runs execute on @stuartmorgan ?

For reference, I'm running the tests with:

flutter_plugins/packages/camera/camera_android/example$ flutter test integration_test/camera_test.dart

Which I assume is correct.

[stuartmorgan]

which I can fix, but I think it's unlikely it will solve the problem you posted. Applying the fix locally, all tests are then green.

Could you post a new PR that starts as a revert of the revert, and then has a commit that fixes the test issue you found locally (so we can easily review just the diff)? I can then push a commit to that PR to trigger the tests and we can see how it looks in CI.

I'm testing this on an Android device, what device do the automatic runs execute on @stuartmorgan ?

It looks like it's a Samsung Galaxy S9 and a Pixel 5.

For reference, I'm running the tests with:

That should run the same tests. Unfortunately there's no easy way to run them exactly the same way as the CI runs them (we hope to add simulator tests at some point to avoid this disconnect), but that's usually not an issue.

[adam-harwood]

Could you post a new PR that starts as a revert of the revert, and then has a commit that fixes the test issue you found locally (so we can easily review just the diff)?

Sure @stuartmorgan : https://github.com/flutter/plugins/pull/6808