Pass SdpAudioFormat through Channel, without converting to CodecInst

webrtc/modules/audio_coding/codecs/builtin_audio_decoder_factory.cc

Index: webrtc/modules/audio_coding/codecs/builtin_audio_decoder_factory.cc
diff --git a/webrtc/modules/audio_coding/codecs/builtin_audio_decoder_factory.cc b/webrtc/modules/audio_coding/codecs/builtin_audio_decoder_factory.cc
index 05b65c3b860f28db093636eea120176bc01388a2..481add25ddceba83b824a69d51befcd68e8f9cb1 100644
--- a/webrtc/modules/audio_coding/codecs/builtin_audio_decoder_factory.cc
+++ b/webrtc/modules/audio_coding/codecs/builtin_audio_decoder_factory.cc
@@ -108,16 +108,18 @@ NamedDecoderConstructor decoder_constructors[] = {
 #ifdef WEBRTC_CODEC_OPUS
     {"opus",
      [](const SdpAudioFormat& format) {
-       rtc::Optional<int> num_channels = [&] {
+       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
@@ -132,32 +134,38 @@ class BuiltinAudioDecoderFactory : public AudioDecoderFactory {
   std::vector<AudioCodecSpec> GetSupportedDecoders() override {
     static std::vector<AudioCodecSpec> specs = {
 #ifdef WEBRTC_CODEC_OPUS
-      { { "opus", 48000, 2, {
-            {"minptime", "10" },
-            {"useinbandfec", "1" }
-          }
-        }, false
-      },
+      {{"opus",
+        48000,
+        2,
+        { {"minptime", "10"},
+          {"stereo", "0"},
+          { "useinbandfec",
+            "1" } }},
+       false},
 #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 {
+    return static_cast<bool>(MakeAudioDecoder(format));

[ossu, 2016/12/12 13:24:46]

I realize you want to land this one, and that this is probably just used in
tests, but I don't approve of this implementation of what should be a simple
checker function - i.e. something expected to be lightweight.

If you want to keep the structure with decoder_constructors[] above, you could
split it up like so:

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

and

  {"pcmu",
   [](const SdpAudioFormat& format) {
     return format.clockrate_hz == 8000 && format.num_channels >= 1;
   },
   [](const SdpAudioFormat& format) {
     return Unique(new AudioDecoderPcmU(format.num_channels));
   }},

for example. Or even fold the name check into the supported lambda.

[kwiberg-webrtc, 2016/12/14 13:09:36]

Done (not exactly what you suggested, but something to the same effect).

[ossu, 2016/12/14 13:53:35]

Alright! IsSupportedDecoder now behaves as I'd expect it to.

+  }
+
   std::unique_ptr<AudioDecoder> MakeAudioDecoder(
       const SdpAudioFormat& format) override {
     for (const auto& dc : decoder_constructors) {