diff --git a/CHANGELOG.md b/CHANGELOG.md index d5356ac4d..68de3c0eb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/). - [SIL.Windows.Forms.Clearshare] New ILicenseWithImage interface handles "GetImage" method for Winforms-dependent licenses, implemented in CreativeCommonsLicense and CustomLicense, and formerly included in LicenseInfo. - [SIL.Core.Clearshare] New tests MetadataBareTests are based on previous MetadataTests in SIL.Windows.Forms.Clearshare. The tests were updated to use ImageSharp instead of Winforms for handling images. - [SIL.Core.Desktop] Added a constant (kBrowserCompatibleUserAgent) to RobustNetworkOperation: a browser-like User Agent string that can be used when making HTTP requests to strict servers. +- [SIL.Core] Added an Exception property to NonFatalErrorReportExpected to return the previous reported non-fatal exception. +- [SIL.Media] Added a static PlaybackErrorMessage property to AudioFactory and a public const, kDefaultPlaybackErrorMessage, that will be used as the default message if the client does not set PlaybackErrorMessage. ### Fixed - [SIL.DictionaryServices] Fix memory leak in LiftWriter @@ -31,6 +33,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). - [SIL.WritingSystems] More fixes to consistently use 繁体中文 and 简体中文 for Traditional and Simplified Chinese native language names, and Chinese (Traditional) and Chinese (Simplified) for their English names. - [SIL.Windows.Forms] Prevent BetterLabel from responding to OnTextChanged when it has been disposed. - [SIL.Windows.Forms] Prevent ContributorsListControl.GetContributionFromRow from throwing an exception when the DataGridView has no valid rows selected. +- [SIL.Media] BREAKING CHANGE (subtle and unlikely): WindowsAudioSession.OnPlaybackStopped now passes itself as the sender instead of a private implementation object, making the event arguments correct. ### Changed - [SIL.Windows.Forms.i18n, SIL.Core.Desktop.i18n] BREAKING CHANGE: Move L10NSharpLocalizer from Windows.Forms to Core.Desktop so it can be accessed without Winforms dependency. @@ -51,6 +54,9 @@ and this project adheres to [Semantic Versioning](http://semver.org/). - [SIL.Windows.Forms] BREAKING CHANGE: Removed optional moreSelected parameter from ToolStripExtensions.InitializeWithAvailableUILocales method. This parameter was no longer being used. Clients that want to have a More menu item that performs a custom action will now need to add it themselves. - [SIL.Windows.Forms] BREAKING CHANGE: LocalizationIncompleteDlg's EmailAddressForLocalizationRequests is no longer autopopulated from LocalizationManager. A new optional constructor parameter, emailAddressForLocalizationRequests, can be used instead. If not supplied, the "More information" controls will be hidden. - [SIL.Core.Desktop] Added an optional userAgentHeader parameter to DoHttpGetAndGetProxyInfo to allow a client to mimic a real browser if necessary. +- [SIL.Media] ISimpleAudioWithEvents now extends ISimpleAudioSession. Technically a breaking change, but the only class in this library that implements it already implemented ISimpleAudioSession. Any custom implementations by clients would very likely have implemented both as well. +- [SIL.Core] Changed the Message property on NonFatalErrorReportExpected (presumably intended for use only in tests) to return the message of the previous reported non-fatal exception if no ordninary non-fatal message has been reported. +- [SIL.Media] In the event of an audio playback error in Windows, the non-fatal exception reported will also include an accompanying (localizable/customizable) user-friendly message. ### Removed - [SIL.Windows.Forms] In .NET 8 builds, removed Scanner and Camera options from the Image Toolbox. diff --git a/Palaso.sln.DotSettings b/Palaso.sln.DotSettings index b94e70997..b35dda771 100644 --- a/Palaso.sln.DotSettings +++ b/Palaso.sln.DotSettings @@ -129,10 +129,12 @@ True True True + True True True True True True + True True True \ No newline at end of file diff --git a/SIL.Core/Reporting/ErrorReport.cs b/SIL.Core/Reporting/ErrorReport.cs index 4667e89ec..7c0a6e214 100644 --- a/SIL.Core/Reporting/ErrorReport.cs +++ b/SIL.Core/Reporting/ErrorReport.cs @@ -6,6 +6,7 @@ using System.Reflection; using System.Runtime.InteropServices; using System.Text; +using JetBrains.Annotations; using Microsoft.Win32; using SIL.IO; using SIL.PlatformUtilities; @@ -29,10 +30,12 @@ public interface IErrorReporter void NotifyUserOfProblem(IRepeatNoticePolicy policy, Exception exception, string message); /// - /// Notify the user of the , if permits. Customize the alternate button label. Wait for user input. Return button clicked to the user. + /// Notify the user of the , if permits. + /// Customize the alternate button label. Wait for user input. Return button clicked to the user. /// /// The method overload should block and wait for the user to press the required button - /// The method should return if the alternate button is clicked, ErrorResult.OK otherwise + /// The method should return if + /// the alternate button is clicked; otherwise ErrorResult NotifyUserOfProblem(IRepeatNoticePolicy policy, string alternateButton1Label, ErrorResult resultIfAlternateButtonPressed, @@ -144,7 +147,7 @@ public static void SetErrorReporter(IErrorReporter reporter) protected static string s_emailAddress = null; protected static string s_emailSubject = "Exception Report"; private static Action s_onShowDetails; - private static bool s_justRecordNonFatalMessagesForTesting=false; + private static bool s_justRecordNonFatalMessagesForTesting = false; private static string s_previousNonFatalMessage; private static Exception s_previousNonFatalException; @@ -177,23 +180,22 @@ public static string GetVersionForErrorReporting() return ReflectionHelper.LongVersionNumberString; } + [PublicAPI] public static object GetAssemblyAttribute(Type attributeType) { Assembly assembly = Assembly.GetEntryAssembly(); if (assembly != null) { - object[] attributes = - assembly.GetCustomAttributes(attributeType, false); - if (attributes != null && attributes.Length > 0) - { + object[] attributes = assembly.GetCustomAttributes(attributeType, false); + if (attributes.Length > 0) return attributes[0]; - } } return null; } public static string VersionNumberString => ReflectionHelper.VersionNumberString; + [PublicAPI] public static string UserFriendlyVersionString { get @@ -220,40 +222,37 @@ public static string DotNet4VersionFromWindowsRegistry() if (!Platform.IsWindows) return string.Empty; - using (var key = Registry.LocalMachine.OpenSubKey(@"SOFTWARE\Microsoft\NET Framework Setup\NDP\v4\Full")) - { - return key == null ? "(unable to determine)" : $"{key.GetValue("Version")} ({key.GetValue("Release")})"; - } + using var key = Registry.LocalMachine.OpenSubKey(@"SOFTWARE\Microsoft\NET Framework Setup\NDP\v4\Full"); + return key == null ? "(unable to determine)" : $"{key.GetValue("Version")} ({key.GetValue("Release")})"; #endif } /// /// use this in unit tests to cleanly check that a message would have been shown. - /// E.g. using (new SIL.Reporting.ErrorReport.NonFatalErrorReportExpected()) {...} + /// E.g., using (new SIL.Reporting.ErrorReport.NonFatalErrorReportExpected()) {...} /// public class NonFatalErrorReportExpected :IDisposable { - private readonly bool previousJustRecordNonFatalMessagesForTesting; + private readonly bool _previousJustRecordNonFatalMessagesForTesting; public NonFatalErrorReportExpected() { - previousJustRecordNonFatalMessagesForTesting = s_justRecordNonFatalMessagesForTesting; + _previousJustRecordNonFatalMessagesForTesting = s_justRecordNonFatalMessagesForTesting; s_justRecordNonFatalMessagesForTesting = true; s_previousNonFatalMessage = null;//this is a static, so a previous unit test could have filled it with something (yuck) } public void Dispose() { - s_justRecordNonFatalMessagesForTesting= previousJustRecordNonFatalMessagesForTesting; + s_justRecordNonFatalMessagesForTesting = _previousJustRecordNonFatalMessagesForTesting; if (s_previousNonFatalException == null && s_previousNonFatalMessage == null) throw new Exception("Non Fatal Error Report was expected but wasn't generated."); s_previousNonFatalMessage = null; + s_previousNonFatalException = null; } /// /// use this to check the actual contents of the message that was triggered /// - public string Message - { - get { return s_previousNonFatalMessage; } - } + public string Message => s_previousNonFatalMessage ?? s_previousNonFatalException?.Message; + public Exception Exception => s_previousNonFatalException; } /// @@ -280,10 +279,7 @@ public void Dispose() /// /// use this to check the actual contents of the message that was triggered /// - public string Message - { - get { return s_previousNonFatalMessage; } - } + public string Message => s_previousNonFatalMessage; } /// @@ -291,8 +287,8 @@ public string Message /// public static string EmailAddress { - set { s_emailAddress = value; } - get { return s_emailAddress; } + set => s_emailAddress = value; + get => s_emailAddress; } /// @@ -300,8 +296,8 @@ public static string EmailAddress /// public static string EmailSubject { - set { s_emailSubject = value; } - get { return s_emailSubject; } + set => s_emailSubject = value; + get => s_emailSubject; } /// @@ -317,14 +313,8 @@ public static string EmailSubject /// public static Action OnShowDetails { - get - { - return s_onShowDetails ?? (s_onShowDetails = ReportNonFatalExceptionWithMessage); - } - set - { - s_onShowDetails = value; - } + get => s_onShowDetails ?? (s_onShowDetails = ReportNonFatalExceptionWithMessage); + set => s_onShowDetails = value; } /// ------------------------------------------------------------------------------------ @@ -358,7 +348,7 @@ public static void AddStandardProperties() else AddProperty("DotNetVersion", DotNet4VersionFromWindowsRegistry()); // https://docs.microsoft.com/en-us/dotnet/api/system.environment.version?view=netframework-4.8 warns that using the Version property is - // no longer recommended for .NET Framework 4.5 or later. I'm leaving it in in hope that it may once again become useful. Note that, as + // no longer recommended for .NET Framework 4.5 or later. I'm leaving it in with the hope that it may once again become useful. Note that, as // of .NET Framework 4.8, it is *not* marked as deprecated. // https://www.mono-project.com/docs/about-mono/versioning/#framework-versioning (accessed 2020-04-02) states that // "Mono's System.Environment.Version property [. . .] should be the same version number that .NET would return." @@ -400,7 +390,7 @@ class Version private readonly PlatformID _platform; private readonly int _major; private readonly int _minor; - public string Label { get; private set; } + public string Label { get; } public Version(PlatformID platform, int major, int minor, string label) { @@ -421,9 +411,11 @@ public static string GetOperatingSystemLabel() { if (Environment.OSVersion.Platform == PlatformID.Unix) { - var startInfo = new ProcessStartInfo("lsb_release", "-si -sr -sc"); - startInfo.RedirectStandardOutput = true; - startInfo.UseShellExecute = false; + var startInfo = new ProcessStartInfo("lsb_release", "-si -sr -sc") + { + RedirectStandardOutput = true, + UseShellExecute = false + }; var proc = new Process { StartInfo = startInfo }; try { @@ -434,7 +426,7 @@ public static string GetOperatingSystemLabel() var si = proc.StandardOutput.ReadLine(); var sr = proc.StandardOutput.ReadLine(); var sc = proc.StandardOutput.ReadLine(); - return String.Format("{0} {1} {2}", si, sr, sc); + return $"{si} {sr} {sc}"; } } catch (Exception) @@ -445,17 +437,19 @@ public static string GetOperatingSystemLabel() else { // https://msdn.microsoft.com/en-us/library/windows/desktop/ms724832%28v=vs.85%29.aspx - var list = new List(); - list.Add(new Version(PlatformID.Win32NT, 5, 0, "Windows 2000")); - list.Add(new Version(PlatformID.Win32NT, 5, 1, "Windows XP")); - list.Add(new Version(PlatformID.Win32NT, 6, 0, "Vista")); - list.Add(new Version(PlatformID.Win32NT, 6, 1, "Windows 7")); - // After Windows 8 the Environment.OSVersion started misreporting information unless - // your app has a manifest which says it supports the OS it is running on. This is not - // helpful if someone starts using an app built before the OS is released. Anything that - // reports its self as Windows 8 is suspect, and must get the version info another way. - list.Add(new Version(PlatformID.Win32NT, 6, 3, "Windows 8.1")); - list.Add(new Version(PlatformID.Win32NT, 10, 0, "Windows 10")); + var list = new List + { + new Version(PlatformID.Win32NT, 5, 0, "Windows 2000"), + new Version(PlatformID.Win32NT, 5, 1, "Windows XP"), + new Version(PlatformID.Win32NT, 6, 0, "Vista"), + new Version(PlatformID.Win32NT, 6, 1, "Windows 7"), + // After Windows 8 the Environment.OSVersion started misreporting information unless + // your app has a manifest which says it supports the OS it is running on. This is not + // helpful if someone starts using an app built before the OS is released. Anything that + // reports its self as Windows 8 is suspect, and must get the version info another way. + new Version(PlatformID.Win32NT, 6, 3, "Windows 8.1"), + new Version(PlatformID.Win32NT, 10, 0, "Windows 10") + }; foreach (var version in list) { @@ -529,9 +523,7 @@ public static ErrorResult NotifyUserOfProblem(IRepeatNoticePolicy policy, { var message = string.Format(messageFmt, args); return NotifyUserOfProblemWrapper(message, null, () => - { - return _errorReporter.NotifyUserOfProblem(policy, alternateButton1Label, resultIfAlternateButtonPressed, message); - }); + _errorReporter.NotifyUserOfProblem(policy, alternateButton1Label, resultIfAlternateButtonPressed, message)); } /// @@ -540,7 +532,7 @@ public static ErrorResult NotifyUserOfProblem(IRepeatNoticePolicy policy, /// * Checks if s_justRecordNonFatalMessagesForTesting is true and if so, skips calling the notification/reporting logic. /// * Calls UsageReporter.ReportException if exception is non-null /// - /// The message that would be showed to the user + /// The message that would be shown to the user /// The associated exception. May be null. /// A delegate that takes no arguments. /// It should call (via a closure) the actual core logic of having the _errorReporter notify the user of the problem. @@ -551,6 +543,7 @@ protected static ErrorResult NotifyUserOfProblemWrapper(string message, Exceptio if (s_justRecordNonFatalMessagesForTesting) { s_previousNonFatalMessage = message; + s_previousNonFatalException = exception; return ErrorResult.OK; } @@ -564,7 +557,7 @@ protected static ErrorResult NotifyUserOfProblemWrapper(string message, Exceptio } /// - /// Bring up a "yellow box" that let's them send in a report, then return to the program. + /// Bring up a "yellow box" that lets them send in a report, then return to the program. /// This version assumes the message has already been formatted with any arguments. /// public static void ReportNonFatalExceptionWithMessage(Exception error, string message) @@ -575,8 +568,9 @@ public static void ReportNonFatalExceptionWithMessage(Exception error, string me } /// - /// Bring up a "yellow box" that let's them send in a report, then return to the program. + /// Bring up a "yellow box" that lets them send in a report, then return to the program. /// + [PublicAPI] public static void ReportNonFatalExceptionWithMessage(Exception error, string message, params object[] args) { s_previousNonFatalMessage = message; @@ -585,7 +579,7 @@ public static void ReportNonFatalExceptionWithMessage(Exception error, string me } /// - /// Bring up a "yellow box" that let's them send in a report, then return to the program. + /// Bring up a "yellow box" that lets them send in a report, then return to the program. /// Use this one only when you don't have an exception (else you're not reporting the exception's message) /// public static void ReportNonFatalMessageWithStackTrace(string message, params object[] args) @@ -593,8 +587,9 @@ public static void ReportNonFatalMessageWithStackTrace(string message, params ob s_previousNonFatalMessage = message; _errorReporter.ReportNonFatalMessageWithStackTrace(message, args); } + /// - /// Bring up a "green box" that let's them send in a report, then exit. + /// Bring up a "green box" that lets them send in a report, then exit. /// public static void ReportFatalMessageWithStackTrace(string message, params object[] args) { @@ -616,7 +611,7 @@ public static void ReportNonFatalException(Exception exception, IRepeatNoticePol { if (s_justRecordNonFatalMessagesForTesting) { - ErrorReport.s_previousNonFatalException = exception; + s_previousNonFatalException = exception; return; } _errorReporter.ReportNonFatalException(exception, policy); @@ -664,10 +659,7 @@ public bool ShouldShowMessage(string message) return true; } - public string ReoccurrenceMessage - { - get { return string.Empty; } - } + public string ReoccurrenceMessage => string.Empty; } public class ShowOncePerSessionBasedOnExactMessagePolicy :IRepeatNoticePolicy @@ -687,10 +679,7 @@ public bool ShouldShowMessage(string message) return true; } - public string ReoccurrenceMessage - { - get { return "This message will not be shown again this session."; } - } + public string ReoccurrenceMessage => "This message will not be shown again this session."; public static void Reset() { diff --git a/SIL.Media.Tests/AudioSessionTests.cs b/SIL.Media.Tests/AudioSessionTests.cs index c80b8b46c..d3051fbce 100644 --- a/SIL.Media.Tests/AudioSessionTests.cs +++ b/SIL.Media.Tests/AudioSessionTests.cs @@ -5,9 +5,11 @@ using System.Media; using System.Threading; using System.Windows.Forms; +using NAudio.Wave; using NUnit.Framework; using SIL.IO; using SIL.Media.Tests.Properties; +using SIL.Reporting; using SIL.TestUtilities; namespace SIL.Media.Tests @@ -28,89 +30,69 @@ public void CheckPlatformSupport() [Test] public void StopRecordingAndSaveAsWav_NotRecording_Throws() { - using (var x = AudioFactory.CreateAudioSession(Path.GetRandomFileName())) - { - Assert.Throws(() => x.StopRecordingAndSaveAsWav()); - } + using var x = AudioFactory.CreateAudioSession(Path.GetRandomFileName()); + Assert.Throws(() => x.StopRecordingAndSaveAsWav()); } [Test] public void StopRecordingAndSaveAsWav_WhileRecording_NoExceptionThrown() { - using (var session = new RecordingSession(100)) - { - session.Recorder.StartRecording(); - Thread.Sleep(100); - session.Recorder.StopRecordingAndSaveAsWav(); - } + using var session = new RecordingSession(100); + session.Recorder.StartRecording(); + Thread.Sleep(100); + session.Recorder.StopRecordingAndSaveAsWav(); } [Test] [NUnit.Framework.Category("RequiresAudioInputDevice")] public void Play_WhileRecording_Throws() { - using (var session = new RecordingSession()) - { - Assert.Throws(() => session.Recorder.Play()); - } + using var session = new RecordingSession(); + Assert.Throws(() => session.Recorder.Play()); } [Test] public void CanRecord_FileDoesNotExist_True() { - using (var x = AudioFactory.CreateAudioSession(Path.GetRandomFileName())) - { - Assert.That(x.FilePath, Does.Not.Exist, "SETUP condition not met"); - Assert.IsTrue(x.CanRecord); - } + using var x = AudioFactory.CreateAudioSession(Path.GetRandomFileName()); + Assert.That(x.FilePath, Does.Not.Exist, "SETUP condition not met"); + Assert.IsTrue(x.CanRecord); } [Test] public void CanStop_NonExistentFile_False() { - using (var x = AudioFactory.CreateAudioSession(Path.GetRandomFileName())) - { - Assert.That(x.FilePath, Does.Not.Exist, "SETUP condition not met"); - Assert.IsFalse(x.CanStop); - } + using var x = AudioFactory.CreateAudioSession(Path.GetRandomFileName()); + Assert.That(x.FilePath, Does.Not.Exist, "SETUP condition not met"); + Assert.IsFalse(x.CanStop); } [Test] public void CanRecord_ConstructWithEmptyFile_True() { - using (var f = new TempFile()) - { - using (var x = AudioFactory.CreateAudioSession(f.Path)) - { - Assert.That(x.FilePath, Is.EqualTo(f.Path), "SETUP condition not met"); - Assert.That(x.FilePath, Does.Exist, "SETUP condition not met"); - Assert.IsTrue(x.CanRecord); - } - } + using var f = new TempFile(); + using var x = AudioFactory.CreateAudioSession(f.Path); + Assert.That(x.FilePath, Is.EqualTo(f.Path), "SETUP condition not met"); + Assert.That(x.FilePath, Does.Exist, "SETUP condition not met"); + Assert.IsTrue(x.CanRecord); } [Test] public void Play_FileEmpty_Throws() { - using (var f = new TempFile()) - { - using (var x = AudioFactory.CreateAudioSession(f.Path)) - { - Assert.That(x.FilePath, Is.EqualTo(f.Path), "SETUP condition not met"); - Assert.That(x.FilePath, Does.Exist, "SETUP condition not met"); - Assert.Throws(() => x.Play()); - } - } + using var f = new TempFile(); + using var x = AudioFactory.CreateAudioSession(f.Path); + Assert.That(x.FilePath, Is.EqualTo(f.Path), "SETUP condition not met"); + Assert.That(x.FilePath, Does.Exist, "SETUP condition not met"); + Assert.Throws(() => x.Play()); } [Test] public void Play_FileDoesNotExist_Throws() { - using (var x = AudioFactory.CreateAudioSession(Path.GetRandomFileName())) - { - Assert.That(x.FilePath, Does.Not.Exist, "SETUP condition not met"); - Assert.Throws(() => x.Play()); - } + using var x = AudioFactory.CreateAudioSession(Path.GetRandomFileName()); + Assert.That(x.FilePath, Does.Not.Exist, "SETUP condition not met"); + Assert.Throws(() => x.Play()); } /// @@ -123,58 +105,48 @@ public void Play_FileDoesNotExist_Throws() [NUnit.Framework.Category("RequiresAudioOutputDevice")] public void CanStop_WhilePlaying_True() { - using (var file = TempFile.FromResource(Resources.finished, ".wav")) - { - using (var x = AudioFactory.CreateAudioSession(file.Path)) - { - x.Play(); - Thread.Sleep(200); - Assert.That(x.CanStop, Is.True, "Playback should last more than 200 ms."); - // We used to test this in a separate test: - // StopPlaying_WhilePlaying_NoExceptionThrown - // But now that would be redundant since we need to stop playback in order to - // safely dispose. - x.StopPlaying(); - } - } + using var file = GetTempAudioFile("wav"); + using var x = AudioFactory.CreateAudioSession(file.Path); + x.Play(); + Thread.Sleep(200); + Assert.That(x.CanStop, Is.True, "Playback should last more than 200 ms."); + // We used to test this in a separate test: + // StopPlaying_WhilePlaying_NoExceptionThrown + // But now that would be redundant since we need to stop playback in order to + // safely dispose. + x.StopPlaying(); } [Test] [NUnit.Framework.Category("RequiresAudioInputDevice")] public void RecordAndStop_FileAlreadyExists_FileReplaced() { - using (var f = new TempFile()) + using var f = new TempFile(); + var oldInfo = new FileInfo(f.Path); + var oldLength = oldInfo.Length; + Assert.AreEqual(0, oldLength); + var oldTimestamp = oldInfo.LastWriteTimeUtc; + using (var x = AudioFactory.CreateAudioSession(f.Path)) { - var oldInfo = new FileInfo(f.Path); - var oldLength = oldInfo.Length; - Assert.AreEqual(0, oldLength); - var oldTimestamp = oldInfo.LastWriteTimeUtc; - using (var x = AudioFactory.CreateAudioSession(f.Path)) - { - x.StartRecording(); - Thread.Sleep(1000); - x.StopRecordingAndSaveAsWav(); - } - var newInfo = new FileInfo(f.Path); - Assert.Greater(newInfo.LastWriteTimeUtc, oldTimestamp); - Assert.Greater(newInfo.Length, oldLength); + x.StartRecording(); + Thread.Sleep(1000); + x.StopRecordingAndSaveAsWav(); } + var newInfo = new FileInfo(f.Path); + Assert.Greater(newInfo.LastWriteTimeUtc, oldTimestamp); + Assert.Greater(newInfo.Length, oldLength); } [Test] [NUnit.Framework.Category("RequiresAudioInputDevice")] public void IsRecording_WhileRecording_True() { - using (var f = new TempFile()) - { - using (var x = AudioFactory.CreateAudioSession(f.Path)) - { - x.StartRecording(); - Thread.Sleep(100); - Assert.IsTrue(x.IsRecording); - x.StopRecordingAndSaveAsWav(); - } - } + using var f = new TempFile(); + using var x = AudioFactory.CreateAudioSession(f.Path); + x.StartRecording(); + Thread.Sleep(100); + Assert.IsTrue(x.IsRecording); + x.StopRecordingAndSaveAsWav(); } [Test] @@ -182,67 +154,55 @@ public void IsRecording_WhileRecording_True() [NUnit.Framework.Category("RequiresAudioInputDevice")] public void RecordThenPlay_SmokeTest() { - using (var f = new TempFile()) - { - var w = new BackgroundWorker(); - // ReSharper disable once RedundantDelegateCreation - w.DoWork += new DoWorkEventHandler((o, args) => SystemSounds.Exclamation.Play()); + using var f = new TempFile(); + using var w = new BackgroundWorker(); + // ReSharper disable once RedundantDelegateCreation + w.DoWork += new DoWorkEventHandler((o, args) => SystemSounds.Exclamation.Play()); - using (var x = AudioFactory.CreateAudioSession(f.Path)) + using var x = AudioFactory.CreateAudioSession(f.Path); + x.StartRecording(); + w.RunWorkerAsync(); + Thread.Sleep(1000); + x.StopRecordingAndSaveAsWav(); + bool stopped = false; + bool isPlayingInEventHandler = false; + ((ISimpleAudioWithEvents)x).PlaybackStopped += (o, args) => + { + // assert here is swallowed, probably because not receiving exceptions from background worker. + // We want to check that isPlaying is false even during the event handler. + isPlayingInEventHandler = x.IsPlaying; + stopped = true; + }; + var watch = Stopwatch.StartNew(); + x.Play(); + while (!stopped) + { + Thread.Sleep(20); + Application.DoEvents(); + if (watch.ElapsedMilliseconds > 2000) { - x.StartRecording(); - w.RunWorkerAsync(); - Thread.Sleep(1000); - x.StopRecordingAndSaveAsWav(); - bool stopped = false; - bool isPlayingInEventHandler = false; - ((ISimpleAudioWithEvents)x).PlaybackStopped += (o, args) => - { - // assert here is swallowed, probably because not receiving exceptions from background worker. - // We want to check that isPlaying is false even during the event handler. - isPlayingInEventHandler = x.IsPlaying; - stopped = true; - }; - var watch = Stopwatch.StartNew(); - x.Play(); - while (!stopped) - { - Thread.Sleep(20); - Application.DoEvents(); - if (watch.ElapsedMilliseconds > 2000) - { - x.StopPlaying(); - Assert.Fail("stop event not received"); - } - } - // After playback is stopped we shouldn't be reporting that it is playing - Assert.That(isPlayingInEventHandler, Is.False); - Assert.That(x.IsPlaying, Is.False); + x.StopPlaying(); + Assert.Fail("stop event not received"); } } + // After playback is stopped we shouldn't be reporting that it is playing + Assert.That(isPlayingInEventHandler, Is.False); + Assert.That(x.IsPlaying, Is.False); } [Test] [NUnit.Framework.Category("RequiresAudioInputDevice")] public void Play_GiveThaiFileName_ShouldHearTinklingSounds() { - using (var file = TempFile.FromResource(Resources.finished, ".wav")) - { - using (var d = TemporaryFolder.Create(TestContext.CurrentContext)) - { - var soundPath = d.Combine("ก.wav"); - RobustFile.Copy(file.Path, soundPath); - using (var f = TempFile.TrackExisting(soundPath)) - { - using (var y = AudioFactory.CreateAudioSession(f.Path)) - { - y.Play(); - Thread.Sleep(1000); - y.StopPlaying(); - } - } - } - } + using var file = GetTempAudioFile("wav"); + using var d = TemporaryFolder.Create(TestContext.CurrentContext); + var soundPath = d.Combine("ก.wav"); + RobustFile.Copy(file.Path, soundPath); + using var f = TempFile.TrackExisting(soundPath); + using var y = AudioFactory.CreateAudioSession(f.Path); + y.Play(); + Thread.Sleep(1000); + y.StopPlaying(); } /// @@ -292,39 +252,31 @@ public void Dispose() [NUnit.Framework.Category("RequiresAudioInputDevice")] public void CanStop_WhileRecording_True() { - using (var session = new RecordingSession()) - { - Assert.IsTrue(session.Recorder.CanStop); - } + using var session = new RecordingSession(); + Assert.IsTrue(session.Recorder.CanStop); } [Test] [NUnit.Framework.Category("RequiresAudioInputDevice")] public void CanPlay_WhileRecording_False() { - using (var session = new RecordingSession()) - { - Assert.IsFalse(session.Recorder.CanPlay); - } + using var session = new RecordingSession(); + Assert.IsFalse(session.Recorder.CanPlay); } [Test] [NUnit.Framework.Category("RequiresAudioInputDevice")] public void CanRecord_WhileRecording_False() { - using (var session = new RecordingSession()) - { - Assert.IsFalse(session.Recorder.CanRecord); - } + using var session = new RecordingSession(); + Assert.IsFalse(session.Recorder.CanRecord); } [Test] public void StartRecording_WhileRecording_Throws() { - using (var session = new RecordingSession()) - { - Assert.Throws(() => session.Recorder.StartRecording()); - } + using var session = new RecordingSession(); + Assert.Throws(() => session.Recorder.StartRecording()); } /// @@ -338,18 +290,16 @@ public void StartRecording_WhileRecording_Throws() [NUnit.Framework.Category("RequiresAudioInputDevice")] public void CanRecord_WhilePlaying_False() { - using (var session = new RecordingSession(1000)) - { - session.Recorder.Play(); - Thread.Sleep(100); - Assert.That(session.Recorder.IsPlaying, Is.True, - "Should be playing, not recording."); - Assert.That(session.Recorder.CanRecord, Is.False); - } + using var session = new RecordingSession(1000); + session.Recorder.Play(); + Thread.Sleep(100); + Assert.That(session.Recorder.IsPlaying, Is.True, + "Should be playing, not recording."); + Assert.That(session.Recorder.CanRecord, Is.False); } /// - /// For reasons I don't entirely understand, this test will actually pass when run + /// For reasons which I don't entirely understand, this test will actually pass when run /// by itself against a single target framework without an audio output device, but /// to get it to pass when running as part of the fixture or when testing against both /// frameworks, it is necessary to have an audio output device. I tried setting the @@ -360,55 +310,39 @@ public void CanRecord_WhilePlaying_False() [NUnit.Framework.Category("RequiresAudioOutputDevice")] public void CanPlay_WhilePlaying_False() { - using (var file = TempFile.FromResource(Resources.finished, ".wav")) - { - using (var x = AudioFactory.CreateAudioSession(file.Path)) - { - x.Play(); - Thread.Sleep(200); - Assert.That(x.CanPlay, Is.False, "Playback should last more than 200 ms."); - x.StopPlaying(); - } - } + using var file = GetTempAudioFile("wav"); + using var x = AudioFactory.CreateAudioSession(file.Path); + x.Play(); + Thread.Sleep(200); + Assert.That(x.CanPlay, Is.False, "Playback should last more than 200 ms."); + x.StopPlaying(); } [Test] public void IsRecording_AfterRecording_False() { - using (var f = new TempFile()) - { - using (ISimpleAudioSession x = RecordSomething(f)) - { - Assert.IsFalse(x.IsRecording); - } - } + using var f = new TempFile(); + using ISimpleAudioSession x = RecordSomething(f); + Assert.IsFalse(x.IsRecording); } [Test] public void RecordThenStop_CanPlay_IsTrue() { - using (var f = new TempFile()) - { - using (ISimpleAudioSession x = RecordSomething(f)) - { - Assert.IsTrue(x.CanPlay); - } - } + using var f = new TempFile(); + using ISimpleAudioSession x = RecordSomething(f); + Assert.IsTrue(x.CanPlay); } [Test] [NUnit.Framework.Category("RequiresAudioInputDevice")] public void RecordThenPlay_OK() { - using (var f = new TempFile()) - { - using (ISimpleAudioSession x = RecordSomething(f)) - { - x.Play(); - Thread.Sleep(100); - x.StopPlaying(); - } - } + using var f = new TempFile(); + using ISimpleAudioSession x = RecordSomething(f); + x.Play(); + Thread.Sleep(100); + x.StopPlaying(); } private static ISimpleAudioSession RecordSomething(TempFile f) @@ -420,54 +354,149 @@ private static ISimpleAudioSession RecordSomething(TempFile f) return x; } - [Test] - public void Play_DoesPlay () + /// + /// Tests using the to get an appropriate player for the + /// current hardware platform and calling to + /// play an audio file and then calling . + /// Note that often -- but not necessarily always -- the request to stop may occur before + /// playback has actually begun. + /// + [TestCase("wav")] + [TestCase("mp3")] + public void PlayAndStopPlaying_NonWindows_DoesNotThrow(string type) { - using (var file = TempFile.FromResource(Resources.finished, ".wav")) + using var file = GetTempAudioFile(type); + using var session = AudioFactory.CreateAudioSession(file.Path); + +#if NET462 || NET48 // On Windows, substitute a test device that simulates a longer media file. + if (session is WindowsAudioSession windowsAudioSession) + windowsAudioSession.TestNAudioOutputDevice = + new TestNAudioWaveOutEvent(); +#endif + + Assert.That(session.IsPlaying, Is.False); + Assert.DoesNotThrow(() => session.Play()); + // Note that the following does not necessarily mean the actual playback has begun, but + // at least it has been queued to start. + Assert.That(session.IsPlaying, Is.True); + Assert.DoesNotThrow(() => session.StopPlaying()); + if (session.IsPlaying) { - using (var x = AudioFactory.CreateAudioSession(file.Path)) +#if NET462 || NET48 + Thread.Sleep(TestNAudioWaveOutEvent.kDelayWhenStopping * 2); +#else + Thread.Sleep(1000); +#endif + Assert.That(session.IsPlaying, Is.False, + "Stop playing should have (immediately or eventually) stopped playback."); + } + } + +#if NET462 || NET48 // These tests won't compile in .NET 8 because WindowsAudioSession is not + // available on that platform. None of the tests in this fixture run in .NET 8. + + /// + /// Tests that *on Windows* the gets a player that implements + /// . Then it tests calling + /// to begin playing an audio file and calling + /// , ensuring that we are notified when + /// playback stops. + /// + [TestCase("wav")] + [TestCase("mp3")] + [Platform(Exclude = "Linux", Reason = "AudioAlsaSession doesn't implement ISimpleAudioWithEvents")] + public void PlayAndStopPlaying_Windows_TrackingOfPlaybackStopped(string type) + { + var playbackCompleted = new ManualResetEventSlim(false); + try + { + bool isPlayingValueInsidePlaybackStopped = true; + // The file gets disposed after playback stops. + using var file = GetTempAudioFile(type); + using var x = AudioFactory.CreateAudioSession(file.Path); + if (!(x is ISimpleAudioWithEvents session)) { - Assert.DoesNotThrow(() => x.Play()); - Assert.DoesNotThrow(() => x.StopPlaying()); + Assert.Fail( + "Expected a player that could inform caller when playback stops."); + return; } + + ((WindowsAudioSession)session).TestNAudioOutputDevice = + new TestNAudioWaveOutEvent(); + + Assert.That(session.IsPlaying, Is.False); + Assert.DoesNotThrow(() => session.Play()); + // Note that the following does not necessarily mean the actual playback has begun, but + // at least it has been queued to start. + Assert.That(session.IsPlaying, Is.True); + Thread.Sleep(100); // Give the playback a chance to actually start. + session.PlaybackStopped += (sender, f) => + { + playbackCompleted.Set(); + if (ReferenceEquals(sender, session)) + isPlayingValueInsidePlaybackStopped = session.IsPlaying; + else + Assert.Fail("PlaybackStopped sender was not the session instance."); + }; + Assert.DoesNotThrow(() => session.StopPlaying()); + Assert.That(session.IsPlaying, Is.True, + "Stop playing merely issued the request, but does not force a hard status change."); + Assert.That(playbackCompleted.Wait(TestNAudioWaveOutEvent.kDelayWhenStopping * 2), Is.True, + "PlaybackStopped event was not raised."); + + Assert.That(isPlayingValueInsidePlaybackStopped, Is.False); + } + finally + { + playbackCompleted.Dispose(); } } + /// + /// Tests that when attempting to play a file whose contents are not a valid audio file, the error is reported as a non-fatal error. + /// [Test] - [Platform(Exclude = "Linux", Reason = "AudioAlsaSession doesn't implement ISimpleAudioWithEvents")] - public void Play_DoesPlayMp3_SmokeTest() + [Platform(Exclude = "Linux", Reason = "Not sure where Linux implementation would fail")] + [Timeout(4000)] + public void Play_InvalidAudioFileThrowsBackgroundException_NonFatalErrorReported() { - // file disposed after playback stopped - using var file = TempFile.FromResource(Resources.ShortMp3, ".mp3"); - using (var x = AudioFactory.CreateAudioSession(file.Path)) + try { - ((ISimpleAudioWithEvents)x).PlaybackStopped += (e, f) => + AudioFactory.PlaybackErrorMessage = "Yikes!"; + using var e = new ErrorReport.NonFatalErrorReportExpected(); + using var file = new TempFile("not valid audio"); + using var session = + (ISimpleAudioWithEvents)AudioFactory.CreateAudioSession(file.Path); + Exception reportedExceptionInPlaybackStopped = null; + session.PlaybackStopped += (sender, args) => { - Debug.WriteLine(f); + reportedExceptionInPlaybackStopped = ((StoppedEventArgs)args).Exception; }; - Assert.That(x.IsPlaying, Is.False); - Assert.DoesNotThrow(() => x.Play()); - Assert.That(x.IsPlaying, Is.True); - Assert.DoesNotThrow(() => x.StopPlaying()); - Assert.That(x.IsPlaying, Is.False); + Assert.DoesNotThrow(() => session.Play(), + "Error should not happen in main thread"); + while (session.IsPlaying) + Thread.Sleep(20); + Assert.That(e.Exception, Is.EqualTo(reportedExceptionInPlaybackStopped)); + Assert.That(e.Message, Is.EqualTo("Yikes!")); + } + finally + { + AudioFactory.PlaybackErrorMessage = null; } } +#endif [Test] [NUnit.Framework.Category("RequiresAudioInputDevice")] public void Record_DoesRecord () { - using (var folder = TemporaryFolder.Create(TestContext.CurrentContext)) - { - string fPath = folder.Combine("dump.ogg"); - using (var x = AudioFactory.CreateAudioSession(fPath)) - { - Assert.DoesNotThrow(() => x.StartRecording()); - Assert.IsTrue(x.IsRecording); - Thread.Sleep(1000); - Assert.DoesNotThrow(() => x.StopRecordingAndSaveAsWav()); - } - } + using var folder = TemporaryFolder.Create(TestContext.CurrentContext); + string fPath = folder.Combine("dump.ogg"); + using var x = AudioFactory.CreateAudioSession(fPath); + Assert.DoesNotThrow(() => x.StartRecording()); + Assert.IsTrue(x.IsRecording); + Thread.Sleep(1000); + Assert.DoesNotThrow(() => x.StopRecordingAndSaveAsWav()); } [Test] @@ -475,27 +504,30 @@ public void Record_DoesRecord () [Explicit] // This test is to be run manually to test long recordings. After the first beep, recite John 3:16" public void Record_LongRecording() { - using (var folder = TemporaryFolder.Create(TestContext.CurrentContext)) - { - string fPath = Path.Combine(folder.Path, "long.wav"); - using (var x = AudioFactory.CreateAudioSession(fPath)) - { - SystemSounds.Beep.Play(); - Assert.DoesNotThrow(() => x.StartRecording()); - Assert.IsTrue(x.IsRecording); - Thread.Sleep(10000); // Records 10 seconds - Assert.DoesNotThrow(() => x.StopRecordingAndSaveAsWav()); - SystemSounds.Beep.Play(); - Assert.IsTrue(x.CanPlay); - Assert.DoesNotThrow(() => x.Play()); - Thread.Sleep(4000); // Plays the first 4 seconds - Assert.DoesNotThrow(() => x.StopPlaying()); - Thread.Sleep(500); // Pause - Assert.DoesNotThrow(() => x.Play()); - Thread.Sleep(6000); // Plays the first 6 seconds - Assert.DoesNotThrow(() => x.StopPlaying()); - } - } + using var folder = TemporaryFolder.Create(TestContext.CurrentContext); + string fPath = Path.Combine(folder.Path, "long.wav"); + using var x = AudioFactory.CreateAudioSession(fPath); + SystemSounds.Beep.Play(); + Assert.DoesNotThrow(() => x.StartRecording()); + Assert.IsTrue(x.IsRecording); + Thread.Sleep(10000); // Records 10 seconds + Assert.DoesNotThrow(() => x.StopRecordingAndSaveAsWav()); + SystemSounds.Beep.Play(); + Assert.IsTrue(x.CanPlay); + Assert.DoesNotThrow(() => x.Play()); + Thread.Sleep(4000); // Plays the first 4 seconds + Assert.DoesNotThrow(() => x.StopPlaying()); + Thread.Sleep(500); // Pause + Assert.DoesNotThrow(() => x.Play()); + Thread.Sleep(6000); // Plays the first 6 seconds + Assert.DoesNotThrow(() => x.StopPlaying()); + } + + private TempFile GetTempAudioFile(string type) + { + return type == "wav" + ? TempFile.FromResource(Resources.finished, $".{type}") + : TempFile.FromResource(Resources.ShortMp3, $".{type}"); } } } diff --git a/SIL.Media.Tests/TestNAudioWaveOutEvent.cs b/SIL.Media.Tests/TestNAudioWaveOutEvent.cs new file mode 100644 index 000000000..95170d488 --- /dev/null +++ b/SIL.Media.Tests/TestNAudioWaveOutEvent.cs @@ -0,0 +1,75 @@ +using System; +using NAudio.Wave; +using Timer = System.Timers.Timer; + +#if NET462 || NET48 +namespace SIL.Media +{ + internal class TestNAudioWaveOutEvent : INAudioOutputDevice + { + public const int kDelayWhenStopping = 50; + private readonly double _simulatedMediaTime; + private ISampleProvider _provider; + private Timer _timer; + private object _lock = new object(); + + public event EventHandler PlaybackStopped; + + internal TestNAudioWaveOutEvent(double simulatedMediaTime = 10000) + { + _simulatedMediaTime = simulatedMediaTime; + } + + public void Dispose() + { + } + + public PlaybackState PlaybackState { get; private set; } + public void Init(IWaveProvider provider) + { + if (_provider != null) + throw new InvalidOperationException("Do not call Init more than once!"); + _provider = provider.ToSampleProvider() ?? throw new ArgumentNullException(nameof(provider)); + PlaybackState = PlaybackState.Stopped; + } + + public void Play() + { + _provider.Take(new TimeSpan(1)); // This will throw if not backed by a legit file. + lock (_lock) + { + _timer = new Timer(_simulatedMediaTime); + _timer.Elapsed += (sender, args) => + { + lock (_lock) + MediaPlaybackStopped(); + }; + _timer.Start(); + PlaybackState = PlaybackState.Playing; + } + } + + public void Stop() + { + lock (_lock) + { + if (_provider != null && PlaybackState == PlaybackState.Playing) + _timer.Interval = kDelayWhenStopping; + else + MediaPlaybackStopped(); + } + } + + private void MediaPlaybackStopped() + { + _timer?.Dispose(); + + if (PlaybackState == PlaybackState.Playing) + { + PlaybackState = PlaybackState.Stopped; + PlaybackStopped?.Invoke(this, new StoppedEventArgs()); + } + } + } +} +#endif \ No newline at end of file diff --git a/SIL.Media/AssemblyInfo.cs b/SIL.Media/AssemblyInfo.cs new file mode 100644 index 000000000..315b9ec34 --- /dev/null +++ b/SIL.Media/AssemblyInfo.cs @@ -0,0 +1,9 @@ +// Copyright (c) 2026 SIL Global +// This software is licensed under the MIT License (http://opensource.org/licenses/MIT) + +using System; +using System.Runtime.CompilerServices; + +[assembly: CLSCompliant(true)] + +[assembly: InternalsVisibleTo("SIL.Media.Tests")] diff --git a/SIL.Media/AudioFactory.cs b/SIL.Media/AudioFactory.cs index bd0bf8277..84a002cc3 100644 --- a/SIL.Media/AudioFactory.cs +++ b/SIL.Media/AudioFactory.cs @@ -9,6 +9,7 @@ namespace SIL.Media { public class AudioFactory { + public const string kDefaultPlaybackErrorMessage = "An error occurred during audio playback."; /// /// Creates an audio session for the given file path. @@ -77,5 +78,12 @@ private static void RedirectIrrKlangAssembly() AppDomain.CurrentDomain.AssemblyResolve += handler; } #endif + + /// + /// Message to show when an error occurs during audio playback. Client can set this to + /// display a localized/customized message. If not set, the default message + /// () will be used. + /// + public static string PlaybackErrorMessage { get; set; } } } diff --git a/SIL.Media/INAudioOutputDevice.cs b/SIL.Media/INAudioOutputDevice.cs new file mode 100644 index 000000000..478740997 --- /dev/null +++ b/SIL.Media/INAudioOutputDevice.cs @@ -0,0 +1,14 @@ +using System; +using NAudio.Wave; + +namespace SIL.Media +{ + internal interface INAudioOutputDevice : IDisposable + { + event EventHandler PlaybackStopped; + PlaybackState PlaybackState { get; } + void Init(IWaveProvider provider); + void Play(); + void Stop(); + } +} diff --git a/SIL.Media/ISimpleAudioSession.cs b/SIL.Media/ISimpleAudioSession.cs index 8b515c410..d8adced00 100644 --- a/SIL.Media/ISimpleAudioSession.cs +++ b/SIL.Media/ISimpleAudioSession.cs @@ -1,4 +1,5 @@ using System; +using JetBrains.Annotations; namespace SIL.Media { @@ -14,11 +15,12 @@ public interface ISimpleAudioSession : IDisposable bool CanStop { get; } bool CanPlay { get; } void Play(); + [PublicAPI] void SaveAsWav(string filePath); void StopPlaying(); } - public interface ISimpleAudioWithEvents + public interface ISimpleAudioWithEvents : ISimpleAudioSession { event EventHandler PlaybackStopped; } diff --git a/SIL.Media/NaudioWaveOutEventAdapter.cs b/SIL.Media/NaudioWaveOutEventAdapter.cs new file mode 100644 index 000000000..1060b723c --- /dev/null +++ b/SIL.Media/NaudioWaveOutEventAdapter.cs @@ -0,0 +1,46 @@ +using System; +using NAudio.Wave; + +#if NET462 || NET48 +namespace SIL.Media +{ + internal class NAudioWaveOutEventAdapter : INAudioOutputDevice + { + private readonly WaveOutEvent _impl; + + public event EventHandler PlaybackStopped; + + internal NAudioWaveOutEventAdapter() + { + _impl = new WaveOutEvent(); + _impl.PlaybackStopped += PlaybackStoppedImplementation; + } + + private void PlaybackStoppedImplementation(object sender, StoppedEventArgs e) + { + PlaybackStopped?.Invoke(this, e); + } + + public void Dispose() + { + _impl.Dispose(); + } + + public PlaybackState PlaybackState => _impl.PlaybackState; + public void Init(IWaveProvider provider) + { + _impl.Init(provider); + } + + public void Play() + { + _impl.Play(); + } + + public void Stop() + { + _impl.Stop(); + } + } +} +#endif \ No newline at end of file diff --git a/SIL.Media/WindowsAudioSession.cs b/SIL.Media/WindowsAudioSession.cs index bede8d56b..bbbd6d62b 100644 --- a/SIL.Media/WindowsAudioSession.cs +++ b/SIL.Media/WindowsAudioSession.cs @@ -1,23 +1,24 @@ -// Copyright (c) 2015-2025 SIL Global +// Copyright (c) 2015-2026 SIL Global // This software is licensed under the MIT License (http://opensource.org/licenses/MIT) -/// Not supported in .NET 8+ due to IrrKlang +// Not supported in .NET 8+ due to IrrKlang #if NET462 || NET48 using System; -using System.ComponentModel; using System.IO; +using System.Threading.Tasks; using IrrKlang; using NAudio.Wave; using SIL.Code; +using SIL.Reporting; namespace SIL.Media { /// /// A Windows implementation of an ISimpleAudioSession. - /// Uses IrrKlang for recording and NAudio for playback. + /// Uses for recording and for playback. /// - internal class WindowsAudioSession : ISimpleAudioSession, ISimpleAudioWithEvents + internal class WindowsAudioSession : ISimpleAudioWithEvents { private readonly IrrKlang.IAudioRecorder _recorder; private readonly ISoundEngine _engine = CreateSoundEngine(); @@ -25,8 +26,11 @@ internal class WindowsAudioSession : ISimpleAudioSession, ISimpleAudioWithEvents private DateTime _startRecordingTime; private DateTime _stopRecordingTime; private readonly SoundFile _soundFile; - private WaveOutEvent _outputDevice; + private INAudioOutputDevice _outputDevice; private AudioFileReader _audioFile; + internal INAudioOutputDevice TestNAudioOutputDevice; + + private readonly object _lock = new object(); /// /// Will be raised when playing is over /// @@ -53,9 +57,13 @@ private static ISoundEngine CreateSoundEngine() return new ISoundEngine(SoundOutputDriver.NullDriver); } } + private INAudioOutputDevice CreateNAudioOutputDevice() + { + return TestNAudioOutputDevice ?? new NAudioWaveOutEventAdapter(); + } /// - /// Constructor for an AudioSession using the IrrKlang library + /// Constructor for an AudioSession using the library /// public WindowsAudioSession(string filePath) { @@ -77,7 +85,6 @@ public void StartRecording() _recorder.StartRecordingBufferedAudio(22000, SampleFormat.Signed16Bit, 1); _startRecordingTime = DateTime.Now; - } public void StopRecordingAndSaveAsWav() @@ -107,7 +114,14 @@ public double LastRecordingMilliseconds public bool IsRecording => _recorder != null && _recorder.IsRecording; - public bool IsPlaying { get; set; } + public bool IsPlaying + { + get + { + lock(_lock) + return _outputDevice != null; + } + } public bool CanRecord => !IsPlaying && !IsRecording; @@ -117,29 +131,45 @@ public double LastRecordingMilliseconds private void OnPlaybackStopped(object sender, StoppedEventArgs args) { - lock (FilePath) + lock (_lock) + CleanupPlaybackResources(); + + PlaybackStopped?.Invoke(this, args); + } + + private void CleanupPlaybackResources() + { + if (_outputDevice != null) { - if (_outputDevice != null) + _outputDevice.PlaybackStopped -= OnPlaybackStopped; + try { + // Dispose calls Stop, which can throw. _outputDevice.Dispose(); - _outputDevice = null; - if (_audioFile != null) - { - _audioFile.Dispose(); - _audioFile = null; - } } + catch (Exception e) + { + Console.WriteLine(e); + // We're disposing. We probably don't care what went wrong. + } + + if (_audioFile != null) + { + _audioFile.Dispose(); + _audioFile = null; + } + + _outputDevice = null; } - IsPlaying = false; - PlaybackStopped?.Invoke(sender, args); } /// - /// The current version of Play uses NAudio for playback. IrrKlang had issues with playback. - /// In the future it may be best to try the latest version of IrrKlang and see if true safe - /// cross-platform recording and playback can be accomplished now. This would eliminate the need for - /// the AlsaAudio classes on linux. Note: irrKlang was upgraded to v. 1.6 in Nov 2024, but I did - /// not re-check to see if it works for playback on all platforms. + /// The current version of Play uses NAudio for playback. had issues + /// with playback. In the future it may be best to try the latest version of + /// and see if true safe cross-platform recording and playback can + /// be accomplished now. This would eliminate the need for the + /// classes on linux. Note: was upgraded to v. 1.6 in Nov 2024, but + /// I did not re-check to see if it works for playback on all platforms. /// public void Play() { @@ -150,41 +180,45 @@ public void Play() if (new FileInfo(FilePath).Length == 0) throw new FileLoadException("Trying to play empty file"); - var worker = new BackgroundWorker(); - IsPlaying = true; - worker.DoWork += (sender, args) => + lock (_lock) + { + if (_outputDevice != null) + return; // Already playing. Don't start again. + _outputDevice = CreateNAudioOutputDevice(); + } + + Task.Run(() => { try { - lock (FilePath) + lock (_lock) { if (_outputDevice == null) { - _outputDevice = new WaveOutEvent(); - _outputDevice.PlaybackStopped += OnPlaybackStopped; + // Playback was stopped before it even got started. Don't try to start it now. + return; } + + _outputDevice.PlaybackStopped += OnPlaybackStopped; if (_audioFile == null) { _audioFile = new AudioFileReader(FilePath); _outputDevice.Init(_audioFile); } + _outputDevice.Play(); } } catch (Exception e) { - // Try to clean things up as best we can...no easy way to test this, though. - // We don't want to be permanently in the playing state. - IsPlaying = false; - // And, in case something critical is waiting for this... OnPlaybackStopped(this, new StoppedEventArgs(e)); // Maybe the system has another way of playing it that works? e.g., most default players will handle mp3. // But it seems risky...maybe we will be trying to play another sound or do some recording? // Decided not to do this for now. - // The main thread has gone on with other work, don't have any current way to report the exception. + ErrorReport.ReportNonFatalExceptionWithMessage(e, + AudioFactory.PlaybackErrorMessage ?? AudioFactory.kDefaultPlaybackErrorMessage); } - }; - worker.RunWorkerAsync(); + }); } private class SoundFile : IFileFactory @@ -269,22 +303,33 @@ public void SaveAsWav(string path) public void StopPlaying() { - if (IsPlaying) + lock (_lock) { - OnPlaybackStopped(this, new StoppedEventArgs()); + if (_outputDevice == null) // !IsPlaying + return; + + if (_outputDevice.PlaybackState == PlaybackState.Stopped) + CleanupPlaybackResources(); + else + _outputDevice.Stop(); } + } + + public void Dispose() + { + lock (_lock) + CleanupPlaybackResources(); + try { _engine.RemoveAllSoundSources(); } catch (Exception) { - // We'll just ignore any errors on stopping the sounds (they probably aren't playing). + // We'll just ignore any errors on stopping the sounds (We don't use irrKlang for playback anyway). } - } - - public void Dispose() - { + + _engine?.Dispose(); _recorder.Dispose(); _soundFile.CloseFile(); } diff --git a/SIL.Windows.Forms/Reporting/WinFormsErrorReporter.cs b/SIL.Windows.Forms/Reporting/WinFormsErrorReporter.cs index f34bf4cff..b370dda05 100644 --- a/SIL.Windows.Forms/Reporting/WinFormsErrorReporter.cs +++ b/SIL.Windows.Forms/Reporting/WinFormsErrorReporter.cs @@ -90,7 +90,7 @@ public void ReportNonFatalException(Exception exception, IRepeatNoticePolicy pol } /// - /// Bring up a "yellow box" that let's them send in a report, then return to the program. + /// Bring up a "yellow box" that lets them send in a report, then return to the program. /// public void ReportNonFatalExceptionWithMessage(Exception error, string message, params object[] args) { @@ -99,7 +99,7 @@ public void ReportNonFatalExceptionWithMessage(Exception error, string message, } /// - /// Bring up a "yellow box" that let's them send in a report, then return to the program. + /// Bring up a "yellow box" that lets them send in a report, then return to the program. /// Use this one only when you don't have an exception (else you're not reporting the exception's message) /// public void ReportNonFatalMessageWithStackTrace(string message, params object[] args)