From 6bfb122cd1ac6f41c6983000220fdc2109aefa0f Mon Sep 17 00:00:00 2001 From: Andrew MacDonald Date: Sun, 8 Apr 2018 22:43:35 -0700 Subject: [PATCH] Add a display_id parameter to the desktopCapturer API. (#12417) * Add a screen_api_id parameter to the desktopCapturer API. When using the DirectX capturer on Windows, there was previously no way to associate desktopCapturer/getUserMedia and electron.screen API screens. This new parameter provides the association. * Fix non-Windows build. * Fix Mac. * Fix Mac harder. * JS lint * clang-format C++ code. * IWYU * display_id, Linux comment, better test * lint * Fix tests on Linux. * Add display_id documentation. --- atom/browser/api/atom_api_desktop_capturer.cc | 78 ++++++++++++++++--- atom/browser/api/atom_api_desktop_capturer.h | 11 +++ .../api/structures/desktop-capturer-source.md | 5 ++ lib/browser/desktop-capturer.js | 3 +- lib/renderer/api/desktop-capturer.js | 3 +- spec/api-desktop-capturer-spec.js | 33 +++++++- 6 files changed, 119 insertions(+), 14 deletions(-) diff --git a/atom/browser/api/atom_api_desktop_capturer.cc b/atom/browser/api/atom_api_desktop_capturer.cc index d27838ad22d..9c3042a0eca 100644 --- a/atom/browser/api/atom_api_desktop_capturer.cc +++ b/atom/browser/api/atom_api_desktop_capturer.cc @@ -4,32 +4,41 @@ #include "atom/browser/api/atom_api_desktop_capturer.h" +#include + using base::PlatformThreadRef; #include "atom/common/api/atom_api_native_image.h" #include "atom/common/native_mate_converters/gfx_converter.h" +#include "base/strings/string_number_conversions.h" #include "base/strings/utf_string_conversions.h" #include "chrome/browser/media/desktop_media_list.h" #include "content/public/browser/desktop_capture.h" #include "native_mate/dictionary.h" #include "third_party/webrtc/modules/desktop_capture/desktop_capture_options.h" #include "third_party/webrtc/modules/desktop_capture/desktop_capturer.h" +#if defined(OS_WIN) +#include "third_party/webrtc/modules/desktop_capture/win/dxgi_duplicator_controller.h" +#include "ui/display/win/display_info.h" +#endif // defined(OS_WIN) #include "atom/common/node_includes.h" namespace mate { -template<> -struct Converter { - static v8::Local ToV8(v8::Isolate* isolate, - const DesktopMediaList::Source& source) { +template <> +struct Converter { + static v8::Local ToV8( + v8::Isolate* isolate, + const atom::api::DesktopCapturer::Source& source) { mate::Dictionary dict(isolate, v8::Object::New(isolate)); - content::DesktopMediaID id = source.id; - dict.Set("name", base::UTF16ToUTF8(source.name)); + content::DesktopMediaID id = source.media_list_source.id; + dict.Set("name", base::UTF16ToUTF8(source.media_list_source.name)); dict.Set("id", id.ToString()); - dict.Set( - "thumbnail", - atom::api::NativeImage::Create(isolate, gfx::Image(source.thumbnail))); + dict.Set("thumbnail", + atom::api::NativeImage::Create( + isolate, gfx::Image(source.media_list_source.thumbnail))); + dict.Set("display_id", source.display_id); return ConvertToV8(isolate, dict); } }; @@ -52,6 +61,9 @@ void DesktopCapturer::StartHandling(bool capture_window, const gfx::Size& thumbnail_size) { webrtc::DesktopCaptureOptions options = content::CreateDesktopCaptureOptions(); +#if defined(OS_WIN) + using_directx_capturer_ = options.allow_directx_capturer(); +#endif // defined(OS_WIN) std::unique_ptr screen_capturer( capture_screen ? webrtc::DesktopCapturer::CreateScreenCapturer(options) @@ -82,7 +94,53 @@ void DesktopCapturer::OnSourceThumbnailChanged(int index) { } bool DesktopCapturer::OnRefreshFinished() { - Emit("finished", media_list_->GetSources()); + const auto media_list_sources = media_list_->GetSources(); + std::vector sources; + for (const auto& media_list_source : media_list_sources) { + sources.emplace_back( + DesktopCapturer::Source{media_list_source, std::string()}); + } + +#if defined(OS_WIN) + // Gather the same unique screen IDs used by the electron.screen API in order + // to provide an association between it and desktopCapturer/getUserMedia. + // This is only required when using the DirectX capturer, otherwise the IDs + // across the APIs already match. + if (using_directx_capturer_) { + std::vector device_names; + // Crucially, this list of device names will be in the same order as + // |media_list_sources|. + webrtc::DxgiDuplicatorController::Instance()->GetDeviceNames(&device_names); + int device_name_index = 0; + for (auto& source : sources) { + if (source.media_list_source.id.type == + content::DesktopMediaID::TYPE_SCREEN) { + const auto& device_name = device_names[device_name_index++]; + std::wstring wide_device_name; + base::UTF8ToWide(device_name.c_str(), device_name.size(), + &wide_device_name); + const int64_t device_id = + display::win::DisplayInfo::DeviceIdFromDeviceName( + wide_device_name.c_str()); + source.display_id = base::Int64ToString(device_id); + } + } + } +#elif defined(OS_MACOSX) + // On Mac, the IDs across the APIs match. + for (auto& source : sources) { + if (source.media_list_source.id.type == + content::DesktopMediaID::TYPE_SCREEN) { + source.display_id = + base::Int64ToString(source.media_list_source.id.id); + } + } +#endif // defined(OS_WIN) +// TODO(ajmacd): Add Linux support. The IDs across APIs differ but Chrome only +// supports capturing the entire desktop on Linux. Revisit this if individual +// screen support is added. + + Emit("finished", sources); return false; } diff --git a/atom/browser/api/atom_api_desktop_capturer.h b/atom/browser/api/atom_api_desktop_capturer.h index 571f08bcfc6..77058896e0b 100644 --- a/atom/browser/api/atom_api_desktop_capturer.h +++ b/atom/browser/api/atom_api_desktop_capturer.h @@ -5,6 +5,8 @@ #ifndef ATOM_BROWSER_API_ATOM_API_DESKTOP_CAPTURER_H_ #define ATOM_BROWSER_API_ATOM_API_DESKTOP_CAPTURER_H_ +#include + #include "atom/browser/api/event_emitter.h" #include "chrome/browser/media/desktop_media_list_observer.h" #include "chrome/browser/media/native_desktop_media_list.h" @@ -17,6 +19,12 @@ namespace api { class DesktopCapturer: public mate::EventEmitter, public DesktopMediaListObserver { public: + struct Source { + DesktopMediaList::Source media_list_source; + // Will be an empty string if not available. + std::string display_id; + }; + static mate::Handle Create(v8::Isolate* isolate); static void BuildPrototype(v8::Isolate* isolate, @@ -40,6 +48,9 @@ class DesktopCapturer: public mate::EventEmitter, private: std::unique_ptr media_list_; +#if defined(OS_WIN) + bool using_directx_capturer_; +#endif // defined(OS_WIN) DISALLOW_COPY_AND_ASSIGN(DesktopCapturer); }; diff --git a/docs/api/structures/desktop-capturer-source.md b/docs/api/structures/desktop-capturer-source.md index 3ebe4ce49c7..083f9e64f0a 100644 --- a/docs/api/structures/desktop-capturer-source.md +++ b/docs/api/structures/desktop-capturer-source.md @@ -12,3 +12,8 @@ `thumbnailSize` specified in the `options` passed to `desktopCapturer.getSources`. The actual size depends on the scale of the screen or window. +* `display_id` String - A unique identifier that will correspond to the `id` of + the matching [Display](display.md) returned by the [Screen API](../screen.md). + On some platforms, this is equivalent to the `XX` portion of the `id` field + above and on others it will differ. It will be an empty string if not + available. diff --git a/lib/browser/desktop-capturer.js b/lib/browser/desktop-capturer.js index 48a7a2ad691..c7fe1abf67c 100644 --- a/lib/browser/desktop-capturer.js +++ b/lib/browser/desktop-capturer.js @@ -43,7 +43,8 @@ desktopCapturer.emit = (event, name, sources) => { return { id: source.id, name: source.name, - thumbnail: source.thumbnail.toDataURL() + thumbnail: source.thumbnail.toDataURL(), + display_id: source.display_id } }) diff --git a/lib/renderer/api/desktop-capturer.js b/lib/renderer/api/desktop-capturer.js index b9320c893fc..75d3a258bf0 100644 --- a/lib/renderer/api/desktop-capturer.js +++ b/lib/renderer/api/desktop-capturer.js @@ -35,7 +35,8 @@ exports.getSources = function (options, callback) { results.push({ id: source.id, name: source.name, - thumbnail: nativeImage.createFromDataURL(source.thumbnail) + thumbnail: nativeImage.createFromDataURL(source.thumbnail), + display_id: source.display_id }) }) return results diff --git a/spec/api-desktop-capturer-spec.js b/spec/api-desktop-capturer-spec.js index de623162e59..c85220bce65 100644 --- a/spec/api-desktop-capturer-spec.js +++ b/spec/api-desktop-capturer-spec.js @@ -1,5 +1,5 @@ const assert = require('assert') -const {desktopCapturer, remote} = require('electron') +const {desktopCapturer, remote, screen} = require('electron') const isCI = remote.getGlobal('isCi') @@ -40,7 +40,7 @@ describe('desktopCapturer', () => { desktopCapturer.getSources({types: ['window', 'screen']}, callback) }) - it('responds to subsequest calls of different options', (done) => { + it('responds to subsequent calls of different options', (done) => { let callCount = 0 const callback = (error, sources) => { callCount++ @@ -51,4 +51,33 @@ describe('desktopCapturer', () => { desktopCapturer.getSources({types: ['window']}, callback) desktopCapturer.getSources({types: ['screen']}, callback) }) + + it('returns an empty display_id for window sources on Windows and Mac', (done) => { + // Linux doesn't return any window sources. + if (process.platform !== 'win32' && process.platform !== 'darwin') { + return done() + } + desktopCapturer.getSources({types: ['window']}, (error, sources) => { + assert.equal(error, null) + assert.notEqual(sources.length, 0) + sources.forEach((source) => { assert.equal(source.display_id.length, 0) }) + done() + }) + }) + + it('returns display_ids matching the Screen API on Windows and Mac', (done) => { + if (process.platform !== 'win32' && process.platform !== 'darwin') { + return done() + } + const displays = screen.getAllDisplays() + desktopCapturer.getSources({types: ['screen']}, (error, sources) => { + assert.equal(error, null) + assert.notEqual(sources.length, 0) + assert.equal(sources.length, displays.length) + for (let i = 0; i < sources.length; i++) { + assert.equal(sources[i].display_id, displays[i].id) + } + done() + }) + }) })