-
Notifications
You must be signed in to change notification settings - Fork 115
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
Merge doohickey branch to master #175
Conversation
Fix the parameter descriptions for RTCAddTrackEvent
Merge juberti:master to doohickey branch
Updated to use MediaStreamTrack instead of MediaStream. I'm merging this to the doohickey branch, so that work can continue there.
<!-- [[OPEN ISSUE: How many <code>MediaStream</code>s are created | ||
when you receive multiple conflicting pranswers?]] --></p> | ||
</li> | ||
|
||
<li> | ||
Add <var>track</var> to the <code>MediaStream</code>(s) <var>streams</var> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The variable "streams" turns up out of the blue here and without a proper description.
Let's merge this rather soon and continue to work on in from within the document. |
We (someone, does not need to be Justin) need to fix the merge conflicts before we really review on all this. I'm having a hard time sorting out the parts that were removed. |
|
||
<p>The actual encoding and transmission of <code>MediaStreamTrack</code>s is managed through | ||
objects called <code>RTCRtpSender</code>s. Similarly, the reception and decoding of | ||
<code>MediaStreamTrack</code>s is managed through objects called <code>RTCRtpReceiver</code>s.</p> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we need more here to explain what a RtpSender / Receiver represents. I'm trying to get at the issue of how many of them do you have. Are they 1:1 with tracks, etc ...
Seems to me this is the right place to explain they are 1:1 with tracks, and the issues around the timing of when they are created
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
by all objects implementing the <code><a>RTCPeerConnection</a></code> | ||
interface. It is called any time a <code>MediaStreamTrack</code> is added | ||
by the remote peer. This will be fired only as a result of | ||
<code>setRemoteDescription</code>. ontrack happens as early as |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not sure this is right. Early media creates a track before the setRemote. In the case of a PRanswer one can get multiple tracks before after a PRanswer but before the Answer.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For PRANSWER, there is no issue - PRANSWER is a remote description. The tricky part is media before PRANSWER. We need to decide if we are going to handle this or not.
… a MediaStreamTrack
…stream as it's created
I've updated the doohickey branch with fixes for my comments |
Address comments from Cullen, and one from Adam
Section 4 is now agnostic of MediaStreamTracks, and this lines up well with the new note about when MSTs are generated.
Just uploaded a new version. I believe all comments have been addressed. I will take one more pass over this today to see if there are any remaining nits, but I think this is ready to be merged now. |
- phantomjs --ignore-ssl-errors=true --ssl-protocol=tlsv1 respec/tools/respec2html.js -e -w webrtc.html output.html | ||
- python webidl-checker/webidl-check output.html | ||
- "! (perl -T linkchecker/bin/checklink -S 0 -q -b output.html |grep \"^\")" | ||
env: WIDLPROC_PATH=$TRAVIS_BUILD_DIR/widlproc/obj/widlproc |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need to not delete this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, I think this was just a merge thing. Will fix.
Merge doohickey branch to master
No description provided.