Re: ACTION-95: Constraints usage in the WebRTC spec

On 11/26/13 12:17 PM, Justin Uberti wrote:
> On Tue, Nov 26, 2013 at 7:33 AM, Harald Alvestrand 
> <harald@alvestrand.no <mailto:harald@alvestrand.no>> wrote:
>
>     Thanks for putting together this analysis!
>
>
>     On 11/26/2013 03:33 PM, Adam Bergkvist wrote:
>
>         Constraints in the WebRTC specification.
>
>         The spec currently uses constraints in three ways:
>         1. Constructor argument
>         2. Arugment to method modifying an object (e.g. updateIce())
>         3. Arugment to method performing an action on an object (e.g.
>         addStream())
>
>         The first two usages seem to work OK. 1) is similar to
>         constructing a Constrainable object and then calling
>         applyConstraints() and 2) is pretty much applyConstraints()
>         with a different name. 3) is trickier. Even though the call is
>         made on a Constrainable object, the constraints are associated
>         with an operation made on the Constrainable object and not the
>         object itself. I think we should avoid this kind of usage.
>
>
>     To my mind, the operations in 3) should be grouped into two groups:
>
>     - Those that "really" apply the constraint to a newly created
>     object that has, up to now, been hidden. In the case of addStream,
>     I think we "found" the newly created object at our last meeting.
>
>     - Those that don't have a constrainable object (either hidden or
>     overt) to apply the constraint to. In these cases, I question the
>     value of this usage, and agree that options dictionaries are
>     likely to be better.
>
>
>         To evaluate each constraint, I've used the following two
>         questions. If the answer is yes to both of these questions
>         then a settable property should not be a constraint.
>
>         1. Is there a set of possible values that are the same for all
>         browsers?
>         2. Will this settable property succeed if applied at the
>         correct state? I.e., it shouldn't fail because a value is out
>         of the range of what the browser is capable of.
>
>         Even though the answer to question 2 may be "no", there might
>         still be a reason to use a regular settings dictionary.
>
>         # Constraints
>
>         ## IceTransports
>         Used with PeerConnection() Constructor and updateIce()
>
>         Controls which types of candidates the ICE agent is allowed to
>         use.
>
>         The possible values are "none", "relay" and "all". If a
>         browser, for some special reason, is configured to not support
>         all these values, that might be a reason to have it as a
>         constraint.
>
>         Proposal: Move to RTCConfiguration?
>
>
> I agree that RTCConfiguration seems like a better home for this 
> ICE-related property, since RTCConfiguration already holds the other 
> ICE-related properties. As a result, I think that updateIce() should 
> probably become setConfiguration() or updateConfiguration().
>
> Note though that we may still need some way to pass in global settings 
> to the PeerConnection, e.g. the identity to use for DTLS. Therefore, 
> we may still need a settings dictionary in addition to the 
> RTCConfiguration (unless we want to make RTCConfiguration the sole 
> global-settings object).

+1 on updateConfiguration() and making RTCConfiguration the sole 
global-settings object.

>         ## OfferToReceiveVideo/Audio
>         Used with createOffer()
>
>         Used to express a preference for receiving a media type (even
>         though the media line is not added as a result of addStream()).
>
>         Proposal: Move to CreateOfferOptions dictionary
>
>         ## IceRestart
>         Used with createOffer()
>
>         Force generation of new ICE credentials.
>
>         Proposal: Move to a CreateOfferOptions dictionary
>
>         ## VoiceActivityDetection
>         No usage documented.
>
>         Enables VAD, if available.
>
>         Proposal: Should be a constraint on the DooHickey object.
>
>
> As defined right now, this constraint to createOffer controls the 
> inclusion of the CN (RFC 3389) codecs in the offer, which are used 
> when doing VAD with codecs that don't have built-in CN, e.g. G.711. 
> That seems like something we may need to retain, even if we have some 
> other property on the MediaStreamTrackSender that controls whether CN 
> is *actually* used for the specified track.

In other words, proposal: Move to a CreateOfferOptions dictionary.

>         ## RequestIdentity
>         Used with PeerConnection() Constructor, createOffer() and
>         createAnswer()
>
>         Indicates whether an identity should be requested.
>
>         Proposal: Move to RTCConfiguration and
>         CreateOfferAnswerOptions dictionary
>
>         # addStream() Method
>
>         I think we should remove the MediaConstraints argument here
>         since it's an operation of type 3) mentioned above. It's also
>         a question about what precision we can have on the constraints
>         since a MediaStream can contain several tracks and it's likely
>         that the script would like to address constraints to
>         individual MediaStreamTrack objects. We're probably better off
>         with having constraints on the DooHickey object.
>
>
>     I think it would make sense to have addStream constrain the
>     DooHickey (whatever we choose to call it), in the same way that
>     getUserMedia() constrains the resulting MediaStreamTracks.
>
>
> While your logic is reasonable, I'm still not sure a constraint makes 
> sense here, especially since we don't have a great example of how this 
> should be used. (And, of course, there is the pending proposal to 
> refactor addStream into addTrack.) I would suggest omitting it, or 
> making it a dictionary, until we have a better idea regarding the usage.

+1.

.: Jan-Ivar :.