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
auto-submit bot pushed a commit that referenced this pull request Jun 3, 2025
This PR adds support for platform views as on optional way of displaying a video (as an alternative to Flutter's `Texture` widget). Texture-based approach is still the default setting when creating a new player.

Platform interface was updated in #8453 
Platform implementations were added in these PRs:
- iOS: #8237 
- Android: #8466 

Closes [flutter/issues/86613](flutter/flutter#86613).
androidseb pushed a commit to androidseb/packages that referenced this pull request Jun 8, 2025
This PR is a subset of flutter#8237 - it only adds platform interface changes.

Partially resolves [86613](flutter/flutter#86613).
androidseb pushed a commit to androidseb/packages that referenced this pull request Jun 8, 2025
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](flutter/flutter#86613) (not
fully though, as it is not yet available in the app-facing package).

## Pre-launch Checklist

- [x] I read the [Contributor Guide] and followed the process outlined
there for submitting PRs.
- [x] I read the [Tree Hygiene] page, which explains my
responsibilities.
- [x] I read and followed the [relevant style guides] and ran the
auto-formatter. (Unlike the flutter/flutter repo, the flutter/packages
repo does use `dart format`.)
- [x] I signed the [CLA].
- [x] The title of the PR starts with the name of the package surrounded
by square brackets, e.g. `[shared_preferences]`
- [x] I [linked to at least one issue that this PR fixes] in the
description above.
- [x] I updated `pubspec.yaml` with an appropriate new version according
to the [pub versioning philosophy], or this PR is [exempt from version
changes].
- [x] I updated `CHANGELOG.md` to add a description of the change,
[following repository CHANGELOG style], or this PR is [exempt from
CHANGELOG changes].
- [x] I updated/added relevant documentation (doc comments with `///`).
- [x] I added new tests to check the change I am making, or this PR is
[test-exempt].
- [x] All existing and new tests are passing.

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

<!-- Links -->
[Contributor Guide]:
https://github.com/flutter/packages/blob/main/CONTRIBUTING.md
[Tree Hygiene]:
https://github.com/flutter/flutter/blob/master/docs/contributing/Tree-hygiene.md
[relevant style guides]:
https://github.com/flutter/packages/blob/main/CONTRIBUTING.md#style
[CLA]: https://cla.developers.google.com/
[Discord]:
https://github.com/flutter/flutter/blob/master/docs/contributing/Chat.md
[linked to at least one issue that this PR fixes]:
https://github.com/flutter/flutter/blob/master/docs/contributing/Tree-hygiene.md#overview
[pub versioning philosophy]: https://dart.dev/tools/pub/versioning
[exempt from version changes]:
https://github.com/flutter/flutter/blob/master/docs/ecosystem/contributing/README.md#version
[following repository CHANGELOG style]:
https://github.com/flutter/flutter/blob/master/docs/ecosystem/contributing/README.md#changelog-style
[exempt from CHANGELOG changes]:
https://github.com/flutter/flutter/blob/master/docs/ecosystem/contributing/README.md#changelog
[test-exempt]:
https://github.com/flutter/flutter/blob/master/docs/contributing/Tree-hygiene.md#tests
FMorschel pushed a commit to FMorschel/packages that referenced this pull request Jun 9, 2025
This PR is a subset of flutter#8237 - it only adds platform interface changes.

Partially resolves [86613](flutter/flutter#86613).
FMorschel pushed a commit to FMorschel/packages that referenced this pull request Jun 9, 2025
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](flutter/flutter#86613) (not
fully though, as it is not yet available in the app-facing package).

## Pre-launch Checklist

- [x] I read the [Contributor Guide] and followed the process outlined
there for submitting PRs.
- [x] I read the [Tree Hygiene] page, which explains my
responsibilities.
- [x] I read and followed the [relevant style guides] and ran the
auto-formatter. (Unlike the flutter/flutter repo, the flutter/packages
repo does use `dart format`.)
- [x] I signed the [CLA].
- [x] The title of the PR starts with the name of the package surrounded
by square brackets, e.g. `[shared_preferences]`
- [x] I [linked to at least one issue that this PR fixes] in the
description above.
- [x] I updated `pubspec.yaml` with an appropriate new version according
to the [pub versioning philosophy], or this PR is [exempt from version
changes].
- [x] I updated `CHANGELOG.md` to add a description of the change,
[following repository CHANGELOG style], or this PR is [exempt from
CHANGELOG changes].
- [x] I updated/added relevant documentation (doc comments with `///`).
- [x] I added new tests to check the change I am making, or this PR is
[test-exempt].
- [x] All existing and new tests are passing.

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

<!-- Links -->
[Contributor Guide]:
https://github.com/flutter/packages/blob/main/CONTRIBUTING.md
[Tree Hygiene]:
https://github.com/flutter/flutter/blob/master/docs/contributing/Tree-hygiene.md
[relevant style guides]:
https://github.com/flutter/packages/blob/main/CONTRIBUTING.md#style
[CLA]: https://cla.developers.google.com/
[Discord]:
https://github.com/flutter/flutter/blob/master/docs/contributing/Chat.md
[linked to at least one issue that this PR fixes]:
https://github.com/flutter/flutter/blob/master/docs/contributing/Tree-hygiene.md#overview
[pub versioning philosophy]: https://dart.dev/tools/pub/versioning
[exempt from version changes]:
https://github.com/flutter/flutter/blob/master/docs/ecosystem/contributing/README.md#version
[following repository CHANGELOG style]:
https://github.com/flutter/flutter/blob/master/docs/ecosystem/contributing/README.md#changelog-style
[exempt from CHANGELOG changes]:
https://github.com/flutter/flutter/blob/master/docs/ecosystem/contributing/README.md#changelog
[test-exempt]:
https://github.com/flutter/flutter/blob/master/docs/contributing/Tree-hygiene.md#tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[video_player] Feature request: Switch to PlatformView on iOS (and macOS when possible) to fix performance issues
5 participants