Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1963053002:
Direct IP connect functionality for AppRTC Android demo. (Closed)
|
Created:
4 years, 7 months ago by sakal Modified:
4 years, 7 months ago Reviewers:
magjed_webrtc CC:
webrtc-reviews_webrtc.org, tterriberry_mozilla.com Base URL:
https://chromium.googlesource.com/external/webrtc.git@master Target Ref:
refs/pending/heads/master Project:
webrtc Visibility:
Public. |
DescriptionDirect IP connect functionality for AppRTC Android demo.
This allows connecting between clients without using external servers, which is useful to OEMs if they are working in a network without internet connection. Implementation uses custom AppRTCClient that replaces WebSocketRTCClient if roomId looks like an IP. Instead of a web socket, this class uses direct TCP connection between peers as a signaling channel.
Committed: https://crrev.com/299ccdee0c08b520961025ef7945ca204e378b49
Cr-Commit-Position: refs/heads/master@{#12789}
Patch Set 1 #
Total comments: 37
Patch Set 2 : Fixes according to reviewer comments. #Patch Set 3 : Fixed one more 8 space indent to 4 space. #Patch Set 4 : Removed empty param description since they are not allowed according to the style guide. #
Total comments: 4
Patch Set 5 : Fixes according to the comments. #Patch Set 6 : Rebase #Patch Set 7 : Add test for TCPChanneClient #Patch Set 8 : Added a few missing spaces #Patch Set 9 : Reorder field modifiers #
Total comments: 10
Patch Set 10 : Fixes according to comments #Patch Set 11 : Remove unused imports #Patch Set 12 : Moved RobolectricLooperExecutor implementation to a separate class #Patch Set 13 : Made running boolean volatile #
Total comments: 1
Patch Set 14 : Typo fix #Patch Set 15 : Add Override to checkOnLooperThread #
Messages
Total messages: 27 (12 generated)
Description was changed from ========== Direct IP connect functionality for AppRTC Android demo. BUG= ========== to ========== Direct IP connect functionality for AppRTC Android demo. BUG= ==========
sakal@webrtc.org changed reviewers: + magjed@webrtc.org
Magnus, please take a look.
Looks great overall! I mostly have some nit comments. Also, you should use 4 space indent when breaking lines and make sure comments end with a dot. https://codereview.webrtc.org/1963053002/diff/1/webrtc/examples/androidapp/sr... File webrtc/examples/androidapp/src/org/appspot/apprtc/CallActivity.java (right): https://codereview.webrtc.org/1963053002/diff/1/webrtc/examples/androidapp/sr... webrtc/examples/androidapp/src/org/appspot/apprtc/CallActivity.java:244: if(loopback || !DirectRTCClient.IP_PATTERN.matcher(roomId).matches()) { nit: space between 'if' and '('. https://codereview.webrtc.org/1963053002/diff/1/webrtc/examples/androidapp/sr... File webrtc/examples/androidapp/src/org/appspot/apprtc/DirectRTCClient.java (right): https://codereview.webrtc.org/1963053002/diff/1/webrtc/examples/androidapp/sr... webrtc/examples/androidapp/src/org/appspot/apprtc/DirectRTCClient.java:2: * Copyright 2015 The WebRTC Project Authors. All rights reserved. nit: 2016 https://codereview.webrtc.org/1963053002/diff/1/webrtc/examples/androidapp/sr... webrtc/examples/androidapp/src/org/appspot/apprtc/DirectRTCClient.java:30: * connections. They are not supported. nit: Merge last two sentences to something like: "This class does not support loopback connections." https://codereview.webrtc.org/1963053002/diff/1/webrtc/examples/androidapp/sr... webrtc/examples/androidapp/src/org/appspot/apprtc/DirectRTCClient.java:38: "(" nit: 4 space indent. Same for other places as well. https://codereview.webrtc.org/1963053002/diff/1/webrtc/examples/androidapp/sr... webrtc/examples/androidapp/src/org/appspot/apprtc/DirectRTCClient.java:83: // Loopback connections should always use the WebSocketRTCClient nit: The code is very clear, so I would just remove these two comments. https://codereview.webrtc.org/1963053002/diff/1/webrtc/examples/androidapp/sr... webrtc/examples/androidapp/src/org/appspot/apprtc/DirectRTCClient.java:119: matcher.matches(); Check if matches() returns false and/or check IllegalStateException when calling mather.group() and reportError in those cases. https://codereview.webrtc.org/1963053002/diff/1/webrtc/examples/androidapp/sr... webrtc/examples/androidapp/src/org/appspot/apprtc/DirectRTCClient.java:122: String portStr = endpoint.substring(ip.length()); nit: I think it's a little bit clearer if you use matcher.group(2). I think it will return null in the case of no port number. https://codereview.webrtc.org/1963053002/diff/1/webrtc/examples/androidapp/sr... webrtc/examples/androidapp/src/org/appspot/apprtc/DirectRTCClient.java:235: new LinkedList<PeerConnection.IceServer>(), isServer, null, null, null, null, null); nit: Add comments about the null parameters, e.g. null /* clientId */. https://codereview.webrtc.org/1963053002/diff/1/webrtc/examples/androidapp/sr... webrtc/examples/androidapp/src/org/appspot/apprtc/DirectRTCClient.java:265: new LinkedList<PeerConnection.IceServer>(), false, null, null, null, sdp, null); ditto: Add comments about the null parameters, e.g. null /* clientId */. https://codereview.webrtc.org/1963053002/diff/1/webrtc/examples/androidapp/sr... File webrtc/examples/androidapp/src/org/appspot/apprtc/TCPChannelClient.java (right): https://codereview.webrtc.org/1963053002/diff/1/webrtc/examples/androidapp/sr... webrtc/examples/androidapp/src/org/appspot/apprtc/TCPChannelClient.java:192: return; break instead of return? https://codereview.webrtc.org/1963053002/diff/1/webrtc/examples/androidapp/sr... webrtc/examples/androidapp/src/org/appspot/apprtc/TCPChannelClient.java:201: final String executorMessage = message; Move declaration of |message| inside while loop and make it final, and remove |executorMessage|. https://codereview.webrtc.org/1963053002/diff/1/webrtc/examples/androidapp/sr... webrtc/examples/androidapp/src/org/appspot/apprtc/TCPChannelClient.java:231: } catch (IOException e) { Catch just rawSocket.close() so that eventListener.onTCPClose() is called regardless if rawSocket.close() fails or not? https://codereview.webrtc.org/1963053002/diff/1/webrtc/examples/androidapp/sr... webrtc/examples/androidapp/src/org/appspot/apprtc/TCPChannelClient.java:246: Log.e(TAG, "Sending data on closed socket."); You need to return here, otherwise it will crash on the next line. https://codereview.webrtc.org/1963053002/diff/1/webrtc/examples/androidapp/sr... webrtc/examples/androidapp/src/org/appspot/apprtc/TCPChannelClient.java:259: private InetAddress address; Make these variables final. https://codereview.webrtc.org/1963053002/diff/1/webrtc/examples/androidapp/sr... webrtc/examples/androidapp/src/org/appspot/apprtc/TCPChannelClient.java:303: public boolean isServer() { add @Override? https://codereview.webrtc.org/1963053002/diff/1/webrtc/examples/androidapp/sr... webrtc/examples/androidapp/src/org/appspot/apprtc/TCPChannelClient.java:309: private InetAddress address; Make these variables final. https://codereview.webrtc.org/1963053002/diff/1/webrtc/examples/androidapp/sr... webrtc/examples/androidapp/src/org/appspot/apprtc/TCPChannelClient.java:318: public Socket connect() { add @Override? https://codereview.webrtc.org/1963053002/diff/1/webrtc/examples/androidapp/sr... webrtc/examples/androidapp/src/org/appspot/apprtc/TCPChannelClient.java:329: public boolean isServer() { add @Override?
Fixed the issues. https://codereview.webrtc.org/1963053002/diff/1/webrtc/examples/androidapp/sr... File webrtc/examples/androidapp/src/org/appspot/apprtc/CallActivity.java (right): https://codereview.webrtc.org/1963053002/diff/1/webrtc/examples/androidapp/sr... webrtc/examples/androidapp/src/org/appspot/apprtc/CallActivity.java:244: if(loopback || !DirectRTCClient.IP_PATTERN.matcher(roomId).matches()) { On 2016/05/10 14:50:02, magjed_webrtc wrote: > nit: space between 'if' and '('. Done. https://codereview.webrtc.org/1963053002/diff/1/webrtc/examples/androidapp/sr... File webrtc/examples/androidapp/src/org/appspot/apprtc/DirectRTCClient.java (right): https://codereview.webrtc.org/1963053002/diff/1/webrtc/examples/androidapp/sr... webrtc/examples/androidapp/src/org/appspot/apprtc/DirectRTCClient.java:2: * Copyright 2015 The WebRTC Project Authors. All rights reserved. On 2016/05/10 14:50:02, magjed_webrtc wrote: > nit: 2016 Done. https://codereview.webrtc.org/1963053002/diff/1/webrtc/examples/androidapp/sr... webrtc/examples/androidapp/src/org/appspot/apprtc/DirectRTCClient.java:30: * connections. They are not supported. On 2016/05/10 14:50:02, magjed_webrtc wrote: > nit: Merge last two sentences to something like: "This class does not support > loopback connections." Done. https://codereview.webrtc.org/1963053002/diff/1/webrtc/examples/androidapp/sr... webrtc/examples/androidapp/src/org/appspot/apprtc/DirectRTCClient.java:38: "(" On 2016/05/10 14:50:02, magjed_webrtc wrote: > nit: 4 space indent. Same for other places as well. Done. https://codereview.webrtc.org/1963053002/diff/1/webrtc/examples/androidapp/sr... webrtc/examples/androidapp/src/org/appspot/apprtc/DirectRTCClient.java:83: // Loopback connections should always use the WebSocketRTCClient On 2016/05/10 14:50:02, magjed_webrtc wrote: > nit: The code is very clear, so I would just remove these two comments. Done. https://codereview.webrtc.org/1963053002/diff/1/webrtc/examples/androidapp/sr... webrtc/examples/androidapp/src/org/appspot/apprtc/DirectRTCClient.java:119: matcher.matches(); On 2016/05/10 14:50:02, magjed_webrtc wrote: > Check if matches() returns false and/or check IllegalStateException when calling > mather.group() and reportError in those cases. Done. https://codereview.webrtc.org/1963053002/diff/1/webrtc/examples/androidapp/sr... webrtc/examples/androidapp/src/org/appspot/apprtc/DirectRTCClient.java:122: String portStr = endpoint.substring(ip.length()); On 2016/05/10 14:50:02, magjed_webrtc wrote: > nit: I think it's a little bit clearer if you use matcher.group(2). I think it > will return null in the case of no port number. matcher.group(2) would returns some part inside the IP address depending on the format. We can use matcher.group(matcher.groupCount()) though. Done. https://codereview.webrtc.org/1963053002/diff/1/webrtc/examples/androidapp/sr... webrtc/examples/androidapp/src/org/appspot/apprtc/DirectRTCClient.java:235: new LinkedList<PeerConnection.IceServer>(), isServer, null, null, null, null, null); On 2016/05/10 14:50:02, magjed_webrtc wrote: > nit: Add comments about the null parameters, e.g. null /* clientId */. Done. https://codereview.webrtc.org/1963053002/diff/1/webrtc/examples/androidapp/sr... webrtc/examples/androidapp/src/org/appspot/apprtc/DirectRTCClient.java:265: new LinkedList<PeerConnection.IceServer>(), false, null, null, null, sdp, null); On 2016/05/10 14:50:02, magjed_webrtc wrote: > ditto: Add comments about the null parameters, e.g. null /* clientId */. Done. https://codereview.webrtc.org/1963053002/diff/1/webrtc/examples/androidapp/sr... File webrtc/examples/androidapp/src/org/appspot/apprtc/TCPChannelClient.java (right): https://codereview.webrtc.org/1963053002/diff/1/webrtc/examples/androidapp/sr... webrtc/examples/androidapp/src/org/appspot/apprtc/TCPChannelClient.java:192: return; On 2016/05/10 14:50:02, magjed_webrtc wrote: > break instead of return? Yep, you're right. https://codereview.webrtc.org/1963053002/diff/1/webrtc/examples/androidapp/sr... webrtc/examples/androidapp/src/org/appspot/apprtc/TCPChannelClient.java:201: final String executorMessage = message; On 2016/05/10 14:50:03, magjed_webrtc wrote: > Move declaration of |message| inside while loop and make it final, and remove > |executorMessage|. Done. https://codereview.webrtc.org/1963053002/diff/1/webrtc/examples/androidapp/sr... webrtc/examples/androidapp/src/org/appspot/apprtc/TCPChannelClient.java:231: } catch (IOException e) { On 2016/05/10 14:50:03, magjed_webrtc wrote: > Catch just rawSocket.close() so that eventListener.onTCPClose() is called > regardless if rawSocket.close() fails or not? This is something I thought about. eventListener.onTCPClose() will just cause the call to end with message "Remote end hung up; dropping PeerConnection". eventListener.onTCPError() causes a dialog to be shown with the error and OK ends the call. Do we want both to happen? https://codereview.webrtc.org/1963053002/diff/1/webrtc/examples/androidapp/sr... webrtc/examples/androidapp/src/org/appspot/apprtc/TCPChannelClient.java:246: Log.e(TAG, "Sending data on closed socket."); On 2016/05/10 14:50:03, magjed_webrtc wrote: > You need to return here, otherwise it will crash on the next line. Correct, fixed. Also changed this to reportError because this is a quite critical error that should probably end the call. https://codereview.webrtc.org/1963053002/diff/1/webrtc/examples/androidapp/sr... webrtc/examples/androidapp/src/org/appspot/apprtc/TCPChannelClient.java:259: private InetAddress address; On 2016/05/10 14:50:03, magjed_webrtc wrote: > Make these variables final. Done. https://codereview.webrtc.org/1963053002/diff/1/webrtc/examples/androidapp/sr... webrtc/examples/androidapp/src/org/appspot/apprtc/TCPChannelClient.java:303: public boolean isServer() { On 2016/05/10 14:50:03, magjed_webrtc wrote: > add @Override? Done. https://codereview.webrtc.org/1963053002/diff/1/webrtc/examples/androidapp/sr... webrtc/examples/androidapp/src/org/appspot/apprtc/TCPChannelClient.java:309: private InetAddress address; On 2016/05/10 14:50:02, magjed_webrtc wrote: > Make these variables final. Done. https://codereview.webrtc.org/1963053002/diff/1/webrtc/examples/androidapp/sr... webrtc/examples/androidapp/src/org/appspot/apprtc/TCPChannelClient.java:318: public Socket connect() { On 2016/05/10 14:50:03, magjed_webrtc wrote: > add @Override? Done. https://codereview.webrtc.org/1963053002/diff/1/webrtc/examples/androidapp/sr... webrtc/examples/androidapp/src/org/appspot/apprtc/TCPChannelClient.java:329: public boolean isServer() { On 2016/05/10 14:50:03, magjed_webrtc wrote: > add @Override? Done.
This is a rather big change, so please add a rationale in the CL description, e.g. this functionality is needed so that OEMs can test without a backend server. Please also briefly comment how it's implemented. I'm not an owner of these files, so you will have to add glaznev@webrtc.org as a reviewer as well. https://codereview.webrtc.org/1963053002/diff/1/webrtc/examples/androidapp/sr... File webrtc/examples/androidapp/src/org/appspot/apprtc/TCPChannelClient.java (right): https://codereview.webrtc.org/1963053002/diff/1/webrtc/examples/androidapp/sr... webrtc/examples/androidapp/src/org/appspot/apprtc/TCPChannelClient.java:231: } catch (IOException e) { On 2016/05/11 08:38:50, sakal wrote: > On 2016/05/10 14:50:03, magjed_webrtc wrote: > > Catch just rawSocket.close() so that eventListener.onTCPClose() is called > > regardless if rawSocket.close() fails or not? > > This is something I thought about. eventListener.onTCPClose() will just cause > the call to end with message "Remote end hung up; dropping PeerConnection". > eventListener.onTCPError() causes a dialog to be shown with the error and OK > ends the call. Do we want both to happen? Probably not. Then the code is good as it is. https://codereview.webrtc.org/1963053002/diff/60001/webrtc/examples/androidap... File webrtc/examples/androidapp/src/org/appspot/apprtc/DirectRTCClient.java (right): https://codereview.webrtc.org/1963053002/diff/60001/webrtc/examples/androidap... webrtc/examples/androidapp/src/org/appspot/apprtc/DirectRTCClient.java:117: if(!matcher.matches()) { nit: add space between 'if' and '(' https://codereview.webrtc.org/1963053002/diff/60001/webrtc/examples/androidap... webrtc/examples/androidapp/src/org/appspot/apprtc/DirectRTCClient.java:127: port = Integer.parseInt(portStr.substring(1)); This will never throw NumberFormatException as long as the regexp matches, right?
https://codereview.webrtc.org/1963053002/diff/60001/webrtc/examples/androidap... File webrtc/examples/androidapp/src/org/appspot/apprtc/DirectRTCClient.java (right): https://codereview.webrtc.org/1963053002/diff/60001/webrtc/examples/androidap... webrtc/examples/androidapp/src/org/appspot/apprtc/DirectRTCClient.java:117: if(!matcher.matches()) { On 2016/05/11 11:22:56, magjed_webrtc wrote: > nit: add space between 'if' and '(' Done. https://codereview.webrtc.org/1963053002/diff/60001/webrtc/examples/androidap... webrtc/examples/androidapp/src/org/appspot/apprtc/DirectRTCClient.java:127: port = Integer.parseInt(portStr.substring(1)); On 2016/05/11 11:22:56, magjed_webrtc wrote: > This will never throw NumberFormatException as long as the regexp matches, > right? It will throw a NumberFormatException if the number is too long. I added a catch for that. I also put the port number in it's own group so we can get rid of the substr.
Description was changed from ========== Direct IP connect functionality for AppRTC Android demo. BUG= ========== to ========== Direct IP connect functionality for AppRTC Android demo. This allows connecting between clients without using external servers, which is useful to OEMs if they are working in a network without internet connection. Implementation uses custom AppRTCClient that replaces WebSocketRTCClient if roomId looks like an IP. Instead of a web socket, this class uses direct TCP connection between peers as a signaling channel. BUG= ==========
sakal@webrtc.org changed reviewers: + glaznev@webrtc.org
AlexG, can you take a look at this changelist, please?
Description was changed from ========== Direct IP connect functionality for AppRTC Android demo. This allows connecting between clients without using external servers, which is useful to OEMs if they are working in a network without internet connection. Implementation uses custom AppRTCClient that replaces WebSocketRTCClient if roomId looks like an IP. Instead of a web socket, this class uses direct TCP connection between peers as a signaling channel. BUG= ========== to ========== Direct IP connect functionality for AppRTC Android demo. This allows connecting between clients without using external servers, which is useful to OEMs if they are working in a network without internet connection. Implementation uses custom AppRTCClient that replaces WebSocketRTCClient if roomId looks like an IP. Instead of a web socket, this class uses direct TCP connection between peers as a signaling channel. ==========
On 2016/05/11 12:58:50, sakal wrote: > AlexG, can you take a look at this changelist, please? Me and perkj@ were just added as owners of webrtc/examples/ so removing Alex from reviewer list.
magjed@webrtc.org changed reviewers: - glaznev@webrtc.org
This is a lot of new code so you should probably try to add a few tests. It looks relatively easy to add unittests for at least TCPChannelClient.
Patchset #7 (id:120001) has been deleted
On 2016/05/12 12:42:12, magjed_webrtc wrote: > This is a lot of new code so you should probably try to add a few tests. It > looks relatively easy to add unittests for at least TCPChannelClient. I've added a test for TCPChannelClient now.
Patchset #10 (id:200001) has been deleted
https://codereview.webrtc.org/1963053002/diff/180001/webrtc/examples/androidj... File webrtc/examples/androidjunit/src/org/appspot/apprtc/TCPChannelClientTest.java (right): https://codereview.webrtc.org/1963053002/diff/180001/webrtc/examples/androidj... webrtc/examples/androidjunit/src/org/appspot/apprtc/TCPChannelClientTest.java:56: private final Queue<Runnable> executorQueue = new ArrayDeque<Runnable>(); If you make |executorQueue| a BlockingQueue instead, I think you can remove some of the synchronized stuff and make it a bit cleaner. https://codereview.webrtc.org/1963053002/diff/180001/webrtc/examples/androidj... webrtc/examples/androidjunit/src/org/appspot/apprtc/TCPChannelClientTest.java:57: @Mock LooperExecutor executor; I don't really understand why you need to mock LooperExecutor. What happens if you don't? https://codereview.webrtc.org/1963053002/diff/180001/webrtc/examples/androidj... webrtc/examples/androidjunit/src/org/appspot/apprtc/TCPChannelClientTest.java:70: while (true) { while (!shouldStop) ? https://codereview.webrtc.org/1963053002/diff/180001/webrtc/examples/androidj... webrtc/examples/androidjunit/src/org/appspot/apprtc/TCPChannelClientTest.java:71: Runnable runnable; nit: Make |runnable| final. https://codereview.webrtc.org/1963053002/diff/180001/webrtc/examples/androidj... webrtc/examples/androidjunit/src/org/appspot/apprtc/TCPChannelClientTest.java:107: private Runnable runnable; nit: Make |runnable| final.
https://codereview.webrtc.org/1963053002/diff/180001/webrtc/examples/androidj... File webrtc/examples/androidjunit/src/org/appspot/apprtc/TCPChannelClientTest.java (right): https://codereview.webrtc.org/1963053002/diff/180001/webrtc/examples/androidj... webrtc/examples/androidjunit/src/org/appspot/apprtc/TCPChannelClientTest.java:56: private final Queue<Runnable> executorQueue = new ArrayDeque<Runnable>(); On 2016/05/17 12:54:37, magjed_webrtc wrote: > If you make |executorQueue| a BlockingQueue instead, I think you can remove some > of the synchronized stuff and make it a bit cleaner. Done. https://codereview.webrtc.org/1963053002/diff/180001/webrtc/examples/androidj... webrtc/examples/androidjunit/src/org/appspot/apprtc/TCPChannelClientTest.java:57: @Mock LooperExecutor executor; On 2016/05/17 12:54:38, magjed_webrtc wrote: > I don't really understand why you need to mock LooperExecutor. What happens if > you don't? Mocking LooperExecutor was a selection of multiple bad options. Options I explored: 1. Use native LooperExecutor: Because Robolectric Looper requires idle-method of ShadowLooper to be called in order to execute runnables in queue, this option would require me to wait some predefined amount of time for the runnables to be posted by TCPChannelClient receiver thread before calling idle. Waiting predefined amount of time in an unit test is bad. It would also require me to add public method getHandler to LooperExecutor to get access to ShadowLooper, which is bad. 2. Mock LooperExecutor but run runnables in main thread. Two possible subsolutions: a) Wait specific amount of time for the runnables to be posted. Waiting predefined amount of time in an unit test is bad. b) Wait for specific number of runnables to be posted. I feel like number of runnables posted is implementation detail, so we shouldn't rely on it in an unit test. ShadowLooper has a method called idleConstantly but it doesn't behave in the expected way. It only runs runnables immediately if they are posted from the Looper thread itself. https://codereview.webrtc.org/1963053002/diff/180001/webrtc/examples/androidj... webrtc/examples/androidjunit/src/org/appspot/apprtc/TCPChannelClientTest.java:70: while (true) { On 2016/05/17 12:54:38, magjed_webrtc wrote: > while (!shouldStop) ? Done. https://codereview.webrtc.org/1963053002/diff/180001/webrtc/examples/androidj... webrtc/examples/androidjunit/src/org/appspot/apprtc/TCPChannelClientTest.java:71: Runnable runnable; On 2016/05/17 12:54:37, magjed_webrtc wrote: > nit: Make |runnable| final. Done. https://codereview.webrtc.org/1963053002/diff/180001/webrtc/examples/androidj... webrtc/examples/androidjunit/src/org/appspot/apprtc/TCPChannelClientTest.java:107: private Runnable runnable; On 2016/05/17 12:54:38, magjed_webrtc wrote: > nit: Make |runnable| final. Done.
lgtm https://codereview.webrtc.org/1963053002/diff/280001/webrtc/examples/androidj... File webrtc/examples/androidjunit/src/org/appspot/apprtc/util/RobolectricLooperExecutor.java (right): https://codereview.webrtc.org/1963053002/diff/280001/webrtc/examples/androidj... webrtc/examples/androidjunit/src/org/appspot/apprtc/util/RobolectricLooperExecutor.java:114: public boolean checkOnLooperThread() { nit: Add @Override
The CQ bit was checked by sakal@webrtc.org
The patchset sent to the CQ was uploaded after l-g-t-m from magjed@webrtc.org Link to the patchset: https://codereview.webrtc.org/1963053002/#ps320001 (title: "Add Override to checkOnLooperThread")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1963053002/320001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1963053002/320001
Message was sent while issue was closed.
Description was changed from ========== Direct IP connect functionality for AppRTC Android demo. This allows connecting between clients without using external servers, which is useful to OEMs if they are working in a network without internet connection. Implementation uses custom AppRTCClient that replaces WebSocketRTCClient if roomId looks like an IP. Instead of a web socket, this class uses direct TCP connection between peers as a signaling channel. ========== to ========== Direct IP connect functionality for AppRTC Android demo. This allows connecting between clients without using external servers, which is useful to OEMs if they are working in a network without internet connection. Implementation uses custom AppRTCClient that replaces WebSocketRTCClient if roomId looks like an IP. Instead of a web socket, this class uses direct TCP connection between peers as a signaling channel. ==========
Message was sent while issue was closed.
Committed patchset #15 (id:320001)
Message was sent while issue was closed.
Description was changed from ========== Direct IP connect functionality for AppRTC Android demo. This allows connecting between clients without using external servers, which is useful to OEMs if they are working in a network without internet connection. Implementation uses custom AppRTCClient that replaces WebSocketRTCClient if roomId looks like an IP. Instead of a web socket, this class uses direct TCP connection between peers as a signaling channel. ========== to ========== Direct IP connect functionality for AppRTC Android demo. This allows connecting between clients without using external servers, which is useful to OEMs if they are working in a network without internet connection. Implementation uses custom AppRTCClient that replaces WebSocketRTCClient if roomId looks like an IP. Instead of a web socket, this class uses direct TCP connection between peers as a signaling channel. Committed: https://crrev.com/299ccdee0c08b520961025ef7945ca204e378b49 Cr-Commit-Position: refs/heads/master@{#12789} ==========
Message was sent while issue was closed.
Patchset 15 (id:??) landed as https://crrev.com/299ccdee0c08b520961025ef7945ca204e378b49 Cr-Commit-Position: refs/heads/master@{#12789} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
