Since bug 162147 was fixed our RealtimeMediaSourceCenter is broken because the media devices are enumerated in the instance running in the UI process. The instance running in the Web process has an empty sourceMap hashmap. I think that to solve this we should move the enumeration process in a CaptureDeviceManager implementation, similar to what the Mac port does. We will also likely need a synchronous OWR API to list devices. The API we have for this right now is asynchronous.
(In reply to comment #0) > Since bug 162147 was fixed our RealtimeMediaSourceCenter is broken because > the media devices are enumerated in the instance running in the UI process. > The instance running in the Web process has an empty sourceMap hashmap. I > think that to solve this we should move the enumeration process in a > CaptureDeviceManager implementation, similar to what the Mac port does. > > We will also likely need a synchronous OWR API to list devices. The API we > have for this right now is asynchronous. Or you can make RealtimeMediaSourceCenter::getMediaStreamDevices asynchronous, e.g. it could take a lambda: using GetDevicesHandler = std::function<void(Vector<CaptureDevice>&&)>; virtual void getMediaStreamDevices(GetDevicesHandler) = 0;
Ah yes, good idea!
Created attachment 294059 [details] Patch
Comment on attachment 294059 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=294059&action=review > Source/WebCore/platform/mediastream/openwebrtc/RealtimeMediaSourceCenterOwr.cpp:173 > + m_completionHandler(MediaStreamPrivate::create(audioSources, videoSources)); Hum here I think that either audioSources or videoSources could be NULL. Is it properly handled?
(In reply to comment #4) > Comment on attachment 294059 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=294059&action=review > > > Source/WebCore/platform/mediastream/openwebrtc/RealtimeMediaSourceCenterOwr.cpp:173 > > + m_completionHandler(MediaStreamPrivate::create(audioSources, videoSources)); > > Hum here I think that either audioSources or videoSources could be NULL. Is > it properly handled? Yes, the completion handler basically checks if they are null or not to request audio or video permissions to the user. Thanks for the review.
This patch doesn't work very well here :( Please wait before landing this. I started experimenting with a proper CaptureDeviceManager implementation. The main drawback I see so far is that we'd need to list devices once from the UI process and once from the Web process. Also I think we can "forge" device IDs using their name maybe?
Created attachment 294356 [details] Patch
Comment on attachment 294356 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=294356&action=review > Source/WebCore/platform/mediastream/openwebrtc/RealtimeMediaSourceCenterOwr.cpp:148 > +#ifndef NDEBUG Why this in a ifdef ? test sources can be useful whatever the WebKit build config.
(In reply to comment #8) > Comment on attachment 294356 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=294356&action=review > > > Source/WebCore/platform/mediastream/openwebrtc/RealtimeMediaSourceCenterOwr.cpp:148 > > +#ifndef NDEBUG > > Why this in a ifdef ? test sources can be useful whatever the WebKit build > config. Right, I was considering it a debug feature but I think you are right.
Created attachment 294365 [details] Patch
Comment on attachment 294365 [details] Patch Rejecting attachment 294365 [details] from commit-queue. Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-03', 'apply-attachment', '--no-update', '--non-interactive', 294365, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit Last 500 characters of output: 1 FAILED at 81. Hunk #2 succeeded at 147 (offset 2 lines). Hunk #3 succeeded at 176 with fuzz 1 (offset 2 lines). 1 out of 3 hunks FAILED -- saving rejects to file Source/WebCore/platform/mediastream/openwebrtc/RealtimeMediaSourceCenterOwr.cpp.rej patching file Source/WebCore/platform/mediastream/openwebrtc/RealtimeMediaSourceCenterOwr.h Failed to run "[u'/Volumes/Data/EWS/WebKit/Tools/Scripts/svn-apply', '--force', '--reviewer', u'Philippe Normand']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit Full output: http://webkit-queues.webkit.org/results/2497039
The patch is not valid anymore after r208559, the API of the ValidConstraintsHandler does not use RealtimeMediaSource anymore. This is good because we can avoid creating the dummy source objects in the UI process.
Created attachment 294489 [details] Patch
Comment on attachment 294489 [details] Patch Clearing flags on attachment: 294489 Committed r208587: <http://trac.webkit.org/changeset/208587>
All reviewed patches have been landed. Closing bug.