Rename Call::Config to CallConfig, keep old name as alias.

We want api/peerconnectioninterface.h (and corresponding build target)
to not depend on call.h, and generally we treat Call as an internal,
non-api, class. But we need CallFactoryInterface in the api in order to
enable use of PeerConnection with or without support for media.

Making CallConfig a top-level class makes it possible to forward declare
it, together with Call, for use in callfactoryinterface.h and
peerconnectioninterface.h.

Delete the peerconnection_and_implicit_call_api target, replaced by
new target callfactory_api, to link between Call and Peerconnection.

Bug: webrtc:7504
Change-Id: I5e3978ef89bcd6705e94536f8676bcf89fc82fe1
Reviewed-on: https://webrtc-review.googlesource.com/46201
Reviewed-by: Stefan Holmer <stefan@webrtc.org>
Reviewed-by: Patrik Höglund <phoglund@webrtc.org>
Reviewed-by: Taylor Brandstetter <deadbeef@webrtc.org>
Commit-Queue: Niels Moller <nisse@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#22020}

api/BUILD.gn

@@ -37,6 +37,13 @@ rtc_source_set("call_api") {
   ]
 }
 
+rtc_source_set("callfactory_api") {
+  visibility = [ "*" ]
+  sources = [
+    "call/callfactoryinterface.h",
+  ]
+}
+
 rtc_static_library("libjingle_peerconnection_api") {
   visibility = [ "*" ]
   cflags = []
@@ -60,6 +67,7 @@ rtc_static_library("libjingle_peerconnection_api") {
     "mediatypes.h",
     "notifier.h",
     "peerconnectionfactoryproxy.h",
+    "peerconnectioninterface.h",
     "peerconnectionproxy.h",
     "proxy.cc",
     "proxy.h",
@@ -94,8 +102,9 @@ rtc_static_library("libjingle_peerconnection_api") {
     ":array_view",
     ":audio_mixer_api",
     ":audio_options_api",
+    ":callfactory_api",
+    ":libjingle_logging_api",
     ":optional",
-    ":peerconnection_and_implicit_call_api",
     ":rtc_stats_api",
     ":video_frame_api",
     "audio_codecs:audio_codecs_api",
@@ -105,6 +114,7 @@ rtc_static_library("libjingle_peerconnection_api") {
     # file, really. All these should arguably go away in time.
     "..:typedefs",
     "..:webrtc_common",
+    "../logging:rtc_event_log_api",
     "../media:rtc_media_config",
     "../modules/audio_processing:audio_processing_statistics",
     "../rtc_base:checks",
@@ -113,24 +123,17 @@ rtc_static_library("libjingle_peerconnection_api") {
     "../rtc_base:rtc_base_approved",
     "../rtc_base:stringutils",
   ]
+
   if (is_nacl) {
     # This is needed by .h files included from rtc_base.
     deps += [ "//native_client_sdk/src/libraries/nacl_io" ]
   }
 }
 
+# TODO(bugs.webrtc.org/7504): Dummy target, kept temporarily because
+# chromium edpends on it.
 rtc_source_set("peerconnection_and_implicit_call_api") {
   visibility = [ "*" ]
-
-  # The peerconnectioninterface.h file pulls in call/callfactoryinterface.h
-  # and the entire call module with it. We need to either get rid of this
-  # dependency or pull most of call/ into the API. For now, silence the warnings
-  # this creates since it creates a circular dependency (call very much depends
-  # on API). See bugs.webrtc.org/8667.
-  check_includes = false
-  sources = [
-    "peerconnectioninterface.h",
-  ]
 }
 
 rtc_source_set("libjingle_logging_api") {
@@ -375,7 +378,6 @@ if (rtc_include_tests) {
       "fakemetricsobserver.h",
     ]
     deps = [
-      "../api:peerconnection_and_implicit_call_api",
       "../media:rtc_media_base",
       "../rtc_base:checks",
       "../rtc_base:rtc_base_approved",

api/DEPS

@@ -4,6 +4,7 @@ include_rules = [
   "+media",
   "+p2p",
   "+pc",
+  "+logging/rtc_event_log/rtc_event_log_factory_interface.h",
 ]
 
 specific_include_rules = {

api/call/callfactoryinterface.h

@@ -8,15 +8,17 @@
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
-#ifndef CALL_CALLFACTORYINTERFACE_H_
+#ifndef API_CALL_CALLFACTORYINTERFACE_H_
-#define CALL_CALLFACTORYINTERFACE_H_
+#define API_CALL_CALLFACTORYINTERFACE_H_
 
 #include <memory>
 
-#include "call/call.h"
-
 namespace webrtc {
 
+// These classes are not part of the API, and are treated as opaque pointers.
+class Call;
+struct CallConfig;
+
 // This interface exists to allow webrtc to be optionally built without media
 // support (i.e., if only being used for data channels). PeerConnectionFactory
 // is constructed with a CallFactoryInterface, which may or may not be null.
@@ -24,11 +26,11 @@ class CallFactoryInterface {
  public:
   virtual ~CallFactoryInterface() {}
 
-  virtual Call* CreateCall(const Call::Config& config) = 0;
+  virtual Call* CreateCall(const CallConfig& config) = 0;
 };
 
 std::unique_ptr<CallFactoryInterface> CreateCallFactory();
 
 }  // namespace webrtc
 
-#endif  // CALL_CALLFACTORYINTERFACE_H_
+#endif  // API_CALL_CALLFACTORYINTERFACE_H_

api/peerconnectioninterface.h

@@ -80,6 +80,7 @@
 #include "api/audio_codecs/audio_decoder_factory.h"
 #include "api/audio_codecs/audio_encoder_factory.h"
 #include "api/audio_options.h"
+#include "api/call/callfactoryinterface.h"
 #include "api/datachannelinterface.h"
 #include "api/dtmfsenderinterface.h"
 #include "api/jsep.h"
@@ -94,12 +95,19 @@
 #include "api/statstypes.h"
 #include "api/turncustomizer.h"
 #include "api/umametrics.h"
-#include "call/callfactoryinterface.h"
 #include "logging/rtc_event_log/rtc_event_log_factory_interface.h"
 #include "media/base/mediaconfig.h"
-#include "media/base/videocapturer.h"
+// TODO(bugs.webrtc.org/6353): cricket::VideoCapturer is deprecated and should
-#include "p2p/base/portallocator.h"
+// be deleted from the PeerConnection api.
+#include "media/base/videocapturer.h"  // nogncheck
+// TODO(bugs.webrtc.org/7447): We plan to provide a way to let applications
+// inject a PacketSocketFactory and/or NetworkManager, and not expose
+// PortAllocator in the PeerConnection api.
+#include "p2p/base/portallocator.h"  // nogncheck
+// TODO(nisse): The interface for bitrate allocation strategy belongs in api/.
+#include "rtc_base/bitrateallocationstrategy.h"
 #include "rtc_base/network.h"
+#include "rtc_base/platform_file.h"
 #include "rtc_base/rtccertificate.h"
 #include "rtc_base/rtccertificategenerator.h"
 #include "rtc_base/socketaddress.h"
@@ -119,7 +127,7 @@ class WebRtcVideoEncoderFactory;
 namespace webrtc {
 class AudioDeviceModule;
 class AudioMixer;
-class CallFactoryInterface;
+class AudioProcessing;
 class MediaConstraintsInterface;
 class VideoDecoderFactory;
 class VideoEncoderFactory;

call/BUILD.gn

@@ -14,7 +14,6 @@ rtc_source_set("call_interfaces") {
     "audio_send_stream.h",
     "audio_state.h",
     "call.h",
-    "callfactoryinterface.h",
     "flexfec_receive_stream.h",
     "syncable.cc",
     "syncable.h",
@@ -139,6 +138,7 @@ rtc_static_library("call") {
     ":rtp_sender",
     ":video_stream_api",
     "..:webrtc_common",
+    "../api:callfactory_api",
     "../api:optional",
     "../api:transport_api",
     "../audio",

call/call.h

@@ -72,49 +72,51 @@ class PacketReceiver {
   virtual ~PacketReceiver() {}
 };
 
+struct CallConfig {
+  explicit CallConfig(RtcEventLog* event_log) : event_log(event_log) {
+    RTC_DCHECK(event_log);
+  }
+
+  static constexpr int kDefaultStartBitrateBps = 300000;
+
+  // Bitrate config used until valid bitrate estimates are calculated. Also
+  // used to cap total bitrate used. This comes from the remote connection.
+  struct BitrateConfig {
+    int min_bitrate_bps = 0;
+    int start_bitrate_bps = kDefaultStartBitrateBps;
+    int max_bitrate_bps = -1;
+  } bitrate_config;
+
+  // The local client's bitrate preferences. The actual configuration used
+  // is a combination of this and |bitrate_config|. The combination is
+  // currently more complicated than a simple mask operation (see
+  // SetBitrateConfig and SetBitrateConfigMask). Assumes that 0 <= min <=
+  // start <= max holds for set parameters.
+  struct BitrateConfigMask {
+    rtc::Optional<int> min_bitrate_bps;
+    rtc::Optional<int> start_bitrate_bps;
+    rtc::Optional<int> max_bitrate_bps;
+  };
+
+  // AudioState which is possibly shared between multiple calls.
+  // TODO(solenberg): Change this to a shared_ptr once we can use C++11.
+  rtc::scoped_refptr<AudioState> audio_state;
+
+  // Audio Processing Module to be used in this call.
+  // TODO(solenberg): Change this to a shared_ptr once we can use C++11.
+  AudioProcessing* audio_processing = nullptr;
+
+  // RtcEventLog to use for this call. Required.
+  // Use webrtc::RtcEventLog::CreateNull() for a null implementation.
+  RtcEventLog* event_log = nullptr;
+};
+
 // A Call instance can contain several send and/or receive streams. All streams
 // are assumed to have the same remote endpoint and will share bitrate estimates
 // etc.
 class Call {
  public:
-  struct Config {
+  using Config = CallConfig;
-    explicit Config(RtcEventLog* event_log) : event_log(event_log) {
-      RTC_DCHECK(event_log);
-    }
-
-    static constexpr int kDefaultStartBitrateBps = 300000;
-
-    // Bitrate config used until valid bitrate estimates are calculated. Also
-    // used to cap total bitrate used. This comes from the remote connection.
-    struct BitrateConfig {
-      int min_bitrate_bps = 0;
-      int start_bitrate_bps = kDefaultStartBitrateBps;
-      int max_bitrate_bps = -1;
-    } bitrate_config;
-
-    // The local client's bitrate preferences. The actual configuration used
-    // is a combination of this and |bitrate_config|. The combination is
-    // currently more complicated than a simple mask operation (see
-    // SetBitrateConfig and SetBitrateConfigMask). Assumes that 0 <= min <=
-    // start <= max holds for set parameters.
-    struct BitrateConfigMask {
-      rtc::Optional<int> min_bitrate_bps;
-      rtc::Optional<int> start_bitrate_bps;
-      rtc::Optional<int> max_bitrate_bps;
-    };
-
-    // AudioState which is possibly shared between multiple calls.
-    // TODO(solenberg): Change this to a shared_ptr once we can use C++11.
-    rtc::scoped_refptr<AudioState> audio_state;
-
-    // Audio Processing Module to be used in this call.
-    // TODO(solenberg): Change this to a shared_ptr once we can use C++11.
-    AudioProcessing* audio_processing = nullptr;
-
-    // RtcEventLog to use for this call. Required.
-    // Use webrtc::RtcEventLog::CreateNull() for a null implementation.
-    RtcEventLog* event_log = nullptr;
-  };
 
   struct Stats {
     std::string ToString(int64_t time_ms) const;

call/callfactory.cc

@@ -12,6 +12,8 @@
 
 #include <memory>
 
+#include "call/call.h"
+
 namespace webrtc {
 
 Call* CallFactory::CreateCall(const Call::Config& config) {

call/callfactory.h

@@ -11,14 +11,14 @@
 #ifndef CALL_CALLFACTORY_H_
 #define CALL_CALLFACTORY_H_
 
-#include "call/callfactoryinterface.h"
+#include "api/call/callfactoryinterface.h"
 
 namespace webrtc {
 
 class CallFactory : public CallFactoryInterface {
   ~CallFactory() override {}
 
-  Call* CreateCall(const Call::Config& config) override;
+  Call* CreateCall(const CallConfig& config) override;
 };
 
 }  // namespace webrtc

examples/BUILD.gn

@@ -558,8 +558,8 @@ if (is_linux || is_win) {
     configs += [ ":peerconnection_client_warnings_config" ]
 
     deps += [
+      "../api:libjingle_peerconnection_api",
       "../api:libjingle_peerconnection_test_api",
-      "../api:peerconnection_and_implicit_call_api",
       "../api:video_frame_api",
       "../api/audio_codecs:builtin_audio_decoder_factory",
       "../api/audio_codecs:builtin_audio_encoder_factory",
@@ -692,7 +692,6 @@ if (is_win || is_android) {
     deps = [
       "../api:libjingle_peerconnection_api",
       "../api:libjingle_peerconnection_test_api",
-      "../api:peerconnection_and_implicit_call_api",
       "../api:video_frame_api",
       "../api/audio_codecs:builtin_audio_decoder_factory",
       "../api/audio_codecs:builtin_audio_encoder_factory",

pc/BUILD.gn

@@ -182,7 +182,6 @@ rtc_static_library("peerconnection") {
     "../api:call_api",
     "../api:libjingle_peerconnection_api",
     "../api:optional",
-    "../api:peerconnection_and_implicit_call_api",
     "../api:rtc_stats_api",
     "../api/video_codecs:video_codecs_api",
     "../call:call_interfaces",
@@ -216,8 +215,8 @@ rtc_static_library("create_pc_factory") {
 
   deps = [
     "../api:audio_mixer_api",
+    "../api:callfactory_api",
     "../api:libjingle_peerconnection_api",
-    "../api:peerconnection_and_implicit_call_api",
     "../api/audio_codecs:audio_codecs_api",
     "../api/video_codecs:video_codecs_api",
     "../call",
@@ -354,7 +353,6 @@ if (rtc_include_tests) {
       "..:webrtc_common",
       "../api:libjingle_peerconnection_api",
       "../api:libjingle_peerconnection_test_api",
-      "../api:peerconnection_and_implicit_call_api",
       "../api:rtc_stats_api",
       "../call:call_interfaces",
       "../logging:rtc_event_log_api",
@@ -462,7 +460,6 @@ if (rtc_include_tests) {
       ":rtc_pc_base",
       "../api:libjingle_peerconnection_api",
       "../api:mock_rtp",
-      "../api:peerconnection_and_implicit_call_api",
       "../rtc_base:checks",
       "../rtc_base:stringutils",
     ]
@@ -474,6 +471,7 @@ if (rtc_include_tests) {
       ":libjingle_peerconnection",
       ":pc_test_utils",
       "..:webrtc_common",
+      "../api:callfactory_api",
       "../api:fakemetricsobserver",
       "../api:libjingle_peerconnection_test_api",
       "../api:optional",

pc/createpeerconnectionfactory.cc

@@ -8,10 +8,10 @@
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
+#include "api/call/callfactoryinterface.h"
 #include "api/peerconnectioninterface.h"
 #include "api/video_codecs/video_decoder_factory.h"
 #include "api/video_codecs/video_encoder_factory.h"
-#include "call/callfactoryinterface.h"
 #include "logging/rtc_event_log/rtc_event_log_factory_interface.h"
 #include "media/engine/webrtcmediaengine.h"
 #include "modules/audio_device/include/audio_device.h"

pc/peerconnection_media_unittest.cc

@@ -14,7 +14,7 @@
 
 #include <tuple>
 
-#include "call/callfactoryinterface.h"
+#include "api/call/callfactoryinterface.h"
 #include "logging/rtc_event_log/rtc_event_log_factory.h"
 #include "media/base/fakemediaengine.h"
 #include "p2p/base/fakeportallocator.h"

pc/peerconnectioninternal.h

@@ -18,6 +18,7 @@
 #include <vector>
 
 #include "api/peerconnectioninterface.h"
+#include "call/call.h"
 #include "pc/datachannel.h"
 #include "pc/rtptransceiver.h"
 

sdk/BUILD.gn

@@ -340,7 +340,6 @@ if (is_ios || is_mac) {
         ":videotoolbox_objc",
         ":videotracksource_objc",
         "../api:libjingle_peerconnection_api",
-        "../api:peerconnection_and_implicit_call_api",
         "../api:video_frame_api",
         "../api/audio_codecs:builtin_audio_decoder_factory",
         "../api/audio_codecs:builtin_audio_encoder_factory",
@@ -438,7 +437,6 @@ if (is_ios || is_mac) {
         ":native_video",
         ":peerconnectionfactory_base_objc",
         "../api:libjingle_peerconnection_api",
-        "../api:peerconnection_and_implicit_call_api",
         "../rtc_base:rtc_base",
       ]
     }
@@ -569,7 +567,6 @@ if (is_ios || is_mac) {
         ":native_video",
         ":videotracksource_objc",
         "../api:libjingle_peerconnection_api",
-        "../api:peerconnection_and_implicit_call_api",
         "../api:video_frame_api",
         "../api/video_codecs:video_codecs_api",
         "../common_video",

sdk/android/BUILD.gn

@@ -364,6 +364,7 @@ rtc_static_library("media_jni") {
 
   deps = [
     ":base_jni",
+    "../../api:callfactory_api",
     "../../api/video_codecs:video_codecs_api",
     "../../call:call_interfaces",
     "../../logging:rtc_event_log_api",
@@ -506,7 +507,6 @@ rtc_static_library("peerconnection_jni") {
     ":native_api_jni",
     "../..:webrtc_common",
     "../../api:libjingle_peerconnection_api",
-    "../../api:peerconnection_and_implicit_call_api",
     "../../api/video_codecs:video_codecs_api",
     "../../logging:rtc_event_log_api",
     "../../logging:rtc_event_log_impl_base",

sdk/android/src/jni/pc/media.cc

@@ -11,9 +11,9 @@
 
 #include <utility>
 
+#include "api/call/callfactoryinterface.h"
 #include "api/video_codecs/video_decoder_factory.h"
 #include "api/video_codecs/video_encoder_factory.h"
-#include "call/callfactoryinterface.h"
 #include "logging/rtc_event_log/rtc_event_log_factory_interface.h"
 #include "media/engine/webrtcmediaengine.h"
 #include "modules/audio_device/include/audio_device.h"