Skip to content

Commit

Permalink
Fix thumbs for recording fallback (#1359)
Browse files Browse the repository at this point in the history 
* Fix thumbs for recording fallback

* comments
  • Loading branch information
@mjh1
mjh1 authored Aug 13, 2024
1 parent e871bb8 commit 450abaf
Show file tree
Hide file tree
Showing 4 changed files with 73 additions and 47 deletions.
8 changes: 4 additions & 4 deletions clients/input_copy.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,14 +64,14 @@ func (s *InputCopy) CopyInputToS3(requestID string, inputFile, osTransferURL *ur
}
}

log.Log(requestID, "starting probe", "source", inputFile.String(), "dest", osTransferURL.String())
log.Log(requestID, "starting probe", "source", inputFile.Redacted(), "dest", osTransferURL.Redacted())
inputFileProbe, err := s.Probe.ProbeFile(requestID, signedURL, "-analyzeduration", "15000000")
if err != nil {
log.Log(requestID, "probe failed", "err", err, "source", inputFile.String(), "dest", osTransferURL.String())
log.Log(requestID, "probe failed", "err", err, "source", inputFile.Redacted(), "dest", osTransferURL.Redacted())
return video.InputVideo{}, "", fmt.Errorf("error probing MP4 input file from S3: %w", err)
}

log.Log(requestID, "probe succeeded", "source", inputFile.String(), "dest", osTransferURL.String())
log.Log(requestID, "probe succeeded", "source", inputFile.Redacted(), "dest", osTransferURL.Redacted())
videoTrack, err := inputFileProbe.GetTrack(video.TrackTypeVideo)
hasVideoTrack := err == nil
// verify the duration of the video track and don't process if we can't determine duration
Expand Down Expand Up @@ -229,7 +229,7 @@ func CopyAllInputFiles(requestID string, srcInputUrl, dstOutputUrl *url.URL, dec
}
byteCount += size
}
log.Log(requestID, "Copied", "bytes", byteCount, "source", srcInputUrl.String(), "dest", dstOutputUrl.String())
log.Log(requestID, "Copied", "bytes", byteCount, "source", srcInputUrl.Redacted(), "dest", dstOutputUrl.Redacted())
return nil
}

Expand Down
28 changes: 20 additions & 8 deletions clients/manifest.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,10 +53,13 @@ func RecordingBackupCheck(requestID string, primaryManifestURL, osTransferURL *u
return primaryManifestURL, nil
}

playlist, playlistType, err := downloadManifestWithBackup(requestID, primaryManifestURL.String())
playlistURL, playlist, playlistType, err := downloadManifestWithBackup(requestID, primaryManifestURL.String())
if err != nil {
return nil, fmt.Errorf("error downloading manifest: %w", err)
}
// if we had to use the backup location for the manifest then we need to write a new playlist
newPlaylistRequired := playlistURL != primaryManifestURL.String()

