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 screen lock feature in ExoPlayer #108

Merged
merged 2 commits into from
Sep 9, 2020

Conversation

CarlosOlivo
Copy link
Contributor

@CarlosOlivo CarlosOlivo commented Sep 8, 2020

Closes #77

@Maxr1998
Copy link
Member

Maxr1998 commented Sep 8, 2020

About the external player feature, I actually just pushed a PR that moves the external player selection to the native client settings 😅
So, you can remove that code again ^^

@CarlosOlivo
Copy link
Contributor Author

About the external player feature, I actually just pushed a PR that moves the external player selection to the native client settings 😅
So, you can remove that code again ^^

Nice timing.

@Maxr1998
Copy link
Member

Maxr1998 commented Sep 8, 2020

Nice timing.

Yeah, sorry 🙈
We quickly spotted this after (actually even before) merging the PR yesterday, and decided to merge the player settings to a single location. I started working on it yesterday evening, but only finished it a short while ago.

app/src/main/res/layout/activity_player.xml Outdated Show resolved Hide resolved
app/src/main/res/layout/activity_player.xml Outdated Show resolved Hide resolved
@Maxr1998
Copy link
Member

Maxr1998 commented Sep 8, 2020

Thanks a lot for the PR, the code looks really good! Spotted some small issues, would appreciate if you could resolve them before merge 😇

@Maxr1998 Maxr1998 added enhancement New feature or request exoplayer Related to the ExoPlayer integration labels Sep 8, 2020
@CarlosOlivo
Copy link
Contributor Author

Nice timing.

Yeah, sorry 🙈
We quickly spotted this after (actually even before) merging the PR yesterday, and decided to merge the player settings to a single location. I started working on it yesterday evening, but only finished it a short while ago.

Well, I can say the same, I started working on this since yesterday and just finish it now, sorry for the spam.
Now the PR is as it should be from the beginning. I will check what I can correct.

@CarlosOlivo
Copy link
Contributor Author

I don't know if it's an error or WIP, but ExoPlayer now don't report when the playback stops or ends, it keeps reporting "/Sessions /Playing/Progress", in the background, I don't know since when, in the v2.0.0-rc.3 version it works correctly, reporting "/Sessions/Playing/Stopped" at the end. @Maxr1998 @nielsvanvelzen

@nielsvanvelzen
Copy link
Member

That might be a regression of #104. We'll need to look into that.

@Maxr1998
Copy link
Member

Maxr1998 commented Sep 9, 2020

I don't know if it's an error or WIP, but ExoPlayer now don't report when the playback stops or ends, it keeps reporting "/Sessions /Playing/Progress", in the background, I don't know since when, in the v2.0.0-rc.3 version it works correctly, reporting "/Sessions/Playing/Stopped" at the end. @Maxr1998 @nielsvanvelzen

Interesting, in my testing it successfully marked the title as played, but that may be because the progress was high enough (you can set in Jellyfin after what percentage played it's considered as watched, after all). I previously thought that the ending submission works by setting the progress to the runtime length, but there seems to be another API route then. I'll investigate.

Copy link
Member

@Maxr1998 Maxr1998 left a comment

Choose a reason for hiding this comment

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

I'll give this another test locally, and merge it afterwards 👍

@Maxr1998 Maxr1998 merged commit 0b50eba into jellyfin:master Sep 9, 2020
@Maxr1998
Copy link
Member

Maxr1998 commented Sep 9, 2020

Thanks again for the PR @CarlosOlivo, looking forward to see more contributions from you in the future!

@CarlosOlivo CarlosOlivo deleted the screen-lock branch September 9, 2020 13:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request exoplayer Related to the ExoPlayer integration
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add "screen lock" feature
3 participants