Pass SdpAudioFormat through Channel, without converting to CodecInst

BUG=webrtc:5805 Review-Url: https://codereview.webrtc.org/2516993002 Cr-Commit-Position: refs/heads/master@{#16165}
2017-01-19 07:03:59 -08:00
parent 093dac142b
commit d32bf75721
21 changed files with 315 additions and 137 deletions
--- a/webrtc/modules/audio_coding/codecs/audio_decoder_factory.h
+++ b/webrtc/modules/audio_coding/codecs/audio_decoder_factory.h
@ -27,6 +27,8 @@ class AudioDecoderFactory : public rtc::RefCountInterface {
 public:
  virtual std::vector<AudioCodecSpec> GetSupportedDecoders() = 0;

+  virtual bool IsSupportedDecoder(const SdpAudioFormat& format) = 0;
+
  virtual std::unique_ptr<AudioDecoder> MakeAudioDecoder(
      const SdpAudioFormat& format) = 0;
 };
--- a/webrtc/modules/audio_coding/codecs/audio_decoder_factory_unittest.cc
+++ b/webrtc/modules/audio_coding/codecs/audio_decoder_factory_unittest.cc
@ -122,8 +122,8 @@ TEST(AudioDecoderFactoryTest, CreateOpus) {
        if (stereo != "XX") {
          params["stereo"] = stereo;
        }
-        bool good =
-            (hz == 48000 && channels == 2 && (stereo == "0" || stereo == "1"));
+        const bool good = (hz == 48000 && channels == 2 &&
+                           (stereo == "XX" || stereo == "0" || stereo == "1"));
        EXPECT_EQ(good, static_cast<bool>(adf->MakeAudioDecoder(SdpAudioFormat(
                            "opus", hz, channels, std::move(params)))));
      }
--- a/webrtc/modules/audio_coding/codecs/audio_format.cc
+++ b/webrtc/modules/audio_coding/codecs/audio_format.cc
@ -22,14 +22,28 @@ SdpAudioFormat::SdpAudioFormat(const char* name,
                               int num_channels)
    : name(name), clockrate_hz(clockrate_hz), num_channels(num_channels) {}

+SdpAudioFormat::SdpAudioFormat(const std::string& name,
+                               int clockrate_hz,
+                               int num_channels)
+    : name(name), clockrate_hz(clockrate_hz), num_channels(num_channels) {}
+
 SdpAudioFormat::SdpAudioFormat(const char* name,
                               int clockrate_hz,
                               int num_channels,
-                               Parameters&& param)
+                               const Parameters& param)
    : name(name),
      clockrate_hz(clockrate_hz),
      num_channels(num_channels),
-      parameters(std::move(param)) {}
+      parameters(param) {}
+
+SdpAudioFormat::SdpAudioFormat(const std::string& name,
+                               int clockrate_hz,
+                               int num_channels,
+                               const Parameters& param)
+    : name(name),
+      clockrate_hz(clockrate_hz),
+      num_channels(num_channels),
+      parameters(param) {}

 SdpAudioFormat::~SdpAudioFormat() = default;
 SdpAudioFormat& SdpAudioFormat::operator=(const SdpAudioFormat&) = default;
--- a/webrtc/modules/audio_coding/codecs/audio_format.h
+++ b/webrtc/modules/audio_coding/codecs/audio_format.h
@ -26,10 +26,15 @@ struct SdpAudioFormat {
  SdpAudioFormat(const SdpAudioFormat&);
  SdpAudioFormat(SdpAudioFormat&&);
  SdpAudioFormat(const char* name, int clockrate_hz, int num_channels);
+  SdpAudioFormat(const std::string& name, int clockrate_hz, int num_channels);
  SdpAudioFormat(const char* name,
                 int clockrate_hz,
                 int num_channels,
-                 Parameters&& param);
+                 const Parameters& param);
+  SdpAudioFormat(const std::string& name,
+                 int clockrate_hz,
+                 int num_channels,
+                 const Parameters& param);
  ~SdpAudioFormat();

  SdpAudioFormat& operator=(const SdpAudioFormat&);
