-
Notifications
You must be signed in to change notification settings - Fork 3.2k
[video_player_avfoundation] iOS platform view support #8237
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
[video_player_avfoundation] iOS platform view support #8237
Conversation
… and platform view for each video
0b8d7ec
to
3b55eec
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 minor comments (none that need another round of review, and feel free to ignore the optional ones if you disagree with them). For the Id
/Identifier
thing, you can use your judgement about where to stop renaming.
If there's anything you'd specifically like me to re-review let me know and I'll take a look, but otherwise feel free to land once the comments are addressed!
...undation/darwin/video_player_avfoundation/Sources/video_player_avfoundation/FVPVideoPlayer.m
Outdated
Show resolved
Hide resolved
...undation/darwin/video_player_avfoundation/Sources/video_player_avfoundation/FVPVideoPlayer.m
Outdated
Show resolved
Hide resolved
...win/video_player_avfoundation/Sources/video_player_avfoundation/FVPTextureBasedVideoPlayer.m
Show resolved
Hide resolved
...on/darwin/video_player_avfoundation/Sources/video_player_avfoundation/FVPVideoPlayerPlugin.m
Outdated
Show resolved
Hide resolved
...on/darwin/video_player_avfoundation/Sources/video_player_avfoundation/FVPVideoPlayerPlugin.m
Outdated
Show resolved
Hide resolved
...on/darwin/video_player_avfoundation/Sources/video_player_avfoundation/FVPVideoPlayerPlugin.m
Outdated
Show resolved
Hide resolved
...on/darwin/video_player_avfoundation/Sources/video_player_avfoundation/FVPVideoPlayerPlugin.m
Outdated
Show resolved
Hide resolved
...rces/video_player_avfoundation/include/video_player_avfoundation/FVPNativeVideoViewFactory.h
Outdated
Show resolved
Hide resolved
...ources/video_player_avfoundation/include/video_player_avfoundation/FVPVideoPlayer_Internal.h
Show resolved
Hide resolved
@stuartmorgan @hellohuanlin Thanks for the review, I addressed all of your comments. For Thanks for the iteration here, I'm merging it now. |
flutter/packages@625023a...8542af3 2025-02-15 stuartmorgan@google.com [various] Enable `permissive-` for Windows plugin examples (flutter/packages#8636) 2025-02-15 stuartmorgan@google.com [pigeon] Update task queue handling (flutter/packages#8627) 2025-02-14 goderbauer@google.com Update CODEOWNERS (flutter/packages#8628) 2025-02-14 32538273+ValentinVignal@users.noreply.github.com [flutter_adaptive_scaffold] Fix some memory leaks (flutter/packages#8546) 2025-02-14 mchudy@users.noreply.github.com [camera] Fix crash when setting activeFormat on FLTCaptureDevice (flutter/packages#8630) 2025-02-13 mchudy@users.noreply.github.com [camera] Remove remaining OCMock usage in tests (flutter/packages#8624) 2025-02-13 10687576+bparrishMines@users.noreply.github.com [webview_flutter_wkwebview] Change callback methods with a non-null return type to non-null (flutter/packages#8564) 2025-02-12 737941+loic-sharma@users.noreply.github.com [google_sign_in_ios] Adds Swift Package Manager support (flutter/packages#7356) 2025-02-12 pawel.jakubowski@leancode.pl [camera_avfoundation] Migrate tests to Swift - part 1 (flutter/packages#8603) 2025-02-12 43054281+camsim99@users.noreply.github.com [video_player] Re-enables `asset videos live stream duration != 0` test for Android (flutter/packages#8610) 2025-02-12 32538273+ValentinVignal@users.noreply.github.com [go_router_builder] Add support for `TypedStatefulShellBranch`'s `preload` (flutter/packages#8587) 2025-02-12 magder@google.com [local_auth_darwin] Fix test name for clarity (flutter/packages#8499) 2025-02-11 ditman@gmail.com [ci] Manually roll master, set -Xmx4G (flutter/packages#8586) 2025-02-11 49699333+dependabot[bot]@users.noreply.github.com [dependabot]: Bump the gradle-plugin group across 4 directories with 1 update (flutter/packages#8551) 2025-02-10 10687576+bparrishMines@users.noreply.github.com [pigeon] Add errors for ProxyAPI callback methods and null instances when reading in a ProxyApiBaseCodec (flutter/packages#8567) 2025-02-10 98884136+berhili098@users.noreply.github.com [shared_preferences]Fix : SetState returning future (flutter/packages#8398) 2025-02-10 stuartmorgan@google.com [various] Add deprecation notices to READMEs (flutter/packages#8598) 2025-02-10 mchudy@users.noreply.github.com [camera] Remove OCMock from CameraSettingsTests, CameraMethodChannelTests and CameraSessionPresetsTests (flutter/packages#8592) 2025-02-10 mchudy@users.noreply.github.com [camera] Remove OCMock from FLTCamPhotoCaptureTests, FLTSavePhotoDelegateTests and StreamingTests (flutter/packages#8590) 2025-02-07 32538273+ValentinVignal@users.noreply.github.com [go_router] Add `preload` parameter to `StatefulShellBranchData.$branch` (flutter/packages#8545) 2025-02-07 pawel.jakubowski@leancode.pl [video_player_avfoundation] iOS platform view support (flutter/packages#8237) 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 flutter-ecosystem@google.com 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
This PR adds support for platform views on iOS as a way of displaying a video. When creating a video, it's now possible to choose between texture view approach (rendered using
Texture
widget on the Flutter side) and platform view approach (rendered on the native side, usingAVPlayerLayer
).FVPVideoPlayer
class now has nothing to do with texture. The texture-related code was moved from it toFVPTextureBasedVideoPlayer
- a subclass ofFVPVideoPlayer
that adds texture functionality. In the plugin class (createWithOptions
method) we create either the basic version (for platform view) or the texture subclass (in case of texture approach) based on the parameter passed in from Flutter side.Platform view is only supported on iOS, no MacOS implementation is added in this PR. The functionality is not yet exposed in the app-facing package (only in example app) - it will be done later, once we add the Android implementation. The PR does not introduce breaking changes, I followed the rule "non-breaking changes, even at the expense of a less-clean API" (
buildViewWithOptions
andcreateWithOptions
methods).Up to this point, the variable naming relied heavily on the texture (we had a lot of
textureId
variables and properties). Since now you can use a platform view instead of a texture view, these variables and parameters were renamed to justplayerId
.I left some comments in the PR to clarify/discuss some choices.
Resolves #86613 (not fully though, as it is not yet available in the app-facing package).
Pre-launch Checklist
dart format
.)[shared_preferences]
pubspec.yaml
with an appropriate new version according to the pub versioning philosophy, or this PR is exempt from version changes.CHANGELOG.md
to add a description of the change, following repository CHANGELOG style, or this PR is exempt from CHANGELOG changes.///
).If you need help, consider asking for advice on the #hackers-new channel on Discord.