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

Trickplay is being killed too soon #13116

Closed
tcely opened this issue Nov 27, 2024 · 2 comments · May be fixed by jellyfin/jellyfin-web#6356
Closed

Trickplay is being killed too soon #13116

tcely opened this issue Nov 27, 2024 · 2 comments · May be fixed by jellyfin/jellyfin-web#6356
Labels
wontfix This will not be worked on

Comments

@tcely
Copy link

tcely commented Nov 27, 2024 •
edited
Loading

There should really be a mention on the settings page that when using "Idle" process priority, the interval between images needs to be set higher.

jellyfin/MediaBrowser.Model/Configuration/ServerConfiguration.cs

Lines 211 to 213 in 1272bb9

public int ImageExtractionTimeoutMs { get; set; }

My logs are full of "Creating trickplay files" followed by "Killing ffmpeg process" because while sometimes there were 7 seconds between images, often enough the system wasn't idle and it took much longer.

I'm trying 30 seconds now, to see if that's a more useful timeout. Maybe the default should just be to wait longer?

jellyfin/MediaBrowser.MediaEncoding/Encoder/MediaEncoder.cs

Lines 44 to 53 in 1272bb9

/// <summary>
/// The default SDR image extraction timeout in milliseconds.
/// </summary>
internal const int DefaultSdrImageExtractionTimeout = 10000;
/// <summary>
/// The default HDR image extraction timeout in milliseconds.
/// </summary>
internal const int DefaultHdrImageExtractionTimeout = 20000;

jellyfin/MediaBrowser.MediaEncoding/Encoder/MediaEncoder.cs

Lines 991 to 1055 in 1272bb9

// Need to give ffmpeg enough time to make all the thumbnails, which could be a while,
// but we still need to detect if the process hangs.
// Making the assumption that as long as new jpegs are showing up, everything is good.
bool isResponsive = true;
int lastCount = 0;
var timeoutMs = _configurationManager.Configuration.ImageExtractionTimeoutMs;
timeoutMs = timeoutMs <= 0 ? DefaultHdrImageExtractionTimeout : timeoutMs;
while (isResponsive && !cancellationToken.IsCancellationRequested)
{
try
{
await process.WaitForExitAsync(TimeSpan.FromMilliseconds(timeoutMs)).ConfigureAwait(false);
ranToCompletion = true;
break;
}
catch (OperationCanceledException)
{
// We don't actually expect the process to be finished in one timeout span, just that one image has been generated.
}
var jpegCount = _fileSystem.GetFilePaths(targetDirectory).Count();
isResponsive = jpegCount > lastCount;
lastCount = jpegCount;
}
if (!ranToCompletion)
{
if (!isResponsive)
{
_logger.LogInformation("Trickplay process unresponsive.");
}
_logger.LogInformation("Stopping trickplay extraction.");
StopProcess(processWrapper, 1000);
}
}
var exitCode = ranToCompletion ? processWrapper.ExitCode ?? 0 : -1;
if (exitCode == -1)
{
_logger.LogError("ffmpeg image extraction failed for {ProcessDescription}", processDescription);
// Cleanup temp folder here, because the targetDirectory is not returned and the cleanup for failed ffmpeg process is not possible for caller.
// Ideally the ffmpeg should not write any files if it fails, but it seems like it is not guaranteed.
try
{
Directory.Delete(targetDirectory, true);
}
catch (Exception e)
{
_logger.LogError(e, "Failed to delete ffmpeg temp directory {TargetDirectory}", targetDirectory);
}
throw new FfmpegException(string.Format(CultureInfo.InvariantCulture, "ffmpeg image extraction failed for {0}", processDescription));
}
return targetDirectory;
}
}

I had no idea why the logs were not making sense until I searched / browsed GitHub to see what the conditions were for "Trickplay process unresponsive" in the code.
@gnattu
Copy link
Member

gnattu commented Nov 27, 2024

30 seconds is not a more useful timeout and that is a ridiculously high timeout. If your CPU needs 30 seconds to even extract one single frame you really should not enable this feature because the generation will take years or even lifetime.

@gnattu gnattu closed this as not planned Won't fix, can't repro, duplicate, stale Nov 27, 2024
@gnattu gnattu added the wontfix This will not be worked on label Nov 27, 2024
@tcely
Copy link
Author

tcely commented Nov 27, 2024 •
edited
Loading

As I understand the code, if any image takes longer than 1020 seconds, then the ffmpeg will be killed. It's not as if it's only checking for the first image to appear then leaving the ffmpeg to complete in the idle time.

The idle priority means that when the system is busy for longer than 20 seconds, no idle cycles are assigned to trickplay, then the process is killed.

This is suboptimal behavior, because it wasted all the completed images and if the process were not killed it would have completed as normal when idle cycles became available.

For reference, here is one of my recent successful Trickplay creation tasks with the 30 second timeout active:

[12:18:43] [INF] [73] Jellyfin.Server.Implementations.Trickplay.TrickplayManager: Creating trickplay files at 320 width
[12:21:07] [INF] [78] Jellyfin.Server.Implementations.Trickplay.TrickplayManager: Finished creation of trickplay files

As you can see, when there are idle cycles available it doesn't take years, but extended busy periods do occur. Also, this is a background scheduled task, so I wouldn't actually care if it took more than 30 minutes total.

@tcely tcely changed the title Trickplay being killed too soon Trickplay is being killed too soon Nov 27, 2024
tcely added a commit to tcely/jellyfin-web that referenced this issue Nov 28, 2024
@tcely
Suggest ImageExtractionTimeoutMs changes for Idle priority
1837e50 
After seeing repeated logs about `ffmpeg` being started then killed for being unresponsive, I tracked down this configuration setting on GitHub.

So far, my Trickplay generation is much more reliable with a timeout of 30,000 instead.

See: jellyfin/jellyfin#13116
@tcely tcely mentioned this issue Nov 28, 2024
Open
@felix920506 felix920506 moved this to Not A Bug in Jellyfin - Triage Board Dec 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
wontfix This will not be worked on
Projects
Jellyfin - Triage Board
Archived in project
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Suggest ImageExtractionTimeoutMs changes for Idle priority tcely/jellyfin-web
2 participants
@tcely @gnattu