diff --git a/CHANGELOG.md b/CHANGELOG.md index e58114ac..f86cff8d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,7 @@ as of 2.0.0. - UI: replace book grid borders with rounded card backgrounds, highlighted on hover, in book grid, book list, book carousel and author carousel - Gutenberg: replace `--with-author-details` with `--with-author-bio` and `--with-author-portrait` (#66) - Author details: fetch biographies and portraits in the language the ZIM is built in, resolved through Wikipedia's interlanguage links, and keep the English picture when the localized article has none (#65) +- Gutenberg: prefer the cover declared by the downloaded EPUB over Project Gutenberg's ~200px `cover.medium.jpg` thumbnail, which yields sharper covers without an extra download; the thumbnail stays as fallback (#68) ### Fixed diff --git a/scraper/src/papers2zim/core/covers.py b/scraper/src/papers2zim/core/covers.py index 03d224ff..80db1a8b 100644 --- a/scraper/src/papers2zim/core/covers.py +++ b/scraper/src/papers2zim/core/covers.py @@ -18,16 +18,29 @@ from papers2zim.core.rewriters.image_rewriter import ImageProcessor -def extract_cover(content: bytes, format_name: str) -> bytes | None: - """Extract and WebP-encode a cover image from a downloaded book file.""" +def extract_cover( + content: bytes, + format_name: str, + *, + max_width: int | None = None, + declared_only: bool = False, +) -> bytes | None: + """Extract and WebP-encode a cover image from a downloaded book file. + + max_width scales the result down (never up) before encoding. + declared_only restricts EPUB covers to the image the package explicitly + declares as its cover, instead of falling back to the first image found. + """ try: if format_name == "pdf": image = _pdf_first_page(content) elif format_name == "epub": - image = _epub_cover(content) + image = _epub_cover(content, declared_only=declared_only) else: return None - return ImageProcessor.optimize_image_content(image) if image else None + if not image: + return None + return ImageProcessor.optimize_image_content(image, max_width=max_width) except Exception as exc: logger.debug("Could not extract %s cover: %s", format_name, exc) return None @@ -44,7 +57,7 @@ def _pdf_first_page(content: bytes) -> bytes | None: document.close() -def _epub_cover(content: bytes) -> bytes | None: +def _epub_cover(content: bytes, *, declared_only: bool = False) -> bytes | None: with zipfile.ZipFile(io.BytesIO(content)) as archive: parser = etree.XMLParser(resolve_entities=False, no_network=True) container = etree.fromstring(archive.read("META-INF/container.xml"), parser) @@ -67,7 +80,7 @@ def _epub_cover(content: bytes) -> bytes | None: None, ) cover_item = manifest.get(cover_id) if cover_id else None - if cover_item is None: + if cover_item is None and not declared_only: cover_item = next( ( item @@ -76,7 +89,7 @@ def _epub_cover(content: bytes) -> bytes | None: ), None, ) - if cover_item is None: + if cover_item is None and not declared_only: cover_item = next( ( item diff --git a/scraper/src/papers2zim/core/rewriters/image_rewriter.py b/scraper/src/papers2zim/core/rewriters/image_rewriter.py index f4b600cb..58c9ac64 100644 --- a/scraper/src/papers2zim/core/rewriters/image_rewriter.py +++ b/scraper/src/papers2zim/core/rewriters/image_rewriter.py @@ -8,6 +8,7 @@ from pathlib import Path from bs4 import BeautifulSoup +from PIL.Image import Resampling from PIL.Image import open as pilopen from zimscraperlib.image.optimization import OptimizeWebpOptions @@ -58,9 +59,17 @@ def get_output_filename(filename: str) -> str: return filename @staticmethod - def optimize_image_content(file_content: bytes) -> bytes: - """Convert and optimize image content to WebP format.""" + def optimize_image_content( + file_content: bytes, max_width: int | None = None + ) -> bytes: + """Convert and optimize image content to WebP format. + + When max_width is set, images wider than that are scaled down + (never up) before conversion. + """ dst = io.BytesIO() with pilopen(io.BytesIO(file_content)) as image: + if max_width is not None and image.width > max_width: + image.thumbnail((max_width, image.height), Resampling.LANCZOS) image.save(dst, format="WEBP", **default_webp_options) return dst.getvalue() diff --git a/scraper/src/papers2zim/sources/gutenberg/exporter.py b/scraper/src/papers2zim/sources/gutenberg/exporter.py index e1ab101f..b8c3665a 100644 --- a/scraper/src/papers2zim/sources/gutenberg/exporter.py +++ b/scraper/src/papers2zim/sources/gutenberg/exporter.py @@ -4,6 +4,7 @@ from functools import partial from papers2zim.constants import logger +from papers2zim.core.covers import extract_cover from papers2zim.core.download_engine import DownloadEngine from papers2zim.core.epub_optimizer import optimize_epub_bytes from papers2zim.core.models import Work @@ -27,6 +28,32 @@ _COVER_BASENAME_RE = re.compile(r"^(?:cover|\d+-cover)\.(?:jpe?g|png|webp)$", re.I) +COVER_MAX_WIDTH = 400 + + +def _embedded_epub_cover(book_files: dict[str, bytes], work: Work) -> bytes | None: + """Return the WebP-encoded cover declared by the downloaded EPUB, if any. + + Project Gutenberg only publishes a roughly 200px-wide + ``pg{id}.cover.medium.jpg`` thumbnail, while the EPUB ships the same + artwork at full resolution. Preferring the embedded image avoids a mirror + round-trip and keeps the cover unambiguously part of the work itself, + which is what Project Gutenberg's own distribution policy asks for. + + Only a cover the EPUB explicitly declares is used: unlike the generic + fallback, an undeclared first image is often a decorative ornament and + would be worse than the mirror thumbnail. + """ + epub_content = book_files.get(fname_for(work, "epub")) + if not epub_content: + return None + return extract_cover( + epub_content, + "epub", + max_width=COVER_MAX_WIDTH, + declared_only=True, + ) + def is_cover_asset(work_id: str, filename: str) -> bool: """Return True when a book file is a Project Gutenberg cover asset""" @@ -68,13 +95,19 @@ def export_book( target=html_cover_path, ) else: - # No HTML cover - download from mirror - cover_image = download_book_cover(mirror_url, work, engine) + # No HTML cover: the EPUB carries the same artwork as the mirror's + # thumbnail at full resolution, so prefer it and skip the round-trip. + cover_image = _embedded_epub_cover(book_files, work) + + if cover_image: + logger.debug(f"Using embedded EPUB cover for book #{work.id}") + else: + cover_image = download_book_cover(mirror_url, work, engine) + if cover_image: + logger.debug(f"Using downloaded cover for book #{work.id}") + cover_image = ImageProcessor.optimize_image_content(cover_image) if cover_image: - logger.debug(f"Using downloaded cover for book #{work.id}") - # the mirror serves JPEG; convert to WebP to match cover_path/mimetype - cover_image = ImageProcessor.optimize_image_content(cover_image) assembler.add_item_for( path=cover_path, content=cover_image, diff --git a/scraper/tests/conftest.py b/scraper/tests/conftest.py index 52030f92..79a92edf 100644 --- a/scraper/tests/conftest.py +++ b/scraper/tests/conftest.py @@ -1,10 +1,46 @@ +import io +import zipfile from unittest.mock import MagicMock import pytest +from PIL import Image from papers2zim.core.models import CollectionRef, Cover, Creator, Work +@pytest.fixture +def make_epub(): + """Build a minimal EPUB carrying a single JPEG cover image. + + Call with declared=False to omit the cover declaration, which is how a + package ends up with only an undeclared image. + """ + + def _make(*, declared: bool = True, size: tuple = (64, 96)) -> bytes: + image = io.BytesIO() + Image.new("RGB", size, "white").save(image, format="JPEG") + meta = '' if declared else "" + properties = ' properties="cover-image"' if declared else "" + epub = io.BytesIO() + with zipfile.ZipFile(epub, "w") as archive: + archive.writestr("mimetype", "application/epub+zip") + archive.writestr( + "META-INF/container.xml", + '' + "", + ) + archive.writestr( + "OPS/package.opf", + f"{meta}" + f'", + ) + archive.writestr("OPS/cover.jpg", image.getvalue()) + return epub.getvalue() + + return _make + + @pytest.fixture def tmp_output_dir(tmp_path): """Temporary output folder for ZIMs and downloads""" diff --git a/scraper/tests/test_core_covers.py b/scraper/tests/test_core_covers.py index 960a85e7..9455959d 100644 --- a/scraper/tests/test_core_covers.py +++ b/scraper/tests/test_core_covers.py @@ -3,6 +3,8 @@ import io import zipfile +from PIL import Image + from papers2zim.core.covers import _epub_cover, extract_cover @@ -27,3 +29,27 @@ def test_epub_cover_decodes_manifest_href_before_archive_lookup(): archive.writestr("OPS/cover image.jpg", b"cover image") assert _epub_cover(epub.getvalue()) == b"cover image" + + +def test_extract_cover_scales_result_down_to_max_width(make_epub): + cover = extract_cover(make_epub(size=(640, 960)), "epub", max_width=400) + assert cover is not None + assert Image.open(io.BytesIO(cover)).size == (400, 600) + + +def test_extract_cover_never_upscales_to_reach_max_width(make_epub): + cover = extract_cover(make_epub(size=(64, 96)), "epub", max_width=400) + assert cover is not None + assert Image.open(io.BytesIO(cover)).size == (64, 96) + + +def test_extract_cover_keeps_declared_cover_when_not_restricted(make_epub): + assert extract_cover(make_epub(), "epub") is not None + + +def test_extract_cover_declared_only_ignores_undeclared_image(make_epub): + assert extract_cover(make_epub(declared=False), "epub", declared_only=True) is None + + +def test_extract_cover_falls_back_to_undeclared_image_by_default(make_epub): + assert extract_cover(make_epub(declared=False), "epub") is not None diff --git a/scraper/tests/test_gutenberg_exporter.py b/scraper/tests/test_gutenberg_exporter.py index ff5a90cf..dbaf60b3 100644 --- a/scraper/tests/test_gutenberg_exporter.py +++ b/scraper/tests/test_gutenberg_exporter.py @@ -8,6 +8,7 @@ from papers2zim.core.models import Work from papers2zim.sources.gutenberg.exporter import ( + COVER_MAX_WIDTH, export_book, handle_book_files, is_cover_asset, @@ -181,6 +182,80 @@ def test_export_book_still_uses_icon_linked_html_cover(): ) +def _stored_cover(assembler): + """Return the bytes stored for the ZIM cover path.""" + path = "covers/11_cover_image.webp" + for call in assembler.add_item_for.call_args_list: + if call.kwargs.get("path") == path: + return call.kwargs["content"] + return None + + +def test_export_book_prefers_embedded_epub_cover_over_mirror(make_epub): + work = _work() + assembler = MagicMock() + engine = MagicMock() + with patch("papers2zim.sources.gutenberg.exporter.download_book_cover") as download: + export_book( + work=work, + book_files={"11.epub": make_epub(size=(800, 1200))}, + formats=["epub"], + mirror_url="https://example.com", + assembler=assembler, + engine=engine, + _zim_name="test", + _title_search=False, + ) + download.assert_not_called() + cover = _stored_cover(assembler) + assert cover is not None + assert Image.open(io.BytesIO(cover)).size == (COVER_MAX_WIDTH, 600) + + +def test_export_book_falls_back_to_mirror_when_epub_cover_is_undeclared(make_epub): + work = _work() + assembler = MagicMock() + engine = MagicMock() + with patch( + "papers2zim.sources.gutenberg.exporter.download_book_cover", + return_value=_image_bytes("JPEG"), + ) as download: + export_book( + work=work, + book_files={"11.epub": make_epub(declared=False)}, + formats=["epub"], + mirror_url="https://example.com", + assembler=assembler, + engine=engine, + _zim_name="test", + _title_search=False, + ) + download.assert_called_once() + assert _stored_cover(assembler) is not None + + +def test_export_book_falls_back_to_mirror_when_no_epub_is_present(): + work = _work() + assembler = MagicMock() + engine = MagicMock() + with patch( + "papers2zim.sources.gutenberg.exporter.download_book_cover", + return_value=_image_bytes("JPEG"), + ) as download: + export_book( + work=work, + book_files={"11.pdf": b"%PDF-1.4 fake"}, + formats=["pdf"], + mirror_url="https://example.com", + assembler=assembler, + engine=engine, + _zim_name="test", + _title_search=False, + ) + download.assert_called_once() + assert _stored_cover(assembler) is not None + + def test_export_book_prefers_bundled_cover_over_mismatched_icon_link(): """Regression test for #49: PG sometimes tags a decorative illustration as the `` cover (e.g. a thin title-page divider) while also