Skip to content

PowerPoint2007 Reader : every image is decoded into a GD bitmap and re-encoded on the way out #985

Description

@dkulyk

Reader/PowerPoint2007::loadShapeDrawing() reads the media entry, calls imagecreatefromstring()
and hands the shape a Gd adapter. Gd::getContents() renders that bitmap back through
imagejpeg/imagepng when the deck is saved. A read-modify-write round trip is therefore
decode -> raw bitmap -> re-encode, for every picture, every time -- even when nothing touched it.

Measured on one 4000x3000 JPEG (media entry 3 622 271 bytes), against the same deck read with a
Drawing\File whose path is zip://<archive>#ppt/media/image1.jpeg:

today (Gd) zip:// (File)
RSS after load() +49.2 MB +0.4 MB
RSS after save() +52.7 MB +2.8 MB
media written 2.90 MB 3.45 MB
byte-identical 0 of 1 1 of 1

Three separate harms:

  • Memory. The bitmap is ~48 MB per twelve-megapixel image, and every image on the deck is held
    at once.
  • Fidelity. The JPEG is lossily re-compressed on every pass. A round trip of the repository's
    own Sample_12.pptx returns 0 of 4 media entries byte-identical.
  • memory_limit cannot see it. GD allocates outside PHP's allocator:
    memory_get_peak_usage() reported 14.9 MB where RSS grew by 49 MB. The process is killed by the
    OS rather than by PHP, which is why this reads as "sometimes dies on big files" instead of as an
    exhausted memory limit.

The machinery is already there

PhpOffice\Common\File parses zip://<archive>#<entry> by hand (ZipArchive::getFromName),
Drawing\File::getContents() is a one-line call into it, and PptMedia writes whatever
getContents() returns. Verified: a Drawing\File pointed at a zip:// path passes through the
Writer byte for byte with no new code. The Reader would hand back File instead of Gd; sizes come
from a:xfrm, not from the pixels, so nothing needs decoding.

Three things to settle before it is written, which is why this is an issue and not a pull request:

  1. Common\File reopens the archive on every read -- 40x slower than one open across 200 entries,
    and O(n^2) overall. It wants a cache.
  2. The source archive has to outlive the save. A stream, or a swept temp file, with the current
    in-memory path as the fallback.
  3. Returning File where callers expect Gd is a BC break. A Reader flag in 1.x and the default in
    2.0 is one way; another is for the adapter to decode lazily, so the type stays Gd and only the
    untouched images skip the round trip. Which of the two is wanted is the maintainer's call.

Related: #849 went after the same win, but
bought it with a temp-file copy per medium. This needs no copy at all.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions