From ee782cb084d7efc290a7ea8587bd7acffd3aec41 Mon Sep 17 00:00:00 2001 From: emranemran Date: Tue, 30 Jan 2024 10:30:15 -0800 Subject: [PATCH] coordinator: add logging to debug issues with MP4 generation --- pipeline/coordinator.go | 7 +++++-- pipeline/coordinator_test.go | 22 +++++++++++----------- 2 files changed, 16 insertions(+), 13 deletions(-) diff --git a/pipeline/coordinator.go b/pipeline/coordinator.go index 445607c05..c64e9fcbf 100644 --- a/pipeline/coordinator.go +++ b/pipeline/coordinator.go @@ -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) @@ -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 } @@ -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 } diff --git a/pipeline/coordinator_test.go b/pipeline/coordinator_test.go index a4ac5742f..e07bcad0e 100644 --- a/pipeline/coordinator_test.go +++ b/pipeline/coordinator_test.go @@ -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", ) }