Pass SdpAudioFormat through Channel, without converting to CodecInst

webrtc/modules/audio_coding/codecs/builtin_audio_decoder_factory.cc

 /*
  *  Copyright (c) 2016 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 90 matching lines @@
            return Unique(new AudioDecoderG722);
          if (format.num_channels == 2)
            return Unique(new AudioDecoderG722Stereo);
        }
        return Unique(nullptr);
      }},
 #endif
 #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
                   ? Unique(new AudioDecoderOpus(*num_channels))
                   : nullptr;
      }},
 #endif
 };
 
 class BuiltinAudioDecoderFactory : public AudioDecoderFactory {
  public:
   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) {
       if (STR_CASE_CMP(format.name.c_str(), dc.name) == 0) {
         std::unique_ptr<AudioDecoder> dec = dc.constructor(format);
         if (dec) {
           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());
         }
         return dec;
       }
     }
     return nullptr;
   }
 };
 
 }  // namespace
 
 rtc::scoped_refptr<AudioDecoderFactory> CreateBuiltinAudioDecoderFactory() {
   return rtc::scoped_refptr<AudioDecoderFactory>(
       new rtc::RefCountedObject<BuiltinAudioDecoderFactory>);
 }
 
 }  // namespace webrtc