From 7f796e6bda129c98777444938901afadfac5c5f9 Mon Sep 17 00:00:00 2001 From: Matjaz Debelak Date: Fri, 9 Aug 2024 10:43:33 +0200 Subject: [PATCH 1/4] Prevent super short chapters from interfering --- services/vidispine/chapter_test.go | 58 ++++++++++++++++++++++++++--- services/vidispine/vsapi/chapter.go | 18 +++++++-- 2 files changed, 66 insertions(+), 10 deletions(-) diff --git a/services/vidispine/chapter_test.go b/services/vidispine/chapter_test.go index 6b7f14f7..66855e50 100644 --- a/services/vidispine/chapter_test.go +++ b/services/vidispine/chapter_test.go @@ -53,27 +53,32 @@ func TestMergeTerseTimecodes(t *testing.T) { assert.Equal(t, expected, result) } -func Test_GetChapterMetaForClips(t *testing.T) { +// This is an edge case where the annotations overlap by a few frames +func Test_GetChapterMetaForClips_Overlapping(t *testing.T) { clips := []*Clip{ { VideoFile: "/dummy/file.mp4", - InSeconds: 1907.7599999999948, - OutSeconds: 3025.399999999994, + InSeconds: 1420.1199999999953, + OutSeconds: 2767.439999999988, SequenceIn: 0, SequenceOut: 0, AudioFiles: map[string]*AudioFile{ "nor": { - VXID: "VX-486737", - Streams: []int{1, 2}, + VXID: "VX-489605", + Streams: []int{2}, File: "/dummy/file.wav", }, }, SubtitleFiles: map[string]string{}, JSONTranscriptFile: "", - VXID: "VX-486737", + VXID: "VX-489598", }, } + if os.Getenv("VIDISPINE_BASE_URL") == "" { + t.Skip("VIDISPINE_BASE_URL is not set") + } + client := vsapi.NewClient( os.Getenv("VIDISPINE_BASE_URL"), os.Getenv("VIDISPINE_USERNAME"), @@ -82,9 +87,50 @@ func Test_GetChapterMetaForClips(t *testing.T) { out, err := GetChapterMetaForClips(client, clips) assert.NoError(t, err) assert.NotEmpty(t, out) + assert.Len(t, out, 1) for i, chapter := range out { assert.GreaterOrEqual(t, len(chapter.Meta.Terse["title"]), 1, "Chapter in loop iteration %d has no title", i) } +} + +// This is an edge case where the annotations overlap by a few frames +func Test_GetChapterMetaForClips_Overlapping2(t *testing.T) { + clips := []*Clip{ + { + VideoFile: "/dummy/file.mp4", + InSeconds: 3750.9199999999983, + OutSeconds: 3906.87999999999, + SequenceIn: 0, + SequenceOut: 0, + AudioFiles: map[string]*AudioFile{ + "nor": { + VXID: "VX-489605", + Streams: []int{2}, + File: "/dummy/file.wav", + }, + }, + SubtitleFiles: map[string]string{}, + JSONTranscriptFile: "", + VXID: "VX-489598", + }, + } + + if os.Getenv("VIDISPINE_BASE_URL") == "" { + t.Skip("VIDISPINE_BASE_URL is not set") + } + + client := vsapi.NewClient( + os.Getenv("VIDISPINE_BASE_URL"), + os.Getenv("VIDISPINE_USERNAME"), + os.Getenv("VIDISPINE_PASSWORD"), + ) + out, err := GetChapterMetaForClips(client, clips) + assert.NoError(t, err) + assert.NotEmpty(t, out) + assert.Len(t, out, 1) + for i, chapter := range out { + assert.GreaterOrEqual(t, len(chapter.Meta.Terse["title"]), 1, "Chapter in loop iteration %d has no title", i) + } } diff --git a/services/vidispine/vsapi/chapter.go b/services/vidispine/vsapi/chapter.go index 21112b25..f3de0de9 100644 --- a/services/vidispine/vsapi/chapter.go +++ b/services/vidispine/vsapi/chapter.go @@ -21,13 +21,19 @@ func (c *Client) GetChapterMeta(itemVXID string, inTc, outTc float64) (map[strin outClips := map[string]*MetadataResult{} for key, clip := range clips { + duration := 0.0 for _, field := range clip.Terse { for _, value := range field { - trimTimecodesToBeWithinRange(value, inTc, outTc) + duration = trimTimecodesToBeWithinRange(value, inTc, outTc) } } + if duration < 10.0 { + // If the trimmed chapter duration is < 10s we don't consider it a valid chapter + continue + } + outClips[key] = clip } @@ -35,12 +41,16 @@ func (c *Client) GetChapterMeta(itemVXID string, inTc, outTc float64) (map[strin } // trimTimecodesToBeWithinRange ensures that the timecodes are within the clip range -func trimTimecodesToBeWithinRange(value *MetadataField, inTc, outTc float64) { - if valueStart, _ := vscommon.TCToSeconds(value.Start); valueStart < inTc { +func trimTimecodesToBeWithinRange(value *MetadataField, inTc, outTc float64) float64 { + valueStart, _ := vscommon.TCToSeconds(value.Start) + if valueStart < inTc { value.Start = fmt.Sprintf("%.0f@PAL", inTc*25) } - if valueEnd, _ := vscommon.TCToSeconds(value.End); valueEnd > outTc { + valueEnd, _ := vscommon.TCToSeconds(value.End) + if valueEnd > outTc { value.End = fmt.Sprintf("%.0f@PAL", outTc*25) } + + return valueEnd - valueStart } From 461759692237e34f5041b57b6e3ef775bdf425c1 Mon Sep 17 00:00:00 2001 From: Matjaz Debelak Date: Mon, 12 Aug 2024 11:23:50 +0200 Subject: [PATCH 2/4] Updated a comment --- services/vidispine/chapter_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/services/vidispine/chapter_test.go b/services/vidispine/chapter_test.go index 66855e50..a1459b9d 100644 --- a/services/vidispine/chapter_test.go +++ b/services/vidispine/chapter_test.go @@ -95,6 +95,7 @@ func Test_GetChapterMetaForClips_Overlapping(t *testing.T) { } // This is an edge case where the annotations overlap by a few frames +// This test is identical to Test_GetChapterMetaForClips_Overlapping but with a different subclip that was exibithing the same behaviour func Test_GetChapterMetaForClips_Overlapping2(t *testing.T) { clips := []*Clip{ { From 6ce7679190c9c4c868c6768264e51a1b93fe71fd Mon Sep 17 00:00:00 2001 From: Matjaz Debelak Date: Wed, 14 Aug 2024 09:21:29 +0200 Subject: [PATCH 3/4] Merge two nearly identical tests --- services/vidispine/chapter_test.go | 114 ++++++++++++----------------- 1 file changed, 48 insertions(+), 66 deletions(-) diff --git a/services/vidispine/chapter_test.go b/services/vidispine/chapter_test.go index a1459b9d..9e231c13 100644 --- a/services/vidispine/chapter_test.go +++ b/services/vidispine/chapter_test.go @@ -55,83 +55,65 @@ func TestMergeTerseTimecodes(t *testing.T) { // This is an edge case where the annotations overlap by a few frames func Test_GetChapterMetaForClips_Overlapping(t *testing.T) { - clips := []*Clip{ - { - VideoFile: "/dummy/file.mp4", - InSeconds: 1420.1199999999953, - OutSeconds: 2767.439999999988, - SequenceIn: 0, - SequenceOut: 0, - AudioFiles: map[string]*AudioFile{ - "nor": { - VXID: "VX-489605", - Streams: []int{2}, - File: "/dummy/file.wav", - }, - }, - SubtitleFiles: map[string]string{}, - JSONTranscriptFile: "", - VXID: "VX-489598", - }, - } - if os.Getenv("VIDISPINE_BASE_URL") == "" { t.Skip("VIDISPINE_BASE_URL is not set") } - client := vsapi.NewClient( - os.Getenv("VIDISPINE_BASE_URL"), - os.Getenv("VIDISPINE_USERNAME"), - os.Getenv("VIDISPINE_PASSWORD"), - ) - out, err := GetChapterMetaForClips(client, clips) - assert.NoError(t, err) - assert.NotEmpty(t, out) - assert.Len(t, out, 1) - - for i, chapter := range out { - assert.GreaterOrEqual(t, len(chapter.Meta.Terse["title"]), 1, "Chapter in loop iteration %d has no title", i) - } -} - -// This is an edge case where the annotations overlap by a few frames -// This test is identical to Test_GetChapterMetaForClips_Overlapping but with a different subclip that was exibithing the same behaviour -func Test_GetChapterMetaForClips_Overlapping2(t *testing.T) { - clips := []*Clip{ + testData := [][]*Clip{ + { + { + VideoFile: "/dummy/file.mp4", + InSeconds: 1420.1199999999953, + OutSeconds: 2767.439999999988, + SequenceIn: 0, + SequenceOut: 0, + AudioFiles: map[string]*AudioFile{ + "nor": { + VXID: "VX-489605", + Streams: []int{2}, + File: "/dummy/file.wav", + }, + }, + SubtitleFiles: map[string]string{}, + JSONTranscriptFile: "", + VXID: "VX-489598", + }, + }, { - VideoFile: "/dummy/file.mp4", - InSeconds: 3750.9199999999983, - OutSeconds: 3906.87999999999, - SequenceIn: 0, - SequenceOut: 0, - AudioFiles: map[string]*AudioFile{ - "nor": { - VXID: "VX-489605", - Streams: []int{2}, - File: "/dummy/file.wav", + { + VideoFile: "/dummy/file.mp4", + InSeconds: 3750.9199999999983, + OutSeconds: 3906.87999999999, + SequenceIn: 0, + SequenceOut: 0, + AudioFiles: map[string]*AudioFile{ + "nor": { + VXID: "VX-489605", + Streams: []int{2}, + File: "/dummy/file.wav", + }, }, + SubtitleFiles: map[string]string{}, + JSONTranscriptFile: "", + VXID: "VX-489598", }, - SubtitleFiles: map[string]string{}, - JSONTranscriptFile: "", - VXID: "VX-489598", }, } - if os.Getenv("VIDISPINE_BASE_URL") == "" { - t.Skip("VIDISPINE_BASE_URL is not set") - } + for _, clips := range testData { - client := vsapi.NewClient( - os.Getenv("VIDISPINE_BASE_URL"), - os.Getenv("VIDISPINE_USERNAME"), - os.Getenv("VIDISPINE_PASSWORD"), - ) - out, err := GetChapterMetaForClips(client, clips) - assert.NoError(t, err) - assert.NotEmpty(t, out) - assert.Len(t, out, 1) + client := vsapi.NewClient( + os.Getenv("VIDISPINE_BASE_URL"), + os.Getenv("VIDISPINE_USERNAME"), + os.Getenv("VIDISPINE_PASSWORD"), + ) + out, err := GetChapterMetaForClips(client, clips) + assert.NoError(t, err) + assert.NotEmpty(t, out) + assert.Len(t, out, 1) - for i, chapter := range out { - assert.GreaterOrEqual(t, len(chapter.Meta.Terse["title"]), 1, "Chapter in loop iteration %d has no title", i) + for i, chapter := range out { + assert.GreaterOrEqual(t, len(chapter.Meta.Terse["title"]), 1, "Chapter in loop iteration %d has no title", i) + } } } From c51d863c2c660e712477ca7dfbafbd89cd310ff1 Mon Sep 17 00:00:00 2001 From: Matjaz Debelak Date: Thu, 15 Aug 2024 11:27:57 +0200 Subject: [PATCH 4/4] Add comment --- services/vidispine/chapter_test.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/services/vidispine/chapter_test.go b/services/vidispine/chapter_test.go index 9e231c13..b8e0682d 100644 --- a/services/vidispine/chapter_test.go +++ b/services/vidispine/chapter_test.go @@ -59,6 +59,13 @@ func Test_GetChapterMetaForClips_Overlapping(t *testing.T) { t.Skip("VIDISPINE_BASE_URL is not set") } + // The important thing here is the VXID of the main file (not the audio file), + // together with the in and out points of the clip. + // + // The situation is that two annotations (chapters) in MB overlap by a few frames, + // making the system think that there are two chapters between the in and out point. + // While this is techinically correct, it is not what we want, if one of the "chapters" + // is very short (less than 10 seconds). At this point it is unlikely to be a real chapter and we can ignore it. testData := [][]*Clip{ { {