The world is not ready for :=

Fixes #894

docs/release_notes.rst

@@ -10,13 +10,21 @@ that is, output messages may be improved at any release level, so parsing them
 may be unreliable. Use the API to depend on precise behavior.
 
 The public API may be useful in scripts that launch OCRmyPDF processes or that
-wish to use some of its features for working with PDFs.
+wish to use some of its features for working with PDFs..
 
-.. note::
+v13.3.0
+=======
 
-   Python 3.6 reaches end of life on December 23, 2021. We have already ended support
-   for Python 3.6 but might release fixes for critical issues if necessary before that
-   date.
+-  Made a harmless but "scary" exception after failing to optimize an image less scary.
+-  Added a warning if a page image is too large for unpaper to clean. The image is
+   passed through without cleaning. This is due to a hard-coded limitation in a
+   C library used by unpaper so it cannot be rectified easily.
+-  We now use better default settings when calling img2pdf.
+-  We no longer try to optimize images that we failed to save in certain situations.
+-  We now account for some differences in text output from Tesseract 5 that differs
+   from Tesseract 4.
+-  Better handling of Ghostscript producing empty images when attempting to rasterize
+   page images.
 
 v13.2.0
 =======

src/ocrmypdf/_exec/ghostscript.py

@@ -18,7 +18,7 @@ from shutil import which
 from subprocess import PIPE, CalledProcessError
 from typing import Optional
 
-from PIL import Image
+from PIL import Image, UnidentifiedImageError
 
 from ocrmypdf.exceptions import MissingDependencyError, SubprocessOutputError
 from ocrmypdf.helpers import Resolution
@@ -71,7 +71,8 @@ def jpeg_passthrough_available() -> bool:
 
 
 def _gs_error_reported(stream) -> bool:
