Fix build and address comments

9 files changed, 82 insertions, 97 deletions

diff --git a/Emby.Server.Implementations/ApplicationHost.cs b/Emby.Server.Implementations/ApplicationHost.cs
index 7c3f59af2..c5ac27ed4 100644
--- a/Emby.Server.Implementations/ApplicationHost.cs
+++ b/Emby.Server.Implementations/ApplicationHost.cs
@@ -878,7 +878,7 @@ namespace Emby.Server.Implementations
             serviceCollection.AddSingleton(typeof(IResourceFileManager), typeof(ResourceFileManager));
             serviceCollection.AddSingleton<EncodingHelper>();
 
-            serviceCollection.AddSingleton(typeof(MediaBrowser.Controller.MediaEncoding.IAttachmentExtractor),typeof(MediaBrowser.MediaEncoding.Attachments.AttachmentExtractor));
+            serviceCollection.AddSingleton(typeof(IAttachmentExtractor), typeof(MediaBrowser.MediaEncoding.Attachments.AttachmentExtractor));
 
             _displayPreferencesRepository.Initialize();
 
diff --git a/Emby.Server.Implementations/Data/SqliteItemRepository.cs b/Emby.Server.Implementations/Data/SqliteItemRepository.cs
index 2206816a5..91ca8477d 100644
--- a/Emby.Server.Implementations/Data/SqliteItemRepository.cs
+++ b/Emby.Server.Implementations/Data/SqliteItemRepository.cs
@@ -55,8 +55,8 @@ namespace Emby.Server.Implementations.Data
             queryPrefixText.Append("insert into mediaattachments (");
             foreach (var column in _mediaAttachmentSaveColumns)
             {
-                queryPrefixText.Append(column);
-                queryPrefixText.Append(',');
+                queryPrefixText.Append(column)
+                    .Append(',');
             }
 
             queryPrefixText.Length -= 1;
@@ -449,6 +449,7 @@ namespace Emby.Server.Implementations.Data
             "Filename",
             "MIMEType"
         };
+
         private static readonly string _mediaAttachmentInsertPrefix;
 
         private static string GetSaveItemCommandText()
@@ -6208,7 +6209,10 @@ where AncestorIdText not null and ItemValues.Value not null and ItemValues.Type
             return list;
         }
 
-        public void SaveMediaAttachments(Guid id, List<MediaAttachment> attachments, CancellationToken cancellationToken)
+        public void SaveMediaAttachments(
+            Guid id,
+            List<MediaAttachment> attachments,
+            CancellationToken cancellationToken)
         {
             CheckDisposed();
             if (id == Guid.Empty)
@@ -6237,24 +6241,22 @@ where AncestorIdText not null and ItemValues.Value not null and ItemValues.Type
             }
         }
 
-        private void InsertMediaAttachments(byte[] idBlob, List<MediaAttachment> attachments, IDatabaseConnection db, CancellationToken cancellationToken)
+        private void InsertMediaAttachments(
+            byte[] idBlob,
+            List<MediaAttachment> attachments,
+            IDatabaseConnection db,
+            CancellationToken cancellationToken)
         {
-            var startIndex = 0;
-            var insertAtOnce = 10;
+            const int InsertAtOnce = 10;
 
-            while (startIndex < attachments.Count)
+            for (var startIndex = 0; startIndex < attachments.Count; startIndex += InsertAtOnce)
             {
                 var insertText = new StringBuilder(_mediaAttachmentInsertPrefix);
 
-                var endIndex = Math.Min(attachments.Count, startIndex + insertAtOnce);
+                var endIndex = Math.Min(attachments.Count, startIndex + InsertAtOnce);
 
                 for (var i = startIndex; i < endIndex; i++)
                 {
-                    if (i != startIndex)
-                    {
-                        insertText.Append(',');
-                    }
-
                     var index = i.ToString(CultureInfo.InvariantCulture);
                     insertText.Append("(@ItemId, ");
 
@@ -6265,9 +6267,11 @@ where AncestorIdText not null and ItemValues.Value not null and ItemValues.Type
 
                     insertText.Length -= 1;
 
-                    insertText.Append(")");
+                    insertText.Append("),");
                 }
 
+                insertText.Length--;
+
                 cancellationToken.ThrowIfCancellationRequested();
 
                 using (var statement = PrepareStatement(db, insertText.ToString()))
@@ -6291,8 +6295,6 @@ where AncestorIdText not null and ItemValues.Value not null and ItemValues.Type
                     statement.Reset();
                     statement.MoveNext();
                 }
-
-                startIndex += insertAtOnce;
             }
         }
 
