[camera_web] Re: Support for camera stream on web#7950
[camera_web] Re: Support for camera stream on web#7950TecHaxter wants to merge 73 commits intoflutter:mainfrom
Conversation
…d before expecting mock response
…d before expecting mock response
…d before expecting mock response
…d before expecting mock response
…d before expecting mock response
…d before expecting mock response
…m, and used window.animationFrames
…le height and width in takeFrame
…seOffScreenCanvas
|
From triage: @mdebbar This looks to be ready for re-review. |
|
From triage: @mdebbar, ping on this re-review. |
|
From triage: Still waiting on web team review. |
mdebbar
left a comment
mdebbar
left a comment
There was a problem hiding this comment.
Overall looks good! Sorry for the long delay.
I only have a few suggestions and nits. After that it should be good to go. Thank you for the contribution!
| final bool canUseOffscreenCanvas; | ||
|
|
||
| /// The tolerance for the camera streaming frame time. | ||
| final int _frameTimeToleranceMs = 8; |
There was a problem hiding this comment.
Can this be a getter that changes dynamically based on the fps value?
E.g.
| final int _frameTimeToleranceMs = 8; | |
| final get int _frameTimeToleranceMs => 1000 / cameraStreamFPS ~/ 2; |
| ///Used in [takeFrame] if `OffscreenCanvas` is supported | ||
| web.OffscreenCanvas? _offscreenCanvas; | ||
|
|
||
| ///Returns frame at a specific time using video element |
There was a problem hiding this comment.
| ///Returns frame at a specific time using video element | |
| /// Returns frame at a specific time using video element |
| ///Used in [takeFrame] if `OffscreenCanvas` is not supported | ||
| web.CanvasElement? _canvasElement; | ||
|
|
||
| ///Used in [takeFrame] if `OffscreenCanvas` is supported |
There was a problem hiding this comment.
| ///Used in [takeFrame] if `OffscreenCanvas` is supported | |
| /// Used in [takeFrame] if `OffscreenCanvas` is supported |
| return jsUtil.hasProperty(window, 'OffscreenCanvas'.toJS); | ||
| } | ||
|
|
||
| ///Used in [takeFrame] if `OffscreenCanvas` is not supported |
There was a problem hiding this comment.
| ///Used in [takeFrame] if `OffscreenCanvas` is not supported | |
| /// Used in [takeFrame] if `OffscreenCanvas` is not supported |
| } | ||
| } | ||
|
|
||
| ///Used to check if browser has OffscreenCanvas capability |
There was a problem hiding this comment.
| ///Used to check if browser has OffscreenCanvas capability | |
| /// Used to check if browser has OffscreenCanvas capability |
| _offscreenCanvas!.width != width || | ||
| _offscreenCanvas!.height != height) { |
There was a problem hiding this comment.
Can we resize the existing offscreen canvas instead of creating a new one?
There was a problem hiding this comment.
That would be a great optimization. Done👍
I have also optimized the offscreen canvas context call by preserving it from the first call.
Please do take a final look at that. Many thanks.
| _canvasElement!.width != width || | ||
| _canvasElement!.height != height) { |
| 'Computed dimensions are zero: width=$width, height=$height', | ||
| ); | ||
| } | ||
| late web.ImageData imageData; |
There was a problem hiding this comment.
I think this can be:
| late web.ImageData imageData; | |
| final web.ImageData imageData; |
DemoDummy example app source code 2026-04-03.19-47-24.mp4 |
|
Some lints have been reported here: https://logs.chromium.org/logs/flutter/buildbucket/cr-buildbucket/8685251527310718641/+/u/Run_package_tests/analyze/stdout |
mdebbar
dismissed
ditman’s stale review
Many changes have been done since this review.
| ({int? audioBitrate, int? videoBitrate})? recorderOptions, | ||
| }) : recorderOptions = | ||
| recorderOptions ?? (audioBitrate: null, videoBitrate: null), |
There was a problem hiding this comment.
Why this change?
mockitowalks the constructor and inspects parameter defaults.- For a
constrecord literal, it goes throughsource_genconstant revival (reviveInstance). - That code path of the current version of the dependency does not handle record constants properly.
- As a result, it hits a null and throws Null check operator used on a null value.
6 participants
This PR aims to provide support for strartImageStream and stopImageStream on Web.
#92460
Based on #6944 from the archived plugins repository
Pre-launch Checklist
dart format.)[shared_preferences]pubspec.yamlwith an appropriate new version according to the [pub versioning philosophy], or this PR is [exempt from version changes].CHANGELOG.mdto add a description of the change, [following repository CHANGELOG style].///).POV: my first PR on a public repo
Contains required commits from the PR #6443, resolves unnecessary 202 commits