-
-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
Implement multi-part audiobook support #11517
base: master
Are you sure you want to change the base?
Conversation
…yback from the most recently played position
This pull request has merge conflicts. Please resolve the conflicts so the PR can be successfully reviewed and merged. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know enough about audio books to review this properly. How are they generally structured? Are audio files chapters or can they be cut by length/size? Should we be agnostic about it?
It also sounds like an audio book is really just a playlist with some extra state (resume point). Have you explored that avenue?
|
||
if (audioFiles == 0 || nonAudioFiles / audioFiles > 1) | ||
{ | ||
_logger.LogDebug("Less than half of the files in {0} were audio files, probably not an AudioBook directory", args.Path); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this check needed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
By my understanding, resolvers are attempted until one succeeds. The priority value is high (low in sorted list) so it shouldn't come up often, but I didn't want to assume it would always only be resolving audiobooks. This catches cases where there is an unresolved directory which contains at least one file with an audio extension but mostly other files which I believe is unlikely for an audiobook. I imagine most cases will be a directory containing some number of audiobook files (chapters / parts) and maybe a text file or two with some arbitrary information.
@@ -756,6 +757,13 @@ private void OnPlaybackStart(User user, BaseItem item) | |||
data.Played = false; | |||
} | |||
|
|||
// When we start playing an AudioBookFile, mark all previous chapters as played, and |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't really do this for tv. I don't know if that is comparable though
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right. This is the behavior I think most people would be expecting from audiobook playback. It makes it easier to resume from your last location (if you want to skip ahead or start from somewhere in the middle of a book started elsewhere, etc.) and ensures "Continue Listening" has a single, intuitive point (index and ticks).
return [(this, MediaSourceType.Default)]; | ||
} | ||
|
||
public void SetFilesPlayed(User user, IUserDataManager userDataManager) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know this is how it's done in the past, but we generally don't like having this much logic in the entities as "untangling" them becomes a big hassle. Entities should contain some state and operations on their own values, but bringing in other dependencies is too much.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point. Do you have any thoughts on where I should put this logic?
targetItem.Album = sourceItem.Album; | ||
} | ||
|
||
// TODO: Create and register provider specific to book information and AudioBook information |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is Google Books a good candidate for that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like it might. I had thought it would require an API key, but not for basic search which I think is all that's required.
I was thinking of using openlibrary
I've seen audio books come in either singular large files with an accompanying .cue file with the chapter markers (Audible does this with .cue files). Or as 1 file per chapter (LibriVox does this). In either situation the audio book file(s) should be in its own folder. Jellyfin does not currently read in the .cue file for monolithic audio books, so there is no way to skip chapters. |
Not deeply. However, playlists do not have the continuation/resume behavior people expect from audiobooks so I would have to reimplement the AudioBook entity as a/similar to the Playlist entity or embed a Playlist within an AudioBook and use its methods for resolution and retrieval. I think this would get rid of some new code and potentially the entire AudioBookFiles class, but I don't know how it would get the same functionality out of the web interface. Currently, the web changes add logic to the playbackmanager to recognize "AudioBook" and "AudioBookFile" entities and do some stuff to set the playback state appropriately. Without the "AudioBookFile" class, I don't know how to get the web interface to set the state from the playbackmanger as it would only appear as the base "Audio" entity. If you're familiar with how the jellyfin-web works, I'd really appreciate any help understanding it so I could do something more sophisticated/long term. |
Changes in OpenAPI specification found. Expand to see details.What's Changed
|
This pull request has merge conflicts. Please resolve the conflicts so the PR can be successfully reviewed and merged. |
Modify the server to handle audiobooks which are split into multiple audio files. This assumes all files for a single audiobook are in their own directory and each directory only contains files for a single audiobook.
By itself, the changes in this merge request allow the server to handle resolving multi-part audiobooks into a single parent AudioBook entity and serve to the client under the normal books view. Playback can be started from the beginning of the audiobook and continue through all chapters but cannot be "resumed" once the play session is stopped.
When combined with the changes in the web pull request, playback can be resumed from the most recent stopping point from the books list view or from the "Continue Listening" entry.
Changes
Issues
Fixes #10668