Skip to content

[camera] Camera with MediaSettings: platform implementations (federated) - #5223

Merged
auto-submit[bot] merged 224 commits into
flutter:mainfrom
mtbo-org:camera-with-settings-platform-implementations
Apr 5, 2024
Merged

[camera] Camera with MediaSettings: platform implementations (federated)#5223
auto-submit[bot] merged 224 commits into
flutter:mainfrom
mtbo-org:camera-with-settings-platform-implementations

Conversation

@PROGrand

@PROGrandPROGrand commented Oct 24, 2023•
edited
Loading

Copy link
Copy Markdown
Contributor

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), because camera/camera depends on implementations camera/camera_android, camera/camera_web etc.

PROGrandand others added 30 commits March 29, 2023 17:46
@PROGrand
added MediaSettings
f294625
@PROGrand
updated changelog
5ca79b9
@PROGrand
MediaSettings introduced.
e08e046
@PROGrand
Merge branch 'flutter:main' into main
21617de
@PROGrand
excerpts added. formatted.
59d66d3
@PROGrand
versions and changelogs
aeec92e
@PROGrand
CameraPlatform.createCameraWithSettings
869b787
@PROGrand
Merge branch 'main' into main
3cf6030
@PROGrand
Merge branch 'flutter:main' into main
6ac763e
@PROGrand
Update CHANGELOG.md
a40177a
@PROGrand
Update camera_test.dart
fea1fc7
MediaSettings.low changed to const MediaSettings(resolutionPreset: ResolutionPreset.low).
@PROGrand
Update main.dart
9f71f8d
@PROGrand
MediaSettings.low removed
4a0ac77
@PROGrand
media settings low
8fcf1ae
@PROGrand
merger mediasettings
da4ecd8
@PROGrand
resolved multiple@hellohuanlinзкщзÑproposals
16645ef
@PROGrand
fps control
d7dea54
@PROGrand
Fixed pausing preview on already paused or closed session.
6caea7e
@PROGrand
Prevent pausing preview on uninitialized controller.
4bec8f3
@PROGrand
Prevent pausing preview on disposed controller.
11b43c1
@PROGrand
Merge branch 'flutter:main' into main
772e011
@PROGrand
formatted
dbb752d
@PROGrand
Merge branch 'main' into main
7f01b93
@PROGrand@camsim99
Update packages/camera/camera_android/CHANGELOG.md
3506af1
Co-authored-by: Camille Simon <43054281+camsim99@users.noreply.github.com>
@PROGrand@bparrishMines
Update packages/camera/camera/CHANGELOG.md
8f95dc2
Co-authored-by: Maurice Parrish <10687576+bparrishMines@users.noreply.github.com>
@PROGrand
bump gradle:7.20 in CHANGELOG
6b1f96c
@PROGrand
commented feature properties
6f97b51
@PROGrand
android_camera_camerax CHANGELOD update
d96f9d5
@PROGrand
revert default enableAudio to false
401adce
@PROGrand
properly commented IntFeature
8edd212
@PROGrand

PROGrand commented Jan 31, 2024

Copy link
Copy Markdown
ContributorAuthor

@stuartmorgan
Hello, i applied all suggestions. Including your ones for camera_windows. @ditman approved camera_web.

stuartmorgan-g
stuartmorgan-g requested changes Feb 1, 2024
View reviewed changes

@stuartmorgan-gstuartmorgan-g left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Windows mostly looks good; I did a pass of the Obj-C after some of it caught my attention while skimming the other implementations, and I left a number of comments there.

