libtgvoip icon indicating copy to clipboard operation
libtgvoip copied to clipboard

Fix default device not respecting ducking rules bug and handle leak

Open SpriteOvO opened this issue 3 years ago • 6 comments

Issue: telegramdesktop/tdesktop#8192

For my network reasons I was unable to compile tdesktop's dependency library FFmpeg, so I referenced libtgvoip and wrote a minimal test example that successfully reproduced the problem.

In this line: res=enumerator->GetDefaultAudioEndpoint(eRender, eCommunications, &device);

The problem only occurs in devices returned via GetDefaultAudioEndpoint with eCommunications. If replace eCommunications with eConsole or eMultimedia, the problem will not occur.

I think it's a Windows bug, but we can't fix it that way because the msdn docs say that the role argument may affect the returned default device in the future.

Link When the user changes the default rendering or capture device, the system assigns all three device roles (eConsole, eMultimedia, and eCommunications) to that device. Thus, GetDefaultAudioEndpoint always selects the default rendering or capture device, regardless of which role is indicated by the role parameter. In a future version of Windows, the user interface might enable the user to assign individual roles to different devices. In that case, the selection of a rendering or capture device by GetDefaultAudioEndpoint might depend on the role parameter. Thus, the behavior of an audio application developed to run in Windows Vista might change when run in a future version of Windows.

But there is a workaround to fix it, that we just get the name of the default device from GetDefaultAudioEndpoint, and then call GetDevice with the name to get the device.

Some references: https://social.microsoft.com/Forums/SECURITY/zh-CN/1b500f1e-744b-41d7-9c20-66ed83bda055/wasapi-stream-with-default-communictaions-device-does-not-trigger-ducking-for-other-applications?forum=windowspro-audiodevelopment https://chromium.googlesource.com/chromium/src/media/+/d3d352cbce26b68596104356a5420d65544fe903%5E!/ https://github.com/mumble-voip/mumble/blob/54e313481d2fe2fe36fbd30f55c5f055cbbee615/src/mumble/WASAPI.cpp#L335-L359

Minimal test example: https://github.com/SpriteOvO/TgCallBugTest

BTW, there is a handle leak in the device enumeration, that is, the device Release should be called after IMMDeviceCollection::Item.

Link Through this method, the caller obtains a counted reference to the interface. The caller is responsible for releasing the interface, when it is no longer needed, by calling the interface's Release method.

Finally, since I cannot compile tdesktop, the changes in this PR are untested. I only tested successfully with my minimal test example. But I'm happy to help test the compiled files.

SpriteOvO avatar Sep 06 '20 22:09 SpriteOvO