diff --git a/CHANGELOG.md b/CHANGELOG.md index 959006d..e09b44e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -60,10 +60,17 @@ listed under **Changed** and are the reason this is a minor bump rather than a p `IMGFORGE_RETURN_ATTACHMENT`, and `IMGFORGE_QUALITY` set the starting value for the matching option, which a URL still overrides. +- **Smart gravity.** `gravity:sm` hands the window choice to libvips' `smartcrop`, which scores the image for the + region a viewer's eye would settle on. It is the answer when no fixed anchor is right for every image in a + catalogue, and it applies to `crop` and to the implicit crop a `fill` resize performs. imgproxy gates this one + behind its Pro tier. + - **New processing options.** `extend_aspect_ratio` (`exar`) pads to the requested shape without reaching its size. - `keep_copyright` (`kcr`) carries the EXIF copyright across a metadata strip. `preserve_hdr` (`ph`) keeps a high - bit-depth image high bit-depth and retains its gain map. `enforce_thumbnail` (`eth`) uses the source's embedded - EXIF thumbnail. `skip_processing` (`skp`) returns the source untouched for the formats it lists. + `keep_copyright` (`kcr`) carries the EXIF copyright across a metadata strip for JPEG, PNG and WebP output — an + APP1 segment, an `eXIf` chunk, and an `EXIF` chunk respectively, synthesising the extended header WebP needs. + TIFF, AVIF and HEIF can hold EXIF too but are not implemented; they strip as normal. `preserve_hdr` (`ph`) keeps a high bit-depth image high bit-depth and retains its gain map. + `enforce_thumbnail` (`eth`) uses the source's embedded EXIF thumbnail. `skip_processing` (`skp`) returns the + source untouched for the formats it lists. - **Colour management.** The image is converted into a colourspace the pipeline's operations are written for before processing and back for the encoder, through the source's embedded ICC profile when it has one. A CMYK source no @@ -114,6 +121,9 @@ listed under **Changed** and are the reason this is a minor bump rather than a p - **A resizing type with no dimensions leaves the image alone** instead of failing. `resize:fill` on its own names no target; imgproxy returns the image unresized, and now so does imgforge. +- **An unrecognised `webp_options` preset is refused** rather than accepted and then dropped on the way to the + encoder, where a typo silently produced a different image. + - Booleans accept imgproxy's spellings (`1`, `t`, `T`, `true`, `TRUE`, `True`) rather than only `1` and `true`. - `min_width` and `min_height` are accepted alongside imgforge's hyphenated `min-width` and `min-height`. - `padding` accepts three values, following the CSS shorthand, rather than rejecting them. diff --git a/ROADMAP.md b/ROADMAP.md index adb310e..b1829f9 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -10,31 +10,38 @@ Measurements below were taken on libvips 8.16.1 (the version the published image imgproxy gates a number of options behind its Pro tier that imgforge implements for free: `resizing_algorithm`, `background_alpha`, `watermark_url`, all four `*_options` encoder groups, the `adjust`/`brightness`/`contrast`/ -`saturation` family, and `page`/`pages`/`disable_animation`. +`saturation` family, `page`/`pages`/`disable_animation`, and smart gravity (`gravity:sm`). -0.18.0 closed the free-tier gap that remained. Every option in imgproxy's free tier is now implemented rather than -merely parsed, with the exceptions listed under **Known gaps** below. +0.18.0 closed the free-tier gap that remained. Every option in imgproxy's free tier is implemented rather than +merely parsed, and every option imgforge parses is applied. ## Known gaps in what imgforge accepts -- **`keep_copyright` on non-JPEG output** — libvips' `keep` flags have no copyright granularity, so imgforge reads - the EXIF `Copyright` and `Artist` fields from the source and splices a minimal EXIF segment into the encoded - result. That mechanism only exists for JPEG. PNG and WebP can carry EXIF too, and the same approach would work - for them; nobody has needed it yet. -- **`preserve_hdr` on libvips below 8.16** — the `gainmap` keep flag does not exist there. imgforge checks the - runtime version once and drops the flag rather than naming it, so the request succeeds and loses only the gain - map; the high bit-depth half still works. The drop is logged. The published image ships 8.16.1 and is - unaffected. -- **`webp_options` preset** — only libvips' own preset names reach the encoder. Others are ignored rather than - failing, because an unknown name makes libvips reject the whole encode. +Every option imgforge parses is applied, with three conditionals — each of which degrades rather than failing the +request, and each logged when it takes effect: + +- **`preserve_hdr` on libvips below 8.16.** The `gainmap` keep flag does not exist there, so imgforge checks the + runtime version once and drops the flag rather than failing the encode. The request loses the gain map and keeps + the high bit depth. The published image ships 8.16.1 and is unaffected. +- **`keep_copyright` on TIFF, AVIF or HEIF output.** All three containers can hold EXIF, but imgforge only writes + it into JPEG, PNG and WebP. The others strip as normal and the option is a no-op for them. +- **`trim` on an animated source.** Trim measures one image's borders and each frame has its own, which no animated + container can represent — the frames share a canvas. The option is dropped for animations, as it is in imgproxy. ## imgproxy Pro options imgforge does not implement Listed so the comparison is honest rather than because they are planned: `autoquality`, `crop_aspect_ratio`, `objects_position` and the object-detection family, `monochrome`, `duotone`, `colorize`, `gradient`, `unsharp_masking`, `blur_areas`, `style`, `dpi`, `color_profile`, `hashsum`, `watermark_text`/`_size`/`_rotate`/ -`_shadow`, `fallback_image_url`, and the `video_thumbnail_*` family. Smart gravity (`gravity:sm`) is Pro as well; -libvips does expose `smartcrop`, so it is the one entry here that would be cheap. +`_shadow`, `fallback_image_url`, and the `video_thumbnail_*` family. + +Most of these need something libvips does not provide on its own — an object detector, a quality search loop, a CSS +parser. The ones that do not are `crop_aspect_ratio` (pure arithmetic on the crop extents), `monochrome`, `duotone`, +and `colorize` (each a recombination matrix or a blend), and `watermark_size`/`watermark_rotate`. None is hard; +none has been asked for either, which is why they sit here rather than in the code. + +Smart gravity used to be on this list with a note that it would be cheap. It was, and it shipped: `gravity:sm` +hands the window choice to libvips' `smartcrop`, which is the one thing a geometric anchor cannot do. ## Performance @@ -103,6 +110,9 @@ and they are the only form that can express a *combination* of metadata `keep` f in the repository and generating a correct one by hand is not obviously cheaper than checking in a file. - **`enforce_thumbnail`** is covered for the "no thumbnail" and "malformed thumbnail" paths but not for a JPEG that actually carries one, for the same reason. +- **`keep_copyright`** round-trips through a real encode of each container imgforge writes EXIF into — JPEG, PNG + and WebP, including a WebP that already has an extended header. Not covered: a container whose existing EXIF + block is malformed. TIFF, AVIF and HEIF can carry EXIF but have no writer, so there is nothing to cover yet. ## How to extend this list diff --git a/doc/5_processing_options.md b/doc/5_processing_options.md index a45f7d7..0faf750 100644 --- a/doc/5_processing_options.md +++ b/doc/5_processing_options.md @@ -15,7 +15,7 @@ Unrecognised directive *names* are ignored rather than rejected, so a typo silen | `resizing_algorithm` | `ra` | `algorithm` | Interpolation kernel for resize operations. Defaults to `lanczos3`. | | `width` | `w` | `value` | Sets a target width (infers height). Implies `fit`. | | `height` | `h` | `value` | Sets a target height (infers width). Implies `fit`. | -| `gravity` | `g` | `type[:x_offset[:y_offset]]` | Controls crop/fill anchoring (`ce`, `noea`, `fp`, ...). Defaults to `ce:0:0`. | +| `gravity` | `g` | `type[:x_offset[:y_offset]]` | Controls crop/fill anchoring (`ce`, `noea`, `fp`, `sm`, ...). Defaults to `ce:0:0`. | | `flip` | `fl` | `horizontal[:vertical]` | Flips the image horizontally and/or vertically. Defaults to no flip. | | `enlarge` | `el` | `bool` | Allows upscaling globally. Defaults to `false`. | | `extend` | `ex` | `bool[:gravity]` | Pads to target dimensions after resize. Defaults to `false:ce:0:0`. | @@ -43,7 +43,7 @@ Unrecognised directive *names* are ignored rather than rejected, so a typo silen | `max_bytes` | `mb` | `bytes` | Re-encodes lossy formats at lower quality until the byte target is reached or quality reaches `1`. | | `strip_metadata` | `sm` | `bool` | Drops encoder metadata when supported by the output format. | | `strip_color_profile` | `scp` | `bool` | Drops the embedded colour profile, leaving other metadata alone. | -| `keep_copyright` | `kcr` | `bool` | Retains the EXIF copyright and artist tags across a metadata strip. JPEG output only. | +| `keep_copyright` | `kcr` | `bool` | Retains the EXIF copyright and artist tags across a metadata strip. JPEG, PNG, and WebP output. | | `preserve_hdr` | `ph` | `bool` | Keeps a high bit-depth image high bit-depth and carries its gain map through. Gain maps need libvips 8.16+; older builds keep the depth and drop the map. | | `enforce_thumbnail` | `eth` | `bool` | Uses the source's embedded EXIF thumbnail instead of the full image when one is present. | | `jpeg_options` | `jpgo` | `progressive:no_subsample:trellis:dering:scans:quant_table` | Advanced JPEG encoder switches. | @@ -151,12 +151,24 @@ Pick on appearance rather than speed: the kernel is rarely where the processing - Canvas alignment for `extend` and `extend_aspect_ratio`, each of which can also carry its own. - Watermark positioning, via the `watermark` option's own position argument. -imgforge accepts imgproxy's gravity anchors: `ce`, `no`, `so`, `ea`, `we`, `noea`, `nowe`, `soea`, `sowe`, and `fp`. +imgforge accepts imgproxy's gravity anchors: `ce`, `no`, `so`, `ea`, `we`, `noea`, `nowe`, `soea`, `sowe`, `fp`, and `sm`. The last two are scoped, exactly as in imgproxy, because they do not mean anything everywhere: + +| Where | `fp` | `sm` | +| ----- | ---- | ---- | +| `crop`, `fill`, `fill-down`, `gravity` | yes | yes | +| `extend`, `extend_aspect_ratio` | yes | **no** — nothing to examine | +| `watermark` position | **no** | **no** — a watermark is placed, not found | + +A gravity used where it does not apply is rejected with `400 Bad Request` rather than quietly falling back to `ce`. **Offsets** nudge the window away from its anchor. A magnitude of 1 or more is a pixel count; anything smaller is a fraction of the axis being positioned. `gravity:no:0:20` takes the window from 20px below the top edge; `gravity:no:0:0.1` takes it from a tenth of the way down. The window is still clamped to the image, so an offset cannot push it off the edge. **Focus point** — `gravity:fp:x:y` — reads the two arguments as coordinates between 0 and 1 and centres the result on that point. `gravity:fp:0.5:0.25` keeps the middle of the upper quarter in view, which is the usual answer for portraits where a centre crop cuts off the head. +**Smart** — `gravity:sm` — hands the choice to libvips, which scores the image for the region a viewer's eye would settle on and puts the window there. It is the answer when no fixed anchor is right for every image: a catalogue of mixed portraits and landscapes has no single correct crop, and a focus point has to be supplied per image. imgproxy charges for this one. + +Two costs. It has to examine real pixels, so it forces the decode rather than composing into libvips' lazy pipeline — though in a `fill` it runs *after* the resize, so it examines the small image rather than the source. And it takes no offsets: the window is chosen, not positioned. It applies only where there is content to choose from, which is why the table above rules it out for watermark placement and for the two extends: both position the image on a canvas larger than itself rather than selecting a window inside it. + ### Minimum dimensions & zoom - `min-width` and `min-height` trigger an extra resize pass when the image is still smaller after primary resizing. This pass **upscales regardless of `enlarge`** — the minimums are a floor, and `enlarge:false` does not override them. Use them only when you actually want a guaranteed size. @@ -235,7 +247,7 @@ Defaults to `85` for lossy codecs (JPEG, WebP, AVIF). `quality` is ignored for l - `strip_metadata` and `strip_color_profile` map to libvips metadata retention controls for formats that expose them. - `jpeg_options` maps to progressive JPEG, chroma subsampling, trellis quantization, overshoot deringing, optimized scans, and quant table controls. - `png_options` maps to interlacing and palette quantization controls. -- `webp_options` maps to lossless, smart chroma subsampling, and encoder preset controls. Preset names outside libvips' own set (`default`, `picture`, `photo`, `drawing`, `icon`, `text`) are accepted in URLs but ignored by the encoder. +- `webp_options` maps to lossless, smart chroma subsampling, and encoder preset controls. The preset must be one of libvips' own — `default`, `picture`, `photo`, `drawing`, `icon`, `text` — and anything else is refused with `400`. It used to be accepted and then dropped on the way to the encoder, so a typo silently produced a different image. - `avif_options` maps to AVIF/HEIF chroma subsampling. Every format is encoded through the libvips save suffix (`.webp[Q=80,keep=all]` and friends) rather than the crate's generated save bindings. Those bindings name encoder properties that only exist in libvips 8.16 and later — `exact` on webpsave, `tune` on heifsave, `keep-duplicate-frames` on gifsave — and an older libvips rejects the whole call with `no property named ...`, so nothing encodes at all. The suffix parser sets only the options named, which keeps one code path working across libvips versions, and it is also the only form that can express a *combination* of metadata `keep` flags. @@ -249,7 +261,7 @@ A result too large for its output container is scaled down to fit rather than ha ### Metadata - **`strip_metadata`** drops the descriptive tags (EXIF, XMP, IPTC) and leaves the colour profile alone. **`strip_color_profile`** does the reverse. Set both to drop everything. -- **`keep_copyright`** carries the EXIF `Copyright` and `Artist` tags across a `strip_metadata`. libvips has no copyright granularity in its `keep` flags — they are `none|exif|xmp|iptc|icc|other|gainmap|all` — so imgforge reads the two fields from the source and splices a minimal EXIF segment back into the encoded result. That mechanism is JPEG-only; other output formats strip as normal, and the option is a no-op for them. +- **`keep_copyright`** carries the EXIF `Copyright` and `Artist` tags across a `strip_metadata`. libvips has no copyright granularity in its `keep` flags — they are `none|exif|xmp|iptc|icc|other|gainmap|all` — so imgforge reads the two fields from the source and writes a minimal EXIF block back into the encoded result: an APP1 segment for JPEG, an `eXIf` chunk for PNG, and an `EXIF` chunk for WebP, synthesising the extended header WebP needs to carry one. Every other output format strips as normal and the option is a no-op for it — including TIFF, AVIF and HEIF, which *can* hold EXIF but have no writer here yet. - **`preserve_hdr`** keeps a high bit-depth source at its own depth when the output format can carry it (PNG, TIFF, AVIF, HEIF) and retains the gain map that makes the image HDR, even while other metadata is being stripped. The gain-map half needs libvips 8.16 or later, where the `gainmap` keep flag was added. On an older build imgforge detects the runtime version and drops that flag rather than failing: the request succeeds, keeps its bit depth, and loses only the gain map. A successful response on such a build is therefore not proof that the gain map survived — the drop is logged when it happens. - **`enforce_thumbnail`** uses the source's embedded EXIF thumbnail in place of the full image whenever one is present, which turns a large JPEG into a very cheap request. The thumbnail is usually a few hundred pixels wide, so the result is only as good as that; a thumbnail that will not decode falls back to the full image rather than failing. diff --git a/src/processing/metadata.rs b/src/processing/metadata.rs index 0528d47..74caf35 100644 --- a/src/processing/metadata.rs +++ b/src/processing/metadata.rs @@ -18,6 +18,9 @@ const MARKER_SOS: u8 = 0xDA; /// The identifier that opens the Exif payload of an APP1 segment. const EXIF_IDENTIFIER: &[u8] = b"Exif\0\0"; +/// The eight bytes every PNG opens with. +const PNG_SIGNATURE: &[u8] = b"\x89PNG\r\n\x1a\n"; + /// EXIF tag numbers, as they appear in an IFD entry. const TAG_COPYRIGHT: u16 = 0x8298; const TAG_ARTIST: u16 = 0x013B; @@ -105,9 +108,13 @@ pub fn read_copyright(image_bytes: &[u8]) -> Copyright { } } -/// Builds a minimal little-endian Exif APP1 payload carrying only the fields in +/// Builds a minimal little-endian TIFF/Exif block carrying only the fields in /// `copyright`. -fn build_exif_payload(copyright: &Copyright) -> Option> { +/// +/// This is the payload every container wants: JPEG wraps it behind the `Exif\0\0` +/// identifier in an APP1 segment, PNG stores it bare in an `eXIf` chunk, and +/// WebP stores it bare in an `EXIF` chunk. +fn build_exif_tiff(copyright: &Copyright) -> Option> { let mut entries: Vec<(u16, Vec)> = Vec::new(); if let Some(value) = copyright.copyright.as_deref() { entries.push((TAG_COPYRIGHT, nul_terminated(value))); @@ -150,15 +157,14 @@ fn build_exif_payload(copyright: &Copyright) -> Option> { // No IFD1: the thumbnail, if there was one, went with the strip. directory.extend_from_slice(&0u32.to_le_bytes()); - let mut payload = Vec::from(EXIF_IDENTIFIER); - payload.extend_from_slice(b"II"); - payload.extend_from_slice(&42u16.to_le_bytes()); - payload.extend_from_slice(&8u32.to_le_bytes()); - payload.extend_from_slice(&directory); - payload.extend_from_slice(&values); + let mut tiff = Vec::with_capacity(8 + directory.len() + values.len()); + tiff.extend_from_slice(b"II"); + tiff.extend_from_slice(&42u16.to_le_bytes()); + tiff.extend_from_slice(&8u32.to_le_bytes()); + tiff.extend_from_slice(&directory); + tiff.extend_from_slice(&values); - // An APP1 segment carries its own length in two bytes, including those two. - (payload.len() + 2 <= usize::from(u16::MAX)).then_some(payload) + Some(tiff) } fn nul_terminated(value: &str) -> Vec { @@ -167,32 +173,264 @@ fn nul_terminated(value: &str) -> Vec { bytes } -/// Re-attaches a copyright statement to encoded JPEG bytes. +/// Re-attaches a copyright statement to an encoded image. /// -/// Only JPEG: it is the format that carries EXIF natively and the one that -/// nearly every copyright-bearing source uses. Returns the input untouched when -/// there is nothing to attach or the output is not a JPEG, so callers can apply -/// it unconditionally. +/// libvips' `keep` flags are `none|exif|xmp|iptc|icc|other|gainmap|all`, with no +/// copyright granularity, so retaining one field across a metadata strip means +/// putting it back afterwards. Each container stores the same TIFF/Exif block +/// differently, so there is one writer per container and an untouched return +/// for anything else — callers can apply this unconditionally. pub fn attach_copyright(encoded: Vec, copyright: &Copyright) -> Vec { if copyright.is_empty() { return encoded; } - if encoded.len() < 2 || encoded[0] != MARKER_PREFIX || encoded[1] != MARKER_SOI { - debug!("Copyright retention skipped: output is not a JPEG"); - return encoded; - } - let Some(payload) = build_exif_payload(copyright) else { + let Some(tiff) = build_exif_tiff(copyright) else { return encoded; }; - let mut out = Vec::with_capacity(encoded.len() + payload.len() + 4); + let attached = if is_jpeg(&encoded) { + attach_to_jpeg(&encoded, &tiff) + } else if is_png(&encoded) { + attach_to_png(&encoded, &tiff) + } else if is_webp(&encoded) { + attach_to_webp(&encoded, &tiff) + } else { + // Not "cannot carry EXIF" — TIFF, AVIF and HEIF all can. imgforge just + // has no writer for them, and saying otherwise sent anyone reading the + // log looking for a container limitation that does not exist. + debug!("Copyright retention skipped: imgforge does not write EXIF into this output container"); + None + }; + + attached.unwrap_or(encoded) +} + +fn is_jpeg(bytes: &[u8]) -> bool { + bytes.len() >= 2 && bytes[0] == MARKER_PREFIX && bytes[1] == MARKER_SOI +} + +fn is_png(bytes: &[u8]) -> bool { + bytes.starts_with(PNG_SIGNATURE) +} + +fn is_webp(bytes: &[u8]) -> bool { + bytes.len() >= 12 && bytes.starts_with(b"RIFF") && &bytes[8..12] == b"WEBP" +} + +/// JPEG: an APP1 segment holding `Exif\0\0` and then the TIFF block, spliced in +/// directly after the start-of-image marker. +fn attach_to_jpeg(encoded: &[u8], tiff: &[u8]) -> Option> { + let payload_len = EXIF_IDENTIFIER.len() + tiff.len(); + // A segment carries its own length in two bytes, including those two. + if payload_len + 2 > usize::from(u16::MAX) { + return None; + } + + let mut out = Vec::with_capacity(encoded.len() + payload_len + 4); out.extend_from_slice(&encoded[..2]); out.push(MARKER_PREFIX); out.push(MARKER_APP1); - out.extend_from_slice(&((payload.len() + 2) as u16).to_be_bytes()); - out.extend_from_slice(&payload); + out.extend_from_slice(&((payload_len + 2) as u16).to_be_bytes()); + out.extend_from_slice(EXIF_IDENTIFIER); + out.extend_from_slice(tiff); out.extend_from_slice(&encoded[2..]); - out + Some(out) +} + +/// PNG: an `eXIf` chunk holding the TIFF block bare, placed before the first +/// `IDAT` as the specification requires. +/// +/// Any `eXIf` chunk already present is dropped rather than left in place. A +/// reader takes the first one it finds, so appending a second would leave the +/// copyright unreadable behind whatever the encoder had already written. +fn attach_to_png(encoded: &[u8], tiff: &[u8]) -> Option> { + let chunk = png_chunk(b"eXIf", tiff)?; + + let mut out = Vec::with_capacity(encoded.len() + chunk.len()); + out.extend_from_slice(PNG_SIGNATURE); + + let mut inserted = false; + for (chunk_type, span) in png_chunks(encoded)? { + if &chunk_type == b"eXIf" { + continue; + } + // The specification wants metadata before the image data. + if !inserted && &chunk_type == b"IDAT" { + out.extend_from_slice(&chunk); + inserted = true; + } + out.extend_from_slice(&encoded[span]); + } + + inserted.then_some(out) +} + +/// Walks a PNG's chunk list, yielding each chunk's type and its byte range. +fn png_chunks(encoded: &[u8]) -> Option)>> { + let mut chunks = Vec::new(); + let mut offset = PNG_SIGNATURE.len(); + + while offset + 8 <= encoded.len() { + let length = u32::from_be_bytes(encoded[offset..offset + 4].try_into().ok()?) as usize; + let chunk_type: [u8; 4] = encoded[offset + 4..offset + 8].try_into().ok()?; + // The length covers the data alone; the chunk also carries a 4-byte + // length, a 4-byte type, and a 4-byte CRC. + let end = offset.checked_add(length)?.checked_add(12)?; + if end > encoded.len() { + return None; + } + + chunks.push((chunk_type, offset..end)); + offset = end; + } + + Some(chunks) +} + +fn png_chunk(chunk_type: &[u8; 4], data: &[u8]) -> Option> { + let length = u32::try_from(data.len()).ok()?; + + let mut chunk = Vec::with_capacity(data.len() + 12); + chunk.extend_from_slice(&length.to_be_bytes()); + chunk.extend_from_slice(chunk_type); + chunk.extend_from_slice(data); + // The CRC covers the type and the data, but not the length. + chunk.extend_from_slice(&crc32(&chunk[4..]).to_be_bytes()); + Some(chunk) +} + +/// CRC-32/ISO-HDLC, which is what PNG chunks carry. +/// +/// Computed bitwise rather than through a table: a copyright string is a few +/// dozen bytes, so the table would cost more to build than it saves. +fn crc32(data: &[u8]) -> u32 { + let mut crc = 0xFFFF_FFFFu32; + for byte in data { + crc ^= u32::from(*byte); + for _ in 0..8 { + let mask = (crc & 1).wrapping_neg(); + crc = (crc >> 1) ^ (0xEDB8_8320 & mask); + } + } + !crc +} + +/// WebP: an `EXIF` chunk holding the TIFF block bare, appended to the RIFF +/// container. +/// +/// The container specification only permits metadata chunks in the *extended* +/// format, which is announced by a `VP8X` chunk carrying a flags byte. A simple +/// lossy or lossless WebP has no such chunk, so one is synthesised from the +/// canvas size; a file that already has one — every animation does — just has +/// its EXIF flag set. As with PNG, an existing `EXIF` chunk is replaced rather +/// than duplicated. +fn attach_to_webp(encoded: &[u8], tiff: &[u8]) -> Option> { + let mut body: Vec = Vec::with_capacity(encoded.len() + tiff.len() + 16); + let mut has_extended_header = false; + + for (fourcc, span) in webp_chunks(encoded)? { + if &fourcc == b"EXIF" { + continue; + } + let start = body.len(); + body.extend_from_slice(&encoded[span]); + if &fourcc == b"VP8X" { + // Bit 3 of the flags byte, which is the first byte of the payload, + // marks the presence of an EXIF chunk. + *body.get_mut(start + 8)? |= 0b0000_1000; + has_extended_header = true; + } + } + + if !has_extended_header { + let (width, height, has_alpha) = webp_canvas(&body)?; + if width > 1 << 24 || height > 1 << 24 { + return None; + } + + let mut header = Vec::with_capacity(body.len() + 18); + header.extend_from_slice(b"VP8X"); + header.extend_from_slice(&10u32.to_le_bytes()); + // Bit 3 marks EXIF; bit 4 marks alpha. Declaring EXIF while omitting + // the alpha a VP8L bitstream actually carries leaves the container's + // feature flags contradicting its contents, which a strict reader may + // refuse or read as fully opaque. + header.push(0b0000_1000 | if has_alpha { 0b0001_0000 } else { 0 }); + header.extend_from_slice(&[0, 0, 0]); + // The canvas dimensions are stored as 24-bit values, one less than the + // real size. + header.extend_from_slice(&(width - 1).to_le_bytes()[..3]); + header.extend_from_slice(&(height - 1).to_le_bytes()[..3]); + header.extend_from_slice(&body); + body = header; + } + + body.extend_from_slice(b"EXIF"); + body.extend_from_slice(&u32::try_from(tiff.len()).ok()?.to_le_bytes()); + body.extend_from_slice(tiff); + // Every RIFF chunk is padded to an even length. + if !body.len().is_multiple_of(2) { + body.push(0); + } + + let mut out = Vec::with_capacity(body.len() + 12); + out.extend_from_slice(b"RIFF"); + // The RIFF size counts everything after itself, which includes "WEBP". + out.extend_from_slice(&u32::try_from(body.len() + 4).ok()?.to_le_bytes()); + out.extend_from_slice(b"WEBP"); + out.extend_from_slice(&body); + Some(out) +} + +/// Walks a WebP's chunk list, yielding each chunk's fourCC and its byte range +/// including the header and any padding byte. +fn webp_chunks(encoded: &[u8]) -> Option)>> { + let mut chunks = Vec::new(); + let mut offset = 12; + + while offset + 8 <= encoded.len() { + let fourcc: [u8; 4] = encoded[offset..offset + 4].try_into().ok()?; + let size = u32::from_le_bytes(encoded[offset + 4..offset + 8].try_into().ok()?) as usize; + let padded = size + usize::from(!size.is_multiple_of(2)); + let end = offset.checked_add(8)?.checked_add(padded)?; + if end > encoded.len() { + return None; + } + + chunks.push((fourcc, offset..end)); + offset = end; + } + + Some(chunks) +} + +/// Reads the canvas size and alpha flag out of a simple WebP's bitstream. +/// +/// Only needed for a file with no `VP8X` chunk, which by definition is a single +/// lossy (`VP8 `) or lossless (`VP8L`) frame. +fn webp_canvas(body: &[u8]) -> Option<(u32, u32, bool)> { + let fourcc = body.get(..4)?; + let payload = body.get(8..)?; + + if fourcc == b"VP8 " { + // A key frame: a 3-byte tag, the 3-byte start code, then 14-bit + // dimensions each followed by a 2-bit scale. Lossy WebP without a VP8X + // header has no alpha channel. + let header = payload.get(6..10)?; + let width = u32::from(u16::from_le_bytes([header[0], header[1]]) & 0x3FFF); + let height = u32::from(u16::from_le_bytes([header[2], header[3]]) & 0x3FFF); + (width > 0 && height > 0).then_some((width, height, false)) + } else if fourcc == b"VP8L" { + // A 1-byte signature, then 14 bits of width-1, 14 bits of height-1, and + // a single alpha-is-used bit, packed little-endian. + let bits = u32::from_le_bytes(payload.get(1..5)?.try_into().ok()?); + let width = (bits & 0x3FFF) + 1; + let height = ((bits >> 14) & 0x3FFF) + 1; + let has_alpha = (bits >> 28) & 1 == 1; + Some((width, height, has_alpha)) + } else { + None + } } /// Extracts the JPEG thumbnail embedded in a source's EXIF data. @@ -226,23 +464,79 @@ pub fn embedded_thumbnail(image_bytes: &[u8]) -> Option> { #[cfg(test)] mod tests { use super::*; + use crate::processing::save; + use crate::test_support::init_vips; + use libvips::{ops, VipsImage}; - #[test] - fn copyright_round_trips_through_a_rebuilt_app1_segment() { - // A JPEG stripped of metadata, then given its copyright back, has to be - // readable by an ordinary EXIF parser again — otherwise `keep_copyright` - // silently produces a file whose copyright no tool can find. - let mut jpeg = vec![0xFF, 0xD8]; - jpeg.extend_from_slice(&[0xFF, 0xDA, 0x00, 0x02]); - - let copyright = Copyright { + fn copyright() -> Copyright { + Copyright { copyright: Some("(c) 2026 Example".to_string()), artist: Some("A Photographer".to_string()), - }; + } + } - let tagged = attach_copyright(jpeg.clone(), ©right); - assert_ne!(tagged, jpeg, "the segment should have been spliced in"); - assert_eq!(read_copyright(&tagged), copyright); + /// A real encode of the given format, so the container being spliced is the + /// one imgforge actually produces rather than a hand-built approximation. + fn encoded(format: &str) -> Vec { + init_vips(); + let image = ops::black(17, 9).expect("probe image"); + // An odd width and height make the RIFF padding path matter. + save::save_image(image, format, 80).expect("format encodes") + } + + /// The whole point of `keep_copyright`: after a strip and a re-attach, an + /// ordinary EXIF parser has to find the fields again. A splice that any + /// tool cannot read is the same as having dropped them. + #[test] + fn copyright_round_trips_through_every_container_that_carries_exif() { + for format in ["jpeg", "png", "webp"] { + let bare = encoded(format); + assert!( + read_copyright(&bare).is_empty(), + "{format}: the probe should start with no copyright" + ); + + let tagged = attach_copyright(bare.clone(), ©right()); + assert_ne!(tagged, bare, "{format}: nothing was spliced in"); + assert_eq!(read_copyright(&tagged), copyright(), "{format}: not readable again"); + + // And the result must still decode as an image of the same size. + let decoded = VipsImage::new_from_buffer(&tagged, "").expect("tagged image still decodes"); + assert_eq!((decoded.get_width(), decoded.get_height()), (17, 9), "{format}"); + } + } + + /// An animated WebP already carries a VP8X chunk, so the writer sets its + /// EXIF flag rather than synthesising a second one. + #[test] + fn copyright_survives_on_a_webp_that_already_has_an_extended_header() { + init_vips(); + // A gradient rather than a flat colour: libwebp collapses frames that + // are byte-identical, so three black frames would encode as one and the + // test would prove nothing about whether the splice kept them. + let gradient = ops::xyz(16, 24).expect("probe image"); + let gradient = ops::cast(&gradient, ops::BandFormat::Uchar).expect("cast to 8 bit"); + let animated = save::save_image_with_options( + gradient, + "webp", + 80, + &crate::processing::options::SaveOptions::default(), + Some(8), + None, + ) + .expect("animated webp encodes"); + + let before = VipsImage::new_from_buffer(&animated, "n=-1").expect("animation decodes"); + assert_eq!(before.get_n_pages(), 3, "the probe should be a three-frame animation"); + + let tagged = attach_copyright(animated, ©right()); + assert_eq!(read_copyright(&tagged), copyright()); + + // The extended header was already there, so the writer sets its EXIF + // flag rather than synthesising a second one — which would leave the + // file with two VP8X chunks and no decoder willing to read it. + let decoded = VipsImage::new_from_buffer(&tagged, "n=-1").expect("tagged animation still decodes"); + assert_eq!(decoded.get_n_pages(), 3, "the frames must survive the splice"); } #[test] @@ -250,14 +544,10 @@ mod tests { let jpeg = vec![0xFF, 0xD8, 0xFF, 0xDA, 0x00, 0x02]; assert_eq!(attach_copyright(jpeg.clone(), &Copyright::default()), jpeg); - // A non-JPEG output cannot carry an APP1 segment, so it is returned - // untouched rather than corrupted with one. - let png = vec![0x89, b'P', b'N', b'G']; - let copyright = Copyright { - copyright: Some("(c) 2026".to_string()), - artist: None, - }; - assert_eq!(attach_copyright(png.clone(), ©right), png); + // A container with nowhere to put EXIF is returned untouched rather + // than corrupted with a chunk it cannot describe. + let tiff = b"II*\0rest of a tiff".to_vec(); + assert_eq!(attach_copyright(tiff.clone(), ©right()), tiff); } #[test] @@ -267,4 +557,12 @@ mod tests { assert_eq!(embedded_thumbnail(&jpeg), None); assert_eq!(embedded_thumbnail(b"not an image"), None); } + + #[test] + fn png_chunk_crcs_match_the_reference_algorithm() { + // IEND is the one PNG chunk whose bytes are fixed by the specification, + // so its CRC is a published constant to check the implementation against. + let chunk = png_chunk(b"IEND", &[]).expect("chunk builds"); + assert_eq!(chunk, vec![0, 0, 0, 0, b'I', b'E', b'N', b'D', 0xAE, 0x42, 0x60, 0x82]); + } } diff --git a/src/processing/mod.rs b/src/processing/mod.rs index 66782aa..8db578e 100644 --- a/src/processing/mod.rs +++ b/src/processing/mod.rs @@ -151,22 +151,26 @@ pub fn process_image( } } - let mut output_vec = save::save_image_with_options( + // The copyright block is spliced into the encoded bytes, so its size has to + // be part of the budget `max_bytes` searches against. Measuring the bare + // encode and then growing it meant a request could come back over the limit + // when a lower quality would have fitted. + let copyright = parsed_options + .save + .retains_copyright() + .then(|| metadata::read_copyright(source_bytes)) + .filter(|copyright| !copyright.is_empty()); + let attach = copyright.map(|copyright| move |encoded: Vec| metadata::attach_copyright(encoded, ©right)); + + let output_vec = save::save_image_with_options( img, &output_format, parsed_options.quality_for(&output_format), &parsed_options.save, page_height.filter(|_| save::format_supports_animation(&output_format)), + attach.as_ref().map(|attach| attach as save::Finalize<'_>), )?; - if parsed_options.save.retains_copyright() { - let copyright = metadata::read_copyright(source_bytes); - if !copyright.is_empty() { - debug!("Re-attaching copyright after metadata strip"); - output_vec = metadata::attach_copyright(output_vec, ©right); - } - } - let output_bytes = Bytes::from(output_vec); debug!("Image processing complete"); diff --git a/src/processing/options/effects.rs b/src/processing/options/effects.rs index e873e33..46ce7db 100644 --- a/src/processing/options/effects.rs +++ b/src/processing/options/effects.rs @@ -93,8 +93,10 @@ impl WatermarkPosition { pub fn parse(value: &str) -> Option { match value { "re" => Some(Self::Replicate), + // A watermark is placed, not discovered, so the two content-aware + // gravities have no meaning here. other => GravityType::parse(other) - .filter(|kind| *kind != GravityType::FocusPoint) + .filter(|kind| *kind != GravityType::FocusPoint && !kind.is_content_aware()) .map(Self::Anchor), } } diff --git a/src/processing/options/encoder.rs b/src/processing/options/encoder.rs index ed0d10a..41ef4fc 100644 --- a/src/processing/options/encoder.rs +++ b/src/processing/options/encoder.rs @@ -113,10 +113,29 @@ pub(super) fn parse_png_options(args: &[String], png: &mut PngOptions) -> Result Ok(()) } -pub(super) fn parse_webp_options(args: &[String], webp: &mut WebpOptions) { +/// The preset names libvips defines. imgproxy's `webp_options` preset takes the +/// same set, since both are naming libwebp's own presets. +pub const WEBP_PRESETS: [&str; 6] = ["default", "picture", "photo", "drawing", "icon", "text"]; + +pub(super) fn parse_webp_options(args: &[String], webp: &mut WebpOptions) -> Result<(), OptionParseError> { webp.lossless = parse_optional_bool(args, 0); webp.smart_subsample = parse_optional_bool(args, 1); + if let Some(value) = arg(args, 2) { - webp.preset = Some(value.to_lowercase()); + let preset = value.to_lowercase(); + // Previously an unrecognised name was dropped on the way to the + // encoder, because interpolating it into the option string would make + // libvips reject the whole encode. Silently ignoring it meant a typo + // produced a different image with no indication why, so it is now + // refused where every other bad argument is refused. + if !WEBP_PRESETS.contains(&preset.as_str()) { + return Err(OptionParseError::invalid(format!( + "webp_options preset must be one of: {}", + WEBP_PRESETS.join(", ") + ))); + } + webp.preset = Some(preset); } + + Ok(()) } diff --git a/src/processing/options/geometry.rs b/src/processing/options/geometry.rs index f23de6a..1f60a47 100644 --- a/src/processing/options/geometry.rs +++ b/src/processing/options/geometry.rs @@ -86,6 +86,9 @@ pub enum GravityType { SouthWest, /// The offsets name a point, in 0..1 of each axis, to centre the result on. FocusPoint, + /// libvips picks the window, by looking for the part of the image a viewer + /// would look at. + Smart, } impl GravityType { @@ -101,10 +104,17 @@ impl GravityType { "soea" => Some(Self::SouthEast), "sowe" => Some(Self::SouthWest), "fp" => Some(Self::FocusPoint), + "sm" => Some(Self::Smart), _ => None, } } + /// Whether the window position is chosen by looking at the pixels rather + /// than computed from the geometry. + pub const fn is_content_aware(self) -> bool { + matches!(self, Self::Smart) + } + pub const fn as_str(self) -> &'static str { match self { Self::Center => "ce", @@ -117,14 +127,16 @@ impl GravityType { Self::SouthEast => "soea", Self::SouthWest => "sowe", Self::FocusPoint => "fp", + Self::Smart => "sm", } } } /// An anchor plus its offsets. /// -/// For every anchor but [`GravityType::FocusPoint`] the offsets nudge the -/// window away from the anchor: an absolute pixel count when the magnitude is +/// [`GravityType::Smart`] ignores the offsets: the window is chosen from the +/// image's content. For every anchor but that and [`GravityType::FocusPoint`] +/// the offsets nudge the window away from the anchor: an absolute pixel count when the magnitude is /// at least 1, otherwise a fraction of the axis being positioned. Focus point /// instead reads them as the coordinates, in 0..1, that the result centres on. /// Both readings come from imgproxy, whose `calcPosition` this mirrors. @@ -155,7 +167,7 @@ impl Gravity { }; let kind = GravityType::parse(kind).ok_or_else(|| { OptionParseError::invalid(format!( - "{option} gravity must be one of: ce, no, so, ea, we, noea, nowe, soea, sowe, fp" + "{option} gravity must be one of: ce, no, so, ea, we, noea, nowe, soea, sowe, fp, sm" )) })?; @@ -226,10 +238,29 @@ pub struct Extend { impl Extend { /// Parses `enabled[:gravity_type[:x[:y]]]`. + /// + /// Smart gravity is refused. Extending *adds* canvas around the image + /// rather than choosing a window inside it, so there is nothing for + /// `smartcrop` to look at; the value would reach `calc_position`, fall + /// through to the centre branch, and quietly behave as `ce`. A URL that + /// appears to work and does something else is worse than one that is + /// rejected. imgproxy draws the line in the same place — its + /// `ExtendGravityTypes` omits `sm` while keeping `fp`, which does mean + /// something here: it positions the image against a point on the canvas. pub fn parse(args: &[String], option: &'static str) -> Result { let enabled = arg(args, 0).map(parse_boolean).unwrap_or(false); let gravity = match arg(args, 1) { - Some(_) => Some(Gravity::parse(args, 1, option)?), + Some(_) => { + let gravity = Gravity::parse(args, 1, option)?; + if gravity.kind.is_content_aware() { + return Err(OptionParseError::invalid(format!( + "{option} gravity cannot be {}: it positions the image on a larger canvas, \ + which has no content to choose from", + gravity.kind.as_str() + ))); + } + Some(gravity) + } None => None, }; Ok(Self { enabled, gravity }) diff --git a/src/processing/options/mod.rs b/src/processing/options/mod.rs index c49f35c..3e97f22 100644 --- a/src/processing/options/mod.rs +++ b/src/processing/options/mod.rs @@ -12,7 +12,7 @@ mod geometry; mod names; pub use effects::{Adjust, Watermark, WatermarkPosition, Zoom}; -pub use encoder::{AvifOptions, JpegOptions, PngOptions, SaveOptions, WebpOptions}; +pub use encoder::{AvifOptions, JpegOptions, PngOptions, SaveOptions, WebpOptions, WEBP_PRESETS}; pub use error::OptionParseError; pub use geometry::{Crop, Extend, Flip, Gravity, GravityType, Resize, ResizingType, Trim}; @@ -550,7 +550,7 @@ fn apply_option(option: &ProcessingOption, parsed: &mut ParsedOptions) -> Result } JPEG_OPTIONS | JPEG_OPTIONS_SHORT => encoder::parse_jpeg_options(args, &mut parsed.save.jpeg)?, PNG_OPTIONS | PNG_OPTIONS_SHORT => encoder::parse_png_options(args, &mut parsed.save.png)?, - WEBP_OPTIONS | WEBP_OPTIONS_SHORT => encoder::parse_webp_options(args, &mut parsed.save.webp), + WEBP_OPTIONS | WEBP_OPTIONS_SHORT => encoder::parse_webp_options(args, &mut parsed.save.webp)?, AVIF_OPTIONS | AVIF_OPTIONS_SHORT => { parsed.save.avif.no_subsample = error::parse_optional_bool(args, 0); } diff --git a/src/processing/save.rs b/src/processing/save.rs index 9c14339..a352c18 100644 --- a/src/processing/save.rs +++ b/src/processing/save.rs @@ -171,9 +171,17 @@ pub fn format_max_dimension(format: &str) -> Option { /// Saves an image to bytes in the specified format. pub fn save_image(img: VipsImage, format: &str, quality: u8) -> Result, SaveError> { - save_image_with_options(img, format, quality, &SaveOptions::default(), None) + save_image_with_options(img, format, quality, &SaveOptions::default(), None, None) } +/// A caller-supplied rewrite of the encoded bytes. +/// +/// Whatever it produces is what the client receives, so the `max_bytes` search +/// has to measure its output rather than the encoder's — otherwise a step that +/// grows the file, such as splicing a copyright block back in, can push a +/// response over a limit that a lower quality would have met. +pub type Finalize<'a> = &'a dyn Fn(Vec) -> Vec; + /// Saves an image to bytes using imgproxy-compatible encoder controls. /// /// `page_height` carries the frame height of an animation. libvips stores an @@ -185,6 +193,7 @@ pub fn save_image_with_options( quality: u8, options: &SaveOptions, page_height: Option, + finalize: Option>, ) -> Result, SaveError> { let Some(spec) = canonical_format(format) else { return Err(SaveError::UnsupportedFormat { @@ -198,7 +207,7 @@ pub fn save_image_with_options( }); } - encode_with_max_bytes(&img, spec, quality, options, page_height) + encode_with_max_bytes(&img, spec, quality, options, page_height, finalize) } fn encode_with_max_bytes( @@ -207,14 +216,21 @@ fn encode_with_max_bytes( quality: u8, options: &SaveOptions, page_height: Option, + finalize: Option>, ) -> Result, SaveError> { + let finish = |bytes: Vec| match finalize { + Some(finalize) => finalize(bytes), + None => bytes, + }; + let Some(max_bytes) = options.max_bytes else { - return encode_once(img, spec, quality, options, page_height); + return Ok(finish(encode_once(img, spec, quality, options, page_height)?)); }; let mut quality = quality.clamp(1, 100); loop { - let bytes = encode_once(img, spec, quality, options, page_height)?; + // Measured after finalizing, so the budget covers what is actually sent. + let bytes = finish(encode_once(img, spec, quality, options, page_height)?); if bytes.len() <= max_bytes || quality <= 1 { return Ok(bytes); } @@ -318,21 +334,18 @@ impl Suffix { } } -/// Maps a requested WebP preset to the matching vips nickname. +/// Maps a requested WebP preset to the vips nickname of the same name. /// -/// `preset` reaches us as free text from the URL, so only names vips actually -/// defines may be interpolated into the option string; anything else is -/// dropped and the encoder default applies. +/// The parser has already refused anything outside this set, but the value +/// still reaches an option string that vips parses, so it is matched against +/// the known names once more rather than interpolated on trust. A `preset` +/// carrying `],lossless` would otherwise close the bracket and set an option +/// the request never asked for. fn webp_preset_nickname(preset: &str) -> Option<&'static str> { - match preset { - "default" => Some("default"), - "picture" => Some("picture"), - "photo" => Some("photo"), - "drawing" => Some("drawing"), - "icon" => Some("icon"), - "text" => Some("text"), - _ => None, - } + crate::processing::options::WEBP_PRESETS + .iter() + .find(|known| **known == preset) + .copied() } /// Builds the encoder suffix for one format. diff --git a/src/processing/tests/effects_tests.rs b/src/processing/tests/effects_tests.rs index ab5929d..11f46ad 100644 --- a/src/processing/tests/effects_tests.rs +++ b/src/processing/tests/effects_tests.rs @@ -1,5 +1,5 @@ use crate::processing::colorspace; -use crate::processing::options::{Adjust, Crop, Flip, Gravity, GravityType, Trim, Zoom}; +use crate::processing::options::{Adjust, Crop, Flip, Gravity, GravityType, Resize, ResizingType, Trim, Zoom}; use crate::processing::transform::{self, TransformError}; use libvips::{ops, VipsImage}; @@ -671,3 +671,74 @@ fn a_wide_gamut_source_is_converted_through_its_profile() { "the round trip should land close to the original, got {recovered:.2}" ); } + +/// Smart gravity asks libvips which part of the image a viewer would look at, +/// which is the one thing a geometric anchor cannot do. The subject here sits +/// in the bottom-right, so a centre crop misses it entirely and any fixed +/// anchor would only be right for images built like this one. +#[test] +fn test_smart_gravity_finds_the_subject_a_centre_crop_would_miss() { + init_vips(); + let source = create_image_with_subject_at((120, 120), (86, 86, 24, 24)); + + let smart = transform::crop_image( + image_from(source.clone()), + &Crop { + width: 48.0, + height: 48.0, + gravity: Some(Gravity::new(GravityType::Smart)), + }, + &Gravity::new(GravityType::Smart), + ) + .unwrap(); + let centred = transform::crop_image( + image_from(source), + &Crop { + width: 48.0, + height: 48.0, + gravity: None, + }, + &Gravity::default(), + ) + .unwrap(); + + assert_eq!((smart.get_width(), smart.get_height()), (48, 48)); + + // The subject is dark on a light field, so the crop that found it is the + // darker one by a wide margin. + let smart_luminance = mean_luminance(&decode_rgba(&smart)); + let centred_luminance = mean_luminance(&decode_rgba(¢red)); + assert!( + smart_luminance < centred_luminance - 20.0, + "smart crop should have found the subject: smart {smart_luminance:.1}, centred {centred_luminance:.1}" + ); +} + +/// The same applies to the implicit crop a `fill` resize performs. +#[test] +fn test_smart_gravity_positions_the_fill_window() { + init_vips(); + let source = create_image_with_subject_at((200, 100), (160, 30, 32, 40)); + + let resize = Resize { + resizing_type: ResizingType::Fill, + width: 60, + height: 60, + }; + let smart = transform::apply_resize( + image_from(source.clone()), + &resize, + &Gravity::new(GravityType::Smart), + None, + false, + 1.0, + ) + .unwrap(); + let centred = transform::apply_resize(image_from(source), &resize, &Gravity::default(), None, false, 1.0).unwrap(); + + assert_eq!((smart.get_width(), smart.get_height()), (60, 60)); + assert!( + mean_luminance(&decode_rgba(&smart)) < mean_luminance(&decode_rgba(¢red)) - 10.0, + "the fill window should have moved toward the subject" + ); +} diff --git a/src/processing/tests/options_parse_tests.rs b/src/processing/tests/options_parse_tests.rs index b77511b..332c0b5 100644 --- a/src/processing/tests/options_parse_tests.rs +++ b/src/processing/tests/options_parse_tests.rs @@ -1330,3 +1330,63 @@ fn test_a_dimensionless_resizing_type_is_still_remembered() { assert!(parsed.resize.is_none(), "no dimensions means no resize"); assert_eq!(parsed.resizing_type, Some(ResizingType::Fill)); } + +/// An unrecognised WebP preset used to be dropped on the way to the encoder, +/// so a typo silently produced a different image. It is refused now, like every +/// other bad argument. +#[test] +fn test_unknown_webp_preset_is_rejected() { + let with_preset = |preset: &str| { + parse_all_options(vec![ProcessingOption { + name: "webp_options".to_string(), + args: vec![String::new(), String::new(), preset.to_string()], + }]) + }; + + for preset in ["default", "picture", "photo", "drawing", "icon", "text", "PHOTO"] { + let parsed = with_preset(preset).unwrap_or_else(|err| panic!("{preset} should parse: {err}")); + assert_eq!(parsed.save.webp.preset.as_deref(), Some(preset.to_lowercase().as_str())); + } + + for preset in ["photograph", "photo],lossless", "sharp"] { + assert!(with_preset(preset).is_err(), "{preset} should be refused"); + } +} + +/// Gravity means different things in different places, and imgproxy scopes it +/// accordingly: `CropGravityTypes` carries `sm` and `fp`, `ExtendGravityTypes` +/// carries only `fp`, and `WatermarkGravityTypes` carries neither. `sm` reaching +/// an extend fell through `calc_position` to the centre branch, so the request +/// succeeded and silently behaved as `ce`. +#[test] +fn smart_gravity_is_scoped_to_the_options_that_can_use_it() { + let with_gravity = |option: &str, gravity: &str| { + parse_all_options(vec![ProcessingOption { + name: option.to_string(), + args: vec!["true".to_string(), gravity.to_string()], + }]) + }; + + for option in ["extend", "extend_aspect_ratio"] { + assert!( + with_gravity(option, "sm").is_err(), + "{option} has no content to choose from and must refuse sm" + ); + // The rest of the anchors still work, including the focus point, which + // does mean something when positioning on a larger canvas. + for gravity in ["ce", "no", "soea", "fp"] { + assert!( + with_gravity(option, gravity).is_ok(), + "{option}:{gravity} should still parse" + ); + } + } + + // Cropping is exactly where smart gravity belongs. + let cropped = parse_all_options(vec![ProcessingOption { + name: "crop".to_string(), + args: vec!["100".to_string(), "100".to_string(), "sm".to_string()], + }]) + .expect("crop:100:100:sm is the option smart gravity exists for"); + assert!(cropped.crop.is_some()); +} diff --git a/src/processing/tests/save_tests.rs b/src/processing/tests/save_tests.rs index 61a1fa1..3d2d2c2 100644 --- a/src/processing/tests/save_tests.rs +++ b/src/processing/tests/save_tests.rs @@ -58,7 +58,7 @@ fn test_webp_save_lossless_applies() { options.webp.lossless = Some(true); let img = VipsImage::new_from_buffer(&base, "").unwrap(); - let lossless = save::save_image_with_options(img, "webp", 80, &options, None).unwrap(); + let lossless = save::save_image_with_options(img, "webp", 80, &options, None, None).unwrap(); let img = VipsImage::new_from_buffer(&base, "").unwrap(); let lossy = save::save_image(img, "webp", 80).unwrap(); @@ -87,7 +87,7 @@ fn test_webp_save_honors_max_bytes() { ..Default::default() }; let img = VipsImage::new_from_buffer(&base, "").unwrap(); - let bounded = save::save_image_with_options(img, "webp", 95, &options, None).unwrap(); + let bounded = save::save_image_with_options(img, "webp", 95, &options, None, None).unwrap(); assert!( bounded.len() <= budget && bounded.len() < unbounded.len(), @@ -145,11 +145,17 @@ fn test_webp_save_suffix_clamps_quality() { } #[test] -fn test_webp_save_suffix_drops_unknown_preset() { - // `preset` arrives as free text from the URL, so anything vips does not - // define must never reach the option string. - let mut options = SaveOptions::default(); - options.webp.preset = Some("photo],lossless".to_string()); +fn test_webp_save_suffix_never_interpolates_an_unknown_preset() { + // The parser refuses these, but the suffix goes through vips' own option + // parser, so a value that could close the bracket and set an option the + // request never asked for must not reach it even if it arrives another way. + let options = SaveOptions { + webp: crate::processing::options::WebpOptions { + preset: Some("photo],lossless".to_string()), + ..Default::default() + }, + ..SaveOptions::default() + }; assert_eq!( save::save_suffix("webp", 75, &options, None).unwrap(), ".webp[Q=75,keep=all]" @@ -336,3 +342,51 @@ fn format_ceilings_match_the_encoders_own_limits() { assert_eq!(format_max_dimension("png"), None); assert_eq!(format_max_dimension("tiff"), None); } + +/// `max_bytes` has to measure what the client receives. The copyright block is +/// spliced into the encoded bytes afterwards, so measuring the bare encode let +/// a response exceed the budget when a lower quality would have fitted. +#[test] +fn test_max_bytes_measures_the_finalized_bytes() { + init_vips(); + let base = create_textured_image(200, 200); + + // A finalizer that appends a fixed block, standing in for the metadata + // splice. Large relative to the gap between quality steps, so that ignoring + // it would demonstrably stop the search a step early. + let padding = vec![0u8; 4096]; + let finalize = |mut bytes: Vec| { + bytes.extend_from_slice(&padding); + bytes + }; + + // A budget that a low-quality encode plus the padding just fits. + let img = VipsImage::new_from_buffer(&base, "").unwrap(); + let budget = save::save_image(img, "webp", 20).unwrap().len() + padding.len(); + + let options = SaveOptions { + max_bytes: Some(budget), + ..SaveOptions::default() + }; + let img = VipsImage::new_from_buffer(&base, "").unwrap(); + let bounded = save::save_image_with_options(img, "webp", 95, &options, None, Some(&finalize)).unwrap(); + + assert!( + bounded.len() <= budget, + "the budget must cover the finalized bytes: {} > {budget}", + bounded.len() + ); + assert!( + bounded.ends_with(&padding), + "the finalizer's output is what should have been returned" + ); + + // Without counting the padding the loop would have stopped a step early, + // so the same budget on the bare encode alone leaves room to spare. + let img = VipsImage::new_from_buffer(&base, "").unwrap(); + let unfinalized = save::save_image_with_options(img, "webp", 95, &options, None, None).unwrap(); + assert!( + unfinalized.len() + padding.len() > budget, + "the test is only meaningful if the padding is what forces the extra step" + ); +} diff --git a/src/processing/tests_support.rs b/src/processing/tests_support.rs index 4a88499..40aa525 100644 --- a/src/processing/tests_support.rs +++ b/src/processing/tests_support.rs @@ -191,3 +191,33 @@ pub fn frame_count(bytes: &[u8]) -> i32 { .expect("encoded image should decode") .get_n_pages() } + +/// A flat field with one small, high-contrast block inset at (`x`, `y`). +/// +/// Built for the content-aware crop: a uniform image gives an attention map no +/// algorithm can distinguish from noise, so the test needs exactly one place +/// worth looking at, positioned away from the centre. +pub fn create_image_with_subject_at(size: (u32, u32), subject: (u32, u32, u32, u32)) -> Vec { + let (width, height) = size; + let (x, y, block_w, block_h) = subject; + let mut img: ImageBuffer, Vec> = ImageBuffer::from_pixel(width, height, Rgba([250, 250, 250, 255])); + for py in y..(y + block_h).min(height) { + for px in x..(x + block_w).min(width) { + img.put_pixel(px, py, Rgba([5, 5, 5, 255])); + } + } + let mut bytes: Vec = Vec::new(); + img.write_to(&mut std::io::Cursor::new(&mut bytes), image::ImageFormat::Png) + .unwrap(); + bytes +} + +/// The mean luminance of a decoded image, for asking "did the dark subject end +/// up inside this crop?" without depending on exact pixel positions. +pub fn mean_luminance(decoded: &RgbaImage) -> f64 { + let total: f64 = decoded + .pixels() + .map(|pixel| 0.299 * f64::from(pixel[0]) + 0.587 * f64::from(pixel[1]) + 0.114 * f64::from(pixel[2])) + .sum(); + total / decoded.pixels().len() as f64 +} diff --git a/src/processing/transform/geometry.rs b/src/processing/transform/geometry.rs index 29d5b6e..910e311 100644 --- a/src/processing/transform/geometry.rs +++ b/src/processing/transform/geometry.rs @@ -89,6 +89,23 @@ pub fn calc_position( (left, top) } +/// Crops to a window libvips chooses by looking at the image. +/// +/// `smartcrop` scores the image for the region a viewer's eye would settle on, +/// which is the one thing a geometric gravity cannot do: a centre crop of a +/// portrait decapitates the subject, and no fixed anchor fixes that for every +/// image in a catalogue. +/// +/// It has to see real pixels, so unlike every other window here it forces the +/// decode rather than composing into libvips' lazy pipeline. +pub fn smart_crop(img: &VipsImage, width: i32, height: i32) -> Result { + let options = ops::SmartcropOptions { + interesting: ops::Interesting::Attention, + ..Default::default() + }; + ops::smartcrop_with_opts(img, width, height, &options).map_err(vips("Error finding a smart crop")) +} + /// Crops an image to the region named by a [`Crop`]. /// /// A zero extent means "the whole axis", and an extent below 1 is a fraction of @@ -116,6 +133,10 @@ pub fn crop_image(img: VipsImage, crop: &Crop, gravity: &Gravity) -> Result, Vec> = ImageBuffer::from_pixel(240, 120, Rgba([250, 250, 250, 255])); + for y in 30..90 { + for x in 170..230 { + source.put_pixel(x, y, Rgba([5, 5, 5, 255])); + } + } + let mut bytes = Vec::new(); + source + .write_to(&mut std::io::Cursor::new(&mut bytes), image::ImageFormat::Png) + .unwrap(); + + Mock::given(method("GET")) + .and(path("/subject.png")) + .respond_with( + ResponseTemplate::new(200) + .set_body_bytes(bytes) + .insert_header("Content-Type", "image/png"), + ) + .expect(1..) + .mount(&server) + .await; + + let encoded = URL_SAFE_NO_PAD.encode(format!("{}/subject.png", server.uri()).as_bytes()); + + let fetch = |gravity: &str| { + let uri = format!("/unsafe/rs:fill:48:48/g:{gravity}/format:png/{encoded}"); + async move { + let state = + create_test_state_with_cache(create_test_config(vec![], vec![], true), ImgforgeCache::None).await; + let app = axum::Router::new() + .route("/{*path}", axum::routing::get(image_forge_handler)) + .with_state(state); + make_request(app, &uri).await + } + }; + + let (smart_status, smart_body) = fetch("sm").await; + let (centre_status, centre_body) = fetch("ce").await; + assert_eq!(smart_status, StatusCode::OK, "smart gravity should be accepted"); + assert_eq!(centre_status, StatusCode::OK); + + let decoded = image::load_from_memory(&smart_body).expect("the response should be a decodable image"); + assert_eq!( + image::GenericImageView::dimensions(&decoded), + (48, 48), + "the fill target should be honoured" + ); + // Compared as mean brightness rather than as raw bytes: the subject is dark, + // so a window that found it is measurably darker than the centre one — and a + // failure prints two numbers instead of two PNGs. + let mean = |body: &[u8]| { + let image = image::load_from_memory(body).expect("a decodable image").to_luma8(); + image.pixels().map(|p| f64::from(p[0])).sum::() / image.pixels().len() as f64 + }; + let smart_mean = mean(&smart_body); + let centre_mean = mean(¢re_body); + assert!( + smart_mean < centre_mean - 10.0, + "smart gravity should have moved the window onto the subject \ + (smart {smart_mean:.1} should be darker than centre {centre_mean:.1})" + ); + + // And the scoping holds end to end: `sm` is refused where it means nothing. + let uri = format!("/unsafe/rs:fit:48:48/ex:true:sm/{encoded}"); + let state = create_test_state_with_cache(create_test_config(vec![], vec![], true), ImgforgeCache::None).await; + let app = axum::Router::new() + .route("/{*path}", axum::routing::get(image_forge_handler)) + .with_state(state); + let (status, _) = make_request(app, &uri).await; + assert_eq!( + status, + StatusCode::BAD_REQUEST, + "extend has no content to choose from, so sm must be refused" + ); +} + +/// `keep_copyright` through the handler, checking the bytes a client actually +/// receives rather than the helper that builds them: the EXIF block has to +/// survive encoding into each container that has a writer. +#[tokio::test] +async fn keep_copyright_reaches_the_encoded_response_for_png_and_webp() { + let server = MockServer::start().await; + let source = jpeg_with_copyright("Imgforge Test", "A. Photographer"); + + Mock::given(method("GET")) + .and(path("/rights.jpg")) + .respond_with( + ResponseTemplate::new(200) + .set_body_bytes(source) + .insert_header("Content-Type", "image/jpeg"), + ) + .expect(1..) + .mount(&server) + .await; + + let encoded = URL_SAFE_NO_PAD.encode(format!("{}/rights.jpg", server.uri()).as_bytes()); + + for format in ["jpeg", "png", "webp"] { + let uri = format!("/unsafe/rs:fit:32:32/sm:true/kcr:true/format:{format}/{encoded}"); + let state = create_test_state_with_cache(create_test_config(vec![], vec![], true), ImgforgeCache::None).await; + let app = axum::Router::new() + .route("/{*path}", axum::routing::get(image_forge_handler)) + .with_state(state); + let (status, body) = make_request(app, &uri).await; + + assert_eq!(status, StatusCode::OK, "format:{format} should succeed"); + assert!( + contains_bytes(&body, b"Imgforge Test"), + "format:{format} should carry the copyright through the strip" + ); + assert!( + contains_bytes(&body, b"A. Photographer"), + "format:{format} should carry the artist through the strip" + ); + } +} + +/// Whether `needle` appears anywhere in `haystack`. +fn contains_bytes(haystack: &[u8], needle: &[u8]) -> bool { + haystack.windows(needle.len()).any(|window| window == needle) +} + +/// A JPEG carrying EXIF `Copyright` and `Artist`, built by hand so the fixture +/// needs no tooling and no checked-in binary. +fn jpeg_with_copyright(copyright: &str, artist: &str) -> Vec { + use image::{ImageBuffer, ImageFormat, Rgb}; + + let mut base = Vec::new(); + ImageBuffer::, Vec>::from_pixel(64, 64, Rgb([120, 90, 60])) + .write_to(&mut std::io::Cursor::new(&mut base), ImageFormat::Jpeg) + .unwrap(); + + // Two ASCII entries whose values live past the IFD, which is where anything + // longer than four bytes has to go. + let copyright = format!("{copyright}\0"); + let artist = format!("{artist}\0"); + let ifd_end = 8 + 2 + 12 * 2 + 4; + let artist_offset = ifd_end; + let copyright_offset = artist_offset + artist.len(); + + let mut tiff = Vec::new(); + tiff.extend_from_slice(b"II"); + tiff.extend_from_slice(&42u16.to_le_bytes()); + tiff.extend_from_slice(&8u32.to_le_bytes()); + tiff.extend_from_slice(&2u16.to_le_bytes()); + for (tag, value, offset) in [ + (0x013Bu16, &artist, artist_offset), + (0x8298u16, ©right, copyright_offset), + ] { + tiff.extend_from_slice(&tag.to_le_bytes()); + tiff.extend_from_slice(&2u16.to_le_bytes()); // ASCII + tiff.extend_from_slice(&(value.len() as u32).to_le_bytes()); + tiff.extend_from_slice(&(offset as u32).to_le_bytes()); + } + tiff.extend_from_slice(&0u32.to_le_bytes()); + tiff.extend_from_slice(artist.as_bytes()); + tiff.extend_from_slice(copyright.as_bytes()); + + let mut app1 = Vec::from(&b"Exif\0\0"[..]); + app1.extend_from_slice(&tiff); + + let mut out = Vec::new(); + out.extend_from_slice(&base[..2]); + out.extend_from_slice(&[0xFF, 0xE1]); + out.extend_from_slice(&((app1.len() + 2) as u16).to_be_bytes()); + out.extend_from_slice(&app1); + out.extend_from_slice(&base[2..]); + out +} /// Both of these settings change the bytes of a response whose URL never /// changes, and neither carries a version bump to retire what it invalidates. /// The cache key has to carry them, and the request path has to actually pass