WebRtcVoiceMediaChannel::AddRecvStream: Don't call SetRecPayloadType

webrtc/modules/audio_coding/neteq/decoder_database.cc

 /*
  *  Copyright (c) 2012 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 105 matching lines @@
 bool DecoderDatabase::Empty() const { return decoders_.empty(); }
 
 int DecoderDatabase::Size() const { return static_cast<int>(decoders_.size()); }
 
 void DecoderDatabase::Reset() {
   decoders_.clear();
   active_decoder_type_ = -1;
   active_cng_decoder_type_ = -1;
 }
 
+std::vector<int> DecoderDatabase::SetCodecs(
+    const std::map<int, SdpAudioFormat>& codecs) {
+  // First collect all payload types that we'll remove or reassign, then remove

[the sun, 2017/03/14 20:07:01]

As discussed, we're not allowed to remove or reassign PT mappings. Besides, this
should all be taken care of in WVoMC (the API boundary) - at this level we
should be able to "just" latch in the new map (ignoring previous PT
assignments). May be a good idea to verify previous mappings are still there
though.

Or, did I miss something?

[kwiberg-webrtc, 2017/03/16 10:24:12]

I tried to do this (DCHECK that the old map is a subset of the new one), but it
caused ~75 tests to fail. I then tried to also create channels with an empty
decoder map, but that didn't solve the problem.

For good reasons or bad, NetEq already had the ability to remove and reassign
payload type mappings, and our tests make use of it. I'm sure that could be
fixed (since, as you say, it's not allowed), but fixing it as part of this CL is
probably not a good idea---it's big enough already.

Should I post a bug about it (removing the remove/reassign capabilities from
NetEq, and dealing with the fallout), and leave the capability in for now? The
alternative for this CL would be to not make a SetCodecs() method in NetEq, but
use the old RegisterPayload(), Remove() etc.---which would not really solve the
problem at all.

+  // them from the database.
+  std::vector<int> changed_payload_types;
+  for (const std::pair<uint8_t, const DecoderInfo&> kv : decoders_) {
+    auto i = codecs.find(kv.first);
+    if (i == codecs.end() || i->second != kv.second.GetFormat()) {
+      changed_payload_types.push_back(kv.first);
+    }
+  }
+  for (int pl_type : changed_payload_types) {
+    Remove(pl_type);
+  }
+
+  // Enter the new and changed payload type mappings into the database.
+  for (const auto& kv : codecs) {
+    const int& rtp_payload_type = kv.first;
+    const SdpAudioFormat& audio_format = kv.second;
+    RTC_DCHECK_GE(rtp_payload_type, 0);
+    RTC_DCHECK_LE(rtp_payload_type, 0x7f);
+    if (decoders_.count(rtp_payload_type) == 0) {
+      decoders_.insert(std::make_pair(
+          rtp_payload_type, DecoderInfo(audio_format, decoder_factory_.get())));
+    } else {
+      // The mapping for this payload type hasn't changed.
+    }
+  }
+
+  return changed_payload_types;
+}
+
 int DecoderDatabase::RegisterPayload(uint8_t rtp_payload_type,
                                      NetEqDecoder codec_type,
                                      const std::string& name) {
   if (rtp_payload_type > 0x7F) {
     return kInvalidRtpPayloadType;
   }
   // kCodecArbitrary is only supported through InsertExternal.
   if (codec_type == NetEqDecoder::kDecoderArbitrary ||
       !CodecSupported(codec_type)) {
     return kCodecNotSupported;
@@ skipping 178 matching lines @@
       // Payload type is not found.
       LOG(LS_WARNING) << "CheckPayloadTypes: unknown RTP payload type "
                       << static_cast<int>(it->payload_type);
       return kDecoderNotFound;
     }
   }
   return kOK;
 }
 
 }  // namespace webrtc