Closed
Bug 1197045
Opened 9 years ago
Closed 7 years ago
Add some basic audio hardware/driver/format information to about:support
Categories
(Core :: Audio/Video: cubeb, defect, P2)
Core
Audio/Video: cubeb
Tracking
()
RESOLVED
FIXED
mozilla56
Tracking | Status | |
---|---|---|
firefox56 | --- | fixed |
People
(Reporter: kinetik, Assigned: chunmin)
References
(Depends on 1 open bug, Blocks 2 open bugs)
Details
Attachments
(8 files, 10 obsolete files)
165.43 KB,
image/png
|
Details | |
10.57 KB,
patch
|
Details | Diff | Splinter Review | |
15.76 KB,
patch
|
Details | Diff | Splinter Review | |
15.11 KB,
patch
|
Details | Diff | Splinter Review | |
59 bytes,
text/x-review-board-request
|
kinetik
:
review+
|
Details |
59 bytes,
text/x-review-board-request
|
padenot
:
review+
|
Details |
59 bytes,
text/x-review-board-request
|
Felipe
:
review+
|
Details |
59 bytes,
text/x-review-board-request
|
Felipe
:
review+
|
Details |
To make it easier to file audio bugs, it'd be useful to add some audio-related information to about:support ala the existing graphics information. Right now we have to ask users to run dxdiag *and* manually dig information out of the advanced settings of the sound control panel. We don't (yet) support device selection, so we only need to query the default output device for now. For a first pass, the following data would be useful: - libcubeb backend being used available via cubeb_get_backend_id() - Sound hardware name and driver name and version need a new libcubeb API to query this - Default audio format (sample rate, bit depth, channel count, speaker mask) partial support in libcubeb's API to query this, would need to extend slightly CCing Paul and Jesup in case they have input or need additional info for WebAudio/WebRTC stuff.
Reporter | ||
Updated•9 years ago
|
Flags: needinfo?(ajones)
Comment 1•9 years ago
|
||
System reported minimal latency would be useful, there are at least two headsets that return an value that is wrong, on OSX.
Updated•9 years ago
|
Flags: needinfo?(ajones)
Priority: -- → P2
Updated•9 years ago
|
Component: Audio/Video: MediaStreamGraph → Audio/Video: cubeb
Comment 2•8 years ago
|
||
alwu, You should be interested in this bug! This is not a urgent bug, you can put this in your queue and come back when you are available.
Assignee: nobody → alwu
Assignee | ||
Comment 3•7 years ago
|
||
Could I take this bug if you're not working on this?
Flags: needinfo?(alwu)
Updated•7 years ago
|
Assignee: alwu → cchang
Flags: needinfo?(alwu)
Assignee | ||
Comment 4•7 years ago
|
||
Assignee | ||
Comment 5•7 years ago
|
||
Assignee | ||
Comment 6•7 years ago
|
||
Assignee | ||
Comment 7•7 years ago
|
||
Assignee | ||
Comment 8•7 years ago
|
||
Assignee | ||
Comment 9•7 years ago
|
||
Assignee | ||
Updated•7 years ago
|
Attachment #8876663 -
Attachment description: part6: Show audio input devices on about:support → [WIP] part6: Show audio input devices on about:support
Assignee | ||
Comment 10•7 years ago
|
||
Notes: - It's better to re-arrange all the patches - Add driver info
Assignee | ||
Comment 11•7 years ago
|
||
Correct title for input device list - replace "output devices" by "input devices"
Attachment #8876663 -
Attachment is obsolete: true
Assignee | ||
Comment 12•7 years ago
|
||
Attachment #8876664 -
Attachment is obsolete: true
Assignee | ||
Comment 13•7 years ago
|
||
Attachment #8876662 -
Attachment is obsolete: true
Assignee | ||
Comment 14•7 years ago
|
||
Attachment #8876972 -
Attachment is obsolete: true
Assignee | ||
Comment 15•7 years ago
|
||
Attachment #8865805 -
Attachment is obsolete: true
Attachment #8865806 -
Attachment is obsolete: true
Attachment #8876660 -
Attachment is obsolete: true
Attachment #8876661 -
Attachment is obsolete: true
Attachment #8877026 -
Attachment is obsolete: true
Attachment #8877027 -
Attachment is obsolete: true
Assignee | ||
Comment 16•7 years ago
|
||
Assignee | ||
Comment 17•7 years ago
|
||
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Reporter | ||
Comment 21•7 years ago
|
||
mozreview-review |
Comment on attachment 8883506 [details] Bug 1197045 - part1: Create an AudioDeviceInfo to expose the native device information; https://reviewboard.mozilla.org/r/154424/#review159724 ::: dom/media/AudioDeviceInfo.h:17 (Diff revision 1) > + NS_DECL_NSIAUDIODEVICEINFO > + > + explicit AudioDeviceInfo(const nsAString& aId, > + const nsAString& aName, > + const nsAString& aGroupId, > + const nsAString& aVendor, Indentation is off here. ::: dom/media/AudioDeviceInfo.h:33 (Diff revision 1) > + uint32_t aHigestLatency); > + > +private: > + virtual ~AudioDeviceInfo() = default; > + > + nsString mId; cubeb_devid is an opaque type, you can't assume it's a string (or any displayable type) and there's no point exposing it via any UI. The intended purpose of it is to uniquely select devices when creating streams via cubeb_stream_init. ::: dom/media/AudioDeviceInfo.h:47 (Diff revision 1) > + uint32_t mMaxChannels; > + uint32_t mDefaultRate; > + uint32_t mMaxRate; > + uint32_t mMinRate; > + uint32_t mLowestLatency; > + uint32_t mHigestLatency; Typo: highest But it's probably better to call this min/max latency. ::: dom/media/AudioDeviceInfo.cpp:8 (Diff revision 1) > +NS_IMPL_ISUPPORTS(AudioDeviceInfo, nsIAudioDeviceInfo) > + > +AudioDeviceInfo::AudioDeviceInfo(const nsAString& aId, > + const nsAString& aName, > + const nsAString& aGroupId, > + const nsAString& aVendor, Indentation is off here too. ::: dom/media/nsIAudioDeviceInfo.idl:14 (Diff revision 1) > + > + readonly attribute DOMString groupId; > + > + readonly attribute DOMString vendor; > + > + // type: Unknow/Input/Output Typo: unknown ::: dom/media/nsIAudioDeviceInfo.idl:20 (Diff revision 1) > + const unsigned short TYPE_UNKNOWN = 0; > + const unsigned short TYPE_INPUT = 1; > + const unsigned short TYPE_OUTPUT = 2; > + readonly attribute unsigned short type; > + > + // state: disabled/Enabled/Unplugged Inconsistent capitalization. Upper case d in disabled? And make the other comments match capitalizations style too. ::: dom/media/nsIAudioDeviceInfo.idl:42 (Diff revision 1) > + const unsigned short FMT_F32LE = 0x1000; > + const unsigned short FMT_F32BE = 0x2000; > + readonly attribute unsigned short supportedFormat; > + readonly attribute unsigned short defaultFormat; > + > + // Max number of channels: [1, 255] Where does the [1, 255] originate from?
Attachment #8883506 -
Flags: review?(kinetik) → review-
Reporter | ||
Comment 22•7 years ago
|
||
mozreview-review |
Comment on attachment 8883506 [details] Bug 1197045 - part1: Create an AudioDeviceInfo to expose the native device information; https://reviewboard.mozilla.org/r/154424/#review159730 Thanks for working on this. Generally looks good, but r- for a few small issues. Also, from the original bug comment, one item I feel is very important for supporting the audio code, and which is painful to ask users to find via other methods (e.g. looking in Device Manager, running dxdiag.exe): "- Sound hardware name and driver name and version need a new libcubeb API to query this" If this part isn't going to be handled by this patch series/in this bug, please file a new bug for this so that it stays on the radar.
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Comment 26•7 years ago
|
||
mozreview-review-reply |
Comment on attachment 8883506 [details] Bug 1197045 - part1: Create an AudioDeviceInfo to expose the native device information; https://reviewboard.mozilla.org/r/154424/#review159724 > cubeb_devid is an opaque type, you can't assume it's a string (or any displayable type) and there's no point exposing it via any UI. The intended purpose of it is to uniquely select devices when creating streams via cubeb_stream_init. OK, I'll remove the id. > Typo: highest > > But it's probably better to call this min/max latency. Thanks! > Where does the [1, 255] originate from? The limit of the channel count is 256 in cubeb_mixer: http://searchfox.org/mozilla-central/rev/5e1e8d2f244bd8c210a578ff1f65c3b720efe34e/media/libcubeb/src/cubeb_mixer.h#31
Reporter | ||
Comment 27•7 years ago
|
||
mozreview-review |
Comment on attachment 8883506 [details] Bug 1197045 - part1: Create an AudioDeviceInfo to expose the native device information; https://reviewboard.mozilla.org/r/154424/#review160092
Attachment #8883506 -
Flags: review?(kinetik) → review+
Comment 28•7 years ago
|
||
mozreview-review |
Comment on attachment 8883508 [details] Bug 1197045 - part3: Add audio devices information to about:support; https://reviewboard.mozilla.org/r/154428/#review160800 Not to call any favorites :P, but I believe the graphics section will be more often used than the media section, so I'd be prefer if you added it after the graphics one.
Attachment #8883508 -
Flags: review?(felipc) → review+
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 32•7 years ago
|
||
mozreview-review |
Comment on attachment 8883507 [details] Bug 1197045 - part2: Expose audio channels, layout, sample-rate, and native device information via DOMWindowUtils; https://reviewboard.mozilla.org/r/154426/#review159620 r+ with the comments addressed. ::: dom/media/CubebUtils.cpp:540 (Diff revision 1) > +} > + > +uint16_t ConvertCubebType(cubeb_device_type aType) > +{ > + // Store the mapping table to global space instead of stack space > + // so it won't be created and destroyed on every call. Please use a normal variable here. Putting `static` here means that this is always allocated, and uses memory all the time. We don't really care about the time complexity here. ::: dom/media/CubebUtils.cpp:553 (Diff revision 1) > + > +uint16_t ConvertCubebState(cubeb_device_state aState) > +{ > + // Store the mapping table to global space instead of stack space > + // so it won't be created and destroyed on every call. > + static uint16_t map[] = { Same.
Attachment #8883507 -
Flags: review?(padenot) → review+
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Updated•7 years ago
|
Keywords: checkin-needed
Comment 38•7 years ago
|
||
Pushed by ryanvm@gmail.com: https://hg.mozilla.org/integration/autoland/rev/57842092a282 part1: Create an AudioDeviceInfo to expose the native device information; r=kinetik https://hg.mozilla.org/integration/autoland/rev/feea60a002a7 part2: Expose audio channels, layout, sample-rate, and native device information via DOMWindowUtils; r=padenot https://hg.mozilla.org/integration/autoland/rev/78b66a4c6b7c part3: Add audio devices information to about:support; r=Felipe
Keywords: checkin-needed
Backed out for test failures like https://treeherder.mozilla.org/logviewer.html#?job_id=115501591&repo=autoland https://hg.mozilla.org/integration/autoland/rev/fdce816e6cc663aa94b7b90b5495c8b4b6696947
Flags: needinfo?(cchang)
Comment hidden (mozreview-request) |
Comment 42•7 years ago
|
||
mozreview-review |
Comment on attachment 8887817 [details] Bug 1197045 - part4: Test case; https://reviewboard.mozilla.org/r/158730/#review164882
Attachment #8887817 -
Flags: review?(felipc) → review+
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Updated•7 years ago
|
Keywords: checkin-needed
Comment 45•7 years ago
|
||
Pushed by cbook@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/a4ea1d5fcb4e part1: Create an AudioDeviceInfo to expose the native device information; r=kinetik https://hg.mozilla.org/integration/autoland/rev/cf756c62b0a6 part2: Expose audio channels, layout, sample-rate, and native device information via DOMWindowUtils; r=padenot https://hg.mozilla.org/integration/autoland/rev/fd48cc71efef part3: Add audio devices information to about:support; r=Felipe https://hg.mozilla.org/integration/autoland/rev/3af0f387a8f2 part4: Test case; r=Felipe
Keywords: checkin-needed
Comment 46•7 years ago
|
||
Backed out for eslint failure at Troubleshoot.jsm:401: 'winUtils' is assigned a value but never used: https://hg.mozilla.org/integration/autoland/rev/a5869e4529ac8d87845583c760215fa6568e9da9 https://hg.mozilla.org/integration/autoland/rev/ff34fb39ccb5084092cc47a811cfb25e383acaa1 https://hg.mozilla.org/integration/autoland/rev/40795195e9b64cc6022f5cbd978aca5d1a8ca276 https://hg.mozilla.org/integration/autoland/rev/051e50899b7adb6df22a32f2710fd60f020d1cc8 Push with failure: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=3af0f387a8f2cbaefcd744a925df6be7cab3b8af&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=runnable Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=116899846&repo=autoland > TEST-UNEXPECTED-ERROR | /home/worker/checkouts/gecko/toolkit/modules/Troubleshoot.jsm:401:9 | 'winUtils' is assigned a value but never used. (no-unused-vars)
Flags: needinfo?(cchang)
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Comment 49•7 years ago
|
||
Fixed: https://treeherder.mozilla.org/#/jobs?repo=try&revision=22c0429f1a56&selectedJob=117266913
Flags: needinfo?(cchang)
Assignee | ||
Updated•7 years ago
|
Keywords: checkin-needed
Comment 50•7 years ago
|
||
Pushed by cbook@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/1711c8455215 part1: Create an AudioDeviceInfo to expose the native device information; r=kinetik https://hg.mozilla.org/integration/autoland/rev/118833b05c41 part2: Expose audio channels, layout, sample-rate, and native device information via DOMWindowUtils; r=padenot https://hg.mozilla.org/integration/autoland/rev/062fb0855b3c part3: Add audio devices information to about:support; r=Felipe https://hg.mozilla.org/integration/autoland/rev/8ab4d7eefbf6 part4: Test case; r=Felipe
Keywords: checkin-needed
Comment 51•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/1711c8455215 https://hg.mozilla.org/mozilla-central/rev/118833b05c41 https://hg.mozilla.org/mozilla-central/rev/062fb0855b3c https://hg.mozilla.org/mozilla-central/rev/8ab4d7eefbf6
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox56:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
Assignee | ||
Updated•2 years ago
|
Blocks: about-media
You need to log in
before you can comment on or make changes to this bug.
Description
•