mediaPlaylist, err := convertToMediaPlaylist(playlist, playlistType)
if err != nil {
return nil, err
Expand All @@ -81,10 +84,19 @@ func RecordingBackupCheck(requestID string, primaryManifestURL, osTransferURL *u
if err != nil {
return nil, fmt.Errorf("failed to find segment file %s: %w", segURL.Redacted(), err)
}
if actualSegURL != segURL.String() {
// if we had to use the backup location for any segment then we need a new manifest file with new segment URLs
// pointing to wherever they are found, primary or backup
newPlaylistRequired = true
}
segment.URI = actualSegURL
}

// write the manifest to storage and update the manifestURL variable
if !newPlaylistRequired {
return primaryManifestURL, nil
}

// write the updated manifest to storage and update the manifestURL variable
outputStorageURL := osTransferURL.JoinPath("input.m3u8")
err = backoff.Retry(func() error {
return UploadToOSURL(outputStorageURL.String(), "", strings.NewReader(mediaPlaylist.String()), ManifestUploadTimeout)
Expand Down Expand Up @@ -118,7 +130,7 @@ func convertToMediaPlaylist(playlist m3u8.Playlist, playlistType m3u8.ListType)
return *mediaPlaylist, nil
}

func downloadManifestWithBackup(requestID, sourceManifestOSURL string) (m3u8.Playlist, m3u8.ListType, error) {
func downloadManifestWithBackup(requestID, sourceManifestOSURL string) (string, m3u8.Playlist, m3u8.ListType, error) {
var playlist, playlistBackup m3u8.Playlist
var playlistType, playlistTypeBackup m3u8.ListType
var size, sizeBackup int
Expand Down Expand Up @@ -146,21 +158,21 @@ func downloadManifestWithBackup(requestID, sourceManifestOSURL string) (m3u8.Pla
// (only not found errors passthrough below)
primaryNotFound, backupNotFound := errors.IsObjectNotFound(errPrimary), errors.IsObjectNotFound(errBackup)
if primaryNotFound && backupNotFound {
return nil, 0, errPrimary
return "", nil, 0, errPrimary
}
if errPrimary != nil && !primaryNotFound {
return nil, 0, errPrimary
return "", nil, 0, errPrimary
}
if errBackup != nil && !backupNotFound {
return nil, 0, errBackup
return "", nil, 0, errBackup
}

// Return the largest manifest as the most recent version
hasBackup := backupManifestURL != "" && errBackup == nil
if hasBackup && (errPrimary != nil || sizeBackup > size) {
return playlistBackup, playlistTypeBackup, nil
return backupManifestURL, playlistBackup, playlistTypeBackup, nil
}
return playlist, playlistType, errPrimary
return sourceManifestOSURL, playlist, playlistType, errPrimary
}

func downloadManifest(requestID, sourceManifestOSURL string) (playlist m3u8.Playlist, playlistType m3u8.ListType, size int, err error) {
Expand Down
76 changes: 43 additions & 33 deletions clients/manifest_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -370,49 +370,56 @@ seg-1.ts
`

tests := []struct {
name string
primaryManifest string
backupManifest string
primarySegments []string
backupSegments []string
name string
primaryManifest string
backupManifest string
primarySegments []string
backupSegments []string
expectedURLChange bool
}{
{
name: "happy. all segments and manifest available on primary",
primaryManifest: completeManifest,
backupManifest: "",
primarySegments: []string{"seg-0.ts", "seg-1.ts", "seg-2.ts", "seg-3.ts"},
name: "happy. all segments and manifest available on primary",
primaryManifest: completeManifest,
backupManifest: "",
primarySegments: []string{"seg-0.ts", "seg-1.ts", "seg-2.ts", "seg-3.ts"},
expectedURLChange: false,
},
{
name: "all segments and manifest available on backup",
primaryManifest: inCompleteManifest,
backupManifest: completeManifest,
backupSegments: []string{"seg-0.ts", "seg-1.ts", "seg-2.ts", "seg-3.ts"},
name: "all segments and manifest available on backup",
primaryManifest: inCompleteManifest,
backupManifest: completeManifest,
backupSegments: []string{"seg-0.ts", "seg-1.ts", "seg-2.ts", "seg-3.ts"},
expectedURLChange: true,
},
{
name: "all segments on backup and newest manifest on primary",
primaryManifest: completeManifest,
backupManifest: inCompleteManifest,
backupSegments: []string{"seg-0.ts", "seg-1.ts", "seg-2.ts", "seg-3.ts"},
name: "all segments on backup and newest manifest on primary",
primaryManifest: completeManifest,
backupManifest: inCompleteManifest,
backupSegments: []string{"seg-0.ts", "seg-1.ts", "seg-2.ts", "seg-3.ts"},
expectedURLChange: true,
},
{
name: "all segments on primary and newest manifest on backup",
primaryManifest: inCompleteManifest,
backupManifest: completeManifest,
primarySegments: []string{"seg-0.ts", "seg-1.ts", "seg-2.ts", "seg-3.ts"},
name: "all segments on primary and newest manifest on backup",
primaryManifest: inCompleteManifest,
backupManifest: completeManifest,
primarySegments: []string{"seg-0.ts", "seg-1.ts", "seg-2.ts", "seg-3.ts"},
expectedURLChange: true,
},
{
name: "segments split between primary and backup, newest manifest on primary",
primaryManifest: completeManifest,
backupManifest: inCompleteManifest,
primarySegments: []string{"seg-0.ts", "seg-2.ts"},
backupSegments: []string{"seg-1.ts", "seg-3.ts"},
name: "segments split between primary and backup, newest manifest on primary",
primaryManifest: completeManifest,
backupManifest: inCompleteManifest,
primarySegments: []string{"seg-0.ts", "seg-2.ts"},
backupSegments: []string{"seg-1.ts", "seg-3.ts"},
expectedURLChange: true,
},
{
name: "segments split between primary and backup, newest manifest on backup",
primaryManifest: inCompleteManifest,
backupManifest: completeManifest,
primarySegments: []string{"seg-0.ts", "seg-2.ts"},
backupSegments: []string{"seg-1.ts", "seg-3.ts"},
name: "segments split between primary and backup, newest manifest on backup",
primaryManifest: inCompleteManifest,
backupManifest: completeManifest,
primarySegments: []string{"seg-0.ts", "seg-2.ts"},
backupSegments: []string{"seg-1.ts", "seg-3.ts"},
expectedURLChange: true,
},
}
for _, tt := range tests {
Expand Down Expand Up @@ -443,8 +450,10 @@ seg-1.ts
require.NoError(t, err)
}

renditionUrl, err := RecordingBackupCheck("requestID", toUrl(t, filepath.Join(dir, "primary", "index.m3u8")), toUrl(t, filepath.Join(dir, "transfer")))
primary := toUrl(t, filepath.Join(dir, "primary", "index.m3u8"))
renditionUrl, err := RecordingBackupCheck("requestID", primary, toUrl(t, filepath.Join(dir, "transfer")))
require.NoError(t, err)
require.Equal(t, tt.expectedURLChange, renditionUrl != primary)

file, err := os.Open(renditionUrl.String())
require.NoError(t, err)
Expand All @@ -458,7 +467,8 @@ seg-1.ts

require.Len(t, mediaPlaylist.GetAllSegments(), 4)
for i, segment := range mediaPlaylist.GetAllSegments() {
require.True(t, filepath.IsAbs(segment.URI))
// If the URL has been updated then it means we should have a new playlist with absolute URLs
require.Equal(t, filepath.IsAbs(segment.URI), tt.expectedURLChange, "Expected absolute URL? %t: %s", tt.expectedURLChange, segment.URI)
require.True(t, true, strings.HasSuffix(segment.URI, fmt.Sprintf("seg-%d.ts", i)))
}
})
Expand Down
8 changes: 6 additions & 2 deletions thumbnails/thumbnails.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,11 @@ func GenerateThumbsFromManifest(requestID, input string, output *url.URL) error
for _, segment := range mediaPlaylist.GetAllSegments() {
segment := segment
uploadGroup.Go(func() error {
segURL := inputURL.JoinPath("..", segment.URI)
segURL, _ := url.Parse(segment.URI)
// if the URL is valid and absolute then we should just use it as is, otherwise append the path to inputURL
if segURL == nil || !segURL.IsAbs() {
segURL = inputURL.JoinPath("..", segment.URI)
}
var (
rc io.ReadCloser
err error
Expand All @@ -204,7 +208,7 @@ func GenerateThumbsFromManifest(requestID, input string, output *url.URL) error
return err
}, clients.DownloadRetryBackoff())
if err != nil {
return fmt.Errorf("error downloading manifest: %w", err)
return fmt.Errorf("error downloading segment %s: %w", segURL.Redacted(), err)
}
defer rc.Close()

Expand Down

0 comments on commit 450abaf

Please sign in to comment.