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

fix: retain exoplayer on orientation change #6428

Merged
merged 1 commit into from
Sep 5, 2024

Conversation

Pittvandewitt
Copy link
Contributor

This PR is basically the last step of #6307
Now you should feel the improvement upon rotation change ;)

The ExoPlayer is no longer stopped on onDestroy, because it is already released in its viewmodel onDestroy method.

@Bnyro Bnyro changed the title Retain exoplayer on orientation change fix: retain exoplayer on orientation change Sep 3, 2024
Copy link
Member

@Bnyro Bnyro left a comment

Choose a reason for hiding this comment

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

Wait, this would also mean that the current player video stays alive when the user closes a video and it still continues to buffer.

Apart from that viewModel.isOrientationChange already conditionally does what you're trying to achieve with this PR, doesn't it?

@Pittvandewitt
Copy link
Contributor Author

When the video closes with PlayerFragment, PlayerViewModel is also destroyed and PlayerViewModel.onCleared is called, where the exoplayer is released. Or does ExoPlayer.stop also need to be called before ExoPlayer.release?

With this PR I notice no interruption when changing orientation, but with the current implementation of viewModel.isOrientationChange I always do, especially when pressing the full screen button.

@Bnyro
Copy link
Member

Bnyro commented Sep 5, 2024

When the video closes with PlayerFragment, PlayerViewModel is also destroyed and PlayerViewModel.onCleared is called,

You're right, I thought that would only be called in the activity's onDestroy and not in the fragments, but I tested it and it looks like you're right.

@Bnyro Bnyro merged commit 5bd6cc7 into libre-tube:master Sep 5, 2024
3 checks passed
@Pittvandewitt Pittvandewitt deleted the fix/retain-exoplayer branch October 1, 2024 15: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.

2 participants