Comment threadpackages/camera/camera_android/example/pubspec.yaml
Comment threadpackages/camera/camera_avfoundation/example/ios/RunnerTests/CameraSettingsTests.m Outdated
Comment threadpackages/camera/camera_avfoundation/example/ios/RunnerTests/CameraSettingsTests.m Outdated
Comment threadpackages/camera/camera_avfoundation/example/ios/RunnerTests/CameraSettingsTests.m Outdated
Comment threadpackages/camera/camera_avfoundation/example/ios/RunnerTests/CameraSettingsTests.m Outdated
Comment threadpackages/camera/camera_avfoundation/ios/Classes/CameraPlugin.m Outdated
Comment threadpackages/camera/camera_avfoundation/ios/Classes/CameraPlugin.m Outdated
Comment threadpackages/camera/camera_avfoundation/ios/Classes/FLTCam.m Outdated
Comment threadpackages/camera/camera_avfoundation/ios/Classes/FLTCam.m Outdated
Comment threadpackages/camera/camera_windows/windows/record_handler.h Outdated
@jmagman

jmagman commented Feb 14, 2024

Copy link
Copy Markdown
Member

@stuartmorgan any more changes requested here? Or is this ready to be merged?

stuartmorgan-g
stuartmorgan-g requested changes Feb 20, 2024
View reviewed changes

@stuartmorgan-gstuartmorgan-g left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks for the test structure changes, that looks much safer than all the class mocking.

Comment threadpackages/camera/camera_android_camerax/example/pubspec.yaml Outdated
Comment threadpackages/camera/camera_avfoundation/example/pubspec.yaml Outdated
Comment threadpackages/camera/camera_web/example/pubspec.yaml Outdated
Comment threadpackages/camera/camera_windows/example/pubspec.yaml Outdated
Comment threadpackages/camera/camera_avfoundation/ios/Classes/CameraPlugin.m Outdated
Comment threadpackages/camera/camera_avfoundation/ios/Classes/FLTCam.h Outdated
Comment threadpackages/camera/camera_avfoundation/ios/Classes/FLTCam.m Outdated
Comment threadpackages/camera/camera_avfoundation/ios/Classes/FLTCam.m Outdated
Comment threadpackages/camera/camera_avfoundation/ios/Classes/FLTCam.m Outdated
Comment threadpackages/camera/camera_avfoundation/ios/Classes/FLTCam.h Outdated
jmagman
jmagman reviewed Mar 6, 2024
View reviewed changes
Comment threadpackages/camera/camera_avfoundation/ios/Classes/FLTCamMediaSettings.m Outdated
Comment threadpackages/camera/camera_avfoundation/ios/Classes/FLTCamMediaSettings.m Outdated
@jmagman

jmagman commented Mar 13, 2024

Copy link
Copy Markdown
Member

@stuartmorgan this is blocked on your review

stuartmorgan-g
stuartmorgan-g requested changes Mar 14, 2024
View reviewed changes
Comment threadpackages/camera/camera_avfoundation/ios/Classes/FLTCamMediaSettings.h Outdated
Comment threadpackages/camera/camera_avfoundation/ios/Classes/FLTCamMediaSettingsAVWrapper.h Outdated
Comment threadpackages/camera/camera_avfoundation/ios/Classes/FLTCamMediaSettingsAVWrapper.h Outdated
Comment threadpackages/camera/camera_avfoundation/example/ios/RunnerTests/CameraSettingsTests.m Outdated
Comment threadpackages/camera/camera_avfoundation/ios/Classes/FLTCamMediaSettings.m Outdated
@PROGrand

PROGrand commented Mar 14, 2024•
edited
Loading

Copy link
Copy Markdown
ContributorAuthor

@stuartmorgan Thank, you for review. Your suggestions are applied.

@vashworth

vashworth commented Apr 3, 2024

Copy link
Copy Markdown
Contributor

@stuartmorgan Can you take a look when you have time?

stuartmorgan-g
stuartmorgan-g requested changes Apr 4, 2024
View reviewed changes

@stuartmorgan-gstuartmorgan-g left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Sorry for the delay again; just a few last small things.

Comment threadpackages/camera/camera_avfoundation/ios/Classes/FLTCamMediaSettings.m Outdated
Comment threadpackages/camera/camera_avfoundation/ios/Classes/FLTCamMediaSettings.h Outdated
Comment threadpackages/camera/camera_avfoundation/ios/Classes/FLTCamMediaSettings.m Outdated
Comment threadpackages/camera/camera_avfoundation/ios/Classes/FLTCam.m Outdated
@PROGrand

PROGrand commented Apr 5, 2024

Copy link
Copy Markdown
ContributorAuthor

@stuartmorgan suggestions applied

stuartmorgan-g
stuartmorgan-g approved these changes Apr 5, 2024
View reviewed changes

@stuartmorgan-gstuartmorgan-g left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM! Could you rebase and squash a few of the commits together to get this under 250 commits, so that the CLA check will pass without intervention?

PROGrandand others added 3 commits April 5, 2024 17:13
@PROGrand
applied suggestions. 05/04/2024
adf6929
suggestions applied
AssertPositiveNumberOrNil: macro to inline
formatted
.class to +class
Update packages/camera/camera_avfoundation/ios/Classes/FLTCamMediaSettings.m
Co-authored-by: Jenn Magder <magder@google.com>
applied suggestions. 02/25/2024.
dependency injection (DI) variant for unit test
ObjC suggestions applied
suggestion applied
updated camera_platform_interface versions
reverted changes to camera
merged 01/07/2024
refactored and co0mmented warning suppressions. renamed MediaRecorderBuilder.RecordingParameters
merged 12/07/2023
@PROGrand
Merge branch 'main' into camera-with-settings-platform-implementations
864b2e1
@PROGrand
rebased, squashed, merged
30837eb
stuartmorgan-g
stuartmorgan-g requested changes Apr 5, 2024
View reviewed changes

@stuartmorgan-gstuartmorgan-g left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

A bunch of files showed as changed since review, and because of the squash I can't tell if that's a GitHub artifact of the squash, or actual changes, so I re-reviewed those files. That turned up a few more minor comments which may be things I just didn't previously review, but I've flagged them regardless. Sorry that means one more quick pass.

Comment threadpackages/camera/camera_web/example/pubspec.yaml Outdated
Comment threadpackages/camera/camera_windows/example/pubspec.yaml Outdated
Comment threadpackages/camera/camera_avfoundation/example/lib/camera_controller.dart Outdated
Comment threadpackages/camera/camera_avfoundation/example/lib/main.dart Outdated
Comment threadpackages/camera/camera_android_camerax/example/pubspec.yaml Outdated
Comment threadpackages/camera/camera_android_camerax/pubspec.yaml Outdated
@PROGrand

PROGrand commented Apr 5, 2024

Copy link
Copy Markdown
ContributorAuthor

@stuartmorgan resolved

stuartmorgan-g
stuartmorgan-g approved these changes Apr 5, 2024
View reviewed changes

@stuartmorgan-gstuartmorgan-g left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks!

@engine-flutter-autorollengine-flutter-autoroll mentioned this pull request Apr 8, 2024
Merged
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bparrishMinesbparrishMinesbparrishMines left review comments
@stuartmorgan-gstuartmorgan-gstuartmorgan-g approved these changes
@jmagmanjmagmanjmagman approved these changes
@ditmanditmanditman approved these changes
@camsim99camsim99camsim99 approved these changes
@cbrackencbrackenAwaiting requested review from cbracken
@hellohuanlinhellohuanlinAwaiting requested review from hellohuanlin
@LouiseHsuLouiseHsuAwaiting requested review from LouiseHsu
@PiinksPiinksAwaiting requested review from Piinks
@HixieHixieAwaiting requested review from Hixie
@gspencergooggspencergoogAwaiting requested review from gspencergoog
@chunhtaichunhtaiAwaiting requested review from chunhtai
@domesticmousedomesticmouseAwaiting requested review from domesticmouse

Assignees

No one assigned

Labels

autosubmitMerge PR when tree becomes green via auto submit Appfederated: partial_changesPR that contains changes for only a single package of a federated plugin changep: cameraplatform-androidplatform-iosplatform-macosplatform-webplatform-windows

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

9 participants

@PROGrand@stuartmorgan-g@bparrishMines@ebjorklund01@jmagman@vashworth@ditman@hellohuanlin@camsim99