From f60f2b43402247bd8a90f4074464e2e450451a56 Mon Sep 17 00:00:00 2001 From: John Andrews Date: Tue, 21 May 2024 07:58:01 +1200 Subject: [PATCH] FF-1554: Fixed bitrate issue in convert audio --- .../Nodes/ConvertFlowElements/ConvertNode.cs | 85 +++++++++--------- AudioNodes/Tests/AudioTestBase.cs | 20 +---- AudioNodes/Tests/ConvertTests.cs | 38 +++++--- AudioNodes/Tests/CreateAudioBookTests.cs | 2 +- AudioNodes/Tests/EmbedArtworkTests.cs | 6 +- AudioNodes/Tests/_TestBase.cs | 70 +++++++++++++++ FileFlows.Plugin.dll | Bin 141312 -> 141312 bytes FileFlows.Plugin.pdb | Bin 34388 -> 34388 bytes 8 files changed, 149 insertions(+), 72 deletions(-) create mode 100644 AudioNodes/Tests/_TestBase.cs diff --git a/AudioNodes/Nodes/ConvertFlowElements/ConvertNode.cs b/AudioNodes/Nodes/ConvertFlowElements/ConvertNode.cs index ef27cc05..852edc83 100644 --- a/AudioNodes/Nodes/ConvertFlowElements/ConvertNode.cs +++ b/AudioNodes/Nodes/ConvertFlowElements/ConvertNode.cs @@ -63,47 +63,44 @@ namespace FileFlows.AudioNodes codec, }; - if (Codec.ToLowerInvariant() == "mp3" || ogg || Codec.ToLowerInvariant() == "aac") + if(bitrate is > 10 and <= 20) { - if (bitrate is >= 10 and <= 20) + bool mp3 = Codec.Equals("mp3", StringComparison.InvariantCultureIgnoreCase); + bool aac = Codec.Equals("aac", StringComparison.InvariantCultureIgnoreCase); + if(mp3 == false && aac == false && ogg == false) + throw new Exception("Variable bitrate not supported in codec: " + Codec); + + bitrate = (Bitrate - 10); + if (mp3) { - bitrate = (Bitrate - 10); - if (Codec.ToLowerInvariant() == "mp3") - { - // ogg is reversed - bitrate = 10 - bitrate; - } - - - args.Logger?.ILog($"Using variable bitrate setting '{bitrate}' for codec '{Codec}'"); - - if (codec == "libfdk_aac") - { - ffArgs.AddRange(new[] - { - "-vbr", - Math.Min(Math.Max(1, bitrate / 2), 5).ToString() - }); - } - else - { - ffArgs.AddRange(new[] - { - "-qscale:a", - bitrate.ToString() - }); - } - - if (Codec == "aac" && HighEfficiency) - { - extension = "m4a"; - ffArgs.AddRange(new[] { "-profile:a", "aac_he_v2" }); - } + // ogg is reversed + bitrate = 10 - bitrate; + } + + args.Logger?.ILog($"Using variable bitrate setting '{bitrate}' for codec '{Codec}'"); + + if (codec == "libfdk_aac") + { + ffArgs.AddRange(new[] + { + "-vbr", + Math.Min(Math.Max(1, bitrate / 2), 5).ToString() + }); + } + else + { + ffArgs.AddRange(new[] + { + "-qscale:a", + bitrate.ToString() + }); + } + + if (Codec == "aac" && HighEfficiency) + { + extension = "m4a"; + ffArgs.AddRange(new[] { "-profile:a", "aac_he_v2" }); } - } - else if(bitrate is > 10 and <= 20) - { - throw new Exception("Variable bitrate not supported in codec: " + Codec); } else if (bitrate != 0) { @@ -275,9 +272,15 @@ namespace FileFlows.AudioNodes /// the output to call next public override int Execute(NodeParameters args) { - AudioInfo AudioInfo = GetAudioInfo(args); - if (AudioInfo == null) + var aiResult = GetAudioInfo(args); + if (aiResult.Failed(out string error)) + { + args.FailureReason = error; + args.Logger.ELog(error); return -1; + } + + AudioInfo AudioInfo = aiResult.Value; var ffmpegExeResult = GetFFmpeg(args); if (ffmpegExeResult.Failed(out string ffmpegError)) @@ -335,7 +338,7 @@ namespace FileFlows.AudioNodes ffArgs.Add(twoPass.Normalization); } } - + var metadata = MetadataHelper.GetMetadataParameters(AudioInfo); if (metadata?.Any() == true) ffArgs.AddRange(metadata); diff --git a/AudioNodes/Tests/AudioTestBase.cs b/AudioNodes/Tests/AudioTestBase.cs index 4a1fc745..a33569ed 100644 --- a/AudioNodes/Tests/AudioTestBase.cs +++ b/AudioNodes/Tests/AudioTestBase.cs @@ -5,25 +5,11 @@ using Microsoft.VisualStudio.TestTools.UnitTesting; namespace AudioNodes.Tests; -public abstract class AudioTestBase +public abstract class AudioTestBase : TestBase { - private TestContext testContextInstance; - - internal TestLogger logger = new (); - - public TestContext TestContext - { - get { return testContextInstance; } - set { testContextInstance = value; } - } - - protected readonly string ffmpeg = (OperatingSystem.IsLinux() ? "/usr/local/bin/ffmpeg" : @"C:\utils\ffmpeg\ffmpeg.exe"); - protected readonly string ffprobe = (OperatingSystem.IsLinux() ? "/usr/local/bin/ffprobe" : @"C:\utils\ffmpeg\ffprobe.exe"); - - protected NodeParameters GetNodeParameters(string file, bool isDirectory = false) { - var args = new FileFlows.Plugin.NodeParameters(file, logger, isDirectory, string.Empty, new LocalFileService()); + var args = new FileFlows.Plugin.NodeParameters(file, Logger, isDirectory, string.Empty, new LocalFileService()); args.GetToolPathActual = (string tool) => { @@ -33,7 +19,7 @@ public abstract class AudioTestBase return ffprobe; return null; }; - args.TempPath = @"/home/john/Music/temp"; + args.TempPath = TempPath; return args; } diff --git a/AudioNodes/Tests/ConvertTests.cs b/AudioNodes/Tests/ConvertTests.cs index b4afe8a5..5401ef60 100644 --- a/AudioNodes/Tests/ConvertTests.cs +++ b/AudioNodes/Tests/ConvertTests.cs @@ -3,6 +3,7 @@ using Microsoft.VisualStudio.TestTools.UnitTesting; using System.IO; using AudioNodes.Tests; +using FileFlows.Plugin.Services; namespace FileFlows.AudioNodes.Tests; @@ -112,17 +113,34 @@ public class ConvertTests : AudioTestBase [TestMethod] - public void Convert_AacToMp3() + public void Convert_Mp3ToMp3_Bitrate() { + var args = GetNodeParameters(TestFile_Mp3); + var af = new AudioFile(); + af.PreExecute(args); + af.Execute(args); // need to read the Audio info and set it + + ConvertToMP3 ele = new(); + ele.PreExecute(args); + ele.Bitrate = 64; + int output = ele.Execute(args); + TestContext.WriteLine(Logger.ToString()); - const string file = @"D:\music\temp\37f315a0-4afc-4a72-a0b4-eb7eb681b9b3.aac"; - - ConvertToMP3 node = new(); - var args = new FileFlows.Plugin.NodeParameters(file, new TestLogger(), false, string.Empty, null);; - args.GetToolPathActual = (string tool) => @"C:\utils\ffmpeg\ffmpeg.exe"; - args.TempPath = @"D:\music\temp"; - new AudioFile().Execute(args); // need to read the Audio info and set it - int output = node.Execute(args); + Assert.AreEqual(1, output); + } + [TestMethod] + public void Convert_Mp3ToMp3_Bitrate_Variable() + { + var args = GetNodeParameters(TestFile_Mp3); + var af = new AudioFile(); + af.PreExecute(args); + af.Execute(args); // need to read the Audio info and set it + + ConvertToMP3 ele = new(); + ele.PreExecute(args); + ele.Bitrate = 3; + int output = ele.Execute(args); + TestContext.WriteLine(Logger.ToString()); Assert.AreEqual(1, output); } @@ -217,7 +235,7 @@ public class ConvertTests : AudioTestBase node.PreExecute(args); int output = node.Execute(args); - string log = logger.ToString(); + string log = Logger.ToString(); TestContext.WriteLine(log); Assert.AreEqual(1, output); diff --git a/AudioNodes/Tests/CreateAudioBookTests.cs b/AudioNodes/Tests/CreateAudioBookTests.cs index 0121d090..81b5300d 100644 --- a/AudioNodes/Tests/CreateAudioBookTests.cs +++ b/AudioNodes/Tests/CreateAudioBookTests.cs @@ -62,7 +62,7 @@ public class CreateAudioBookTests : AudioTestBase int output = node.Execute(args); - var log = logger.ToString(); + var log = Logger.ToString(); TestContext.WriteLine(log); Assert.AreEqual(expected, output); diff --git a/AudioNodes/Tests/EmbedArtworkTests.cs b/AudioNodes/Tests/EmbedArtworkTests.cs index 9eef57a1..9520c6a6 100644 --- a/AudioNodes/Tests/EmbedArtworkTests.cs +++ b/AudioNodes/Tests/EmbedArtworkTests.cs @@ -76,14 +76,14 @@ public class EmbedArtworkTests : AudioTestBase convertNode.PreExecute(args); var result = convertNode.Execute(args); - var log = logger.ToString(); + var log = Logger.ToString(); Assert.AreEqual(1, result); - logger.Clear(); + Logger.Clear(); var ele = new EmbedArtwork(); var output = ele.Execute(args); - log = logger.ToString(); + log = Logger.ToString(); TestContext.WriteLine(log); Assert.AreEqual(1, output); System.IO.File.Move(args.WorkingFile, diff --git a/AudioNodes/Tests/_TestBase.cs b/AudioNodes/Tests/_TestBase.cs new file mode 100644 index 00000000..cffa168e --- /dev/null +++ b/AudioNodes/Tests/_TestBase.cs @@ -0,0 +1,70 @@ +#if(DEBUG) + +using System.Runtime.InteropServices; +using Microsoft.VisualStudio.TestTools.UnitTesting; +using System.IO; + +namespace FileFlows.AudioNodes.Tests; + +[TestClass] +public abstract class TestBase +{ + /// + /// The test context instance + /// + private TestContext testContextInstance; + + internal TestLogger Logger = new(); + + /// + /// Gets or sets the test context + /// + public TestContext TestContext + { + get => testContextInstance; + set => testContextInstance = value; + } + + public string TestPath { get; private set; } + public string TempPath { get; private set; } + public string FfmpegPath { get; private set; } + + public readonly bool IsWindows = RuntimeInformation.IsOSPlatform(OSPlatform.Windows); + public readonly bool IsLinux = RuntimeInformation.IsOSPlatform(OSPlatform.Linux); + + protected readonly string ffmpeg = (OperatingSystem.IsLinux() ? "/usr/local/bin/ffmpeg" : @"C:\utils\ffmpeg\ffmpeg.exe"); + protected readonly string ffprobe = (OperatingSystem.IsLinux() ? "/usr/local/bin/ffprobe" : @"C:\utils\ffmpeg\ffprobe.exe"); + + + [TestInitialize] + public void TestInitialize() + { + this.TestPath = this.TestPath?.EmptyAsNull() ?? (IsLinux ? "~/src/ff-files/test-files/audio" : @"d:\audio\testfiles"); + this.TempPath = this.TempPath?.EmptyAsNull() ?? (IsLinux ? "~/src/ff-files/temp" : @"d:\audio\temp"); + this.FfmpegPath = this.FfmpegPath?.EmptyAsNull() ?? (IsLinux ? "/usr/local/bin/ffmpeg" : @"C:\utils\ffmpeg\ffmpeg.exe"); + + this.TestPath = this.TestPath.Replace("~/", Environment.GetFolderPath(Environment.SpecialFolder.UserProfile) + "/"); + this.TempPath = this.TempPath.Replace("~/", Environment.GetFolderPath(Environment.SpecialFolder.UserProfile) + "/"); + this.FfmpegPath = this.FfmpegPath.Replace("~/", Environment.GetFolderPath(Environment.SpecialFolder.UserProfile) + "/"); + + if (Directory.Exists(this.TempPath) == false) + Directory.CreateDirectory(this.TempPath); + } + + [TestCleanup] + public void CleanUp() + { + TestContext.WriteLine(Logger.ToString()); + } + + protected virtual void TestStarting() + { + + } + + protected string TestFile_Mp3 => Path.Combine(TestPath, "mp3.mp3"); + protected string TestFile_Flac => Path.Combine(TestPath, "flac.flac"); + protected string TestFile_Wav => Path.Combine(TestPath, "wav.wav"); +} + +#endif \ No newline at end of file diff --git a/FileFlows.Plugin.dll b/FileFlows.Plugin.dll index b517791cac6ced12a94c4bbcdf28117211928aa3..29c46d2fc2f863c4ae372be2a4c3e122bdf4492c 100644 GIT binary patch delta 250 zcmZp;z|nAlV?qZD|GSE>jXkYBj9Yt{9JmD9iXD!0bXU92y=ldk*0RZo#O9KD^ delta 250 zcmZp;z|nAlV?qZ@V{!*5ypOMA<`-F?r zYvh>%1RnY>oWAV-Ti->}nT_vC&HhfmBhO^702S>`f{KFFcl>WwDCD%T%R7AQ^LA?m zrmrmiCJbpnn8aYtV8UR`U<#zo87vu67|eid3m~1!U;tzp0{MnO6=^^@5Z{s^8K}n) Y$VvpNNd~I4WH17%G1^|Q%GAgN01F~gegFUf diff --git a/FileFlows.Plugin.pdb b/FileFlows.Plugin.pdb index 221f945e2829e537753e999c3ff8d24a6c115803..41f029127aefb2e24501cb06f700b1260032c1a2 100644 GIT binary patch delta 2846 zcmZwIdr(tX9tZH>O+pf0Av{Gl_yQ3SLIQ*&2=WNTqUK?esug@74?%a8!76GUqjek| zhmO>!1>6p@K8i!Bk7(G^>f+X-6%|E`0;?!2h*m+n6hvv+Z!*=tyfdHQ`Q@D7J@=mb z%AFF{of1~*7N0+49>!P7O-Tp1?fZXrSsnCgbj+8hOY>M$*Tjm*MJ}Rsuo<>Pm7K7N z=zHW>liN1i=si zF)$NSARY9u5SBtNY=d1;rnJv3rb?xlj++r`;VgUy|AwF7CftPw&;xtp#B>n8griUc zXW$AnLo3{e|H3cuD~tfm6cY~w;0q!MgfNhTS%HuUYS4fQGGGN1Kp|{|ZLk|kp$rZ~ zHJpNfLIYfZn{W>vLLa<h#h;T6BNwFF$E1{;Dy2D1R1ab z3ZM{*VJqx}60kr8R5>QMA0V8n)!}p!Y;YbfK@+q9;$=rG4wA%_zl#cfl*-3 z4k8y=1j`@~MxpP6AiAC&MC~vHUUPzI7A%IN@HMo+6DXb=L|t>SO1(Llj?W9GZ@>l@ zpb>6BD?ET+=!YTrW1f8#!Nk=BlK}i+A_RjNVqq4jK?@6Dsm8v~bjowI0dznUKvqXS zkz{7=M2gqi3nr0DJBjig`DFULb~4rfnNQPAp>T-S+3VhJQz=e2l{Pv07}~32(Ldk> zoPs*I06)StxDD;_8Ybvj6bFmJyjjnt3aEl=J%_&4JJFAD4FoDD+A5rt z?D<@B0R?Cw2R?y)Z~`tu8@vQ(1DB>4c$5O8dLE@4__WyIOnC+uDl)j3X%mur3Kx9z zU1*!Zm8>wRaK*>pmFkgSge%Z&aHHo2cN#GW$jK-mH}HlDP^%Qs6tp2m4@xx(NpJL| zOrsYT=7oiMQNGceHo+eF+~`APMqfG#HO9aC(ix*4UIRb6gto~z4p+dR{$m_ZeMS)t z8ts=mlxCYkNe2sI8LWi0Pz+|+1$*HOs5HI3MWOW7yBvQKCOmMWNmBZsNlI^k|KZz0 zdn=>o3&b>N^>!VYq&e((&KP#xq)_FE*&I$dezbDg z^vrDtvEJQ}$EK@-hIl?HOv{t_RI^}tk~3XeK*@XxvwDV~_B78gTIYZIwI-lLq|yY7 zFMQpSkhOUD$WwM(<+JK#u^k7tFMNs%Q)`(!_lG3m<$)0&m2OYW-c=ddY1KRT@T$(H zd30r+DGzczw4`mML(BX-_4yKS|7SH&0%4EVVDN_@dP%H@)$fU#+~L#qw|LRsF8}UF zAyo=(e(7;#_7jD!`9}TCu;ObQq&H-BpVrgwGn+OPR0Xuk7=F&yY1L-@r_J&j3@`!+iUUspl~DflPf2Q?Id}C@?v@ zueTq{ex^M*L95HUA%^L(GV2#HO+rWrV0c4|_~0c1kr47io+e~b3c-Y;RM;XQDs*;yu0y3pV0T$i zJ1SNzMI1oU;+rTa+Jd%26&EWM8Bx1Mu>{zy*mgHQSm}O~DgMQs`TTyroOABE_uNIKx{o9~MT53>1zc5|+R+(7}6<0|l@NK7>-Jg#Azh z4R9H*!ApaXhg6rMrU5{!jx_!(XTV^A$3J_x}Be83+~iY1ModOfmgsJgcA>Dfji6w1*j9Gl$s#* zrLhFdWD&&IM34~bl@a8jv9w9ZMU zfIDyxhF}z)!3&^d%PInhmmEMs@PxUr0Kz~4agYe9uoAMZ`vlTDYo1Nj$+PLAwe}&o zA;pJcQY@3sp@ftXj^e(;4j+9x zD%A<-BA68deEbD;74=up4ZS*hn$S7Wq|T9S^^W8SZr}}Pm5$_#yih-jG9-!)~b3yHbtbjhf)J{sT9;7(o0J$esZ0Z;m;-iyZd-elHW zE_V)M(1Lr+QO#e+2(`&GMcY2^D zOKBoqL}sIlU}8>k!#kEHhJKZ?klEHcU2cQSwbon(51DVRIZ6Sty|rE+>4-cF`1Dw1 zPd_Fw6jP-Q_AngQa?t8PpX){#TVk^w5RWy<1d5mZIpi-?Sz{SusBjojB4l$oBo?ZyF_*&$!H)qhoBnNY zMr?O*w`EheV`mhPBGaI=TF2$3$29@}vIc|EMz`9$Xu9XivkA~qvfXzTvmFJ)2kb*XAOQWal>2B^<1eH{H3x#u0q1g)E$%R1YQT9>S?Of46QtMn^HgcgU>Q?4h!TSOy!_c2JMRm!VwwrnTTQ-KTQ6xUz7}fn< z{Qka>6kg+ZTNFO6x7+(Fl?7KkO$S$%WbK?fyvBGzd;G}p!Kc+djl-qaJ{xiO*m1YI z#?@_^AnnEO)xu&nBdLmx&0P?YpPwU-kVxfnWu8o`$XgR870bjC`N}YsLKpp#;|od#(#zmrp?4$e#*3SnRb7sUB