Templated AudioEncoderFactory

webrtc/api/audio_codecs/audio_encoder_factory_template.h

Index: webrtc/api/audio_codecs/audio_encoder_factory_template.h
diff --git a/webrtc/api/audio_codecs/audio_encoder_factory_template.h b/webrtc/api/audio_codecs/audio_encoder_factory_template.h
new file mode 100644
index 0000000000000000000000000000000000000000..0780c8c37b972b47e3220eb460a8dd7f881d5d5d
--- /dev/null
+++ b/webrtc/api/audio_codecs/audio_encoder_factory_template.h
@@ -0,0 +1,140 @@
+/*
+ *  Copyright (c) 2017 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.
+ */
+
+#ifndef WEBRTC_API_AUDIO_CODECS_AUDIO_ENCODER_FACTORY_TEMPLATE_H_
+#define WEBRTC_API_AUDIO_CODECS_AUDIO_ENCODER_FACTORY_TEMPLATE_H_
+
+#include <memory>
+#include <vector>
+
+#include "webrtc/api/audio_codecs/audio_encoder_factory.h"
+#include "webrtc/base/scoped_ref_ptr.h"
+
+namespace webrtc {
+
+namespace audio_encoder_factory_template_impl {
+
+template <typename... Ts>
+struct Helper;
+
+// Base case: 0 template parameters.
+template <>
+struct Helper<> {
+  static void AppendSupportedEncoders(std::vector<AudioCodecSpec>* specs) {}
+  static rtc::Optional<AudioCodecInfo> QueryAudioEncoder(
+      const SdpAudioFormat& format) {
+    return rtc::Optional<AudioCodecInfo>();
+  }
+  static std::unique_ptr<AudioEncoder> MakeAudioEncoder(
+      int payload_type,
+      const SdpAudioFormat& format) {
+    return nullptr;
+  }
+};
+
+// Inductive case: Called with n + 1 template parameters, calls subroutines
+// with n template parameters.
+template <typename T, typename... Ts>
+struct Helper<T, Ts...> {
+  static void AppendSupportedEncoders(std::vector<AudioCodecSpec>* specs) {
+    T::AppendSupportedEncoders(specs);
+    Helper<Ts...>::AppendSupportedEncoders(specs);
+  }
+  static rtc::Optional<AudioCodecInfo> QueryAudioEncoder(
+      const SdpAudioFormat& format) {
+    auto opt_config = T::SdpToConfig(format);
+    return opt_config ? rtc::Optional<AudioCodecInfo>(
+                            T::QueryAudioEncoder(*opt_config))
+                      : Helper<Ts...>::QueryAudioEncoder(format);
+  }
+  static std::unique_ptr<AudioEncoder> MakeAudioEncoder(
+      int payload_type,
+      const SdpAudioFormat& format) {
+    auto opt_config = T::SdpToConfig(format);
+    if (opt_config) {
+      auto enc = T::MakeAudioEncoder(*opt_config, payload_type);
+      RTC_CHECK(enc);

[the sun, 2017/06/16 09:18:06]

Is this an expected runtime condition or not? If the encoder is hardware backed,
we could run out of resources and should be able to handle that without
crashing.

[kwiberg-webrtc, 2017/06/16 09:57:01]

It's a CHECK, so it's not an expected runtime condition---I intended it to catch
bugs in codec implementations. But if failing to create an encoder that we
thought we could create is a real-world case, I'll just remove the check.

[ossu, 2017/06/16 13:36:53]

Should this be handled as failing to create an AudioEncoder object, though? The
outside code relies on the fact that if QueryAudioEncoder() says yes, we will be
able to create that encoder. There are no fallbacks.

[kwiberg-webrtc, 2017/06/16 13:40:08]

Well, in what other way could we possibly handle it? Hmm. By forcing all codecs
to fall back to software implementations, I guess?

+      return enc;
+    } else {
+      return Helper<Ts...>::MakeAudioEncoder(payload_type, format);
+    }
+  }
+};
+
+template <typename... Ts>
+class AudioEncoderFactoryT : public AudioEncoderFactory {
+ public:
+  std::vector<AudioCodecSpec> GetSupportedEncoders() override {
+    static const std::vector<AudioCodecSpec> specs = [] {

[the sun, 2017/06/16 09:18:06]

Caching the specs preclude dynamically changing the list of supported codecs in
face of e.g. running out of resources (like hardware encoding).

[kwiberg-webrtc, 2017/06/16 09:57:01]

Yes, I hadn't considered that. Fixed.

[ossu, 2017/06/16 13:36:53]

I think this isn't the wrong place to handle that. At the point
GetSupportedEncoders is called, its too early to say if we'll be able to use
that hardware resource or not. The output from this is already cached by
WebRtcVoiceEngine, for example. Building the list on-demand is probably not much
more expensive than copying it and would allow the application developer to
change the list of codecs dynamically in some specific cases. So... fine, I
guess. :)

[kwiberg-webrtc, 2017/06/16 13:40:08]

Acknowledged.

+      std::vector<AudioCodecSpec> specs;
+      Helper<Ts...>::AppendSupportedEncoders(&specs);
+      return specs;
+    }();
+    return specs;
+  }
+
+  rtc::Optional<AudioCodecInfo> QueryAudioEncoder(
+      const SdpAudioFormat& format) override {
+    return Helper<Ts...>::QueryAudioEncoder(format);
+  }
+
+  std::unique_ptr<AudioEncoder> MakeAudioEncoder(
+      int payload_type,
+      const SdpAudioFormat& format) override {
+    return Helper<Ts...>::MakeAudioEncoder(payload_type, format);
+  }
+};  // namespace audio_encoder_factory_template_impl

[ossu, 2017/06/16 09:11:35]

nit: It's just the class ending here, the namespace ends two lines down. :)

[the sun, 2017/06/16 09:18:06]

bad comment

[kwiberg-webrtc, 2017/06/16 09:57:01]

Done.

+
+}  // namespace audio_encoder_factory_template_impl
+
+// Make an AudioEncoderFactory that can create instances of the given encoders.