--- a/webrtc/modules/audio_coding/codecs/audio_format_conversion.cc
+++ b/webrtc/modules/audio_coding/codecs/audio_format_conversion.cc
@ -10,21 +10,79 @@

 #include "webrtc/modules/audio_coding/codecs/audio_format_conversion.h"

+#include <string.h>
+
+#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 MakeCodecInst(int payload_type,
+                        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));
+  ci.pltype = payload_type;
+  strncpy(ci.plname, name, sizeof(ci.plname));
+  ci.plname[sizeof(ci.plname) - 1] = '\0';
+  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) {
+  if (STR_CASE_CMP(ci.plname, "g722") == 0) {
+    RTC_CHECK_EQ(16000, ci.plfreq);
    RTC_CHECK(ci.channels == 1 || ci.channels == 2);
    return {"g722", 8000, static_cast<int>(ci.channels)};
-  } else if (STR_CASE_CMP(ci.plname, "opus") == 0 && ci.plfreq == 48000) {
+  } else if (STR_CASE_CMP(ci.plname, "opus") == 0) {
+    RTC_CHECK_EQ(48000, ci.plfreq);
    RTC_CHECK(ci.channels == 1 || ci.channels == 2);
-    return {"opus", 48000, 2, {{"stereo", ci.channels == 1 ? "0" : "1"}}};
+    return ci.channels == 1
+               ? SdpAudioFormat("opus", 48000, 2)
+               : SdpAudioFormat("opus", 48000, 2, {{"stereo", "1"}});
  } else {
    return {ci.plname, ci.plfreq, rtc::checked_cast<int>(ci.channels)};
  }
 }

+CodecInst SdpToCodecInst(int payload_type, const SdpAudioFormat& audio_format) {
+  if (STR_CASE_CMP(audio_format.name.c_str(), "g722") == 0) {
+    RTC_CHECK_EQ(8000, audio_format.clockrate_hz);
+    RTC_CHECK(audio_format.num_channels == 1 || audio_format.num_channels == 2);
+    return MakeCodecInst(payload_type, "g722", 16000,
+                         audio_format.num_channels);
+  } else if (STR_CASE_CMP(audio_format.name.c_str(), "opus") == 0) {
+    RTC_CHECK_EQ(48000, audio_format.clockrate_hz);
+    RTC_CHECK_EQ(2, audio_format.num_channels);
+    const int num_channels = [&] {
+      auto stereo = audio_format.parameters.find("stereo");
+      if (stereo != audio_format.parameters.end()) {
+        if (stereo->second == "0") {
+          return 1;
+        } else if (stereo->second == "1") {
+          return 2;
+        } else {
+          RTC_CHECK(false);  // Bad stereo parameter.
+        }
+      }
+      return 1;  // Default to mono.
+    }();
+    return MakeCodecInst(payload_type, "opus", 48000, num_channels);
+  } else {
+    return MakeCodecInst(payload_type, audio_format.name.c_str(),
+                         audio_format.clockrate_hz, audio_format.num_channels);
+  }
+}
+
 }  // namespace webrtc
--- a/webrtc/modules/audio_coding/codecs/audio_format_conversion.h
+++ b/webrtc/modules/audio_coding/codecs/audio_format_conversion.h
@ -17,6 +17,7 @@
 namespace webrtc {

 SdpAudioFormat CodecInstToSdp(const CodecInst& codec_inst);
+CodecInst SdpToCodecInst(int payload_type, const SdpAudioFormat& audio_format);

 }  // namespace webrtc

