[camera_avfoundation] Implementation swift migration - part 8

[RobertOdrowaz]

Migrates camera implementation as part of flutter/flutter#119109

This PR migrates the 5th chunk of FLTCam class to Swift:

• setVideoFormat
• stopVideoRecording
• stopImageStream
• stopping accelerometer updates (deinit)
• setDescriptionWhileRecording
• createConnection (adds Swift implementation, FLTCam still depends on private non-static implementation)

NOTE: setDescriptionWhileRecording is migrated close to verbatim, the issue affecting it remains.

Some properties of the FLTCam have to be temporarily made public so that they are accessible in DefaultCamera.

Pre-Review Checklist

• I read the Contributor Guide and followed the process outlined there for submitting PRs.
• I read the Tree Hygiene page, which explains my responsibilities.
• I read and followed the relevant style guides and ran the auto-formatter.
• I signed the CLA.
• The title of the PR starts with the name of the package surrounded by square brackets, e.g. [shared_preferences]
• I linked to at least one issue that this PR fixes in the description above.
• I updated pubspec.yaml with an appropriate new version according to the pub versioning philosophy, or I have commented below to indicate which version change exemption this PR falls under¹.
• I updated CHANGELOG.md to add a description of the change, following repository CHANGELOG style, or I have commented below to indicate which CHANGELOG exemption this PR falls under¹.
• I updated/added any relevant documentation (doc comments with ///).
• I added new tests to check the change I am making, or I have commented below to indicate which test exemption this PR falls under¹.
• All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

Footnotes

1. Regular contributors who have demonstrated familiarity with the repository guidelines only need to comment if the PR is not auto-exempted by repo tooling. ↩ ↩² ↩³

[stuartmorgan-g]

We're trialing Gemini Code Assist for GitHub in this repo, and I'm going to run it here since this is an interesting category of PR that we don't have many examples of. We may well choose to ignore some of its feedback since the goal is not to change behavior, but I'm curious to see how it handles a Swift migration PR.

[stuartmorgan-g]

/gemini review

[gemini-code-assist]

Hello stuartmorgan-g,

Thank you for initiating this review and for providing the context of this being part of a larger Swift migration effort. I've thoroughly reviewed the changes in this pull request, focusing on correctness, maintainability, and adherence to the migration goals.

Overall, the changes look solid and are a clear step forward in the Swift migration. The logic has been successfully translated from Objective-C to Swift, maintaining the existing functionality and introducing a more Swifty approach where appropriate.

Here's a summary of my findings:

• CHANGELOG.md and pubspec.yaml: The version bump and changelog entries accurately reflect the changes in this PR.
• DefaultCamera.swift: The migration of setVideoFormat, stopVideoRecording, stopImageStream, setDescriptionWhileRecording, and the addition of createConnection are well-executed. The didSet for videoFormat and the deinit for motionManager demonstrate good Swift practices for property observation and resource management. The new createConnection static helper function improves modularity and reusability within the Swift implementation.
• FLTCam.m and FLTCam.h: The corresponding Objective-C code has been correctly refactored by removing the migrated methods and properties. The changes to property attributes (e.g., captureDevice from readonly to strong) in FLTCam.h are necessary to support the Swift subclass's management of these properties, aligning with the migration strategy.

Minor Suggestion:

• Consistency in createConnection's videoFormat usage: In DefaultCamera.swift, within the createConnection static function (lines 74-75 in the RIGHT side of DIFF 2), the videoFormat is cast as Any when setting captureVideoOutput.videoSettings. For consistency with how videoFormat is handled in its didSet (lines 15-17 in DIFF 1 of the same file), and for clearer type representation in Objective-C interop, consider using NSNumber(value: videoFormat) instead of videoFormat as Any.

  // packages/camera/camera_avfoundation/ios/camera_avfoundation/Sources/camera_avfoundation/DefaultCamera.swift
  // Line 74
  captureVideoOutput.videoSettings = [
    kCVPixelBufferPixelFormatTypeKey as String: NSNumber(value: videoFormat)
  ]

  Severity: low - This is a minor stylistic and consistency improvement, not impacting correctness.

Overall, this is a well-structured and correct migration step. Great work, RobertOdrowaz!

Let me know if you have any questions or would like me to elaborate on any points.

[gemini-code-assist]

Code Review

This pull request migrates several methods from Objective-C to Swift, including setVideoFormat, stopVideoRecording, stopImageStream, stopping accelerometer updates, setDescriptionWhileRecording, and createConnection. The changes involve updating the DefaultCamera class and modifying the FLTCam class. The migration appears to be well-structured, with the use of temporary public properties to facilitate access during the transition. The code includes appropriate error handling and checks for various conditions, such as recording status and device capabilities. The changes also include updates to the CHANGELOG.md file to reflect the migrated methods.

[stuartmorgan-g]

Hm, that was much worse than the first Java change we tried it on. It'll be interesting to see if poor-quality suggestions for Swift is a pattern.