EXIF orientation: bake it in when normalizing images (#103)
Build image / build-and-push (push) Successful in 8s
Gadfly review (reusable) / review (pull_request) Successful in 7m8s
Adversarial Review (Gadfly) / review (pull_request) Successful in 7m8s

imagenorm decoded and re-encoded to JPEG but ignored the EXIF Orientation
tag. Phone cameras store the sensor pixels one way and set an EXIF flag to
rotate on display, so a "portrait" JPEG is really a landscape bitmap tagged
"rotate 90°" — and our re-encode strips EXIF, so without baking the rotation
in, a packet photographed in portrait reaches the vision model sideways
(bad OCR) and any future thumbnail is wrong.

- exifOrientation: a small pure-Go parser that walks the JPEG APP1/Exif
  segment for tag 0x0112, returning 1 (normal) for non-JPEG or unparseable
  input — never guess a rotation onto a correct image. No cgo, no new dep.
- applyOrientation: bakes in all 8 orientations (the 4 rotations + mirrors)
  after downscale (cheaper to rotate the small image; a 90° turn swaps the
  sides but not the longest edge, so the downscale bound still holds).
- Non-JPEG paths (HEIC/webp/png) are untouched — their decoders own
  orientation and they carry no JPEG EXIF.

Tests build oriented JPEGs (a corner marker + a spliced Exif APP1) and
assert the marker lands where each orientation says, dims swapping for the
quarter-turns; plus parser defaults for non-JPEG / no-EXIF / garbage.
Stays CGO_ENABLED=0.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01H3zbym8Doka2d7D48maSgZ
This commit is contained in:
2026-07-22 01:23:08 -04:00
co-authored by Claude Opus 4.8
parent 8a27df9e9c
commit 0ab90a01cb
3 changed files with 256 additions and 11 deletions
+1 -1
View File
@@ -13,7 +13,7 @@ Work is tracked in Gitea issues; the tracking epic links every piece in dependen
- **Users:** multi-user with ownership. Users own gardens; a garden can be shared with other users as viewer (read) or editor (edit content). Owner additionally shares/deletes. The first registered user is `is_admin` (set race-free inside the INSERT); admin gates instance-wide Settings (`requireAdmin`), the only thing that reads that flag. - **Users:** multi-user with ownership. Users own gardens; a garden can be shared with other users as viewer (read) or editor (edit content). Owner additionally shares/deletes. The first registered user is `is_admin` (set race-free inside the INSERT); admin gates instance-wide Settings (`requireAdmin`), the only thing that reads that flag.
- **Auth:** OIDC-first (Authentik is the primary IdP), local argon2id passwords as an optional fallback. - **Auth:** OIDC-first (Authentik is the primary IdP), local argon2id passwords as an optional fallback.
- **Instance settings (#79):** admin-editable, instance-wide config in a single-row `instance_settings` table — pansy's first DB-stored *instance* state (everything else hangs off a garden/object). Today it holds the agent model + on/off; **secrets never move here**`OLLAMA_CLOUD_API_KEY` stays in the env so it doesn't land in backups or the undo history. Precedence: Settings value → env → default. The live agent Runner sits behind an `atomic.Pointer` in the API layer (`agentHolder`) with its routes always registered, so a settings change swaps it with no restart and no race against in-flight requests; `/capabilities` reads that pointer, so it reports what's live rather than what was configured at boot. The model/registry knowledge lives in one leaf package (`internal/agentmodel`) that both the runner and the settings validator import — agent imports service, so it can live in neither. - **Instance settings (#79):** admin-editable, instance-wide config in a single-row `instance_settings` table — pansy's first DB-stored *instance* state (everything else hangs off a garden/object). Today it holds the agent model + on/off; **secrets never move here**`OLLAMA_CLOUD_API_KEY` stays in the env so it doesn't land in backups or the undo history. Precedence: Settings value → env → default. The live agent Runner sits behind an `atomic.Pointer` in the API layer (`agentHolder`) with its routes always registered, so a settings change swaps it with no restart and no race against in-flight requests; `/capabilities` reads that pointer, so it reports what's live rather than what was configured at boot. The model/registry knowledge lives in one leaf package (`internal/agentmodel`) that both the runner and the settings validator import — agent imports service, so it can live in neither.
- **Seed-packet capture (#81):** photograph a packet → a *vision* model (separate `vision_model` setting) reads it into structured fields via one-shot `majordomo.Generate[SeedPacket]` — NOT an agent loop, so the extraction can't touch the garden; it only reads a picture and returns data. The image is normalized to JPEG at the upload boundary (`internal/imagenorm`: decodes HEIC/webp/png/jpeg, since majordomo's stdlib media path can't do HEIC — the iPhone default). The hard part is **catalog matching, not OCR**: a wrong auto-match splits a variety's seed-lot history across duplicate rows, so the service NEVER auto-creates — it surfaces ranked candidates (`matchPlants`) and the user confirms, then `CreateFromPacket` makes the plant (new or existing) + the lot. Plants/lots aren't in the undo history (they're catalog/inventory), so there's no change set to wrap. The extractor is injectable on the service (`WithPacketExtractor`) so the whole path tests hermetically against majordomo's `fake` provider. - **Seed-packet capture (#81):** photograph a packet → a *vision* model (separate `vision_model` setting) reads it into structured fields via one-shot `majordomo.Generate[SeedPacket]` — NOT an agent loop, so the extraction can't touch the garden; it only reads a picture and returns data. The image is normalized to JPEG at the upload boundary (`internal/imagenorm`: decodes HEIC/webp/png/jpeg, since majordomo's stdlib media path can't do HEIC — the iPhone default; it also bakes in the JPEG EXIF orientation, since the re-encode strips EXIF and a phone photo tagged "rotate 90°" would otherwise reach the model sideways). The hard part is **catalog matching, not OCR**: a wrong auto-match splits a variety's seed-lot history across duplicate rows, so the service NEVER auto-creates — it surfaces ranked candidates (`matchPlants`) and the user confirms, then `CreateFromPacket` makes the plant (new or existing) + the lot. Plants/lots aren't in the undo history (they're catalog/inventory), so there's no change set to wrap. The extractor is injectable on the service (`WithPacketExtractor`) so the whole path tests hermetically against majordomo's `fake` provider.
- **Agentic future:** integration with majordomo/executus via typed Go tools (`llm.DefineTool[Args]`) wrapping the same service layer the REST API uses — not MCP/OpenAPI. - **Agentic future:** integration with majordomo/executus via typed Go tools (`llm.DefineTool[Args]`) wrapping the same service layer the REST API uses — not MCP/OpenAPI.
## Domain model ## Domain model
+137 -10
View File
@@ -28,6 +28,7 @@ package imagenorm
import ( import (
"bytes" "bytes"
"encoding/binary"
"errors" "errors"
"fmt" "fmt"
"image" "image"
@@ -104,9 +105,10 @@ var (
) )
// Normalize reads an image of any supported format (JPEG, PNG, HEIC, WebP), // Normalize reads an image of any supported format (JPEG, PNG, HEIC, WebP),
// downscales it to fit opts.MaxDim on its longest edge, and returns it re-encoded // downscales it to fit opts.MaxDim on its longest edge, applies the JPEG EXIF
// as JPEG, plus the decoded format name (e.g. "heic") — handy for logging what a // orientation so the pixels come out upright, and returns it re-encoded as JPEG,
// phone actually sent. On any error the returned bytes are nil and format is "". // plus the decoded format name (e.g. "heic") — handy for logging what a phone
// actually sent. On any error the returned bytes are nil and format is "".
// //
// Errors, by cause: // Errors, by cause:
// - input over opts.MaxBytes, or a decoded canvas over maxDecodePixels / // - input over opts.MaxBytes, or a decoded canvas over maxDecodePixels /
@@ -120,13 +122,18 @@ var (
// pre-decode pixel/dimension check, and a recover around the third-party decoders // pre-decode pixel/dimension check, and a recover around the third-party decoders
// (a malformed HEIC/WebP shouldn't take the process down). // (a malformed HEIC/WebP shouldn't take the process down).
// //
// Two known gaps, both deferred to the upload handler that wires this in (#81): // EXIF orientation: phone cameras store the sensor pixels in one orientation and
// - EXIF ORIENTATION is not applied, so a portrait phone photo tagged // set an EXIF tag to rotate on display, so a JPEG "portrait" photo is really a
// "rotate 90°" comes out sideways. That's best fixed and tested with a real // landscape bitmap tagged "rotate 90°" — and the re-encode below strips EXIF,
// oriented photo end-to-end, which the library has no consumer for yet. // which is exactly why the rotation must be BAKED IN here. applyOrientation does
// - There is no context: image.Decode is CPU-bound and not cancellable // that for the JPEG path (the format phone uploads overwhelmingly arrive in);
// mid-decode, so a caller that needs a hard deadline should run Normalize // other formats carry no JPEG EXIF and their decoders own orientation, so they're
// under its own timeout. The size guards keep the work finite regardless. // left as decoded.
//
// One known gap, deferred to the upload handler (#81): there is no context —
// image.Decode is CPU-bound and not cancellable mid-decode, so a caller that
// needs a hard deadline should run Normalize under its own timeout. The size
// guards keep the work finite regardless.
func Normalize(r io.Reader, opts Options) (out []byte, format string, err error) { func Normalize(r io.Reader, opts Options) (out []byte, format string, err error) {
// Cap the read at MaxBytes+1 so we can tell "exactly at the cap" from "over". // Cap the read at MaxBytes+1 so we can tell "exactly at the cap" from "over".
// maxBytes() is always a sane positive (default 25 MiB); guard the +1 anyway. // maxBytes() is always a sane positive (default 25 MiB); guard the +1 anyway.
@@ -161,7 +168,11 @@ func Normalize(r io.Reader, opts Options) (out []byte, format string, err error)
return nil, "", err return nil, "", err
} }
// Downscale first (cheaper to rotate the small image), then bake in the EXIF
// orientation so the JPEG we emit is upright. A 90° rotation swaps the sides
// but not the longest edge, so the downscale bound still holds after it.
img = downscale(img, opts.maxDim()) img = downscale(img, opts.maxDim())
img = applyOrientation(img, exifOrientation(raw))
var buf bytes.Buffer var buf bytes.Buffer
if err := jpeg.Encode(&buf, img, &jpeg.Options{Quality: jpegQuality}); err != nil { if err := jpeg.Encode(&buf, img, &jpeg.Options{Quality: jpegQuality}); err != nil {
@@ -188,6 +199,122 @@ func decodeSafely(raw []byte) (img image.Image, format string, err error) {
return img, format, nil return img, format, nil
} }
// applyOrientation returns img with the EXIF orientation (1..8) baked in, so the
// pixels are upright and no display-time rotation is needed. Orientation 1 (and
// anything out of range) is a no-op. Values 5..8 are 90° rotations, which swap
// the output's width and height. Reads through image.Image.At and writes an RGBA,
// so it works whatever concrete type decode/downscale produced.
func applyOrientation(img image.Image, o int) image.Image {
if o <= 1 || o > 8 {
return img
}
b := img.Bounds()
w, h := b.Dx(), b.Dy()
swap := o >= 5 // a quarter-turn: output dimensions transpose
dst := image.NewRGBA(image.Rect(0, 0, w, h))
if swap {
dst = image.NewRGBA(image.Rect(0, 0, h, w))
}
for y := range h {
for x := range w {
c := img.At(b.Min.X+x, b.Min.Y+y)
var dx, dy int
switch o {
case 2: // mirror horizontal
dx, dy = w-1-x, y
case 3: // rotate 180
dx, dy = w-1-x, h-1-y
case 4: // mirror vertical
dx, dy = x, h-1-y
case 5: // transpose (mirror across the main diagonal)
dx, dy = y, x
case 6: // rotate 90° clockwise
dx, dy = h-1-y, x
case 7: // transverse (mirror across the anti-diagonal)
dx, dy = h-1-y, w-1-x
case 8: // rotate 90° counter-clockwise
dx, dy = y, w-1-x
}
dst.Set(dx, dy, c)
}
}
return dst
}
// exifOrientation extracts the EXIF Orientation tag (1..8) from raw image bytes,
// returning 1 (normal) when it's absent or unparseable — the safe default, since
// a wrong guess rotates a correct image. Only the JPEG APP1/Exif path is parsed:
// that's the format uploaded phone photos overwhelmingly arrive in, and the other
// decoders own their own orientation.
func exifOrientation(raw []byte) int {
// A JPEG is a run of FFxx marker segments after the SOI (FFD8). Walk them
// looking for APP1 (FFE1) carrying "Exif\0\0"; stop at the scan data (SOS).
if len(raw) < 4 || raw[0] != 0xFF || raw[1] != 0xD8 {
return 1
}
for i := 2; i+4 <= len(raw); {
if raw[i] != 0xFF {
return 1 // not aligned on a marker; give up rather than misread
}
marker := raw[i+1]
if marker == 0xD9 || marker == 0xDA {
return 1 // EOI / start-of-scan: no more headers to read
}
segLen := int(raw[i+2])<<8 | int(raw[i+3])
if segLen < 2 || i+2+segLen > len(raw) {
return 1
}
if marker == 0xE1 {
if o, ok := orientationFromApp1(raw[i+4 : i+2+segLen]); ok {
return o
}
}
i += 2 + segLen
}
return 1
}
// orientationFromApp1 reads the Orientation tag from a JPEG APP1 segment body
// (everything after the 2-byte length): "Exif\0\0" then a TIFF block holding
// IFD0. Returns (0, false) if the segment isn't Exif or the tag is missing.
func orientationFromApp1(seg []byte) (int, bool) {
const prefix = "Exif\x00\x00"
if len(seg) < len(prefix)+8 || string(seg[:len(prefix)]) != prefix {
return 0, false
}
tiff := seg[len(prefix):]
var bo binary.ByteOrder
switch string(tiff[0:2]) {
case "II":
bo = binary.LittleEndian
case "MM":
bo = binary.BigEndian
default:
return 0, false
}
ifd := int(bo.Uint32(tiff[4:8])) // offset to IFD0 from the TIFF start
if ifd < 8 || ifd+2 > len(tiff) {
return 0, false
}
n := int(bo.Uint16(tiff[ifd : ifd+2]))
for k := range n {
off := ifd + 2 + k*12 // each IFD entry is 12 bytes
if off+12 > len(tiff) {
return 0, false
}
if bo.Uint16(tiff[off:off+2]) != 0x0112 { // Orientation tag
continue
}
// SHORT value lives in the first 2 bytes of the entry's value field.
v := int(bo.Uint16(tiff[off+8 : off+10]))
if v >= 1 && v <= 8 {
return v, true
}
return 0, false
}
return 0, false
}
// downscale returns img shrunk so its longest edge is at most maxDim, preserving // downscale returns img shrunk so its longest edge is at most maxDim, preserving
// aspect ratio. An image already within bounds is returned unchanged (no // aspect ratio. An image already within bounds is returned unchanged (no
// re-sampling, no quality loss beyond the JPEG round-trip). Uses Catmull-Rom for // re-sampling, no quality loss beyond the JPEG round-trip). Uses Catmull-Rom for
+118
View File
@@ -5,6 +5,7 @@ import (
"encoding/binary" "encoding/binary"
"hash/crc32" "hash/crc32"
"image" "image"
"image/color"
"image/jpeg" "image/jpeg"
"image/png" "image/png"
"os" "os"
@@ -200,3 +201,120 @@ func TestNormalizeRejectsPixelBomb(t *testing.T) {
}) })
} }
} }
// orientedJPEG builds a JPEG whose top-left quadrant is white and the rest black
// — a marker to track through a rotation — tagged with the given EXIF orientation
// (1..8). The marker lets a test assert the pixels actually moved to where that
// orientation says they should.
func orientedJPEG(t *testing.T, orient int) []byte {
t.Helper()
const w, h = 40, 24
m := image.NewRGBA(image.Rect(0, 0, w, h))
for y := range h {
for x := range w {
c := color.RGBA{0, 0, 0, 255}
if x < w/2 && y < h/2 {
c = color.RGBA{255, 255, 255, 255}
}
m.Set(x, y, c)
}
}
var jb bytes.Buffer
if err := jpeg.Encode(&jb, m, &jpeg.Options{Quality: 95}); err != nil {
t.Fatalf("encode jpeg: %v", err)
}
if orient == 0 {
return jb.Bytes() // caller wants a plain JPEG with no EXIF
}
return spliceExifOrientation(t, jb.Bytes(), orient)
}
// spliceExifOrientation inserts a minimal little-endian Exif APP1 segment
// carrying just the Orientation tag right after the JPEG SOI marker.
func spliceExifOrientation(t *testing.T, jpg []byte, orient int) []byte {
t.Helper()
var tiff bytes.Buffer
tiff.WriteString("II") // little-endian
_ = binary.Write(&tiff, binary.LittleEndian, uint16(0x2A))
_ = binary.Write(&tiff, binary.LittleEndian, uint32(8)) // IFD0 offset
_ = binary.Write(&tiff, binary.LittleEndian, uint16(1)) // one entry
_ = binary.Write(&tiff, binary.LittleEndian, uint16(0x0112))
_ = binary.Write(&tiff, binary.LittleEndian, uint16(3)) // SHORT
_ = binary.Write(&tiff, binary.LittleEndian, uint32(1)) // count
_ = binary.Write(&tiff, binary.LittleEndian, uint16(orient)) // value
_ = binary.Write(&tiff, binary.LittleEndian, uint16(0)) // value pad
_ = binary.Write(&tiff, binary.LittleEndian, uint32(0)) // next IFD
payload := append([]byte("Exif\x00\x00"), tiff.Bytes()...)
segLen := len(payload) + 2
seg := []byte{0xFF, 0xE1, byte(segLen >> 8), byte(segLen)}
seg = append(seg, payload...)
out := make([]byte, 0, len(jpg)+len(seg))
out = append(out, jpg[:2]...) // SOI
out = append(out, seg...)
return append(out, jpg[2:]...)
}
func bright(c color.Color) bool {
r, g, b, _ := c.RGBA() // 16-bit
return (r+g+b)/3 > 0x8000
}
// TestNormalizeAppliesExifOrientation is the #103 regression: a phone photo tagged
// "rotate 90°" must come out of Normalize with the pixels upright, not sideways —
// the re-encode strips EXIF, so the rotation has to be baked into the bitmap.
func TestNormalizeAppliesExifOrientation(t *testing.T) {
// The white marker starts centred at (10,6) in the 40x24 source. For each
// orientation, wantW/H is the corrected canvas and (mx,my) is where that
// marker must land — derived from the same transform Normalize applies.
cases := []struct {
orient int
wantW, wantH int
mx, my int
}{
{0, 40, 24, 10, 6}, // no EXIF → unchanged, marker top-left
{1, 40, 24, 10, 6}, // normal → unchanged
{3, 40, 24, 29, 17}, // 180 → bottom-right
{6, 24, 40, 17, 10}, // 90° CW → top-right (dims swap)
{8, 24, 40, 6, 29}, // 90° CCW → bottom-left (dims swap)
}
for _, tc := range cases {
out, format, err := Normalize(bytes.NewReader(orientedJPEG(t, tc.orient)), Options{})
if err != nil {
t.Fatalf("orient %d: Normalize err = %v", tc.orient, err)
}
if format != "jpeg" {
t.Errorf("orient %d: format = %q, want jpeg", tc.orient, format)
}
m, _, err := image.Decode(bytes.NewReader(out))
if err != nil {
t.Fatalf("orient %d: decode output: %v", tc.orient, err)
}
if m.Bounds().Dx() != tc.wantW || m.Bounds().Dy() != tc.wantH {
t.Errorf("orient %d: output %dx%d, want %dx%d",
tc.orient, m.Bounds().Dx(), m.Bounds().Dy(), tc.wantW, tc.wantH)
}
if !bright(m.At(m.Bounds().Min.X+tc.mx, m.Bounds().Min.Y+tc.my)) {
t.Errorf("orient %d: white marker not at (%d,%d) — orientation not applied",
tc.orient, tc.mx, tc.my)
}
}
}
// TestExifOrientationParsing pins the parser against non-JPEG and no-EXIF inputs,
// which must default to 1 (never guess a rotation onto a correct image).
func TestExifOrientationParsing(t *testing.T) {
if o := exifOrientation(pngBytes(t, 8, 8)); o != 1 {
t.Errorf("PNG orientation = %d, want 1 (no JPEG EXIF path)", o)
}
if o := exifOrientation(orientedJPEG(t, 0)); o != 1 {
t.Errorf("JPEG without EXIF orientation = %d, want 1", o)
}
if o := exifOrientation(orientedJPEG(t, 6)); o != 6 {
t.Errorf("JPEG tagged 6 → %d, want 6", o)
}
if o := exifOrientation([]byte("not an image")); o != 1 {
t.Errorf("garbage bytes → %d, want 1", o)
}
}