diff --git a/frontend/src/dialog/photo/people.vue b/frontend/src/dialog/photo/people.vue index 7906f756e..82ad56683 100644 --- a/frontend/src/dialog/photo/people.vue +++ b/frontend/src/dialog/photo/people.vue @@ -31,7 +31,7 @@ - @@ -39,7 +39,7 @@ - @@ -52,6 +52,7 @@ this.busy = false); }, confirm(marker) { marker.Score = 100; marker.Invalid = false; - this.model.updateMarker(marker); + this.busy = true; + this.model.updateMarker(marker).finally(() => this.busy = false); }, clearSubject(marker) { - this.model.clearMarkerSubject(marker); + this.busy = true; + this.model.clearMarkerSubject(marker).finally(() => this.busy = false); }, updateName(marker) { // Don't save empty name. @@ -150,8 +155,8 @@ export default { } marker.SubjectSrc = src.Manual; - - this.model.updateMarker(marker); + this.busy = true; + this.model.updateMarker(marker).finally(() => this.busy = false); }, }, }; diff --git a/frontend/src/model/photo.js b/frontend/src/model/photo.js index feaf287fd..97804275f 100644 --- a/frontend/src/model/photo.js +++ b/frontend/src/model/photo.js @@ -848,8 +848,6 @@ export class Photo extends RestModel { return Promise.reject("invalid marker id"); } - marker.MarkerSrc = src.Manual; - const file = this.mainFile(); if (!file || !file.UID) { diff --git a/internal/entity/marker.go b/internal/entity/marker.go index a816284a9..be42c5588 100644 --- a/internal/entity/marker.go +++ b/internal/entity/marker.go @@ -3,13 +3,12 @@ package entity import ( "encoding/json" "fmt" + "strings" "time" + "github.com/photoprism/photoprism/internal/face" "github.com/photoprism/photoprism/internal/form" "github.com/photoprism/photoprism/pkg/txt" - "github.com/ulule/deepcopier" - - "github.com/photoprism/photoprism/internal/face" ) const ( @@ -92,19 +91,34 @@ func (m *Marker) Update(attr string, value interface{}) error { // SaveForm updates the entity using form data and stores it in the database. func (m *Marker) SaveForm(f form.Marker) error { - if err := deepcopier.Copy(m).From(f); err != nil { - return err + changed := false + + if m.MarkerInvalid != f.MarkerInvalid { + m.MarkerInvalid = f.MarkerInvalid + changed = true } - if f.MarkerName != "" { - m.MarkerName = txt.Title(txt.Clip(f.MarkerName, txt.ClipKeyword)) + if !m.MarkerInvalid && m.Score < 100 { + m.Score = 100 + changed = true } - if err := m.SyncSubject(true); err != nil { - return err + if f.SubjectSrc == SrcManual && strings.TrimSpace(f.MarkerName) != "" { + m.SubjectSrc = SrcManual + m.MarkerName = txt.Title(txt.Clip(f.MarkerName, txt.ClipDefault)) + + if err := m.SyncSubject(true); err != nil { + return err + } + + changed = true } - return m.Save() + if changed { + return m.Save() + } + + return nil } // SetFace sets a new face for this marker. @@ -118,11 +132,8 @@ func (m *Marker) SetFace(f *Face) (updated bool, err error) { } // Any reason we don't want to set a new face for this marker? - if m.SubjectSrc != SrcManual || f.SubjectUID == m.SubjectUID { + if m.SubjectSrc != SrcManual || f.SubjectUID == "" || m.SubjectUID == "" || f.SubjectUID == m.SubjectUID { // Don't skip if subject wasn't set manually, or subjects match. - } else if f.SubjectUID != "" && m.SubjectUID == "" { - log.Debugf("faces: rejected subject %s for marker %d with unknown subject, source %s", txt.Quote(f.SubjectUID), m.ID, m.SubjectSrc) - return false, nil } else if reported, err := f.ReportCollision(m.Embeddings()); err != nil { return false, err } else if reported { @@ -217,7 +228,7 @@ func (m *Marker) SyncSubject(updateRelated bool) error { Updates(Values{"SubjectUID": m.SubjectUID, "SubjectSrc": SrcAuto}).Error; err != nil { return fmt.Errorf("%s (update related markers)", err) } else { - log.Debugf("marker: matched subject %s with face %s", subj.SubjectName, m.FaceID) + log.Debugf("marker: matched %s with %s", subj.SubjectName, m.FaceID) } return nil diff --git a/internal/entity/subject.go b/internal/entity/subject.go index d1918a289..69acadc66 100644 --- a/internal/entity/subject.go +++ b/internal/entity/subject.go @@ -27,7 +27,7 @@ type Subject struct { SubjectType string `gorm:"type:VARBINARY(8);" json:"Type" yaml:"Type"` SubjectSrc string `gorm:"type:VARBINARY(8);" json:"Src" yaml:"Src"` SubjectSlug string `gorm:"type:VARBINARY(255);index;" json:"Slug" yaml:"-"` - SubjectName string `gorm:"type:VARCHAR(255);index;" json:"Name" yaml:"Name"` + SubjectName string `gorm:"type:VARCHAR(255);unique_index;" json:"Name" yaml:"Name"` SubjectDescription string `gorm:"type:TEXT;" json:"Description" yaml:"Description,omitempty"` SubjectNotes string `gorm:"type:TEXT;" json:"Notes,omitempty" yaml:"Notes,omitempty"` Favorite bool `json:"Favorite" yaml:"Favorite,omitempty"` @@ -44,8 +44,8 @@ type Subject struct { // UnknownPerson can be used as a placeholder for unknown people. var UnknownPerson = Subject{ SubjectUID: "j000000000000000", - SubjectSlug: "unknown", - SubjectName: "Unknown", + SubjectSlug: "", + SubjectName: "", SubjectType: SubjectPerson, SubjectSrc: SrcDefault, Favorite: false, @@ -59,7 +59,7 @@ func CreateUnknownPerson() { // TableName returns the entity database table name. func (Subject) TableName() string { - return "subjects_dev4" + return "subjects_dev5" } // BeforeCreate creates a random UID if needed before inserting a new row to the database. @@ -77,13 +77,14 @@ func NewSubject(name, subjectType, subjectSrc string) *Subject { subjectType = SubjectPerson } - if name == "" { - name = UnknownName - } - subjectName := txt.Title(txt.Clip(name, txt.ClipDefault)) subjectSlug := slug.Make(txt.Clip(name, txt.ClipSlug)) + // Name is required. + if subjectName == "" || subjectSlug == "" { + return nil + } + result := &Subject{ SubjectSlug: subjectSlug, SubjectName: subjectName, @@ -144,7 +145,7 @@ func (m *Subject) Updates(values interface{}) error { func FirstOrCreateSubject(m *Subject) *Subject { result := Subject{} - if err := UnscopedDb().Where("subject_type = ? AND subject_slug = ?", m.SubjectType, m.SubjectSlug).First(&result).Error; err == nil { + if err := UnscopedDb().Where("subject_name LIKE ?", m.SubjectName).First(&result).Error; err == nil { return &result } else if createErr := m.Create(); createErr == nil { if !m.Hidden && m.SubjectType == SubjectPerson { @@ -156,10 +157,10 @@ func FirstOrCreateSubject(m *Subject) *Subject { } return m - } else if err := UnscopedDb().Where("subject_type = ? AND subject_slug = ?", m.SubjectType, m.SubjectSlug).First(&result).Error; err == nil { + } else if err := UnscopedDb().Where("subject_name LIKE ?", m.SubjectName).First(&result).Error; err == nil { return &result } else { - log.Errorf("subject: %s (find or create %s)", createErr, m.SubjectSlug) + log.Errorf("subject: %s while creating %s", createErr, txt.Quote(m.SubjectName)) } return nil @@ -173,13 +174,7 @@ func FindSubject(s string) *Subject { result := Subject{} - db := Db() - - if rnd.IsPPID(s, 'j') { - db = db.Where("subject_uid = ?", s) - } else { - db = db.Where("subject_slug = ?", slug.Make(txt.Clip(s, txt.ClipSlug))) - } + db := Db().Where("subject_uid = ?", s) if err := db.First(&result).Error; err != nil { return nil diff --git a/internal/entity/subject_test.go b/internal/entity/subject_test.go index e7ec0017a..ce4feff79 100644 --- a/internal/entity/subject_test.go +++ b/internal/entity/subject_test.go @@ -122,18 +122,32 @@ func TestSubject_Restore(t *testing.T) { func TestFindSubject(t *testing.T) { t.Run("success", func(t *testing.T) { m := NewSubject("Find Me", SubjectPerson, SrcAuto) - err := m.Save() - if err != nil { + + if err := m.Save(); err != nil { t.Fatal(err) } - found := FindSubject("find me") - assert.Equal(t, "Find Me", found.SubjectName) + + if s := FindSubject(m.SubjectName); s != nil { + t.Fatal("result must be nil") + } + + if s := FindSubject(m.SubjectUID); s != nil { + assert.Equal(t, "Find Me", s.SubjectName) + } else { + t.Fatal("result must not be nil") + } + }) + t.Run("unknown person", func(t *testing.T) { + if s := FindSubject(UnknownPerson.SubjectUID); s != nil { + assert.Equal(t, "", s.SubjectName) + } else { + t.Fatal("result must not be nil") + } }) t.Run("nil", func(t *testing.T) { r := FindSubject("XXX") assert.Nil(t, r) }) - } func TestSubject_Links(t *testing.T) { diff --git a/internal/form/marker.go b/internal/form/marker.go index 9b83cb41a..2bb7d52fe 100644 --- a/internal/form/marker.go +++ b/internal/form/marker.go @@ -4,13 +4,8 @@ import "github.com/ulule/deepcopier" // Marker represents an image marker edit form. type Marker struct { - MarkerType string `json:"Type"` - MarkerSrc string `json:"Src"` - MarkerName string `json:"Name"` - SubjectUID string `json:"SubjectUID"` SubjectSrc string `json:"SubjectSrc"` - FaceID string `json:"FaceID"` - Score int `json:"Score"` + MarkerName string `json:"Name"` MarkerInvalid bool `json:"Invalid"` } diff --git a/internal/form/marker_test.go b/internal/form/marker_test.go index 0b84e6e0e..778545174 100644 --- a/internal/form/marker_test.go +++ b/internal/form/marker_test.go @@ -9,22 +9,12 @@ import ( func TestNewMarker(t *testing.T) { t.Run("success", func(t *testing.T) { var m = struct { - MarkerType string - MarkerSrc string - MarkerName string - SubjectUID string SubjectSrc string - FaceID string - Score int + MarkerName string MarkerInvalid bool }{ - MarkerType: "face", - MarkerSrc: "image", + SubjectSrc: "manual", MarkerName: "Foo", - SubjectUID: "3h59wvth837b5vyiub35", - SubjectSrc: "meta", - FaceID: "zz", - Score: 100, MarkerInvalid: true, } @@ -34,13 +24,8 @@ func TestNewMarker(t *testing.T) { t.Fatal(err) } - assert.Equal(t, "face", f.MarkerType) - assert.Equal(t, "image", f.MarkerSrc) + assert.Equal(t, "manual", f.SubjectSrc) assert.Equal(t, "Foo", f.MarkerName) - assert.Equal(t, "3h59wvth837b5vyiub35", f.SubjectUID) - assert.Equal(t, "zz", f.FaceID) - assert.Equal(t, "meta", f.SubjectSrc) - assert.Equal(t, 100, f.Score) assert.Equal(t, true, f.MarkerInvalid) }) } diff --git a/internal/mutex/mutex.go b/internal/mutex/mutex.go index e02f7af0c..5cb0cd5d3 100644 --- a/internal/mutex/mutex.go +++ b/internal/mutex/mutex.go @@ -10,9 +10,10 @@ var ( SyncWorker = Busy{} ShareWorker = Busy{} MetaWorker = Busy{} + FacesWorker = Busy{} ) // WorkersBusy returns true if any worker is busy. func WorkersBusy() bool { - return MainWorker.Busy() || SyncWorker.Busy() || ShareWorker.Busy() || MetaWorker.Busy() + return MainWorker.Busy() || SyncWorker.Busy() || ShareWorker.Busy() || MetaWorker.Busy() || FacesWorker.Busy() } diff --git a/internal/photoprism/faces.go b/internal/photoprism/faces.go index 9d584bf77..5e662a2c6 100644 --- a/internal/photoprism/faces.go +++ b/internal/photoprism/faces.go @@ -45,11 +45,11 @@ func (w *Faces) Start(opt FacesOptions) (err error) { return fmt.Errorf("facial recognition is disabled") } - if err := mutex.MainWorker.Start(); err != nil { + if err := mutex.FacesWorker.Start(); err != nil { return err } - defer mutex.MainWorker.Stop() + defer mutex.FacesWorker.Stop() // Remove invalid reference IDs from markers table. if removed, err := query.RemoveInvalidMarkerReferences(); err != nil { @@ -60,6 +60,15 @@ func (w *Faces) Start(opt FacesOptions) (err error) { log.Debugf("faces: no invalid references") } + // Add known marker subjects. + if affected, err := query.AddFaceMarkerSubjects(); err != nil { + log.Errorf("faces: %s (match markers with subjects)", err) + } else if affected > 0 { + log.Infof("faces: added %d known marker subjects", affected) + } else { + log.Debugf("faces: no subjects were missing") + } + // Optimize existing face clusters. if res, err := w.Optimize(); err != nil { return err @@ -69,15 +78,6 @@ func (w *Faces) Start(opt FacesOptions) (err error) { log.Debugf("faces: no clusters could be merged") } - // Add known marker subjects. - if affected, err := query.AddMarkerSubjects(); err != nil { - log.Errorf("faces: %s (match markers with subjects)", err) - } else if affected > 0 { - log.Infof("faces: added %d known marker subjects", affected) - } else { - log.Debugf("faces: no subjects were missing") - } - var added entity.Faces // Cluster existing face embeddings. @@ -108,7 +108,12 @@ func (w *Faces) Start(opt FacesOptions) (err error) { // Cancel stops the current operation. func (w *Faces) Cancel() { - mutex.MainWorker.Cancel() + mutex.FacesWorker.Cancel() +} + +// Canceled tests if face clustering and matching should be stopped. +func (w *Faces) Canceled() bool { + return mutex.FacesWorker.Canceled() || mutex.MainWorker.Canceled() || mutex.MetaWorker.Canceled() } // Disabled tests if facial recognition is disabled. diff --git a/internal/photoprism/faces_match.go b/internal/photoprism/faces_match.go index 638e15c2a..110d49c46 100644 --- a/internal/photoprism/faces_match.go +++ b/internal/photoprism/faces_match.go @@ -5,7 +5,6 @@ import ( "time" "github.com/photoprism/photoprism/internal/entity" - "github.com/photoprism/photoprism/internal/mutex" "github.com/photoprism/photoprism/internal/query" ) @@ -38,7 +37,7 @@ func (w *Faces) Match(opt FacesOptions) (result FacesMatchResult, err error) { return result, err } - limit := 500 + limit := 100 offset := 0 for { @@ -53,7 +52,7 @@ func (w *Faces) Match(opt FacesOptions) (result FacesMatchResult, err error) { } for _, marker := range markers { - if mutex.MainWorker.Canceled() { + if w.Canceled() { return result, fmt.Errorf("worker canceled") } diff --git a/internal/query/markers.go b/internal/query/markers.go index c9e3ea5a0..894058a46 100644 --- a/internal/query/markers.go +++ b/internal/query/markers.go @@ -4,7 +4,6 @@ import ( "fmt" "github.com/photoprism/photoprism/internal/entity" - "github.com/photoprism/photoprism/pkg/txt" ) // MarkerByID returns a Marker based on the ID. @@ -74,40 +73,6 @@ func Embeddings(single, unclustered bool) (result entity.Embeddings, err error) return result, nil } -// AddMarkerSubjects adds and references known marker subjects. -func AddMarkerSubjects() (affected int64, err error) { - var markers entity.Markers - - if err := Db(). - Where("face_id <> '' AND subject_uid = '' AND subject_src = ?", entity.SrcAuto). - Where("marker_invalid = 0 AND marker_type = ?", entity.MarkerFace). - Where("marker_name <> ''"). - Order("marker_name"). - Find(&markers).Error; err != nil { - return affected, err - } else if len(markers) == 0 { - return affected, nil - } - - for _, m := range markers { - if faceId := m.FaceID; faceId == "" { - // Do nothing. - } else if subj := entity.NewSubject(m.MarkerName, entity.SubjectPerson, entity.SrcMarker); subj == nil { - log.Errorf("faces: subject should not be nil - bug?") - } else if subj = entity.FirstOrCreateSubject(subj); subj == nil { - log.Errorf("faces: failed adding subject %s for marker %d", txt.Quote(m.MarkerName), m.ID) - } else if err := m.Updates(entity.Values{"SubjectUID": subj.SubjectUID, "SubjectSrc": entity.SrcAuto}); err != nil { - return affected, err - } else if err := Db().Model(&entity.Face{}).Where("id = ? AND subject_uid = ''", faceId).Update("SubjectUID", subj.SubjectUID).Error; err != nil { - return affected, err - } else { - affected++ - } - } - - return affected, err -} - // RemoveInvalidMarkerReferences deletes invalid reference IDs from the markers table. func RemoveInvalidMarkerReferences() (removed int64, err error) { // Remove subject and face relationships for invalid markers. diff --git a/internal/query/markers_test.go b/internal/query/markers_test.go index 57a94e0cd..063daf022 100644 --- a/internal/query/markers_test.go +++ b/internal/query/markers_test.go @@ -59,13 +59,6 @@ func TestEmbeddings(t *testing.T) { } } -func TestAddMarkerSubjects(t *testing.T) { - affected, err := AddMarkerSubjects() - - assert.NoError(t, err) - assert.GreaterOrEqual(t, affected, int64(1)) -} - func TestRemoveInvalidMarkerReferences(t *testing.T) { affected, err := RemoveInvalidMarkerReferences() diff --git a/internal/query/subjects.go b/internal/query/subjects.go index e5ecdae8c..6145c89c3 100644 --- a/internal/query/subjects.go +++ b/internal/query/subjects.go @@ -3,6 +3,8 @@ package query import ( "fmt" + "github.com/photoprism/photoprism/pkg/txt" + "github.com/photoprism/photoprism/internal/entity" ) @@ -10,7 +12,7 @@ import ( func Subjects(limit, offset int) (result entity.Subjects, err error) { stmt := Db() - stmt = stmt.Order("subject_slug").Limit(limit).Offset(offset) + stmt = stmt.Order("subject_name").Limit(limit).Offset(offset) err = stmt.Find(&result).Error return result, err @@ -26,3 +28,49 @@ func RemoveDanglingMarkerSubjects() (removed int64, err error) { return res.RowsAffected, res.Error } + +// AddFaceMarkerSubjects adds and references known marker subjects. +func AddFaceMarkerSubjects() (affected int64, err error) { + var markers entity.Markers + + if err := Db(). + Where("subject_uid = '' AND marker_name <> ''"). + Where("marker_invalid = 0 AND marker_type = ?", entity.MarkerFace). + Order("marker_name"). + Find(&markers).Error; err != nil { + return affected, err + } else if len(markers) == 0 { + return affected, nil + } + + var name string + var subj *entity.Subject + + for _, m := range markers { + if name == m.MarkerName && subj != nil { + // Do nothing. + } else if subj = entity.NewSubject(m.MarkerName, entity.SubjectPerson, entity.SrcMarker); subj == nil { + log.Errorf("faces: subject should not be nil - bug?") + continue + } else if subj = entity.FirstOrCreateSubject(subj); subj == nil { + log.Errorf("faces: failed adding subject %s", txt.Quote(m.MarkerName)) + continue + } else { + affected++ + } + + name = m.MarkerName + + if err := m.Updates(entity.Values{"SubjectUID": subj.SubjectUID}); err != nil { + return affected, err + } + + if m.FaceID == "" { + continue + } else if err := Db().Model(&entity.Face{}).Where("id = ? AND subject_uid = ''", m.FaceID).Update("SubjectUID", subj.SubjectUID).Error; err != nil { + return affected, err + } + } + + return affected, err +} diff --git a/internal/query/subjects_test.go b/internal/query/subjects_test.go index d66d2c57c..b7e8eb3b5 100644 --- a/internal/query/subjects_test.go +++ b/internal/query/subjects_test.go @@ -31,3 +31,10 @@ func TestRemoveDanglingMarkerSubjects(t *testing.T) { assert.Equal(t, int64(1), affected) } + +func TestAddFaceMarkerSubjects(t *testing.T) { + affected, err := AddFaceMarkerSubjects() + + assert.NoError(t, err) + assert.GreaterOrEqual(t, affected, int64(2)) +} diff --git a/internal/workers/workers.go b/internal/workers/workers.go index 484fa9490..b3b24cfc3 100644 --- a/internal/workers/workers.go +++ b/internal/workers/workers.go @@ -94,7 +94,7 @@ func StartShare(conf *config.Config) { } } -// StartShare runs the sync worker once. +// StartSync runs the sync worker once. func StartSync(conf *config.Config) { if !mutex.SyncWorker.Busy() { go func() {