Merge pull request #267 from microsoft/pete-dev

Fix app-to-app MIDI stability problem

build/staging/version/BundleInfo.wxi

@@ -1,4 +1,4 @@
 <Include>
   <?define SetupVersionName="Developer Preview 5" ?>
-  <?define SetupVersionNumber="1.0.24039.2325" ?>
+  <?define SetupVersionNumber="1.0.24040.2018" ?>
 </Include>

diagnostics/trace-logging/trace.bat

@@ -0,0 +1,12 @@
+@echo off
+echo Must be run from an administrator command prompt
+
+wpr -start MidiServices.wprp -filemode
+
+echo Now go recreate the issue. Hit enter when done.
+
+pause
+
+wpr -stop TraceCaptureFile.etl
+
+start TraceCaptureFile.etl

samples/csharp-net/app-to-app-midi-cs/MainWindow.xaml.cs

@@ -38,19 +38,19 @@ public MainWindow()
 
             Notes = notes.Select(n=>new Note() { NoteNumber = n, Connection = _connection, GroupIndex = 0, ChannelIndex = 0 }).ToList();
 
-            //this.Closed += MainWindow_Closed;
+            this.Closed += MainWindow_Closed;
 
             //this.AppWindow.MoveAndResize(new Windows.Graphics.RectInt32(100, 100, 600, 600));
 
             this.SetWindowSize(500, 550);
             this.SetIsAlwaysOnTop(true);
 
