From 80452d70cb9efed9d891c1f36674559322a075ca Mon Sep 17 00:00:00 2001 From: "glaznev@webrtc.org" Date: Fri, 9 Jan 2015 19:34:06 +0000 Subject: [PATCH] Sync Android AppRTCDemo with internal repo. - Fixed some Lint warnings. - Switch to OPUS by default. - Add check to WebSocket connection that public methods are called on correct thread. R=jiayl@webrtc.org, wzh@webrtc.org Review URL: https://webrtc-codereview.appspot.com/36669004 git-svn-id: http://webrtc.googlecode.com/svn/trunk@8032 4adac7df-926f-26a2-2b94-8c16560cd09d --- .../appspot/apprtc/AppRTCDemoActivity.java | 14 +++-- .../appspot/apprtc/PeerConnectionClient.java | 17 +++++- .../appspot/apprtc/RoomParametersFetcher.java | 24 ++++---- .../apprtc/WebSocketChannelClient.java | 61 +++++++++++-------- .../appspot/apprtc/WebSocketRTCClient.java | 18 +++--- .../org/appspot/apprtc/util/AppRTCUtils.java | 4 +- .../apprtc/util/AsyncHttpURLConnection.java | 15 +++-- .../appspot/apprtc/util/LooperExecutor.java | 7 ++- 8 files changed, 96 insertions(+), 64 deletions(-) diff --git a/talk/examples/android/src/org/appspot/apprtc/AppRTCDemoActivity.java b/talk/examples/android/src/org/appspot/apprtc/AppRTCDemoActivity.java index 2e38cc2e38..cb12200bdc 100644 --- a/talk/examples/android/src/org/appspot/apprtc/AppRTCDemoActivity.java +++ b/talk/examples/android/src/org/appspot/apprtc/AppRTCDemoActivity.java @@ -116,6 +116,7 @@ public class AppRTCDemoActivity extends Activity Thread.setDefaultUncaughtExceptionHandler( new UnhandledExceptionHandler(this)); iceConnected = false; + signalingParameters = null; rootView = findViewById(android.R.id.content); encoderStatView = (TextView) findViewById(R.id.encoder_stat); @@ -277,6 +278,10 @@ public class AppRTCDemoActivity extends Activity pc.createPeerConnectionFactory( thisCopy, hwCodec, VideoRendererGui.getEGLContext(), thisCopy); } + if (signalingParameters != null) { + Log.w(TAG, "EGL context is ready after room connection."); + onConnectedToRoomInternal(signalingParameters); + } } }); } @@ -487,14 +492,11 @@ public class AppRTCDemoActivity extends Activity // are routed to UI thread. private void onConnectedToRoomInternal(final SignalingParameters params) { signalingParameters = params; - logAndToast("Creating peer connection..."); if (pc == null) { - // Create peer connection factory if render EGL context ready event - // has not been fired yet. - pc = new PeerConnectionClient(); - pc.createPeerConnectionFactory( - this, hwCodec, VideoRendererGui.getEGLContext(), this); + Log.w(TAG, "Room is connected, but EGL context is not ready yet."); + return; } + logAndToast("Creating peer connection..."); pc.createPeerConnection( localRender, remoteRender, signalingParameters, startBitrate); if (pc.isHDVideo()) { diff --git a/talk/examples/android/src/org/appspot/apprtc/PeerConnectionClient.java b/talk/examples/android/src/org/appspot/apprtc/PeerConnectionClient.java index 4092f6bf76..c93d8611b8 100644 --- a/talk/examples/android/src/org/appspot/apprtc/PeerConnectionClient.java +++ b/talk/examples/android/src/org/appspot/apprtc/PeerConnectionClient.java @@ -63,6 +63,7 @@ import java.util.regex.Pattern; */ public class PeerConnectionClient { private static final String TAG = "PCRTCClient"; + private static final boolean PREFER_ISAC = false; public static final String VIDEO_TRACK_ID = "ARDAMSv0"; public static final String AUDIO_TRACK_ID = "ARDAMSa0"; @@ -178,7 +179,8 @@ public class PeerConnectionClient { Context context, boolean vp8HwAcceleration, EGLContext renderEGLContext) { - Log.d(TAG, "Create peer connection factory."); + Log.d(TAG, "Create peer connection factory with EGLContext " + + renderEGLContext); isError = false; if (!PeerConnectionFactory.initializeAndroidGlobals( context, true, true, vp8HwAcceleration, renderEGLContext)) { @@ -292,6 +294,7 @@ public class PeerConnectionClient { @Override public void run() { if (pc != null && !isError) { + Log.d(TAG, "PC Create OFFER"); isInitiator = true; pc.createOffer(sdpObserver, sdpMediaConstraints); } @@ -304,6 +307,7 @@ public class PeerConnectionClient { @Override public void run() { if (pc != null && !isError) { + Log.d(TAG, "PC create ANSWER"); isInitiator = false; pc.createAnswer(sdpObserver, sdpMediaConstraints); } @@ -333,7 +337,10 @@ public class PeerConnectionClient { if (pc == null || isError) { return; } - String sdpDescription = preferISAC(sdp.description); + String sdpDescription = sdp.description; + if (PREFER_ISAC) { + sdpDescription = preferISAC(sdpDescription); + } if (startBitrate > 0) { sdpDescription = setStartBitrate(sdpDescription, startBitrate); } @@ -672,8 +679,12 @@ public class PeerConnectionClient { reportError("Multiple SDP create."); return; } + String sdpDescription = origSdp.description; + if (PREFER_ISAC) { + sdpDescription = preferISAC(sdpDescription); + } final SessionDescription sdp = new SessionDescription( - origSdp.type, preferISAC(origSdp.description)); + origSdp.type, sdpDescription); localSdp = sdp; executor.execute(new Runnable() { @Override diff --git a/talk/examples/android/src/org/appspot/apprtc/RoomParametersFetcher.java b/talk/examples/android/src/org/appspot/apprtc/RoomParametersFetcher.java index 58e8d4fdcf..127d12729b 100644 --- a/talk/examples/android/src/org/appspot/apprtc/RoomParametersFetcher.java +++ b/talk/examples/android/src/org/appspot/apprtc/RoomParametersFetcher.java @@ -82,22 +82,22 @@ public class RoomParametersFetcher { this.events = events; httpConnection = new AsyncHttpURLConnection("POST", registerUrl, null, - new AsyncHttpEvents() { - @Override - public void OnHttpError(String errorMessage) { - Log.e(TAG, "Room connection error: " + errorMessage); - events.onSignalingParametersError(errorMessage); - } + new AsyncHttpEvents() { + @Override + public void onHttpError(String errorMessage) { + Log.e(TAG, "Room connection error: " + errorMessage); + events.onSignalingParametersError(errorMessage); + } - @Override - public void OnHttpComplete(String response) { - RoomHttpResponseParse(response); - } - }); + @Override + public void onHttpComplete(String response) { + roomHttpResponseParse(response); + } + }); httpConnection.send(); } - private void RoomHttpResponseParse(String response) { + private void roomHttpResponseParse(String response) { Log.d(TAG, "Room response: " + response); try { LinkedList iceCandidates = null; diff --git a/talk/examples/android/src/org/appspot/apprtc/WebSocketChannelClient.java b/talk/examples/android/src/org/appspot/apprtc/WebSocketChannelClient.java index ff26651444..b6f4dd786f 100644 --- a/talk/examples/android/src/org/appspot/apprtc/WebSocketChannelClient.java +++ b/talk/examples/android/src/org/appspot/apprtc/WebSocketChannelClient.java @@ -26,6 +26,10 @@ */ package org.appspot.apprtc; +import org.appspot.apprtc.util.AsyncHttpURLConnection; +import org.appspot.apprtc.util.AsyncHttpURLConnection.AsyncHttpEvents; +import org.appspot.apprtc.util.LooperExecutor; + import android.util.Log; import de.tavendo.autobahn.WebSocket.WebSocketConnectionObserver; @@ -39,15 +43,12 @@ import java.net.URI; import java.net.URISyntaxException; import java.util.LinkedList; -import org.appspot.apprtc.util.AsyncHttpURLConnection; -import org.appspot.apprtc.util.AsyncHttpURLConnection.AsyncHttpEvents; -import org.appspot.apprtc.util.LooperExecutor; - /** * WebSocket client implementation. - * All public methods should be called from a looper executor thread - * passed in constructor. - * All events are issued on the same thread. + * + *

All public methods should be called from a looper executor thread + * passed in a constructor, otherwise exception will be thrown. + * All events are dispatched on the same thread. */ public class WebSocketChannelClient { @@ -66,10 +67,10 @@ public class WebSocketChannelClient { private boolean closeEvent; // WebSocket send queue. Messages are added to the queue when WebSocket // client is not registered and are consumed in register() call. - private LinkedList wsSendQueue; + private final LinkedList wsSendQueue; /** - * WebSocketConnectionState is the names of possible WS connection states. + * Possible WebSocket connection states. */ public enum WebSocketConnectionState { NEW, CONNECTED, REGISTERED, CLOSED, ERROR @@ -77,7 +78,7 @@ public class WebSocketChannelClient { /** * Callback interface for messages delivered on WebSocket. - * All events are invoked from UI thread. + * All events are dispatched from a looper executor thread. */ public interface WebSocketChannelEvents { public void onWebSocketOpen(); @@ -102,6 +103,7 @@ public class WebSocketChannelClient { public void connect(final String wsUrl, final String postUrl, final String roomID, final String clientID) { + checkIfCalledOnValidThread(); if (state != WebSocketConnectionState.NEW) { Log.e(TAG, "WebSocket is already connected."); return; @@ -125,6 +127,7 @@ public class WebSocketChannelClient { } public void register() { + checkIfCalledOnValidThread(); if (state != WebSocketConnectionState.CONNECTED) { Log.w(TAG, "WebSocket register() in state " + state); return; @@ -138,28 +141,25 @@ public class WebSocketChannelClient { ws.sendTextMessage(json.toString()); state = WebSocketConnectionState.REGISTERED; // Send any previously accumulated messages. - synchronized (wsSendQueue) { - for (String sendMessage : wsSendQueue) { - send(sendMessage); - } - wsSendQueue.clear(); + for (String sendMessage : wsSendQueue) { + send(sendMessage); } + wsSendQueue.clear(); } catch (JSONException e) { reportError("WebSocket register JSON error: " + e.getMessage()); } } public void send(String message) { + checkIfCalledOnValidThread(); switch (state) { case NEW: case CONNECTED: // Store outgoing messages and send them after websocket client // is registered. Log.d(TAG, "WS ACC: " + message); - synchronized (wsSendQueue) { - wsSendQueue.add(message); - return; - } + wsSendQueue.add(message); + return; case ERROR: case CLOSED: Log.e(TAG, "WebSocket send() in error or closed state : " + message); @@ -181,15 +181,14 @@ public class WebSocketChannelClient { } // This call can be used to send WebSocket messages before WebSocket - // connection is opened. However for now this way of sending messages - // is not used until possible race condition of arriving ice candidates - // send through websocket before SDP answer sent through http post will be - // resolved. + // connection is opened. public void post(String message) { + checkIfCalledOnValidThread(); sendWSSMessage("POST", message); } public void disconnect(boolean waitForComplete) { + checkIfCalledOnValidThread(); Log.d(TAG, "Disonnect WebSocket. State: " + state); if (state == WebSocketConnectionState.REGISTERED) { send("{\"type\": \"bye\"}"); @@ -209,9 +208,10 @@ public class WebSocketChannelClient { // sending any pending messages to deleted looper thread. if (waitForComplete) { synchronized (closeEventLock) { - if (!closeEvent) { + while (!closeEvent) { try { closeEventLock.wait(CLOSE_TIMEOUT); + break; } catch (InterruptedException e) { Log.e(TAG, "Wait error: " + e.toString()); } @@ -242,17 +242,26 @@ public class WebSocketChannelClient { AsyncHttpURLConnection httpConnection = new AsyncHttpURLConnection( method, postUrl, message, new AsyncHttpEvents() { @Override - public void OnHttpError(String errorMessage) { + public void onHttpError(String errorMessage) { reportError("WS " + method + " error: " + errorMessage); } @Override - public void OnHttpComplete(String response) { + public void onHttpComplete(String response) { } }); httpConnection.send(); } + // Helper method for debugging purposes. Ensures that WebSocket method is + // called on a looper thread. + private void checkIfCalledOnValidThread() { + if (!executor.checkOnLooperThread()) { + throw new IllegalStateException( + "WebSocket method is not called on valid thread"); + } + } + private class WebSocketObserver implements WebSocketConnectionObserver { @Override public void onOpen() { diff --git a/talk/examples/android/src/org/appspot/apprtc/WebSocketRTCClient.java b/talk/examples/android/src/org/appspot/apprtc/WebSocketRTCClient.java index 3d3cdf67b0..aa886b6455 100644 --- a/talk/examples/android/src/org/appspot/apprtc/WebSocketRTCClient.java +++ b/talk/examples/android/src/org/appspot/apprtc/WebSocketRTCClient.java @@ -26,14 +26,15 @@ */ package org.appspot.apprtc; -import android.util.Log; - -import org.appspot.apprtc.util.AsyncHttpURLConnection; -import org.appspot.apprtc.util.AsyncHttpURLConnection.AsyncHttpEvents; -import org.appspot.apprtc.util.LooperExecutor; import org.appspot.apprtc.RoomParametersFetcher.RoomParametersFetcherEvents; import org.appspot.apprtc.WebSocketChannelClient.WebSocketChannelEvents; import org.appspot.apprtc.WebSocketChannelClient.WebSocketConnectionState; +import org.appspot.apprtc.util.AsyncHttpURLConnection; +import org.appspot.apprtc.util.AsyncHttpURLConnection.AsyncHttpEvents; +import org.appspot.apprtc.util.LooperExecutor; + +import android.util.Log; + import org.json.JSONException; import org.json.JSONObject; import org.webrtc.IceCandidate; @@ -64,7 +65,6 @@ public class WebSocketRTCClient implements AppRTCClient, private boolean initiator; private SignalingEvents events; private WebSocketChannelClient wsClient; - private RoomParametersFetcher fetcher; private ConnectionState roomState; private String postMessageUrl; private String byeMessageUrl; @@ -109,7 +109,7 @@ public class WebSocketRTCClient implements AppRTCClient, // Create WebSocket client. wsClient = new WebSocketChannelClient(executor, this); // Get room parameters. - fetcher = new RoomParametersFetcher(loopback, url, + new RoomParametersFetcher(loopback, url, new RoomParametersFetcherEvents() { @Override public void onSignalingParametersReady( @@ -371,12 +371,12 @@ public class WebSocketRTCClient implements AppRTCClient, AsyncHttpURLConnection httpConnection = new AsyncHttpURLConnection( "POST", url, message, new AsyncHttpEvents() { @Override - public void OnHttpError(String errorMessage) { + public void onHttpError(String errorMessage) { reportError("GAE POST error: " + errorMessage); } @Override - public void OnHttpComplete(String response) { + public void onHttpComplete(String response) { if (messageType == MessageType.MESSAGE) { try { JSONObject roomJson = new JSONObject(response); diff --git a/talk/examples/android/src/org/appspot/apprtc/util/AppRTCUtils.java b/talk/examples/android/src/org/appspot/apprtc/util/AppRTCUtils.java index 51896ba226..cb6d3b1b94 100644 --- a/talk/examples/android/src/org/appspot/apprtc/util/AppRTCUtils.java +++ b/talk/examples/android/src/org/appspot/apprtc/util/AppRTCUtils.java @@ -29,8 +29,10 @@ package org.appspot.apprtc.util; import android.os.Build; import android.util.Log; -import java.lang.Thread; +/** + * AppRTCUtils provides helper functions for managing thread safety. + */ public final class AppRTCUtils { private AppRTCUtils() { diff --git a/talk/examples/android/src/org/appspot/apprtc/util/AsyncHttpURLConnection.java b/talk/examples/android/src/org/appspot/apprtc/util/AsyncHttpURLConnection.java index 784ff30697..506b76fd04 100644 --- a/talk/examples/android/src/org/appspot/apprtc/util/AsyncHttpURLConnection.java +++ b/talk/examples/android/src/org/appspot/apprtc/util/AsyncHttpURLConnection.java @@ -45,9 +45,12 @@ public class AsyncHttpURLConnection { private final String message; private final AsyncHttpEvents events; + /** + * Http requests callbacks. + */ public interface AsyncHttpEvents { - public void OnHttpError(String errorMessage); - public void OnHttpComplete(String response); + public void onHttpError(String errorMessage); + public void onHttpComplete(String response); } public AsyncHttpURLConnection(String method, String url, String message, @@ -99,18 +102,18 @@ public class AsyncHttpURLConnection { // Get response. int responseCode = connection.getResponseCode(); if (responseCode != 200) { - events.OnHttpError("Non-200 response to " + method + " to URL: " + events.onHttpError("Non-200 response to " + method + " to URL: " + url + " : " + connection.getHeaderField(null)); return; } InputStream responseStream = connection.getInputStream(); String response = drainStream(responseStream); responseStream.close(); - events.OnHttpComplete(response); + events.onHttpComplete(response); } catch (SocketTimeoutException e) { - events.OnHttpError("HTTP " + method + " to " + url + " timeout"); + events.onHttpError("HTTP " + method + " to " + url + " timeout"); } catch (IOException e) { - events.OnHttpError("HTTP " + method + " to " + url + " error: " + events.onHttpError("HTTP " + method + " to " + url + " error: " + e.getMessage()); } } diff --git a/talk/examples/android/src/org/appspot/apprtc/util/LooperExecutor.java b/talk/examples/android/src/org/appspot/apprtc/util/LooperExecutor.java index 50091d00b8..4a1dc13b96 100644 --- a/talk/examples/android/src/org/appspot/apprtc/util/LooperExecutor.java +++ b/talk/examples/android/src/org/appspot/apprtc/util/LooperExecutor.java @@ -82,7 +82,7 @@ public class LooperExecutor extends Thread implements Executor { return; } running = false; - handler.post( new Runnable() { + handler.post(new Runnable() { @Override public void run() { Looper.myLooper().quitSafely(); @@ -91,6 +91,11 @@ public class LooperExecutor extends Thread implements Executor { }); } + // Checks if current thread is a looper thread. + public boolean checkOnLooperThread() { + return (Thread.currentThread().getId() == threadId); + } + @Override public synchronized void execute(final Runnable runnable) { if (!running) {