nitpicking the code and examples

I notice that we have this:

pc.onicecandidate = function(e) {
  if (e.candidate) { <<<<<
...

The fact that candidate can be undefined/null/0/' is not specified in
the definition of RTCPeerConnectionIceEvent.

Other things raised on 10.2:
No error callback set for getUserMedia.
Setting the source of the various views uses remoteView.src instead of
remoteView.objectSrc.
The control flow is not particularly illustrative.
The code should run in a browser.
onnegotiationneeded is expected to fire before starting to use the
RTCPeerConnection - *this is obviously an issue*
The example should show how streams are examined when they arrive.  If
negotiation removes video, then it might be worth looking at (this
isn't necessarily applicable to the first example).
The JSON.stringify and JSON.parse calls aren't necessary in the example.
The example needs to use braces. Always.

On 10.2:
The TODO was considered objectionable.

On 10.3:
We deferred this one.

10.4:
The startup assumes that we have an initial pool size of greater than zero.
We need to decide whether we want the above to be part of the example
call flow.  Some support for not showing this.
Step 3&8 should show "new RTCPeerConnection()"
We need to decide whether we need a simple call flow AND a "boss" call
flow with all the extra stuff.
We need to decide if we have an example that includes trickle. (View
was that every major feature in the spec needs an example that
demonstrates those features.)
This is missing getUserMedia().  We should show it, including consent.
 On both ends.
onnegotiationneeded doesn't appear after addStream.
We need to decide if the simple example includes data streams.
We need to decide if the diagrams show all the callbacks.
Step 21 is a problem: it should be onaddstream.  At least once for
each stream added on the other end.  This is where we might be able to
look at the stream and see what is arriving.
addStream(data) should be createDataChannel().
Arguments for createAnswer don't include Provisional and RelayOnly.
Provisional is in the SDP type, relay only is a constraint on the
RTCPeerConnection constructor or updateIce.  Cullen doesn't like this.
Permission (Step 28) needs some explanation.
onRemoteStream(data) is not the data channel callback, it might not
even be a real thing
Step 33 (or maybe 35) can't (according to some) continue until after
the signaling is complete.
Step 36 (which depends on 35) might not be valid.  We need to decide
whether the data stream is created in response to packets or
signaling, or something.
onRemoveStream is called/fired when you setRemoteDescription (move 47
way, way up).  Cullen might disagree.
Step 50 may be in the wrong place - we need to think about which peer
is the ICE controlling peer and get this right.

Received on Wednesday, 6 February 2013 17:37:42 UTC