Repository: microsoft/terminal (language: cpp)

Issue:
We might need another dependency for MIDI support
~Promoted from a Teams thread. Tracking here so we don't lose it. ~

Notes from @DHowett:
> @j4james handled this for us, by adding a package dependency on `GmDls`. We need to make sure that when Terminal is installed, users get access to it. I am somewhat concerned about what this means for Inbox Terminal, though: It costs us 3MB on disk which will be aggressively counted against us by Windows Perf Gates
> I looked at the code that loads gm.dls and it seems like it was never updated to consider Centennial apps. If a process has "strong identity" (a package), it forces a catalog enumeration to find the GmDls package. We might be able to defray the cost of pulling in a second copy of gm.dls to the Windows image by giving it the ability to "fail over" to the system32 one.


Discussion / hints:
Btw, if you want to revert PR #13208 until we can figure out a solution for this, that's fine by me. It's not exactly an essential feature.

And if in turns out that we can't use the MIDI API, I could try and come up with another way to produce the sound. I just chose MIDI because it seemed easiest.
Plan of record right now is to stash this behind velocity, so it only ships in Preview (and critically, not in the OS-side build). That way we can get a little longer window on figuring out how we want to resolve this.
Hi, I just tried to compile Terminal from 4e20a8631cbb5b20defe549a0e75bc7719969bee and installing it from a loose package as detailed in https://github.com/microsoft/terminal/blob/main/doc/building.md. 

I can confirm that installing the loose package fails for me with (pardon my French!)
~~~
Add-AppxPackage : Deployment failed with HRESULT: 0x80073CF3, Échec des mises à jour, de la dépendance ou de la validation des conflits du package.
Windows ne peut pas installer le package WindowsTerminalDev_0.0.1.0_x64__8wekyb3d8bbwe, car ce package dépend d’une infrastructure qui n’a pas pu être trouvée. Indiquez
l’infrastructure «Microsoft.Midi.GmDls» publiée par «CN=Microsoft Corporation, O=Microsoft Corporation, L=Redmond, S=Washington, C=US», avec une architecture neutre ou de
processeur x64 et la version minimale 1.0.0.0, en plus de ce package à installer. Les infrastructures avec le nom «Microsoft.Midi.GmDls» installées actuellement sont
Windows ne peut pas installer le package WindowsTerminalDev_0.0.1.0_x64__8wekyb3d8bbwe, car ce package dépend d’une infrastructure qui n’a pas pu être trouvée. Indiquez
l’infrastructure «Microsoft.Midi.GmDls» publiée par «CN=Microsoft Corporation, O=Microsoft Corporation, L=Redmond, S=Washington, C=US», avec une architecture neutre ou de
processeur x64 et la version minimale 1.0.0.0, en plus de ce package à installer. Les infrastructures avec le nom «Microsoft.Midi.GmDls» installées actuellement sont: {}
NOTE: For additional information, look for [ActivityId] 92e6da5a-7c16-0002-9b44-ea92167cd801 in the Event Log or use the command line Get-AppPackageLog -ActivityID
92e6da5a-7c16-0002-9b44-ea92167cd801
At line:1 char:1
+ Add-AppxPackage -Path ..\loose\AppxManifest.xml -Register -ForceUpdat ...
+ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    + CategoryInfo          : WriteError: (C:\Users\me...ppxManifest.xml:String) [Add-AppxPackage], IOException
    + FullyQualifiedErrorId : DeploymentError,Microsoft.Windows.Appx.PackageManager.Commands.AddAppxPackageCommand
~~~

