From 8cf028e0715e352b8b1f61cc8497fa71ef8820d8 Mon Sep 17 00:00:00 2001 From: Mark McDowall Date: Sat, 10 Jun 2017 18:07:24 -0700 Subject: [PATCH] Fixed: Improve sample rejection message when MediaInfo is not available Closes #1967 --- .../DownloadedEpisodesImportServiceFixture.cs | 12 +--- ...rviceFixture.cs => DetectSampleFixture.cs} | 62 ++++++------------ .../ImportDecisionMakerFixture.cs | 12 +++- .../MediaInfo/VideoFileInfoReaderFixture.cs | 7 +- .../NzbDrone.Core.Test.csproj | 2 +- .../DownloadedEpisodesImportService.cs | 5 +- .../MediaFiles/EpisodeImport/DetectSample.cs | 64 ++++++------------- .../EpisodeImport/DetectSampleResult.cs | 9 +++ .../EpisodeImport/ImportDecisionMaker.cs | 6 +- .../Specifications/NotSampleSpecification.cs | 9 ++- .../MediaInfo/VideoFileInfoReader.cs | 11 +--- src/NzbDrone.Core/NzbDrone.Core.csproj | 1 + 12 files changed, 76 insertions(+), 124 deletions(-) rename src/NzbDrone.Core.Test/MediaFiles/EpisodeImport/{SampleServiceFixture.cs => DetectSampleFixture.cs} (74%) create mode 100644 src/NzbDrone.Core/MediaFiles/EpisodeImport/DetectSampleResult.cs diff --git a/src/NzbDrone.Core.Test/MediaFiles/DownloadedEpisodesImportServiceFixture.cs b/src/NzbDrone.Core.Test/MediaFiles/DownloadedEpisodesImportServiceFixture.cs index 501741dc5..25866457f 100644 --- a/src/NzbDrone.Core.Test/MediaFiles/DownloadedEpisodesImportServiceFixture.cs +++ b/src/NzbDrone.Core.Test/MediaFiles/DownloadedEpisodesImportServiceFixture.cs @@ -164,11 +164,9 @@ namespace NzbDrone.Core.Test.MediaFiles Mocker.GetMock() .Setup(s => s.IsSample(It.IsAny(), - It.IsAny(), It.IsAny(), - It.IsAny(), It.IsAny())) - .Returns(true); + .Returns(DetectSampleResult.Sample); Subject.ProcessRootFolder(new DirectoryInfo(_droneFactory)); @@ -236,11 +234,9 @@ namespace NzbDrone.Core.Test.MediaFiles Mocker.GetMock() .Setup(s => s.IsSample(It.IsAny(), - It.IsAny(), It.IsAny(), - It.IsAny(), It.IsAny())) - .Returns(true); + .Returns(DetectSampleResult.Sample); Mocker.GetMock() .Setup(s => s.GetFiles(It.IsAny(), SearchOption.AllDirectories)) @@ -347,11 +343,9 @@ namespace NzbDrone.Core.Test.MediaFiles Mocker.GetMock() .Setup(s => s.IsSample(It.IsAny(), - It.IsAny(), It.IsAny(), - It.IsAny(), It.IsAny())) - .Returns(true); + .Returns(DetectSampleResult.Sample); Mocker.GetMock() .Setup(s => s.GetFileSize(It.IsAny())) diff --git a/src/NzbDrone.Core.Test/MediaFiles/EpisodeImport/SampleServiceFixture.cs b/src/NzbDrone.Core.Test/MediaFiles/EpisodeImport/DetectSampleFixture.cs similarity index 74% rename from src/NzbDrone.Core.Test/MediaFiles/EpisodeImport/SampleServiceFixture.cs rename to src/NzbDrone.Core.Test/MediaFiles/EpisodeImport/DetectSampleFixture.cs index dd3090116..303eb68cd 100644 --- a/src/NzbDrone.Core.Test/MediaFiles/EpisodeImport/SampleServiceFixture.cs +++ b/src/NzbDrone.Core.Test/MediaFiles/EpisodeImport/DetectSampleFixture.cs @@ -14,7 +14,7 @@ using NzbDrone.Core.Tv; namespace NzbDrone.Core.Test.MediaFiles.EpisodeImport { [TestFixture] - public class SampleServiceFixture : CoreTest + public class DetectSampleFixture : CoreTest { private Series _series; private LocalEpisode _localEpisode; @@ -42,11 +42,6 @@ namespace NzbDrone.Core.Test.MediaFiles.EpisodeImport }; } - private void GivenFileSize(long size) - { - _localEpisode.Size = size; - } - private void GivenRuntime(int seconds) { Mocker.GetMock() @@ -58,7 +53,7 @@ namespace NzbDrone.Core.Test.MediaFiles.EpisodeImport public void should_return_false_if_season_zero() { _localEpisode.Episodes[0].SeasonNumber = 0; - ShouldBeFalse(); + ShouldBeNotSample(); } [Test] @@ -66,7 +61,7 @@ namespace NzbDrone.Core.Test.MediaFiles.EpisodeImport { _localEpisode.Path = @"C:\Test\some.show.s01e01.flv"; - ShouldBeFalse(); + ShouldBeNotSample(); Mocker.GetMock().Verify(c => c.GetRunTime(It.IsAny()), Times.Never()); } @@ -76,7 +71,7 @@ namespace NzbDrone.Core.Test.MediaFiles.EpisodeImport { _localEpisode.Path = @"C:\Test\some.show.s01e01.strm"; - ShouldBeFalse(); + ShouldBeNotSample(); Mocker.GetMock().Verify(c => c.GetRunTime(It.IsAny()), Times.Never()); } @@ -85,12 +80,9 @@ namespace NzbDrone.Core.Test.MediaFiles.EpisodeImport public void should_use_runtime() { GivenRuntime(120); - GivenFileSize(1000.Megabytes()); Subject.IsSample(_localEpisode.Series, - _localEpisode.Quality, _localEpisode.Path, - _localEpisode.Size, _localEpisode.IsSpecial); Mocker.GetMock().Verify(v => v.GetRunTime(It.IsAny()), Times.Once()); @@ -101,7 +93,7 @@ namespace NzbDrone.Core.Test.MediaFiles.EpisodeImport { GivenRuntime(60); - ShouldBeTrue(); + ShouldBeSample(); } [Test] @@ -109,7 +101,7 @@ namespace NzbDrone.Core.Test.MediaFiles.EpisodeImport { GivenRuntime(600); - ShouldBeFalse(); + ShouldBeNotSample(); } [Test] @@ -118,7 +110,7 @@ namespace NzbDrone.Core.Test.MediaFiles.EpisodeImport _series.Runtime = 6; GivenRuntime(299); - ShouldBeFalse(); + ShouldBeNotSample(); } [Test] @@ -127,7 +119,7 @@ namespace NzbDrone.Core.Test.MediaFiles.EpisodeImport _series.Runtime = 2; GivenRuntime(60); - ShouldBeFalse(); + ShouldBeNotSample(); } [Test] @@ -136,29 +128,19 @@ namespace NzbDrone.Core.Test.MediaFiles.EpisodeImport _series.Runtime = 2; GivenRuntime(10); - ShouldBeTrue(); - } - - [Test] - public void should_fall_back_to_file_size_if_mediainfo_dll_not_found_acceptable_size() - { - Mocker.GetMock() - .Setup(s => s.GetRunTime(It.IsAny())) - .Throws(); - - GivenFileSize(1000.Megabytes()); - ShouldBeFalse(); + ShouldBeSample(); } [Test] - public void should_fall_back_to_file_size_if_mediainfo_dll_not_found_undersize() + public void should_return_indeterminate_if_mediainfo_result_is_null() { Mocker.GetMock() .Setup(s => s.GetRunTime(It.IsAny())) .Throws(); - GivenFileSize(1.Megabytes()); - ShouldBeTrue(); + Subject.IsSample(_localEpisode.Series, + _localEpisode.Path, + _localEpisode.IsSpecial).Should().Be(DetectSampleResult.Indeterminate); } [Test] @@ -167,7 +149,7 @@ namespace NzbDrone.Core.Test.MediaFiles.EpisodeImport GivenRuntime(600); _series.SeriesType = SeriesTypes.Daily; _localEpisode.Episodes[0].SeasonNumber = 0; - ShouldBeFalse(); + ShouldBeNotSample(); } [Test] @@ -176,25 +158,21 @@ namespace NzbDrone.Core.Test.MediaFiles.EpisodeImport _series.SeriesType = SeriesTypes.Anime; _localEpisode.Episodes[0].SeasonNumber = 0; - ShouldBeFalse(); + ShouldBeNotSample(); } - private void ShouldBeTrue() + private void ShouldBeSample() { Subject.IsSample(_localEpisode.Series, - _localEpisode.Quality, - _localEpisode.Path, - _localEpisode.Size, - _localEpisode.IsSpecial).Should().BeTrue(); + _localEpisode.Path, + _localEpisode.IsSpecial).Should().Be(DetectSampleResult.Sample); } - private void ShouldBeFalse() + private void ShouldBeNotSample() { Subject.IsSample(_localEpisode.Series, - _localEpisode.Quality, _localEpisode.Path, - _localEpisode.Size, - _localEpisode.IsSpecial).Should().BeFalse(); + _localEpisode.IsSpecial).Should().Be(DetectSampleResult.NotSample); } } } diff --git a/src/NzbDrone.Core.Test/MediaFiles/EpisodeImport/ImportDecisionMakerFixture.cs b/src/NzbDrone.Core.Test/MediaFiles/EpisodeImport/ImportDecisionMakerFixture.cs index 700bd720d..d69fb7725 100644 --- a/src/NzbDrone.Core.Test/MediaFiles/EpisodeImport/ImportDecisionMakerFixture.cs +++ b/src/NzbDrone.Core.Test/MediaFiles/EpisodeImport/ImportDecisionMakerFixture.cs @@ -333,8 +333,16 @@ namespace NzbDrone.Core.Test.MediaFiles.EpisodeImport GivenVideoFiles(videoFiles.ToList()); Mocker.GetMock() - .Setup(s => s.IsSample(_series, It.IsAny(), It.Is(c => c.Contains("sample")), It.IsAny(), It.IsAny())) - .Returns(true); + .Setup(s => s.IsSample(_series, It.IsAny(), It.IsAny())) + .Returns((Series s, string path, bool special) => + { + if (path.Contains("sample")) + { + return DetectSampleResult.Sample; + } + + return DetectSampleResult.NotSample; + }); var folderInfo = Parser.Parser.ParseTitle("Series.Title.S01E01"); diff --git a/src/NzbDrone.Core.Test/MediaFiles/MediaInfo/VideoFileInfoReaderFixture.cs b/src/NzbDrone.Core.Test/MediaFiles/MediaInfo/VideoFileInfoReaderFixture.cs index 5ccd1e4eb..c4a57f2a2 100644 --- a/src/NzbDrone.Core.Test/MediaFiles/MediaInfo/VideoFileInfoReaderFixture.cs +++ b/src/NzbDrone.Core.Test/MediaFiles/MediaInfo/VideoFileInfoReaderFixture.cs @@ -1,4 +1,4 @@ -using System.IO; +using System.IO; using FluentAssertions; using Moq; using NUnit.Framework; @@ -30,11 +30,9 @@ namespace NzbDrone.Core.Test.MediaFiles.MediaInfo { var path = Path.Combine(TestContext.CurrentContext.TestDirectory, "Files", "Media", "H264_sample.mp4"); - Subject.GetRunTime(path).Seconds.Should().Be(10); - + Subject.GetRunTime(path).Value.Seconds.Should().Be(10); } - [Test] public void get_info() { @@ -86,7 +84,6 @@ namespace NzbDrone.Core.Test.MediaFiles.MediaInfo info.VideoCodec.Should().Be("AVC"); info.VideoFps.Should().Be(24); info.Width.Should().Be(480); - } [Test] diff --git a/src/NzbDrone.Core.Test/NzbDrone.Core.Test.csproj b/src/NzbDrone.Core.Test/NzbDrone.Core.Test.csproj index dd057b641..d770a3eea 100644 --- a/src/NzbDrone.Core.Test/NzbDrone.Core.Test.csproj +++ b/src/NzbDrone.Core.Test/NzbDrone.Core.Test.csproj @@ -284,7 +284,7 @@ - + diff --git a/src/NzbDrone.Core/MediaFiles/DownloadedEpisodesImportService.cs b/src/NzbDrone.Core/MediaFiles/DownloadedEpisodesImportService.cs index 159717a60..ea7491d62 100644 --- a/src/NzbDrone.Core/MediaFiles/DownloadedEpisodesImportService.cs +++ b/src/NzbDrone.Core/MediaFiles/DownloadedEpisodesImportService.cs @@ -113,10 +113,7 @@ namespace NzbDrone.Core.MediaFiles return false; } - var size = _diskProvider.GetFileSize(videoFile); - var quality = QualityParser.ParseQuality(videoFile); - - if (!_detectSample.IsSample(series, quality, videoFile, size, episodeParseResult.IsPossibleSpecialEpisode)) + if (_detectSample.IsSample(series, videoFile, episodeParseResult.IsPossibleSpecialEpisode) != DetectSampleResult.Sample) { _logger.Warn("Non-sample file detected: [{0}]", videoFile); return false; diff --git a/src/NzbDrone.Core/MediaFiles/EpisodeImport/DetectSample.cs b/src/NzbDrone.Core/MediaFiles/EpisodeImport/DetectSample.cs index 0f890eafd..9c5e185b0 100644 --- a/src/NzbDrone.Core/MediaFiles/EpisodeImport/DetectSample.cs +++ b/src/NzbDrone.Core/MediaFiles/EpisodeImport/DetectSample.cs @@ -1,16 +1,14 @@ using System; -using System.Collections.Generic; using System.IO; using NLog; using NzbDrone.Core.MediaFiles.MediaInfo; -using NzbDrone.Core.Qualities; using NzbDrone.Core.Tv; namespace NzbDrone.Core.MediaFiles.EpisodeImport { public interface IDetectSample { - bool IsSample(Series series, QualityModel quality, string path, long size, bool isSpecial); + DetectSampleResult IsSample(Series series, string path, bool isSpecial); } public class DetectSample : IDetectSample @@ -18,22 +16,18 @@ namespace NzbDrone.Core.MediaFiles.EpisodeImport private readonly IVideoFileInfoReader _videoFileInfoReader; private readonly Logger _logger; - private static List _largeSampleSizeQualities = new List { Quality.HDTV1080p, Quality.WEBDL1080p, Quality.Bluray1080p }; - public DetectSample(IVideoFileInfoReader videoFileInfoReader, Logger logger) { _videoFileInfoReader = videoFileInfoReader; _logger = logger; } - public static long SampleSizeLimit => 70.Megabytes(); - - public bool IsSample(Series series, QualityModel quality, string path, long size, bool isSpecial) + public DetectSampleResult IsSample(Series series, string path, bool isSpecial) { if (isSpecial) { _logger.Debug("Special, skipping sample check"); - return false; + return DetectSampleResult.NotSample; } var extension = Path.GetExtension(path); @@ -41,61 +35,39 @@ namespace NzbDrone.Core.MediaFiles.EpisodeImport if (extension != null && extension.Equals(".flv", StringComparison.InvariantCultureIgnoreCase)) { _logger.Debug("Skipping sample check for .flv file"); - return false; + return DetectSampleResult.NotSample; } if (extension != null && extension.Equals(".strm", StringComparison.InvariantCultureIgnoreCase)) { _logger.Debug("Skipping sample check for .strm file"); - return false; + return DetectSampleResult.NotSample; } - try - { - var runTime = _videoFileInfoReader.GetRunTime(path); - var minimumRuntime = GetMinimumAllowedRuntime(series); - - if (runTime.TotalMinutes.Equals(0)) - { - _logger.Error("[{0}] has a runtime of 0, is it a valid video file?", path); - return true; - } - - if (runTime.TotalSeconds < minimumRuntime) - { - _logger.Debug("[{0}] appears to be a sample. Runtime: {1} seconds. Expected at least: {2} seconds", path, runTime, minimumRuntime); - return true; - } - } + var runTime = _videoFileInfoReader.GetRunTime(path); - catch (DllNotFoundException) + if (!runTime.HasValue) { - _logger.Debug("Falling back to file size detection"); - - return CheckSize(size, quality); + _logger.Error("Failed to get runtime from the file, make sure mediainfo is available"); + return DetectSampleResult.Indeterminate; } - _logger.Debug("Runtime is over 90 seconds"); - return false; - } + var minimumRuntime = GetMinimumAllowedRuntime(series); - private bool CheckSize(long size, QualityModel quality) - { + if (runTime.Value.TotalMinutes.Equals(0)) { - if (size < SampleSizeLimit * 2) - { - _logger.Debug("1080p file is less than sample limit"); - return true; - } + _logger.Error("[{0}] has a runtime of 0, is it a valid video file?", path); + return DetectSampleResult.Sample; } - if (size < SampleSizeLimit) + if (runTime.Value.TotalSeconds < minimumRuntime) { - _logger.Debug("File is less than sample limit"); - return true; + _logger.Debug("[{0}] appears to be a sample. Runtime: {1} seconds. Expected at least: {2} seconds", path, runTime, minimumRuntime); + return DetectSampleResult.Sample; } - return false; + _logger.Debug("Runtime is over 90 seconds"); + return DetectSampleResult.NotSample; } private int GetMinimumAllowedRuntime(Series series) diff --git a/src/NzbDrone.Core/MediaFiles/EpisodeImport/DetectSampleResult.cs b/src/NzbDrone.Core/MediaFiles/EpisodeImport/DetectSampleResult.cs new file mode 100644 index 000000000..81266a717 --- /dev/null +++ b/src/NzbDrone.Core/MediaFiles/EpisodeImport/DetectSampleResult.cs @@ -0,0 +1,9 @@ +namespace NzbDrone.Core.MediaFiles.EpisodeImport +{ + public enum DetectSampleResult + { + Indeterminate, + Sample, + NotSample + } +} diff --git a/src/NzbDrone.Core/MediaFiles/EpisodeImport/ImportDecisionMaker.cs b/src/NzbDrone.Core/MediaFiles/EpisodeImport/ImportDecisionMaker.cs index 1931e9a04..859c8111d 100644 --- a/src/NzbDrone.Core/MediaFiles/EpisodeImport/ImportDecisionMaker.cs +++ b/src/NzbDrone.Core/MediaFiles/EpisodeImport/ImportDecisionMaker.cs @@ -170,11 +170,9 @@ namespace NzbDrone.Core.MediaFiles.EpisodeImport return videoFiles.Count(file => { - var size = _diskProvider.GetFileSize(file); - var fileQuality = QualityParser.ParseQuality(file); - var sample = _detectSample.IsSample(series, GetQuality(folderInfo, fileQuality, series), file, size, folderInfo.IsPossibleSpecialEpisode); + var sample = _detectSample.IsSample(series, file, folderInfo.IsPossibleSpecialEpisode); - if (sample) + if (sample == DetectSampleResult.Sample) { return false; } diff --git a/src/NzbDrone.Core/MediaFiles/EpisodeImport/Specifications/NotSampleSpecification.cs b/src/NzbDrone.Core/MediaFiles/EpisodeImport/Specifications/NotSampleSpecification.cs index d482b8806..ea3848d39 100644 --- a/src/NzbDrone.Core/MediaFiles/EpisodeImport/Specifications/NotSampleSpecification.cs +++ b/src/NzbDrone.Core/MediaFiles/EpisodeImport/Specifications/NotSampleSpecification.cs @@ -26,16 +26,19 @@ namespace NzbDrone.Core.MediaFiles.EpisodeImport.Specifications } var sample = _detectSample.IsSample(localEpisode.Series, - localEpisode.Quality, localEpisode.Path, - localEpisode.Size, localEpisode.IsSpecial); - if (sample) + if (sample == DetectSampleResult.Sample) { return Decision.Reject("Sample"); } + else if (sample == DetectSampleResult.Indeterminate) + { + return Decision.Reject("Unable to determine if file is a sample"); + } + return Decision.Accept(); } } diff --git a/src/NzbDrone.Core/MediaFiles/MediaInfo/VideoFileInfoReader.cs b/src/NzbDrone.Core/MediaFiles/MediaInfo/VideoFileInfoReader.cs index a1bf6aa86..d4c2df482 100644 --- a/src/NzbDrone.Core/MediaFiles/MediaInfo/VideoFileInfoReader.cs +++ b/src/NzbDrone.Core/MediaFiles/MediaInfo/VideoFileInfoReader.cs @@ -9,7 +9,7 @@ namespace NzbDrone.Core.MediaFiles.MediaInfo public interface IVideoFileInfoReader { MediaInfoModel GetMediaInfo(string filename); - TimeSpan GetRunTime(string filename); + TimeSpan? GetRunTime(string filename); } public class VideoFileInfoReader : IVideoFileInfoReader @@ -181,16 +181,11 @@ namespace NzbDrone.Core.MediaFiles.MediaInfo return null; } - public TimeSpan GetRunTime(string filename) + public TimeSpan? GetRunTime(string filename) { var info = GetMediaInfo(filename); - if (info == null) - { - return new TimeSpan(); - } - - return info.RunTime; + return info?.RunTime; } private TimeSpan GetBestRuntime(int audio, int video, int general) diff --git a/src/NzbDrone.Core/NzbDrone.Core.csproj b/src/NzbDrone.Core/NzbDrone.Core.csproj index afdd19028..ec0356692 100644 --- a/src/NzbDrone.Core/NzbDrone.Core.csproj +++ b/src/NzbDrone.Core/NzbDrone.Core.csproj @@ -745,6 +745,7 @@ +