From ca26195f05e6e16ae276b57b67367fda52476228 Mon Sep 17 00:00:00 2001
From: Andreas Traczyk <andreas.traczyk@savoirfairelinux.com>
Date: Fri, 25 Oct 2019 15:36:00 -0400
Subject: [PATCH] videomonitor(win32): bridge ffmpeg/win32api device ids

- Fixes wstring conversion issue when obtaining video capture device
  friendly name, and uses device id when adding/removing devices using
  the usb-pnp monitoring APIs. As the uid suffix reported by the pnp
  api is different from what returns from the device enumerator, we
  strip the uid and make substring searches for device nodes instead
  of exact matches.

Change-Id: Ibe3d79ed0632c663c2b067772ca592d5c5d466e6
Gitlab: #171
---
 src/media/video/video_device_monitor.cpp      |   6 +-
 .../video/winvideo/video_device_impl.cpp      | 255 ++++++++++--------
 .../winvideo/video_device_monitor_impl.cpp    | 116 +++-----
 src/string_utils.cpp                          |  12 -
 src/string_utils.h                            |   1 -
 5 files changed, 181 insertions(+), 209 deletions(-)

diff --git a/src/media/video/video_device_monitor.cpp b/src/media/video/video_device_monitor.cpp
index 7508e4661c..6bffe160ba 100644
--- a/src/media/video/video_device_monitor.cpp
+++ b/src/media/video/video_device_monitor.cpp
@@ -284,7 +284,7 @@ vector<VideoDevice>::iterator
 VideoDeviceMonitor::findDeviceByNode(const string& node)
 {
     for (auto it = devices_.begin(); it != devices_.end(); ++it)
-        if (it->getNode() == node)
+        if (it->getNode().find(node) != std::string::npos)
             return it;
     return devices_.end();
 }
@@ -293,7 +293,7 @@ vector<VideoDevice>::const_iterator
 VideoDeviceMonitor::findDeviceByNode(const string& node) const
 {
     for (auto it = devices_.cbegin(); it != devices_.cend(); ++it)
-        if (it->getNode() == node)
+        if (it->getNode().find(node) != std::string::npos)
             return it;
     return devices_.end();
 }
@@ -348,5 +348,5 @@ VideoDeviceMonitor::unserialize(const YAML::Node &in)
     else
         defaultDevice_ = first;
 }
-
+#pragma optimize("", on)
 }} // namespace jami::video
diff --git a/src/media/video/winvideo/video_device_impl.cpp b/src/media/video/winvideo/video_device_impl.cpp
index d77d2b7821..44155cb268 100644
--- a/src/media/video/winvideo/video_device_impl.cpp
+++ b/src/media/video/winvideo/video_device_impl.cpp
@@ -67,7 +67,8 @@ class VideoDeviceImpl {
 };
 
 VideoDeviceImpl::VideoDeviceImpl(const std::string& path)
-    : name(path)
+    : device(path)
+    , name()
     , cInterface(new CaptureGraphInterfaces())
 {
     setup();
@@ -81,14 +82,14 @@ VideoDeviceImpl::setup()
         nullptr,
         CLSCTX_INPROC_SERVER,
         IID_ICaptureGraphBuilder2,
-        (void**) &cInterface->captureGraph_);
+        (void**)&cInterface->captureGraph_);
     if (FAILED(hr))
         return fail("Could not create the Filter Graph Manager");
 
     hr = CoCreateInstance(CLSID_FilterGraph,
         nullptr,
-        CLSCTX_INPROC_SERVER,IID_IGraphBuilder,
-        (void**) &cInterface->graph_);
+        CLSCTX_INPROC_SERVER, IID_IGraphBuilder,
+        (void**)&cInterface->graph_);
     if (FAILED(hr))
         return fail("Could not add the graph builder!");
 
@@ -96,122 +97,152 @@ VideoDeviceImpl::setup()
     if (FAILED(hr))
         return fail("Could not set filtergraph.");
 
