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

Fix lockup when downloading clips with metadata #907

Merged
merged 2 commits into from
Nov 28, 2023
Merged

Fix lockup when downloading clips with metadata #907

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4403b5d
Change generated clip temp file name
ScrubN Nov 28, 2023
99d60a1
Add process exit check to mitigate possible .NET bug
ScrubN Nov 28, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 13 additions & 1 deletion TwitchDownloaderCore/ClipDownloader.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ void DownloadProgressHandler(StreamCopyProgress streamProgress)
TwitchHelper.CreateDirectory(downloadOptions.TempFolder);
}

var tempFile = Path.Combine(downloadOptions.TempFolder, $"clip_{DateTimeOffset.Now.ToUnixTimeMilliseconds()}_{Path.GetRandomFileName()}");
var tempFile = Path.Combine(downloadOptions.TempFolder, $"{downloadOptions.Id}_{DateTimeOffset.UtcNow.Ticks}.mp4");
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using DateTimeOffset.UtcNow.Ticks instead of DateTimeOffset.UtcNow.ToUnixTimeMilliseconds() is necessary to avoid conflicting names if the same url is pasted more than once in url mass downloader. Yes I could have left the name as it was before, but this name is more friendly to end users.

try
{
await DownloadFileTaskAsync(downloadUrl, tempFile, downloadOptions.ThrottleKib, new Progress<StreamCopyProgress>(DownloadProgressHandler), cancellationToken);
Expand All @@ -74,6 +74,12 @@ void DownloadProgressHandler(StreamCopyProgress streamProgress)
var clipChapter = TwitchHelper.GenerateClipChapter(clipInfo.data.clip);
await EncodeClipWithMetadata(tempFile, downloadOptions.Filename, clipInfo.data.clip, clipChapter, cancellationToken);

if (!File.Exists(downloadOptions.Filename))
{
File.Move(tempFile, downloadOptions.Filename);
throw new FileNotFoundException("Unable to serialize metadata (is FFmpeg missing?). The download has been completed without custom metadata.");
}

_progress.Report(new ProgressReport(ReportType.SameLineStatus, "Encoding Clip Metadata 100%"));
_progress.Report(new ProgressReport(100));
}
Expand Down Expand Up @@ -163,6 +169,12 @@ await FfmpegMetadata.SerializeAsync(metadataFile, clipMetadata.broadcaster.displ
};

process.Start();

// If the process has exited before we call WaitForExitAsync, the thread locks up.
// This was probably not intended by the .NET team, but it's an issue regardless.
if (process.HasExited)
return;

await process.WaitForExitAsync(cancellationToken);
}
finally
Expand Down
Loading