[camera_web] Re: Support for camera stream on web #7950

TecHaxter · 2024-10-28T20:19:35Z

This PR aims to provide support for strartImageStream and stopImageStream on Web.

#92460

Based on #6944 from the archived plugins repository

Pre-launch Checklist

POV: my first PR on a public repo
Contains required commits from the PR #6443, resolves unnecessary 202 commits

…d before expecting mock response

…m, and used window.animationFrames

…le height and width in takeFrame

…seOffScreenCanvas

…m takeFrame function of camera service, using the function directly

…or instead of intialize()

…Frequently

…t OffscreenCanvas in camera service

…t OffscreenCanvas in camera test

… accurate fps

…ter_packages into camera_web_stream

TecHaxter · 2024-11-17T12:14:52Z

Hi @ditman The PR is ready for another review, Thanks :)

stuartmorgan-g · 2025-01-14T20:51:37Z

From triage: Ping @ditman on this re-review.

stuartmorgan-g · 2025-03-25T19:39:19Z

Update from triage: we're working through trying to find a web reviewer with availability to look at this. Apologies for the delay.

Piinks · 2025-06-04T23:29:23Z

@yjbanov can you suggest a reviewer for this change? It has been blocked on a web review.

mdebbar · 2025-08-06T18:06:22Z

/gemini review

gemini-code-assist

Code Review

This PR introduces support for image streams on the web for the camera plugin. The changes are well-structured, with new functionality added to Camera and CameraService classes, and corresponding tests.

My review has identified a few areas for improvement:

A critical issue in CameraService where a null-check operator could lead to a runtime error.
Some maintainability issues in Camera related to a C-style variable declaration and a magic number.
Opportunities to reduce code duplication in the new integration tests.

Overall, this is a great contribution. Addressing these points will improve the robustness and maintainability of the code.

packages/camera/camera_web/lib/src/camera_service.dart

gemini-code-assist · 2025-08-06T18:07:50Z

packages/camera/camera_web/lib/src/camera.dart