[the sun, 2017/06/16 09:18:06]

TODO: Point at builtin_audio_encoder_factory for an example of how it is used.

[kwiberg-webrtc, 2017/06/16 09:57:01]

Done.

+//
+// Each encoder type is given as a template argument to the function; it should
+// be a struct with the following static member functions:
+//
+//   // Converts |audio_format| to a ConfigType instance. Returns an empty
+//   // optional if |audio_format| doesn't correctly specify an encoder of our
+//   // type.
+//   rtc::Optional<ConfigType> SdpToConfig(const SdpAudioFormat& audio_format);
+//
+//   // Appends zero or more AudioCodecSpecs to the list that will be returned
+//   // by AudioEncoderFactory::GetSupportedEncoders().
+//   void AppendSupportedEncoders(std::vector<AudioCodecSpec>* specs);
+//
+//   // Returns information about how this format would be encoded. Used to
+//   // implement AudioEncoderFactory::QueryAudioEncoder().
+//   AudioCodecInfo QueryAudioEncoder(const ConfigType& config);
+//
+//   // Creates an AudioEncoder for the specified format. Used to implement
+//   // AudioEncoderFactory::MakeAudioEncoder().
+//   std::unique_ptr<AudioEncoder> MakeAudioEncoder(const ConfigType& config,
+//                                                  int payload_type);
+//
+// ConfigType should be a type that encapsulates all the settings needed to
+// create an AudioDecoder.

[ossu, 2017/06/16 11:20:45]

I think we should mention that the order of the codecs determines how codecs are
prioritized, with the first being the most preferred one.

[kwiberg-webrtc, 2017/06/16 13:03:24]

Oh, yeah, that's an excellent idea! Done.

+//
+// NOTE: This function is still under development and may change without notice.
+template <typename... Ts>
+rtc::scoped_refptr<AudioEncoderFactory> CreateAudioEncoderFactory() {
+  // There's no technical reason we couldn't allow zero template parameters,
+  // but such a factory couldn't create any encoders, and callers can do this
+  // by mistake by simply forgetting the <> altogether. So we forbid it in
+  // order to prevent caller foot-shooting.
+  static_assert(sizeof...(Ts) >= 1,
+                "Caller must give at least one template parameter");
+
+  return rtc::scoped_refptr<AudioEncoderFactory>(
+      new rtc::RefCountedObject<
+          audio_encoder_factory_template_impl::AudioEncoderFactoryT<Ts...>>());
+}
+
+}  // namespace webrtc
+
+#endif  // WEBRTC_API_AUDIO_CODECS_AUDIO_ENCODER_FACTORY_TEMPLATE_H_