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

AudioNavigator.seek cannot cross previous resource boundary #394

Open
domkm opened this issue Feb 24, 2024 · 10 comments
Open

AudioNavigator.seek cannot cross previous resource boundary #394

domkm opened this issue Feb 24, 2024 · 10 comments
Labels
feature request Something is missing

Comments

@domkm
Copy link
Contributor

domkm commented Feb 24, 2024

Describe the bug

seek seems unusable for a slider component or rewind button in a multi-resource audiobook, because it cannot go further back than the beginning of the current resource.

How to reproduce?

  1. Create an AudioNavigator from an audiobook with multiple resources.
  2. Observe that navigator.seek(to: time) and navigator.seek(by: -time) cannot move the current playback position from resource n to resource n-1. Instead, it moves playback to the beginning of resource n.

Readium version

I tried 2.6.1 and develop

OS version

17.2

Testing device

simulator

Environment

Xcode 15.2
Build version 15C500b

Additional context

No response

@domkm domkm added the bug Something isn't working label Feb 24, 2024
@mickael-menu mickael-menu added feature request Something is missing and removed bug Something isn't working labels Feb 26, 2024
@mickael-menu
Copy link
Member

It works as intended. An audiobook player usually provides scrubbing control for the current resource only.

Hovewer, I get that it may not fit all needs. How about these alternatives?

enum TimeScope {
    case currentResource
    case publication
}

func seek(to time: Double, in scope: TimeScope = .currentResource)

navigator.seek(to: 30, in: .publication)

and having seek(by:) skip to the previous resource. Although for the next resource, I think it might be more user-friendly to still start from the beginning of the next chapter instead of actually skipping x seconds.

Are you interested in contributing these changes?

@domkm
Copy link
Contributor Author

domkm commented Feb 28, 2024

I wonder if perhaps we are conflating resources and chapters, or perhaps I am misunderstanding. Does Readium process embedded chapter metadata and convert to "virtual" resources? If not, I would love if there was a unified way to deal with publication chapters for both embedded chapters (like in an M4B) and per-file chapters (like in a zipped audiobook of MP3s). Any thoughts on providing a unified API for navigating chapters?

Regarding the above TimeScope proposal, I'm open to contributing it, but I would quite like to hear your thoughts about chapter navigation abstraction first.

@mickael-menu
Copy link
Member

It doesn't, in Readium a "resource" always refers to a single entry in a ZIP package, and/or an entry in a Readium Audiobook Manifest.

There's no plan to parse a chapter list from a M4B directly in the toolkit's AudioParser. But you could implement a custom PublicationParser for M4B which returns an audiobook Publication, which would work with the AudioNavigator.

For chapters (as opposed to resource), we store them in the publication.tableOfContents property, which is a tree structure. We don't have any API to navigate relatively through chapters right now, mainly because in EPUB it gets complicated as sorting chapters require to parse the HTML.

But for an audio table of contents it should be easy, as they would use a time fragment or progression percentage. What would an ideal chapter navigation API look like to you?

@domkm
Copy link
Contributor Author

domkm commented Mar 5, 2024

I didn't know about publication.tableOfContents. Cool!

Does "no plan" to parse a chapter list in AudioParser mean Readium has decided against it or that it's not specifically planned but would be accepted as a contribution?
I think the ideal case would be the parser populating tableOfContents from the asset in whatever way it is provided (chapterized MP3 files, M4B/MP3 metadata, CUE file, etc.).

I'm not sure about the ideal API for utilizing chapters, but I'll try to talk through my potential use cases. For current playback and book overview, I'd want to be able to display chapter title, duration, and progress. For playback, I'd want to be able to seek forward and backward within chapters and between chapters, essentially what you proposed for resource seeking with time scopes. Perhaps this points to TimeScope needing a case for currentChapter?

@mickael-menu
Copy link
Member

mickael-menu commented Mar 5, 2024

Does "no plan" to parse a chapter list in AudioParser mean Readium has decided against it or that it's not specifically planned but would be accepted as a contribution?

It depends on whether this requires a third-party dependency. We aim to minimize them in the toolkit.
In any case, adding extensibility to the AudioParser for the app to integrate a table of contents algorithm could be interesting.

For current playback and book overview, I'd want to be able to display chapter title, duration, and progress.

Maybe we need a tableOfContentsIndex in the MediaPlaybackInfo? I hesitate calling it chapter, as we don't really have this concept in the APIs. And a table of contents item is not necessarily a chapter.

@domkm
Copy link
Contributor Author

domkm commented Mar 15, 2024

It depends on whether this requires a third-party dependency. We aim to minimize them in the toolkit. In any case, adding extensibility to the AudioParser for the app to integrate a table of contents algorithm could be interesting.

That makes sense. I'll look into this at some point and see what can be provided without extra dependencies.

Maybe we need a tableOfContentsIndex in the MediaPlaybackInfo? I hesitate calling it chapter, as we don't really have this concept in the APIs. And a table of contents item is not necessarily a chapter.

Sounds good!

I've also been reading swift-toolkit to better understand how it works, and I'm confused by the differences between a custom AudioParser and using a Publication.Builder.Transform with the default AudioParser. In what ways is a PublicationParser more powerful than a Publication.Builder.Transform?

@mickael-menu
Copy link
Member

In what ways is a PublicationParser more powerful than a Publication.Builder.Transform?

The PublicationParser creates the components of a Publication.Builder from scratch. Usually you implement it to parse a given publication specification (EPUB, PDF, Zipped audiobook) into a Publication object.

Publication.Builder.Transform is an extension point for modifying Publication.Builder components before creating the Publication object. You can for example adjust the publication metadata or reorder/filter the reading order. As it's independent of a publication format, it can be used across various PublicationParser implementations.

@domkm
Copy link
Contributor Author

domkm commented Mar 16, 2024

Thanks for the explanation! It sounds like Publication.Builder.Transform will be sufficient for adding table of contents information since it is able to modify the manifest, as long as the audiobook is otherwise parsable by the default parser.

@domkm
Copy link
Contributor Author

domkm commented Apr 2, 2024

Would you accept a PR that changes AudioParser to include additional information in the manifest without requiring any other dependencies? I would like to include resource durations, bitrates, etc.

@mickael-menu
Copy link
Member

Sure thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request Something is missing
Projects
None yet
Development

No branches or pull requests

2 participants