Simplify IsFmtpParam according to RFC 4855.

webrtc/pc/webrtcsdp.cc

 /*
  *  Copyright 2011 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.
  */
 
@@ skipping 1579 matching lines @@
     if (fmtp == parameters.begin()) {
       *os << kSdpDelimiterSpace;
     } else {
       *os << kSdpDelimiterSemicolon;
     }
     WriteFmtpParameter(fmtp->first, fmtp->second, os);
   }
 }
 
 bool IsFmtpParam(const std::string& name) {
-  const char* kFmtpParams[] = {
-      // TODO(hta): Split FMTP parameters apart from parameters in general.
-      // FMTP parameters are codec specific, not generic.
-      kCodecParamMinPTime,
-      kCodecParamSPropStereo,
-      kCodecParamStereo,
-      kCodecParamUseInbandFec,
-      kCodecParamUseDtx,
-      kCodecParamStartBitrate,
-      kCodecParamMaxBitrate,
-      kCodecParamMinBitrate,
-      kCodecParamMaxQuantization,
-      kCodecParamSctpProtocol,
-      kCodecParamSctpStreams,
-      kCodecParamMaxAverageBitrate,
-      kCodecParamMaxPlaybackRate,
-      kCodecParamAssociatedPayloadType,
-      cricket::kH264FmtpPacketizationMode,
-      cricket::kH264FmtpLevelAsymmetryAllowed,
-      cricket::kH264FmtpProfileLevelId,
-      cricket::kFlexfecFmtpRepairWindow};
-  for (size_t i = 0; i < arraysize(kFmtpParams); ++i) {
-    if (name.compare(kFmtpParams[i]) == 0) {
-      return true;
-    }
-  }
-  return false;

[ossu, 2017/01/27 13:00:42]

I added a debug printout here and ran through our tests. Only ptime and maxptime
was filtered out, so our previous implementation seems to match what the RFC
says, albeit in a roundabout way.

+  // RFC 4855, section 3 specifies the mapping of media formats to SDP
+  // parameters. Only ptime, maxptime, channels and rate are placed outside of
+  // the fmtp line. In WebRTC, channels and rate are already handled separately
+  // and thus not included in the CodecParameterMap.
+  return stricmp(name.c_str(), kCodecParamPTime) != 0 &&
+         stricmp(name.c_str(), kCodecParamMaxPTime) != 0;

[hta-webrtc, 2017/01/27 13:59:30]

There's a functional change here. You're switching from case sensitve compare to
case insensitive compare.
Do we have a spec-valid reason for that?
(I don't guarantee that the old way was right!)

[ossu, 2017/01/27 15:13:25]

Yes, the parameter names are case insensitive and we treat them like that in
other places. Referring back to RFC 4855, we have:

  The format for a parameter is specified in RFC 2045 as
        attribute "=" value

and in RFC 2045:

       parameter := attribute "=" value

       attribute := token
                    ; Matching of attributes
                    ; is ALWAYS case-insensitive.

 }
 
 // Retreives fmtp parameters from |params|, which may contain other parameters
 // as well, and puts them in |fmtp_parameters|.
 void GetFmtpParams(const cricket::CodecParameterMap& params,
                    cricket::CodecParameterMap* fmtp_parameters) {
   for (cricket::CodecParameterMap::const_iterator iter = params.begin();
        iter != params.end(); ++iter) {
     if (IsFmtpParam(iter->first)) {
       (*fmtp_parameters)[iter->first] = iter->second;
@@ skipping 1576 matching lines @@
     UpdateCodec<AudioContentDescription, cricket::AudioCodec>(
         media_desc, payload_type, feedback_param);
   } else if (media_type == cricket::MEDIA_TYPE_VIDEO) {
     UpdateCodec<VideoContentDescription, cricket::VideoCodec>(
         media_desc, payload_type, feedback_param);
   }
   return true;
 }
 
 }  // namespace webrtc