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

Complete Flutter VLC Player Rewrite - co-authroed with @alr2413 #159

Merged

Conversation

mitchross
Copy link
Contributor

@mitchross mitchross commented Jan 10, 2021

#158

@alr2413 and I have been hard at work past few months rewriting VLC plugin from scratch to support plugin v2. See https://medium.com/flutter/modern-flutter-plugin-development-4c3ee015cf5a

Here we have done a few things.

  1. Leverages V2 Android Flutter Plugin design pattern.
  2. All VLC features enabled.
  3. Flutter Platform Interface to support any platform. The platform interface allows for platform specific implementation, ie web, desktop, linux, ios, android. Currently only iOS and Android are supported.
  4. Flutter Pigeon for type safe method calls. No more string matching on method calls! (https://pub.dev/packages/pigeon)

Couple things.

We started a brand new repo for the work. It can be found here -> https://github.com/alr2413/flt_vlc_player/tree/master/flutter_vlc_player

This PR forks from Solid Software, incorporates @alr2413 code from their Repo and packages it into one singular PR commit.

Testing

  1. If you switch to this branch/pull. You need to goto example folder inside flutter_vlc_folder , delete ios and android folder. Then Inside example run "flutter create . ", The git switch over is causing flutter a hard time in example to run it. Idk why.

Copy link
Collaborator

@solid-software solid-software left a comment

Choose a reason for hiding this comment

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

Great! Thank you!

Could you please restore funding.yaml and do some cleanup (remove deprecated/commented out files)? Also we seem to have lost some changelog versions and maybe some other changes we added while you were working on this?

Thanks for your awesome work!

@mitchross
Copy link
Contributor Author

Unable to run example app on iOS device:

$ pwd
/Users/yuriiprykhodko/IdeaProjects/flutter_vlc_player/flutter_vlc_player/example

$ flutter pub get
Running "flutter pub get" in example...                             0.5s

$ flutter run
Launching lib/main.dart on iPhone in debug mode...
Expected ios/Runner.xcodeproj/project.pbxproj but this file is missing.
No application found for TargetPlatform.ios.
Is your project missing an ios/Runner/Info.plist?
Consider running "flutter create ." to create one.

Try this....

I cannot figure out why its doing this. The gitignore may be wrong...

(base) mitchross@MacBook-Pro example % ls
README.md	build		flutter_02.log	pubspec.lock
android		example.iml	ios		pubspec.yaml
assets		flutter_01.log	lib		test
(base) mitchross@MacBook-Pro example % pwd
/Users/mitchross/Documents/Programming/flutter/flutter_vlc_player/flutter_vlc_player/example
(base) mitchross@MacBook-Pro example % rm -rf ios
(base) mitchross@MacBook-Pro example % flutter create .
Recreating project ....

Then in your git tool of choice discard all of the changes besides the xcworkspace changes. If you keep the files after the create it will fail.

Screen Shot 2021-01-11 at 10 32 59 AM

Discard these
Screen Shot 2021-01-11 at 10 33 58 AM
Screen Shot 2021-01-11 at 10 33 53 AM
Screen Shot 2021-01-11 at 10 33 03 AM

@solid-yuriiprykhodko
Copy link
Collaborator

Yeah, git clean and rm -rf ios did the trick, thanks!

@mitchross
Copy link
Contributor Author

@solid-software @solid-pavloprykhodko its ready for merge now. cc @alr2413

@mitchross
Copy link
Contributor Author

@solid-software .. cc @alr2413 Its come to our attention that this change cannot be published right away without some changes. If you try to manually point a new flutter project at this branch and do a flutter pub get, it will error out saying no pubspec was found. This is because the root folder name and the library folder name are the same.

There is a fix, we can take the contents of 'flutter_vlc_player' and move them to the root of the project. This will mostly work however there is one issue. You cant publish a package with path dependencies. Ie

 
   flutter_vlc_player_platform_interface:
          path: flutter_vlc_platform_interface

Instead we need

flutter_vlc_player_platform_interface: ^1.0.0

However, we cant test this locally right now because flutter_vlc_player_platform_interface is not published yet.

See -> https://dart.dev/tools/pub/dependencies

Path dependencies are useful for local development, but do not work when sharing code with the outside world—not everyone can get to your file system. Because of this, you cannot upload a package to the pub.dev site if it has any path dependencies in its pubspec.

Refs -
https://github.com/flutter/plugins/blob/master/packages/video_player/video_player/pubspec.yaml
https://github.com/flutter/plugins/blob/master/packages/video_player/video_player_platform_interface/pubspec.yaml

Comment on lines 96 to 101

First run 'git clean'
Delete existing ios folder from root of flutter project.
Run this command flutter create -i swift .
Be sure to follow instructions above after for plist and pod file changes.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's split out the commands as codeblocks - they're easier to read and copy that way. Example below:

  1. Clean the repo:
git clean
  1. Delete existing ios folder from root of flutter project. If you have some custom changes made to the iOS app - rename it or copy somewhere outside the project.
  2. Re-create the iOS app:
flutter create -i swift .

Make sure to update the project according to warnings shown by the flutter tools. (Update Info.plist, Podfile).
If you have some changes made to the iOS app, recreate the app using above method and copy in the changed files.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

Comment on lines 84 to 87
## API
```dart
TBD
```
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not sure we need an API description in the README. Let's remove this section.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

To start using the plugin, copy this code or follow the example project in 'flutter_vlc_player/example'

```dart
TBD
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you please add a minimal example here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added!

@solid-yuriiprykhodko
Copy link
Collaborator

The player UI becomes unresponsive when it finishes playing. The Dart code seems to come through OK, controller.play gets dispatched. The problem is probably in the platform implementations. The bug shows on both Android and iOS.

@mitchross
Copy link
Contributor Author

mitchross commented Jan 19, 2021

The player UI becomes unresponsive when it finishes playing. The Dart code seems to come through OK, controller.play gets dispatched. The problem is probably in the platform implementations. The bug shows on both Android and iOS.

I have fixed this and pushed. We can discuss if this is not best approach, but is working on both platforms now :) ... cc @alr2413

@mitchross
Copy link
Contributor Author

The player UI becomes unresponsive when it finishes playing. The Dart code seems to come through OK, controller.play gets dispatched. The problem is probably in the platform implementations. The bug shows on both Android and iOS.

I have fixed this and pushed. We can discuss if this is not best approach, but is working on both platforms now :) ... cc @alr2413

Update 1pm 1/19/2021. Fully resolved.

Copy link
Contributor

@yu-pri yu-pri left a comment

Choose a reason for hiding this comment

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

LGTM!
Once again, great work!

Copy link
Collaborator

@solid-yuriiprykhodko solid-yuriiprykhodko left a comment

Choose a reason for hiding this comment

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

Sorry, wrong account. LGTM!

@solid-software solid-software merged commit 19c3d76 into solid-software:master Jan 20, 2021
@alr2413
Copy link
Contributor

alr2413 commented Jan 21, 2021

Ping @solid-software,

The PR is already merged, but it's not released & not published in "pub.dev" repo.
Is there any issue?

@solid-software
Copy link
Collaborator

Pong @alr2413 :)

No issues, we would probably want to merge other PRs as well before releasing the plugin:

#162
#161

Thanks again for your contribution!

@mitchross mitchross deleted the platform_interface_vlc_pluginv2 branch March 9, 2021 03:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants