Pass SdpAudioFormat through Channel, without converting to CodecInst

webrtc/modules/audio_coding/codecs/audio_format_conversion.cc

Index: webrtc/modules/audio_coding/codecs/audio_format_conversion.cc
diff --git a/webrtc/modules/audio_coding/codecs/audio_format_conversion.cc b/webrtc/modules/audio_coding/codecs/audio_format_conversion.cc
index ef9aa4479b9be95466f25ca24b1a5569c87778df..165a1adb2618f14a72ab333862796f6e698b5fb2 100644
--- a/webrtc/modules/audio_coding/codecs/audio_format_conversion.cc
+++ b/webrtc/modules/audio_coding/codecs/audio_format_conversion.cc
@@ -10,11 +10,36 @@
 
 #include "webrtc/modules/audio_coding/codecs/audio_format_conversion.h"
 
+#include <cstring>

[the sun, 2016/12/08 10:53:52]

The code base generally uses string.h

[kwiberg-webrtc, 2016/12/09 02:39:01]

Yes, unfortunately. Fixed.

+
+#include "webrtc/base/array_view.h"
 #include "webrtc/base/checks.h"
+#include "webrtc/base/optional.h"
 #include "webrtc/base/safe_conversions.h"
+#include "webrtc/base/sanitizer.h"
 
 namespace webrtc {
 
+namespace {
+
+CodecInst MakeCI(int payload_type,

[the sun, 2016/12/08 10:53:52]

MakeCodecInst

[kwiberg-webrtc, 2016/12/09 02:39:02]

Done.

+                 const char* name,
+                 int sample_rate,
+                 int num_channels) {
+  // Create a CodecInst with some fields set. The remaining fields are zeroed,
+  // but we tell MSan to consider them uninitialized.
+  CodecInst ci = {0};
+  rtc::MsanMarkUninitialized(rtc::MakeArrayView(&ci, 1));

[the sun, 2016/12/08 10:53:52]

nice!

+  ci.pltype = payload_type;
+  std::strncpy(ci.plname, name, sizeof(ci.plname));
+  ci.plname[sizeof(ci.plname) - 1] = '\0';

[the sun, 2016/12/08 10:53:52]

don't you need to init the trailing zero as well? won't we get sanitizer reports
if comparing against it otherwise?

[kwiberg-webrtc, 2016/12/09 02:39:01]

No, strncpy copies the null terminator---unless the copying was cut short by the
count argument before we reached it. The explicit null write here guards against
that case.

[the sun, 2016/12/09 12:29:49]

Ah, sorry. Read wrong.

+  ci.plfreq = sample_rate;
+  ci.channels = rtc::checked_cast<size_t>(num_channels);
+  return ci;
+}
+
+}  // namespace
+
 SdpAudioFormat CodecInstToSdp(const CodecInst& ci) {
   if (STR_CASE_CMP(ci.plname, "g722") == 0 && ci.plfreq == 16000) {
     RTC_CHECK(ci.channels == 1 || ci.channels == 2);
@@ -27,4 +52,32 @@ SdpAudioFormat CodecInstToSdp(const CodecInst& ci) {
   }
 }
 
+CodecInst SdpToCodecInst(int payload_type, const SdpAudioFormat& audio_format) {
+  if (STR_CASE_CMP(audio_format.name.c_str(), "g722") == 0 &&
+      audio_format.clockrate_hz == 8000 &&
+      (audio_format.num_channels == 1 || audio_format.num_channels == 2)) {
+    return MakeCI(payload_type, "g722", 16000, audio_format.num_channels);

[the sun, 2016/12/08 10:53:52]

no other parameters that would need to be copied?

[kwiberg-webrtc, 2016/12/09 02:39:01]

Like what? CodecInst doesn't have the extra parameter map, and SdpAudioFormat
doesn't have a rate field.

[the sun, 2016/12/09 12:29:49]

Acknowledged.

+  } else if (STR_CASE_CMP(audio_format.name.c_str(), "opus") == 0 &&
+             audio_format.clockrate_hz == 48000 &&
+             audio_format.num_channels == 2) {
+    const rtc::Optional<int> num_channels = [&] {
+      const auto stereo = audio_format.parameters.find("stereo");
+      if (stereo != audio_format.parameters.end()) {
+        if (stereo->second == "0") {
+          return rtc::Optional<int>(1);
+        } else if (stereo->second == "1") {
+          return rtc::Optional<int>(2);
+        }
+      }
+      return rtc::Optional<int>();
+    }();
+    if (num_channels) {
+      return MakeCI(payload_type, "opus", 48000, *num_channels);
+    }
+  }
+
+  return MakeCI(payload_type, audio_format.name.c_str(),
+                audio_format.clockrate_hz, audio_format.num_channels);
+}
+
 }  // namespace webrtc