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

ExoPlayer: Implement media segment support #1507

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

jakobkukla
Copy link
Contributor

@jakobkukla jakobkukla commented Nov 10, 2024

Initial support for media segments.

Most of this is shamelessly copied from the Android TV implementation in jellyfin/jellyfin-androidtv#4052. @nielsvanvelzen, I tried to reflect that in the commit message. Let me know if that's a problem.

Changes

The playback related implementation is pretty much copied 1:1 from ATV. This implements the Skip and Nothing actions. The Unknown segment type is not supported. AskToSkip is not implemented yet but I plan to do so in a following PR.

I reused the settings from the web client by implementing a JS plugin and subscribing to settings changes. I usually try to avoid JavaScript in my daily life and have never worked with the nativeshell plugin system before. I'd appreciate if you'd take a closer look at these changes.

On another note: I'm not quite sure when to use koin's inject() vs get(). I tried to look at how the objects were injected in other places and followed that.

Issues

Depends on #1506
Fixes #1500

@jellyfin-bot jellyfin-bot added this to the v2.7.0 milestone Nov 10, 2024
@jakobkukla jakobkukla force-pushed the media-segments branch 3 times, most recently from 69dfad7 to fa329d9 Compare November 10, 2024 21:06
@jakobkukla jakobkukla marked this pull request as ready for review November 10, 2024 21:08
@felixschndr

This comment was marked as off-topic.

@jellyfin-bot jellyfin-bot added the merge conflict Conflicts prevent merging label Jan 26, 2025
Most of the proposed changes in the files listed below have been shamelessly
copied from the Android TV implementation in jellyfin/jellyfin-androidtv#4052.

Authorship of these changes belongs to nielsvanvelzen.

app/src/main/java/org/jellyfin/mobile/player/PlayerViewModel.kt
app/src/main/java/org/jellyfin/mobile/player/mediasegments/MediaSegmentAction.kt
app/src/main/java/org/jellyfin/mobile/player/mediasegments/MediaSegmentRepository.kt
app/src/main/java/org/jellyfin/mobile/utils/extensions/MediaSegment.kt
@jellyfin-bot jellyfin-bot removed the merge conflict Conflicts prevent merging label Jan 28, 2025
@jakobkukla
Copy link
Contributor Author

rebased on master

@felixschndr

This comment was marked as off-topic.


@JavascriptInterface
fun setSegmentTypeAction(typeString: String, actionString: String) {
val type: MediaSegmentType = when(typeString) {

Check warning

Code scanning / detekt

Reports spaces around keywords Warning

Missing spacing after "when"
else -> return
}

val action: MediaSegmentAction = when(actionString) {

Check warning

Code scanning / detekt

Reports spaces around keywords Warning

Missing spacing after "when"
for (mediaSegment in mediaSegments) {
val action = mediaSegmentRepository.getMediaSegmentAction(mediaSegment)

when(action) {

Check warning

Code scanning / detekt

Reports spaces around keywords Warning

Missing spacing after "when"
val player = playerOrNull ?: return

player.createMessage { _, _ ->
// We can't seek directly on the ExoPlayer instance as not all media is seekable

Check warning

Code scanning / detekt

Reports mis-indented code Warning

Unexpected indentation (16) (should be 12)
Comment on lines 440 to 441
// We can't seek directly on the ExoPlayer instance as not all media is seekable
// the seek function in the PlaybackController checks this and optionally starts a transcode

Check warning

Code scanning / detekt

Reports mis-indented code Warning

Unexpected indentation (16) (should be 12)
Comment on lines 443 to 444
// TODO: The above is probably true for jellyfin-android as well.
// But I believe there is no such logic here.

Check warning

Code scanning / detekt

Reports mis-indented code Warning

Unexpected indentation (16) (should be 12)
Comment on lines 444 to 445
// But I believe there is no such logic here.
viewModelScope.launch(Dispatchers.Main) {

Check warning

Code scanning / detekt

Reports mis-indented code Warning

Unexpected indentation (16) (should be 12)
Comment on lines 445 to 446
viewModelScope.launch(Dispatchers.Main) {
player.seekTo(mediaSegment.end.inWholeMilliseconds)

Check warning

Code scanning / detekt

Reports mis-indented code Warning

Unexpected indentation (20) (should be 16)
Comment on lines 446 to 447
player.seekTo(mediaSegment.end.inWholeMilliseconds)
}

Check warning

Code scanning / detekt

Reports mis-indented code Warning

Unexpected indentation (16) (should be 12)
Comment on lines 446 to 448
player.seekTo(mediaSegment.end.inWholeMilliseconds)
}
}

Check warning

Code scanning / detekt

Reports mis-indented code Warning

Unexpected indentation (12) (should be 8)
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.

3 participants