feat(ios): add audio track selection support for AVFoundation #10313
Conversation
nateshmbhat
requested review from
LongCatIsLooong and
hellohuanlin
as code owners
![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 introduces audio track selection support for AVFoundation, enabling users to retrieve and select audio tracks from both HLS streams and regular video files. The changes include modifications to the CHANGELOG, platform-specific code (ObjC, Swift), example app, and test files. The minimum supported SDK versions have been updated, and new methods (getAudioTracks(), selectAudioTrack()) have been implemented. The code adheres to the repository's style guide, focusing on objective summaries and using code as the source of truth.
|
|
||
| * Updates minimum supported version to iOS 13 and macOS 10.15. | ||
| * Updates minimum supported SDK version to Flutter 3.35/Dart 3.9. | ||
| * Implements `getAudioTracks()` and `selectAudioTrack()` methods. |
There was a problem hiding this comment.
It's great to see the new features documented in the changelog. However, consider adding a bit more detail about the specific use cases or benefits of these methods for developers.
packages/video_player/video_player_avfoundation/darwin/video_player_avfoundation.podspec
Outdated
Show resolved
Hide resolved
packages/video_player/video_player_avfoundation/darwin/video_player_avfoundation/Package.swift
Outdated
Show resolved
Hide resolved
| // Skip nil options | ||
| if (!option || [option isKindOfClass:[NSNull class]]) { | ||
| continue; |
There was a problem hiding this comment.
Consider adding a comment explaining why nil options are skipped. This will help future developers understand the logic behind this check.
| } | ||
| // For asset tracks, we don't have a direct way to select them in AVFoundation | ||
| // This would require more complex track selection logic that's not commonly used |
There was a problem hiding this comment.
Add a comment explaining why direct selection isn't possible and what the limitations are. This will help future developers understand the current implementation and potential areas for improvement.
packages/video_player/video_player_avfoundation/example/ios/Flutter/AppFrameworkInfo.plist
Outdated
Show resolved
Hide resolved
packages/video_player/video_player_avfoundation/example/ios/Runner.xcodeproj/project.pbxproj
Outdated
Show resolved
Hide resolved
packages/video_player/video_player_avfoundation/example/macos/Podfile
Outdated
Show resolved
Hide resolved
packages/video_player/video_player_avfoundation/example/macos/Runner.xcodeproj/project.pbxproj
Outdated
Show resolved
Hide resolved
| assets: | ||
| - assets/flutter-mark-square-64.png | ||
| - assets/Butterfly-209.mp4 | ||
| # FOR TESTING AND INITIAL REVIEW ONLY. DO NOT MERGE. |
There was a problem hiding this comment.
Per the repository style guide, the review agent should not comment about dependency_overrides sections in pubspec.yaml files that have a comment starting with: "FOR TESTING AND INITIAL REVIEW ONLY. DO NOT MERGE." 1
Style Guide References
Footnotes
- Added getAudioTracks() method supporting both HLS media selection and regular asset tracks - Added selectAudioTrack() method with support for both track selection mechanisms - Included comprehensive test coverage for audio track functionality with edge cases
nateshmbhat
force-pushed
the
28-oct-platform-avfoundation
branch
from
4784bd8 to
459e7f8
Compare
nateshmbhat
changed the title
- Update iOS minimum deployment target from 12.0 to 13.0 - Update macOS minimum deployment target from 10.14 to 10.15 - Add CocoaPods framework embedding build phases to Xcode projects
|
@LongCatIsLooong @tarrinneal could you please review this PR |
2 participants
AVFoundation Platform PR for : #9925
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 under[^1].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 under[^1].///).