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 sleep at end of chapter #1647

Merged
merged 1 commit into from Oct 15, 2023
Merged

Conversation

tomaThomas
Copy link
Contributor

@tomaThomas tomaThomas commented Dec 17, 2022

Implements #1323

I'm not familiar with Kotlin and the android media player, so I'm not sure if this is a good way to implement it (periodically check similar to normal sleep timer). Seems a bit hacky to me.

Also, the UI could be improved, but I don't have a better idea right now.

voice
voice2

Let me know what you think

@tomaThomas tomaThomas changed the title Add sleep atend of chapter Add sleep at end of chapter Dec 17, 2022
@theAkito
Copy link

Great feature, I need that one.

I might look at this, but it seems like the maintainer is currently inactive. 🙁

@waot
Copy link

waot commented Mar 25, 2023

There is a new release out, so hopefully it can be incorporated soon :)

@PaulWoitaschek
Copy link
Owner

Thanks for your contribution! The taken approach is a little hacky though. What do you think about attempting the solution that was suggested here? google/ExoPlayer#10262 (comment)

@tomaThomas
Copy link
Contributor Author

What do you think about attempting the solution that was suggested here? google/ExoPlayer#10262 (comment)

That looks like a proper solution, I'll see if I can change it
What do you think about the button?

@PaulWoitaschek
Copy link
Owner

The button is fine. I want to rework the UI of that whole thing anyways.

@PaulWoitaschek
Copy link
Owner

Hmm the more I think about it I think the suggested approach can't work with setPauseAtEndOf because exoplayer doesn't know anything about the chapter marks.

@tomaThomas
Copy link
Contributor Author

Hmm the more I think about it I think the suggested approach can't work with setPauseAtEndOf because exoplayer doesn't know anything about the chapter marks.

You mean that it doesn't work if there are multiple chapters within a single audio file?

I updated my main branch, I guess it is still not the best solution and could be cleaned up a bit. I added onMediaItemTransition to the PlayStateDelegatingListener, which does work for separate files

@PaulWoitaschek
Copy link
Owner

Just a little heads up: I'm currently very busy and want to first focus on stability before pushing new features. So unfortunatelly this PR has to wait for a while.

@tomaThomas
Copy link
Contributor Author

Just a little heads up: I'm currently very busy and want to first focus on stability before pushing new features. So unfortunatelly this PR has to wait for a while.

No worries, I'll keep the branch updated because I'm using my own build

@obbardc
Copy link

obbardc commented May 2, 2023

Would it make sense to have an option "End of chapter" which sets a timer for the remaining length of the chapter?

e.g. when you press the button it just works out the remaining time left and sets that as the timer length? :-)

This would allow you to reuse most of the existing timer functionality and you wouldn't need a callback to check the remaining length.

@tomaThomas
Copy link
Contributor Author

Would it make sense to have an option "End of chapter" which sets a timer for the remaining length of the chapter?

I did try that first, but wasn't happy with it. It was not that exact, pausing / skipping backwards / changing playback speed etc. messed it up, and fade out would need to be disabled

@tomaThomas tomaThomas force-pushed the main branch 2 times, most recently from a1b84e7 to db64c34 Compare May 10, 2023 12:51
@obbardc
Copy link

obbardc commented May 10, 2023

Looking great, no hacky timers needed! <3

@PaulWoitaschek
Copy link
Owner

Heads up:
In preparation for merging this PR I've reamped the layout so we can easily find a good UI option for this:
#2091

@PaulWoitaschek PaulWoitaschek merged commit c27034d into PaulWoitaschek:main Oct 15, 2023
2 checks passed
@PaulWoitaschek
Copy link
Owner

Wonderful, thank you for the contribution! And sorry for taking ages to merge it 🙈

@PaulWoitaschek
Copy link
Owner

PaulWoitaschek commented Jan 20, 2024

Sorry to tell, but bad news 😢

The implementation has bugs when files contained multiple chapters. As right now the main branch was not releasable for a while I decided to temporarily revert the change until it's bug free.
#2264

I'm looking forward for an updated PR 🙏

@tomaThomas
Copy link
Contributor Author

That was the limitation of doing this at the ExoPlayer level - I thought you were aware. I was surprised you merged it in the first place ^^
I thought about setting a time based sleep timer for the remaining time, but that didn't bother me enough because I don't have such audiobooks. But I will probably deal with it next month when I've got more time

@PaulWoitaschek
Copy link
Owner

Maybe this could be an approach:
google/ExoPlayer#4176 (comment)

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.

None yet

5 participants