+    required int fps,
+  }) {
+    int? animationFrameId;
+    num then = 0, fpsInterval = 1000 / fps;


It's preferred to declare each variable on its own line. This C-style declaration can be less readable and is not idiomatic Dart.

num then = 0; num fpsInterval = 1000 / fps;

and let's give then a more descriptive name like lastFrameTimestamp.

This is addressed.

gemini-code-assist · 2025-08-06T18:07:50Z

packages/camera/camera_web/lib/src/camera.dart

+      final num elapsed = timestamp - then;
+
+      // if we're close to the next frame (by ~8ms), do it.
+      if (fpsInterval - elapsed <= 8) {


The number 8 here is a magic number. While the comment explains its purpose, it would be clearer to define it as a named constant (e.g., _frameTimeToleranceMs). This improves readability and maintainability, as the value's purpose is self-documenting and it can be easily changed in one place.

This is addressed.

gemini-code-assist · 2025-08-06T18:07:50Z

packages/camera/camera_web/example/integration_test/camera_service_test.dart

+      testWidgets(
+        'returns true if broswer has OffscreenCanvas '
+        'otherwise false',
+        (WidgetTester widgetTester) async {
+          when(
+            () => jsUtil.hasProperty(
+              window,
+              'OffscreenCanvas'.toJS,
+            ),
+          ).thenReturn(true);
+          final bool hasOffScreenCanvas =
+              cameraService.hasPropertyOffScreenCanvas();
+          expect(
+            hasOffScreenCanvas,
+            true,
+          );
+          when(
+            () => jsUtil.hasProperty(
+              window,
+              'OffscreenCanvas'.toJS,
+            ),
+          ).thenReturn(false);
+          final bool hasNotOffScreenCanvas =
+              cameraService.hasPropertyOffScreenCanvas();
+          expect(
+            hasNotOffScreenCanvas,
+            false,
+          );
+        },


This test and the subsequent test on lines 944-1003 are nearly identical, with the only difference being the mocked return value of hasPropertyOffScreenCanvas. This duplication can be reduced by creating a shared test implementation or using a for loop to run the test for both true and false cases. This will make the tests more maintainable.

This is addressed now.

gemini-code-assist · 2025-08-06T18:07:50Z

packages/camera/camera_web/example/integration_test/camera_test.dart

+      testWidgets(
+        'CameraImageData bytes is a multiple of 4 '
+        'when browser supports OffscreenCanvas',
+        (WidgetTester tester) async {
+          final VideoElement videoElement = getVideoElementWithBlankStream(
+            const Size(10, 10),
+          );
+
+          final Camera camera = Camera(
+            textureId: textureId,
+            cameraService: cameraService,
+          )..videoElement = videoElement;
+
+          when(() => cameraService.hasPropertyOffScreenCanvas()).thenReturn(
+            true,
+          );
+
+          when(
+            () => cameraService.takeFrame(videoElement),
+          ).thenAnswer(
+            (_) => CameraImageData(
+              format: const CameraImageFormat(
+                ImageFormatGroup.unknown,
+                raw: 0,
+              ),
+              planes: <CameraImagePlane>[
+                CameraImagePlane(
+                  bytes: Uint8List(32),
+                  bytesPerRow: videoElement.width * 4,
+                ),
+              ],
+              height: 10,
+              width: 10,
+            ),
+          );
+
+          final CameraImageData cameraImageData =
+              await camera.cameraFrameStream().first;
+          expect(
+            cameraImageData,
+            equals(
+              isA<CameraImageData>().having(
+                (CameraImageData e) => e.planes.first.bytes.length % 4,
+                'bytes',
+                equals(0),
+              ),
+            ),
+          );
+        },


This test and the subsequent test on lines 1763-1812 are almost identical, differing only in the mocked value for hasPropertyOffScreenCanvas. To improve maintainability and reduce code duplication, consider refactoring them into a single test function that is parameterized with the boolean for OffscreenCanvas support, or use a loop.

This has been addressed.

gemini-code-assist · 2025-08-06T18:07:50Z

packages/camera/camera_web/lib/src/camera.dart

+  // TODO(replace): introduced fps in
+  /// [CameraImageStreamOptions]


This TODO should be addressed, and the fps should be introduced in [CameraImageStreamOptions].

I don't think this TODO needs to be addressed in this PR, because PR checks would fail if it would contain changes in more than one package.

stuartmorgan-g · 2025-08-19T18:34:55Z

From triage: @mdebbar What's the status of the web review here?

mdebbar

This looks really close to being ready to land. I just some minor polishing comments.

mdebbar · 2025-08-22T18:13:20Z

packages/camera/camera_web/CHANGELOG.md

-## NEXT
+## 0.3.6

+* Supporting camera image stream on web.


To keep this consistent with the rest of the file:

Suggested change

* Supporting camera image stream on web.

* Adds support for camera image stream on web.

This is addressed.

mdebbar · 2025-08-22T18:17:09Z

packages/camera/camera_web/example/integration_test/camera_service_test.dart

+          when(
+            () => jsUtil.hasProperty(
+              window,
+              'OffscreenCanvas'.toJS,
+            ),
+          ).thenReturn(true);
+          final bool hasOffScreenCanvas =
+              cameraService.hasPropertyOffScreenCanvas();
+          expect(
+            hasOffScreenCanvas,
+            true,
+          );
+          when(
+            () => jsUtil.hasProperty(
+              window,
+              'OffscreenCanvas'.toJS,
+            ),
+          ).thenReturn(false);
+          final bool hasNotOffScreenCanvas =
+              cameraService.hasPropertyOffScreenCanvas();
+          expect(
+            hasNotOffScreenCanvas,
+            false,
+          );


Can use a loop to simplify:

Suggested change

when(

() => jsUtil.hasProperty(

window,

'OffscreenCanvas'.toJS,

),

).thenReturn(true);

final bool hasOffScreenCanvas =

cameraService.hasPropertyOffScreenCanvas();

expect(

hasOffScreenCanvas,

true,

);

when(

() => jsUtil.hasProperty(

window,

'OffscreenCanvas'.toJS,

),

).thenReturn(false);

final bool hasNotOffScreenCanvas =

cameraService.hasPropertyOffScreenCanvas();

expect(

hasNotOffScreenCanvas,

false,

);

for (final supportsOffscreenCanvas in [true, false]) {

when(

() => jsUtil.hasProperty(

window,

'OffscreenCanvas'.toJS,

),

).thenReturn(value);

final bool hasOffScreenCanvas =

cameraService.hasPropertyOffScreenCanvas();

expect(

cameraService.hasPropertyOffScreenCanvas(),

supportsOffscreenCanvas ? isTrue : isFalse,

);

}

Just so you know – this has been addressed now.

mdebbar · 2025-08-22T18:21:55Z

packages/camera/camera_web/example/integration_test/camera_service_test.dart

+      testWidgets(
+        'returns Camera Image of Size '
+        'when videoElement is of Size '
+        'when browser supports OffscreenCanvas',
+        (WidgetTester widgetTester) async {


This test and the one below are basically the same, the only difference is the boolean value of the hasPropertyOffScreenCanvas mock.

Combine both into a single test:

testWidgets( 'returns Camera Image of Size ' 'when videoElement is of Size ' 'regardless of OffscreenCanvas support', (WidgetTester widgetTester) async { for (final supportsOffscreenCanvas in [true, false]) { ... } }

This has been resolved now.

mdebbar · 2025-08-22T18:24:03Z

packages/camera/camera_web/example/integration_test/camera_test.dart

+      testWidgets(
+        'CameraImageData bytes is a multiple of 4 '
+        'when browser supports OffscreenCanvas',
+        (WidgetTester tester) async {


Can be combined with the test below, similar to my previous comment.

I've taken care of this.

mdebbar · 2025-08-22T18:25:21Z

packages/camera/camera_web/lib/src/camera.dart


+  /// Used to check if allowed to paint canvas off screen
+  @visibleForTesting
+  bool canUseOffscreenCanvas = false;


Looks like this never changes, so let's make it final:

Suggested change

bool canUseOffscreenCanvas = false;

final bool canUseOffscreenCanvas;

This is addressed now.

mdebbar · 2025-08-22T18:28:56Z

packages/camera/camera_web/lib/src/camera.dart

+  // TODO(replace): introduced fps in
+  /// [CameraImageStreamOptions]


Please follow the style guide for TODOs: https://github.com/flutter/flutter/blob/master/docs/contributing/Style-guide-for-Flutter-repo.md#avoid-checking-in-comments-that-ask-questions

I have updated the TODO comment. But haven't created an issue for that. Should I create one?

Update: I've created an issue and included that in the comment.

mdebbar · 2025-09-25T16:11:05Z

packages/camera/camera_web/lib/src/camera.dart

+    required int fps,
+  }) {
+    int? animationFrameId;
+    num then = 0, fpsInterval = 1000 / fps;


and let's give then a more descriptive name like lastFrameTimestamp.

stuartmorgan-g · 2025-11-04T19:25:20Z

From triage: @mdebbar This looks to be ready for re-review.

TecHaxter added 30 commits October 29, 2024 00:57

[camera/camera_web] Supporting camera stream on web

2552bdc

[camera_web] Version and Changelog updated

72a7127

[camera_web] - integration test updated: mock videoElement initialize…

d366566

…d before expecting mock response

[camera_web] - integration test updated: mock videoElement initialize…

4a18e22

…d before expecting mock response

[camera_web] camera service takeFrame with better exception handelling

b12f7e7

[camera_web] - integration test updated: mock videoElement initialize…

8697046

…d before expecting mock response

[camera_web] camera service takeFrame with better exception handelling

b42fbb9

[camera_web] tested takeFrame camera service function

b7b9342

[camera_web] - integration test updated: mock videoElement initialize…

8ec3a04

…d before expecting mock response

[camera_web] camera service takeFrame with better exception handelling

622841c

[camera_web] tested takeFrame camera service function

096ac4d

[camera_web] tested cameraFrameStream

ed6f086

[camera_web] - integration test updated: mock videoElement initialize…

d1244a0

…d before expecting mock response

[camera_web] camera service takeFrame with better exception handelling

d891a36

[camera_web] tested takeFrame camera service function

7dfb194

[camera_web] tested cameraFrameStream

b9111cb

[camera_web] platform version updated according to guidelines

f80cb03

[camera_web] - integration test updated: mock videoElement initialize…

41ac242

…d before expecting mock response

[camera_web] camera service takeFrame with better exception handelling

3edea8c

[camera_web] tested takeFrame camera service function

d1e4d95

[camera_web] tested cameraFrameStream

86da719

[camera_web] platform version updated according to guidelines

dcf48f9

style: removed late from late final_cameraFrameStreamController

7e01244

refactor: cameraFrameStream handles looping of animation frames

6d20e38

refactor: saperated _triggerAnimationFramesLoop from cameraFrameStrea…

f56879e

…m, and used window.animationFrames

chore: can use off screen canvas constant

1e7c869

chore: removed unnecessary string interpolation from videoElement sty…

5b10bce

…le height and width in takeFrame

chore: reusing the OffscreenCanvas or CanvasElement depending on canU…

90bb72e

…seOffScreenCanvas

refactor: removed getCameraImageDataFromBytes to be used in-line

b196180

fix: setup hasPropertyOffScreenCanvas in tests

5c1c662

TecHaxter added 5 commits November 5, 2024 02:45

[camera_web] chore: removed canUseOffScreenCanvas named paramater fro…

e46ef6f

…m takeFrame function of camera service, using the function directly

[camera_web] chore: initializing canUseOffscreenCanvas from construct…

5e7bb5e

…or instead of intialize()

[camera_web]: replaced dynamic with Object? in map value for willRead…

c48418b

…Frequently

[camera_web] test: camera image stream test cases for with and withou…

a0cd373

…t OffscreenCanvas in camera service

[camera_web] test: camera frame stream test cases for with and withou…

d7b7d5c

…t OffscreenCanvas in camera test

TecHaxter force-pushed the camera_web_stream branch from 5721618 to d7b7d5c Compare November 6, 2024 18:57

Merge branch 'main' into camera_web_stream

6cd955a

TecHaxter requested a review from ditman November 6, 2024 19:10

TecHaxter added 3 commits November 12, 2024 14:17

chore: updated animate function in _triggerAnimationFramesLoop to get…

a959df5

… accurate fps

Merge branch 'main' into camera_web_stream

526160b

Merge branch 'camera_web_stream' of https://github.com/TecHaxter/flut…

98af1e3

…ter_packages into camera_web_stream

stuartmorgan-g added the triage-web Should be looked at in web triage label Jan 14, 2025

mdebbar self-requested a review June 11, 2025 18:23

gemini-code-assist bot reviewed Aug 6, 2025

View reviewed changes

Merge branch 'main' into camera_web_stream

94954d1

mdebbar reviewed Sep 25, 2025

View reviewed changes

TecHaxter added 6 commits September 27, 2025 21:15

[camera_web] chore: addressed review comments

006de9c

Merge branch 'main' into camera_web_stream

f5b3793

[camera_web] fix: reformatting

f0a1d27

[camera_web] fix: todo comment

def248d

[camera_web] fix: todo comment

1b5e9a1

[camera_web] fix: reformatting

33174ca

TecHaxter requested a review from mdebbar September 28, 2025 11:06

		// TODO(replace): introduced fps in
		/// [CameraImageStreamOptions]

	* Supporting camera image stream on web.
	* Adds support for camera image stream on web.

	bool canUseOffscreenCanvas = false;
	final bool canUseOffscreenCanvas;

[camera_web] Re: Support for camera stream on web #7950

Are you sure you want to change the base?

[camera_web] Re: Support for camera stream on web #7950

Conversation

TecHaxter commented Oct 28, 2024

Pre-launch Checklist

Uh oh!

TecHaxter commented Nov 17, 2024

Uh oh!

stuartmorgan-g commented Jan 14, 2025

Uh oh!

stuartmorgan-g commented Mar 25, 2025

Uh oh!

Piinks commented Jun 4, 2025

Uh oh!

mdebbar commented Aug 6, 2025

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

gemini-code-assist bot Aug 6, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Aug 6, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Aug 6, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Aug 6, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Aug 6, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

stuartmorgan-g commented Aug 19, 2025

Uh oh!

mdebbar left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

TecHaxter Sep 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

TecHaxter Sep 27, 2025 •

edited

Loading