Closed
Bug 934667
Opened 11 years ago
Closed 10 years ago
Setup preferences for Selecting Loopback devices in WebRTC
Categories
(Core :: WebRTC: Audio/Video, defect)
Tracking
()
RESOLVED
FIXED
mozilla29
People
(Reporter: snandaku, Assigned: snandaku)
References
Details
Attachments
(1 file, 7 obsolete files)
5.98 KB,
patch
|
jesup
:
review+
RyanVM
:
checkin+
|
Details | Diff | Splinter Review |
While working on End-to-End audio performance framework in Talos, it was concluded having mozGetUserMedia() select loopback devices would be useful.
Comment 1•11 years ago
|
||
Just from talos or from content? With a pref or constraint?
It is from content and as a preference. I have a working code that does this is dom/content/MediManager.cpp and testing it currently
Comment 3•11 years ago
|
||
We discussed this on IRC, I think what we wanted was to be able to set a pref that would indicate what devices to use for audio/video, so that the test harness could set those prefs and then getUserMedia() would simply return the chosen devices.
Updated•11 years ago
|
Assignee: nobody → snandaku
Attachment #830100 -
Attachment is obsolete: true
Attachment #830100 -
Attachment is patch: false
Attachment #8336939 -
Attachment is obsolete: true
Attachment #8336939 -
Attachment is patch: false
Attachment #8336944 -
Attachment is obsolete: true
Attachment #8336944 -
Attachment is patch: false
Attachment #8336944 -
Attachment is obsolete: false
Attachment #8336944 -
Attachment is patch: true
Attachment #8336944 -
Attachment is obsolete: true
Attachment #8336959 -
Attachment is obsolete: true
Comment on attachment 8336961 [details] [diff] [review] Bug934667.patch Patch to add preference for choosing audio, video loopback devices for GUM.
Attachment #8336961 -
Flags: review?(rjesup)
Comment 10•11 years ago
|
||
Comment on attachment 8336961 [details] [diff] [review] Bug934667.patch Review of attachment 8336961 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/media/MediaManager.cpp @@ +735,5 @@ > for (uint32_t len = sources.Length(), i = 0; i < len; i++) { > +#ifdef DEBUG > + sources[i]->GetName(deviceName); > + if(media_device_name && strlen(media_device_name)) { > + if(deviceName.EqualsASCII(media_device_name)) { spaces after if strlen() > 0 or != 0 Comment why this is being done
Attachment #8336961 -
Flags: review?(rjesup) → review+
Assignee | ||
Comment 11•11 years ago
|
||
Comment on attachment 8336961 [details] [diff] [review] Bug934667.patch Review of attachment 8336961 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/media/MediaManager.cpp @@ +735,5 @@ > for (uint32_t len = sources.Length(), i = 0; i < len; i++) { > +#ifdef DEBUG > + sources[i]->GetName(deviceName); > + if(media_device_name && strlen(media_device_name)) { > + if(deviceName.EqualsASCII(media_device_name)) { This is needed in the case, when the preference is present but no value is specified ie as in empty string. I will add the spacing and upload patch for final review. Thanks Randell
Assignee | ||
Comment 12•11 years ago
|
||
Attachment #8336961 -
Attachment is obsolete: true
Assignee | ||
Comment 13•11 years ago
|
||
Comment on attachment 8343216 [details] [diff] [review] Add DEBUG only preferences for setting loopback media devices Taking R+ forward with comments incorporated
Attachment #8343216 -
Flags: review?(rjesup)
Assignee | ||
Comment 14•11 years ago
|
||
https://tbpl.mozilla.org/?tree=Try&rev=7bf594627462
Comment 15•10 years ago
|
||
Comment on attachment 8343216 [details] [diff] [review] Add DEBUG only preferences for setting loopback media devices Review of attachment 8343216 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/media/MediaManager.cpp @@ +1044,5 @@ > + // Check if the preference for using loopback devices is enabled. > + nsresult rv; > + nsCOMPtr<nsIPrefService> prefs = do_GetService("@mozilla.org/preferences-service;1", &rv); > + if (NS_SUCCEEDED(rv)) { > + nsCOMPtr<nsIPrefBranch> branch = do_QueryInterface(prefs); Just noticed: this is in a Run() with NS_ASSERTION(!NS_IsMainThread(), "Don't call on main thread"); But prefs cannot be safely access anywhere except main-thread. So you need to access them on mainthread, and pass the values to here.
Attachment #8343216 -
Flags: review?(rjesup) → review-
Assignee | ||
Comment 16•10 years ago
|
||
Attachment #8343216 -
Attachment is obsolete: true
Attachment #8344348 -
Attachment is obsolete: true
Attachment #8344348 -
Attachment is patch: false
Assignee | ||
Comment 17•10 years ago
|
||
Assignee | ||
Comment 18•10 years ago
|
||
Comment on attachment 8344349 [details] [diff] [review] Add DEBUG only preferences for setting loopback media devices Moved preference checking to main thread. Thanks Randell for catching this.
Attachment #8344349 -
Flags: review?(rjesup)
Assignee | ||
Comment 19•10 years ago
|
||
https://tbpl.mozilla.org/?tree=Try&rev=f722466c0920
Updated•10 years ago
|
Attachment #8344349 -
Flags: review?(rjesup) → review+
Assignee | ||
Comment 20•10 years ago
|
||
Comment on attachment 8344349 [details] [diff] [review] Add DEBUG only preferences for setting loopback media devices Taking r+ from Randell forward
Attachment #8344349 -
Flags: checkin?(rjesup)
Comment 21•10 years ago
|
||
Comment on attachment 8344349 [details] [diff] [review] Add DEBUG only preferences for setting loopback media devices https://hg.mozilla.org/integration/mozilla-inbound/rev/622e6c1191eb In the future, you can just use the checkin-needed bug keyword for landing. :)
Attachment #8344349 -
Flags: checkin?(rjesup) → checkin+
https://hg.mozilla.org/mozilla-central/rev/622e6c1191eb
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla29
Comment 23•10 years ago
|
||
Is there a reason you enabled this only for debug builds? Seems like this would be generally useful for automation.
You need to log in
before you can comment on or make changes to this bug.
Description
•