Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

coordinator: add logging to debug issues with MP4 generation #1113

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

coordinator: add logging to debug issues with MP4 generation #1113

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 5 additions & 2 deletions pipeline/coordinator.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -325,7 +325,7 @@ func (c *Coordinator) StartUploadJob(p UploadJobPayload) {
si.SourceFile = osTransferURL.String() // OS URL used by mist
si.SignedSourceURL = signedNewSourceURL // http(s) URL used by mediaconvert
si.InputFileInfo = inputVideoProbe
si.GenerateMP4 = ShouldGenerateMP4(sourceURL, p.Mp4TargetURL, p.FragMp4TargetURL, p.Mp4OnlyShort, si.InputFileInfo.Duration)
si.GenerateMP4 = ShouldGenerateMP4(p.RequestID, sourceURL, p.Mp4TargetURL, p.FragMp4TargetURL, p.Mp4OnlyShort, si.InputFileInfo.Duration)
si.DownloadDone = time.Now()

log.AddContext(p.RequestID, "new_source_url", si.SourceFile)
Expand Down Expand Up @@ -376,15 +376,17 @@ func checkClipResolution(p UploadJobPayload, inputVideoProbe *video.InputVideo,
}
}

func ShouldGenerateMP4(sourceURL, mp4TargetUrl *url.URL, fragMp4TargetUrl *url.URL, mp4OnlyShort bool, durationSecs float64) bool {
func ShouldGenerateMP4(requestID string, sourceURL, mp4TargetUrl *url.URL, fragMp4TargetUrl *url.URL, mp4OnlyShort bool, durationSecs float64) bool {
// Skip mp4 generation if we weren't able to determine the duration of the input file for any reason
if durationSecs == 0.0 {
log.Log(requestID, "Skipping MP4 generation because duration of input could not be determined")
return false
}
// We're currently memory-bound for generating MP4s above a certain file size
// This has been hitting us for long recordings, so do a crude "is it longer than 3 hours?" check and skip the MP4 if it is
const maxRecordingMP4Duration = 12 * time.Hour
if clients.IsHLSInput(sourceURL) && durationSecs > maxRecordingMP4Duration.Seconds() {
log.Log(requestID, "Skipping MP4 generation because duration of HLS input exceeds maxRecordingMP4Duration")
return false
}

Expand All @@ -396,6 +398,7 @@ func ShouldGenerateMP4(sourceURL, mp4TargetUrl *url.URL, fragMp4TargetUrl *url.U
return true
}

log.Log(requestID, "Skipping MP4 generation because a MP4 target URL is not set")
return false
}

Expand Down
22 changes: 11 additions & 11 deletions pipeline/coordinator_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -796,67 +796,67 @@ func TestMP4Generation(t *testing.T) {

require.False(
t,
ShouldGenerateMP4(mp4SourceURL, nil, nil, true, 60),
ShouldGenerateMP4("test", mp4SourceURL, nil, nil, true, 60),
"Should NOT generate an MP4 if the MP4 target URL isn't present",
)

require.True(
t,
ShouldGenerateMP4(mp4SourceURL, mp4TargetURL, fragMp4TargetURL, true, 60),
ShouldGenerateMP4("test", mp4SourceURL, mp4TargetURL, fragMp4TargetURL, true, 60),
"SHOULD generate an MP4 for a short source MP4 input even if 'only short MP4s' mode is enabled",
)

require.True(
t,
ShouldGenerateMP4(hlsSourceURL, mp4TargetURL, nil, true, 60),
ShouldGenerateMP4("test", hlsSourceURL, mp4TargetURL, nil, true, 60),
"SHOULD generate an MP4 for a short source HLS input even if 'only short MP4s' mode is enabled",
)

require.False(
t,
ShouldGenerateMP4(mp4SourceURL, mp4TargetURL, nil, true, 60*10),
ShouldGenerateMP4("test", mp4SourceURL, mp4TargetURL, nil, true, 60*10),
"Should NOT generate an MP4 for a long source MP4 input if 'only short MP4s' mode is enabled",
)

require.False(
t,
ShouldGenerateMP4(hlsSourceURL, mp4TargetURL, nil, true, 60*10),
ShouldGenerateMP4("test", hlsSourceURL, mp4TargetURL, nil, true, 60*10),
"Should NOT generate an MP4 for a long source HLS input if 'only short MP4s' mode is enabled",
)

require.True(
t,
ShouldGenerateMP4(mp4SourceURL, mp4TargetURL, nil, false, 60*10),
ShouldGenerateMP4("test", mp4SourceURL, mp4TargetURL, nil, false, 60*10),
"SHOULD generate an MP4 for a long source MP4 input if 'only short MP4s' mode is disabled",
)

require.True(
t,
ShouldGenerateMP4(hlsSourceURL, mp4TargetURL, nil, false, 60*10),
ShouldGenerateMP4("test", hlsSourceURL, mp4TargetURL, nil, false, 60*10),
"SHOULD generate an MP4 for a long source HLS input if 'only short MP4s' mode is disabled",
)

require.False(
t,
ShouldGenerateMP4(hlsSourceURL, mp4TargetURL, nil, false, 60*60*13),
ShouldGenerateMP4("test", hlsSourceURL, mp4TargetURL, nil, false, 60*60*13),
"SHOULD NOT generate an MP4 for a VERY long source HLS input even if 'only short MP4s' mode is disabled",
)

require.True(
t,
ShouldGenerateMP4(hlsSourceURL, nil, fragMp4TargetURL, false, 60*60*1),
ShouldGenerateMP4("test", hlsSourceURL, nil, fragMp4TargetURL, false, 60*60*1),
"SHOULD generate an MP4 for a fragmented Mp4 regardless of 'only short MP4s' mode",
)

require.False(
t,
ShouldGenerateMP4(hlsSourceURL, nil, nil, false, 60*60*13),
ShouldGenerateMP4("test", hlsSourceURL, nil, nil, false, 60*60*13),
"SHOULD NOT generate an MP4 if no valid mp4 or fmp4 URL was provided",
)

require.False(
t,
ShouldGenerateMP4(hlsSourceURL, mp4TargetURL, fragMp4TargetURL, false, 0),
ShouldGenerateMP4("test", hlsSourceURL, mp4TargetURL, fragMp4TargetURL, false, 0),
"SHOULD NOT generate an MP4 if duration is 0 regardless of a valid mp4/fmp4 URL",
)
}