Embed face regions in videos instead of API re-linking (issue #30)

ARCHITECTURE.md

@@ -23,7 +23,7 @@ With `-faces`, the asset's face boxes are fetched from `GET /api/faces` (the v3
 
 Two mechanisms keep the round trip stable. First, faces with `sourceType: exif` are echoes of the file's own regions (the server's metadata import); they are dropped whenever a detected face carries the same normalized name, because counting both would grow the region set by one on every replace. The match is by name rather than person ID since names are how the importer links regions back to people — with two person records sharing a name, the echo can land on the record recognition did not pick. Echoes still count when they are a name's only face (server without ML). Second, the coordinate comparison widens its per-axis tolerance to two raster pixels: Immich's importer floors region corners to whole pixels, which on small images shifts a round-tripped coordinate by more than the base tolerance. Face edits also get their own pre-upload freshness check (mirroring the `updatedAt` one, which face operations do not bump): the boxes are re-fetched right before upload and the asset is skipped if they moved.
 
-Videos take a different path: MWG regions are an image-only container concept, and Immich re-detects faces on a re-uploaded video's thumbnail but drops the person link (the boxes come back as fresh unnamed clusters). So instead of embedding metadata, the replace flow re-links people over the API — after the new asset exists, every face on the old asset that carries a person is recreated on the new one with `POST /faces` (`recreateFaces` in process/faces.go, run from `finalizeReplacement` before the old asset is trashed, where the source links still live). Boxes are copied verbatim from `GET /api/faces` — no orientation math, since both sides speak the same pixel space Immich reported. People already linked on the target are skipped so repeated runs do not stack duplicates, and unnamed clusters are preserved too (createFace needs a person, so only faceless boxes are dropped). The endpoint arrived with manual face tagging in Immich 1.127, and faces it creates are `sourceType: manual` (they survive later detection jobs since 1.127); a `404` from it means the server is older, and the step is skipped with a warning rather than failing the replace.
+Videos embed the same MWG regions, through the same `BuildFaceRegions` path — Immich's metadata importer reads `RegionInfo` from a video container just as it does from an image (verified against a live server), so a re-uploaded video's people survive exactly like a photo's. The one difference is the orientation source: a video has no EXIF `Orientation`, so `regionOrientation` reads the QuickTime `Rotation` tag and maps it to the equivalent orientation value (`videoRotationToOrientation`: 0°→1, 90°→6, 270°→8) that feeds the same `rasterRegion` inverse. A 180° or non-cardinal rotation returns not-anchorable and the video's regions are skipped: Immich was observed to re-orient 90°/270° video regions on import but not 180°, so writing a 180° region would misplace the box. Only `mp4`/`mov`/`m4v` are eligible (`SupportsVideoMetadataEmbedding`); other containers exiftool cannot write are skipped.
 
 External-library assets (`libraryId` set; null means internal) are skipped on replace runs: API uploads always land in the internal library, so a replacement would migrate the asset and duplicate it at the next library scan. Read-only modes (dry-run, export) still process them. The `libraryId` semantics hold on every server the client can address: the plural `/api/assets` routes and nullable `libraryId` both arrived in Immich 1.106. The practical floors are higher anyway — `/api/assets/copy` (used by every replace that yields a new ID) first appears around v2.2, and `/api/server/about` (auto-detection) in 1.113 — so a set `libraryId` reliably means external wherever the tool can operate.
 

README.md

