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

Add support for audio and video tags in epub3 and add firstImageIsCover option #178

Open
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

uriesk
Copy link

@uriesk uriesk commented Jan 10, 2025

Epub3 supports <video> and <audio> tags. This pull request adds support for them.
The media gets downloaded the same as images do.

Add firstImageIsCover option. When true, the first appearing image inside the ebook will get marked as cover in the manifest, so epub viewers can use it as thumbnail.

Caveats

  • Audio and Video files get put into an audiovideo subfolder, which got recommended here.
  • I change occuring controls attributes to controls="Controls", cause this is what XHTML wants. controls="controls" didn't work, cause rehype-stringify would change it back, but XHTML isn't case sensitive in attributes, so it works.
  • an additional little commit, to not add <meta name="cover" content="image_cover"/ in the manifest if there is no cover (calibre ebook editor gave a warning for this).
  • add start of content to guide
  • additional ad nav landmarks to epub3, since <guide> is deprecated and at least Apple has the requirement for landmarks to exist.

@uriesk uriesk changed the title Add support for audio and video tags in epub3 Add support for audio and video tags in epub3 and add firstImageIsCover option Jan 11, 2025
@uriesk
Copy link
Author

uriesk commented Jan 11, 2025

@e-adrien hi, i am now ready with this PR. I decided to add multiple commits for different issues to it, because it would be too cumbersome otherwise, as everything happens in only three files.

I did check with epubcheck-5.2.0 and with the check of calibre ebook editor, to make sure that everything is valid.

@e-adrien
Copy link

Hi @uriesk, thank you for this PR.

I think it will be better to use useFirstImageAsCover for the parameter name instead of firstImageIsCover.
I also think that the Audio and Video files embedding should be an optional feature (downloadAudioVideoFiles parameter, false by default).

Do you mind adding a test here ?

Thank you again for your work.

@@ -13,7 +13,7 @@
<dc:creator opf:role="aut" opf:file-as="<%= author.length ? author.join(",") : author %>"><%= author.length ? author.join(",") : author %></dc:creator>
<dc:date opf:event="modification"><%= date %></dc:date>
<dc:language><%= lang || "en" %></dc:language>
<meta name="cover" content="image_cover" />
<% if(locals.cover || (locals.firstImageIsCover && images.length)) { %><meta name="cover" content="<%= locals.cover ? 'image_cover' : 'image_0' %>"/><% } %>

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we can avoid these if by puting some logic in this method

private async makeCover(): Promise<void> {

No cover and No ToC v2
No cover and No ToC v3
Audiovideo
First image as cover v2
First image as cover v3
@uriesk
Copy link
Author

uriesk commented Jan 24, 2025

@e-adrien added a bunch of test cases, fixed the formatting, renamed to useFirstImageAsCover and added downloadAudioVideoFiles

I don't like the long condition in the template either, but i didn't want to touch the makeCover stuff because it does so much extra things with creating a separate page for the cover.
Feel free to refactor it, but i personally think it's good how it is now.

Edit: the removing of the leading newline in that ibook file fixes an xml warning in calibres ebook editor

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

Successfully merging this pull request may close these issues.

2 participants