1197045 - Add some basic audio hardware/driver/format information to about:support

Status: RESOLVED FIXED

(Core :: Audio/Video: cubeb, defect, P2)

Description

[Matthew Gregan [:kinetik]]

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.

Comment 1

[Paul Adenot (:padenot)]

System reported minimal latency would be useful, there are at least two headsets that return an value that is wrong, on OSX.

Comment 2

[Blake Wu [:bwu][:blakewu]]

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.

Comment 3

[C.M.Chang[:chunmin]]

Could I take this bug if you're not working on this?

Comment 4

[C.M.Chang[:chunmin]]

Attachment: [WIP] part1: Add max channel counts to about:support

Comment 5

[C.M.Chang[:chunmin]]

Attachment: [WIP] part2: Add channel layout to about:support

Comment 6

[C.M.Chang[:chunmin]]

Attachment: [WIP] part3: Add sample rate to about:support

Comment 7

[C.M.Chang[:chunmin]]

Attachment: [WIP] part4: Create a media section in about:support and move related infomation to here

Comment 8

[C.M.Chang[:chunmin]]

Attachment: [WIP] part5: Show audio output devices on about:support

Comment 9

[C.M.Chang[:chunmin]]

Attachment: [WIP] part6: Show audio input devices on about:support

Comment 10

[C.M.Chang[:chunmin]]

Attachment: [screenshot] media info on about:support

Notes:
- It's better to re-arrange all the patches
- Add driver info

Comment 11

[C.M.Chang[:chunmin]]

Attachment: [WIP] part6: Show audio input devices on about:support

Correct title for input device list
- replace "output devices" by "input devices"

Comment 12

[C.M.Chang[:chunmin]]

Attachment: [screenshot] media info on about:support

Comment 13

[C.M.Chang[:chunmin]]

Attachment: [WIP] part5: Create an AudioDeviceInfo module to expose the native device information

Comment 14

[C.M.Chang[:chunmin]]

Attachment: [WIP] part6: Show input and output devices information on about:support

Comment 15

[C.M.Chang[:chunmin]]

Attachment: part1: Create an AudioDeviceInfo to expose the native device information

Comment 16

[C.M.Chang[:chunmin]]

Attachment: part2: Expose audio channels, layout, sample-rate, and native device information via DOMWindowUtils

Comment 17

[C.M.Chang[:chunmin]]

Attachment: part3: Show audio device information on about:support

Comment 18

[C.M.Chang[:chunmin]]

Attachment: Bug 1197045 - part1: Create an AudioDeviceInfo to expose the native device information;

Review commit: https://reviewboard.mozilla.org/r/154424/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/154424/

Comment 19

[C.M.Chang[:chunmin]]

Attachment: Bug 1197045 - part2: Expose audio channels, layout, sample-rate, and native device information via DOMWindowUtils;

Review commit: https://reviewboard.mozilla.org/r/154426/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/154426/

Comment 20

[C.M.Chang[:chunmin]]

Attachment: Bug 1197045 - part3: Add audio devices information to about:support;

Review commit: https://reviewboard.mozilla.org/r/154428/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/154428/

Comment 21

[Matthew Gregan [:kinetik]]

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?

Comment 22

[Matthew Gregan [:kinetik]]

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 23

[C.M.Chang[:chunmin]]

Comment on attachment 8883506 [details]
Bug 1197045 - part1: Create an AudioDeviceInfo to expose the native device information;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/154424/diff/1-2/

Comment 24

[C.M.Chang[:chunmin]]

Comment on attachment 8883507 [details]
Bug 1197045 - part2: Expose audio channels, layout, sample-rate, and native device information via DOMWindowUtils;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/154426/diff/1-2/

Comment 25

[C.M.Chang[:chunmin]]

Comment on attachment 8883508 [details]
Bug 1197045 - part3: Add audio devices information to about:support;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/154428/diff/1-2/

Comment 26