-    return True if re.search(r'error', stream, flags=re.IGNORECASE) else False
+    match = re.search(r'error', stream, flags=re.IGNORECASE)
+    return bool(match)
 
 
 def rasterize_pdf(
@@ -124,20 +125,27 @@ def rasterize_pdf(
         if _gs_error_reported(stderr):
             log.error(stderr)
 
-    with Image.open(BytesIO(p.stdout)) as im:
-        if rotation is not None:
-            log.debug("Rotating output by %i", rotation)
-            # rotation is a clockwise angle and Image.ROTATE_* is
-            # counterclockwise so this cancels out the rotation
-            if rotation == 90:
-                im = im.transpose(Image.ROTATE_90)
-            elif rotation == 180:
-                im = im.transpose(Image.ROTATE_180)
-            elif rotation == 270:
-                im = im.transpose(Image.ROTATE_270)
-            if rotation % 180 == 90:
-                page_dpi = page_dpi.flip_axis()
-        im.save(fspath(output_file), dpi=page_dpi)
+    try:
+        with Image.open(BytesIO(p.stdout)) as im:
+            if rotation is not None:
+                log.debug("Rotating output by %i", rotation)
+                # rotation is a clockwise angle and Image.ROTATE_* is
+                # counterclockwise so this cancels out the rotation
+                if rotation == 90:
+                    im = im.transpose(Image.ROTATE_90)
+                elif rotation == 180:
+                    im = im.transpose(Image.ROTATE_180)
+                elif rotation == 270:
+                    im = im.transpose(Image.ROTATE_270)
+                if rotation % 180 == 90:
+                    page_dpi = page_dpi.flip_axis()
+            im.save(fspath(output_file), dpi=page_dpi)
+    except UnidentifiedImageError:
+        log.error(
+            f"Ghostscript (using {raster_device} at {raster_dpi} dpi) produced "
+            "an invalid page image file."
+        )
+        raise
 
 
 class GhostscriptFollower:
@@ -161,8 +169,7 @@ class GhostscriptFollower:
                 )
                 return
         else:
-            m = self.re_page.match(line.strip())
-            if m:
+            if self.re_page.match(line.strip()):
                 self.progressbar.update()
 
 

src/ocrmypdf/_exec/tesseract.py

@@ -344,7 +344,7 @@ def generate_hocr(
         _generate_null_hocr(output_hocr, output_text, input_file)
     except CalledProcessError as e:
         tesseract_log_output(e.output)
-        if b'Image too large' in e.output:
+        if b'Image too large' in e.output or b'Empty page!!' in e.output:
             _generate_null_hocr(output_hocr, output_text, input_file)
             return
 
@@ -416,7 +416,7 @@ def generate_pdf(
         use_skip_page(output_pdf, output_text)
     except CalledProcessError as e:
         tesseract_log_output(e.output)
-        if b'Image too large' in e.output:
+        if b'Image too large' in e.output or b'Empty page!!' in e.output:
             use_skip_page(output_pdf, output_text)
             return
         raise SubprocessOutputError() from e

src/ocrmypdf/_exec/unpaper.py

@@ -13,6 +13,7 @@
 import logging
 import os
 import shlex
+from contextlib import contextmanager
 from decimal import Decimal
 from pathlib import Path
 from subprocess import PIPE, STDOUT
@@ -22,60 +23,84 @@ from typing import List, Optional, Tuple, Union
 from PIL import Image
 
 from ocrmypdf.exceptions import MissingDependencyError, SubprocessOutputError
-from ocrmypdf.subprocess import get_version
-from ocrmypdf.subprocess import run as external_run
+from ocrmypdf.subprocess import get_version, run
+
+UNPAPER_IMAGE_PIXEL_LIMIT = 256 * 1024 * 1024
 
 DecFloat = Union[Decimal, float]
 
 log = logging.getLogger(__name__)
 
 
+class UnpaperImageTooLargeError(Exception):
+    def __init__(
+        self,
+        w,
+        h,
+        message="Image with size {}x{} is too large for cleaning with 'unpaper'.",
+    ):
+        self.w = w
+        self.h = h
+        self.message = message.format(w, h)
+        super().__init__(self.message)
+
+
 def version() -> str:
     return get_version('unpaper')
 
 
-def _setup_unpaper_io(tmpdir: Path, input_file: Path) -> Tuple[Path, Path]:
+def _convert_image(im: Image.Image) -> Tuple[Image.Image, bool, str]:
     SUFFIXES = {'1': '.pbm', 'L': '.pgm', 'RGB': '.ppm'}
-    with Image.open(input_file) as im:
-        im_modified = False
-        if im.mode not in SUFFIXES:
-            log.info("Converting image to other colorspace")
-            try:
-                if im.mode == 'P' and len(im.getcolors()) == 2:
-                    im = im.convert(mode='1')
-                else:
-                    im = im.convert(mode='RGB')
-            except OSError as e:
-                raise MissingDependencyError(
-                    "Could not convert image with type " + im.mode
-                ) from e
-            else:
-                im_modified = True
+    im_modified = False
+
+    if im.mode not in SUFFIXES:
+        log.info("Converting image to other colorspace")
         try:
-            suffix = SUFFIXES[im.mode]
-        except KeyError:
+            if im.mode == 'P' and len(im.getcolors()) == 2:
+                im = im.convert(mode='1')
+            else:
+                im = im.convert(mode='RGB')
+        except OSError as e:
             raise MissingDependencyError(
-                "Failed to convert image to a supported format."
-            ) from None
-
-        if im_modified or input_file.suffix != '.pnm':
-            input_pnm = tmpdir / 'input.pnm'
-            im.save(input_pnm, format='PPM')
+                "Could not convert image with type " + im.mode
+            ) from e
         else:
-            # No changes, PNG input, just use the file we already have
-            input_pnm = input_file
-        output_pnm = tmpdir / f'output{suffix}'
-    return input_pnm, output_pnm
+            im_modified = True
+    try:
+        suffix = SUFFIXES[im.mode]
+    except KeyError:
+        raise MissingDependencyError(
+            "Failed to convert image to a supported format."
+        ) from None
+    return im, im_modified, suffix
 
 
-def run(
+@contextmanager
+def _setup_unpaper_io(input_file: Path) -> Tuple[Path, Path, Path]:
+    with Image.open(input_file) as im:
+        if im.width * im.height >= UNPAPER_IMAGE_PIXEL_LIMIT:
+            raise UnpaperImageTooLargeError(w=im.width, h=im.height)
+        im, im_modified, suffix = _convert_image(im)
+
+        with TemporaryDirectory() as tmpdir:
+            tmppath = Path(tmpdir)
+            if im_modified or input_file.suffix != '.pnm':
+                input_pnm = tmppath / 'input.pnm'
+                im.save(input_pnm, format='PPM')
+            else:
+                # No changes, PNG input, just use the file we already have
+                input_pnm = input_file
+
+            output_pnm = tmppath / f'output{suffix}'
+            yield input_pnm, output_pnm, tmppath
+
+
+def run_unpaper(
     input_file: Path, output_file: Path, *, dpi: DecFloat, mode_args: List[str]
 ) -> None:
     args_unpaper = ['unpaper', '-v', '--dpi', str(round(dpi, 6))] + mode_args
 
-    with TemporaryDirectory() as tmpdir:
-        input_pnm, output_pnm = _setup_unpaper_io(Path(tmpdir), input_file)
-
+    with _setup_unpaper_io(input_file) as (input_pnm, output_pnm, tmpdir):
         # To prevent any shenanigans from accepting arbitrary parameters in
         # --unpaper-args, we:
         # 1) run with cwd set to a tmpdir with only unpaper's files
@@ -84,7 +109,7 @@ def run(
         # This should ensure that a user cannot clobber some other file with
         # their unpaper arguments (whether intentionally or otherwise)
         args_unpaper.extend([os.fspath(input_pnm), os.fspath(output_pnm)])
-        external_run(
+        run(
             args_unpaper,
             close_fds=True,
             check=True,
@@ -117,7 +142,7 @@ def clean(
     *,
     dpi: DecFloat,
     unpaper_args: Optional[List[str]] = None,
-):
+) -> Path:
     default_args = [
         '--layout',
         'none',
@@ -131,4 +156,9 @@ def clean(
     ]
     if not unpaper_args:
         unpaper_args = default_args
-    run(input_file, output_file, dpi=dpi, mode_args=unpaper_args)
+    try:
+        run_unpaper(input_file, output_file, dpi=dpi, mode_args=unpaper_args)
+        return output_file
+    except UnpaperImageTooLargeError as e:
+        log.warning(str(e))
+        return input_file

src/ocrmypdf/_pipeline.py

@@ -18,7 +18,7 @@ from typing import Dict, Iterable, Optional
 import img2pdf
 import pikepdf
 from pikepdf.models.metadata import encode_pdf_date
-from PIL import Image, ImageColor, ImageDraw
+from PIL import Image, ImageDraw
 
 from ocrmypdf._concurrent import Executor
 from ocrmypdf._exec import unpaper
@@ -32,7 +32,7 @@ from ocrmypdf.exceptions import (
     PriorOcrFoundError,
     UnsupportedImageFormatError,
 )
-from ocrmypdf.helpers import Resolution, safe_symlink
+from ocrmypdf.helpers import IMG2PDF_KWARGS, Resolution, safe_symlink
 from ocrmypdf.hocrtransform import HocrTransform
 from ocrmypdf.optimize import optimize
 from ocrmypdf.pdfa import generate_pdfa_ps
@@ -98,8 +98,8 @@ def triage_image_file(input_file, output_file, options):
             img2pdf.convert(
                 os.fspath(input_file),
                 layout_fun=layout_fun,
-                with_pdfrw=False,
                 outputstream=outf,
+                **IMG2PDF_KWARGS,
             )
         log.info("Successfully converted to PDF, processing...")
     except img2pdf.ImageOpenError as e:
@@ -494,13 +494,12 @@ def preprocess_deskew(input_file: Path, page_context: PageContext):
 def preprocess_clean(input_file: Path, page_context: PageContext):
     output_file = page_context.get_path('pp_clean.png')
     dpi = get_page_square_dpi(page_context.pageinfo, page_context.options)
-    unpaper.clean(
+    return unpaper.clean(
         input_file,
         output_file,
         dpi=dpi.x,
         unpaper_args=page_context.options.unpaper_args,
     )
-    return output_file
 
 
 def create_ocr_image(image: Path, page_context: PageContext):
@@ -613,7 +612,7 @@ def create_pdf_page_from_image(
 
         layout_fun = img2pdf.get_layout_fun(pagesize)
         img2pdf.convert(
-            imfile, with_pdfrw=False, layout_fun=layout_fun, outputstream=pdf
+            imfile, layout_fun=layout_fun, outputstream=pdf, **IMG2PDF_KWARGS
         )
         log.debug('convert done')
 

src/ocrmypdf/cli.py

@@ -142,7 +142,7 @@ Online documentation is located at:
         'output_file',
         metavar="output_pdf",
         help="Output searchable PDF file (or '-' to write to standard output). "
-        "Existing files will be ovewritten. If same as input file, the "
+        "Existing files will be overwritten. If same as input file, the "
         "input file will be updated only if processing is successful.",
     )
     parser.add_argument(

src/ocrmypdf/helpers.py

@@ -19,10 +19,21 @@ from math import isclose, isfinite
 from pathlib import Path
 from typing import Any, Sequence
 
+import img2pdf
 import pikepdf
+from packaging.version import Version
 
 log = logging.getLogger(__name__)
 
+if Version(img2pdf.__version__) < Version('0.4.0'):
+    IMG2PDF_KWARGS = dict(without_pdfw=True)
+elif Version(img2pdf.__version__) < Version('0.4.3'):
+    IMG2PDF_KWARGS = dict(engine=img2pdf.Engine.pikepdf)
+else:
+    IMG2PDF_KWARGS = dict(
+        engine=img2pdf.Engine.pikepdf, rotation=img2pdf.Rotation.ifvalid
+    )
+
 
 class Resolution(namedtuple('Resolution', ('x', 'y'))):
     """The number of pixels per inch in each 2D direction.

src/ocrmypdf/optimize.py

@@ -41,7 +41,7 @@ from ocrmypdf._concurrent import Executor, SerialExecutor
 from ocrmypdf._exec import jbig2enc, pngquant
 from ocrmypdf._jobcontext import PdfContext
 from ocrmypdf.exceptions import OutputFileAccessError
-from ocrmypdf.helpers import safe_symlink
+from ocrmypdf.helpers import IMG2PDF_KWARGS, safe_symlink
 
 log = logging.getLogger(__name__)
 
@@ -200,7 +200,11 @@ def extract_image_generic(
     elif not pim.indexed and pim.colorspace in pim.SIMPLE_COLORSPACES:
         # An optimization opportunity here, not currently taken, is directly
         # generating a PNG from compressed data
-        pim.as_pil_image().save(png_name(root, xref))
+        try:
+            pim.as_pil_image().save(png_name(root, xref))
+        except NotImplementedError:
+            log.warning("PDF contains an atypical image that cannot be optimized.")
+            return None
         return XrefExt(xref, '.png')
     elif (
         not pim.indexed
@@ -450,7 +454,7 @@ def transcode_jpegs(
 def _transcode_png(pike: Pdf, filename: Path, xref: Xref) -> bool:
     output = filename.with_suffix('.png.pdf')
     with output.open('wb') as f:
-        img2pdf.convert(fspath(filename), outputstream=f)
+        img2pdf.convert(fspath(filename), outputstream=f, **IMG2PDF_KWARGS)
 
     with Pdf.open(output) as pdf_image:
         foreign_image = next(iter(pdf_image.pages[0].images.values()))

tests/test_ghostscript.py

@@ -6,11 +6,13 @@
 
 
 import logging
+import subprocess
 from decimal import Decimal
+from unittest.mock import patch
 
 import pikepdf
 import pytest
-from PIL import Image
+from PIL import Image, UnidentifiedImageError
 
 from ocrmypdf._exec.ghostscript import rasterize_pdf
 from ocrmypdf.exceptions import ExitCode
@@ -124,3 +126,20 @@ def test_ghostscript_feature_elision(resources, outpdf):
         '--plugin',
         'tests/plugins/gs_feature_elision.py',
     )
+
+
+def test_rasterize_pdf_errors(resources, no_outpdf, caplog):
+    with patch('ocrmypdf._exec.ghostscript.run') as mock:
+        # ghostscript can produce
+        mock.return_value = subprocess.CompletedProcess(
+            ['fakegs'], returncode=0, stdout=b'', stderr=b'error this is an error'
+        )
+        with pytest.raises(UnidentifiedImageError):
+            rasterize_pdf(
+                resources / 'francais.pdf',
+                no_outpdf,
+                raster_device='pngmono',
+                raster_dpi=Resolution(100, 100),
+            )
+        assert "this is an error" in caplog.text
+        assert "invalid page image file" in caplog.text

tests/test_optimize.py

@@ -17,7 +17,7 @@ from PIL import Image, ImageDraw
 from ocrmypdf import optimize as opt
 from ocrmypdf._exec import jbig2enc, pngquant
 from ocrmypdf._exec.ghostscript import rasterize_pdf
-from ocrmypdf.helpers import Resolution
+from ocrmypdf.helpers import IMG2PDF_KWARGS, Resolution
 
 from .conftest import check_ocrmypdf
 
@@ -139,8 +139,8 @@ def test_multiple_pngs(resources, outdir):
         img2pdf.convert(
             fspath(resources / 'baiona_colormapped.png'),
             fspath(resources / 'baiona_gray.png'),
-            with_pdfrw=False,
             outputstream=inpdf,
+            **IMG2PDF_KWARGS,
         )
 
     def mockquant(input_file, output_file, *_args):

tests/test_pdfinfo.py

@@ -17,7 +17,7 @@ from reportlab.pdfgen.canvas import Canvas
 
 from ocrmypdf import pdfinfo
 from ocrmypdf.exceptions import InputFileError
-from ocrmypdf.helpers import Resolution
+from ocrmypdf.helpers import IMG2PDF_KWARGS, Resolution
 from ocrmypdf.pdfinfo import Colorspace, Encoding
 from ocrmypdf.pdfinfo.layout import PDFPage
 
@@ -67,9 +67,9 @@ def test_single_page_image(eight_by_eight, outpdf):
         img2pdf.convert(
             bio,
             producer="img2pdf",
-            with_pdfrw=False,
             layout_fun=layout_fun,
             outputstream=f,
+            **IMG2PDF_KWARGS,
         )
     info = pdfinfo.PdfInfo(outpdf)
 

tests/test_rotation.py

@@ -18,7 +18,7 @@ from reportlab.pdfgen.canvas import Canvas
 
 from ocrmypdf._exec import ghostscript
 from ocrmypdf._plugin_manager import get_plugin_manager
-from ocrmypdf.helpers import Resolution
+from ocrmypdf.helpers import IMG2PDF_KWARGS, Resolution
 from ocrmypdf.pdfinfo import PdfInfo
 
 from .conftest import check_ocrmypdf, run_ocrmypdf
@@ -230,6 +230,7 @@ def test_rotate_page_level(image_angle, page_angle, resources, outdir):
             memimg.read(),
             layout_fun=img2pdf.get_fixed_dpi_layout_fun((200, 200)),
             outputstream=mempdf,
+            **IMG2PDF_KWARGS,
         )
         mempdf.seek(0)
         pike = pikepdf.open(mempdf)

tests/test_unpaper.py

@@ -5,19 +5,24 @@
 # file, You can obtain one at http://mozilla.org/MPL/2.0/.
 
 
+import logging
 from os import fspath
 from unittest.mock import patch
 
 import pytest
+from PIL import Image
 
+from ocrmypdf._exec import unpaper
 from ocrmypdf._plugin_manager import get_parser_options_plugins
 from ocrmypdf._validation import check_options
 from ocrmypdf.exceptions import ExitCode, MissingDependencyError
 
-from .conftest import check_ocrmypdf, have_unpaper, run_ocrmypdf
+from .conftest import check_ocrmypdf, have_unpaper, ocrmypdf_exec, run_ocrmypdf
 
 # pylint: disable=redefined-outer-name
 
+needs_unpaper = pytest.mark.skipif(not have_unpaper(), reason="requires unpaper")
+
 
 def test_no_unpaper(resources, no_outpdf):
     input_ = fspath(resources / "c02-22.pdf")
@@ -45,7 +50,7 @@ def test_old_unpaper(resources, no_outpdf):
         mock.assert_called()
 
 
-@pytest.mark.skipif(not have_unpaper(), reason="requires unpaper")
+@needs_unpaper
 def test_clean(resources, outpdf):
     check_ocrmypdf(
         resources / "skew.pdf",
@@ -56,7 +61,7 @@ def test_clean(resources, outpdf):
     )
 
 
-@pytest.mark.skipif(not have_unpaper(), reason="requires unpaper")
+@needs_unpaper
 def test_unpaper_args_valid(resources, outpdf):
     check_ocrmypdf(
         resources / "skew.pdf",
@@ -69,7 +74,7 @@ def test_unpaper_args_valid(resources, outpdf):
     )
 
 
-@pytest.mark.skipif(not have_unpaper(), reason="requires unpaper")
+@needs_unpaper
 def test_unpaper_args_invalid_filename(resources, outpdf):
     p = run_ocrmypdf(
         resources / "skew.pdf",
@@ -84,7 +89,7 @@ def test_unpaper_args_invalid_filename(resources, outpdf):
     assert p.returncode == ExitCode.bad_args
 
 
-@pytest.mark.skipif(not have_unpaper(), reason="requires unpaper")
+@needs_unpaper
 def test_unpaper_args_invalid(resources, outpdf):
     p = run_ocrmypdf(
         resources / "skew.pdf",
@@ -98,3 +103,16 @@ def test_unpaper_args_invalid(resources, outpdf):
     # Can't tell difference between unpaper choking on bad arguments or some
     # other unpaper failure
     assert p.returncode == ExitCode.child_process_error
+
+
+@needs_unpaper
+def test_unpaper_image_too_big(resources, outdir, caplog):
+    with patch('ocrmypdf._exec.unpaper.UNPAPER_IMAGE_PIXEL_LIMIT', 42):
+        infile = resources / 'crom.png'
+        unpaper.clean(infile, outdir / 'out.png', dpi=300) == infile
+
+        assert any(
+            'too large for cleaning' in rec.message
+            for rec in caplog.get_records('call')
+            if rec.levelno == logging.WARNING
+        )