Skip to content

[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

Merged

Conversation

FirentisTFW
Copy link
Contributor

@FirentisTFW FirentisTFW commented Dec 6, 2024

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, using AVPlayerLayer).

FVPVideoPlayer class now has nothing to do with texture. The texture-related code was moved from it to FVPTextureBasedVideoPlayer - a subclass of FVPVideoPlayer 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 and createWithOptions 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 just playerId.

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

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

@FirentisTFW FirentisTFW force-pushed the feature/video-player-platform-view-support branch from 0b8d7ec to 3b55eec Compare February 5, 2025 16:29
Copy link
Contributor

@stuartmorgan-g stuartmorgan-g left a 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!

@FirentisTFW
Copy link
Contributor Author

@stuartmorgan @hellohuanlin Thanks for the review, I addressed all of your comments.

For Id/Identifier - I renamed the Id to Identifier where possible. It worked great, now Id is mostly left in pigeon-generated files.
I also applied optional suggestions from the last review.

Thanks for the iteration here, I'm merging it now.

@FirentisTFW FirentisTFW merged commit 24d6d9c into flutter:main Feb 7, 2025
78 checks passed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Feb 10, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Feb 17, 2025
github-merge-queue bot pushed a commit to flutter/flutter that referenced this pull request Feb 17, 2025
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants