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 Hybrid composition support for Android #314

Merged
merged 2 commits into from Apr 28, 2022
Merged

Add Hybrid composition support for Android #314

merged 2 commits into from Apr 28, 2022

Conversation

XuanTung95
Copy link
Contributor

@XuanTung95 XuanTung95 commented Apr 26, 2022

  • minSdkVersion from 17 -> 20 as required by Virtual display.
  • Bump ibvlc-all:3.5.0-eap4 -> libvlc-all:3.5.0-eap6
  • Add bool virtualDisplay to specify whether Virtual displays or Hybrid composition is used on Android. Default is Virtual displays.
  • Bump version of flutter_vlc_player_platform_interface: 2.0.0 -> 2.0.1, flutter_vlc_player: 7.1.1 -> 7.1.2
  • Refers: platform-views

@solid-software
Copy link
Collaborator

Hey, @mitchross @alr2413 what do you think about this PR?

@solid-software
Copy link
Collaborator

@XuanTung95 - looks great, thanks! Could you please also update CHANGELOG.md for both plugins?

@alr2413
Copy link
Contributor

alr2413 commented Apr 26, 2022

Looks fine to me.
Just one question: is it still possible to use this library on android devices with SDK below 20 or not?

@mitchross
Copy link
Contributor

mitchross commented Apr 26, 2022

What is the use case for this?

Edit - I see #313

@XuanTung95 what are implications for iOS?

@XuanTung95
Copy link
Contributor Author

Just one question: is it still possible to use this library on android devices with SDK below 20 or not?

As the doc

android {
    defaultConfig {
        minSdkVersion 19 // if using Hybrid composition.
        minSdkVersion 20 // if using Virtual display.
    }
}

flutter_vlc_player uses Virtual display as default so min SDK 20 seems fair.

what are implications for iOS?

iOS only uses Hybrid composition as the doc. So there is nothing to do for iOS.

@XuanTung95
Copy link
Contributor Author

Could you please also update CHANGELOG.md for both plugins?

I updated CHANGELOG and bump version of flutter_vlc_player to 7.1.2

@mitchross
Copy link
Contributor

Why not have Hybrid Composition default if it is more performant?

@XuanTung95
Copy link
Contributor Author

Why not have Hybrid Composition default if it is more performant?

It would be a breaking change and need to be tested carefully. And there are different behaviors between them as they take different approaches.
And I think Virtual display lag is a bug and will be fixed soon, #102492

@mitchross
Copy link
Contributor

Why not have Hybrid Composition default if it is more performant?

It would be a breaking change and need to be tested carefully. And there are different behaviors between them as they take different approaches. And I think Virtual display lag is a bug and will be fixed soon, #102492

Hmm if its a breaking change to use it, then this flutter_vlc_lib needs to be updated to major version as this is no longer a minor or patch change.

@XuanTung95
Copy link
Contributor Author

I think it's not a breaking change because the default still uses Virtual display as before. Changing the default to Hybrid Composition may affect other apps.

@mitchross
Copy link
Contributor

I think it's not a breaking change because the default still uses Virtual display as before. Changing the default to Hybrid Composition may affect other apps.

I think there should be a disclaimer/documentation when to use hybrid composition vs virtual display. It appears your original issue is addressing performance issues, wouldn't that affect all?

@XuanTung95
Copy link
Contributor Author

The doc of Flutter team above already describes what is Virtual Display/Hybrid and their performance. I think we can add it in Readme.

It appears your original issue is addressing performance issues, wouldn't that affect all?

Yes. But it only occurs when creating the platform view. I did not see lag after they are created. I'm not sure about when to use hybrid composition vs virtual display, their behavior is complex. Forks can try to use hybrid composition if there are lags.

libvlc also causes frame delay when calling init() and stop() as I see.

@solid-software
Copy link
Collaborator

Are we good to merge this one?

@mitchross
Copy link
Contributor

Are we good to merge this one?

The instructions are unclear when you need this. Is it fine to merge? Yes

Will average users of flutter who are using this package know when to use hybrid vs virtual display? I highly doubt it. These are implementation details deep into Flutter's framework. Personally, Id like a very well-documented reason when to use each as it relates to VLC not to the generic flutter doc linked.

@solid-software
Copy link
Collaborator

I think I'm fine with the way it's now, as it in some cases improves performance. So let's get this in.
There is a related flutter bug - maybe after we get response there we'll know better why it's different.

@solid-software solid-software merged commit 55e2753 into solid-software:master Apr 28, 2022
@bailyzheng
Copy link

minSdkVersion should be 19 as a lot of tv devices are Android 4.4.

@pinpong
Copy link
Contributor

pinpong commented Mar 13, 2023

Are there any reason why this package is not using Texture?

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