From 2bfd21decf760c2211090e2ba1a2fca8c1976469 Mon Sep 17 00:00:00 2001 From: Hoshino Lina Date: Sun, 1 Nov 2026 18:34:34 +0900 Subject: Always extract attachments individually Since e627c723, attachments can be extracted by filename in one ffmpeg command. This makes the auto-filename codepath redundant. Letting ffmpeg extract all attachments is broken in ffmpeg upstream. This was worked around in jellyfin/jellyfin-ffmpeg#720 but affects distributions that do not use jellyfin-ffmpeg. Since there is no good reason to keep this codepath, just delete it. This obsoletes the above ffmpeg patch and should be more robust in the future, since Jellyfin no longer depends on guessing exactly what behavior ffmpeg has when constructing, allowing, or denying attachment filenames. --- .../Attachments/AttachmentExtractor.cs | 131 +-------------------- 1 file changed, 4 insertions(+), 127 deletions(-) diff --git a/MediaBrowser.MediaEncoding/Attachments/AttachmentExtractor.cs b/MediaBrowser.MediaEncoding/Attachments/AttachmentExtractor.cs index fbe8afc66e..756b67e063 100644 --- a/MediaBrowser.MediaEncoding/Attachments/AttachmentExtractor.cs +++ b/MediaBrowser.MediaEncoding/Attachments/AttachmentExtractor.cs @@ -100,22 +100,10 @@ namespace MediaBrowser.MediaEncoding.Attachments MediaSourceInfo mediaSource, CancellationToken cancellationToken) { - var shouldExtractOneByOne = mediaSource.MediaAttachments.Any(a => !string.IsNullOrEmpty(a.FileName) - && !string.Equals(PathHelper.GetSafeLeafFileName(a.FileName), a.FileName, StringComparison.Ordinal)); - if (shouldExtractOneByOne && !inputFile.EndsWith(".mks", StringComparison.OrdinalIgnoreCase)) - { - await ExtractAllAttachmentsIndividuallyInternal( - inputFile, - mediaSource, - cancellationToken).ConfigureAwait(false); - } - else - { - await ExtractAllAttachmentsInternal( - inputFile, - mediaSource, - cancellationToken).ConfigureAwait(false); - } + await ExtractAllAttachmentsIndividuallyInternal( + inputFile, + mediaSource, + cancellationToken).ConfigureAwait(false); } private async Task ExtractAllAttachmentsIndividuallyInternal( @@ -252,117 +240,6 @@ namespace MediaBrowser.MediaEncoding.Attachments } } - private async Task ExtractAllAttachmentsInternal( - string inputFile, - MediaSourceInfo mediaSource, - CancellationToken cancellationToken) - { - var inputPath = _mediaEncoder.GetInputArgument(inputFile, mediaSource); - - ArgumentException.ThrowIfNullOrEmpty(inputPath); - - var outputFolder = _pathManager.GetAttachmentFolderPath(mediaSource.Id); - if (outputFolder is null) - { - _logger.LogDebug("Skipping attachment extraction for input {InputFile}: MediaSource Id is not a GUID.", inputFile); - return; - } - - using (await _semaphoreLocks.LockAsync(outputFolder, cancellationToken).ConfigureAwait(false)) - { - var directory = Directory.CreateDirectory(outputFolder); - var fileNames = directory.GetFiles("*", SearchOption.TopDirectoryOnly).Select(f => f.Name).ToHashSet(); - var missingFiles = mediaSource.MediaAttachments.Where(a => a.FileName is not null && !fileNames.Contains(a.FileName) && !string.Equals(a.Codec, "mjpeg", StringComparison.OrdinalIgnoreCase)); - if (!missingFiles.Any()) - { - // Skip extraction if all files already exist - return; - } - - // Files without video/audio streams (e.g. MKS subtitle files) don't need a dummy - // output since there are no streams to process. Omit "-t 0 -f null null" so ffmpeg - // doesn't fail trying to open an output with no streams. It will exit with code 1 - // ("at least one output file must be specified") which is expected and harmless - // since we only need the -dump_attachment side effect. - var hasVideoOrAudioStream = mediaSource.MediaStreams - .Any(s => s.Type == MediaStreamType.Video || s.Type == MediaStreamType.Audio); - var processArgs = string.Format( - CultureInfo.InvariantCulture, - "-dump_attachment:t \"\" -y {0} -i {1} {2}", - inputPath.EndsWith(".concat\"", StringComparison.OrdinalIgnoreCase) ? "-f concat -safe 0" : string.Empty, - inputPath, - hasVideoOrAudioStream ? "-t 0 -f null null" : string.Empty); - - int exitCode; - - using (var process = new Process - { - StartInfo = new ProcessStartInfo - { - Arguments = processArgs, - FileName = _mediaEncoder.EncoderPath, - UseShellExecute = false, - CreateNoWindow = true, - WindowStyle = ProcessWindowStyle.Hidden, - WorkingDirectory = outputFolder, - ErrorDialog = false - }, - EnableRaisingEvents = true - }) - { - _logger.LogInformation("{File} {Arguments}", process.StartInfo.FileName, process.StartInfo.Arguments); - - process.Start(); - - try - { - await process.WaitForExitAsync(cancellationToken).ConfigureAwait(false); - exitCode = process.ExitCode; - } - catch (OperationCanceledException) - { - process.Kill(true); - exitCode = -1; - } - } - - var failed = false; - - if (exitCode != 0) - { - if (hasVideoOrAudioStream || exitCode != 1) - { - failed = true; - - _logger.LogWarning("Deleting extracted attachments {Path} due to failure: {ExitCode}", outputFolder, exitCode); - try - { - Directory.Delete(outputFolder); - } - catch (IOException ex) - { - _logger.LogError(ex, "Error deleting extracted attachments {Path}", outputFolder); - } - } - } - - if (!failed && !Directory.Exists(outputFolder)) - { - failed = true; - } - - if (failed) - { - _logger.LogError("ffmpeg attachment extraction failed for {InputPath} to {OutputPath}", inputPath, outputFolder); - - throw new InvalidOperationException( - string.Format(CultureInfo.InvariantCulture, "ffmpeg attachment extraction failed for {0} to {1}", inputPath, outputFolder)); - } - - _logger.LogInformation("ffmpeg attachment extraction completed for {InputPath} to {OutputPath}", inputPath, outputFolder); - } - } - private async Task GetAttachmentStream( MediaSourceInfo mediaSource, MediaAttachment mediaAttachment, -- cgit v1.2.3