Implement internal metadata and image provider for EPUBs

[itegulov]

This PR implements two major things:

• EpubMetadataProvider: Opens EPUB as a zip acrhive, finds its OPF file and uses it to fill in book's metadata (e.g. title, description, authors etc).
• EpubMetadataImageProvider: Opens EPUB as a zip archive, figures out where the cover image is located and loads it from EPUB using IProviderManager

I have tried out the plugin with this PR in on a library with 982 EPUBs (a mix of both 2.0 and 3.0 versions) and it took my machine 18 minutes to index them.

EpubMetadataImageProvider is not perfect as I could not figure out which interface to implement to better reflect its nature. ILocalImageFileProvider is not suitable as it requires me to return FileSystemMetadata which I can't provide as the image is located inside of an EPUB. I managed to get it to work by using IProviderManager's ability to download and save images from streams, but I am open to suggestions if there is a better way to do it.

[dkanada]

Have you looked at IDynamicImageProvider in the same area as the other two image providers? That might work as an alternative, but I haven't looked into it yet.

[itegulov]

@dkanada IDynamicImageProvider seems to be what I was looking for. However, simply changing EpubMetadataImageProvider's base type from ILocalImageFileProvider to IDynamicImageProvider is causing jellyfin not to call EpubMetadataImageProvider at all. Here is an excerpt from debug logs showing what is being called on a library refresh:

[2020-05-15 21:58:05.315 +10:00] [DBG] [18] App: Running "CollectionFolderImageProvider" for "/var/lib/jellyfin/root/default/Books"
[2020-05-15 21:58:05.403 +10:00] [DBG] [18] App: Running "FolderImageProvider" for "/jellyfin-test"
[2020-05-15 21:58:05.493 +10:00] [DBG] [18] App: Running "GoogleBooksProvider" for "/jellyfin-test/book1.epub"
[2020-05-15 21:58:08.984 +10:00] [DBG] [24] App: "GoogleBooksProvider" returned no metadata for "/jellyfin-test/book1.epub"
[2020-05-15 21:58:07.502 +10:00] [DBG] [24] App: Running "EpubMetadataProvider" for "/jellyfin-test/book1.epub"
[2020-05-15 21:58:07.520 +10:00] [DBG] [24] App: Running "GoogleBooksImageProvider" for "/jellyfin-test/book1.epub"

Do you know if there is any additional conditions for when a IDynamicImageProvider is called? I tried making GetSupportedImages return every single image type available, but jellyfin still woudn't call the dynamic provider.

[dkanada]

The weekend is coming up so I'll look into it then! I'm glad to see books getting some attention. Not sure how familiar you are with JavaScript but here was a short attempt at getting book support in the web client.

[itegulov]

@dkanada I am not so hot on JavaScript, but more than happy to help with web epub reader. I have been toying with epubjs today with some minor results (only managed to add controls for browsing through epub). However, I am having a hard time understanding jellyfin-web plugin lifecycle. Do you know if there is any documentation available on this matter? In particular, I am interested in destroying the view once user presses the back button. Right now epub reader view gets stuck on the screen until user refreshes the page.

[dkanada]

Unfortunately the plugins are probably the least understood section of the web client right now, and no one has looked too deeply into how they work yet. I have a general idea of how we can implement the actual interface though. The real blocker on that branch was that the reader itself wasn't rendering the book correctly at all for me. I was only getting the title page and couldn't scroll or navigate through the book at all.

[dkanada]

Do you know if there is any additional conditions for when a IDynamicImageProvider is called?

I think the relevant code is right here and it seems to behave similar to the remote providers, not sure why it wouldn't be working.

[itegulov]

@dkanada After some amount of looking through the logs and jellyfin source code, I figured it out. Previously, my implementation of Supports for EpubMetadataImageProvider looked like this:

public bool Supports(BaseItem item)
{
    return item is Book && Path.GetExtension(item.Path) == ".epub";
}

The idea was to limit provider to only process books with EPUB extension. However, jellyfin was testing the provider with a dummy item whose type was book, but its path was /var/lib/jellyfin/metadata/dummy. When my provider said that it does not support this item (as its extension is not .epub), jellyfin decided that this provider is not suitable for processing any book items. It kind of makes sense, but this is only happening when the base type is either IDynamicImageProvider or IRemoteImageProvider and not ILocalImageProvider weirdly so there might a bug there somewhere.

Anyway, I weakened Supports method and adapted the provider to be an IDynamicImageProvider so I think the code is ready for a review whenever you are available.

Thanks for going through this issue with me!

[dkanada]

Nice! We should really support your strict support conditions to prevent needless calls to providers but for now you can probably work around this issue by checking the path in the methods themselves and returning an empty result when the item isn't an EPUB file.

[itegulov]

for now you can probably work around this issue by checking the path in the methods themselves and returning an empty result when the item isn't an EPUB file.

Yep, that is exactly what I did

[itegulov]

Regarding epub reader: here is my branch with a minimalistic epub reader which renders books properly (at least for me) and you can navigate through the book by pressing left/right arrow keys.

A short demo of me navigating through Moby Dick:

P.S. I don't think this PR is the ideal place to discuss epub reader, but I don't know where we should do it otherwise. Should I just open a WIP PR to jellyfin-web?

[dkanada]

A pull request sounds perfect, we can discuss the implementation there! One thing I'd like to mention already is that I don't think we want separate pages for the media players. The only one that currently uses this pattern is videoosd.html but I'd like to remove it eventually. It messes up the history and prevents us from utilizing the benefits of an SPA while media is playing.

Ideally, both the book player and video player would use a generic dialog component that would overlay a window on the current page. There would be a button on the top right to shrink the player to the bottom left, which would let users navigate anywhere while keeping the player open. I'm not sure if you've used YouTube on a phone, but basically the same behavior.

[itegulov]

@dkanada Done! Please see https://github.com/jellyfin/jellyfin-web/pull/1263. Let's continue our conversation there.

[dkanada]

That logging change might be difficult without the package nightlies on NuGet.

[crobibero]

That logging change might be difficult without the package nightlies on NuGet.

Typed logging is already supported, the change was to remove generic logging injection

[itegulov]

@crobibero Thanks for the review! I have addressed your comments and converted all ILogger instances to the typed values. Sorry for not following C# coding conventions; I don't usually work with C#. Is there any generally accepted automatic code formatter we can use for this project to enforce it?

[crobibero]

Good point, we will likely be adding stylecop for the 10.6 build wave. I'll re-review the changes in the morning, but thanks for fixing the Logger instances