RTP: refactor a little bit and fix few minor bugs.

Change-Id: I063644507f26996ded462972afcb550a4528dac8

5 files changed, 54 insertions, 55 deletions

diff --git a/voip/java/android/net/rtp/AudioGroup.java b/voip/java/android/net/rtp/AudioGroup.java
index 3e7ace8..8c19062 100644
--- a/voip/java/android/net/rtp/AudioGroup.java
+++ b/voip/java/android/net/rtp/AudioGroup.java
@@ -142,34 +142,34 @@ public class AudioGroup {
     private native void nativeSetMode(int mode);
 
     // Package-private method used by AudioStream.join().
-    synchronized void add(AudioStream stream, AudioCodec codec, int dtmfType) {
+    synchronized void add(AudioStream stream) {
         if (!mStreams.containsKey(stream)) {
             try {
-                int socket = stream.dup();
+                AudioCodec codec = stream.getCodec();
                 String codecSpec = String.format("%d %s %s", codec.type,
                         codec.rtpmap, codec.fmtp);
-                nativeAdd(stream.getMode(), socket,
+                int id = nativeAdd(stream.getMode(), stream.getSocket(),
                         stream.getRemoteAddress().getHostAddress(),
-                        stream.getRemotePort(), codecSpec, dtmfType);
-                mStreams.put(stream, socket);
+                        stream.getRemotePort(), codecSpec, stream.getDtmfType());
+                mStreams.put(stream, id);
             } catch (NullPointerException e) {
                 throw new IllegalStateException(e);
             }
         }
     }
 
-    private native void nativeAdd(int mode, int socket, String remoteAddress,
+    private native int nativeAdd(int mode, int socket, String remoteAddress,
             int remotePort, String codecSpec, int dtmfType);
 
     // Package-private method used by AudioStream.join().
     synchronized void remove(AudioStream stream) {
-        Integer socket = mStreams.remove(stream);
-        if (socket != null) {
-            nativeRemove(socket);
+        Integer id = mStreams.remove(stream);
+        if (id != null) {
+            nativeRemove(id);
         }
     }
 
-    private native void nativeRemove(int socket);
+    private native void nativeRemove(int id);
 
     /**
      * Sends a DTMF digit to every {@link AudioStream} in this group. Currently
@@ -192,15 +192,14 @@ public class AudioGroup {
      * Removes every {@link AudioStream} in this group.
      */
     public void clear() {
-        synchronized (this) {
-            mStreams.clear();
-            nativeRemove(-1);
+        for (AudioStream stream : getStreams()) {
+            stream.join(null);
         }
     }
 
     @Override
     protected void finalize() throws Throwable {
-        clear();
+        nativeRemove(0);
         super.finalize();
     }
 }
diff --git a/voip/java/android/net/rtp/AudioStream.java b/voip/java/android/net/rtp/AudioStream.java
index b7874f7..5cd1abc 100644
--- a/voip/java/android/net/rtp/AudioStream.java
+++ b/voip/java/android/net/rtp/AudioStream.java
@@ -94,7 +94,7 @@ public class AudioStream extends RtpStream {
                 mGroup = null;
             }
             if (group != null) {
-                group.add(this, mCodec, mDtmfType);
+                group.add(this);
                 mGroup = group;
             }
         }
diff --git a/voip/java/android/net/rtp/RtpStream.java b/voip/java/android/net/rtp/RtpStream.java
index e94ac42..b9d75cd 100644
--- a/voip/java/android/net/rtp/RtpStream.java
+++ b/voip/java/android/net/rtp/RtpStream.java
@@ -54,7 +54,7 @@ public class RtpStream {
     private int mRemotePort = -1;
     private int mMode = MODE_NORMAL;
 
-    private int mNative;
+    private int mSocket = -1;
     static {
         System.loadLibrary("rtp_jni");
     }
@@ -165,7 +165,9 @@ public class RtpStream {
         mRemotePort = port;
     }
 
-    synchronized native int dup();
+    int getSocket() {
+        return mSocket;
+    }
 
     /**
      * Releases allocated resources. The stream becomes inoperable after calling
@@ -175,13 +177,15 @@ public class RtpStream {
      * @see #isBusy()
      */
     public void release() {
-        if (isBusy()) {
-            throw new IllegalStateException("Busy");
+        synchronized (this) {
+            if (isBusy()) {
+                throw new IllegalStateException("Busy");
+            }
+            close();
         }
-        close();
     }
 
-    private synchronized native void close();
+    private native void close();
 
     @Override
     protected void finalize() throws Throwable {
diff --git a/voip/jni/rtp/AudioGroup.cpp b/voip/jni/rtp/AudioGroup.cpp
index b9bbd16..673a650 100644
--- a/voip/jni/rtp/AudioGroup.cpp
+++ b/voip/jni/rtp/AudioGroup.cpp
@@ -478,7 +478,7 @@ public:
     bool setMode(int mode);
     bool sendDtmf(int event);
     bool add(AudioStream *stream);
-    bool remove(int socket);
+    bool remove(AudioStream *stream);
     bool platformHasAec() { return mPlatformHasAec; }
 
 private:
@@ -691,20 +691,19 @@ bool AudioGroup::add(AudioStream *stream)
     return true;
 }
 
-bool AudioGroup::remove(int socket)
+bool AudioGroup::remove(AudioStream *stream)
 {
     mNetworkThread->requestExitAndWait();
 
-    for (AudioStream *stream = mChain; stream->mNext; stream = stream->mNext) {
-        AudioStream *target = stream->mNext;
-        if (target->mSocket == socket) {
-            if (epoll_ctl(mEventQueue, EPOLL_CTL_DEL, socket, NULL)) {
+    for (AudioStream *chain = mChain; chain->mNext; chain = chain->mNext) {
+        if (chain->mNext == stream) {
+            if (epoll_ctl(mEventQueue, EPOLL_CTL_DEL, stream->mSocket, NULL)) {
                 ALOGE("epoll_ctl: %s", strerror(errno));
                 return false;
             }
-            stream->mNext = target->mNext;
-            ALOGD("stream[%d] leaves group[%d]", socket, mDeviceSocket);
-            delete target;
+            chain->mNext = stream->mNext;
+            ALOGD("stream[%d] leaves group[%d]", stream->mSocket, mDeviceSocket);
+            delete stream;
             break;
         }
     }
@@ -931,7 +930,7 @@ exit:
 static jfieldID gNative;
 static jfieldID gMode;
 
-void add(JNIEnv *env, jobject thiz, jint mode,
+int add(JNIEnv *env, jobject thiz, jint mode,
     jint socket, jstring jRemoteAddress, jint remotePort,
     jstring jCodecSpec, jint dtmfType)
 {
@@ -943,16 +942,22 @@ void add(JNIEnv *env, jobject thiz, jint mode,
     sockaddr_storage remote;
     if (parse(env, jRemoteAddress, remotePort, &remote) < 0) {
         // Exception already thrown.
-        return;
+        return 0;
     }
     if (!jCodecSpec) {
         jniThrowNullPointerException(env, "codecSpec");
-        return;
+        return 0;
     }
     const char *codecSpec = env->GetStringUTFChars(jCodecSpec, NULL);
     if (!codecSpec) {
         // Exception already thrown.
-        return;
+        return 0;
+    }
+    socket = dup(socket);
+    if (socket == -1) {
+        jniThrowException(env, "java/lang/IllegalStateException",
+            "cannot get stream socket");
+        return 0;
     }
 
     // Create audio codec.
@@ -1001,7 +1006,7 @@ void add(JNIEnv *env, jobject thiz, jint mode,
 
     // Succeed.
     env->SetIntField(thiz, gNative, (int)group);
-    return;
+    return (int)stream;
 
 error:
     delete group;
@@ -1009,13 +1014,14 @@ error:
     delete codec;
     close(socket);
     env->SetIntField(thiz, gNative, 0);
+    return 0;
 }
 
-void remove(JNIEnv *env, jobject thiz, jint socket)
+void remove(JNIEnv *env, jobject thiz, jint stream)
 {
     AudioGroup *group = (AudioGroup *)env->GetIntField(thiz, gNative);
     if (group) {
-        if (socket == -1 || !group->remove(socket)) {
+        if (!stream || !group->remove((AudioStream *)stream)) {
             delete group;
             env->SetIntField(thiz, gNative, 0);
         }
@@ -1039,7 +1045,7 @@ void sendDtmf(JNIEnv *env, jobject thiz, jint event)
 }
 
 JNINativeMethod gMethods[] = {
-    {"nativeAdd", "(IILjava/lang/String;ILjava/lang/String;I)V", (void *)add},
+    {"nativeAdd", "(IILjava/lang/String;ILjava/lang/String;I)I", (void *)add},
     {"nativeRemove", "(I)V", (void *)remove},
     {"nativeSetMode", "(I)V", (void *)setMode},
     {"nativeSendDtmf", "(I)V", (void *)sendDtmf},
diff --git a/voip/jni/rtp/RtpStream.cpp b/voip/jni/rtp/RtpStream.cpp
index 6540099..bfe8e24 100644
--- a/voip/jni/rtp/RtpStream.cpp
+++ b/voip/jni/rtp/RtpStream.cpp
@@ -33,11 +33,11 @@ extern int parse(JNIEnv *env, jstring jAddress, int port, sockaddr_storage *ss);
 
 namespace {
 
-jfieldID gNative;
+jfieldID gSocket;
 
 jint create(JNIEnv *env, jobject thiz, jstring jAddress)
 {
-    env->SetIntField(thiz, gNative, -1);
+    env->SetIntField(thiz, gSocket, -1);
 
     sockaddr_storage ss;
     if (parse(env, jAddress, 0, &ss) < 0) {
@@ -58,7 +58,7 @@ jint create(JNIEnv *env, jobject thiz, jstring jAddress)
         &((sockaddr_in *)&ss)->sin_port : &((sockaddr_in6 *)&ss)->sin6_port;
     uint16_t port = ntohs(*p);
     if ((port & 1) == 0) {
-        env->SetIntField(thiz, gNative, socket);
+        env->SetIntField(thiz, gSocket, socket);
         return port;
     }
     ::close(socket);
@@ -75,7 +75,7 @@ jint create(JNIEnv *env, jobject thiz, jstring jAddress)
             *p = htons(port);
 
             if (bind(socket, (sockaddr *)&ss, sizeof(ss)) == 0) {
-                env->SetIntField(thiz, gNative, socket);
+                env->SetIntField(thiz, gSocket, socket);
                 return port;
             }
         }
@@ -86,25 +86,15 @@ jint create(JNIEnv *env, jobject thiz, jstring jAddress)
     return -1;
 }
 
-jint dup(JNIEnv *env, jobject thiz)
-{
-    int socket = ::dup(env->GetIntField(thiz, gNative));
-    if (socket == -1) {
-        jniThrowException(env, "java/lang/IllegalStateException", strerror(errno));
-    }
-    return socket;
-}
-
 void close(JNIEnv *env, jobject thiz)
 {
-    int socket = env->GetIntField(thiz, gNative);
+    int socket = env->GetIntField(thiz, gSocket);
     ::close(socket);
-    env->SetIntField(thiz, gNative, -1);
+    env->SetIntField(thiz, gSocket, -1);
 }
 
 JNINativeMethod gMethods[] = {
     {"create", "(Ljava/lang/String;)I", (void *)create},
-    {"dup", "()I", (void *)dup},
     {"close", "()V", (void *)close},
 };
 
@@ -114,7 +104,7 @@ int registerRtpStream(JNIEnv *env)
 {
     jclass clazz;
     if ((clazz = env->FindClass("android/net/rtp/RtpStream")) == NULL ||
-        (gNative = env->GetFieldID(clazz, "mNative", "I")) == NULL ||
+        (gSocket = env->GetFieldID(clazz, "mSocket", "I")) == NULL ||
         env->RegisterNatives(clazz, gMethods, NELEM(gMethods)) < 0) {
         ALOGE("JNI registration failed");
         return -1;