Skip to content
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] Platform view support #8237

Open
wants to merge 97 commits into
base: main
Choose a base branch
from

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 FVPVideoPlayerTextureApproach - 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 method, viewType parameter in DataSource).

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 will be renamed to just playerId. This will be done in a separate PR to keep git diff for this one clean.

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.

Comment on lines +42 to +44
/// The next non-texture player ID, initialized to a high number to avoid collisions with
/// texture IDs (which are generated separately).
static int64_t nextNonTexturePlayerId = 1000000;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if this is the best approach, but looks like texture ids start from 0 and then get incremented, so this starting point seems to be pretty safe. Alternatively, I assume we could generate random ids for non-texture players

@FirentisTFW FirentisTFW force-pushed the feature/video-player-platform-view-support branch from 7a727b9 to 0184df3 Compare December 6, 2024 13:00
@FirentisTFW FirentisTFW force-pushed the feature/video-player-platform-view-support branch from 0184df3 to 5f31fc3 Compare December 6, 2024 13:35
@FirentisTFW
Copy link
Contributor Author

One PR check is failing, but this is due to changes in platform interface and local dependency overrides. The code can be reviewed already. Once this PR is reviewed, I'll create another one with platform interface changes only (according to contributing/README.md).

@FirentisTFW FirentisTFW marked this pull request as ready for review December 10, 2024 12:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
1 participant