-            this.Closed += MainWindow_Closed;
-
         }
 
         private void MainWindow_Closed(object sender, WindowEventArgs args)
         {
+            System.Diagnostics.Debug.WriteLine("MainWindow_Closed");
+
             if (_connection != null)
             {
                 _session.DisconnectEndpointConnection(_connection.ConnectionId);

samples/csharp-net/app-to-app-midi-cs/MidiSample.AppToAppMidi.csproj

@@ -29,7 +29,7 @@
   <ItemGroup>
     <PackageReference Include="Microsoft.WindowsAppSDK" Version="1.5.231202003-experimental1" />
     <PackageReference Include="Microsoft.Windows.SDK.BuildTools" Version="10.0.26031-preview" />
-    <PackageReference Include="Windows.Devices.Midi2" Version="1.0.0-preview.3-0145" />
+    <PackageReference Include="Windows.Devices.Midi2" Version="1.0.0-preview.3-0150" />
     <PackageReference Include="WinUIEx" Version="2.3.3" />
     <Manifest Include="$(ApplicationManifest)" />
   </ItemGroup>

src/api/Abstraction/VirtualMidiAbstraction/Midi2.VirtualMidiBidi.cpp

@@ -21,40 +21,66 @@ CMidi2VirtualMidiBiDi::Initialize(
     GUID /* SessionId */
 )
 {
-    OutputDebugString(__FUNCTION__ L" - enter\n");
-
     TraceLoggingWrite(
         MidiVirtualMidiAbstractionTelemetryProvider::Provider(),
-        __FUNCTION__, 
+        __FUNCTION__,
         TraceLoggingLevel(WINEVENT_LEVEL_INFO),
         TraceLoggingPointer(this, "this"),
         TraceLoggingWideString(endpointId, "endpoint id")
         );
 
-    m_callback = Callback;
     m_callbackContext = Context;
     m_endpointId = internal::NormalizeEndpointInterfaceIdWStringCopy(endpointId);
 
     //if (Context != MIDI_PROTOCOL_MANAGER_ENDPOINT_CREATION_CONTEXT)
     {
         HRESULT hr = S_OK;
 
-        // This should use SWD properties and not a string search
+        // TODO: This should use SWD properties and not a string search
 
         if (internal::EndpointInterfaceIdContainsString(m_endpointId, MIDI_VIRT_INSTANCE_ID_DEVICE_PREFIX))
         {
+            TraceLoggingWrite(
+                MidiVirtualMidiAbstractionTelemetryProvider::Provider(),
+                __FUNCTION__,
+                TraceLoggingLevel(WINEVENT_LEVEL_INFO),
+                TraceLoggingPointer(this, "this"),
+                TraceLoggingWideString(L"Initializing device-side BiDi", "message"),
+                TraceLoggingWideString(m_endpointId.c_str(), "endpoint id")
+            );
+
+            m_callback = Callback;
             m_isDeviceSide = true;
 
             LOG_IF_FAILED(hr = AbstractionState::Current().GetEndpointTable()->OnDeviceConnected(m_endpointId, this));
         }
         else if (internal::EndpointInterfaceIdContainsString(m_endpointId, MIDI_VIRT_INSTANCE_ID_CLIENT_PREFIX))
         {
+            TraceLoggingWrite(
+                MidiVirtualMidiAbstractionTelemetryProvider::Provider(),
+                __FUNCTION__,
+                TraceLoggingLevel(WINEVENT_LEVEL_INFO),
+                TraceLoggingPointer(this, "this"),
+                TraceLoggingWideString(L"Initializing client-side BiDi", "message"),
+                TraceLoggingWideString(m_endpointId.c_str(), "endpoint id")
+            );
+
+            m_callback = Callback;
             m_isDeviceSide = false;
 
             LOG_IF_FAILED(hr = AbstractionState::Current().GetEndpointTable()->OnClientConnected(m_endpointId, this));
         }
         else
         {
+            TraceLoggingWrite(
+                MidiVirtualMidiAbstractionTelemetryProvider::Provider(),
+                __FUNCTION__,
+                TraceLoggingLevel(WINEVENT_LEVEL_ERROR),
+                TraceLoggingPointer(this, "this"),
+                TraceLoggingWideString(L"We don't understand the endpoint Id", "message"),
+                TraceLoggingWideString(m_endpointId.c_str(), "endpoint id")
+                );
+
             // we don't understand this endpoint id
 
             hr = E_FAIL;
@@ -85,12 +111,16 @@ CMidi2VirtualMidiBiDi::Cleanup()
     m_callback = nullptr;
     m_callbackContext = 0;
 
+    UnlinkAllAssociatedBiDi();
+
     if (m_isDeviceSide)
     {
         LOG_IF_FAILED(AbstractionState::Current().GetEndpointTable()->OnDeviceDisconnected(m_endpointId));
     }
-
-    UnlinkAssociatedBiDi();
+    else
+    {
+        LOG_IF_FAILED(AbstractionState::Current().GetEndpointTable()->OnClientDisconnected(m_endpointId, this));
+    }
 
     return S_OK;
 }
@@ -117,13 +147,24 @@ CMidi2VirtualMidiBiDi::SendMidiMessage(
     RETURN_HR_IF_NULL(E_INVALIDARG, Message);
     RETURN_HR_IF(E_INVALIDARG, Size < sizeof(uint32_t));
 
-    // if there's no linked bidi, it's not a failure. We just lose the message
-    if (m_linkedBiDiCallback != nullptr)
+    for (auto connection : m_linkedBiDiConnections)
     {
-        //return m_linkedBiDi->SendMidiMessage(Message, Size, Position);
-        return m_linkedBiDiCallback->Callback(Message, Size, Position, m_callbackContext);
+        auto cb = connection->GetCallback();
+
+        if (cb != nullptr)
+        {
+            return cb->Callback(Message, Size, Position, m_callbackContext);
+        }
     }
 
+
+    //// if there's no linked bidi, it's not a failure. We just lose the message
+    //if (m_linkedBiDiCallback != nullptr)
+    //{
+    //    //return m_linkedBiDi->SendMidiMessage(Message, Size, Position);
+    //    return m_linkedBiDiCallback->Callback(Message, Size, Position, m_callbackContext);
+    //}
+
     return S_OK;
 
 }

src/api/Abstraction/VirtualMidiAbstraction/Midi2.VirtualMidiBidi.h

@@ -23,27 +23,53 @@ class CMidi2VirtualMidiBiDi :
 
     HRESULT LinkAssociatedBiDi(_In_ wil::com_ptr_nothrow<CMidi2VirtualMidiBiDi> biDi)
     {
-        m_linkedBiDi = biDi;
+        m_linkedBiDiConnections.push_back(biDi);
 
-        RETURN_IF_FAILED(biDi->QueryInterface(__uuidof(IMidiCallback), (void**)&m_linkedBiDiCallback));
+        //m_linkedBiDi = biDi;
+
+        //RETURN_IF_FAILED(biDi->QueryInterface(__uuidof(IMidiCallback), (void**)&m_linkedBiDiCallback));
 
         return S_OK;
     }
 
-    HRESULT UnlinkAssociatedBiDi()
+    HRESULT UnlinkAllAssociatedBiDi()
     {
-        m_linkedBiDi = nullptr;
-        m_linkedBiDiCallback = nullptr;
+        m_linkedBiDiConnections.clear();
+
+        return S_OK;
+    }
+
+    HRESULT UnlinkAssociatedBiDi(CMidi2VirtualMidiBiDi* biDiToUnlink)
+    {
+        auto it = std::find_if(m_linkedBiDiConnections.begin(), m_linkedBiDiConnections.end(),
+                    [&](const wil::com_ptr_nothrow<CMidi2VirtualMidiBiDi> bidi) { return bidi == biDiToUnlink; });
+
+        if (it != m_linkedBiDiConnections.end())
+        {
+            it->reset();
+            m_linkedBiDiConnections.erase(it);
+        }
+
+        //m_linkedBiDiCallback = nullptr;
 
         return S_OK;
     }
 
+    IMidiCallback* GetCallback()
+    {
+        return m_callback.get();
+    }
+
 private:
-    wil::com_ptr_nothrow<IMidiBiDi> m_linkedBiDi;
+    //wil::com_ptr_nothrow<IMidiBiDi> m_linkedBiDi;
 
-    wil::com_ptr_nothrow<IMidiCallback> m_linkedBiDiCallback;
+    std::vector<wil::com_ptr_nothrow<CMidi2VirtualMidiBiDi>> m_linkedBiDiConnections{};
+
+
+    //wil::com_ptr_nothrow<IMidiCallback> m_linkedBiDiCallback;
     wil::com_ptr_nothrow <IMidiCallback> m_callback;
 
+
     LONGLONG m_callbackContext;
 
     std::wstring m_endpointId{};

src/api/Abstraction/VirtualMidiAbstraction/Midi2.VirtualMidiConfigurationManager.cpp

@@ -16,8 +16,6 @@ CMidi2VirtualMidiConfigurationManager::Initialize(
     IUnknown* MidiDeviceManager
 )
 {
-    OutputDebugString(L"" __FUNCTION__ " Enter");
-
     TraceLoggingWrite(
         MidiVirtualMidiAbstractionTelemetryProvider::Provider(),
         __FUNCTION__,
@@ -45,7 +43,7 @@ CMidi2VirtualMidiConfigurationManager::UpdateConfiguration(
         __FUNCTION__,
         TraceLoggingLevel(WINEVENT_LEVEL_INFO),
         TraceLoggingPointer(this, "this"),
-        TraceLoggingWideString(ConfigurationJsonSection)
+        TraceLoggingWideString(ConfigurationJsonSection, "json")
     );
 
 
@@ -64,8 +62,8 @@ CMidi2VirtualMidiConfigurationManager::UpdateConfiguration(
             __FUNCTION__,
             TraceLoggingLevel(WINEVENT_LEVEL_ERROR),
             TraceLoggingPointer(this, "this"),
-            TraceLoggingWideString(L"Failed to parse Configuration JSON"),
-            TraceLoggingWideString(ConfigurationJsonSection)
+            TraceLoggingWideString(L"Failed to parse Configuration JSON", "message"),
+            TraceLoggingWideString(ConfigurationJsonSection, "json")
         );
 
         return E_FAIL;
@@ -159,8 +157,6 @@ CMidi2VirtualMidiConfigurationManager::UpdateConfiguration(
 HRESULT
 CMidi2VirtualMidiConfigurationManager::Cleanup()
 {
-    OutputDebugString(L"" __FUNCTION__ " Enter");
-
     TraceLoggingWrite(
         MidiVirtualMidiAbstractionTelemetryProvider::Provider(),
         __FUNCTION__,