diff --git a/Emby.Server.Implementations/Library/MediaSourceManager.cs b/Emby.Server.Implementations/Library/MediaSourceManager.cs
index 4eff2807c..ba1564d1f 100644
--- a/Emby.Server.Implementations/Library/MediaSourceManager.cs
+++ b/Emby.Server.Implementations/Library/MediaSourceManager.cs
@@ -137,15 +137,6 @@ namespace Emby.Server.Implementations.Library
         }
 
         /// <inheritdoc />
-        public List<MediaAttachment> GetMediaAttachments(string mediaSourceId)
-        {
-            return GetMediaAttachments(new MediaAttachmentQuery
-            {
-                ItemId = new Guid(mediaSourceId)
-            });
-        }
-
-        /// <inheritdoc />
         public List<MediaAttachment> GetMediaAttachments(Guid itemId)
         {
             return GetMediaAttachments(new MediaAttachmentQuery
diff --git a/Jellyfin.Api/Controllers/StartupController.cs b/Jellyfin.Api/Controllers/StartupController.cs
index 1014c8c56..afc9b8f3d 100644
--- a/Jellyfin.Api/Controllers/StartupController.cs
+++ b/Jellyfin.Api/Controllers/StartupController.cs
@@ -96,7 +96,6 @@ namespace Jellyfin.Api.Controllers
         public StartupUserDto GetFirstUser()
         {
             var user = _userManager.Users.First();
-
             return new StartupUserDto
             {
                 Name = user.Name,
diff --git a/MediaBrowser.Api/Attachments/AttachmentService.cs b/MediaBrowser.Api/Attachments/AttachmentService.cs
index 1ebfaa14b..ef09951b6 100644
--- a/MediaBrowser.Api/Attachments/AttachmentService.cs
+++ b/MediaBrowser.Api/Attachments/AttachmentService.cs
@@ -1,22 +1,14 @@
 using System;
-using System.Collections.Generic;
-using System.Globalization;
 using System.IO;
-using System.Linq;
-using System.Text;
 using System.Threading;
 using System.Threading.Tasks;
-using MediaBrowser.Controller.Entities;
+using MediaBrowser.Controller.Configuration;
 using MediaBrowser.Controller.Library;
 using MediaBrowser.Controller.MediaEncoding;
 using MediaBrowser.Controller.Net;
-using MediaBrowser.Controller.Providers;
 using MediaBrowser.Model.Entities;
-using MediaBrowser.Model.IO;
-using MediaBrowser.Model.Providers;
 using MediaBrowser.Model.Services;
 using Microsoft.Extensions.Logging;
-using MimeTypes = MediaBrowser.Model.Net.MimeTypes;
 
 namespace MediaBrowser.Api.Attachments
 {
@@ -38,7 +30,13 @@ namespace MediaBrowser.Api.Attachments
         private readonly ILibraryManager _libraryManager;
         private readonly IAttachmentExtractor _attachmentExtractor;
 
-        public AttachmentService(ILibraryManager libraryManager, IAttachmentExtractor attachmentExtractor)
+        public AttachmentService(
+            ILogger<AttachmentService> logger,
+            IServerConfigurationManager serverConfigurationManager,
+            IHttpResultFactory httpResultFactory,
+            ILibraryManager libraryManager,
+            IAttachmentExtractor attachmentExtractor)
+            : base(logger, serverConfigurationManager, httpResultFactory)
         {
             _libraryManager = libraryManager;
             _attachmentExtractor = attachmentExtractor;
@@ -46,7 +44,6 @@ namespace MediaBrowser.Api.Attachments
 
         public async Task<object> Get(GetAttachment request)
         {
-            var item = (Video)_libraryManager.GetItemById(request.Id);
             var (attachment, attachmentStream) = await GetAttachment(request).ConfigureAwait(false);
             var mime = string.IsNullOrWhiteSpace(attachment.MIMEType) ? "application/octet-stream" : attachment.MIMEType;
 
diff --git a/MediaBrowser.Controller/Library/IMediaSourceManager.cs b/MediaBrowser.Controller/Library/IMediaSourceManager.cs
index dda8d397a..09e6fda88 100644
--- a/MediaBrowser.Controller/Library/IMediaSourceManager.cs
+++ b/MediaBrowser.Controller/Library/IMediaSourceManager.cs
@@ -41,21 +41,14 @@ namespace MediaBrowser.Controller.Library
         /// <summary>
         /// Gets the media attachments.
         /// </summary>
-        /// <param name="">The item identifier.</param>
+        /// <param name="itemId">The item identifier.</param>
         /// <returns>IEnumerable&lt;MediaAttachment&gt;.</returns>
         List<MediaAttachment> GetMediaAttachments(Guid itemId);
 
         /// <summary>
         /// Gets the media attachments.
         /// </summary>
-        /// <param name="">The The media source identifier.</param>
-        /// <returns>IEnumerable&lt;MediaAttachment&gt;.</returns>
-
-        List<MediaAttachment> GetMediaAttachments(string mediaSourceId);
-        /// <summary>
-        /// Gets the media attachments.
-        /// </summary>
-        /// <param name="">The query.</param>
+        /// <param name="query">The query.</param>
         /// <returns>IEnumerable&lt;MediaAttachment&gt;.</returns>
         List<MediaAttachment> GetMediaAttachments(MediaAttachmentQuery query);
 
diff --git a/MediaBrowser.Controller/MediaEncoding/IAttachmentExtractor.cs b/MediaBrowser.Controller/MediaEncoding/IAttachmentExtractor.cs
index 59c0a3f21..7c7e84de6 100644
--- a/MediaBrowser.Controller/MediaEncoding/IAttachmentExtractor.cs
+++ b/MediaBrowser.Controller/MediaEncoding/IAttachmentExtractor.cs
@@ -2,14 +2,14 @@ using System.IO;
 using System.Threading;
 using System.Threading.Tasks;
 using MediaBrowser.Controller.Entities;
-using MediaBrowser.Model.MediaInfo;
 using MediaBrowser.Model.Entities;
 
 namespace MediaBrowser.Controller.MediaEncoding
 {
     public interface IAttachmentExtractor
     {
-        Task<(MediaAttachment attachment, Stream stream)> GetAttachment(BaseItem item,
+        Task<(MediaAttachment attachment, Stream stream)> GetAttachment(
+            BaseItem item,
             string mediaSourceId,
             int attachmentStreamIndex,
             CancellationToken cancellationToken);
diff --git a/MediaBrowser.MediaEncoding/Attachments/AttachmentExtractor.cs b/MediaBrowser.MediaEncoding/Attachments/AttachmentExtractor.cs
index cb22343c4..c371e8b94 100644
--- a/MediaBrowser.MediaEncoding/Attachments/AttachmentExtractor.cs
+++ b/MediaBrowser.MediaEncoding/Attachments/AttachmentExtractor.cs
@@ -4,44 +4,41 @@ using System.Collections.Concurrent;
 using System.Globalization;
 using System.IO;
 using System.Linq;
-using System.Text;
 using System.Threading;
 using System.Threading.Tasks;
 using MediaBrowser.Common.Configuration;
 using MediaBrowser.Common.Extensions;
-using MediaBrowser.Common.Net;
 using MediaBrowser.Controller.Entities;
 using MediaBrowser.Controller.Library;
 using MediaBrowser.Controller.MediaEncoding;
-using MediaBrowser.Model.Diagnostics;
 using MediaBrowser.Model.Dto;
 using MediaBrowser.Model.Entities;
 using MediaBrowser.Model.IO;
 using MediaBrowser.Model.MediaInfo;
-using MediaBrowser.Model.Serialization;
 using Microsoft.Extensions.Logging;
-using UtfUnknown;
 
 namespace MediaBrowser.MediaEncoding.Attachments
 {
-    public class AttachmentExtractor : IAttachmentExtractor
+    public class AttachmentExtractor : IAttachmentExtractor, IDisposable
     {
-        private readonly ILibraryManager _libraryManager;
         private readonly ILogger _logger;
         private readonly IApplicationPaths _appPaths;
         private readonly IFileSystem _fileSystem;
         private readonly IMediaEncoder _mediaEncoder;
         private readonly IMediaSourceManager _mediaSourceManager;
 
+        private readonly ConcurrentDictionary<string, SemaphoreSlim> _semaphoreLocks =
+            new ConcurrentDictionary<string, SemaphoreSlim>();
+
+        private bool _disposed = false;
+
         public AttachmentExtractor(
-            ILibraryManager libraryManager,
             ILogger<AttachmentExtractor> logger,
             IApplicationPaths appPaths,
             IFileSystem fileSystem,
             IMediaEncoder mediaEncoder,
             IMediaSourceManager mediaSourceManager)
         {
-            _libraryManager = libraryManager;
             _logger = logger;
             _appPaths = appPaths;
             _fileSystem = fileSystem;
@@ -49,8 +46,7 @@ namespace MediaBrowser.MediaEncoding.Attachments
             _mediaSourceManager = mediaSourceManager;
         }
 
-        private string AttachmentCachePath => Path.Combine(_appPaths.DataPath, "attachments");
-
+        /// <inheritdoc />
         public async Task<(MediaAttachment attachment, Stream stream)> GetAttachment(BaseItem item, string mediaSourceId, int attachmentStreamIndex, CancellationToken cancellationToken)
         {
             if (item == null)
@@ -70,12 +66,14 @@ namespace MediaBrowser.MediaEncoding.Attachments
             {
                 throw new ResourceNotFoundException($"MediaSource {mediaSourceId} not found");
             }
+
             var mediaAttachment = mediaSource.MediaAttachments
                 .FirstOrDefault(i => i.Index == attachmentStreamIndex);
             if (mediaAttachment == null)
             {
                 throw new ResourceNotFoundException($"MediaSource {mediaSourceId} has no attachment with stream index {attachmentStreamIndex}");
             }
+
             var attachmentStream = await GetAttachmentStream(mediaSource, mediaAttachment, cancellationToken)
                     .ConfigureAwait(false);
 
@@ -87,49 +85,32 @@ namespace MediaBrowser.MediaEncoding.Attachments
             MediaAttachment mediaAttachment,
             CancellationToken cancellationToken)
         {
-            var inputFiles = new[] { mediaSource.Path };
-            var attachmentPath = await GetReadableFile(mediaSource.Path, inputFiles, mediaSource.Protocol, mediaAttachment, cancellationToken).ConfigureAwait(false);
-            var stream = await GetAttachmentStream(attachmentPath, cancellationToken).ConfigureAwait(false);
-            return stream;
+            var attachmentPath = await GetReadableFile(mediaSource.Path, mediaSource.Path, mediaSource.Protocol, mediaAttachment, cancellationToken).ConfigureAwait(false);
+            return File.OpenRead(attachmentPath);
         }
 
-        private async Task<Stream> GetAttachmentStream(
-            string path,
-            CancellationToken cancellationToken)
-        {
-            return File.OpenRead(path);
-        }
-
-        private async Task<String> GetReadableFile(
+        private async Task<string> GetReadableFile(
             string mediaPath,
-            string[] inputFiles,
+            string inputFile,
             MediaProtocol protocol,
             MediaAttachment mediaAttachment,
             CancellationToken cancellationToken)
         {
             var outputPath = GetAttachmentCachePath(mediaPath, protocol, mediaAttachment.Index);
-            await ExtractAttachment(inputFiles, protocol, mediaAttachment.Index, outputPath, cancellationToken)
+            await ExtractAttachment(inputFile, protocol, mediaAttachment.Index, outputPath, cancellationToken)
                 .ConfigureAwait(false);
 
             return outputPath;
         }
 
-        private readonly ConcurrentDictionary<string, SemaphoreSlim> _semaphoreLocks =
-            new ConcurrentDictionary<string, SemaphoreSlim>();
-
-        private SemaphoreSlim GetLock(string filename)
-        {
-            return _semaphoreLocks.GetOrAdd(filename, key => new SemaphoreSlim(1, 1));
-        }
-
         private async Task ExtractAttachment(
-            string[] inputFiles,
+            string inputFile,
             MediaProtocol protocol,
             int attachmentStreamIndex,
             string outputPath,
             CancellationToken cancellationToken)
         {
-            var semaphore = GetLock(outputPath);
+            var semaphore = _semaphoreLocks.GetOrAdd(outputPath, key => new SemaphoreSlim(1, 1));
 
             await semaphore.WaitAsync(cancellationToken).ConfigureAwait(false);
 
@@ -137,7 +118,11 @@ namespace MediaBrowser.MediaEncoding.Attachments
             {
                 if (!File.Exists(outputPath))
                 {
-                    await ExtractAttachmentInternal(_mediaEncoder.GetInputArgument(inputFiles, protocol), attachmentStreamIndex, outputPath, cancellationToken).ConfigureAwait(false);
+                    await ExtractAttachmentInternal(
+                        _mediaEncoder.GetInputArgument(new[] { inputFile }, protocol),
+                        attachmentStreamIndex,
+                        outputPath,
+                        cancellationToken).ConfigureAwait(false);
                 }
             }
             finally
@@ -186,16 +171,7 @@ namespace MediaBrowser.MediaEncoding.Attachments
 
             _logger.LogInformation("{File} {Arguments}", process.StartInfo.FileName, process.StartInfo.Arguments);
 
-            try
-            {
-                process.Start();
-            }
-            catch (Exception ex)
-            {
-                _logger.LogError(ex, "Error starting ffmpeg");
-
-                throw;
-            }
+            process.Start();
 
             var processTcs = new TaskCompletionSource<bool>();
             process.EnableRaisingEvents = true;
@@ -216,6 +192,7 @@ namespace MediaBrowser.MediaEncoding.Attachments
                     _logger.LogError(ex, "Error killing attachment extraction process");
                 }
             }
+
             var exitCode = ranToCompletion ? process.ExitCode : -1;
 
             process.Dispose();
@@ -270,9 +247,35 @@ namespace MediaBrowser.MediaEncoding.Attachments
             {
                 filename = (mediaPath + attachmentStreamIndex.ToString(CultureInfo.InvariantCulture)).GetMD5().ToString("D");
             }
+
             var prefix = filename.Substring(0, 1);
-            return Path.Combine(AttachmentCachePath, prefix, filename);
+            return Path.Combine(_appPaths.DataPath, "attachments", prefix, filename);
+        }
+
+        /// <inheritdoc />
+        public void Dispose()
+        {
+            Dispose(true);
+            GC.SuppressFinalize(this);
         }
 
+        /// <summary>
+        /// Releases unmanaged and - optionally - managed resources.
+        /// </summary>
+        /// <param name="disposing"><c>true</c> to release both managed and unmanaged resources; <c>false</c> to release only unmanaged resources.</param>
+        protected virtual void Dispose(bool disposing)
+        {
+            if (_disposed)
+            {
+                return;
+            }
+
+            if (disposing)
+            {
+
+            }
+
+            _disposed = true;
+        }
     }
 }
diff --git a/MediaBrowser.MediaEncoding/Encoder/EncodingUtils.cs b/MediaBrowser.MediaEncoding/Encoder/EncodingUtils.cs
index d4aede572..c5da42089 100644
--- a/MediaBrowser.MediaEncoding/Encoder/EncodingUtils.cs
+++ b/MediaBrowser.MediaEncoding/Encoder/EncodingUtils.cs
@@ -43,7 +43,7 @@ namespace MediaBrowser.MediaEncoding.Encoder
         /// </summary>
         /// <param name="path">The path.</param>
         /// <returns>System.String.</returns>
-        private static string GetFileInputArgument(string path)
+        public static string GetFileInputArgument(string path)
         {
             if (path.IndexOf("://") != -1)
             {