- From: Adam Bergkvist <adam.bergkvist@ericsson.com>
- Date: Tue, 19 Jun 2012 17:29:42 +0200
- To: Justin Uberti <juberti@google.com>
- CC: Eric Rescorla <ekr@rtfm.com>, "public-webrtc@w3.org" <public-webrtc@w3.org>
On 2012-06-19 03:48, Justin Uberti wrote: > > > On Mon, Jun 18, 2012 at 7:53 PM, Eric Rescorla <ekr@rtfm.com > <mailto:ekr@rtfm.com>> wrote: > > On Mon, Jun 18, 2012 at 5:13 AM, Adam Bergkvist > <adam.bergkvist@ericsson.com <mailto:adam.bergkvist@ericsson.com>> > wrote: > > On 2012-06-15 21:28, Justin Uberti wrote: > >> > >> At the interim, it was indicated that using MediaConstraints for > >> non-media PeerConnection methods was probably not the right fit, one > >> reason being that most options were only relevant for a specific > method, > >> and it would be good to make it clear which options should be > passed to > >> which methods. > >> > >> Therefore I propose that we define other settings dictionaries, > similar > >> to MediaConstraints, but named specifically for the methods in which > >> they will be used. > > Generally, I like this approach. Some (hopefully easy) nits below. > > >> This results in new dictionaries IceOptions and > >> SessionDescriptionOptions, with values as shown below: > >> > >> IceOptions.AllowedCandidates = ("none", "relay", "all) // "all", > if not > >> specified > >> > >> SessionDescriptionOptions.IncludeAudio = true/false // forces > m=audio > >> line to be included > >> SessionDescriptionOptions.IncludeVideo = true/false // forces > m=video > >> line to be included > > By "forces" you mean "even if I don't include a video stream"? > That's what I'm taking from Adam's comment. > > > Correct. > > What happens if I AddStream() a video stream but set this to > false? > > > One of the following: > 1. The setting is ignored. > 2. The stream is ignored. > 3. An exception is thrown. > > > Maybe it would make more sense to have this *only* control > generating a recv-type media stream and have addstream > control only a send-type. > > > This controls adding a m= line in the offer or answer, which is needed > if we want to send or receive. The original intent was to make the > meaning of this setting to be "include a m=video line if one doesn't > already exist", which would be essentially #1 above. > > Or we could be strict and do #3. > With Ekr's comment above about letting "includeVideo" control the recv-type and addStream() control send-type, then addStream(videoStream) + "includeVideo":false could mean that I want to send video, but not receive video. The nice thing with only having addStream() control the send-type is that if the app can't generate a video stream, then there's no way for the JS app to hard-code that video should be sent anyhow which needs to fail in some way. If I get the current approach, then addStream(videoStream) obviously means that I want to send video, but also implies that I'm willing to receive video (regardless if I set "includeVideo":true or not). A bit less control than the scenario above. > > > >> SessionDescriptionOptions.UseVoiceActivityDetection = true/false // > >> includes CN codecs if true > > > >> SessionDescriptionOptions.RestartIce = true/false // generates offer > >> with new ufrag/pwd > >> SessionDescriptionOptions.GetCapabilities = true/false // generates > >> "capabilities" offer > >> > >> These fit into the existing API on the createOffer/Answer and > updateIce > >> methods: > >> > > Hmm... So I can't update my iceOptions when I do createAnswer? > If I want to convert from relay only I have to do updateIce? > > > Yes - because IceOptions affect more than the SDP - they also affect the > candidates used by the ICEAgent for checks. > > > > >> [Constructor (IceServers configuration, optional IceOptions > iceOptions)] > >> interface PeerConnection { > >> void createOffer(SessionDescriptionCallback > >> successCallback, optional PeerConnectionErrorCallback > failureCallback, > >> optional SessionDescriptionOptions options); > >> void createAnswer(SessionDescriptionCallback > >> successCallback, optional PeerConnectionErrorCallback > failureCallback, > >> optional SessionDescriptionOptions options); > >> ... > >> > >> void updateIce(optionalIceServersconfiguration, > >> optional IceOptions options); > >> > >> > >> Does this look reasonable? > > -Ekr > >
Received on Tuesday, 19 June 2012 15:30:10 UTC