Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(5)

Issue 29001: Revised ice peer refactor

Can't Edit
Can't Publish+Mail
Start Review
Created:
7 years, 5 months ago by ekr-webrtc
Modified:
7 years, 5 months ago
Reviewers:
acain
Visibility:
Public.

Description

Revised ice peer refactor

Patch Set 1 #

Total comments: 15
Unified diffs Side-by-side diffs Delta from patch set Stats Patch
M media/mtransport/test/ice_unittest.cpp View 7 chunks +354 lines, -354 lines 14 comments Download
A media/mtransport/test/test_ice_peer.py View 1 chunk +147 lines, -0 lines 1 comment Download

Messages

Total messages: 4
ekr-webrtc
I made a new patch that just focused on your changes. You're mostly on the ...
7 years, 5 months ago #1
ekr-webrtc
Comments here...
7 years, 5 months ago #2
acain
ACK on all other comments. Thanks for the feedback! http://firefox-codereview.appspot.com/29001/diff/1/media/mtransport/test/ice_unittest.cpp File media/mtransport/test/ice_unittest.cpp (right): http://firefox-codereview.appspot.com/29001/diff/1/media/mtransport/test/ice_unittest.cpp#newcode482 media/mtransport/test/ice_unittest.cpp:482: ...
7 years, 5 months ago #3
ekr-webrtc
7 years, 5 months ago #4
http://firefox-codereview.appspot.com/29001/diff/1/media/mtransport/test/ice_...
File media/mtransport/test/ice_unittest.cpp (right):

http://firefox-codereview.appspot.com/29001/diff/1/media/mtransport/test/ice_...
media/mtransport/test/ice_unittest.cpp:583: }
On 2013/08/21 06:24:03, acain wrote:
> On 2013/08/21 03:19:56, ekr-webrtc wrote:
> > You can remove all this stuff. We're going to really implement trickle, so
you
> > don't need to simulate it in these tests. Sorry I didn'tmention that.
> > 
> OK, I'll need more info on how trickle should work.

There will be individual messages with candidates identified for each stream
(there's a separate callback IceTestPeer will need to subscribe to)
and you just need to add them add them as it does now.

http://firefox-codereview.appspot.com/29001/diff/1/media/mtransport/test/ice_...
media/mtransport/test/ice_unittest.cpp:586: EXPECT_TRUE_WAIT(p->ice_complete(),
5000);
On 2013/08/21 06:24:03, acain wrote:
> On 2013/08/21 03:19:56, ekr-webrtc wrote:
> > This test should also send some data.
> Should both peers send some identifying 'hello' message on each stream (and
fail
> test if nothing was received)? Or should perhaps the offerer send test data on
> each stream and the answerer echos it back?

We'll actually want a mode where we send a lot of data. There's some code
floating around in transport_unittest we can steal later... Suggest one side
just echoes and the other sends arbitrary amounts and checks for response.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 905+:3555e821a5cd+