From d552770da968ad16f0808a18d0e5c2d54da9b2b5 Mon Sep 17 00:00:00 2001 From: ta264 Date: Tue, 19 Mar 2019 13:38:42 +0000 Subject: [PATCH] Fixed: Some bugs in new metadata writing (#677) * Fixed: Don't fail reading m4a files when no 'day' tag set * Fixed: Make sure Quality and MediaInfo are set if tag reading failed * Add more tests for AudioTagService --- .../MediaFiles/AudioTagServiceFixture.cs | 73 ++++++++++++++++++- src/NzbDrone.Core/MediaFiles/AudioTag.cs | 61 ++++++++++++---- 2 files changed, 117 insertions(+), 17 deletions(-) diff --git a/src/NzbDrone.Core.Test/MediaFiles/AudioTagServiceFixture.cs b/src/NzbDrone.Core.Test/MediaFiles/AudioTagServiceFixture.cs index f9dc3c9f3..fad3a9cf4 100644 --- a/src/NzbDrone.Core.Test/MediaFiles/AudioTagServiceFixture.cs +++ b/src/NzbDrone.Core.Test/MediaFiles/AudioTagServiceFixture.cs @@ -11,6 +11,7 @@ using System.Collections; using System.Linq; using NzbDrone.Common.Extensions; using System.Collections.Generic; +using NzbDrone.Test.Common; namespace NzbDrone.Core.Test.MediaFiles.AudioTagServiceFixture { @@ -141,7 +142,11 @@ namespace NzbDrone.Core.Test.MediaFiles.AudioTagServiceFixture { var val1 = (IEnumerable) property.GetValue(a, null); var val2 = (IEnumerable) property.GetValue(b, null); - val1.Should().BeEquivalentTo(val2, $"{property.Name} should be equal"); + + if (val1 != null || val2 != null) + { + val1.Should().BeEquivalentTo(val2, $"{property.Name} should be equal"); + } } } } @@ -208,5 +213,71 @@ namespace NzbDrone.Core.Test.MediaFiles.AudioTagServiceFixture tag.MusicBrainzAlbumComment.Should().BeNull(); tag.MusicBrainzReleaseTrackId.Should().BeNull(); } + + [Test, TestCaseSource(typeof(TestCaseFactory), "TestCases")] + public void should_read_audiotag_from_file_with_no_tags(string filename, string[] skipProperties) + { + GivenFileCopy(filename); + var path = copiedFile; + + Subject.RemoveAllTags(path); + + var tag = Subject.ReadAudioTag(path); + var expected = new AudioTag() { + Performers = new string[0], + AlbumArtists = new string[0] + }; + + VerifySame(tag, expected, skipProperties); + tag.Quality.Should().NotBeNull(); + tag.MediaInfo.Should().NotBeNull(); + } + + [Test, TestCaseSource(typeof(TestCaseFactory), "TestCases")] + public void should_read_parsedtrackinfo_from_file_with_no_tags(string filename, string[] skipProperties) + { + GivenFileCopy(filename); + var path = copiedFile; + + Subject.RemoveAllTags(path); + + var tag = Subject.ReadTags(path); + + tag.Quality.Should().NotBeNull(); + tag.MediaInfo.Should().NotBeNull(); + } + + [Test, TestCaseSource(typeof(TestCaseFactory), "TestCases")] + public void should_set_quality_and_mediainfo_for_corrupt_file(string filename, string[] skipProperties) + { + // use missing to simulate corrupt + var tag = Subject.ReadAudioTag(filename.Replace("nin", "missing")); + var expected = new AudioTag(); + + VerifySame(tag, expected, skipProperties); + tag.Quality.Should().NotBeNull(); + tag.MediaInfo.Should().NotBeNull(); + + ExceptionVerification.ExpectedErrors(1); + } + + [Test, TestCaseSource(typeof(TestCaseFactory), "TestCases")] + public void should_read_file_with_only_title_tag(string filename, string[] ignored) + { + GivenFileCopy(filename); + var path = copiedFile; + + Subject.RemoveAllTags(path); + + var nametag = new AudioTag(); + nametag.Title = "test"; + nametag.Write(path); + + var tag = Subject.ReadTags(path); + tag.Title.Should().Be("test"); + + tag.Quality.Should().NotBeNull(); + tag.MediaInfo.Should().NotBeNull(); + } } } diff --git a/src/NzbDrone.Core/MediaFiles/AudioTag.cs b/src/NzbDrone.Core/MediaFiles/AudioTag.cs index 6ca3ba5b1..9e711222c 100644 --- a/src/NzbDrone.Core/MediaFiles/AudioTag.cs +++ b/src/NzbDrone.Core/MediaFiles/AudioTag.cs @@ -150,7 +150,7 @@ namespace NzbDrone.Core.MediaFiles { var appletag = (TagLib.Mpeg4.AppleTag) file.GetTag(TagTypes.Apple); Media = appletag.GetDashBox("com.apple.iTunes", "MEDIA"); - Date = DateTime.TryParse(appletag.DataBoxes(FixAppleId("day")).First().Text, out tempDate) ? tempDate : default(DateTime?); + Date = DateTime.TryParse(appletag.DataBoxes(FixAppleId("day")).FirstOrDefault()?.Text, out tempDate) ? tempDate : default(DateTime?); OriginalReleaseDate = DateTime.TryParse(appletag.GetDashBox("com.apple.iTunes", "Original Date"), out tempDate) ? tempDate : default(DateTime?); MusicBrainzAlbumComment = appletag.GetDashBox("com.apple.iTunes", "MusicBrainz Album Comment"); MusicBrainzReleaseTrackId = appletag.GetDashBox("com.apple.iTunes", "MusicBrainz Release Track Id"); @@ -168,11 +168,7 @@ namespace NzbDrone.Core.MediaFiles if (bitrate == 0) { // Taglib can't read bitrate for Opus. - // Taglib File.Length is unreliable so use System.IO - var size = new System.IO.FileInfo(path).Length; - var duration = file.Properties.Duration.TotalSeconds; - bitrate = (int) ((size * 8L) / (duration * 1024)); - Logger.Trace($"Estimating bitrate. Size: {size} Duration: {duration} Bitrate: {bitrate}"); + bitrate = EstimateBitrate(file, path); } Logger.Debug("Audio Properties: " + acodec.Description + ", Bitrate: " + bitrate + ", Sample Size: " + @@ -193,17 +189,22 @@ namespace NzbDrone.Core.MediaFiles IsValid = true; } - catch (CorruptFileException ex) - { - Logger.Warn(ex, $"Tag reading failed for {path}. File is corrupt"); - } catch (Exception ex) { - Logger.Warn() - .Exception(ex) - .Message($"Tag reading failed for {path}") - .WriteSentryWarn("Tag reading failed") - .Write(); + if (ex is CorruptFileException) + { + Logger.Warn(ex, $"Tag reading failed for {path}. File is corrupt"); + } + else + { + // Log as error so it goes to sentry with correct fingerprint + Logger.Error(ex, "Tag reading failed for {0}", path); + } + + // make sure these are initialized to avoid errors later on + Quality = QualityParser.ParseQuality(path, null, EstimateBitrate(file, path)); + Logger.Debug($"Quality parsed: {Quality}, Source: {Quality.QualityDetectionSource}"); + MediaInfo = new MediaInfoModel(); } finally { @@ -211,6 +212,25 @@ namespace NzbDrone.Core.MediaFiles } } + private int EstimateBitrate(TagLib.File file, string path) + { + int bitrate = 0; + try + { + // Taglib File.Length is unreliable so use System.IO + var size = new System.IO.FileInfo(path).Length; + var duration = file.Properties.Duration.TotalSeconds; + bitrate = (int) ((size * 8L) / (duration * 1024)); + + Logger.Trace($"Estimating bitrate. Size: {size} Duration: {duration} Bitrate: {bitrate}"); + } + catch + { + } + + return bitrate; + } + private DateTime? ReadId3Date(TagLib.Id3v2.Tag tag, string dateTag) { string date = tag.GetTextAsString(dateTag); @@ -300,6 +320,11 @@ namespace NzbDrone.Core.MediaFiles public void Write(string path) { Logger.Debug($"Starting tag write for {path}"); + + // patch up any null fields to work around TagLib exception for + // WMA with null performers/albumartists + Performers = Performers ?? new string[0]; + AlbumArtists = AlbumArtists ?? new string[0]; TagLib.File file = null; try @@ -544,7 +569,11 @@ namespace NzbDrone.Core.MediaFiles { if (!tag.IsValid) { - return new ParsedTrackInfo { Language = Language.English }; + return new ParsedTrackInfo { + Language = Language.English, + Quality = tag.Quality ?? new QualityModel { Quality = NzbDrone.Core.Qualities.Quality.Unknown }, + MediaInfo = tag.MediaInfo ?? new MediaInfoModel() + }; } var artist = tag.AlbumArtists?.FirstOrDefault();