@@ -143,7 +143,6 @@ On Immich 1.113+ you can scope the API key to exactly what the tool needs (older
 | `asset.delete`   | Trash the old original after a verified replacement             |
 | `album.read`     | Resolve `-album` / `-album all` selections                      |
 | `face.read`      | Fetch face boxes — only needed with `-faces`                    |
-| `face.create`    | Re-link people on a re-uploaded video — only with `-faces`      |
 
 Read-only modes need less: `-dry-run` and `-export-dir` never write to the server, so they only require `server.about`, `asset.read`, `asset.download`, and `album.read` (drop `album.read` too if you only pass asset IDs; add `face.read` if you combine them with `-faces`).
 
@@ -261,7 +260,7 @@ Images use the full tag set below. Supported video containers (`mp4`, `mov`, `m4
 | Location    | `IPTC:City`, `XMP-photoshop:City`, `IPTC:Province-State`, `XMP-photoshop:State`, `IPTC:Country-PrimaryLocationName`, `XMP-photoshop:Country` | Dual IPTC + XMP-photoshop                                                                                |
 | DateTime    | `DateTimeOriginal`, `OffsetTimeOriginal`, `TimeZoneOffset`, `XMP-exif:DateTimeOriginal`, `XMP-xmp:CreateDate`                                | See below; XMP uses ISO 8601                                                                             |
 | Camera      | `Make`, `Model`, `LensModel`                                                                                                                 | Only written if file has no existing value                                                               |
-| Faces       | `XMP-mwg-rs:RegionInfo` (MWG face regions)                                                                                                   | Opt-in via `-faces`; images embed MWG regions, videos re-link people via the API; see below              |
+| Faces       | `XMP-mwg-rs:RegionInfo` (MWG face regions)                                                                                                   | Opt-in via `-faces`; images and supported videos, named people only; see below                           |
 
 ### Face regions (`-faces`)
 
@@ -271,8 +270,8 @@ With `-faces`, every person you have **named** in Immich is written into the fil
 - Regions are written in the stored image's coordinate space, applying the exact inverse of the orientation transform Immich uses on import, so rotated photos round-trip correctly.
 - When the file's regions disagree with Immich, the whole `RegionInfo` structure is **replaced** — Immich is the source of truth, like for every other synced tag. When Immich has no named faces for an asset, existing file regions are left untouched (never cleared).
 - Round-trip bonus: with Immich's _"Import faces from metadata"_ server setting enabled, the names embedded by `-faces` are re-imported when the replaced file is scanned — face names survive the replace. Re-imported regions (`sourceType: exif`) are recognized as echoes of the file's own content and never counted next to a detected face of the same name (names are how the importer links regions to people), so repeated runs converge instead of duplicating regions.
-- **Videos** cannot carry MWG regions, so `-faces` preserves them a different way: after a video is re-uploaded, every assigned person on the old asset is re-linked to the new one over Immich's faces API (the same call the web UI's manual tagging uses). This covers unnamed clusters too, and needs **Immich 1.127+** — older servers are skipped with a warning. Without it, a re-uploaded video keeps its re-detected boxes but loses the person names.
-- The key needs the `face.read` permission, plus `face.create` for the video path (see [API key permissions](#api-key-permissions)).
+- **Supported videos** (`mp4`, `mov`, `m4v`) embed the same MWG regions as images — exiftool writes XMP into the container, and Immich re-imports them when _"Import faces from metadata"_ is enabled, exactly as for photos. Regions are anchored through the video's display `Rotation` (0°, 90°, 270°); a 180° or non-cardinal rotation is left un-embedded rather than risk a misplaced box, and other containers are skipped.
+- The key needs the `face.read` permission (see [API key permissions](#api-key-permissions)).
 - Incremental-cache caveats: the `-all` state cache keys on the set of _names_, so correcting a face's box or reassigning a face between two already-named people does not invalidate it — use `-force` to re-check. Toggling `-faces` on or off changes the cache key, so the first run after a toggle re-checks the library once.
 
 ### DateTime and timezone handling

src/api/faces.go

@@ -1,9 +1,6 @@
 package api
 
 import (
-	"bytes"
-	"encoding/json"
-	"io"
 	"net/http"
 	"net/url"
 
@@ -26,26 +23,3 @@ func (c *ImmichClient) GetAssetFaces(assetID string) ([]model.AssetFaceResponse,
 	}
 	return faces, nil
 }
-
-// CreateFace links a person to an asset at a pixel box (POST /faces), the same
-// endpoint the web UI uses for manual face tagging. It preserves a named person
-// on a re-uploaded asset when metadata regions cannot — notably videos, where
-// Immich re-detects boxes on the new thumbnail but loses the person link.
-func (c *ImmichClient) CreateFace(face model.CreateFaceRequest) error {
-	jsonBody, err := json.Marshal(face)
-	if err != nil {
-		return err
-	}
-	req, err := c.newRequest(http.MethodPost, "/faces", bytes.NewReader(jsonBody))
-	if err != nil {
-		return err
-	}
-	req.Header.Set("Content-Type", "application/json")
-	resp, err := c.doRequest(req)
-	if err != nil {
-		return err
-	}
-	_, _ = io.Copy(io.Discard, resp.Body)
-	resp.Body.Close()
-	return nil
-}

src/api/faces_test.go

@@ -2,7 +2,6 @@ package api
 
 import (
 	"encoding/json"
-	"errors"
 	"net/http"
 	"net/http/httptest"
 	"testing"
@@ -57,44 +56,3 @@ func TestGetAssetFacesServerError(t *testing.T) {
 		t.Fatal("expected error on server failure")
 	}
 }
-
-func TestCreateFace(t *testing.T) {
-	var received model.CreateFaceRequest
-	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		if r.Method != http.MethodPost || r.URL.Path != "/api/faces" {
-			t.Fatalf("unexpected request: %s %s", r.Method, r.URL.Path)
-		}
-		if err := json.NewDecoder(r.Body).Decode(&received); err != nil {
-			t.Fatalf("decode body: %v", err)
-		}
-		w.WriteHeader(http.StatusCreated)
-	}))
-	defer server.Close()
-
-	c := NewImmichClient(server.URL, "key")
-	face := model.CreateFaceRequest{
-		AssetID: "new-asset", PersonID: "p1",
-		X: 100, Y: 50, Width: 200, Height: 200,
-		ImageWidth: 1000, ImageHeight: 500,
-	}
-	if err := c.CreateFace(face); err != nil {
-		t.Fatalf("unexpected error: %v", err)
-	}
-	if received != face {
-		t.Fatalf("server received %+v, want %+v", received, face)
-	}
-}
-
-func TestCreateFaceUnsupportedEndpoint(t *testing.T) {
-	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		http.Error(w, "not found", http.StatusNotFound)
-	}))
-	defer server.Close()
-
-	c := NewImmichClient(server.URL, "key")
-	err := c.CreateFace(model.CreateFaceRequest{AssetID: "a", PersonID: "p"})
-	var status *StatusError
-	if !errors.As(err, &status) || status.StatusCode != http.StatusNotFound {
-		t.Fatalf("expected a 404 StatusError (server too old to skip gracefully), got %v", err)
-	}
-}

src/main.go

@@ -128,7 +128,7 @@ func run() int {
 		return 0
 	}
 
-	uploader := &process.ModernUploader{Client: client, ResolveDuplicate: cfg.ResolveDuplicate, VerifyUpload: cfg.VerifyUpload, Faces: cfg.Faces}
+	uploader := &process.ModernUploader{Client: client, ResolveDuplicate: cfg.ResolveDuplicate, VerifyUpload: cfg.VerifyUpload}
 
 	if cfg.ExportDir != "" {
 		cfg.ExportDir = resolveExportDir(cfg)

src/model/people.go

@@ -25,22 +25,6 @@ type AssetFaceResponse struct {
 	Person        *PersonResponse `json:"person"`
 }
 
-// CreateFaceRequest is the POST /faces payload that links a person to an asset
-// at a pixel box. The coordinates and image dimensions mirror what
-// GetAssetFaces returns, so a box read from one asset re-creates verbatim on
-// another. Immich records it with sourceType "manual", which survives later ML
-// detection jobs (Immich 1.127+).
-type CreateFaceRequest struct {
-	AssetID     string `json:"assetId"`
-	PersonID    string `json:"personId"`
-	X           int    `json:"x"`
-	Y           int    `json:"y"`
-	Width       int    `json:"width"`
-	Height      int    `json:"height"`
-	ImageWidth  int    `json:"imageWidth"`
-	ImageHeight int    `json:"imageHeight"`
-}
-
 // NamedVisibleName returns a person's write-eligible region name and whether
 // they have one: visible and named, mirroring what Immich itself is willing to
 // import from file regions. It is the single definition of that eligibility,
@@ -71,7 +55,10 @@ func NamedPeopleNames(asset AssetResponse) []string {
 // are written. It answers the presence question without allocating and sorting
 // the name list, for the per-asset -all pre-filter.
 func HasFaceRegionsToEmbed(asset AssetResponse) bool {
-	if IsVideoAsset(asset) {
+	// Videos carry MWG regions only in containers exiftool can write; the
+	// rotation-anchoring guard that decides whether a given video is embeddable
+	// runs later, once the file's Rotation tag is known.
+	if IsVideoAsset(asset) && !SupportsVideoMetadataEmbedding(asset) {
 		return false
 	}
 	for _, person := range asset.People {

src/model/people_test.go

@@ -2,13 +2,24 @@ package model
 
 import "testing"
 
-func TestHasFaceRegionsToEmbedExcludesVideos(t *testing.T) {
+func TestHasFaceRegionsToEmbedAllowsSupportedVideos(t *testing.T) {
 	asset := AssetResponse{
 		OriginalMimeType: "video/mp4",
 		People:           []PersonResponse{{ID: "p1", Name: "Alice"}},
 	}
+	if !HasFaceRegionsToEmbed(asset) {
+		t.Fatal("mp4 videos with named people are region-embeddable")
+	}
+}
+
+func TestHasFaceRegionsToEmbedExcludesUnsupportedVideos(t *testing.T) {
+	asset := AssetResponse{
+		OriginalMimeType: "video/x-matroska",
+		OriginalFileName: "clip.mkv",
+		People:           []PersonResponse{{ID: "p1", Name: "Alice"}},
+	}
 	if HasFaceRegionsToEmbed(asset) {
-		t.Fatal("videos cannot hold XMP-mwg-rs regions; must be excluded even with named people")
+		t.Fatal("containers exiftool cannot write must be excluded even with named people")
 	}
 }
 

src/process/faceOrientation_test.go

@@ -0,0 +1,47 @@
+package process
+
+import (
+	"testing"
+
+	"github.com/majorfi/immich-exif/exif"
+	"github.com/majorfi/immich-exif/model"
+)
+
+func TestVideoRotationToOrientation(t *testing.T) {
+	cases := []struct {
+		rotation int
+		want     int
+		ok       bool
+	}{
+		{0, 1, true},
+		{90, 6, true},
+		{270, 8, true},
+		{180, 0, false}, // Immich does not re-orient 180 video regions
+		{45, 0, false},  // non-cardinal
+	}
+	for _, c := range cases {
+		got, ok := videoRotationToOrientation(c.rotation)
+		if got != c.want || ok != c.ok {
+			t.Fatalf("rotation %d: got (%d,%v), want (%d,%v)", c.rotation, got, ok, c.want, c.ok)
+		}
+	}
+}
+
+func TestRegionOrientationImageUsesExifOrientation(t *testing.T) {
+	asset := model.AssetResponse{OriginalMimeType: "image/jpeg"}
+	existing := exif.ExifTagMap{"Orientation": float64(6)}
+	got, ok := regionOrientation(asset, existing)
+	if !ok || got != 6 {
+		t.Fatalf("image should use EXIF Orientation 6, got (%d,%v)", got, ok)
+	}
+}
+
+func TestRegionOrientationVideoUsesRotation(t *testing.T) {
+	asset := model.AssetResponse{OriginalMimeType: "video/mp4"}
+	if got, ok := regionOrientation(asset, exif.ExifTagMap{"Rotation": float64(90)}); !ok || got != 6 {
+		t.Fatalf("video rotation 90 should map to orientation 6, got (%d,%v)", got, ok)
+	}
+	if _, ok := regionOrientation(asset, exif.ExifTagMap{"Rotation": float64(180)}); ok {
+		t.Fatal("video rotation 180 must report not-anchorable")
+	}
+}