I'm currently building from `9dca6c27eeeb4371c87cdc553d01b8876d028d21^` (i.e. 7dbe741e1) just to confirm that 9dca6c27eeeb4371c87cdc553d01b8876d028d21 is  indeed is the culprit, but since I found this issue in the meantime I'm pretty sure now :P
Curious. That package should be available on your system, @phil-blain, somewhere in `C:\Program Files (x86)`. It comes with the Windows SDK or one of the extension SDKs that ships with Visual Studio...
It is indeed in `C:\Program Files (x86)\Microsoft SDKs\Windows Kits\10\ExtensionSDKs\Microsoft.Midi.GmDls\10.0.22000.0`. Maybe the installation does not work because it is not listed as a `PackageDependency` in the genereated `AppxManifest.xml` ?
With #13258 merged, I'm moving this out to 1.16
@DHowett FYI, I've been playing around with an alternate implementation that uses DirectSound to generate the audio in place of Midi. Would it be worthwhile submitting a PR with that change, or do you have other plans for resolving this?

Fix patch:
diff --git a/.github/actions/spelling/expect/expect.txt b/.github/actions/spelling/expect/expect.txt
index 320a4bc1976..c1a1a3015f0 100644
--- a/.github/actions/spelling/expect/expect.txt
+++ b/.github/actions/spelling/expect/expect.txt
@@ -438,7 +438,9 @@ ctlseqs
 Ctlv
 ctor
 CTRLEVENT
+CTRLFREQUENCY
 CTRLKEYSHORTCUTS
+CTRLVOLUME
 Ctx
 Ctxt
 ctype
@@ -661,7 +663,14 @@ dropdown
 DROPDOWNLIST
 DROPFILES
 drv
+DSBCAPS
+DSBLOCK
+DSBPLAY
+DSBUFFERDESC
+DSBVOLUME
 dsm
+dsound
+DSSCL
 dst
 DSwap
 DTest
@@ -682,6 +691,7 @@ dwrite
 dwriteglyphrundescriptionclustermap
 dxgi
 dxgidwm
+dxguid
 dxinterop
 dxp
 dxsm
@@ -718,6 +728,7 @@ endptr
 endregion
 ENQ
 enqueuing
+ENTIREBUFFER
 entrypoint
 ENU
 enum
@@ -934,6 +945,7 @@ gitfilters
 github
 gitlab
 gle
+GLOBALFOCUS
 globals
 GLYPHENTRY
 gmail
@@ -1369,6 +1381,7 @@ lpv
 LPVOID
 LPW
 LPWCH
+lpwfx
 LPWINDOWPOS
 lpwpos
 lpwstr
@@ -2678,6 +2691,7 @@ WANTARROWS
 WANTTAB
 wapproj
 wav
+WAVEFORMATEX
 wbuilder
 wch
 wchar
diff --git a/src/audio/midi/MidiAudio.cpp b/src/audio/midi/MidiAudio.cpp
index c87a26cd426..846bb7a4306 100644
--- a/src/audio/midi/MidiAudio.cpp
+++ b/src/audio/midi/MidiAudio.cpp
@@ -5,55 +5,30 @@
 #include "MidiAudio.hpp"
 #include "../terminal/parser/stateMachine.hpp"
 
