-
Notifications
You must be signed in to change notification settings - Fork 33
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
Clarify and refine the streamer API on mobile platforms #116
Comments
Thanks, with all these new materials we're moving towards drafting a first Streamer API spec!
Definitely, the less stuff we have in the reading apps, the easier it is to upgrade them. Moreover, I think we also need a way for reading apps to plug their own parser implementations for custom formats into R2. Like you said in the other issue, parser selection should be done using the file extension and/or media type, as well as some sniffing capabilities (JSON, ZIP entries) that we could provide helpers for. (On a side-note, I think
Probably, at least for the beginning for backward compatibility. Then it might make sense to expose them anyway if someone really wants to parse an EPUB only, for example.
I fully agree, we need to make sure that reading the content always go through the content filters. This is also more in line with the original architecture schema which doesn't mention
That's interesting. For R1 we used to have priorities as integers, but that might be better to enforce it at the construction level. So I guess:
I like this approach a lot. One might wonder what's the difference between Do we need Merging
I think it makes sense, the server is actually an implementation detail of the Navigator to serve only the HTML publications. Having the server in the streamer means that we have to resort to small hacks to keep the responsibilities of the streamer and the navigator segregated. Having the server in the Navigator also means that there's less code in the reading apps, and that we're sure that we start it only if it's really needed. However, I think reading apps should be able to customize which implementation of the server is used by the Navigator. So a This also means that we don't need to hack the protocol Server {
// Returns the baseURL
func serve(fetcher: Fetcher, at endpoint: String) -> String
} |
I suggest a method like Publication.open(path, parsers=defaultparsers, filters=defaultcontentfilters)
This sounds great.
After discussion, it seems that two CF provided at the same point (for example to PublicationParser) may need some prioritization (for example, the order between decryption and font deobfuscation matters). So there are three possibilities: using priorities as integers or using builder with categories to construct ordered chain, or both, the latter being convenience method (less flexible). |
In this issue, I initially thought of
I think having simply an int priority (like in R1) is simpler and more flexible. And since the content filter chains would be segregated, we could have different constant priorities defined by each module (e.g. DECRYPTION for streamer, INJECTION for navigator, etc.), instead of a global "registry" of priorities. |
There are few uses of format besides parsing. Amongst the reasons you presented, it seems to me that only the last one would be left with the new API: apps may want to store publication format. However your approach doesn't seem to clearly provide them with this information. Either apps are required to call media type detection once again themselves (though Android and iOs are Unix-like, it could theoretically be issues with opened files), or media type (string) should be inserted in In addition, I think it is interesting to provide an ordering mechanism for media type detection. Someone may want to handle a subset format, or a not clearly identified extension, with a special parser. |
There are a few uses left, but not much, here's from Swift:
Yes it's missing. We want to avoid determining the media type several times – especially since we might lack useful information such as HTTP Content-Type at different points – so there must be a way to optionally pass the media type directly to the parser, and to get it back from it. I thought about revamping Publication.open(file: String, mediaType: String? = null): OpenedPublication?
We still need to make sure that the media type computing is consistent, wherever it's done. So I still think registering additional media type app-wise is needed. Is it the singleton you don't like? Because we could also have a |
A register has the advantage of being independent of the call to open. With this solution, we could indeed have But there are also drawbacks: the function depends on the MediaTypeSniffer state and doesn't simply expose the relationship between the open function and the global MediaTypeSniffer. As you suggested, we could pass MediaTypeSniffer (default or custom) to the open function, but if media type is sometimes known, it would require two different functions. One with a sniffer argument, and the other with a mediaType argument. Therefore, my favorite solution is to let apps determine media type before calling This approach sounds to me very simple and natural. Does it fit possible different use cases? |
I think it sounds reasonable, so we don't need to pass However, we want to have the simplest API while allowing a high degree of customization for advanced reading apps. I think the majority of the reading apps will use our default media types and just want to open a On a side note, I think we should use a custom @Throws
Publication.open(file: String, mediaType: MediaType? = null): OpenedPublication |
I think we can close this issue now that we have a few proposals addressing most of it. To answer the initial problems:
The new
These will be fixed with
The
This is not yet addressed, but we can always open an issue for that later. |
Current uses of the streamer in the Kotlin testapp
LibraryActivity
is started, it instantiates aServer
.recyclerViewListClicked
) or added to the library (parseIntent
),Publication
by calling the appropriate parser, depending on the file extension; the result is stored in a PubBox containing thePublication
and theContainer
.Publication
is an Epub, theServer
functionaddEpub
is called (prepareToServe
) for the publication. AFetcher
is instantiated in this call and kept in memory by the router to be used when an HTTP request occurs.Publication
itself along with its path, filename, id and cover are passed to the new activity through intent extras.Current public API
PublicationParser
interface is implemented by all parsers, but every one is individually imported and used by the testappContainer
interface (though it is not consistently used, see below)Fetcher
class is currently not used directly in the testapp, but instantiated inside aServer
Server
class which is instantiated in the testappPublicationParser
publicly exposes mimetype constants.Current issues
Container
to get the cover in the Epub case.ContentFilter
s (see Making the ContentFilter API public kotlin-toolkit#201). Some of these content filters (for instance third-party DRM CF) are required to be passed to the PublicationParser (for example to decrypt navigation and SMIL files), some others should be defined later (for example, navigator-specific ones).Server
(see Revamping the Content Filter architecture #103). In addition, apps should be able to make r2 use their own HTTP server implementation.Some ideas
PublicationParser
interface. To achieve this, thePublicationParser
parse
method could include a parser factory based on path extension. Should format-specific parsers still be public or not?Fetcher
interface rather thanContainer
? It would allow to directly retrieve encrypted and other on-the-fly modified content, besides raw content. AFetcher
has to be instantiated early in the Epub parser code to decrypt navigation and SMIL files, so it can be returned in thePubBox
rather than aContainer
. This will also prevent apps from accidentally bypassing deciphering while accessing resources. Consequently, some content filters should be passed to the PublicationParser by the app.Fetcher
should be passed to reading activities (or probably to the navigator) (see Exposing Fragment instead of Activity kotlin-toolkit#176). So,Fetcher
(at least an interface) should be defined in theshared
module.plus
operation could mix content filter chains or give the content filters from oldFetcher a highest priority by chaining the content filter chain of oldFetcher and the new one.Server
instance. So, the interface might be moved to Shared, and the R2 implementation to the Navigator (or testapp).The text was updated successfully, but these errors were encountered: