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

feat: use exoplayer, hybrid composition #27

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

Conversation

mertalev
Copy link

@mertalev mertalev commented Dec 1, 2024

Description

This PR overhauls the Android implementation to use Exoplayer instead of MediaPlayer and changes the rendering mode to hybrid composition for HDR support (same as iOS). This allows for smoother playback and seeking as well as faster initialization. Additionally, I was unable to use hybrid composition with MediaPlayer's VideoView, but it works with Exoplayer without issue.

It also makes some other adjustments. Looping is now natively supported and managed as it worked better than handling it in Dart from my testing. Playback completion now only sets the "stopped" status in Dart without releasing video resources - this caused issues with Exoplayer. onPlaybackEnded is only called when the video actually stops playing - looping does not fire this event. This removes the guesswork on the caller's side as to whether the UI should be updated or if the video is just going to start again.

@mertalev
Copy link
Author

mertalev commented Dec 1, 2024

This PR makes a lot of changes as I actually intended it to go to a fork. But I'm leaving it open in case you're interested in accepting these changes.

@albemala
Copy link
Owner

albemala commented Dec 16, 2024

hey @mertalev thank you very much for your contribution! I really appreciate it. I'll review your code more closely and give you some more detailed feedback.

regarding using ExoPlayer, i know it supports more formats, but what are other advantages of using it? the original plan for this package was to use only native APIs, not 3rd party dependencies, so i'm not sure if to include exoplayer or not.

BTW I might ask you to split this PR into smaller ones, to make it easier to merge and review. but I'll let you know and give you a follow up on this.

@mertalev
Copy link
Author

mertalev commented Dec 16, 2024

We had buffering issues with MediaPlayer. It sometimes took many seconds for it to start playing a remote video, and seeking could also take several seconds. Exoplayer is much smoother in that regard.

Another thing is that using hybrid composition with MediaPlayer+VideoView always showed a black screen. Since hybrid composition is needed for HDR and works as expected with Exoplayer, it became the clear option.

I can definitely shrink the PR down by removing the looping etc.

…player) (#7)

fix: Don't crash on Android if data source throws Exception

See immich-app/immich#15230 (comment).
So far, only java.lang.Error was caught, but
HttpDataSource.InvalidResponseCodeException is a java.lang.Exception.
@albemala
Copy link
Owner

@mertalev sorry for the late reply, I didn't forget about your PR. you might have seen that I'm actively working on v3 of the plugin, and I have published a few dev releases. one of these releases includes using Expoplayer on Android. I took the code from your PR directly, and adapted it to the new implementation, which is a bit different.

I'm still not sure about using hybrid composition and moving the looping logic to the native side. Could you please give the latest dev version (3.0.0.dev3) a try, and see if it fixes the issues you had? Thank you!

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.

4 participants