[C.M.Chang[:chunmin]]

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

Comment 27

[Matthew Gregan [:kinetik]]

Comment on attachment 8883506 [details]
Bug 1197045 - part1: Create an AudioDeviceInfo to expose the native device information;

https://reviewboard.mozilla.org/r/154424/#review160092

Comment 28

[:Felipe Gomes (needinfo for replies!)]

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.

Comment 29

[C.M.Chang[:chunmin]]

Comment on attachment 8883506 [details]
Bug 1197045 - part1: Create an AudioDeviceInfo to expose the native device information;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/154424/diff/2-3/

Comment 30

[C.M.Chang[:chunmin]]

Comment on attachment 8883507 [details]
Bug 1197045 - part2: Expose audio channels, layout, sample-rate, and native device information via DOMWindowUtils;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/154426/diff/2-3/

Comment 31

[C.M.Chang[:chunmin]]

Comment on attachment 8883508 [details]
Bug 1197045 - part3: Add audio devices information to about:support;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/154428/diff/2-3/

Comment 32

[Paul Adenot (:padenot)]

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.

Comment 33

[C.M.Chang[:chunmin]]

Comment on attachment 8883507 [details]
Bug 1197045 - part2: Expose audio channels, layout, sample-rate, and native device information via DOMWindowUtils;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/154426/diff/3-4/

Comment 34

[C.M.Chang[:chunmin]]

Comment on attachment 8883508 [details]
Bug 1197045 - part3: Add audio devices information to about:support;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/154428/diff/3-4/

Comment 35

[C.M.Chang[:chunmin]]

Comment on attachment 8883506 [details]
Bug 1197045 - part1: Create an AudioDeviceInfo to expose the native device information;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/154424/diff/3-4/

Comment 36

[C.M.Chang[:chunmin]]

Comment on attachment 8883507 [details]
Bug 1197045 - part2: Expose audio channels, layout, sample-rate, and native device information via DOMWindowUtils;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/154426/diff/4-5/

Comment 37

[C.M.Chang[:chunmin]]

Comment on attachment 8883508 [details]
Bug 1197045 - part3: Add audio devices information to about:support;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/154428/diff/4-5/

Comment 38

[Pulsebot]

Pushed by [e-mail]:
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

Comment 39

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if 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

Comment 40

[C.M.Chang[:chunmin]]

Attachment: Bug 1197045 - part4: Test case;

Review commit: https://reviewboard.mozilla.org/r/158730/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/158730/

Comment 41

[C.M.Chang[:chunmin]]

Already upload a patch to fix it.

Comment 42

[:Felipe Gomes (needinfo for replies!)]

Comment on attachment 8887817 [details]
Bug 1197045 - part4: Test case;

https://reviewboard.mozilla.org/r/158730/#review164882

Comment 43

[C.M.Chang[:chunmin]]

Comment on attachment 8883508 [details]
Bug 1197045 - part3: Add audio devices information to about:support;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/154428/diff/5-6/

Comment 44

[C.M.Chang[:chunmin]]

Comment on attachment 8887817 [details]
Bug 1197045 - part4: Test case;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/158730/diff/1-2/

Comment 45

[Pulsebot]

Pushed by [e-mail]:
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

Comment 46

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

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)

Comment 47

[C.M.Chang[:chunmin]]

Comment on attachment 8883508 [details]
Bug 1197045 - part3: Add audio devices information to about:support;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/154428/diff/6-7/

Comment 48

[C.M.Chang[:chunmin]]

Comment on attachment 8887817 [details]
Bug 1197045 - part4: Test case;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/158730/diff/2-3/

Comment 49

[C.M.Chang[:chunmin]]

Fixed: https://treeherder.mozilla.org/#/jobs?repo=try&revision=22c0429f1a56&selectedJob=117266913

Comment 50

[Pulsebot]

Pushed by [e-mail]:
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

Comment 51

[Carsten Book [:Tomcat]]

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