--- a/webrtc/modules/audio_coding/codecs/builtin_audio_decoder_factory.cc
+++ b/webrtc/modules/audio_coding/codecs/builtin_audio_decoder_factory.cc
@ -40,89 +40,133 @@ namespace {

 struct NamedDecoderConstructor {
  const char* name;
-  std::unique_ptr<AudioDecoder> (*constructor)(const SdpAudioFormat&);
-};

-std::unique_ptr<AudioDecoder> Unique(AudioDecoder* d) {
-  return std::unique_ptr<AudioDecoder>(d);
-}
+  // If |format| is good, return true and (if |out| isn't null) reset |*out| to
+  // a new decoder object. If the |format| is not good, return false.
+  bool (*constructor)(const SdpAudioFormat& format,
+                      std::unique_ptr<AudioDecoder>* out);
+};

 // TODO(kwiberg): These factory functions should probably be moved to each
 // decoder.
 NamedDecoderConstructor decoder_constructors[] = {
    {"pcmu",
-     [](const SdpAudioFormat& format) {
-       return format.clockrate_hz == 8000 && format.num_channels >= 1
-                  ? Unique(new AudioDecoderPcmU(format.num_channels))
-                  : nullptr;
+     [](const SdpAudioFormat& format, std::unique_ptr<AudioDecoder>* out) {
+       if (format.clockrate_hz == 8000 && format.num_channels >= 1) {
+         if (out) {
+           out->reset(new AudioDecoderPcmU(format.num_channels));
+         }
+         return true;
+       } else {
+         return false;
+       }
     }},
    {"pcma",
-     [](const SdpAudioFormat& format) {
-       return format.clockrate_hz == 8000 && format.num_channels >= 1
-                  ? Unique(new AudioDecoderPcmA(format.num_channels))
-                  : nullptr;
+     [](const SdpAudioFormat& format, std::unique_ptr<AudioDecoder>* out) {
+       if (format.clockrate_hz == 8000 && format.num_channels >= 1) {
+         if (out) {
+           out->reset(new AudioDecoderPcmA(format.num_channels));
+         }
+         return true;
+       } else {
+         return false;
+       }
     }},
 #ifdef WEBRTC_CODEC_ILBC
    {"ilbc",
-     [](const SdpAudioFormat& format) {
-       return format.clockrate_hz == 8000 && format.num_channels == 1
-                  ? Unique(new AudioDecoderIlbc)
-                  : nullptr;
+     [](const SdpAudioFormat& format, std::unique_ptr<AudioDecoder>* out) {
+       if (format.clockrate_hz == 8000 && format.num_channels == 1) {
+         if (out) {
+           out->reset(new AudioDecoderIlbc);
+         }
+         return true;
+       } else {
+         return false;
+       }
     }},
 #endif
 #if defined(WEBRTC_CODEC_ISACFX)
    {"isac",
-     [](const SdpAudioFormat& format) {
-       return format.clockrate_hz == 16000 && format.num_channels == 1
-                  ? Unique(new AudioDecoderIsacFix(format.clockrate_hz))
-                  : nullptr;
+     [](const SdpAudioFormat& format, std::unique_ptr<AudioDecoder>* out) {
+       if (format.clockrate_hz == 16000 && format.num_channels == 1) {
+         if (out) {
+           out->reset(new AudioDecoderIsacFix(format.clockrate_hz));
+         }
+         return true;
+       } else {
+         return false;
+       }
     }},
 #elif defined(WEBRTC_CODEC_ISAC)
    {"isac",
-     [](const SdpAudioFormat& format) {
-       return (format.clockrate_hz == 16000 || format.clockrate_hz == 32000) &&
-                      format.num_channels == 1
-                  ? Unique(new AudioDecoderIsac(format.clockrate_hz))
-                  : nullptr;
+     [](const SdpAudioFormat& format, std::unique_ptr<AudioDecoder>* out) {
+       if ((format.clockrate_hz == 16000 || format.clockrate_hz == 32000) &&
+           format.num_channels == 1) {
+         if (out) {
+           out->reset(new AudioDecoderIsac(format.clockrate_hz));
+         }
+         return true;
+       } else {
+         return false;
+       }
     }},
 #endif
    {"l16",
-     [](const SdpAudioFormat& format) {
-       return format.num_channels >= 1
-                  ? Unique(new AudioDecoderPcm16B(format.clockrate_hz,
-                                                  format.num_channels))
-                  : nullptr;
+     [](const SdpAudioFormat& format, std::unique_ptr<AudioDecoder>* out) {
+       if (format.num_channels >= 1) {
+         if (out) {
+           out->reset(new AudioDecoderPcm16B(format.clockrate_hz,
+                                             format.num_channels));
+         }
+         return true;
+       } else {
+         return false;
+       }
     }},
 #ifdef WEBRTC_CODEC_G722
    {"g722",
-     [](const SdpAudioFormat& format) {
+     [](const SdpAudioFormat& format, std::unique_ptr<AudioDecoder>* out) {
       if (format.clockrate_hz == 8000) {
-         if (format.num_channels == 1)
-           return Unique(new AudioDecoderG722);
-         if (format.num_channels == 2)
-           return Unique(new AudioDecoderG722Stereo);
+         if (format.num_channels == 1) {
+           if (out) {
+             out->reset(new AudioDecoderG722);
+           }
+           return true;
+         } else if (format.num_channels == 2) {
+           if (out) {
+             out->reset(new AudioDecoderG722Stereo);
+           }
+           return true;
+         }
       }
-       return Unique(nullptr);
+       return false;
     }},
 #endif
 #ifdef WEBRTC_CODEC_OPUS
    {"opus",
-     [](const SdpAudioFormat& format) {
-       rtc::Optional<int> num_channels = [&] {
+     [](const SdpAudioFormat& format, std::unique_ptr<AudioDecoder>* out) {
+       const rtc::Optional<int> num_channels = [&] {
         auto stereo = format.parameters.find("stereo");
         if (stereo != format.parameters.end()) {
           if (stereo->second == "0") {
             return rtc::Optional<int>(1);
           } else if (stereo->second == "1") {
             return rtc::Optional<int>(2);
+           } else {
+             return rtc::Optional<int>();  // Bad stereo parameter.
           }
         }
-         return rtc::Optional<int>();
+         return rtc::Optional<int>(1);  // Default to mono.
       }();
-       return format.clockrate_hz == 48000 && format.num_channels == 2 &&
-                      num_channels
-                  ? Unique(new AudioDecoderOpus(*num_channels))
-                  : nullptr;
+       if (format.clockrate_hz == 48000 && format.num_channels == 2 &&
+           num_channels) {
+         if (out) {
+           out->reset(new AudioDecoderOpus(*num_channels));
+         }
+         return true;
+       } else {
+         return false;
+       }
     }},
 #endif
 };
@ -140,37 +184,48 @@ class BuiltinAudioDecoderFactory : public AudioDecoderFactory {
      },
 #endif
 #if (defined(WEBRTC_CODEC_ISAC) || defined(WEBRTC_CODEC_ISACFX))
-      { { "isac", 16000, 1 }, true },
+      {{"isac", 16000, 1}, true},
 #endif
 #if (defined(WEBRTC_CODEC_ISAC))
-      { { "isac", 32000, 1 }, true },
+      {{"isac", 32000, 1}, true},
 #endif
 #ifdef WEBRTC_CODEC_G722
-      { { "G722", 8000,  1 }, true },
+      {{"G722", 8000, 1}, true},
 #endif
 #ifdef WEBRTC_CODEC_ILBC
-      { { "iLBC", 8000,  1 }, true },
+      {{"iLBC", 8000, 1}, true},
 #endif
-      { { "PCMU", 8000,  1 }, true },
-      { { "PCMA", 8000,  1 }, true }
+      {{"PCMU", 8000, 1}, true},
+      {{"PCMA", 8000, 1}, true}
    };

    return specs;
  }

+  bool IsSupportedDecoder(const SdpAudioFormat& format) override {
+    for (const auto& dc : decoder_constructors) {
+      if (STR_CASE_CMP(format.name.c_str(), dc.name) == 0) {
+        return dc.constructor(format, nullptr);
+      }
+    }
+    return false;
+  }
+
  std::unique_ptr<AudioDecoder> MakeAudioDecoder(
      const SdpAudioFormat& format) override {
    for (const auto& dc : decoder_constructors) {
      if (STR_CASE_CMP(format.name.c_str(), dc.name) == 0) {
-        std::unique_ptr<AudioDecoder> dec = dc.constructor(format);
-        if (dec) {
+        std::unique_ptr<AudioDecoder> decoder;
+        bool ok = dc.constructor(format, &decoder);
+        RTC_DCHECK_EQ(ok, decoder != nullptr);
+        if (decoder) {
          const int expected_sample_rate_hz =
              STR_CASE_CMP(format.name.c_str(), "g722") == 0
                  ? 2 * format.clockrate_hz
                  : format.clockrate_hz;
-          RTC_CHECK_EQ(expected_sample_rate_hz, dec->SampleRateHz());
+          RTC_CHECK_EQ(expected_sample_rate_hz, decoder->SampleRateHz());
        }
-        return dec;
+        return decoder;
      }
    }
    return nullptr;
--- a/webrtc/modules/audio_coding/codecs/mock/mock_audio_decoder_factory.h
+++ b/webrtc/modules/audio_coding/codecs/mock/mock_audio_decoder_factory.h
@ -15,6 +15,7 @@

 #include "webrtc/base/scoped_ref_ptr.h"
 #include "webrtc/modules/audio_coding/codecs/audio_decoder_factory.h"
+#include "webrtc/modules/audio_coding/codecs/builtin_audio_decoder_factory.h"
 #include "webrtc/test/gmock.h"

 namespace webrtc {
@ -22,6 +23,7 @@ namespace webrtc {
 class MockAudioDecoderFactory : public AudioDecoderFactory {
 public:
  MOCK_METHOD0(GetSupportedDecoders, std::vector<AudioCodecSpec>());
+  MOCK_METHOD1(IsSupportedDecoder, bool(const SdpAudioFormat&));
  std::unique_ptr<AudioDecoder> MakeAudioDecoder(
      const SdpAudioFormat& format) {
    std::unique_ptr<AudioDecoder> return_value;
@ -46,6 +48,8 @@ class MockAudioDecoderFactory : public AudioDecoderFactory {
    ON_CALL(*factory.get(), GetSupportedDecoders())
        .WillByDefault(Return(std::vector<webrtc::AudioCodecSpec>()));
    EXPECT_CALL(*factory.get(), GetSupportedDecoders()).Times(AnyNumber());
+    ON_CALL(*factory, IsSupportedDecoder(_)).WillByDefault(Return(false));
+    EXPECT_CALL(*factory, IsSupportedDecoder(_)).Times(AnyNumber());
    EXPECT_CALL(*factory.get(), MakeAudioDecoderMock(_, _)).Times(0);
    return factory;
  }
@ -65,6 +69,8 @@ class MockAudioDecoderFactory : public AudioDecoderFactory {
    ON_CALL(*factory.get(), GetSupportedDecoders())
        .WillByDefault(Return(std::vector<webrtc::AudioCodecSpec>()));
    EXPECT_CALL(*factory.get(), GetSupportedDecoders()).Times(AnyNumber());
+    ON_CALL(*factory, IsSupportedDecoder(_)).WillByDefault(Return(false));
+    EXPECT_CALL(*factory, IsSupportedDecoder(_)).Times(AnyNumber());
    ON_CALL(*factory.get(), MakeAudioDecoderMock(_, _))
        .WillByDefault(SetArgPointee<1>(nullptr));
    EXPECT_CALL(*factory.get(), MakeAudioDecoderMock(_, _)).Times(AnyNumber());