diff --git a/internal/api/download_album.go b/internal/api/download_album.go index 43f43a92b..addcaf7fc 100644 --- a/internal/api/download_album.go +++ b/internal/api/download_album.go @@ -65,6 +65,9 @@ func DownloadAlbum(router *gin.RouterGroup) { if file.FileHash == "" { log.Warnf("download: empty file hash, skipped %s", sanitize.Log(file.FileName)) continue + } else if file.FileName == "" { + log.Warnf("download: empty file name, skipped %s", sanitize.Log(file.FileUID)) + continue } if file.FileSidecar { @@ -89,13 +92,14 @@ func DownloadAlbum(router *gin.RouterGroup) { if fs.FileExists(fileName) { if err := addFileToZip(zipWriter, fileName, alias); err != nil { - log.Error(err) + log.Errorf("download: failed adding %s to album zip (%s)", sanitize.Log(file.FileName), err) Abort(c, http.StatusInternalServerError, i18n.ErrZipFailed) return } + log.Infof("download: added %s as %s", sanitize.Log(file.FileName), sanitize.Log(alias)) } else { - log.Errorf("download: failed finding %s", sanitize.Log(file.FileName)) + log.Warnf("download: album file %s is missing", sanitize.Log(file.FileName)) } } diff --git a/internal/api/download_zip.go b/internal/api/download_zip.go index d65791940..1350a11db 100644 --- a/internal/api/download_zip.go +++ b/internal/api/download_zip.go @@ -100,6 +100,9 @@ func CreateZip(router *gin.RouterGroup) { if file.FileHash == "" { log.Warnf("download: empty file hash, skipped %s", sanitize.Log(file.FileName)) continue + } else if file.FileName == "" { + log.Warnf("download: empty file name, skipped %s", sanitize.Log(file.FileUID)) + continue } if file.FileSidecar { @@ -124,12 +127,14 @@ func CreateZip(router *gin.RouterGroup) { if fs.FileExists(fileName) { if err := addFileToZip(zipWriter, fileName, alias); err != nil { - Error(c, http.StatusInternalServerError, err, i18n.ErrZipFailed) + log.Errorf("download: failed adding %s to zip (%s)", sanitize.Log(file.FileName), err) + Abort(c, http.StatusInternalServerError, i18n.ErrZipFailed) return } + log.Infof("download: added %s as %s", sanitize.Log(file.FileName), sanitize.Log(alias)) } else { - log.Warnf("download: file %s is missing", sanitize.Log(file.FileName)) + log.Warnf("download: media file %s is missing", sanitize.Log(file.FileName)) logError("download", file.Update("FileMissing", true)) } } diff --git a/internal/entity/face.go b/internal/entity/face.go index ed67592a0..6e5c83174 100644 --- a/internal/entity/face.go +++ b/internal/entity/face.go @@ -55,6 +55,19 @@ func NewFace(subjUID, faceSrc string, embeddings face.Embeddings) *Face { return result } +// MatchId returns a compound id for matching. +func (m *Face) MatchId(f Face) string { + if m.ID == "" || f.ID == "" { + return "" + } + + if m.ID < f.ID { + return fmt.Sprintf("%s-%s", m.ID, f.ID) + } else { + return fmt.Sprintf("%s-%s", f.ID, m.ID) + } +} + // SkipMatching checks whether the face should be skipped when matching. func (m *Face) SkipMatching() bool { return m.Embedding().SkipMatching() @@ -171,12 +184,12 @@ func (m *Face) ResolveCollision(embeddings face.Embeddings) (resolved bool, err return false, fmt.Errorf("collision distance must be positive") } else if dist < 0.02 { // Ignore if distance is very small as faces may belong to the same person. - log.Warnf("face %s: clearing ambiguous subject %s, similar face at dist %f with source %s", m.ID, SubjNames.Log(m.SubjUID), dist, SrcString(m.FaceSrc)) + log.Warnf("faces: clearing ambiguous subject %s from face %s, similar face at dist %f with source %s", SubjNames.Log(m.SubjUID), m.ID, dist, SrcString(m.FaceSrc)) // Reset subject UID just in case. m.SubjUID = "" - return false, m.Updates(Values{"SubjUID": m.SubjUID}) + return true, m.Updates(Values{"SubjUID": m.SubjUID}) } else { m.MatchedAt = nil m.Collisions++ diff --git a/internal/entity/face_test.go b/internal/entity/face_test.go index 77b24a015..33676937b 100644 --- a/internal/entity/face_test.go +++ b/internal/entity/face_test.go @@ -141,6 +141,18 @@ func TestNewFace(t *testing.T) { }) } +func TestFace_MatchId(t *testing.T) { + t.Run("A123-B456", func(t *testing.T) { + f1 := Face{ID: "A123"} + f2 := Face{ID: "B456"} + f3 := Face{ID: ""} + + assert.Equal(t, "A123-B456", f1.MatchId(f2)) + assert.Equal(t, "A123-B456", f2.MatchId(f1)) + assert.Equal(t, "", f3.MatchId(f1)) + }) +} + func TestFace_Unsuitable(t *testing.T) { t.Run("True", func(t *testing.T) { m := FaceFixtures.Get("joe-biden") diff --git a/internal/entity/marker.go b/internal/entity/marker.go index 4e0fa48d2..57a82a1a0 100644 --- a/internal/entity/marker.go +++ b/internal/entity/marker.go @@ -133,8 +133,8 @@ func (m *Marker) UpdateFile(file *File) (updated bool) { if !updated || m.MarkerUID == "" { return false - } else if res := UnscopedDb().Model(m).UpdateColumns(Values{"file_uid": m.FileUID, "thumb": m.Thumb}); res.Error != nil { - log.Errorf("marker %s: %s (set file)", m.MarkerUID, res.Error) + } else if err := UnscopedDb().Model(m).UpdateColumns(Values{"file_uid": m.FileUID, "thumb": m.Thumb}).Error; err != nil { + log.Errorf("faces: failed assigning marker %s to file %s (%s)", m.MarkerUID, m.FileUID, err) return false } else { return true @@ -232,7 +232,7 @@ func (m *Marker) SetFace(f *Face, dist float64) (updated bool, err error) { } else if reported, err := f.ResolveCollision(m.Embeddings()); err != nil { return false, err } else if reported { - log.Warnf("marker %s: face %s has ambiguous subjects %s <> %s, subject source %s", sanitize.Log(m.MarkerUID), sanitize.Log(f.ID), sanitize.Log(m.SubjUID), sanitize.Log(f.SubjUID), SrcString(m.SubjSrc)) + log.Warnf("faces: marker %s face %s has ambiguous subjects %s <> %s, subject source %s", sanitize.Log(m.MarkerUID), sanitize.Log(f.ID), sanitize.Log(m.SubjUID), sanitize.Log(f.SubjUID), SrcString(m.SubjSrc)) return false, nil } else { return false, nil @@ -428,10 +428,10 @@ func (m *Marker) Subject() (subj *Subject) { // Create subject? if m.SubjSrc != SrcAuto && m.MarkerName != "" && m.SubjUID == "" { if subj = NewSubject(m.MarkerName, SubjPerson, m.SubjSrc); subj == nil { - log.Errorf("marker %s: invalid subject %s", sanitize.Log(m.MarkerUID), sanitize.Log(m.MarkerName)) + log.Errorf("faces: marker %s has invalid subject %s", sanitize.Log(m.MarkerUID), sanitize.Log(m.MarkerName)) return nil } else if subj = FirstOrCreateSubject(subj); subj == nil { - log.Debugf("marker %s: invalid subject %s", sanitize.Log(m.MarkerUID), sanitize.Log(m.MarkerName)) + log.Debugf("faces: marker %s has invalid subject %s", sanitize.Log(m.MarkerUID), sanitize.Log(m.MarkerName)) return nil } else { m.subject = subj @@ -457,9 +457,9 @@ func (m *Marker) ClearSubject(src string) error { // Find and (soft) delete unused subjects. start := time.Now() if count, err := DeleteOrphanPeople(); err != nil { - log.Errorf("marker %s: %s while removing unused subjects [%s]", sanitize.Log(m.MarkerUID), err, time.Since(start)) + log.Errorf("faces: %s while clearing subject of marker %s [%s]", err, sanitize.Log(m.MarkerUID), time.Since(start)) } else if count > 0 { - log.Debugf("marker %s: removed %s [%s]", sanitize.Log(m.MarkerUID), english.Plural(count, "person", "people"), time.Since(start)) + log.Debugf("faces: %s marked as missing while clearing subject of marker %s [%s]", english.Plural(count, "person", "people"), sanitize.Log(m.MarkerUID), time.Since(start)) } }() @@ -472,7 +472,7 @@ func (m *Marker) ClearSubject(src string) error { } else if resolved, err := m.face.ResolveCollision(m.Embeddings()); err != nil { return err } else if resolved { - log.Debugf("marker %s: resolved ambiguous subjects for face %s", sanitize.Log(m.MarkerUID), sanitize.Log(m.face.ID)) + log.Debugf("faces: marker %s resolved ambiguous subjects for face %s", sanitize.Log(m.MarkerUID), sanitize.Log(m.face.ID)) } // Clear references. @@ -498,7 +498,7 @@ func (m *Marker) Face() (f *Face) { // Add face if size if m.SubjSrc != SrcAuto && m.FaceID == "" { if m.Size < face.ClusterSizeThreshold || m.Score < face.ClusterScoreThreshold { - log.Debugf("marker %s: skipped adding face due to low-quality (size %d, score %d)", sanitize.Log(m.MarkerUID), m.Size, m.Score) + log.Debugf("faces: marker %s skipped adding face due to low-quality (size %d, score %d)", sanitize.Log(m.MarkerUID), m.Size, m.Score) return nil } diff --git a/internal/entity/string_map.go b/internal/entity/string_map.go index d9a18a82b..7b3078572 100644 --- a/internal/entity/string_map.go +++ b/internal/entity/string_map.go @@ -58,6 +58,10 @@ func (s *StringMap) Key(val string) string { // Log returns a string sanitized for logging and using the key as fallback value. func (s *StringMap) Log(key string) (val string) { + if key == "" { + return "" + } + if val = s.Get(key); val != "" { return sanitize.Log(val) } else { diff --git a/internal/entity/subject.go b/internal/entity/subject.go index 583796842..5cc07dd7c 100644 --- a/internal/entity/subject.go +++ b/internal/entity/subject.go @@ -118,8 +118,6 @@ func (m *Subject) Delete() error { subjectMutex.Lock() defer subjectMutex.Unlock() - log.Infof("subject: deleting %s %s", TypeString(m.SubjType), sanitize.Log(m.SubjName)) - event.EntitiesDeleted("subjects", []string{m.SubjUID}) if m.IsPerson() { @@ -133,6 +131,8 @@ func (m *Subject) Delete() error { return err } + log.Infof("subject: marked %s %s as missing", TypeString(m.SubjType), sanitize.Log(m.SubjName)) + return Db().Delete(m).Error } @@ -230,28 +230,37 @@ func FindSubject(uid string) *Subject { } // FindSubjectByName find an existing subject by name. -func FindSubjectByName(name string) (result *Subject) { +func FindSubjectByName(name string) *Subject { name = sanitize.Name(name) if name == "" { return nil } + result := Subject{} uid := SubjNames.Key(name) - if uid == "" { - return nil + switch uid { + case "": + if err := UnscopedDb().Where("subj_name LIKE ?", name).First(&result).Error; err != nil { + log.Debugf("subject: %s not found by name", sanitize.Log(name)) + return nil + } + default: + if found := FindSubject(uid); found == nil { + log.Debugf("subject: %s not found by uid", sanitize.Log(name)) + return nil + } else { + result = *found + } } - // Restore if currently deleted. - if result = FindSubject(uid); result == nil { - log.Debugf("subject: could not find %s", sanitize.Log(result.SubjName)) - return nil - } else if !result.Deleted() { - return result + // Restore if flagged as deleted. + if !result.Deleted() { + return &result } else if err := result.Restore(); err == nil { log.Debugf("subject: restored %s", sanitize.Log(result.SubjName)) - return result + return &result } else { log.Errorf("subject: failed restoring %s (%s)", sanitize.Log(result.SubjName), err) } diff --git a/internal/photoprism/faces_audit.go b/internal/photoprism/faces_audit.go index f95d41a23..6d858d1b7 100644 --- a/internal/photoprism/faces_audit.go +++ b/internal/photoprism/faces_audit.go @@ -24,48 +24,70 @@ func (w *Faces) Audit(fix bool) (err error) { } if n := len(subj); n == 0 { - log.Infof("found no subjects") + log.Infof("faces: found no subjects") } else { - log.Infof("found %s", english.Plural(n, "subject", "subjects")) + log.Infof("faces: found %s", english.Plural(n, "subject", "subjects")) } // Fix non-existent marker subjects references? if n := len(invalidSubj); n == 0 { - log.Infof("found no invalid marker subjects") + log.Infof("faces: found no invalid marker subjects") } else if !fix { - log.Infof("%s with non-existent subjects", english.Plural(n, "marker", "markers")) + log.Infof("faces: %s with non-existent subjects", english.Plural(n, "marker", "markers")) } else if removed, err := query.RemoveNonExistentMarkerSubjects(); err != nil { log.Errorf("faces: %s (remove orphan subjects)", err) } else if removed > 0 { - log.Infof("removed %d / %d markers with non-existent subjects", removed, n) + log.Infof("faces: removed %d / %d markers with non-existent subjects", removed, n) } // Fix non-existent marker face references? if n := len(invalidFaces); n == 0 { - log.Infof("found no invalid marker faces") + log.Infof("faces: found no invalid marker faces") } else if !fix { - log.Infof("%s with non-existent faces", english.Plural(n, "marker", "markers")) + log.Infof("faces: %s with non-existent faces", english.Plural(n, "marker", "markers")) } else if removed, err := query.RemoveNonExistentMarkerFaces(); err != nil { log.Errorf("faces: %s (remove orphan embeddings)", err) } else if removed > 0 { - log.Infof("removed %d / %d markers with non-existent faces", removed, n) + log.Infof("faces: removed %d / %d markers with non-existent faces", removed, n) } conflicts := 0 resolved := 0 - faces, err := query.Faces(true, false, false) + faces, ids, err := query.FacesByID(true, false, false) if err != nil { return err } - faceMap := make(map[string]entity.Face) + // Remembers matched combinations. + done := make(map[string]bool, len(ids)*len(ids)) - for _, f1 := range faces { - faceMap[f1.ID] = f1 + // Find face assignment collisions. + for _, i := range ids { + for _, j := range ids { + var f1, f2 entity.Face - for _, f2 := range faces { + if f, ok := faces[i]; ok { + f1 = f + } else { + continue + } + + if f, ok := faces[j]; ok { + f2 = f + } else { + continue + } + + var matchId string + + // Skip? + if matchId = f1.MatchId(f2); matchId == "" || done[matchId] { + continue + } + + // Compare face 1 with face 2. if matched, dist := f1.Match(face.Embeddings{f2.Embedding()}); matched { if f1.SubjUID == f2.SubjUID { continue @@ -75,74 +97,102 @@ func (w *Faces) Audit(fix bool) (err error) { r := f1.SampleRadius + face.MatchDist - log.Infof("face %s: ambiguous subject at dist %f, Ø %f from %d samples, collision Ø %f", f1.ID, dist, r, f1.Samples, f1.CollisionRadius) + log.Infof("faces: face %s has ambiguous subject at dist %f, Ø %f from %d samples, collision Ø %f", f1.ID, dist, r, f1.Samples, f1.CollisionRadius) if f1.SubjUID != "" { - log.Infof("face %s: subject %s (%s %s)", f1.ID, entity.SubjNames.Log(f1.SubjUID), f1.SubjUID, entity.SrcString(f1.FaceSrc)) + log.Infof("faces: face %s belongs to subject %s (%s %s)", f1.ID, entity.SubjNames.Log(f1.SubjUID), f1.SubjUID, entity.SrcString(f1.FaceSrc)) } else { - log.Infof("face %s: has no subject (%s)", f1.ID, entity.SrcString(f1.FaceSrc)) + log.Infof("faces: face %s has no subject assigned (%s)", f1.ID, entity.SrcString(f1.FaceSrc)) } if f2.SubjUID != "" { - log.Infof("face %s: subject %s (%s %s)", f2.ID, entity.SubjNames.Log(f2.SubjUID), f2.SubjUID, entity.SrcString(f2.FaceSrc)) + log.Infof("faces: face %s belongs to subject %s (%s %s)", f2.ID, entity.SubjNames.Log(f2.SubjUID), f2.SubjUID, entity.SrcString(f2.FaceSrc)) } else { - log.Infof("face %s: has no subject (%s)", f2.ID, entity.SrcString(f2.FaceSrc)) + log.Infof("faces: face %s has no subject assigned (%s)", f2.ID, entity.SrcString(f2.FaceSrc)) } + // Skip fix? if !fix { - // Do nothing. - } else if ok, err := f1.ResolveCollision(face.Embeddings{f2.Embedding()}); err != nil { - log.Errorf("conflict resolution for %s failed, face id %s has collisions with other persons (%s)", entity.SubjNames.Log(f1.SubjUID), f1.ID, err) - } else if ok { - log.Infof("successful conflict resolution for %s, face id %s had collisions with other persons", entity.SubjNames.Log(f1.SubjUID), f1.ID) - resolved++ - } else { - log.Infof("conflict resolution for %s not successful, face id %s still has collisions with other persons", entity.SubjNames.Log(f1.SubjUID), f1.ID) + continue } + + // Resolve. + success, failed := f1.ResolveCollision(face.Embeddings{f2.Embedding()}) + + // Failed? + if failed != nil { + log.Errorf("faces: conflict resolution for %s failed, face %s has collisions with other persons (%s)", entity.SubjNames.Log(f1.SubjUID), f1.ID, failed) + continue + } + + // Success? + if success { + log.Infof("faces: successful conflict resolution for %s, face %s had collisions with other persons", entity.SubjNames.Log(f1.SubjUID), f1.ID) + resolved++ + faces, _, err = query.FacesByID(true, false, false) + logErr("faces", "refresh", err) + } else { + log.Infof("faces: conflict resolution for %s not successful, face %s still has collisions with other persons", entity.SubjNames.Log(f1.SubjUID), f1.ID) + } + + done[matchId] = true } } } + // Show conflict resolution results. if conflicts == 0 { - log.Infof("found no ambiguous subjects") + log.Infof("faces: found no ambiguous subjects") } else if !fix { - log.Infof("%s", english.Plural(conflicts, "ambiguous subject", "ambiguous subjects")) + log.Infof("faces: found %s", english.Plural(conflicts, "ambiguous subject", "ambiguous subjects")) } else { - log.Infof("%s, %d resolved", english.Plural(conflicts, "ambiguous subject", "ambiguous subjects"), resolved) + log.Infof("faces: found %s, %d resolved", english.Plural(conflicts, "ambiguous subject", "ambiguous subjects"), resolved) } + // Show remaining issues. if markers, err := query.MarkersWithSubjectConflict(); err != nil { - log.Errorf("faces: %s (find marker conflicts)", err) + logErr("faces", "find marker conflicts", err) } else { for _, m := range markers { - log.Infof("marker %s: %s subject %s conflicts with face %s subject %s", m.MarkerUID, entity.SrcString(m.SubjSrc), sanitize.Log(subj[m.SubjUID].SubjName), m.FaceID, sanitize.Log(subj[faceMap[m.FaceID].SubjUID].SubjName)) + if m.FaceID == "" { + log.Warnf("faces: marker %s has an empty face id - possible bug", m.MarkerUID) + } else if f, ok := faces[m.FaceID]; !ok { + log.Warnf("faces: marker %s has invalid face %s of subject %s (%s)", m.MarkerUID, m.FaceID, entity.SubjNames.Log(f.SubjUID), f.SubjUID) + } else if m.SubjUID != "" { + log.Infof("faces: marker %s with %s subject %s (%s) conflicts with face %s (%s) of subject %s (%s)", m.MarkerUID, entity.SrcString(m.SubjSrc), entity.SubjNames.Log(m.SubjUID), m.SubjUID, m.FaceID, entity.SrcString(f.FaceSrc), entity.SubjNames.Log(f.SubjUID), f.SubjUID) + } else if m.MarkerName != "" { + log.Infof("faces: marker %s with %s subject name %s conflicts with face %s (%s) of subject %s (%s)", m.MarkerUID, entity.SrcString(m.SubjSrc), sanitize.Log(m.MarkerName), m.FaceID, entity.SrcString(f.FaceSrc), entity.SubjNames.Log(f.SubjUID), f.SubjUID) + } else { + log.Infof("faces: marker %s with unknown subject (%s) conflicts with face %s (%s) of subject %s (%s)", m.MarkerUID, entity.SrcString(m.SubjSrc), m.FaceID, entity.SrcString(f.FaceSrc), entity.SubjNames.Log(f.SubjUID), f.SubjUID) + } + } } // Find and fix orphan face clusters. if orphans, err := entity.OrphanFaces(); err != nil { - log.Errorf("%s while finding orphan face clusters", err) + log.Errorf("faces: %s while finding orphan face clusters", err) } else if l := len(orphans); l == 0 { - log.Infof("found no orphan face clusters") + log.Infof("faces: found no orphan face clusters") } else if !fix { - log.Infof("found %s", english.Plural(l, "orphan face cluster", "orphan face clusters")) + log.Infof("faces: found %s", english.Plural(l, "orphan face cluster", "orphan face clusters")) } else if err := orphans.Delete(); err != nil { - log.Errorf("failed removing %s: %s", english.Plural(l, "orphan face cluster", "orphan face clusters"), err) + log.Errorf("faces: failed removing %s: %s", english.Plural(l, "orphan face cluster", "orphan face clusters"), err) } else { - log.Infof("removed %s", english.Plural(l, "orphan face cluster", "orphan face clusters")) + log.Infof("faces: removed %s", english.Plural(l, "orphan face cluster", "orphan face clusters")) } // Find and fix orphan people. if orphans, err := entity.OrphanPeople(); err != nil { - log.Errorf("%s while finding orphan people", err) + log.Errorf("faces: %s while finding orphan people", err) } else if l := len(orphans); l == 0 { - log.Infof("found no orphan people") + log.Infof("faces: found no orphan people") } else if !fix { - log.Infof("found %s", english.Plural(l, "orphan person", "orphan people")) + log.Infof("faces: found %s", english.Plural(l, "orphan person", "orphan people")) } else if err := orphans.Delete(); err != nil { - log.Errorf("failed fixing %s: %s", english.Plural(l, "orphan person", "orphan people"), err) + log.Errorf("faces: failed fixing %s: %s", english.Plural(l, "orphan person", "orphan people"), err) } else { - log.Infof("removed %s", english.Plural(l, "orphan person", "orphan people")) + log.Infof("faces: removed %s", english.Plural(l, "orphan person", "orphan people")) } return nil diff --git a/internal/photoprism/photoprism.go b/internal/photoprism/photoprism.go index 7d216b6a7..a1e288270 100644 --- a/internal/photoprism/photoprism.go +++ b/internal/photoprism/photoprism.go @@ -34,8 +34,16 @@ var log = event.Log type S []string +// logWarn logs an error as warning and keeps quiet otherwise. func logWarn(prefix string, err error) { if err != nil { log.Warnf("%s: %s", prefix, err.Error()) } } + +// logErr logs an error and keeps quiet otherwise. +func logErr(prefix, action string, err error) { + if err != nil { + log.Errorf("%s: %s (%s)", prefix, err, action) + } +} diff --git a/internal/query/faces.go b/internal/query/faces.go index 594f54df8..37705ebaa 100644 --- a/internal/query/faces.go +++ b/internal/query/faces.go @@ -9,21 +9,73 @@ import ( "github.com/photoprism/photoprism/pkg/sanitize" ) -// Faces returns all (known / unmatched) faces from the index. -func Faces(knownOnly, unmatched, hidden bool) (result entity.Faces, err error) { - stmt := Db() +type IDs []string +type FaceMap map[string]entity.Face - if unmatched { - stmt = stmt.Where("matched_at IS NULL") +// IDs returns all known face ids as slice. +func (m FaceMap) IDs() (ids IDs) { + ids = make(IDs, len(m)) + + for id := range m { + ids = append(ids, id) } + return ids +} + +// Refresh updates the map with current entity values from the database. +func (m FaceMap) Refresh() (err error) { + result := entity.Faces{} + + ids := m.IDs() + + if err = Db().Where("id IN (?)", ids).Find(&result).Error; err != nil { + return err + } + + for _, f := range result { + m[f.ID] = f + } + + return nil +} + +// FacesByID retrieves faces from the database and returns a map with the Face ID as key. +func FacesByID(knownOnly, unmatchedOnly, inclHidden bool) (FaceMap, IDs, error) { + faces, err := Faces(knownOnly, unmatchedOnly, inclHidden) + + if err != nil { + return nil, nil, err + } + + faceIds := make(IDs, len(faces)) + faceMap := make(FaceMap, len(faces)) + + for i, f := range faces { + faceMap[f.ID] = f + faceIds[i] = f.ID + } + + return faceMap, faceIds, nil +} + +// Faces returns all (known / unmatched) faces from the index. +func Faces(knownOnly, unmatchedOnly, inclHidden bool) (result entity.Faces, err error) { + stmt := Db() + if knownOnly { stmt = stmt.Where("subj_uid <> ''") } - err = stmt.Where("face_hidden = ?", hidden). - Order("subj_uid, samples DESC"). - Find(&result).Error + if unmatchedOnly { + stmt = stmt.Where("matched_at IS NULL") + } + + if !inclHidden { + stmt = stmt.Where("face_hidden = ?", false) + } + + err = stmt.Order("subj_uid, samples DESC").Find(&result).Error return result, err } @@ -65,19 +117,19 @@ func MatchFaceMarkers() (affected int64, err error) { } // RemoveAnonymousFaceClusters removes anonymous faces from the index. -func RemoveAnonymousFaceClusters() (removed int64, err error) { +func RemoveAnonymousFaceClusters() (removed int, err error) { res := UnscopedDb(). Delete(entity.Face{}, "subj_uid = '' AND face_src = ?", entity.SrcAuto) - return res.RowsAffected, res.Error + return int(res.RowsAffected), res.Error } // RemoveAutoFaceClusters removes automatically added face clusters from the index. -func RemoveAutoFaceClusters() (removed int64, err error) { +func RemoveAutoFaceClusters() (removed int, err error) { res := UnscopedDb(). Delete(entity.Face{}, "face_src = ?", entity.SrcAuto) - return res.RowsAffected, res.Error + return int(res.RowsAffected), res.Error } // CountNewFaceMarkers counts the number of new face markers in the index. @@ -166,14 +218,40 @@ func MergeFaces(merge entity.Faces) (merged *entity.Face, err error) { // ResolveFaceCollisions resolves collisions of different subject's faces. func ResolveFaceCollisions() (conflicts, resolved int, err error) { - faces, err := Faces(true, false, false) + faces, ids, err := FacesByID(true, false, false) if err != nil { return conflicts, resolved, err } - for _, f1 := range faces { - for _, f2 := range faces { + // Remembers matched combinations. + done := make(map[string]bool, len(ids)*len(ids)) + + // Find face assignment collisions. + for _, i := range ids { + for _, j := range ids { + var f1, f2 entity.Face + + if f, ok := faces[i]; ok { + f1 = f + } else { + continue + } + + if f, ok := faces[j]; ok { + f2 = f + } else { + continue + } + + var matchId string + + // Skip? + if matchId = f1.MatchId(f2); matchId == "" || done[matchId] { + continue + } + + // Compare face 1 with face 2. if matched, dist := f1.Match(face.Embeddings{f2.Embedding()}); matched { if f1.SubjUID == f2.SubjUID { continue @@ -183,28 +261,40 @@ func ResolveFaceCollisions() (conflicts, resolved int, err error) { r := f1.SampleRadius + face.MatchDist - log.Infof("face %s: ambiguous subject at dist %f, Ø %f from %d samples, collision Ø %f", f1.ID, dist, r, f1.Samples, f1.CollisionRadius) + log.Infof("faces: face %s has ambiguous subject at dist %f, Ø %f from %d samples, collision Ø %f", f1.ID, dist, r, f1.Samples, f1.CollisionRadius) if f1.SubjUID != "" { - log.Debugf("face %s: subject %s (%s %s)", f1.ID, entity.SubjNames.Log(f1.SubjUID), f1.SubjUID, entity.SrcString(f1.FaceSrc)) + log.Debugf("faces: face %s has %s subject %s (%s)", f1.ID, entity.SrcString(f1.FaceSrc), entity.SubjNames.Log(f1.SubjUID), f1.SubjUID) } else { - log.Debugf("face %s: has no subject (%s)", f1.ID, entity.SrcString(f1.FaceSrc)) + log.Debugf("faces: face %s has unknown subject (%s)", f1.ID, entity.SrcString(f1.FaceSrc)) } if f2.SubjUID != "" { - log.Debugf("face %s: subject %s (%s %s)", f2.ID, entity.SubjNames.Log(f2.SubjUID), f2.SubjUID, entity.SrcString(f2.FaceSrc)) + log.Debugf("faces: face %s has %s subject %s (%s)", f2.ID, entity.SrcString(f2.FaceSrc), entity.SubjNames.Log(f2.SubjUID), f2.SubjUID) } else { - log.Debugf("face %s: has no subject (%s)", f2.ID, entity.SrcString(f2.FaceSrc)) + log.Debugf("faces: face %s has unknown subject (%s)", f2.ID, entity.SrcString(f2.FaceSrc)) } - if ok, err := f1.ResolveCollision(face.Embeddings{f2.Embedding()}); err != nil { - log.Errorf("face %s: %s", f1.ID, err) - } else if ok { - log.Infof("face %s: conflict has been resolved", f1.ID) - resolved++ - } else { - log.Debugf("face %s: conflict could not be resolved", f1.ID) + // Resolve. + success, failed := f1.ResolveCollision(face.Embeddings{f2.Embedding()}) + + // Failed? + if failed != nil { + log.Errorf("faces: conflict resolution for %s failed, face %s has collisions with other persons (%s)", entity.SubjNames.Log(f1.SubjUID), f1.ID, failed) + continue } + + // Success? + if success { + log.Infof("faces: successful conflict resolution for %s, face %s had collisions with other persons", entity.SubjNames.Log(f1.SubjUID), f1.ID) + resolved++ + faces, _, err = FacesByID(true, false, false) + logErr("faces", "refresh", err) + } else { + log.Infof("faces: conflict resolution for %s not successful, face %s still has collisions with other persons", entity.SubjNames.Log(f1.SubjUID), f1.ID) + } + + done[matchId] = true } } } diff --git a/internal/query/faces_test.go b/internal/query/faces_test.go index a5a0d7bcd..4a0cf37a9 100644 --- a/internal/query/faces_test.go +++ b/internal/query/faces_test.go @@ -31,7 +31,7 @@ func TestFaces(t *testing.T) { t.Fatal(err) } - assert.Empty(t, results) + assert.GreaterOrEqual(t, len(results), 1) }) t.Run("unmatched", func(t *testing.T) { @@ -117,7 +117,7 @@ func TestRemoveAnonymousFaceClusters(t *testing.T) { t.Fatal(err) } - assert.Equal(t, int64(2), removed) + assert.Equal(t, 2, removed) } func TestCountNewFaceMarkers(t *testing.T) { @@ -220,8 +220,8 @@ func TestResolveFaceCollisions(t *testing.T) { t.Fatal(err) } - assert.LessOrEqual(t, 3, c) - assert.LessOrEqual(t, 3, r) + assert.LessOrEqual(t, 1, c) + assert.LessOrEqual(t, 1, r) } func TestRemoveAutoFaceClusters(t *testing.T) { @@ -231,5 +231,5 @@ func TestRemoveAutoFaceClusters(t *testing.T) { t.Fatal(err) } - assert.Equal(t, int64(3), removed) + assert.LessOrEqual(t, 3, removed) } diff --git a/internal/query/markers.go b/internal/query/markers.go index ebbcbdd72..8586988a7 100644 --- a/internal/query/markers.go +++ b/internal/query/markers.go @@ -4,6 +4,8 @@ import ( "fmt" "time" + "github.com/jinzhu/gorm" + "github.com/photoprism/photoprism/internal/entity" "github.com/photoprism/photoprism/internal/face" ) @@ -197,8 +199,12 @@ func MarkersWithNonExistentReferences() (faces entity.Markers, subjects entity.M // MarkersWithSubjectConflict finds markers with conflicting subjects. func MarkersWithSubjectConflict() (results entity.Markers, err error) { + faces := gorm.Expr(entity.Face{}.TableName()) + markers := gorm.Expr(entity.Marker{}.TableName()) + hasSubj := gorm.Expr("(?.subj_uid <> '' OR ?.subj_src <> '')", markers, markers) + err = Db(). - Joins(fmt.Sprintf("JOIN %s f ON f.id = face_id AND f.subj_uid <> %s.subj_uid", entity.Face{}.TableName(), entity.Marker{}.TableName())). + Joins("JOIN ? f ON f.id = face_id AND f.subj_uid <> ?.subj_uid AND ?", faces, markers, hasSubj). Order("face_id"). Find(&results).Error diff --git a/internal/query/query.go b/internal/query/query.go index 43a624ede..3241c3988 100644 --- a/internal/query/query.go +++ b/internal/query/query.go @@ -75,3 +75,10 @@ func UnscopedDb() *gorm.DB { func DbDialect() string { return Db().Dialect().GetName() } + +// logErr logs an error and keeps quiet otherwise. +func logErr(prefix, action string, err error) { + if err != nil { + log.Errorf("%s: %s (%s)", prefix, err, action) + } +} diff --git a/pkg/sanitize/name.go b/pkg/sanitize/name.go index fca883e26..ea94a0d13 100644 --- a/pkg/sanitize/name.go +++ b/pkg/sanitize/name.go @@ -15,7 +15,7 @@ func Name(name string) string { // Remove double quotes and other special characters. name = strings.Map(func(r rune) rune { switch r { - case '"', '`', '~', '\\', '/', '*', '%', '&', '|', '+', '=', '$', '@', '!', '?', ':', ';', '<', '>', '{', '}': + case '"', '`', '~', '\\', '/', '*', '%', '_', '&', '|', '+', '=', '$', '@', '!', '?', ':', ';', '<', '>', '{', '}': return -1 } return r