-namespace
-{
-    class MidiOut
-    {
-    public:
-        static constexpr auto NOTE_OFF = 0x80;
-        static constexpr auto NOTE_ON = 0x90;
-        static constexpr auto PROGRAM_CHANGE = 0xC0;
+#include <dsound.h>
 
-        // We're using a square wave as an approximation of the sound that the
-        // original VT525 terminals might have produced. This is probably not
-        // quite right, but it works reasonably well.
-        static constexpr auto SQUARE_WAVE_SYNTH = 80;
+#pragma comment(lib, "dxguid.lib")
+#pragma comment(lib, "dsound.lib")
 
-        MidiOut() noexcept
-        {
-            if constexpr (Feature_DECPSViaMidiPlayer::IsEnabled())
-            {
-                midiOutOpen(&handle, MIDI_MAPPER, NULL, NULL, CALLBACK_NULL);
-                OutputMessage(PROGRAM_CHANGE, SQUARE_WAVE_SYNTH);
-            }
-        }
-        ~MidiOut() noexcept
-        {
-            if constexpr (Feature_DECPSViaMidiPlayer::IsEnabled())
-            {
-                midiOutClose(handle);
-            }
-        }
-        void OutputMessage(const int b1, const int b2, const int b3 = 0, const int b4 = 0) noexcept
-        {
-            if constexpr (Feature_DECPSViaMidiPlayer::IsEnabled())
-            {
-                midiOutShortMsg(handle, MAKELONG(MAKEWORD(b1, b2), MAKEWORD(b3, b4)));
-            }
-        }
+using Microsoft::WRL::ComPtr;
+using namespace std::chrono_literals;
 
-        MidiOut(const MidiOut&) = delete;
-        MidiOut(MidiOut&&) = delete;
-        MidiOut& operator=(const MidiOut&) = delete;
-        MidiOut& operator=(MidiOut&&) = delete;
+// The WAVE_DATA below is an 8-bit PCM encoding of a triangle wave form.
+// We just play this on repeat at varying frequencies to produce our notes.
+constexpr auto WAVE_SIZE = 16u;
+constexpr auto WAVE_DATA = std::array<byte, WAVE_SIZE>{ 128, 159, 191, 223, 255, 223, 191, 159, 128, 96, 64, 32, 0, 32, 64, 96 };
 
-    private:
-        HMIDIOUT handle = nullptr;
-    };
+MidiAudio::MidiAudio(HWND windowHandle)
+{
+    if (SUCCEEDED(DirectSoundCreate8(nullptr, &_directSound, nullptr)))
+    {
+        if (SUCCEEDED(_directSound->SetCooperativeLevel(windowHandle, DSSCL_NORMAL)))
+        {
+            _createBuffers();
+        }
+    }
 }
 
-using namespace std::chrono_literals;
-
 MidiAudio::~MidiAudio() noexcept
 {
     try
@@ -61,7 +36,7 @@ MidiAudio::~MidiAudio() noexcept
 #pragma warning(suppress : 26447)
         // We acquire the lock here so the class isn't destroyed while in use.
         // If this throws, we'll catch it, so the C26447 warning is bogus.
-        _inUseMutex.lock();
+        const auto lock = std::unique_lock{ _inUseMutex };
     }
     catch (...)
     {
@@ -103,13 +78,26 @@ void MidiAudio::Unlock()
 void MidiAudio::PlayNote(const int noteNumber, const int velocity, const std::chrono::microseconds duration) noexcept
 try
 {
-    // The MidiOut is a local static because we can only have one instance,
-    // and we only want to construct it when it's actually needed.
-    static MidiOut midiOut;
-
-    if (velocity)
+    const auto& buffer = _buffers.at(_activeBufferIndex);
+    if (velocity && buffer)
     {
-        midiOut.OutputMessage(MidiOut::NOTE_ON, noteNumber, velocity);
+        // The formula for frequency is 2^(n/12) * 440Hz, where n is zero for
+        // the A above middle C (A4). In MIDI terms, A4 is note number 69,
+        // which is why we subtract 69. We also need to multiply by the size
+        // of the wave form to determine the frequency that the sound buffer
+        // has to be played to achieve the equivalent note frequency.
+        const auto frequency = std::pow(2.0, (noteNumber - 69.0) / 12.0) * 440.0 * WAVE_SIZE;
+        buffer->SetFrequency(gsl::narrow_cast<DWORD>(frequency));
+        // For the volume, we're using the formula defined in the the General
+        // MIDI Level 2 specification: Gain in dB = 40 * log10(v/127). We need
+        // to multiply by 4000, though, because the SetVolume method expects
+        // the volume to be in hundredths of a decibel.
+        const auto volume = 4000.0 * std::log10(velocity / 127.0);
+        buffer->SetVolume(gsl::narrow_cast<LONG>(volume));
+        // Resetting the buffer to a position that is slightly off from the
+        // last position will help to produce a clearer separation between
+        // tones when repeating sequences of the same note.
+        buffer->SetCurrentPosition((_lastBufferPosition + 12) % WAVE_SIZE);
     }
 
     // By waiting on the shutdown future with the duration of the note, we'll
@@ -117,9 +105,48 @@ try
     // of the wait early if we've been shutdown.
     _shutdownFuture.wait_for(duration);
 
-    if (velocity)
+    if (velocity && buffer)
     {
-        midiOut.OutputMessage(MidiOut::NOTE_OFF, noteNumber, velocity);
+        // When the note ends, we just turn the volume down instead of stopping
+        // the sound buffer. This helps reduce unwanted static between notes.
+        buffer->SetVolume(DSBVOLUME_MIN);
+        buffer->GetCurrentPosition(&_lastBufferPosition, nullptr);
     }
+
+    // Cycling between multiple buffers can also help reduce the static.
+    _activeBufferIndex = (_activeBufferIndex + 1) % _buffers.size();
 }
 CATCH_LOG()
+
+void MidiAudio::_createBuffers() noexcept
+{
+    auto waveFormat = WAVEFORMATEX{};
+    waveFormat.wFormatTag = WAVE_FORMAT_PCM;
+    waveFormat.nChannels = 1;
+    waveFormat.nSamplesPerSec = 8000;
+    waveFormat.wBitsPerSample = 8;
+    waveFormat.nBlockAlign = waveFormat.nChannels * waveFormat.wBitsPerSample / 8;
+    waveFormat.nAvgBytesPerSec = waveFormat.nSamplesPerSec * waveFormat.nBlockAlign;
+
+    auto bufferDescription = DSBUFFERDESC{};
+    bufferDescription.dwSize = sizeof(DSBUFFERDESC);
+    bufferDescription.dwFlags = DSBCAPS_CTRLVOLUME | DSBCAPS_CTRLFREQUENCY | DSBCAPS_GLOBALFOCUS;
+    bufferDescription.dwBufferBytes = WAVE_SIZE;
+    bufferDescription.lpwfxFormat = &waveFormat;
+
+    for (auto& buffer : _buffers)
+    {
+        if (SUCCEEDED(_directSound->CreateSoundBuffer(&bufferDescription, &buffer, nullptr)))
+        {
+            LPVOID bufferPtr;
+            DWORD bufferSize;
+            if (SUCCEEDED(buffer->Lock(0, 0, &bufferPtr, &bufferSize, nullptr, nullptr, DSBLOCK_ENTIREBUFFER)))
+            {
+                std::memcpy(bufferPtr, WAVE_DATA.data(), WAVE_DATA.size());
+                buffer->Unlock(bufferPtr, bufferSize, nullptr, 0);
+            }
+            buffer->SetVolume(DSBVOLUME_MIN);
+            buffer->Play(0, 0, DSBPLAY_LOOPING);
+        }
+    }
+}
diff --git a/src/audio/midi/MidiAudio.hpp b/src/audio/midi/MidiAudio.hpp
index 435276940ab..c29d38d1999 100644
--- a/src/audio/midi/MidiAudio.hpp
+++ b/src/audio/midi/MidiAudio.hpp
@@ -11,13 +11,17 @@ Module Name:
 
 #pragma once
 
+#include <array>
 #include <future>
 #include <mutex>
 
+struct IDirectSound8;
+struct IDirectSoundBuffer;
+
 class MidiAudio
 {
 public:
-    MidiAudio() = default;
+    MidiAudio(HWND windowHandle);
     MidiAudio(const MidiAudio&) = delete;
     MidiAudio(MidiAudio&&) = delete;
     MidiAudio& operator=(const MidiAudio&) = delete;
@@ -30,6 +34,12 @@ class MidiAudio
     void PlayNote(const int noteNumber, const int velocity, const std::chrono::microseconds duration) noexcept;
 
 private:
+    void _createBuffers() noexcept;
+
+    Microsoft::WRL::ComPtr<IDirectSound8> _directSound;
+    std::array<Microsoft::WRL::ComPtr<IDirectSoundBuffer>, 2> _buffers;
+    size_t _activeBufferIndex = 0;
+    DWORD _lastBufferPosition = 0;
     std::promise<void> _shutdownPromise;
     std::future<void> _shutdownFuture;
     std::mutex _inUseMutex;
diff --git a/src/cascadia/TerminalControl/ControlCore.cpp b/src/cascadia/TerminalControl/ControlCore.cpp
index 89512563e4c..f06b4d7090a 100644
--- a/src/cascadia/TerminalControl/ControlCore.cpp
+++ b/src/cascadia/TerminalControl/ControlCore.cpp
@@ -1342,7 +1342,8 @@ namespace winrt::Microsoft::Terminal::Control::implementation
     {
         if (!_midiAudio)
         {
-            _midiAudio = std::make_unique<MidiAudio>();
+            const auto windowHandle = reinterpret_cast<HWND>(_owningHwnd);
+            _midiAudio = std::make_unique<MidiAudio>(windowHandle);
             _midiAudio->Initialize();
         }
         return *_midiAudio;
diff --git a/src/cascadia/TerminalControl/TerminalControlLib.vcxproj b/src/cascadia/TerminalControl/TerminalControlLib.vcxproj
index 478e7a3983a..ab92fc6cb37 100644
--- a/src/cascadia/TerminalControl/TerminalControlLib.vcxproj
+++ b/src/cascadia/TerminalControl/TerminalControlLib.vcxproj
@@ -147,10 +147,6 @@
     <PRIResource Include="Resources\en-US\Resources.resw" />
     <OCResourceDirectory Include="Resources" />
   </ItemGroup>
-  <ItemGroup Condition="'$(WindowsTerminalBranding)'=='' or '$(WindowsTerminalBranding)'=='Dev' or '$(WindowsTerminalBranding)'=='Preview'">
-    <!-- GH#13252 Only vend this dependency for Dev and Preview builds. -->
-    <SDKReference Include="Microsoft.Midi.GmDls, Version=10.0.22000.0" />
-  </ItemGroup>
   <!-- ========================= Project References ======================== -->
   <ItemGroup>
     <ProjectReference Include="..\..\types\lib\types.vcxproj" />
diff --git a/src/features.xml b/src/features.xml
index a8db865ddc7..cbe55f3a5ad 100644
--- a/src/features.xml
+++ b/src/features.xml
@@ -117,18 +117,6 @@
         </alwaysDisabledBrandingTokens>
     </feature>
 
-    <feature>
-        <name>Feature_DECPSViaMidiPlayer</name>
-        <description>Enables playing sound via DECPS using the MIDI player.</description>
-        <stage>AlwaysDisabled</stage>
-        <!-- We're disabling this for WindowsInbox and Stable because it requires an additional
-             package dependency or library dependency. -->
-        <alwaysEnabledBrandingTokens>
-            <brandingToken>Dev</brandingToken>
-            <brandingToken>Preview</brandingToken>
-        </alwaysEnabledBrandingTokens>
-    </feature>
-
     <feature>
         <name>Feature_ScrollbarMarks</name>
         <description>Enables the experimental scrollbar marks feature.</description>
diff --git a/src/host/consoleInformation.cpp b/src/host/consoleInformation.cpp
index d5399b3e764..21c7428d5a4 100644
--- a/src/host/consoleInformation.cpp
+++ b/src/host/consoleInformation.cpp
@@ -382,7 +382,8 @@ MidiAudio& CONSOLE_INFORMATION::GetMidiAudio()
 {
     if (!_midiAudio)
     {
-        _midiAudio = std::make_unique<MidiAudio>();
+        const auto windowHandle = ServiceLocator::LocateConsoleWindow()->GetWindowHandle();
+        _midiAudio = std::make_unique<MidiAudio>(windowHandle);
         _midiAudio->Initialize();
     }
     return *_midiAudio;


Test patch: