From 6ede67149126a0f50deb84bee9364a1dc9e88378 Mon Sep 17 00:00:00 2001 From: tombogle Date: Mon, 16 Feb 2026 15:38:45 -0500 Subject: [PATCH 1/6] +semver:major Fixed minor bug in WindowsAudioSession.PlaybackStopped event parameters Fixed potential locking problem relying on string object for locks. Made ISimpleAudioWithEvents extend ISimpleAudioSession. Improved reliability and functionality in unit tests in AudioSessionTests. --- CHANGELOG.md | 2 + SIL.Media.Tests/AudioSessionTests.cs | 65 ++++++++++++++++++++++------ SIL.Media/ISimpleAudioSession.cs | 2 +- SIL.Media/WindowsAudioSession.cs | 39 ++++++++++------- 4 files changed, 77 insertions(+), 31 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ae5a147be..299ea937b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -33,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. @@ -50,6 +51,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). - [SIL.Windows.Forms.Keyboarding] BREAKING CHANGE: Upgraded to L10nSharp v9. Any clients which also use L10nSharp must also upgrade to v9. - [SIL.Windows.Forms.Keyboarding] Add a reference to L10nSharp.Windows.Forms v9. - [SIL.Windows.Forms] BREAKING CHANGE: ToolStripExtensions.InitializeWithAvailableUILocales() removed the ILocalizationManager parameter. This method no longer provides functionality to display the localization dialog box in response to the user clicking More. +- [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. ### Removed diff --git a/SIL.Media.Tests/AudioSessionTests.cs b/SIL.Media.Tests/AudioSessionTests.cs index c80b8b46c..82280a54e 100644 --- a/SIL.Media.Tests/AudioSessionTests.cs +++ b/SIL.Media.Tests/AudioSessionTests.cs @@ -123,7 +123,7 @@ public void Play_FileDoesNotExist_Throws() [NUnit.Framework.Category("RequiresAudioOutputDevice")] public void CanStop_WhilePlaying_True() { - using (var file = TempFile.FromResource(Resources.finished, ".wav")) + using (var file = GetTempAudioFile("wav")) { using (var x = AudioFactory.CreateAudioSession(file.Path)) { @@ -226,7 +226,7 @@ public void RecordThenPlay_SmokeTest() [NUnit.Framework.Category("RequiresAudioInputDevice")] public void Play_GiveThaiFileName_ShouldHearTinklingSounds() { - using (var file = TempFile.FromResource(Resources.finished, ".wav")) + using (var file = GetTempAudioFile("wav")) { using (var d = TemporaryFolder.Create(TestContext.CurrentContext)) { @@ -360,7 +360,7 @@ public void CanRecord_WhilePlaying_False() [NUnit.Framework.Category("RequiresAudioOutputDevice")] public void CanPlay_WhilePlaying_False() { - using (var file = TempFile.FromResource(Resources.finished, ".wav")) + using (var file = GetTempAudioFile("wav")) { using (var x = AudioFactory.CreateAudioSession(file.Path)) { @@ -420,10 +420,17 @@ 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 . + /// + [TestCase("wav")] + [TestCase("mp3")] + [Platform(Exclude = "Win", Reason = "Redundant. Following test covers this.")] + public void PlayAndStopPlaying_WavFile_DoesNotThrow(string type) { - using (var file = TempFile.FromResource(Resources.finished, ".wav")) + using (var file = GetTempAudioFile(type)) { using (var x = AudioFactory.CreateAudioSession(file.Path)) { @@ -433,24 +440,47 @@ public void Play_DoesPlay () } } - [Test] + /// + /// Tests that *on Windows* the gets a player that implements + /// . Then it tests calling + /// to play 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 Play_DoesPlayMp3_SmokeTest() + public void PlayAndStopPlaying_Mp3_WindowsOnlyTrackingOfPlaybackStopped(string type) { - // file disposed after playback stopped - using var file = TempFile.FromResource(Resources.ShortMp3, ".mp3"); + var playbackCompleted = new ManualResetEventSlim(false); + bool isPlayingValueInsidePlaybackStopped = true; + // The file gets disposed after playback stops. + using var file = GetTempAudioFile(type); using (var x = AudioFactory.CreateAudioSession(file.Path)) { - ((ISimpleAudioWithEvents)x).PlaybackStopped += (e, f) => + if (!(x is ISimpleAudioWithEvents session)) { - Debug.WriteLine(f); + Assert.Fail("Expected a player that could inform caller when playback stops."); + return; + } + + session.PlaybackStopped += (sender, f) => + { + playbackCompleted.Set(); + if (ReferenceEquals(sender, session)) + isPlayingValueInsidePlaybackStopped = session.IsPlaying; + else + Assert.Fail("PlaybackStopped sender was not the session instance."); }; - Assert.That(x.IsPlaying, Is.False); + Assert.That(session.IsPlaying, Is.False); Assert.DoesNotThrow(() => x.Play()); Assert.That(x.IsPlaying, Is.True); Assert.DoesNotThrow(() => x.StopPlaying()); - Assert.That(x.IsPlaying, Is.False); + Assert.That(playbackCompleted.Wait(1800), Is.True, + "PlaybackStopped event was not raised in time. Increase the timeout to accommodate slower hardware if necessary."); + } + Assert.That(isPlayingValueInsidePlaybackStopped, Is.False); } [Test] @@ -497,5 +527,12 @@ public void Record_LongRecording() } } } + + private TempFile GetTempAudioFile(string type) + { + return type == "wav" + ? TempFile.FromResource(Resources.finished, $".{type}") + : TempFile.FromResource(Resources.ShortMp3, $".{type}"); + } } } diff --git a/SIL.Media/ISimpleAudioSession.cs b/SIL.Media/ISimpleAudioSession.cs index 8b515c410..0adc0e59f 100644 --- a/SIL.Media/ISimpleAudioSession.cs +++ b/SIL.Media/ISimpleAudioSession.cs @@ -18,7 +18,7 @@ public interface ISimpleAudioSession : IDisposable void StopPlaying(); } - public interface ISimpleAudioWithEvents + public interface ISimpleAudioWithEvents : ISimpleAudioSession { event EventHandler PlaybackStopped; } diff --git a/SIL.Media/WindowsAudioSession.cs b/SIL.Media/WindowsAudioSession.cs index bede8d56b..fb15dd7bf 100644 --- a/SIL.Media/WindowsAudioSession.cs +++ b/SIL.Media/WindowsAudioSession.cs @@ -1,7 +1,7 @@ -// 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; @@ -15,9 +15,9 @@ 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(); @@ -27,6 +27,7 @@ internal class WindowsAudioSession : ISimpleAudioSession, ISimpleAudioWithEvents private readonly SoundFile _soundFile; private WaveOutEvent _outputDevice; private AudioFileReader _audioFile; + private readonly object _lock = new object(); /// /// Will be raised when playing is over /// @@ -55,7 +56,7 @@ private static ISoundEngine CreateSoundEngine() } /// - /// Constructor for an AudioSession using the IrrKlang library + /// Constructor for an AudioSession using the library /// public WindowsAudioSession(string filePath) { @@ -77,7 +78,6 @@ public void StartRecording() _recorder.StartRecordingBufferedAudio(22000, SampleFormat.Signed16Bit, 1); _startRecordingTime = DateTime.Now; - } public void StopRecordingAndSaveAsWav() @@ -107,7 +107,7 @@ public double LastRecordingMilliseconds public bool IsRecording => _recorder != null && _recorder.IsRecording; - public bool IsPlaying { get; set; } + public bool IsPlaying { get; private set; } public bool CanRecord => !IsPlaying && !IsRecording; @@ -117,7 +117,7 @@ public double LastRecordingMilliseconds private void OnPlaybackStopped(object sender, StoppedEventArgs args) { - lock (FilePath) + lock (_lock) { if (_outputDevice != null) { @@ -130,16 +130,19 @@ private void OnPlaybackStopped(object sender, StoppedEventArgs args) } } } + IsPlaying = false; - PlaybackStopped?.Invoke(sender, args); + + PlaybackStopped?.Invoke(this, 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() { @@ -156,7 +159,7 @@ public void Play() { try { - lock (FilePath) + lock (_lock) { if (_outputDevice == null) { @@ -271,8 +274,12 @@ public void StopPlaying() { if (IsPlaying) { - OnPlaybackStopped(this, new StoppedEventArgs()); + lock (_lock) + { + _outputDevice?.Stop(); + } } + try { _engine.RemoveAllSoundSources(); From 7b4e61e731e4eddb3c6f9905557cae2cd7ff4126 Mon Sep 17 00:00:00 2001 From: tombogle Date: Tue, 17 Feb 2026 06:49:45 -0500 Subject: [PATCH 2/6] Fixed Dispose/lifecycle issues --- SIL.Media.Tests/AudioSessionTests.cs | 59 ++++++++++++++++------------ SIL.Media/WindowsAudioSession.cs | 59 ++++++++++++++++++---------- 2 files changed, 73 insertions(+), 45 deletions(-) diff --git a/SIL.Media.Tests/AudioSessionTests.cs b/SIL.Media.Tests/AudioSessionTests.cs index 82280a54e..ecdfe1900 100644 --- a/SIL.Media.Tests/AudioSessionTests.cs +++ b/SIL.Media.Tests/AudioSessionTests.cs @@ -428,7 +428,7 @@ private static ISimpleAudioSession RecordSomething(TempFile f) [TestCase("wav")] [TestCase("mp3")] [Platform(Exclude = "Win", Reason = "Redundant. Following test covers this.")] - public void PlayAndStopPlaying_WavFile_DoesNotThrow(string type) + public void PlayAndStopPlaying_NonWindows_DoesNotThrow(string type) { using (var file = GetTempAudioFile(type)) { @@ -450,37 +450,46 @@ public void PlayAndStopPlaying_WavFile_DoesNotThrow(string type) [TestCase("wav")] [TestCase("mp3")] [Platform(Exclude = "Linux", Reason = "AudioAlsaSession doesn't implement ISimpleAudioWithEvents")] - public void PlayAndStopPlaying_Mp3_WindowsOnlyTrackingOfPlaybackStopped(string type) + public void PlayAndStopPlaying_Windows_TrackingOfPlaybackStopped(string type) { var playbackCompleted = new ManualResetEventSlim(false); - bool isPlayingValueInsidePlaybackStopped = true; - // The file gets disposed after playback stops. - using var file = GetTempAudioFile(type); - using (var x = AudioFactory.CreateAudioSession(file.Path)) + try { - if (!(x is ISimpleAudioWithEvents session)) + bool isPlayingValueInsidePlaybackStopped = true; + // The file gets disposed after playback stops. + using var file = GetTempAudioFile(type); + using (var x = AudioFactory.CreateAudioSession(file.Path)) { - Assert.Fail("Expected a player that could inform caller when playback stops."); - return; - } + if (!(x is ISimpleAudioWithEvents session)) + { + Assert.Fail( + "Expected a player that could inform caller when playback stops."); + return; + } - session.PlaybackStopped += (sender, f) => - { - playbackCompleted.Set(); - if (ReferenceEquals(sender, session)) - isPlayingValueInsidePlaybackStopped = session.IsPlaying; - else - Assert.Fail("PlaybackStopped sender was not the session instance."); - }; - Assert.That(session.IsPlaying, Is.False); - Assert.DoesNotThrow(() => x.Play()); - Assert.That(x.IsPlaying, Is.True); - Assert.DoesNotThrow(() => x.StopPlaying()); - Assert.That(playbackCompleted.Wait(1800), Is.True, - "PlaybackStopped event was not raised in time. Increase the timeout to accommodate slower hardware if necessary."); + session.PlaybackStopped += (sender, f) => + { + playbackCompleted.Set(); + if (ReferenceEquals(sender, session)) + isPlayingValueInsidePlaybackStopped = session.IsPlaying; + else + Assert.Fail("PlaybackStopped sender was not the session instance."); + }; + Assert.That(session.IsPlaying, Is.False); + Assert.DoesNotThrow(() => x.Play()); + Assert.That(x.IsPlaying, Is.True); + Assert.DoesNotThrow(() => x.StopPlaying()); + Assert.That(playbackCompleted.Wait(1800), Is.True, + "PlaybackStopped event was not raised in time. Increase the timeout to accommodate slower hardware if necessary."); + + } + Assert.That(isPlayingValueInsidePlaybackStopped, Is.False); + } + finally + { + playbackCompleted.Dispose(); } - Assert.That(isPlayingValueInsidePlaybackStopped, Is.False); } [Test] diff --git a/SIL.Media/WindowsAudioSession.cs b/SIL.Media/WindowsAudioSession.cs index fb15dd7bf..f6fe3946b 100644 --- a/SIL.Media/WindowsAudioSession.cs +++ b/SIL.Media/WindowsAudioSession.cs @@ -10,6 +10,7 @@ using IrrKlang; using NAudio.Wave; using SIL.Code; +using SIL.Reporting; namespace SIL.Media { @@ -118,22 +119,36 @@ public double LastRecordingMilliseconds private void OnPlaybackStopped(object sender, StoppedEventArgs args) { 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 thow. _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. + } + _outputDevice = null; + } + + if (_audioFile != null) + { + _audioFile.Dispose(); + _audioFile = null; } IsPlaying = false; - - PlaybackStopped?.Invoke(this, args); } /// @@ -184,7 +199,7 @@ public void Play() // 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.ReportNonFatalException(e); } }; worker.RunWorkerAsync(); @@ -272,13 +287,19 @@ public void SaveAsWav(string path) public void StopPlaying() { - if (IsPlaying) + lock (_lock) { - lock (_lock) - { - _outputDevice?.Stop(); - } + if (!IsPlaying || _outputDevice == null) + return; + + _outputDevice.Stop(); } + } + + public void Dispose() + { + lock (_lock) + CleanupPlaybackResources(); try { @@ -286,12 +307,10 @@ public void StopPlaying() } 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(); } From a18ddfb4257e11377ca2fae7ea4bcb48c47c19d6 Mon Sep 17 00:00:00 2001 From: tombogle Date: Wed, 25 Feb 2026 08:44:18 -0500 Subject: [PATCH 3/6] Replaced inadvertently deleted line in CHANGELOG.md --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 68cd8adf4..ddd314393 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -48,6 +48,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). - [SIL.Windows.Forms.Keyboarding] BREAKING CHANGE: Upgraded to L10nSharp v9. Any clients which also use L10nSharp must also upgrade to v9. - [SIL.Windows.Forms.Keyboarding] Add a reference to L10nSharp.Windows.Forms v9. - [SIL.Windows.Forms] BREAKING CHANGE: ToolStripExtensions.InitializeWithAvailableUILocales() removed the ILocalizationManager parameter. This method no longer provides functionality to display the localization dialog box in response to the user clicking More. +- [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.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. From 7b5a5ccfb9fcd9f25b72e95568c720c3fcbcd474 Mon Sep 17 00:00:00 2001 From: tombogle Date: Wed, 25 Feb 2026 15:21:09 -0500 Subject: [PATCH 4/6] Made AudioSessionTests that test starting and stopping playback deterministic. Fixed threading issues in WindowsAudioSession --- SIL.Media.Tests/AudioSessionTests.cs | 89 +++++++++++++++-------- SIL.Media.Tests/TestNAudioWaveOutEvent.cs | 75 +++++++++++++++++++ SIL.Media/AssemblyInfo.cs | 9 +++ SIL.Media/AudioFactory.cs | 1 - SIL.Media/INAudioOutputDevice.cs | 14 ++++ SIL.Media/ISimpleAudioSession.cs | 2 + SIL.Media/NaudioWaveOutEventAdapter.cs | 46 ++++++++++++ SIL.Media/WindowsAudioSession.cs | 69 ++++++++++++------ 8 files changed, 250 insertions(+), 55 deletions(-) create mode 100644 SIL.Media.Tests/TestNAudioWaveOutEvent.cs create mode 100644 SIL.Media/AssemblyInfo.cs create mode 100644 SIL.Media/INAudioOutputDevice.cs create mode 100644 SIL.Media/NaudioWaveOutEventAdapter.cs diff --git a/SIL.Media.Tests/AudioSessionTests.cs b/SIL.Media.Tests/AudioSessionTests.cs index ecdfe1900..26154875d 100644 --- a/SIL.Media.Tests/AudioSessionTests.cs +++ b/SIL.Media.Tests/AudioSessionTests.cs @@ -349,7 +349,7 @@ public void CanRecord_WhilePlaying_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 @@ -424,26 +424,47 @@ private static ISimpleAudioSession RecordSomething(TempFile f) /// 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")] - [Platform(Exclude = "Win", Reason = "Redundant. Following test covers this.")] public void PlayAndStopPlaying_NonWindows_DoesNotThrow(string type) { - using (var file = GetTempAudioFile(type)) + 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)) - { - Assert.DoesNotThrow(() => x.Play()); - Assert.DoesNotThrow(() => x.StopPlaying()); - } +#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 // This test 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 play an audio file and calling + /// to begin playing an audio file and calling /// , ensuring that we are notified when /// playback stops. /// @@ -458,31 +479,36 @@ public void PlayAndStopPlaying_Windows_TrackingOfPlaybackStopped(string type) bool isPlayingValueInsidePlaybackStopped = true; // The file gets disposed after playback stops. using var file = GetTempAudioFile(type); - using (var x = AudioFactory.CreateAudioSession(file.Path)) + using var x = AudioFactory.CreateAudioSession(file.Path); + if (!(x is ISimpleAudioWithEvents session)) { - if (!(x is ISimpleAudioWithEvents session)) - { - Assert.Fail( - "Expected a player that could inform caller when playback stops."); - return; - } + Assert.Fail( + "Expected a player that could inform caller when playback stops."); + return; + } - session.PlaybackStopped += (sender, f) => - { - playbackCompleted.Set(); - if (ReferenceEquals(sender, session)) - isPlayingValueInsidePlaybackStopped = session.IsPlaying; - else - Assert.Fail("PlaybackStopped sender was not the session instance."); - }; - Assert.That(session.IsPlaying, Is.False); - Assert.DoesNotThrow(() => x.Play()); - Assert.That(x.IsPlaying, Is.True); - Assert.DoesNotThrow(() => x.StopPlaying()); - Assert.That(playbackCompleted.Wait(1800), Is.True, - "PlaybackStopped event was not raised in time. Increase the timeout to accommodate slower hardware if necessary."); + ((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); } @@ -491,6 +517,7 @@ public void PlayAndStopPlaying_Windows_TrackingOfPlaybackStopped(string type) playbackCompleted.Dispose(); } } +#endif [Test] [NUnit.Framework.Category("RequiresAudioInputDevice")] 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..af9b74e6c 100644 --- a/SIL.Media/AudioFactory.cs +++ b/SIL.Media/AudioFactory.cs @@ -9,7 +9,6 @@ namespace SIL.Media { public class AudioFactory { - /// /// Creates an audio session for the given file path. /// On Linux, uses ALSA. On Windows, uses IrrKlang (only supported on .NET Framework 4.6.2/4.8). 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 0adc0e59f..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,6 +15,7 @@ public interface ISimpleAudioSession : IDisposable bool CanStop { get; } bool CanPlay { get; } void Play(); + [PublicAPI] void SaveAsWav(string filePath); void StopPlaying(); } 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 f6fe3946b..dd7d19fcf 100644 --- a/SIL.Media/WindowsAudioSession.cs +++ b/SIL.Media/WindowsAudioSession.cs @@ -5,8 +5,8 @@ #if NET462 || NET48 using System; -using System.ComponentModel; using System.IO; +using System.Threading.Tasks; using IrrKlang; using NAudio.Wave; using SIL.Code; @@ -26,8 +26,10 @@ internal class WindowsAudioSession : 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 @@ -55,6 +57,10 @@ private static ISoundEngine CreateSoundEngine() return new ISoundEngine(SoundOutputDriver.NullDriver); } } + private INAudioOutputDevice CreateNAudioOutputDevice() + { + return TestNAudioOutputDevice ?? new NAudioWaveOutEventAdapter(); + } /// /// Constructor for an AudioSession using the library @@ -108,7 +114,14 @@ public double LastRecordingMilliseconds public bool IsRecording => _recorder != null && _recorder.IsRecording; - public bool IsPlaying { get; private set; } + public bool IsPlaying + { + get + { + lock(_lock) + return _outputDevice != null; + } + } public bool CanRecord => !IsPlaying && !IsRecording; @@ -131,7 +144,7 @@ private void CleanupPlaybackResources() _outputDevice.PlaybackStopped -= OnPlaybackStopped; try { - // Dispose calls Stop, which can thow. + // Dispose calls Stop, which can throw. _outputDevice.Dispose(); } catch (Exception e) @@ -139,16 +152,15 @@ private void CleanupPlaybackResources() Console.WriteLine(e); // We're disposing. We probably don't care what went wrong. } + + if (_audioFile != null) + { + _audioFile.Dispose(); + _audioFile = null; + } + _outputDevice = null; } - - if (_audioFile != null) - { - _audioFile.Dispose(); - _audioFile = null; - } - - IsPlaying = false; } /// @@ -168,9 +180,14 @@ 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 { @@ -178,14 +195,17 @@ public void Play() { 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(); } } @@ -193,16 +213,13 @@ public void Play() { // 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. ErrorReport.ReportNonFatalException(e); } - }; - worker.RunWorkerAsync(); + }); } private class SoundFile : IFileFactory @@ -289,10 +306,16 @@ public void StopPlaying() { lock (_lock) { - if (!IsPlaying || _outputDevice == null) + if (_outputDevice == null) // !IsPlaying return; - _outputDevice.Stop(); + Console.WriteLine( + $"Playback state on thread {System.Threading.Thread.CurrentContext.ContextID}: {_outputDevice.PlaybackState}"); + + if (_outputDevice.PlaybackState == PlaybackState.Stopped) + CleanupPlaybackResources(); + else + _outputDevice.Stop(); } } From 64b5ae1ac7bd6b522e5c6bb23ebeaf8eb9b6345f Mon Sep 17 00:00:00 2001 From: tombogle Date: Wed, 25 Feb 2026 23:51:32 -0500 Subject: [PATCH 5/6] Added a unit test to AudioSessionTests to cover the case of a playback error on a non-UI thread (Windows-only). Also some minor code cleanup --- CHANGELOG.md | 4 + Palaso.sln.DotSettings | 2 + SIL.Core/Reporting/ErrorReport.cs | 129 +++--- SIL.Media.Tests/AudioSessionTests.cs | 411 ++++++++---------- SIL.Media/AudioFactory.cs | 9 + SIL.Media/WindowsAudioSession.cs | 5 +- .../Reporting/WinFormsErrorReporter.cs | 4 +- 7 files changed, 263 insertions(+), 301 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ddd314393..47c484e99 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/). - [SIL.Core.Clearshare] New methods "GetIsStringAvailableForLangId" and "GetDynamicStringOrEnglish" were added to Localizer for use in LicenseInfo's "GetBestLicenseTranslation" method, to remove LicenseInfo's L10NSharp dependency. - [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] 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 @@ -51,6 +53,8 @@ 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.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 26154875d..b599a8583 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 = 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(); - } - } + 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(); + 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 = 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(); - } - } - } - } + 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,14 +290,12 @@ 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); } /// @@ -360,55 +310,39 @@ public void CanRecord_WhilePlaying_False() [NUnit.Framework.Category("RequiresAudioOutputDevice")] public void CanPlay_WhilePlaying_False() { - 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(); - } - } + 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) @@ -458,7 +392,7 @@ public void PlayAndStopPlaying_NonWindows_DoesNotThrow(string type) } } -#if NET462 || NET48 // This test won't compile in .NET 8 because WindowsAudioSession is not +#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. /// @@ -517,51 +451,76 @@ public void PlayAndStopPlaying_Windows_TrackingOfPlaybackStopped(string type) playbackCompleted.Dispose(); } } -#endif + /// + /// 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] - [NUnit.Framework.Category("RequiresAudioInputDevice")] - public void Record_DoesRecord () + [Platform(Exclude = "Linux", Reason = "Not sure where Linux implementation would fail")] + [Timeout(4000)] + public void Play_InvalidAudioFileThrowsBackgroundException_NonFatalErrorReported() { - using (var folder = TemporaryFolder.Create(TestContext.CurrentContext)) + try { - string fPath = folder.Combine("dump.ogg"); - using (var x = AudioFactory.CreateAudioSession(fPath)) + 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) => { - Assert.DoesNotThrow(() => x.StartRecording()); - Assert.IsTrue(x.IsRecording); - Thread.Sleep(1000); - Assert.DoesNotThrow(() => x.StopRecordingAndSaveAsWav()); - } + reportedExceptionInPlaybackStopped = ((StoppedEventArgs)args).Exception; + }; + 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()); + } [Test] [NUnit.Framework.Category("ByHand")] [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) diff --git a/SIL.Media/AudioFactory.cs b/SIL.Media/AudioFactory.cs index af9b74e6c..84a002cc3 100644 --- a/SIL.Media/AudioFactory.cs +++ b/SIL.Media/AudioFactory.cs @@ -9,6 +9,8 @@ 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. /// On Linux, uses ALSA. On Windows, uses IrrKlang (only supported on .NET Framework 4.6.2/4.8). @@ -76,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/WindowsAudioSession.cs b/SIL.Media/WindowsAudioSession.cs index dd7d19fcf..f337ce6e2 100644 --- a/SIL.Media/WindowsAudioSession.cs +++ b/SIL.Media/WindowsAudioSession.cs @@ -211,13 +211,12 @@ public void 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. 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. - ErrorReport.ReportNonFatalException(e); + ErrorReport.ReportNonFatalExceptionWithMessage(e, + AudioFactory.PlaybackErrorMessage ?? AudioFactory.kDefaultPlaybackErrorMessage); } }); } 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) From 68bfce019a529478916665f77a81f62bd9a5507c Mon Sep 17 00:00:00 2001 From: tombogle Date: Thu, 26 Feb 2026 10:44:46 -0500 Subject: [PATCH 6/6] Fixed issues from code review --- SIL.Media.Tests/AudioSessionTests.cs | 2 +- SIL.Media/WindowsAudioSession.cs | 3 --- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/SIL.Media.Tests/AudioSessionTests.cs b/SIL.Media.Tests/AudioSessionTests.cs index b599a8583..d3051fbce 100644 --- a/SIL.Media.Tests/AudioSessionTests.cs +++ b/SIL.Media.Tests/AudioSessionTests.cs @@ -155,7 +155,7 @@ public void IsRecording_WhileRecording_True() public void RecordThenPlay_SmokeTest() { using var f = new TempFile(); - var w = new BackgroundWorker(); + using var w = new BackgroundWorker(); // ReSharper disable once RedundantDelegateCreation w.DoWork += new DoWorkEventHandler((o, args) => SystemSounds.Exclamation.Play()); diff --git a/SIL.Media/WindowsAudioSession.cs b/SIL.Media/WindowsAudioSession.cs index f337ce6e2..bbbd6d62b 100644 --- a/SIL.Media/WindowsAudioSession.cs +++ b/SIL.Media/WindowsAudioSession.cs @@ -308,9 +308,6 @@ public void StopPlaying() if (_outputDevice == null) // !IsPlaying return; - Console.WriteLine( - $"Playback state on thread {System.Threading.Thread.CurrentContext.ContextID}: {_outputDevice.PlaybackState}"); - if (_outputDevice.PlaybackState == PlaybackState.Stopped) CleanupPlaybackResources(); else