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 ExoPlayer backend to playback rewrite #2427

Merged
merged 2 commits into from
Jan 20, 2023

Conversation

nielsvanvelzen
Copy link
Member

There we go, the third big PR for the playback rewrite! Well actually not that big but it adds an important part. This PR is a follow up on #2401 where I didn't include the ExoPlayer backend intentionally.

Changes

  • Add ExoPlayerBackend
    • Supports all current features
    • This will be the only backend in the forseeable future, multi-backend support and smart-backend selection are a low priority for now (but will likely be high priority when video support is added)
    • Some features related to video are removed from this PR

Issues

Part of #1057

@nielsvanvelzen nielsvanvelzen added enhancement New feature or request playback Issue related to media playback labels Jan 17, 2023
@nielsvanvelzen nielsvanvelzen added this to the v0.16.0 milestone Jan 17, 2023
@nielsvanvelzen nielsvanvelzen changed the title Add ExoPlayer playback backend Add ExoPlayer backend to playback rewrite Jan 17, 2023
.also { player -> player.addListener(PlayerListener()) }
}

inner class PlayerListener : Player.Listener {
Copy link
Contributor

Choose a reason for hiding this comment

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

More of a nit, is it worth pulling derivative classe(s) out into a dedicated file(s) early?

The existing Exoplayer/VLC code organically grew and turned into a mixture of boilerplate and implementation, hence I'm wondering if it's worth separating before we add to it?

Copy link
Member Author

Choose a reason for hiding this comment

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

Right now the ExoPlayer implementation is small enough that it doesn't matter. When the class grows it will likely get split into multiple files. I also believe large files aren't a huge problem, the fact that it isn't related to the file it's in is the problem. In the current player code we have pretty much everything in one file, the "ExoPlayerBackend" only contains the binding to ExoPlayer.

@nielsvanvelzen nielsvanvelzen merged commit 5988dca into jellyfin:master Jan 20, 2023
@nielsvanvelzen nielsvanvelzen mentioned this pull request Jan 30, 2023
35 tasks
@nielsvanvelzen nielsvanvelzen deleted the playback-exoplayer branch February 27, 2023 21:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request playback Issue related to media playback
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants