Android AppRTCMobile: Fix SDP video codec reordering for multiple H264 profiles

webrtc/examples/androidapp/src/org/appspot/apprtc/PeerConnectionClient.java

 /*
  *  Copyright 2014 The WebRTC Project Authors. All rights reserved.
  *
  *  Use of this source code is governed by a BSD-style license
  *  that can be found in the LICENSE file in the root of the source
  *  tree. An additional intellectual property rights grant can be found
  *  in the file PATENTS.  All contributing project authors may
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
 package org.appspot.apprtc;
 
 import android.content.Context;
 import android.os.Environment;
 import android.os.ParcelFileDescriptor;
 import android.util.Log;
 import java.io.File;
 import java.io.IOException;
 import java.nio.ByteBuffer;
+import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Collections;
 import java.util.EnumSet;
+import java.util.Iterator;
 import java.util.LinkedList;
 import java.util.List;
 import java.util.Timer;
 import java.util.TimerTask;
 import java.util.concurrent.Executors;
 import java.util.concurrent.ScheduledExecutorService;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 import org.appspot.apprtc.AppRTCClient.SignalingParameters;
 import org.webrtc.AudioSource;
@@ skipping 930 matching lines @@
           bitrateSet = "a=fmtp:" + codecRtpMap + " " + AUDIO_CODEC_PARAM_BITRATE + "="
               + (bitrateKbps * 1000);
         }
         Log.d(TAG, "Add remote SDP line: " + bitrateSet);
         newSdpDescription.append(bitrateSet).append("\r\n");
       }
     }
     return newSdpDescription.toString();
   }
 
+  // Returns the line number containing "m=audio|video", or -1 if no such line exists.

[sakal, 2017/01/27 15:05:15]

nit: Use javadoc

[magjed_webrtc, 2017/01/27 15:16:43]

Done.

+  private static int findMediaDescriptionLine(boolean isAudio, String[] sdpLines) {
+    final String mediaDescription = isAudio ? "m=audio " : "m=video ";
+    for (int i = 0; i < sdpLines.length; ++i) {
+      if (sdpLines[i].startsWith(mediaDescription)) {
+        return i;
+      }
+    }
+    return -1;
+  }
+
+  private static String joinString(
+      Iterable<? extends CharSequence> s, String delimiter, boolean delimiterAtEnd) {
+    Iterator<? extends CharSequence> iter = s.iterator();
+    if (!iter.hasNext()) {
+      return "";
+    }
+    StringBuilder buffer = new StringBuilder(iter.next());
+    while (iter.hasNext()) {
+      buffer.append(delimiter).append(iter.next());
+    }
+    if (delimiterAtEnd) {
+      buffer.append(delimiter);
+    }
+    return buffer.toString();
+  }
+
+  private static String movePayloadTypesToFront(List<String> preferredPayloadTypes, String mLine) {
+    // The format of the media description line should be: m=<media> <port> <proto> <fmt> ...
+    final List<String> origLineParts = Arrays.asList(mLine.split(" "));
+    if (origLineParts.size() <= 3) {
+      Log.e(TAG, "Wrong SDP media description format: " + mLine);
+      return null;
+    }
+    final List<String> header = origLineParts.subList(0, 3);
+    final List<String> unpreferredPayloadTypes =
+        new ArrayList<String>(origLineParts.subList(3, origLineParts.size()));
+    unpreferredPayloadTypes.removeAll(preferredPayloadTypes);
+    // Reconstruct the line with |preferredPayloadTypes| moved to the beginning of the payload
+    // types.
+    final List<String> newLineParts = new ArrayList<String>();
+    newLineParts.addAll(header);
+    newLineParts.addAll(preferredPayloadTypes);

[sakal, 2017/01/27 15:05:15]

This will add all preferred payload types regardless if they originally existed
in the string?

[magjed_webrtc, 2017/01/27 15:16:43]

The preferred payload types are expected to exist in the string. This is how we
use it from the preferCodec() function.

[sakal, 2017/01/27 15:21:44]

Ok, apparently SDP is required to use all codec types it defines.

+    newLineParts.addAll(unpreferredPayloadTypes);
+    return joinString(newLineParts, " ", false /* delimiterAtEnd */);
+  }
+
   private static String preferCodec(String sdpDescription, String codec, boolean isAudio) {
-    String[] lines = sdpDescription.split("\r\n");
-    int mLineIndex = -1;
-    String codecRtpMap = null;
+    final String[] lines = sdpDescription.split("\r\n");
+    final int mLineIndex = findMediaDescriptionLine(isAudio, lines);
+    if (mLineIndex == -1) {
+      Log.w(TAG, "No mediaDescription line, so can't prefer " + codec);
+      return sdpDescription;
+    }
+    // A list with all the payload types with name |codec|. The payload types are integers in the
+    // range 96-127, but they are stored as strings here.
+    final List<String> codecPayloadTypes = new ArrayList<String>();
     // a=rtpmap:<payload type> <encoding name>/<clock rate> [/<encoding parameters>]
-    String regex = "^a=rtpmap:(\\d+) " + codec + "(/\\d+)+[\r]?$";
-    Pattern codecPattern = Pattern.compile(regex);
-    String mediaDescription = "m=video ";
-    if (isAudio) {
-      mediaDescription = "m=audio ";
-    }
-    for (int i = 0; (i < lines.length) && (mLineIndex == -1 || codecRtpMap == null); i++) {
-      if (lines[i].startsWith(mediaDescription)) {
-        mLineIndex = i;
-        continue;
-      }
+    final Pattern codecPattern = Pattern.compile("^a=rtpmap:(\\d+) " + codec + "(/\\d+)+[\r]?$");
+    for (int i = 0; i < lines.length; ++i) {
       Matcher codecMatcher = codecPattern.matcher(lines[i]);
       if (codecMatcher.matches()) {
-        codecRtpMap = codecMatcher.group(1);
+        codecPayloadTypes.add(codecMatcher.group(1));
       }
     }
-    if (mLineIndex == -1) {
-      Log.w(TAG, "No " + mediaDescription + " line, so can't prefer " + codec);
+    if (codecPayloadTypes.isEmpty()) {
+      Log.w(TAG, "No payload types with name " + codec);
       return sdpDescription;
     }
-    if (codecRtpMap == null) {
-      Log.w(TAG, "No rtpmap for " + codec);
+
+    final String newMLine = movePayloadTypesToFront(codecPayloadTypes, lines[mLineIndex]);
+    if (newMLine == null) {
       return sdpDescription;
     }
-    Log.d(TAG, "Found " + codec + " rtpmap " + codecRtpMap + ", prefer at " + lines[mLineIndex]);
-    String[] origMLineParts = lines[mLineIndex].split(" ");
-    if (origMLineParts.length > 3) {
-      StringBuilder newMLine = new StringBuilder();
-      int origPartIndex = 0;
-      // Format is: m=<media> <port> <proto> <fmt> ...
-      newMLine.append(origMLineParts[origPartIndex++]).append(" ");
-      newMLine.append(origMLineParts[origPartIndex++]).append(" ");
-      newMLine.append(origMLineParts[origPartIndex++]).append(" ");
-      newMLine.append(codecRtpMap);
-      for (; origPartIndex < origMLineParts.length; origPartIndex++) {
-        if (!origMLineParts[origPartIndex].equals(codecRtpMap)) {
-          newMLine.append(" ").append(origMLineParts[origPartIndex]);
-        }
-      }
-      lines[mLineIndex] = newMLine.toString();
-      Log.d(TAG, "Change media description: " + lines[mLineIndex]);
-    } else {
-      Log.e(TAG, "Wrong SDP media description format: " + lines[mLineIndex]);
-    }
-    StringBuilder newSdpDescription = new StringBuilder();
-    for (String line : lines) {
-      newSdpDescription.append(line).append("\r\n");
-    }
-    return newSdpDescription.toString();
+    Log.d(TAG, "Change media description from: " + lines[mLineIndex] + " to " + newMLine);
+    lines[mLineIndex] = newMLine;
+    return joinString(Arrays.asList(lines), "\r\n", true /* delimiterAtEnd */);
   }
 
   private void drainCandidates() {
     if (queuedRemoteCandidates != null) {
       Log.d(TAG, "Add " + queuedRemoteCandidates.size() + " remote candidates");
       for (IceCandidate candidate : queuedRemoteCandidates) {
         peerConnection.addIceCandidate(candidate);
       }
       queuedRemoteCandidates = null;
     }
@@ skipping 243 matching lines @@
     public void onCreateFailure(final String error) {
       reportError("createSDP error: " + error);
     }
 
     @Override
     public void onSetFailure(final String error) {
       reportError("setSDP error: " + error);
     }
   }
 }