-
Notifications
You must be signed in to change notification settings - Fork 3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[camera] CameraPlatform.createCameraWithSettings #3615
[camera] CameraPlatform.createCameraWithSettings #3615
Conversation
This looks like a change that needs to follow https://github.com/flutter/flutter/wiki/Contributing-to-Plugins-and-Packages#changing-federated-plugins. This PR doesn't indicate how Android, iOS, and the app-facing package will use this new feature. |
Updated PR description. This is second PR (according to https://github.com/flutter/flutter/wiki/Contributing-to-Plugins-and-Packages#changing-federated-plugins). First PR is here: #3586. Can you please review #3586 ? |
hello, how is it going with this feature? i'd like to add it to my current app, i'm looking forward to it |
Update from triage: this is waiting for the sub-PR referenced above to complete the review process. |
@PROGrand The sub-issue was closed due to lack of updates. Have you decided not to move forward with this? |
Update from triage: the sub-PR has been re-opened, so this is once again waiting on that to complete the review process. |
It looks like I had the ordering inverted in my comments above; this is the sub-PR, and we were waiting for the combo PR to pass review. That seems to be done; @bparrishMines it looks like this is ready for your review. |
packages/camera/camera_platform_interface/lib/src/types/media_settings.dart
Show resolved
Hide resolved
@PROGrand Are you still planning on updating this so that it's ready for another review? |
367bf52
to
1659383
Compare
1659383
to
c2b214d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM with a couple nits
@stuartmorgan / @camsim99 for secondary review
packages/camera/camera_platform_interface/lib/src/method_channel/method_channel_camera.dart
Outdated
Show resolved
Hide resolved
packages/camera/camera_platform_interface/lib/src/method_channel/method_channel_camera.dart
Outdated
Show resolved
Hide resolved
packages/camera/camera_platform_interface/lib/src/method_channel/method_channel_camera.dart
Outdated
Show resolved
Hide resolved
packages/camera/camera_platform_interface/lib/src/types/media_settings.dart
Show resolved
Hide resolved
packages/camera/camera_platform_interface/lib/src/types/media_settings.dart
Outdated
Show resolved
Hide resolved
packages/camera/camera_platform_interface/lib/src/types/media_settings.dart
Outdated
Show resolved
Hide resolved
packages/camera/camera_platform_interface/lib/src/types/media_settings.dart
Outdated
Show resolved
Hide resolved
packages/camera/camera_platform_interface/test/camera_platform_interface_test.dart
Outdated
Show resolved
Hide resolved
packages/camera/camera_platform_interface/lib/src/types/media_settings.dart
Show resolved
Hide resolved
c77fbec
to
9cf640c
Compare
9cf640c
to
393b579
Compare
cf15472
to
ce4bed7
Compare
packages/camera/camera_platform_interface/test/types/media_settings_test.dart
Outdated
Show resolved
Hide resolved
packages/camera/camera_platform_interface/test/camera_platform_interface_test.dart
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks!
flutter/packages@be915be...4bf5114 2023-10-23 [email protected] [camera] CameraPlatform.createCameraWithSettings (flutter/packages#3615) 2023-10-23 49699333+dependabot[bot]@users.noreply.github.com Bump github/codeql-action from 2.22.3 to 2.22.4 (flutter/packages#5201) 2023-10-22 [email protected] Roll Flutter from 6f4850d to 823e083 (3 revisions) (flutter/packages#5198) 2023-10-21 49699333+dependabot[bot]@users.noreply.github.com Bump actions/checkout from 4.1.0 to 4.1.1 (flutter/packages#5167) 2023-10-21 [email protected] Roll Flutter from 0883cb2 to 6f4850d (24 revisions) (flutter/packages#5196) 2023-10-21 [email protected] [ios_platform_images] migrate objC to swift (flutter/packages#4847) 2023-10-20 49699333+dependabot[bot]@users.noreply.github.com [in_app_pur]: Bump org.json:json from 20230618 to 20231013 in /packages/in_app_purchase/in_app_purchase_android/example/android/app (flutter/packages#5149) 2023-10-20 49699333+dependabot[bot]@users.noreply.github.com [in_app_pur]: Bump org.json:json from 20230618 to 20231013 in /packages/in_app_purchase/in_app_purchase_android/android (flutter/packages#5150) 2023-10-20 [email protected] [quick_actions] convert to pigeon (flutter/packages#5159) 2023-10-20 [email protected] [ci] Add build-only Windows Arm64 tests (flutter/packages#5142) 2023-10-20 [email protected] Add '--no-tree-shake-icons' option to `BenchmarkServer` (flutter/packages#5186) 2023-10-20 [email protected] Roll Flutter from c2bd2c1 to 0883cb2 (24 revisions) (flutter/packages#5192) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages-flutter-autoroll Please CC [email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
* main: (139 commits) Change firebase device tests from Android api 30 to 33 (flutter#5185) [url_launcher] Add an `inAppBrowserView` mode (flutter#5205) [ci] Remove device_type property from Windows Arm64 properties (flutter#5193) [url_launcher_web] Disallows launching "javascript:" URLs. (flutter#5180) [local_auth]: Bump androidx.fragment:fragment from 1.6.0 to 1.6.1 in /packages/local_auth/local_auth_android/android (flutter#4600) Roll Flutter from 823e083 to 5e8b5f4 (13 revisions) (flutter#5208) [tool] Add optional swift-format support (flutter#5204) [camera] CameraPlatform.createCameraWithSettings (flutter#3615) Bump github/codeql-action from 2.22.3 to 2.22.4 (flutter#5201) Roll Flutter from 6f4850d to 823e083 (3 revisions) (flutter#5198) Bump actions/checkout from 4.1.0 to 4.1.1 (flutter#5167) Roll Flutter from 0883cb2 to 6f4850d (24 revisions) (flutter#5196) [ios_platform_images] migrate objC to swift (flutter#4847) [in_app_pur]: Bump org.json:json from 20230618 to 20231013 in /packages/in_app_purchase/in_app_purchase_android/example/android/app (flutter#5149) [in_app_pur]: Bump org.json:json from 20230618 to 20231013 in /packages/in_app_purchase/in_app_purchase_android/android (flutter#5150) [quick_actions] convert to pigeon (flutter#5159) [ci] Add build-only Windows Arm64 tests (flutter#5142) Add '--no-tree-shake-icons' option to `BenchmarkServer` (flutter#5186) Roll Flutter from c2bd2c1 to 0883cb2 (24 revisions) (flutter#5192) [ci] Finalize migration to x64 specific Windows platform (flutter#5174) ...
## Platform interface of federated plugin This is the `platform-interface` part of `camera` PR flutter#3586. ## App-facing change Previously, CameraController was unable to setup fps and bitrates, allowing only resolution settings like this: ```dart controller = CameraController(_cameras[0], ResolutionPreset.max); ``` This PR gives additional functionality to set `fps` and `bitrates` via `withSettings`: ```dart controller = CameraController.withSettings( _cameras[0], mediaSettings: const MediaSettings( resolutionPreset: ResolutionPreset.low, fps: 15, videoBitrate: 200000, audioBitrate: 32000, enableAudio: true, ), ); ``` ## Android, iOS, etc. All platforms must implement `CameraPlatform.createCameraWithSettings` in addition to `CameraPlatform.createCamera`, providing platform specific code for `fps` and `bitrate' platform: ```dart Future<int> createCamera( CameraDescription cameraDescription, CameraDescription cameraDescription, ResolutionPreset? resolutionPreset, { ResolutionPreset? resolutionPreset, { bool enableAudio = false, bool enableAudio = false, }) { }) => throw UnimplementedError('createCamera() is not implemented.'); createCameraWithSettings( cameraDescription, MediaSettings( resolutionPreset: resolutionPreset, enableAudio: enableAudio, ), ); /// Creates an uninitialized camera instance and returns the cameraId. Future<int> createCameraWithSettings( CameraDescription cameraDescription, MediaSettings? mediaSettings, ) { throw UnimplementedError('createCameraWithSettings() is not implemented.'); } } ```
…ed) (#5223) ## Platform implementations of federated plugin This is the `platform implementations` part of `camera` PR #3586. `camera_platform_interface: 2.6.0` merged and published in PR #3615. Now repeating steps 3,4 (see [Changing federated plugins](https://github.com/flutter/flutter/wiki/Contributing-to-Plugins-and-Packages#changing-federated-plugins)), because `camera/camera` depends on implementations `camera/camera_android`, `camera/camera_web` etc.
This comment was marked as off-topic.
This comment was marked as off-topic.
You should wait for #3586, or import camera from https://github.com/mtbo-org/packages |
…ed) (flutter#5223) This is the `platform implementations` part of `camera` PR flutter#3586. `camera_platform_interface: 2.6.0` merged and published in PR flutter#3615. Now repeating steps 3,4 (see [Changing federated plugins](https://github.com/flutter/flutter/wiki/Contributing-to-Plugins-and-Packages#changing-federated-plugins)), because `camera/camera` depends on implementations `camera/camera_android`, `camera/camera_web` etc.
…ed) (flutter#5223) ## Platform implementations of federated plugin This is the `platform implementations` part of `camera` PR flutter#3586. `camera_platform_interface: 2.6.0` merged and published in PR flutter#3615. Now repeating steps 3,4 (see [Changing federated plugins](https://github.com/flutter/flutter/wiki/Contributing-to-Plugins-and-Packages#changing-federated-plugins)), because `camera/camera` depends on implementations `camera/camera_android`, `camera/camera_web` etc.
Platform interface of federated plugin
This is the
platform-interface
part ofcamera
PR #3586.App-facing change
Previously, CameraController was unable to setup fps and bitrates, allowing only resolution settings like this:
This PR gives additional functionality to set
fps
andbitrates
viawithSettings
:Android, iOS, etc.
All platforms must implement
CameraPlatform.createCameraWithSettings
in addition toCameraPlatform.createCamera
, providing platform specific code forfps
and `bitrate' platform: