Fixes culture formatting bug for SoX commands (#243)

* Fixes culture formatting bug for SoX commands

In some cultures, where a comma is used as the decimal separator. In those cases, when we format commands for SoX the numbers would be separated with commas and the command would be invalid.

The commit fixes that by ensuring we only use the invariant culture when formatting floating point types.

Fices #242

* Fixes mistake in comment

AudioAnalysis.sln.DotSettings

@@ -280,8 +280,11 @@
 	<s:Boolean x:Key="/Default/ReSpeller/UserDictionaries/=en_005Fus/Words/=parallelized/@EntryIndexedValue">True</s:Boolean>
 	<s:Boolean x:Key="/Default/ReSpeller/UserDictionaries/=en_005Fus/Words/=spectrums/@EntryIndexedValue">True</s:Boolean>
 	<s:Boolean x:Key="/Default/ReSpeller/UserDictionaries/=en_005Fus/Words/=Truskinger/@EntryIndexedValue">True</s:Boolean>
+	<s:Boolean x:Key="/Default/UserDictionary/Words/=Bioacoustics/@EntryIndexedValue">True</s:Boolean>
 	<s:Boolean x:Key="/Default/UserDictionary/Words/=Chromeless/@EntryIndexedValue">True</s:Boolean>
 	<s:Boolean x:Key="/Default/UserDictionary/Words/=dedupe/@EntryIndexedValue">True</s:Boolean>
+	<s:Boolean x:Key="/Default/UserDictionary/Words/=ffmpeg/@EntryIndexedValue">True</s:Boolean>
+	<s:Boolean x:Key="/Default/UserDictionary/Words/=ffprobe/@EntryIndexedValue">True</s:Boolean>
 	<s:Boolean x:Key="/Default/UserDictionary/Words/=Redirector/@EntryIndexedValue">True</s:Boolean>
 	<s:Boolean x:Key="/Default/UserDictionary/Words/=spectrograms/@EntryIndexedValue">True</s:Boolean>
 	<s:Boolean x:Key="/Default/UserDictionary/Words/=Submatrix/@EntryIndexedValue">True</s:Boolean>

src/Acoustics.Tools/Audio/SoxAudioUtility.cs

@@ -188,15 +188,16 @@ protected override string ConstructModifyArgs(FileInfo source, FileInfo output,
             var trim = string.Empty;
             if (request.OffsetStart.HasValue && !request.OffsetEnd.HasValue)
             {
-                trim = "trim " + request.OffsetStart.Value.TotalSeconds;
+                trim = "trim " + request.OffsetStart.Value.TotalSeconds.ToString(CultureInfo.InvariantCulture);
             }
             else if (!request.OffsetStart.HasValue && request.OffsetEnd.HasValue)
             {
-                trim = "trim 0 " + request.OffsetEnd.Value.TotalSeconds;
+                trim = "trim 0 " + request.OffsetEnd.Value.TotalSeconds.ToString(CultureInfo.InvariantCulture);
             }
             else if (request.OffsetStart.HasValue && request.OffsetEnd.HasValue)
             {
-                trim = "trim " + request.OffsetStart.Value.TotalSeconds + " " + (request.OffsetEnd.Value.TotalSeconds - request.OffsetStart.Value.TotalSeconds);
+                var delta = request.OffsetEnd.Value.TotalSeconds - request.OffsetStart.Value.TotalSeconds;
+                trim = FormattableString.Invariant($"trim {request.OffsetStart.Value.TotalSeconds} {delta}");
             }
 
             var bandpass = string.Empty;
@@ -205,20 +206,21 @@ protected override string ConstructModifyArgs(FileInfo source, FileInfo output,
                 switch (request.BandPassType)
                 {
                     case BandPassType.Sinc:
-                        bandpass += "sinc {0}k-{1}k".Format(request.BandpassLow.Value / 1000, request.BandpassHigh.Value / 1000);
+                        bandpass += FormattableString.Invariant(
+                            $"sinc {request.BandpassLow.Value / 1000}k-{request.BandpassHigh.Value / 1000}k");
                         break;
                     case BandPassType.Bandpass:
                         double width = request.BandpassHigh.Value - request.BandpassLow.Value;
-                        var center = width / 2.0;
-                        bandpass += "bandpass {0}k width{k}".Format(center / 1000, width / 1000);
+                        var center = request.BandpassLow.Value + (width / 2.0);
+                        bandpass += FormattableString.Invariant(
+                            $"bandpass { center / 1000 }k { width / 1000 }k");
                         break;
                     case BandPassType.None:
                     default:
                         throw new ArgumentOutOfRangeException();
                 }
             }
 
-
             // example
             // remix down to 1 channel, medium resample quality using steep filter with target sample rate of 11025hz
             // sox input.wav output.wav remix - rate -m -s 11025

tests/Acoustics.Test/Acoustics.Test.csproj

@@ -341,6 +341,8 @@
     <Compile Include="Shared\RangeTests.cs" />
     <Compile Include="TestHelpers\Factories\AudioRecordingFactory.cs" />
     <Compile Include="TestHelpers\ImageHelpers.cs" />
+    <Compile Include="TestHelpers\LeakyFfmpegAudioUtility.cs" />
+    <Compile Include="TestHelpers\LeakySoxAudioUtility.cs" />
     <Compile Include="TestHelpers\TestImage.cs" />
     <Compile Include="TestHelpers\TestSetup.cs" />
     <Compile Include="Tools\AudioFilePreparerTests.cs" />

tests/Acoustics.Test/TestHelpers/LeakyFfmpegAudioUtility.cs

@@ -0,0 +1,36 @@
+// <copyright file="LeakyFfmpegAudioUtility.cs" company="QutEcoacoustics">
+// All code in this file and all associated files are the copyright and property of the QUT Ecoacoustics Research Group (formerly MQUTeR, and formerly QUT Bioacoustics Research Group).
+// </copyright>
+
+namespace Acoustics.Test.TestHelpers
+{
+    using System.IO;
+    using Acoustics.Tools;
+    using Acoustics.Tools.Audio;
+
+    /// <summary>
+    /// So named because we use it to break our abstractions, to make them leaky.
+    /// </summary>
+    public class LeakyFfmpegAudioUtility : FfmpegAudioUtility
+    {
+        public LeakyFfmpegAudioUtility(FileInfo ffmpegExe, FileInfo ffprobeExe)
+            : base(ffmpegExe, ffprobeExe)
+        {
+        }
+
+        public LeakyFfmpegAudioUtility(FileInfo ffmpegExe, FileInfo ffprobeExe, DirectoryInfo temporaryFilesDirectory)
+            : base(ffmpegExe, ffprobeExe, temporaryFilesDirectory)
+        {
+        }
+
+        public string GetConstructedModifyArguments(FileInfo source, FileInfo output, AudioUtilityRequest request)
+        {
+            return this.ConstructModifyArgs(source, output, request);
+        }
+
+        public string GetConstructedInfoArguments(FileInfo source, FileInfo output, AudioUtilityRequest request)
+        {
+            return this.ConstructInfoArgs(source);
+        }
+    }
+}

tests/Acoustics.Test/TestHelpers/LeakySoxAudioUtility.cs

@@ -0,0 +1,42 @@
+// <copyright file="LeakySourcePreparer.cs" company="QutEcoacoustics">
+// All code in this file and all associated files are the copyright and property of the QUT Ecoacoustics Research Group (formerly MQUTeR, and formerly QUT Bioacoustics Research Group).
+// </copyright>
+
+namespace Acoustics.Test.TestHelpers
+{
+    using System;
+    using System.Collections.Generic;
+    using System.IO;
+    using System.Linq;
+    using System.Text;
+    using System.Threading.Tasks;
+    using Acoustics.Shared;
+    using Acoustics.Tools;
+    using Acoustics.Tools.Audio;
+
+    /// <summary>
+    /// So named because we use it to break our abstractions, to make them leaky.
+    /// </summary>
+    public class LeakySoxAudioUtility : SoxAudioUtility
+    {
+        public LeakySoxAudioUtility(FileInfo soxExe)
+            : base(soxExe)
+        {
+        }
+
+        public LeakySoxAudioUtility(FileInfo soxExe, DirectoryInfo temporaryFilesDirectory, bool enableShortNameHack = true)
+            : base(soxExe, temporaryFilesDirectory, enableShortNameHack)
+        {
+        }
+
+        public string GetConstructedModifyArguments(FileInfo source, FileInfo output, AudioUtilityRequest request)
+        {
+            return this.ConstructModifyArgs(source, output, request);
+        }
+
+        public string GetConstructedInfoArguments(FileInfo source)
+        {
+            return this.ConstructInfoArgs(source);
+        }
+    }
+}

tests/Acoustics.Test/Tools/MasterAudioUtilityTests.cs

@@ -315,17 +315,6 @@ public void SegmentsWvCorrectly()
                 TimeSpan.FromMilliseconds(0));
         }
 
-
-
-        /// <summary>
-        /// The test sox.
-        /// </summary>
-        [TestMethod]
-        public void TestSox()
-        {
-            TestHelper.GetAudioUtility().Info(PathHelper.GetTestAudioFile("TorresianCrow.wav"));
-        }
-
         /// <summary>
         /// The validates non existing exe paths.
         /// </summary>

tests/Acoustics.Test/Tools/SoxUtilityTests.cs

@@ -9,7 +9,9 @@
 namespace Acoustics.Test.Tools
 {
     using System;
+    using System.Globalization;
     using System.IO;
+    using System.Threading;
     using Acoustics.Shared;
     using Acoustics.Tools;
     using Acoustics.Tools.Audio;
@@ -102,7 +104,6 @@ public void SoxResamplingShouldBeDeterministic()
                 repeats[r] = reader.Samples;
 
                 File.Delete(output.FullName);
-
             }
 
             for (int i = 1; i < repeats.Length; i++)
@@ -118,7 +119,64 @@ public void SoxResamplingShouldBeDeterministic()
 
                 CollectionAssert.AreEqual(repeats[0], repeats[i], $"Repeat {i} was not identical to repeat 0. Total delta: {totalDifference}");
             }
+        }
+
+        [DataTestMethod]
+        [DataRow("en-AU", BandPassType.Bandpass)]
+        [DataRow("de-DE", BandPassType.Bandpass)]
+        [DataRow("it-it", BandPassType.Bandpass)]
+        [DataRow("es-AR", BandPassType.Bandpass)]
+        [DataRow("en-AU", BandPassType.Sinc)]
+        [DataRow("de-DE", BandPassType.Sinc)]
+        [DataRow("it-it", BandPassType.Sinc)]
+        [DataRow("es-AR", BandPassType.Sinc)]
+        public void SoxCanSegmentWithDifferentLocales(string culture, BandPassType bandPassType)
+        {
+            var originalCulture = Thread.CurrentThread.CurrentCulture;
+            Thread.CurrentThread.CurrentCulture = new CultureInfo(culture);
+            try
+            {
+                var util = new LeakySoxAudioUtility(PathHelper.GetExe(AppConfigHelper.SoxExe));
+
+                var source = TestHelper.GetAudioFile("CaneToad_Gympie_44100.wav");
+
+                // intentionally testing that commas aren't included in the
+                // constructed commands for numbers as a decimal separator
+                var constructedArguments = util.GetConstructedModifyArguments(source, TempFileHelper.NewTempFile(), new AudioUtilityRequest()
+                {
+                    OffsetStart = 123.456.Seconds(),
+                    OffsetEnd = 789.123.Seconds(),
+                    BandpassHigh = 789.456,
+                    BandpassLow = 123.789,
+                    BandPassType = bandPassType,
+                });
 
+                StringAssert.Contains(constructedArguments, "123.456");
+
+                // end is specified as a duration
+                StringAssert.Contains(constructedArguments, "665.667");
+
+                switch (bandPassType)
+                {
+                    case BandPassType.Sinc:
+                        StringAssert.Contains(constructedArguments, "0.123789");
+                        StringAssert.Contains(constructedArguments, "0.789456");
+                        break;
+                    case BandPassType.Bandpass:
+
+                        // bandpass filter is centre + width hence numbers are different
+                        StringAssert.Contains(constructedArguments, "0.4566225");
+                        StringAssert.Contains(constructedArguments, "0.665667");
+                        break;
+                    case BandPassType.None:
+                    default:
+                        throw new ArgumentOutOfRangeException(nameof(bandPassType), bandPassType, null);
+                }
+            }
+            finally
+            {
+                Thread.CurrentThread.CurrentCulture = originalCulture;
+            }
         }
     }
-}
+}