readium/architecture

Model for the Publication's format/type

mickael-menu opened this issue · 9 comments

mickael-menu commented

The way we handle Publication formats is not unified across platforms.

  • We don't have a specified strategy to determine the format of a file (media type, file extension, etc.). So we might see differences across platforms.
  • We have different enums with different cases, presenting a number of issues.

Swift

enum Publication.Format {
    /// Formats natively supported by Readium.
    case cbz, epub, pdf, webpub
    /// Default value when the format is not specified.
    case unknown
}

It's not clear what this represents:

  • PDF is used for both LCPDF and PDF (because they are presented with PDFNavigator)
  • EPUB and WEBPUB are separate but they use the same navigator

Kotlin

enum class Publication.TYPE {
    EPUB, CBZ, FXL, WEBPUB, AUDIO, DiViNa
}

Note that it's very different from the Swift version.
There's also some issues:

  • FXL is separate from EPUB, because they use different navigators
  • EPUB and WEBPUB are separate but they use the same navigator
  • There's no UNKNOWN value for publications without a specified format (eg. parsed from OPDS)

Technically, we shouldn't need the format of the Publication, because in our model a Publication is not limited to one type of document in the readingOrder. For example, we could have a Publication containing a PDF, and then an HTML page.

However, currently the format is needed for several reasons:

  • Despite the fact that a Publication can contain several media types, the current navigator implementation requires a single format throughout the readingOrder.
  • The test-app is deciding which Navigator to create according to the Publication format/type.
  • The format is needed when downloading a publication to set the file extension.
  • The format might be of interest for the test-app (to display it in the user library).

Proposed Solution

  1. I think we should remove the format/type from the Publication model:
  • As we've seen, it contradicts the flexibility of the model
  • In some cases, it doesn't make sense to have a format (e.g. parsing an OPDS entry)

  1. However, and since we still need the format in the test-app, we could move it to the more relevant Container object. It makes sense because the Container represents the Publication file. It could also be useful because if we know the Container's format, we can infer some available files in it (e.g. an EPUB Container has a META-INF/container.xml resource).

  2. Instead of using a Format or Type enum as seen above, we should be very specific and introduce a MediaType enum (so LCPDF could be differentiated from PDF). This enum could centralize all the logics around media types and a specified strategy to determine a media type from a string and/or extension.
    EDIT: I don't think that MediaType should be an enum anymore, see #116 (comment)

  3. We should remove the navigator creation logic from the test-app. For that, we could expose a unique Navigator facade in the navigator repo that would internally wrap the specific navigators. This would be simpler in the test-app, and we would be able to change the implementation without breaking the apps, for example to support multiple formats in a single navigator.

A challenge is that Publication.type/format is used a lot throughout the test-app, so it will be a breaking change. Maybe there're some ways to ease the migration. For example keeping the format in the Publication for some time, with a deprecation warning.

@danielweck Do you have any insights from the JS implementation?

danielweck commented

@danielweck Do you have any insights from the JS implementation?

There is currently only one "navigator" implementation, which renders readingOrder HTML documents of EPUB reflowable and fixed-layout publications. There is currently no support for PDF, audio books, image books (CBZ), etc. Therefore, the current implementation is quite naive, and does not need to discriminate different kinds of publication.

HadrienGardeur commented

This is a complex matter and I don't think that a single solution will help us achieve our goals.

IMO we would need at least:

  • to preserve the info communicated in JSON-LD (@type) when we parse a RWPM or a W3C Audiobook
  • to preserve the media type of the package or manifest that we're using to initialize a Publication
  • a helper that can look into the resources expressed in the readingOrder and indicated if a publication is entirely based on: HTML, audio, images, video or a mixed use case
mickael-menu commented
  • to preserve the info communicated in JSON-LD (@type) when we parse a RWPM or a W3C Audiobook

It's currently available under publication.metadata.type. However, it is not used by the EPUB, CBZ and PDF parsers. Should they add the media type there? The parser is the best place to know the file format.

  • to preserve the media type of the package or manifest that we're using to initialize a Publication

Is it the one provided by the test-app? Or resolved from the file by the parser? I think this is the thing that should be in Container and not Publication (except for @type).

  • a helper that can look into the resources expressed in the readingOrder and indicated if a publication is entirely based on: HTML, audio, images, video or a mixed use case

Definitely, this could be useful for the navigator. Incidentally, this could be based on the @type as well.

Though all of this doesn't help to figure out the format of a file to be imported, hence the MediaType helper.

HadrienGardeur commented

However, it is not used by the EPUB, CBZ and PDF parsers. Should they add the media type there?

We could provide a default value for EPUB (https://schema.org/Book), PDF (https://schema.org/DigitalDocument) and CBZ (https://schema.org/ComicIssue?).

Definitely, this could be useful for the navigator. Incidentally, this could be based on the @type as well.

Ideally I think this should be separate from the other two. There's a difference between what's advertised by a file and what's truly in its readingOrder.

HadrienGardeur commented

Also, something else that we want to keep in mind: using the HTML navigator won't always be the optimal choice.

EPUB 3 comics could benefit from our ability to:

  • create an alternate pointing to an image directly instead of HTML
  • "guess" that these publications should default to an image navigator instead

This would require inspecting the HTML files of an FXL publication, which is probably beyond the scope of this discussion though.

Kobo supports something like this (thanks @JayPanoz for the reference): https://github.com/kobolabs/epub-spec#image-based-fxl-reader

mickael-menu commented

Interesting! That goes definitely in the direction of letting the navigator decide which internal viewer to use.

llemeurfr commented

Should we create a model with one Navigator and N specialized viewers, or an Navigator factory and N specialized Navigators?

mickael-menu commented

I think that if we want to be able to support publications with multiple kind of contents down the line, it would make more sense to have one Navigator with N viewers.

An extension point could be added here to allow custom Viewers seamlessly.

But to start, the apps don't need to be aware of the Viewers. It should stay an implementation detail until we need the extensibility.

mickael-menu commented

I submitted a full proposal for this in this PR: #127