Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow device capabilities to be discoverable via enumerateDevices #211

Merged
merged 6 commits into from
Sep 30, 2015
Merged

Allow device capabilities to be discoverable via enumerateDevices #211

merged 6 commits into from
Sep 30, 2015

Conversation

joeberkovitz
Copy link
Contributor

LC proposal documented in https://lists.w3.org/Archives/Public/public-media-capture/2015Jun/0034.html to allow applications to discover device capabilities.

… the unfiltered scenario of device enumeration.
<code>getCapabilities()</code> on the first <code><a>MediaStreamTrack</a></code> of this type in a
<code><a>MediaStream</a></code> returned by <code>getUserMedia({deviceId: <i>id</i>})</code>
where <i>id</i> is the value of the <code>deviceId</code> attribute of this <code>MediaDeviceInfo</code>.
For output devices, the capabilities describe a hypothetical MediaStreamTrack whose characteristics
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am a bit unsure about the applicability to output devices. Did we ever discuss that?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we missed this in our earlier discussion. However, there is presently no output analogue to getUserMedia() that would hypothetically supply the capabilities of an output device. Without some kind of definition similar in spirit to the above, we seem to have no way to determine the capabilities of output devices until the Audio Output API is fleshed out.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like we should delete the sentence about output devices, and declare that we'll figure that one out later.

@stefhak
Copy link
Contributor

stefhak commented Jul 28, 2015

I don't think we should add ways to determine capabilities of output devices to this spec. Let's focus on the input devices, at least for now.

@joeberkovitz
Copy link
Contributor Author

@stefhak Output devices are more central to the Web Audio API than input devices -- one can't make any sound in Web Audio without an output device.

I have a few questions: Why do you feel it's best to put this off? And when/where do you think we should address them, if not now? Would we change enumerateDevices() down the road or do you think this becomes a brand new API, only for output?

@stefhak
Copy link
Contributor

stefhak commented Jul 29, 2015

After re-reading http://w3c.github.io/mediacapture-main/ I think I was wrong in my initial reaction regarding output devices. enumerateDevices does indeed enumerate output audio devices, and adding capabilities to them does not seem like that big a deal in that perspective.

I'm not sure about the language ("hypothetical MediaStreamTrack") in some parts. Additionally, there is text now about only devices that the user has approved use of will described. Are all output devices automatically in this category?

At the editor's meeting tomorrow I will ask one of the editors to review this PR.

@joeberkovitz
Copy link
Contributor Author

Thanks. A couple of quick responses:

The language of "hypothetical MediaStreamTrack matching the output device characteristics" was included because my understanding is that MediaStreamTracks are not creatable for output devices: they are defined in terms of a source only. Perhaps there is a better way to phrase this.

The device approval policy was intended to track the same permission policy for the rest of enumerateDevices() filterable information, which currently states that once the user has granted access to any device, this filterable information is obtainable for all devices.

I continue to think that some way of requesting permission from the user to obtain info on available devices would clarify this area, and someone proposed that on the list not long ago.

@jan-ivar
Copy link
Member

To me, this begs a lot of questions:

  • navigator.mediaDevices has getUserMedia(), but no API yet for addressing outputs AFAIK, so what will these Capabilities be used for?
  • Permission: Why must I share my mic (or camera) with a sound-producing site/game for it to access info about (not even actual control-access to) speakers? That doesn't make any sense to me. Also, if I don't have a camera or microphone, how can a site gain info about my speakers/output-recording devices?
  • Constraints: Capabilities is one third of the Constrainable pattern (Capabilities/Settings/Constraints), yet there's no Constrainable API for output devices, so is this reuse non-constraints-related? Also, as @joeberkovitz points out, _MediaTrack_Capabilities, are source capabilities, not sink capabilities. Should we really reuse these for cross-purposes?

dictionary MediaTrackCapabilities {
         (double or DoubleRange) volume;
         (long or LongRange)     sampleRate;
         (long or LongRange)     sampleSize;
         sequence<boolean>       echoCancellation;
         (double or DoubleRange) latency;
         (long or LongRange)     channelCount;
         DOMString               deviceId;
         DOMString               groupId;
};

We're surely stretching definitions here.

