[camera_avfoundation] Implementation swift migration - part 11 #9690
Conversation
|
It looks like this pull request may not have tests. Please make sure to add tests or get an explicit test exemption before merging. If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix? Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.If you believe this PR qualifies for a test exemption, contact "@test-exemption-reviewer" in the #hackers channel in Discord (don't just cc them here, they won't see it!). The test exemption team is a small volunteer group, so all reviewers should feel empowered to ask for tests, without delegating that responsibility entirely to the test exemption group. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request continues the migration of the camera_avfoundation plugin from Objective-C to Swift. It successfully migrates startImageStream and setUpCaptureSessionForAudioIfNeeded, and removes the old Objective-C code. The changes are mostly a direct translation, but I found a critical issue in the new setUpCaptureSessionForAudioIfNeeded implementation where a class property was not being set, which could lead to a crash. I've provided a suggestion to fix this.
.../camera_avfoundation/ios/camera_avfoundation/Sources/camera_avfoundation/DefaultCamera.swift
Show resolved
Hide resolved
|
@hellohuanlin take a look at this PR when you have a moment pls |
|
Test examption reason: refactors with no semantic change (e.g. null safety migrations) |
|
@RobertOdrowaz sorry i was out earlier this week. Will take a look later today. |
.../camera_avfoundation/ios/camera_avfoundation/Sources/camera_avfoundation/DefaultCamera.swift
Show resolved
Hide resolved
RobertOdrowaz
force-pushed
the
feature/camera-implementation-swift-migration-part11
branch
from
a1253a9 to
71d55a4
Compare
|
test-exempt: code refactor with no semantic change |
RobertOdrowaz
added
the
autosubmit
flutter/packages@34948d1...a114ac2 2025-08-10 [email protected] [camera_avfoundation] Implementation swift migration - part 11 (flutter/packages#9690) 2025-08-10 [email protected] [camera_avfoundation] Fix crash when streaming while recording (flutter/packages#9691) 2025-08-09 [email protected] Roll Flutter from 3821790 to 1590543 (14 revisions) (flutter/packages#9780) 2025-08-08 [email protected] Roll Flutter from 92a6bfb to 3821790 (17 revisions) (flutter/packages#9778) 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] 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
…r#173556) flutter/packages@34948d1...a114ac2 2025-08-10 [email protected] [camera_avfoundation] Implementation swift migration - part 11 (flutter/packages#9690) 2025-08-10 [email protected] [camera_avfoundation] Fix crash when streaming while recording (flutter/packages#9691) 2025-08-09 [email protected] Roll Flutter from 3821790 to 1590543 (14 revisions) (flutter/packages#9780) 2025-08-08 [email protected] Roll Flutter from 92a6bfb to 3821790 (17 revisions) (flutter/packages#9778) 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] 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: (56 commits) [go_router_builder] Support extension types (flutter#9458) Roll Flutter from e2a347b14a18 to 34c2a3b158b2 (41 revisions) (flutter#9803) [go_router_builder] Migrate to Element2 API and update dependencies (flutter#9649) [in_app_purchase_storekit] Add support for quantity in consumable product purchases (#171570) (flutter#9698) [pigeon] Improves documentation of `ProxyApi` and moves helper functions to a separate file (flutter#9756) Roll Flutter from 1590543f6794 to e2a347b14a18 (1 revision) (flutter#9784) [camera_avfoundation] Implementation swift migration - part 11 (flutter#9690) [camera_avfoundation] Fix crash when streaming while recording (flutter#9691) Roll Flutter from 38217906e95c to 1590543f6794 (14 revisions) (flutter#9780) Roll Flutter from 92a6bfbfd6ef to 38217906e95c (17 revisions) (flutter#9778) [video_player] Improve KVO handling on iOS (flutter#9718) [dependabot]: Bump the test-dependencies group across 15 directories with 7 updates (flutter#9736) Roll Flutter from 9de63a03428f to 92a6bfbfd6ef (11 revisions) (flutter#9769) [google_maps_flutter_platform_interface] Add Advanced markers support (flutter#9737) [camera_android_camerax] Re-land "Force new Surface for each SurfaceRequest" (flutter#9760) [google_maps_flutter] Add ability to perform Google Maps SDK warmup (flutter#9674) Roll Flutter from 59fc766c6fdf to 9de63a03428f (6 revisions) (flutter#9764) [webview_flutter_wkwebview] Extended Web View API on iOS to add flexibility when working with local HTML content (flutter#8787) [local_auth] Differentiate iOS authentication errors (flutter#9705) manual roll to 59fc766c6fdfd03d0983fc95ce8b76793a300dd5 (flutter#9758) ... # Conflicts: # packages/go_router_builder/lib/src/type_helpers.dart # packages/go_router_builder/pubspec.yaml
Migrates camera implementation as part of flutter/flutter#119109
Resolves flutter/flutter#170439. This PR migrates the last one of the problematic methods (
startImageStream) to Switft, which resolves the issue.This PR migrates the 8th chunk of
FLTCamclass to Swift:startImageStreamsetUpCaptureSessionForAudioIfNeededreportErrorMessage(ObjC implementation removal)Some properties of the FLTCam have to be temporarily made public so that they are accessible in DefaultCamera.
Pre-Review Checklist
[shared_preferences]pubspec.yamlwith an appropriate new version according to the pub versioning philosophy, or I have commented below to indicate which version change exemption this PR falls under1.CHANGELOG.mdto add a description of the change, following repository CHANGELOG style, or I have commented below to indicate which CHANGELOG exemption this PR falls under1.///).If you need help, consider asking for advice on the #hackers-new channel on Discord.
Note: The Flutter team is currently trialing the use of Gemini Code Assist for GitHub. Comments from the
gemini-code-assistbot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed.Footnotes
Regular contributors who have demonstrated familiarity with the repository guidelines only need to comment if the PR is not auto-exempted by repo tooling. ↩ ↩2 ↩3