-    ICreateDevEnum *pSysDevEnum = nullptr;
-    hr = CoCreateInstance(CLSID_SystemDeviceEnum,
-        nullptr,
-        CLSCTX_INPROC_SERVER,
-        IID_ICreateDevEnum,
-        (void **)&pSysDevEnum);
-    if (FAILED(hr))
-        return fail("Could not create the enumerator!");
+    ICreateDevEnum *pDevEnum;
+    hr = CoCreateInstance(CLSID_SystemDeviceEnum, NULL,
+        CLSCTX_INPROC_SERVER, IID_PPV_ARGS(&pDevEnum));
 
-    IEnumMoniker* pEnumCat = nullptr;
-    hr = pSysDevEnum->CreateClassEnumerator(
-        CLSID_VideoInputDeviceCategory,
-        &pEnumCat,
-        0);
-    if (SUCCEEDED(hr)) {
-        // Auto-deletion at if {} exist or at exception
-        auto IEnumMonikerDeleter = [](IEnumMoniker* p){ p->Release(); };
-        std::unique_ptr<IEnumMoniker, decltype(IEnumMonikerDeleter)&> pEnumCatGuard {pEnumCat, IEnumMonikerDeleter};
-
-        IMoniker *pMoniker = nullptr;
-        ULONG cFetched;
-        while ((pEnumCatGuard->Next(1, &pMoniker, &cFetched) == S_OK)) {
-            IPropertyBag *pPropBag;
-            hr = pMoniker->BindToStorage(0, 0, IID_IPropertyBag, (void **)&pPropBag);
-            if (FAILED(hr)) {
-                continue;
-            }
-            VARIANT var;
-            VariantInit(&var);
-            hr = pPropBag->Read(L"FriendlyName", &var, 0);
-            if (SUCCEEDED(hr)) {
-                // We want to get the capabilities of a device with the friendly name
-                // that corresponds to what was enumerated by the video device monitor,
-                // and passed in the ctor as the name of this device.
-                if (this->name != bstrToStdString(var.bstrVal)) {
-                    continue;
-                }
-                this->device = std::string("video=") + this->name;
-                hr = pMoniker->BindToObject(
-                    nullptr, nullptr,
-                    IID_IBaseFilter,
-                    (void**)&cInterface->videoInputFilter_);
-                if (SUCCEEDED(hr))
-                    hr = cInterface->graph_->AddFilter(
-                        cInterface->videoInputFilter_,
-                        var.bstrVal);
-                else {
-                    fail("Could not add filter to video device.");
-                }
-                hr = cInterface->captureGraph_->FindInterface(
-                    &PIN_CATEGORY_PREVIEW,
-                    &MEDIATYPE_Video,
-                    cInterface->videoInputFilter_,
-                    IID_IAMStreamConfig,
-                    (void **)&cInterface->streamConf_);
-                if (FAILED(hr)) {
-                    hr = cInterface->captureGraph_->FindInterface(
-                        &PIN_CATEGORY_CAPTURE,
-                        &MEDIATYPE_Video,
-                        cInterface->videoInputFilter_,
-                        IID_IAMStreamConfig,
-                        (void **)&cInterface->streamConf_);
-                    if (FAILED(hr)) {
-                        fail("Couldn't config the stream!");
-                    }
-                }
-                // Device found.
-                break;
-            }
-            VariantClear(&var);
-            pPropBag->Release();
-            pPropBag = nullptr;
+    IEnumMoniker* pEnum = NULL;
+    hr = pDevEnum->CreateClassEnumerator(
+        CLSID_VideoInputDeviceCategory, &pEnum, 0);
+    if (hr == S_FALSE) {
+        hr = VFW_E_NOT_FOUND;
+    }
+    pDevEnum->Release();
+    if (FAILED(hr) || pEnum == nullptr) {
+        JAMI_ERR() << "No webcam found";
+        return;
+    }
+
+    // Auto-deletion at exception
+    auto IEnumMonikerDeleter = [](IEnumMoniker* p) { p->Release(); };
+    std::unique_ptr<IEnumMoniker, decltype(IEnumMonikerDeleter)&> pEnumGuard{ pEnum, IEnumMonikerDeleter };
+
+    IMoniker *pMoniker = NULL;
+    while ((pEnumGuard->Next(1, &pMoniker, NULL) == S_OK)) {
+        IPropertyBag *pPropBag;
+        hr = pMoniker->BindToStorage(0, 0, IID_IPropertyBag, (void **)&pPropBag);
+        if (FAILED(hr)) {
             pMoniker->Release();
-            pMoniker = nullptr;
+            continue;
+        }
+
+        IBindCtx *bind_ctx = NULL;
+        LPOLESTR olestr = NULL;
+
+        hr = CreateBindCtx(0, &bind_ctx);
+        if (hr != S_OK) {
+            continue;
+        }
+        hr = pMoniker->GetDisplayName(bind_ctx, NULL, &olestr);
+        if (hr != S_OK) {
+            continue;
+        }
+        auto unique_name = to_string(olestr);
+        if (unique_name.empty()) {
+            continue;
+        }
+
+        // replace ':' with '_' since ffmpeg uses : to delineate between sources */
+        std::replace(unique_name.begin(), unique_name.end(), ':', '_');
+        unique_name = std::string("video=") + unique_name;
+
+        // We want to get the capabilities of a device with the unique_name
+        // that corresponds to what was enumerated by the video device monitor.
+        if (unique_name.find(this->device) == std::string::npos) {
+            continue;
+        }
+
+        this->device = unique_name;
+
+        // get friendly name
+        VARIANT var;
+        VariantInit(&var);
+        hr = pPropBag->Read(L"Description", &var, 0);
+        if (FAILED(hr)) {
+            hr = pPropBag->Read(L"FriendlyName", &var, 0);
         }
         if (SUCCEEDED(hr)) {
-            int piCount;
-            int piSize;
-            cInterface->streamConf_->GetNumberOfCapabilities(&piCount, &piSize);
-            AM_MEDIA_TYPE *pmt;
-            VIDEO_STREAM_CONFIG_CAPS pSCC;
-            std::map<std::pair<jami::video::VideoSize, jami::video::FrameRate>, LONG> bitrateList;
-            for (int i = 0; i < piCount; i++) {
-                cInterface->streamConf_->GetStreamCaps(i, &pmt, (BYTE*)&pSCC);
-                if (pmt->formattype != FORMAT_VideoInfo) {
-                    continue;
-                }
-                auto videoInfo = (VIDEOINFOHEADER*) pmt->pbFormat;
-                auto size = jami::video::VideoSize(videoInfo->bmiHeader.biWidth, videoInfo->bmiHeader.biHeight);
-                auto rate = jami::video::FrameRate(1e7, videoInfo->AvgTimePerFrame);
-                auto bitrate = videoInfo->dwBitRate;
-                // Only add configurations with positive bitrates.
-                if (bitrate == 0)
-                    continue;
-                // Avoid adding multiple rates with different bitrates.
-                auto ratesIt = rateList_.find(size);
-                if (ratesIt != rateList_.end() &&
-                    std::find(ratesIt->second.begin(), ratesIt->second.end(), rate) != ratesIt->second.end()) {
-                    // Update bitrate and cap map if the bitrate is greater.
-                    auto key = std::make_pair(size, rate);
-                    if (bitrate > bitrateList[key]) {
-                        bitrateList[key] = bitrate;
-                        capMap_[size] = pmt;
-                    }
-                    continue;
-                }
-                // Add new size, rate, bitrate, and cap map.
-                sizeList_.emplace_back(size);
-                rateList_[size].emplace_back(rate);
-                bitrateList[std::make_pair(size, rate)] = bitrate;
+            this->name = to_string(var.bstrVal);
+        }
+        pPropBag->Write(L"FriendlyName", &var);
+
+        hr = pMoniker->BindToObject(
+            nullptr, nullptr,
+            IID_IBaseFilter,
+            (void**)&cInterface->videoInputFilter_);
+        if (SUCCEEDED(hr))
+            hr = cInterface->graph_->AddFilter(
+                cInterface->videoInputFilter_,
+                var.bstrVal);
+        else {
+            fail("Could not add filter to video device.");
+        }
+        hr = cInterface->captureGraph_->FindInterface(
+            &PIN_CATEGORY_PREVIEW,
+            &MEDIATYPE_Video,
+            cInterface->videoInputFilter_,
+            IID_IAMStreamConfig,
+            (void **)&cInterface->streamConf_);
+        if (FAILED(hr)) {
+            hr = cInterface->captureGraph_->FindInterface(
+                &PIN_CATEGORY_CAPTURE,
+                &MEDIATYPE_Video,
+                cInterface->videoInputFilter_,
+                IID_IAMStreamConfig,
+                (void **)&cInterface->streamConf_);
+            if (FAILED(hr)) {
+                fail("Couldn't config the stream!");
+            }
+        }
+
+        VariantClear(&var);
+        pPropBag->Release();
+
+        // Device found.
+        break;
+    }
+    pMoniker->Release();
+
+    if (FAILED(hr) || cInterface->streamConf_ == NULL) {
+        fail("Could not find the video device.");
+    }
+
+    int piCount;
+    int piSize;
+    cInterface->streamConf_->GetNumberOfCapabilities(&piCount, &piSize);
+    AM_MEDIA_TYPE *pmt;
+    VIDEO_STREAM_CONFIG_CAPS pSCC;
+    std::map<std::pair<jami::video::VideoSize, jami::video::FrameRate>, LONG> bitrateList;
+    for (int i = 0; i < piCount; i++) {
+        cInterface->streamConf_->GetStreamCaps(i, &pmt, (BYTE*)&pSCC);
+        if (pmt->formattype != FORMAT_VideoInfo) {
+            continue;
+        }
+        auto videoInfo = (VIDEOINFOHEADER*)pmt->pbFormat;
+        auto size = jami::video::VideoSize(videoInfo->bmiHeader.biWidth, videoInfo->bmiHeader.biHeight);
+        auto rate = jami::video::FrameRate(1e7, videoInfo->AvgTimePerFrame);
+        auto bitrate = videoInfo->dwBitRate;
+        // Only add configurations with positive bitrates.
+        if (bitrate == 0)
+            continue;
+        // Avoid adding multiple rates with different bitrates.
+        auto ratesIt = rateList_.find(size);
+        if (ratesIt != rateList_.end() &&
+            std::find(ratesIt->second.begin(), ratesIt->second.end(), rate) != ratesIt->second.end()) {
+            // Update bitrate and cap map if the bitrate is greater.
+            auto key = std::make_pair(size, rate);
+            if (bitrate > bitrateList[key]) {
+                bitrateList[key] = bitrate;
                 capMap_[size] = pmt;
             }
+            continue;
         }
+        // Add new size, rate, bitrate, and cap map.
+        sizeList_.emplace_back(size);
+        rateList_[size].emplace_back(rate);
+        bitrateList[std::make_pair(size, rate)] = bitrate;
+        capMap_[size] = pmt;
     }
-    pSysDevEnum->Release();
-    pSysDevEnum = NULL;
 }
 
 void
diff --git a/src/media/video/winvideo/video_device_monitor_impl.cpp b/src/media/video/winvideo/video_device_monitor_impl.cpp
index 8ef60e284f..2230d942b8 100644
--- a/src/media/video/winvideo/video_device_monitor_impl.cpp
+++ b/src/media/video/winvideo/video_device_monitor_impl.cpp
@@ -83,66 +83,18 @@ VideoDeviceMonitorImpl::~VideoDeviceMonitorImpl()
 }
 
 std::string
-getDeviceFriendlyName(PDEV_BROADCAST_DEVICEINTERFACE pbdi)
+getDeviceUniqueName(PDEV_BROADCAST_DEVICEINTERFACE pbdi)
 {
-    // As per: https://www.codeproject.com/Articles/14500/Detecting-Hardware-Insertion-and-or-Removal
-    // we need to convert the usb device descriptor string from:
-    //   \\?\USB#Vid_04e8&Pid_503b#0002F9A9828E0F06#{a5dcbf10-6530-11d2-901f-00c04fb951ed}
-    // to something like:
-    //   USB\Vid_04e8&Pid_503b\0002F9A9828E0F06
-    // in order to match the device's registry entry, and finally obtain the friendly name. (-_-)?
-
-    std::string friendlyName;
-    std::string name = pbdi->dbcc_name;
-    name = name.substr(4);
-    auto pos = name.find("#");
-    name.replace(pos, 1, "\\");
-    pos = name.find_last_of("#");
-    name = name.substr(0, pos);
-    pos = name.find("#");
-    name.replace(pos, 1, "\\");
-    std::transform(name.begin(), name.end(), name.begin(),
-        [](unsigned char c) { return std::toupper(c); }
-    );
-
-    DWORD dwFlag = DIGCF_ALLCLASSES;
-    HDEVINFO hDevInfo = SetupDiGetClassDevs(&guidCamera, 0, NULL, dwFlag);
-    if (INVALID_HANDLE_VALUE == hDevInfo) {
-        return {};
-    }
-
-    SP_DEVINFO_DATA* pspDevInfoData =
-        (SP_DEVINFO_DATA*)HeapAlloc(GetProcessHeap(), 0, sizeof(SP_DEVINFO_DATA));
-    pspDevInfoData->cbSize = sizeof(SP_DEVINFO_DATA);
-
-    for (int i = 0; SetupDiEnumDeviceInfo(hDevInfo, i, pspDevInfoData); i++) {
-        GUID guid;
-        guid = pspDevInfoData->ClassGuid;
+    std::string unique_name = pbdi->dbcc_name;
 
-        DWORD DataT;
-        DWORD nSize = 0;
-        TCHAR buf[260];
-
-        if (!SetupDiGetDeviceInstanceId(hDevInfo, pspDevInfoData, buf, sizeof(buf), &nSize)) {
-            break;
-        }
-
-        std::string strBuf(&buf[0]);
-        if (strBuf.find(name) != std::string::npos) {
-            if (SetupDiGetDeviceRegistryProperty(hDevInfo, pspDevInfoData,
-                SPDRP_FRIENDLYNAME, &DataT, (PBYTE)buf, sizeof(buf), &nSize)) {
-                friendlyName = std::string(buf);
-                break;
-            }
-        }
-    }
+    std::transform(unique_name.begin(), unique_name.end(), unique_name.begin(),
+        [](unsigned char c) { return std::tolower(c); }
+    );
 
-    if (pspDevInfoData) {
-        HeapFree(GetProcessHeap(), 0, pspDevInfoData);
-    }
-    SetupDiDestroyDeviceInfoList(hDevInfo);
+    auto pos = unique_name.find_last_of("#");
+    unique_name = unique_name.substr(0, pos);
 
-    return friendlyName;
+    return unique_name;
 }
 
 bool
@@ -207,15 +159,19 @@ VideoDeviceMonitorImpl::WinProcCallback(HWND hWnd, UINT message, WPARAM wParam,
         case DBT_DEVICEREMOVECOMPLETE:
         case DBT_DEVICEARRIVAL:
         {
-            PDEV_BROADCAST_DEVICEINTERFACE p = (PDEV_BROADCAST_DEVICEINTERFACE)lParam;
-            auto friendlyName = getDeviceFriendlyName(p);
-            if (!friendlyName.empty()) {
-                JAMI_DBG() << friendlyName << ((wParam == DBT_DEVICEARRIVAL) ? " plugged" : " unplugged");
+            PDEV_BROADCAST_DEVICEINTERFACE pbdi = (PDEV_BROADCAST_DEVICEINTERFACE)lParam;
+            auto unique_name = getDeviceUniqueName(pbdi);
+            if (!unique_name.empty()) {
+                JAMI_DBG() << unique_name << ((wParam == DBT_DEVICEARRIVAL) ? " plugged" : " unplugged");
                 if (pThis = reinterpret_cast<VideoDeviceMonitorImpl*>(GetWindowLongPtr(hWnd, GWLP_USERDATA))) {
                     if (wParam == DBT_DEVICEARRIVAL) {
-                        pThis->monitor_->addDevice(friendlyName);
+                        auto captureDeviceList = pThis->enumerateVideoInputDevices();
+                        for (auto node : captureDeviceList) {
+                            if (node.find(unique_name) != std::string::npos)
+                                pThis->monitor_->addDevice(node);
+                        }
                     } else if (wParam == DBT_DEVICEREMOVECOMPLETE) {
-                        pThis->monitor_->removeDevice(friendlyName);
+                        pThis->monitor_->removeDevice(unique_name);
                     }
                 }
             }
@@ -288,7 +244,8 @@ VideoDeviceMonitorImpl::enumerateVideoInputDevices()
     }
 
     IEnumMoniker *pEnum = nullptr;
-    hr = pDevEnum->CreateClassEnumerator(CLSID_VideoInputDeviceCategory, &pEnum, 0);
+    hr = pDevEnum->CreateClassEnumerator(
+        CLSID_VideoInputDeviceCategory, &pEnum, 0);
     if (hr == S_FALSE) {
         hr = VFW_E_NOT_FOUND;
     }
@@ -299,7 +256,6 @@ VideoDeviceMonitorImpl::enumerateVideoInputDevices()
     }
 
     IMoniker *pMoniker = NULL;
-    unsigned deviceID = 0;
     while (pEnum->Next(1, &pMoniker, NULL) == S_OK) {
         IPropertyBag *pPropBag;
         HRESULT hr = pMoniker->BindToStorage(0, 0, IID_PPV_ARGS(&pPropBag));
@@ -308,27 +264,25 @@ VideoDeviceMonitorImpl::enumerateVideoInputDevices()
             continue;
         }
 
-        VARIANT var;
-        VariantInit(&var);
-        hr = pPropBag->Read(L"Description", &var, 0);
-        if (FAILED(hr)) {
-            hr = pPropBag->Read(L"FriendlyName", &var, 0);
-        }
+        IBindCtx *bind_ctx = NULL;
+        LPOLESTR olestr = NULL;
 
-        if (SUCCEEDED(hr)) {
-            auto deviceName = bstrToStdString(var.bstrVal);
-            if (!deviceName.empty()) {
-                deviceList.push_back(deviceName);
-            }
-            VariantClear(&var);
+        hr = CreateBindCtx(0, &bind_ctx);
+        if (hr != S_OK) {
+            continue;
+        }
+        hr = pMoniker->GetDisplayName(bind_ctx, NULL, &olestr);
+        if (hr != S_OK) {
+            continue;
+        }
+        auto unique_name = to_string(olestr);
+        if (!unique_name.empty()) {
+            // replace ':' with '_' since ffmpeg uses : to delineate between sources
+            std::replace(unique_name.begin(), unique_name.end(), ':', '_');
+            deviceList.push_back(std::string("video=") + unique_name);
         }
-
-        hr = pPropBag->Write(L"FriendlyName", &var);
 
         pPropBag->Release();
-        pMoniker->Release();
-
-        deviceID++;
     }
     pEnum->Release();
 
diff --git a/src/string_utils.cpp b/src/string_utils.cpp
index adadbae163..cd872a82d6 100644
--- a/src/string_utils.cpp
+++ b/src/string_utils.cpp
@@ -35,7 +35,6 @@
 namespace jami {
 
 #ifdef _WIN32
-
 std::wstring
 to_wstring(const std::string& str, int codePage)
 {
@@ -65,17 +64,6 @@ to_string(const std::wstring& wstr, int codePage)
     }
     return result;
 }
-
-std::string
-bstrToStdString(BSTR bstr)
-{
-    int wslen = ::SysStringLen(bstr);
-    if (wslen != 0) {
-        std::wstring wstr(bstr, wslen);
-        return std::string(wstr.begin(), wstr.end());
-    }
-    return {};
-}
 #endif
 
 std::string
diff --git a/src/string_utils.h b/src/string_utils.h
index 50f8b9097e..593276a702 100644
--- a/src/string_utils.h
+++ b/src/string_utils.h
@@ -43,7 +43,6 @@ std::string to_string(double value);
 #ifdef _WIN32
 std::wstring to_wstring(const std::string& str, int codePage = CP_UTF8);
 std::string to_string(const std::wstring& wstr, int codePage = CP_UTF8);
-std::string bstrToStdString(BSTR bstr);
 #endif
 
 static inline int
-- 
GitLab