@burnburn
Copy link
Contributor

burnburn commented Aug 6, 2015

I looked at this from an editorial/spec consistency standpoint and agree with Jan-Ivar that this crosses the line into more output info (and ultimately control) than the rest of the specification supports. Because output issues can be complex and are definitely different in some cases from input issues, all output work (with the notable exception I'll get to below) is being done in the separate Audio Output specification.
The one exception is that we do enumerate output devices. This exception only occurred because of the absolute failure of WebRTC applications where a user donned a headset and could not choose for the audio to come out of the headset when using the headset microphone.

I think going beyond this is too far and that it would be better for the Audio Output spec to add any additional info for output devices.

Then again, I personally think capabilities info in enumerateDevices is too far, but that's just my personal opinion (not as an editor).

@jan-ivar
Copy link
Member

jan-ivar commented Aug 6, 2015

@burnburn thanks for explaining the reason for the exception.

Having an Audio Output spec add this makes sense to me FWIW. It might be worth pondering whether such a spec would be likely to extend (with the partial webidl keyword) either MediaTrackCapabilities or MediaDeviceInfo. If the latter, we may want to rename info.capabilities to info.inputCapabilities now to anticipate this (assuming we're still going ahead). *

*) Because (MediaTrackCapabilities or OutputCapabilities) capabilities wont work, as two dictionaries are not distinguishable from each other in webidl today.

@alvestrand
Copy link
Contributor

The logic of how we got here is kind of a step by step process.

  • We want to list both input and output devices. Sometimes we even want to indicate groupings between them. So we create one interface to get info on them: enumerateDevices.
  • We want to be able to get capabilities of input devices without actually opening the device. So we add capabilities to the info returned by enumerateDevices.
  • For some properties (example: channel count), it's obvious how they would apply to output devices. It seems strange to need to go to a second API to get information about the properties of the output devices. Thus, it seems logical to add this too to information returned by enumerateDevices.

Each step seems logical. But if we're unhappy with the state at the end, we should think about which step is the one that doesn't hang together.

@jan-ivar
Copy link
Member

It seems to me we've forgotten to define output devices. Relying on their similarity to input devices, is the weak link in this reasoning imho.

@joeberkovitz
Copy link
Contributor Author

I would agree with @jan-ivar that there is a substantial gap in the media capture world right now regarding output devices, and that "piggybacking" a definition of output capabilities on top of input is a shaky approach.

I wouldn't have chosen this except that my understanding from the chairs has been that they want to defer discussions of how to fill this gap, until future consideration of the Audio Output API (which spec is really a kind of prototype that needs considerable revision to serve the most important use cases).

I suppose one question for the group here is whether deferral is desirable, if it leads to a device enumeration API that cannot expose a unified view of input and output devices.

@alvestrand
Copy link
Contributor

If it's "cannot expose now", it seems like an acceptable thing to me.
If it's "cannot expose ever", I'm more worried.

@joeberkovitz
Copy link
Contributor Author

I think that my current approach is to redo this change and rename the new attribute inputCapabilities, and indicate that it is always null for output devices. This avoids the issue that could potentially lead to trouble downstream, namely an assumption that the current interface definitions for capabilities would be usable across both input and output devices.

If no one objects I'll close this PR and create a new one along these lines.

@stefhak
Copy link
Contributor

stefhak commented Aug 20, 2015

@joeberkovitz to me that sounds like a sensible next step.

@alvestrand
Copy link
Contributor

Defining capabilities for output devices seems like a different question to be addressed, but the idea of exposing capabiilities in the device list seems good. If everyone agrees, we should merge this and add a new issue on output device caps.

@alvestrand
Copy link
Contributor

To @joeberkovitz ' comment: I think changing to "inputcapabilities" would be a mistake.
If we define output capabilities, they will either have the same form as for input, or not. If they have the same form, it seems strange to have a different field for them; if they don't, they can have a new field anyway.
Each device is already tagged as being an input or an output device.

@joeberkovitz
Copy link
Contributor Author

@alvestrand I'm fine with that approach, I suppose it has to be either input or output. As per our earlier discussion I believe an attempt to define output capabilities would be post-1.0 unless you want to resolve the other, larger API issues pertaining to output.

@jan-ivar
Copy link
Member

@alvestrand Extending MediaTrackCapabilities to include output capabilites sounds weird to me, if for not other reason than the name (will "media tracks" be used to represent outputs?)

So that would almost certainly leave us with info.capabilities and info.outputCapabilites, where the former is input capabilities. Are we fine with that?

@stefhak
Copy link
Contributor

stefhak commented Aug 27, 2015

@jan-ivar I think that would be fine.
@joeberkovitz could you update the PR to reflect this?

@alvestrand alvestrand assigned adam-be and stefhak and unassigned adam-be Aug 27, 2015
…from capabilities attribute as type may be different in fuure for output devices
@joeberkovitz
Copy link
Contributor Author

@stefhak @jan-ivar I've changed this to remove support for output device capabilities, and also removed strong typing from the capabilities attribute in case we want to return different, output-oriented type in this attribute in the future. I thought this would be better than having both capabilities and outputCapabilities which seems strange, although we retain the potential to do that.

If this is a problem, we could leave the strong typing in.

@stefhak
Copy link
Contributor

stefhak commented Sep 1, 2015

With the exception of the part "Future changes to this API will address the acquisition of output device capabilities." this LGTM. I don't think that kind of forward looking predictions belong in a spec for implementers, that fits more into a group planning doc or similar.

@joeberkovitz
Copy link
Contributor Author

I've removed the statement about future spec intentions, and also changed the phrase "...will always have the value null" to simply "...has the value null" (since this was also forward-looking in a sense).

@joeberkovitz
Copy link
Contributor Author

@jan-ivar I think we should be good now

@alvestrand
Copy link
Contributor

Travis thinks there's a reference error in it, however.... and it's not the same one as the one on the main branch.
Can you take a look?

@dontcallmedom
Copy link
Member

The Travis issues actually look like the same as on master, so I don't think they are blocking (I'll take a stab at fixing the issues separately)

@stefhak
Copy link
Contributor

stefhak commented Sep 25, 2015

@jan-ivar @adam-be does this look good to you?

@joeberkovitz
Copy link
Contributor Author

It looks like gh-pages might have been reset on the repo to an earlier commit by someone. After fetching, my local gh-pages was suddenly ahead of the origin. I left this situation alone.

@adam-be
Copy link
Member

adam-be commented Sep 25, 2015

The Travis CI warnings seem to be unrelated. You could do a rebase if you want to get rid of them.

@@ -2329,7 +2333,8 @@
a copy of <var>resultList</var>, and all its elements, where
the <code><a href=
"#widl-MediaDeviceInfo-label">label</a></code> member is the
empty string.</p>
empty string and the <code><a href=
"#widl-MediaDeviceInfo-capabilities">capabilities</a></code> member is <code>null</code>.</p>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't have the capabilities member (attribute) anymore. I think you can simply remove this since the wanted behaviour is already documented in the description of getCapabilites().

@joeberkovitz
Copy link
Contributor Author

Thanks @adam-be, I should have caught those references. Should be good now.

@@ -2248,6 +2248,10 @@
User Agent's available media input and output devices if enumeration
is successful.</p>

<p>Elements of this sequence which represent input devices will be of type
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use "that" with restrictive clause.

@jan-ivar
Copy link
Member

lgtm on substance. some language nits.

@joeberkovitz
Copy link
Contributor Author

Thanks -- appreciate the pointers. Changes applied.

@joeberkovitz
Copy link
Contributor Author

OK, please let me know if there's a further next step I can take -- I don't have write access and can't merge this request.

@stefhak
Copy link
Contributor

stefhak commented Sep 29, 2015

@joeberkovitz this is now in the hands of the editors (and @adam-be is assigned). Hopefully it can be merged shortly. I'm removing the "submitter action needed" label.

@adam-be
Copy link
Member

adam-be commented Sep 30, 2015

This looks quite ready to me.

@alvestrand
Copy link
Contributor

With all the positive words, I'm hitting the merge button.

alvestrand added a commit that referenced this pull request Sep 30, 2015
Allow device capabilities to be discoverable via enumerateDevices
@alvestrand alvestrand merged commit 49a0926 into w3c:master Sep 30, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants