From be11388a04c1abd8b5d7474f2fa664f41f133053 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Thu, 14 Oct 2021 12:37:21 -0500 Subject: [PATCH 01/13] it honks --- src/cascadia/TerminalApp/Pane.cpp | 7 +++++++ src/cascadia/TerminalApp/Pane.h | 2 ++ src/cascadia/TerminalApp/pch.h | 5 ++++- 3 files changed, 13 insertions(+), 1 deletion(-) diff --git a/src/cascadia/TerminalApp/Pane.cpp b/src/cascadia/TerminalApp/Pane.cpp index e04f558f606..d65166df324 100644 --- a/src/cascadia/TerminalApp/Pane.cpp +++ b/src/cascadia/TerminalApp/Pane.cpp @@ -1133,6 +1133,13 @@ void Pane::_ControlWarningBellHandler(const winrt::Windows::Foundation::IInspect // Audible is set, play the sound const auto soundAlias = reinterpret_cast(SND_ALIAS_SYSTEMHAND); PlaySound(soundAlias, NULL, SND_ALIAS_ID | SND_ASYNC | SND_SENTRY); + + auto uri{ winrt::Windows::Foundation::Uri(L"C:\\Users\\migrie\\Downloads\\memes\\honks\\Honk1.mp3") }; + auto source{ winrt::Windows::Media::Core::MediaSource::CreateFromUri(uri) }; + + auto item{ winrt::Windows::Media::Playback::MediaPlaybackItem(source) }; + p.Source(item); + p.Play(); } if (WI_IsFlagSet(_profile.BellStyle(), winrt::Microsoft::Terminal::Settings::Model::BellStyle::Window)) diff --git a/src/cascadia/TerminalApp/Pane.h b/src/cascadia/TerminalApp/Pane.h index 9892a6e8ae2..1666d6361a6 100644 --- a/src/cascadia/TerminalApp/Pane.h +++ b/src/cascadia/TerminalApp/Pane.h @@ -230,6 +230,8 @@ class Pane : public std::enable_shared_from_this bool _zoomed{ false }; + winrt::Windows::Media::Playback::MediaPlayer p{}; + bool _IsLeaf() const noexcept; bool _HasFocusedChild() const noexcept; void _SetupChildCloseHandlers(); diff --git a/src/cascadia/TerminalApp/pch.h b/src/cascadia/TerminalApp/pch.h index 66026a78dfd..534c5bfabd2 100644 --- a/src/cascadia/TerminalApp/pch.h +++ b/src/cascadia/TerminalApp/pch.h @@ -1,4 +1,4 @@ -// Copyright (c) Microsoft Corporation. +// Copyright (c) Microsoft Corporation. // Licensed under the MIT license. // // pch.h @@ -45,6 +45,9 @@ #include #include #include +#include +#include +#include #include #include From 8fbd32d3c357025574a3eb41bb12a65d43d096c0 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Thu, 14 Oct 2021 12:45:12 -0500 Subject: [PATCH 02/13] many honks --- src/cascadia/TerminalApp/Pane.cpp | 26 ++++++++++++++++++-------- 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/src/cascadia/TerminalApp/Pane.cpp b/src/cascadia/TerminalApp/Pane.cpp index d65166df324..c8e75b3e7b0 100644 --- a/src/cascadia/TerminalApp/Pane.cpp +++ b/src/cascadia/TerminalApp/Pane.cpp @@ -1131,15 +1131,25 @@ void Pane::_ControlWarningBellHandler(const winrt::Windows::Foundation::IInspect if (WI_IsFlagSet(_profile.BellStyle(), winrt::Microsoft::Terminal::Settings::Model::BellStyle::Audible)) { // Audible is set, play the sound - const auto soundAlias = reinterpret_cast(SND_ALIAS_SYSTEMHAND); - PlaySound(soundAlias, NULL, SND_ALIAS_ID | SND_ASYNC | SND_SENTRY); - - auto uri{ winrt::Windows::Foundation::Uri(L"C:\\Users\\migrie\\Downloads\\memes\\honks\\Honk1.mp3") }; - auto source{ winrt::Windows::Media::Core::MediaSource::CreateFromUri(uri) }; + // const auto soundAlias = reinterpret_cast(SND_ALIAS_SYSTEMHAND); + // PlaySound(soundAlias, NULL, SND_ALIAS_ID | SND_ASYNC | SND_SENTRY); + + winrt::Windows::Foundation::Uri honks[]{ + winrt::Windows::Foundation::Uri(L"C:\\Users\\migrie\\Downloads\\memes\\honks\\Honk1.mp3"), + winrt::Windows::Foundation::Uri(L"C:\\Users\\migrie\\Downloads\\memes\\honks\\Honk2.mp3"), + winrt::Windows::Foundation::Uri(L"C:\\Users\\migrie\\Downloads\\memes\\honks\\Honk3.mp3"), + winrt::Windows::Foundation::Uri(L"C:\\Users\\migrie\\Downloads\\memes\\honks\\Honk4.mp3"), + winrt::Windows::Foundation::Uri(L"C:\\Users\\migrie\\Downloads\\memes\\honks\\Honk-muffled1.mp3"), + winrt::Windows::Foundation::Uri(L"C:\\Users\\migrie\\Downloads\\memes\\honks\\Honk-muffled2.mp3"), + winrt::Windows::Foundation::Uri(L"C:\\Users\\migrie\\Downloads\\memes\\honks\\Honk-muffled3.mp3"), + }; + + auto uri{ honks[rand() % ARRAYSIZE(honks)] }; + auto source{ winrt::Windows::Media::Core::MediaSource::CreateFromUri(uri) }; - auto item{ winrt::Windows::Media::Playback::MediaPlaybackItem(source) }; - p.Source(item); - p.Play(); + auto item{ winrt::Windows::Media::Playback::MediaPlaybackItem(source) }; + p.Source(item); + p.Play(); } if (WI_IsFlagSet(_profile.BellStyle(), winrt::Microsoft::Terminal::Settings::Model::BellStyle::Window)) From 4c4fde9da36d1faf9391464db7ea90bd95114a5d Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Thu, 14 Oct 2021 12:57:58 -0500 Subject: [PATCH 03/13] oh yea, it was that easy --- src/cascadia/TerminalApp/Pane.cpp | 34 ++++++++----------- .../TerminalSettingsModel/Profile.cpp | 4 +++ src/cascadia/TerminalSettingsModel/Profile.h | 2 ++ .../TerminalSettingsModel/Profile.idl | 2 ++ 4 files changed, 23 insertions(+), 19 deletions(-) diff --git a/src/cascadia/TerminalApp/Pane.cpp b/src/cascadia/TerminalApp/Pane.cpp index c8e75b3e7b0..87d70bfc053 100644 --- a/src/cascadia/TerminalApp/Pane.cpp +++ b/src/cascadia/TerminalApp/Pane.cpp @@ -1131,25 +1131,21 @@ void Pane::_ControlWarningBellHandler(const winrt::Windows::Foundation::IInspect if (WI_IsFlagSet(_profile.BellStyle(), winrt::Microsoft::Terminal::Settings::Model::BellStyle::Audible)) { // Audible is set, play the sound - // const auto soundAlias = reinterpret_cast(SND_ALIAS_SYSTEMHAND); - // PlaySound(soundAlias, NULL, SND_ALIAS_ID | SND_ASYNC | SND_SENTRY); - - winrt::Windows::Foundation::Uri honks[]{ - winrt::Windows::Foundation::Uri(L"C:\\Users\\migrie\\Downloads\\memes\\honks\\Honk1.mp3"), - winrt::Windows::Foundation::Uri(L"C:\\Users\\migrie\\Downloads\\memes\\honks\\Honk2.mp3"), - winrt::Windows::Foundation::Uri(L"C:\\Users\\migrie\\Downloads\\memes\\honks\\Honk3.mp3"), - winrt::Windows::Foundation::Uri(L"C:\\Users\\migrie\\Downloads\\memes\\honks\\Honk4.mp3"), - winrt::Windows::Foundation::Uri(L"C:\\Users\\migrie\\Downloads\\memes\\honks\\Honk-muffled1.mp3"), - winrt::Windows::Foundation::Uri(L"C:\\Users\\migrie\\Downloads\\memes\\honks\\Honk-muffled2.mp3"), - winrt::Windows::Foundation::Uri(L"C:\\Users\\migrie\\Downloads\\memes\\honks\\Honk-muffled3.mp3"), - }; - - auto uri{ honks[rand() % ARRAYSIZE(honks)] }; - auto source{ winrt::Windows::Media::Core::MediaSource::CreateFromUri(uri) }; - - auto item{ winrt::Windows::Media::Playback::MediaPlaybackItem(source) }; - p.Source(item); - p.Play(); + auto sounds{ _profile.BellSound() }; + if (sounds && sounds.Size() > 0) + { + winrt::Windows::Foundation::Uri uri{ sounds.GetAt(rand() % sounds.Size()) }; + auto source{ winrt::Windows::Media::Core::MediaSource::CreateFromUri(uri) }; + auto item{ winrt::Windows::Media::Playback::MediaPlaybackItem(source) }; + p.Source(item); + p.Play(); + } + else + { + const auto soundAlias = reinterpret_cast(SND_ALIAS_SYSTEMHAND); + PlaySound(soundAlias, NULL, SND_ALIAS_ID | SND_ASYNC | SND_SENTRY); + + } } if (WI_IsFlagSet(_profile.BellStyle(), winrt::Microsoft::Terminal::Settings::Model::BellStyle::Window)) diff --git a/src/cascadia/TerminalSettingsModel/Profile.cpp b/src/cascadia/TerminalSettingsModel/Profile.cpp index 287ce8970a9..b3a89b720ec 100644 --- a/src/cascadia/TerminalSettingsModel/Profile.cpp +++ b/src/cascadia/TerminalSettingsModel/Profile.cpp @@ -45,6 +45,7 @@ static constexpr std::string_view AntialiasingModeKey{ "antialiasingMode" }; static constexpr std::string_view TabColorKey{ "tabColor" }; static constexpr std::string_view BellStyleKey{ "bellStyle" }; static constexpr std::string_view UnfocusedAppearanceKey{ "unfocusedAppearance" }; +static constexpr std::string_view BellSoundKey{ "bellSound" }; Profile::Profile(guid guid) noexcept : _Guid(guid) @@ -134,6 +135,7 @@ winrt::com_ptr Profile::CopySettings() const profile->_SnapOnInput = _SnapOnInput; profile->_AltGrAliasing = _AltGrAliasing; profile->_BellStyle = _BellStyle; + profile->_BellSound = _BellSound; profile->_ConnectionType = _ConnectionType; profile->_Origin = _Origin; profile->_FontInfo = *fontInfo; @@ -221,6 +223,7 @@ void Profile::LayerJson(const Json::Value& json) JsonUtils::GetValueForKey(json, AntialiasingModeKey, _AntialiasingMode); JsonUtils::GetValueForKey(json, TabColorKey, _TabColor); JsonUtils::GetValueForKey(json, BellStyleKey, _BellStyle); + JsonUtils::GetValueForKey(json, BellSoundKey, _BellSound); if (json.isMember(JsonKey(UnfocusedAppearanceKey))) { @@ -374,6 +377,7 @@ Json::Value Profile::ToJson() const JsonUtils::SetValueForKey(json, AntialiasingModeKey, _AntialiasingMode); JsonUtils::SetValueForKey(json, TabColorKey, _TabColor); JsonUtils::SetValueForKey(json, BellStyleKey, _BellStyle); + JsonUtils::SetValueForKey(json, BellSoundKey, _BellSound); // Font settings const auto fontInfoImpl = winrt::get_self(_FontInfo); diff --git a/src/cascadia/TerminalSettingsModel/Profile.h b/src/cascadia/TerminalSettingsModel/Profile.h index f69471332a3..a690ee96132 100644 --- a/src/cascadia/TerminalSettingsModel/Profile.h +++ b/src/cascadia/TerminalSettingsModel/Profile.h @@ -140,6 +140,8 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation INHERITABLE_SETTING(Model::Profile, Model::IAppearanceConfig, UnfocusedAppearance, nullptr); + INHERITABLE_SETTING(Model::Profile, Windows::Foundation::Collections::IVector, BellSound); + private: Model::IAppearanceConfig _DefaultAppearance{ winrt::make(weak_ref(*this)) }; Model::FontConfig _FontInfo{ winrt::make(weak_ref(*this)) }; diff --git a/src/cascadia/TerminalSettingsModel/Profile.idl b/src/cascadia/TerminalSettingsModel/Profile.idl index 670303ff1e7..27670a0ef9b 100644 --- a/src/cascadia/TerminalSettingsModel/Profile.idl +++ b/src/cascadia/TerminalSettingsModel/Profile.idl @@ -81,5 +81,7 @@ namespace Microsoft.Terminal.Settings.Model INHERITABLE_PROFILE_SETTING(Boolean, SnapOnInput); INHERITABLE_PROFILE_SETTING(Boolean, AltGrAliasing); INHERITABLE_PROFILE_SETTING(BellStyle, BellStyle); + + INHERITABLE_PROFILE_SETTING(Windows.Foundation.Collections.IVector, BellSound); } } From cc9487e2b8dfec7693b11fe5623a57dd0c03ab02 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Thu, 14 Oct 2021 15:12:53 -0500 Subject: [PATCH 04/13] allow a single item as a list with length 1 --- src/cascadia/TerminalSettingsModel/JsonUtils.h | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/src/cascadia/TerminalSettingsModel/JsonUtils.h b/src/cascadia/TerminalSettingsModel/JsonUtils.h index cea006b2b0d..157dc9ee098 100644 --- a/src/cascadia/TerminalSettingsModel/JsonUtils.h +++ b/src/cascadia/TerminalSettingsModel/JsonUtils.h @@ -307,9 +307,16 @@ namespace Microsoft::Terminal::Settings::Model::JsonUtils val.reserve(json.size()); ConversionTrait trait; - for (const auto& element : json) + if (json.isArray()) + { + for (const auto& element : json) + { + val.push_back(trait.FromJson(element)); + } + } + else { - val.push_back(trait.FromJson(element)); + val.push_back(trait.FromJson(json)); } return val; @@ -318,7 +325,9 @@ namespace Microsoft::Terminal::Settings::Model::JsonUtils bool CanConvert(const Json::Value& json) const { ConversionTrait trait; - return json.isArray() && std::all_of(json.begin(), json.end(), [trait](const auto& json) mutable -> bool { return trait.CanConvert(json); }); + // If there's only one element provided, then see if we can convert + // that single element into a length-1 array + return (json.isArray() && std::all_of(json.begin(), json.end(), [trait](const auto& json) mutable -> bool { return trait.CanConvert(json); })) || trait.CanConvert(json); } Json::Value ToJson(const std::vector& val) From 2c895b2c18c5e0833747f121b4bd2bee4e882940 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Mon, 1 Nov 2021 12:46:00 -0500 Subject: [PATCH 05/13] it's a singleton now --- src/cascadia/TerminalApp/Pane.cpp | 33 ++++++++++++++++++++++++++----- src/cascadia/TerminalApp/Pane.h | 4 +++- 2 files changed, 31 insertions(+), 6 deletions(-) diff --git a/src/cascadia/TerminalApp/Pane.cpp b/src/cascadia/TerminalApp/Pane.cpp index 87d70bfc053..a6887d76690 100644 --- a/src/cascadia/TerminalApp/Pane.cpp +++ b/src/cascadia/TerminalApp/Pane.cpp @@ -33,6 +33,7 @@ static const Duration AnimationDuration = DurationHelper::FromTimeSpan(winrt::Wi winrt::Windows::UI::Xaml::Media::SolidColorBrush Pane::s_focusedBorderBrush = { nullptr }; winrt::Windows::UI::Xaml::Media::SolidColorBrush Pane::s_unfocusedBorderBrush = { nullptr }; +winrt::Windows::Media::Playback::MediaPlayer Pane::s_bellPlayer = { nullptr }; Pane::Pane(const Profile& profile, const TermControl& control, const bool lastFocused) : _control{ control }, @@ -69,6 +70,15 @@ Pane::Pane(const Profile& profile, const TermControl& control, const bool lastFo _FocusFirstChild(); e.Handled(true); }); + + if (!s_bellPlayer) + { + try + { + s_bellPlayer = winrt::Windows::Media::Playback::MediaPlayer(); + } + CATCH_LOG(); + } } Pane::Pane(std::shared_ptr first, @@ -1108,6 +1118,23 @@ void Pane::_ControlConnectionStateChangedHandler(const winrt::Windows::Foundatio } } +winrt::fire_and_forget Pane::_playBellSound(winrt::Windows::Foundation::Uri uri) +{ + auto weakThis{ shared_from_this() }; + + co_await winrt::resume_foreground(_root.Dispatcher()); + if (auto pane{ weakThis.get() }) + { + if (s_bellPlayer) + { + auto source{ winrt::Windows::Media::Core::MediaSource::CreateFromUri(uri) }; + auto item{ winrt::Windows::Media::Playback::MediaPlaybackItem(source) }; + s_bellPlayer.Source(item); + s_bellPlayer.Play(); + } + } +} + // Method Description: // - Plays a warning note when triggered by the BEL control character, // using the sound configured for the "Critical Stop" system event.` @@ -1135,16 +1162,12 @@ void Pane::_ControlWarningBellHandler(const winrt::Windows::Foundation::IInspect if (sounds && sounds.Size() > 0) { winrt::Windows::Foundation::Uri uri{ sounds.GetAt(rand() % sounds.Size()) }; - auto source{ winrt::Windows::Media::Core::MediaSource::CreateFromUri(uri) }; - auto item{ winrt::Windows::Media::Playback::MediaPlaybackItem(source) }; - p.Source(item); - p.Play(); + _playBellSound(uri); } else { const auto soundAlias = reinterpret_cast(SND_ALIAS_SYSTEMHAND); PlaySound(soundAlias, NULL, SND_ALIAS_ID | SND_ASYNC | SND_SENTRY); - } } diff --git a/src/cascadia/TerminalApp/Pane.h b/src/cascadia/TerminalApp/Pane.h index 1666d6361a6..44cc0b8b522 100644 --- a/src/cascadia/TerminalApp/Pane.h +++ b/src/cascadia/TerminalApp/Pane.h @@ -230,7 +230,7 @@ class Pane : public std::enable_shared_from_this bool _zoomed{ false }; - winrt::Windows::Media::Playback::MediaPlayer p{}; + static winrt::Windows::Media::Playback::MediaPlayer s_bellPlayer; bool _IsLeaf() const noexcept; bool _HasFocusedChild() const noexcept; @@ -285,6 +285,8 @@ class Pane : public std::enable_shared_from_this std::optional _preCalculateAutoSplit(const std::shared_ptr target, const winrt::Windows::Foundation::Size parentSize) const; + winrt::fire_and_forget _playBellSound(winrt::Windows::Foundation::Uri uri); + // Function Description: // - Returns true if the given direction can be used with the given split // type. From 9f4ed0ee18c23930e8503a4fe7ea09c4ec1859b7 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Mon, 1 Nov 2021 14:52:27 -0500 Subject: [PATCH 06/13] allow env vars in paths too --- src/cascadia/TerminalApp/Pane.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/cascadia/TerminalApp/Pane.cpp b/src/cascadia/TerminalApp/Pane.cpp index a6887d76690..431aaba8f9e 100644 --- a/src/cascadia/TerminalApp/Pane.cpp +++ b/src/cascadia/TerminalApp/Pane.cpp @@ -1161,7 +1161,8 @@ void Pane::_ControlWarningBellHandler(const winrt::Windows::Foundation::IInspect auto sounds{ _profile.BellSound() }; if (sounds && sounds.Size() > 0) { - winrt::Windows::Foundation::Uri uri{ sounds.GetAt(rand() % sounds.Size()) }; + winrt::hstring soundPath{ wil::ExpandEnvironmentStringsW(sounds.GetAt(rand() % sounds.Size()).c_str()) }; + winrt::Windows::Foundation::Uri uri { soundPath }; _playBellSound(uri); } else From 684956d848fabc9e971813606d905c66ae14894d Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Mon, 1 Nov 2021 14:57:53 -0500 Subject: [PATCH 07/13] thanks for nothing VS --- src/cascadia/TerminalApp/Pane.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cascadia/TerminalApp/Pane.cpp b/src/cascadia/TerminalApp/Pane.cpp index 431aaba8f9e..60c75c8a555 100644 --- a/src/cascadia/TerminalApp/Pane.cpp +++ b/src/cascadia/TerminalApp/Pane.cpp @@ -1162,7 +1162,7 @@ void Pane::_ControlWarningBellHandler(const winrt::Windows::Foundation::IInspect if (sounds && sounds.Size() > 0) { winrt::hstring soundPath{ wil::ExpandEnvironmentStringsW(sounds.GetAt(rand() % sounds.Size()).c_str()) }; - winrt::Windows::Foundation::Uri uri { soundPath }; + winrt::Windows::Foundation::Uri uri{ soundPath }; _playBellSound(uri); } else From fb1ed58d37ada60f4f3b4071d3117c9317ca47a4 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Tue, 16 Nov 2021 09:38:40 -0600 Subject: [PATCH 08/13] Experiment with removing the static MediaPlayer Somehow, this also causes an exception on teardown north of `Windows.Media.MediaControl.dll!AudioStateMonitorImpl::remove_SoundLevelChanged()`, which is in OS code somewhere. Not sure there's anything we can actually do about this. There are piles of bugs on that function in the OS repo. I think we're going to have to just end up avoiding this entirely. --- src/cascadia/TerminalApp/Pane.cpp | 32 +++++++++++------------ src/cascadia/TerminalApp/Pane.h | 2 +- src/cascadia/TerminalApp/TerminalPage.cpp | 6 +++++ src/cascadia/TerminalApp/TerminalPage.h | 2 ++ 4 files changed, 25 insertions(+), 17 deletions(-) diff --git a/src/cascadia/TerminalApp/Pane.cpp b/src/cascadia/TerminalApp/Pane.cpp index 484f70c1721..427e1184f0e 100644 --- a/src/cascadia/TerminalApp/Pane.cpp +++ b/src/cascadia/TerminalApp/Pane.cpp @@ -33,7 +33,7 @@ static const Duration AnimationDuration = DurationHelper::FromTimeSpan(winrt::Wi winrt::Windows::UI::Xaml::Media::SolidColorBrush Pane::s_focusedBorderBrush = { nullptr }; winrt::Windows::UI::Xaml::Media::SolidColorBrush Pane::s_unfocusedBorderBrush = { nullptr }; -winrt::Windows::Media::Playback::MediaPlayer Pane::s_bellPlayer = { nullptr }; +// winrt::Windows::Media::Playback::MediaPlayer Pane::s_bellPlayer = { nullptr }; Pane::Pane(const Profile& profile, const TermControl& control, const bool lastFocused) : _control{ control }, @@ -71,14 +71,14 @@ Pane::Pane(const Profile& profile, const TermControl& control, const bool lastFo e.Handled(true); }); - if (!s_bellPlayer) - { - try - { - s_bellPlayer = winrt::Windows::Media::Playback::MediaPlayer(); - } - CATCH_LOG(); - } + // if (!s_bellPlayer) + // { + // try + // { + // s_bellPlayer = winrt::Windows::Media::Playback::MediaPlayer(); + // } + // CATCH_LOG(); + // } } Pane::Pane(std::shared_ptr first, @@ -1125,13 +1125,13 @@ winrt::fire_and_forget Pane::_playBellSound(winrt::Windows::Foundation::Uri uri) co_await winrt::resume_foreground(_root.Dispatcher()); if (auto pane{ weakThis.get() }) { - if (s_bellPlayer) - { - auto source{ winrt::Windows::Media::Core::MediaSource::CreateFromUri(uri) }; - auto item{ winrt::Windows::Media::Playback::MediaPlaybackItem(source) }; - s_bellPlayer.Source(item); - s_bellPlayer.Play(); - } + // if (s_bellPlayer) + // { + // auto source{ winrt::Windows::Media::Core::MediaSource::CreateFromUri(uri) }; + // auto item{ winrt::Windows::Media::Playback::MediaPlaybackItem(source) }; + // s_bellPlayer.Source(item); + // s_bellPlayer.Play(); + // } } } diff --git a/src/cascadia/TerminalApp/Pane.h b/src/cascadia/TerminalApp/Pane.h index 3e935d31dd7..f3537cb72d4 100644 --- a/src/cascadia/TerminalApp/Pane.h +++ b/src/cascadia/TerminalApp/Pane.h @@ -229,7 +229,7 @@ class Pane : public std::enable_shared_from_this bool _zoomed{ false }; - static winrt::Windows::Media::Playback::MediaPlayer s_bellPlayer; + // static winrt::Windows::Media::Playback::MediaPlayer s_bellPlayer; bool _IsLeaf() const noexcept; bool _HasFocusedChild() const noexcept; diff --git a/src/cascadia/TerminalApp/TerminalPage.cpp b/src/cascadia/TerminalApp/TerminalPage.cpp index 81e2bc51709..e1d5b8d35f7 100644 --- a/src/cascadia/TerminalApp/TerminalPage.cpp +++ b/src/cascadia/TerminalApp/TerminalPage.cpp @@ -56,6 +56,12 @@ namespace winrt::TerminalApp::implementation _hostingHwnd{} { InitializeComponent(); + + try + { + _bellPlayer = winrt::Windows::Media::Playback::MediaPlayer(); + } + CATCH_LOG(); } // Method Description: diff --git a/src/cascadia/TerminalApp/TerminalPage.h b/src/cascadia/TerminalApp/TerminalPage.h index cf9477d2e3c..8b818dd6672 100644 --- a/src/cascadia/TerminalApp/TerminalPage.h +++ b/src/cascadia/TerminalApp/TerminalPage.h @@ -201,6 +201,8 @@ namespace winrt::TerminalApp::implementation std::shared_ptr _windowIdToast{ nullptr }; std::shared_ptr _windowRenameFailedToast{ nullptr }; + winrt::Windows::Media::Playback::MediaPlayer _bellPlayer{ nullptr }; + void _ShowAboutDialog(); winrt::Windows::Foundation::IAsyncOperation _ShowQuitDialog(); winrt::Windows::Foundation::IAsyncOperation _ShowCloseWarningDialog(); From 02344048ded24500d2315a830c2838575167e1d1 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Tue, 16 Nov 2021 09:40:23 -0600 Subject: [PATCH 09/13] Revert "Experiment with removing the static MediaPlayer" This reverts commit fb1ed58d37ada60f4f3b4071d3117c9317ca47a4. --- src/cascadia/TerminalApp/Pane.cpp | 32 +++++++++++------------ src/cascadia/TerminalApp/Pane.h | 2 +- src/cascadia/TerminalApp/TerminalPage.cpp | 6 ----- src/cascadia/TerminalApp/TerminalPage.h | 2 -- 4 files changed, 17 insertions(+), 25 deletions(-) diff --git a/src/cascadia/TerminalApp/Pane.cpp b/src/cascadia/TerminalApp/Pane.cpp index 427e1184f0e..484f70c1721 100644 --- a/src/cascadia/TerminalApp/Pane.cpp +++ b/src/cascadia/TerminalApp/Pane.cpp @@ -33,7 +33,7 @@ static const Duration AnimationDuration = DurationHelper::FromTimeSpan(winrt::Wi winrt::Windows::UI::Xaml::Media::SolidColorBrush Pane::s_focusedBorderBrush = { nullptr }; winrt::Windows::UI::Xaml::Media::SolidColorBrush Pane::s_unfocusedBorderBrush = { nullptr }; -// winrt::Windows::Media::Playback::MediaPlayer Pane::s_bellPlayer = { nullptr }; +winrt::Windows::Media::Playback::MediaPlayer Pane::s_bellPlayer = { nullptr }; Pane::Pane(const Profile& profile, const TermControl& control, const bool lastFocused) : _control{ control }, @@ -71,14 +71,14 @@ Pane::Pane(const Profile& profile, const TermControl& control, const bool lastFo e.Handled(true); }); - // if (!s_bellPlayer) - // { - // try - // { - // s_bellPlayer = winrt::Windows::Media::Playback::MediaPlayer(); - // } - // CATCH_LOG(); - // } + if (!s_bellPlayer) + { + try + { + s_bellPlayer = winrt::Windows::Media::Playback::MediaPlayer(); + } + CATCH_LOG(); + } } Pane::Pane(std::shared_ptr first, @@ -1125,13 +1125,13 @@ winrt::fire_and_forget Pane::_playBellSound(winrt::Windows::Foundation::Uri uri) co_await winrt::resume_foreground(_root.Dispatcher()); if (auto pane{ weakThis.get() }) { - // if (s_bellPlayer) - // { - // auto source{ winrt::Windows::Media::Core::MediaSource::CreateFromUri(uri) }; - // auto item{ winrt::Windows::Media::Playback::MediaPlaybackItem(source) }; - // s_bellPlayer.Source(item); - // s_bellPlayer.Play(); - // } + if (s_bellPlayer) + { + auto source{ winrt::Windows::Media::Core::MediaSource::CreateFromUri(uri) }; + auto item{ winrt::Windows::Media::Playback::MediaPlaybackItem(source) }; + s_bellPlayer.Source(item); + s_bellPlayer.Play(); + } } } diff --git a/src/cascadia/TerminalApp/Pane.h b/src/cascadia/TerminalApp/Pane.h index f3537cb72d4..3e935d31dd7 100644 --- a/src/cascadia/TerminalApp/Pane.h +++ b/src/cascadia/TerminalApp/Pane.h @@ -229,7 +229,7 @@ class Pane : public std::enable_shared_from_this bool _zoomed{ false }; - // static winrt::Windows::Media::Playback::MediaPlayer s_bellPlayer; + static winrt::Windows::Media::Playback::MediaPlayer s_bellPlayer; bool _IsLeaf() const noexcept; bool _HasFocusedChild() const noexcept; diff --git a/src/cascadia/TerminalApp/TerminalPage.cpp b/src/cascadia/TerminalApp/TerminalPage.cpp index e1d5b8d35f7..81e2bc51709 100644 --- a/src/cascadia/TerminalApp/TerminalPage.cpp +++ b/src/cascadia/TerminalApp/TerminalPage.cpp @@ -56,12 +56,6 @@ namespace winrt::TerminalApp::implementation _hostingHwnd{} { InitializeComponent(); - - try - { - _bellPlayer = winrt::Windows::Media::Playback::MediaPlayer(); - } - CATCH_LOG(); } // Method Description: diff --git a/src/cascadia/TerminalApp/TerminalPage.h b/src/cascadia/TerminalApp/TerminalPage.h index 8b818dd6672..cf9477d2e3c 100644 --- a/src/cascadia/TerminalApp/TerminalPage.h +++ b/src/cascadia/TerminalApp/TerminalPage.h @@ -201,8 +201,6 @@ namespace winrt::TerminalApp::implementation std::shared_ptr _windowIdToast{ nullptr }; std::shared_ptr _windowRenameFailedToast{ nullptr }; - winrt::Windows::Media::Playback::MediaPlayer _bellPlayer{ nullptr }; - void _ShowAboutDialog(); winrt::Windows::Foundation::IAsyncOperation _ShowQuitDialog(); winrt::Windows::Foundation::IAsyncOperation _ShowCloseWarningDialog(); From 93937533bc00c53311867663a0a3a94d73253862 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Tue, 16 Nov 2021 09:51:33 -0600 Subject: [PATCH 10/13] it's BODGY time --- src/cascadia/TerminalApp/Pane.cpp | 21 +++++++++++++++++++++ src/cascadia/WindowsTerminal/AppHost.cpp | 2 +- 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/src/cascadia/TerminalApp/Pane.cpp b/src/cascadia/TerminalApp/Pane.cpp index 484f70c1721..efe515c47dc 100644 --- a/src/cascadia/TerminalApp/Pane.cpp +++ b/src/cascadia/TerminalApp/Pane.cpp @@ -76,6 +76,27 @@ Pane::Pane(const Profile& profile, const TermControl& control, const bool lastFo try { s_bellPlayer = winrt::Windows::Media::Playback::MediaPlayer(); + if (s_bellPlayer) + { + // BODGY + // + // Manually leak a ref to the MediaPlayer we just instantiated. + // We're doing this just like the way we do in AppHost with the + // App itself. + // + // We have to do this because there's some bug in the OS with + // the way a MediaPlayer gets torn down. At time fo writing (Nov + // 2021), if you search for `remove_SoundLevelChanged` in the OS + // repo, you'll find a pile of bugs. + // + // We tried moving the MediaPlayer singleton up to the + // TerminalPage, but alas, that teardown had the same problem. + // So _whatever_. We'll leak it here. It needs to last the + // lifetim of the app anyways, and it'll get cleaned up when the + // Termnial is closed, so whatever. + winrt::Windows::Media::Playback::MediaPlayer p{ s_bellPlayer }; + ::winrt::detach_abi(p); + } } CATCH_LOG(); } diff --git a/src/cascadia/WindowsTerminal/AppHost.cpp b/src/cascadia/WindowsTerminal/AppHost.cpp index 36d4892127d..db0951058bf 100644 --- a/src/cascadia/WindowsTerminal/AppHost.cpp +++ b/src/cascadia/WindowsTerminal/AppHost.cpp @@ -348,7 +348,7 @@ void AppHost::Initialize() _window->SetContent(_logic.GetRoot()); _window->OnAppInitialized(); - // THIS IS A HACK + // BODGY // // We've got a weird crash that happens terribly inconsistently, but pretty // readily on migrie's laptop, only in Debug mode. Apparently, there's some From 148f38253356c2a8463b4539c2a69f35545faaf0 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Tue, 4 Jan 2022 10:13:26 -0600 Subject: [PATCH 11/13] spel --- src/cascadia/TerminalApp/Pane.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/cascadia/TerminalApp/Pane.cpp b/src/cascadia/TerminalApp/Pane.cpp index 2e89751f41f..27ad1dc67c2 100644 --- a/src/cascadia/TerminalApp/Pane.cpp +++ b/src/cascadia/TerminalApp/Pane.cpp @@ -92,8 +92,8 @@ Pane::Pane(const Profile& profile, const TermControl& control, const bool lastFo // We tried moving the MediaPlayer singleton up to the // TerminalPage, but alas, that teardown had the same problem. // So _whatever_. We'll leak it here. It needs to last the - // lifetim of the app anyways, and it'll get cleaned up when the - // Termnial is closed, so whatever. + // lifetime of the app anyways, and it'll get cleaned up when the + // Terminal is closed, so whatever. winrt::Windows::Media::Playback::MediaPlayer p{ s_bellPlayer }; ::winrt::detach_abi(p); } From 796ce9fe3defe708a0c6c2f81c27defef3c8b403 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Wed, 5 Jan 2022 06:06:52 -0600 Subject: [PATCH 12/13] s/fo /of / --- src/cascadia/TerminalApp/Pane.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cascadia/TerminalApp/Pane.cpp b/src/cascadia/TerminalApp/Pane.cpp index 27ad1dc67c2..9784b2b0f70 100644 --- a/src/cascadia/TerminalApp/Pane.cpp +++ b/src/cascadia/TerminalApp/Pane.cpp @@ -85,7 +85,7 @@ Pane::Pane(const Profile& profile, const TermControl& control, const bool lastFo // App itself. // // We have to do this because there's some bug in the OS with - // the way a MediaPlayer gets torn down. At time fo writing (Nov + // the way a MediaPlayer gets torn down. At time of writing (Nov // 2021), if you search for `remove_SoundLevelChanged` in the OS // repo, you'll find a pile of bugs. // From 9f9cd4118ab95740ad76533c130ff33dd013d3ef Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Thu, 6 Jan 2022 11:11:25 -0600 Subject: [PATCH 13/13] Apply suggestions from code review Co-authored-by: Carlos Zamora --- src/cascadia/TerminalApp/Pane.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/cascadia/TerminalApp/Pane.cpp b/src/cascadia/TerminalApp/Pane.cpp index 9784b2b0f70..2fcdab15b9b 100644 --- a/src/cascadia/TerminalApp/Pane.cpp +++ b/src/cascadia/TerminalApp/Pane.cpp @@ -1068,8 +1068,8 @@ winrt::fire_and_forget Pane::_playBellSound(winrt::Windows::Foundation::Uri uri) { if (s_bellPlayer) { - auto source{ winrt::Windows::Media::Core::MediaSource::CreateFromUri(uri) }; - auto item{ winrt::Windows::Media::Playback::MediaPlaybackItem(source) }; + const auto source{ winrt::Windows::Media::Core::MediaSource::CreateFromUri(uri) }; + const auto item{ winrt::Windows::Media::Playback::MediaPlaybackItem(source) }; s_bellPlayer.Source(item); s_bellPlayer.Play(); }