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

Add many more helper methods to player #104

Merged
merged 26 commits into from Sep 22, 2020
Merged

Add many more helper methods to player #104

merged 26 commits into from Sep 22, 2020

Conversation

alr2413
Copy link
Contributor

@alr2413 alr2413 commented Sep 9, 2020

This PR Only implemented and tested on Android.
Features:

  1. fix black screen issue on android devices when rotation changes.
  2. get list of embedded audio tracks (also changed some names to be more like vlc conventions)
  3. get list of embedded subtitle tracks (also changed some names to be more like vlc conventions)
  4. get list of embedded video sections
  5. support local and network videos (isLocalVideo parameter) and subtitles (isLocalSubtitle parameter)
  6. added casting ability to an external display device
  7. added autoplay & looping options
  8. set/get subtitle delay, set/get audio delay
  9. and more .... :)

Also, the example app is updated to show some of these changes.

alr2413 and others added 7 commits August 3, 2020 23:12
…player

# Conflicts:
#	lib/flutter_vlc_player.dart
added getTime, getPlaybackSpeed and getVolume methods.
edited media player opening status to notify that the target url is loading.
fix various bug
added casting feature
@alr2413
Copy link
Contributor Author

alr2413 commented Sep 9, 2020

I've tried to add IOS implementation of this changes, but it's not clear to me how to do that.
I hope someone implement that.

Also, since I've add many parameters to vlc initialize method, I've change that method to support naming parameter structure, so it's not compatible with the prev version of this package. Maybe this PR (after IOS implementation) should be considered as a major revision.

Thanks.

@topilski
Copy link

Nice work @alr2413

@csongorkeller
Copy link

Big thanks for fixing it 👍

@csongorkeller
Copy link

Can you point on @alr2413 which part fixes the android black screen issue? So I can quickly check that on my end as well. THanks in advance

@alr2413
Copy link
Contributor Author

alr2413 commented Sep 10, 2020

Can you point on @alr2413 which part fixes the android black screen issue? So I can quickly check that on my end as well. THanks in advance

The part of code that was in "onSurfaceTextureAvailable" method, was moved to a Runnable closure.

@topilski
Copy link

@alr2413 how can I see swift part not ready?

@alr2413
Copy link
Contributor Author

alr2413 commented Sep 10, 2020

@alr2413 how can I see swift part not ready?

See What? As I mentioned "This PR Only implemented and tested on Android."

@solid-software
Copy link
Collaborator

Thanks for your work @alr2413! We hope that somebody can help with the iOS part to make it complete. Also do you mind resolving all of the conflicts?

Copy link
Contributor

@mitchross mitchross left a comment

Choose a reason for hiding this comment

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

I will do iOS Part..

example/lib/main.dart Outdated Show resolved Hide resolved
Copy link
Contributor Author

@alr2413 alr2413 left a comment

Choose a reason for hiding this comment

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

Thanks for your work @alr2413! We hope that somebody can help with the iOS part to make it complete. Also do you mind resolving all of the conflicts?

As I mentioned, some changes are based on vlc convention (like "audioTrack" instead of "sound"). That's why I proposed to use major revision for the next release.

example/lib/main.dart Outdated Show resolved Hide resolved
@mitchross
Copy link
Contributor

hey @alr2413 do you wanna to add me as a contributor to your repo of this fork? I can help resolve conflicts and complete iOS part

@csongorkeller
Copy link

@alr2413 I've checked in emulator, and the rotation doesn't work for me with the example app in the repo. You also tested with the example app?

@alr2413
Copy link
Contributor Author

alr2413 commented Sep 12, 2020

@alr2413 I've checked in emulator, and the rotation doesn't work for me with the example app in the repo. You also tested with the example app?

What do you mean "the rotation doesn't work"?
Yes, I've tested on my real android device and it's working.

@csongorkeller
Copy link

csongorkeller commented Sep 12, 2020

@alr2413 I've checked in emulator, and the rotation doesn't work for me with the example app in the repo. You also tested with the example app?

