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

webrtc/examples/objc/AppRTCMobile/ARDSDPUtils.m

 /*
  *  Copyright 2015 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.
  */
 
 #import "ARDSDPUtils.h"
 
 #import "WebRTC/RTCLogging.h"
 #import "WebRTC/RTCSessionDescription.h"
 
 @implementation ARDSDPUtils
 
 + (RTCSessionDescription *)
     descriptionForDescription:(RTCSessionDescription *)description
           preferredVideoCodec:(NSString *)codec {
   NSString *sdpString = description.sdp;
   NSString *lineSeparator = @"\n";
   NSString *mLineSeparator = @" ";
   // Copied from PeerConnectionClient.java.
   // TODO(tkchin): Move this to a shared C++ file.
   NSMutableArray *lines =
       [NSMutableArray arrayWithArray:
           [sdpString componentsSeparatedByString:lineSeparator]];
   NSInteger mLineIndex = -1;
-  NSString *codecRtpMap = nil;
+  // An array with all payload types with name |codec|. The payload types are
+  // integers in the range 96-127, but they are stored as strings here.
+  NSMutableArray *codecPayloadTypes = [[NSMutableArray alloc] init];
   // a=rtpmap:<payload type> <encoding name>/<clock rate>
   // [/<encoding parameters>]
   NSString *pattern =
       [NSString stringWithFormat:@"^a=rtpmap:(\\d+) %@(/\\d+)+[\r]?$", codec];
   NSRegularExpression *regex =

[daniela-webrtc, 2016/11/21 16:23:00]

I know this is not part of this CL, but perhaps in future we can consider using
NSScanner for this portion?
I think it would increase the readability without decreasing (if not increasing)
performance.

[magjed_webrtc, 2016/11/22 14:40:24]

Yeah maybe. I'm not familiar with NSScanner so I'm not sure how the code would
end up looking. I guess it's similar to Scanner in Java, and in that case I
actually prefer regex parsing.

       [NSRegularExpression regularExpressionWithPattern:pattern
                                                 options:0
                                                   error:nil];
-  for (NSInteger i = 0; (i < lines.count) && (mLineIndex == -1 || !codecRtpMap);
-       ++i) {
+  for (NSInteger i = 0; i < lines.count; ++i) {
     NSString *line = lines[i];
     if ([line hasPrefix:@"m=video"]) {
       mLineIndex = i;
       continue;
     }
     NSTextCheckingResult *codecMatches =
         [regex firstMatchInString:line
                           options:0
                             range:NSMakeRange(0, line.length)];
     if (codecMatches) {
-      codecRtpMap =
-          [line substringWithRange:[codecMatches rangeAtIndex:1]];
-      continue;
+      [codecPayloadTypes
+          addObject:[line substringWithRange:[codecMatches rangeAtIndex:1]]];
     }
   }
   if (mLineIndex == -1) {

[daniela-webrtc, 2016/11/21 16:23:00]

can we extract the check for the "m=video" even before the loop?
For instance, perhaps some nicer form of the following nested if*
if (![sdpString hasPrefix:@"m=video"]) {
  if (![sdpString containsString:@"\nm=video"]) {
  //handle the use case when no m=video line has been found
  }
}

*this if nest is purely to demonstrate the logic I'm suggesting :)

[magjed_webrtc, 2016/11/22 14:40:24]

Done, I extracted finding the "m=video" line out from this for-loop.

     RTCLog(@"No m=video line, so can't prefer %@", codec);
     return description;
   }
-  if (!codecRtpMap) {
-    RTCLog(@"No rtpmap for %@", codec);
+  if ([codecPayloadTypes count] == 0) {
+    RTCLog(@"No payload types with name %@", codec);
     return description;
   }
   NSArray *origMLineParts =
       [lines[mLineIndex] componentsSeparatedByString:mLineSeparator];
   if (origMLineParts.count > 3) {
     NSMutableArray *newMLineParts =
         [NSMutableArray arrayWithCapacity:origMLineParts.count];
     NSInteger origPartIndex = 0;
     // Format is: m=<media> <port> <proto> <fmt> ...
     [newMLineParts addObject:origMLineParts[origPartIndex++]];
     [newMLineParts addObject:origMLineParts[origPartIndex++]];
     [newMLineParts addObject:origMLineParts[origPartIndex++]];
-    [newMLineParts addObject:codecRtpMap];
+    [newMLineParts addObjectsFromArray:codecPayloadTypes];

[daniela-webrtc, 2016/11/21 16:23:00]

Perhaps we can extend the usage of methods of NSMutableArray further and perhaps
get rid of the origPartIndex and the second for loop.
Something like *: 
 if (origMLineParts.count > 3) {
    NSMutableArray *newMLineParts =	 
         [NSMutableArray arrayWithArray:origMLineParts];
    [newMLineParts removeObjectsInArray:codecPayloadTypes];	
    [newMLineParts insertObjects:codecPayloadTypes atIndexes:[...]];
}
I don't know the usage of insertObjects:atIndexes: by heart, but this link
should provide enough info on the usage
https://developer.apple.com/reference/foundation/nsmutablearray/1416482-inser...

* this is not working code, purely to demonstrate the logic :)

[magjed_webrtc, 2016/11/22 14:40:24]

Done, I tried my best to make it cleaner.

     for (; origPartIndex < origMLineParts.count; ++origPartIndex) {
-      if (![codecRtpMap isEqualToString:origMLineParts[origPartIndex]]) {
+      if (![codecPayloadTypes containsObject:origMLineParts[origPartIndex]]) {
         [newMLineParts addObject:origMLineParts[origPartIndex]];
       }
     }
     NSString *newMLine =
         [newMLineParts componentsJoinedByString:mLineSeparator];
     [lines replaceObjectAtIndex:mLineIndex
                      withObject:newMLine];
   } else {
     RTCLogWarning(@"Wrong SDP media description format: %@", lines[mLineIndex]);
   }
   NSString *mangledSdpString = [lines componentsJoinedByString:lineSeparator];
   return [[RTCSessionDescription alloc] initWithType:description.type
                                                  sdp:mangledSdpString];
 }
 
 @end