What do you mean "the rotation doesn't work"?

Yes, I've tested on my real android device and it's working.

Well,
I've tried to run the example app in the project. And the player doesn't start at all on my end. And when I rotate the screen, I got just a white background instead of the player object. So basically the whole player don't really work for me, and therefore the rotation also doesnt. Can you upload the project hete in a zip, where you run the player in landscape? So I can check with my environment too.
I hope you can help me with this. Thanks in advance, and many thanks for fixing vlc for all of the flutter enthusiasts.

@mitchross
Copy link
Contributor

@csongorkeller Please hold tight. Im actively working on helping out @alr2413. Im working on his code right now...

@csongorkeller
Copy link

android seems like working. Good job @alr2413. But I have an issue with disposing the player. If I want to change the video, I dispose the player with the method you've implemented, but for the new player, I got a black screen without any playing. Do you have any idea what I'm doing wrong?

@mitchross
Copy link
Contributor

Getting close on fixing casting on iOS, will have a update for this PR hopefully by tonight

@topilski
Copy link

1600455142044
Hello, that's only my machine issue?

@mitchross
Copy link
Contributor

@topilski that is not on the latest commit. you appear to behind.

@mitchross
Copy link
Contributor

@solid-software This code is ready for merge. I have reviewed it, tested it, developed on it.

I worked with @alr2413 on the side and brought in @amadeu01

@topilski
Copy link

@mitchross @alr2413 @amadeu01 thank you for your work guys!

fix minor issue in android capture method
added event dispatcher to ios cast device add/remove
updated example code
fix spacing issue in readme
@alr2413
Copy link
Contributor Author

alr2413 commented Sep 21, 2020

Ping @solid-software,
now the code is ready for merge.

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 job @alr2413 @mitchross - I'm blown away with the changes you guys have made!

I've added couple small comments/suggestions - please take a look and let me know what you think.

I think 4.0 would be a game changer - thanks again for your work on this!

Updated method docs
@alr2413
Copy link
Contributor Author

alr2413 commented Sep 21, 2020

@solid-software I've done that.
Also, if you've any suggestion about versioning let me know.
tnx

@mitchross
Copy link
Contributor

@solid-software After this PR is merged there is a few more clean up tasks I'll do in follow up PRs to migrate the Android Plugin to V2 https://flutter.dev/docs/development/packages-and-plugins/plugin-api-migration

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.

LGTM - thanks @alr2413 @mitchross !

@solid-software solid-software merged commit 2b019c8 into solid-software:master Sep 22, 2020
@alr2413
Copy link
Contributor Author

alr2413 commented Sep 22, 2020

@solid-software, @mitchross
I think we should review all opened issues, and close theme if they're solved with this release.

@pharshdev
Copy link

I think the major issues still are:

[A] the "player state change", when the widget is rebuilt which could be from something as simple as orientation change or calling setstate.
Steps to reproduce:

  1. Pause the player
  2. Tilt the device or press a button that calls setstate.

Desired state: Video player paused.
Observed state: Video player starts playing.

[B] Not adhering to App lifecycle. Any video player should be auto paused when the phone screen is turned off. This plugin would infact start playing the video even when the video was paused. Needless to say, a playing video would continue playing.

[C] Player doesnt respond well to player's widget resizing dynamically. I have an app that supports PIP and when i send a running video to PIP, there's a second or two delay before the video starts re-rendering. All this while, the audio is playing fine. Although not majorly observed, this happens when orientation change too.

It looks like we're still missing something in the native implementations as the official vlc player doesnt have these issues.

PS: You can run into these issues even with the example code.

@mitchross
Copy link
Contributor

@pharshdev i'll take care of A,B,C with the Android 2.0 Plugin update migration

@alr2413
Copy link
Contributor Author

alr2413 commented Sep 22, 2020

@pharshdev , @mitchross
Yea, the "A" situation is based on the written code.
Before I clean up code, I've "wasPlaying" variable in Runnable method to check that situation, but I don't remember why I've commented that line.

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

7 participants