From 44e57be3c87894d3cb44425567190b961aedc441 Mon Sep 17 00:00:00 2001 From: Olga Sharonova Date: Wed, 20 Dec 2017 13:13:32 +0000 Subject: [PATCH] Revert "Fix circular dependency in BWE code." MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit 08279b5cf5a8412f7e946cc2bb43703e920f5855. Reason for revert: msvc bots breaking, blocking WebRTC rolls. Sample log: FAILED: obj/third_party/webrtc/modules/congestion_controller/estimators/probe_bitrate_estimator.obj ninja -t msvc -e environment.x64 -- E:\b\c\goma_client/gomacc.exe "e:\b\c\win_toolchain\vs_files\1180cb75833ea365097e279efb2d5d7a42dee4b0\vc\tools\msvc\14.11.25503\bin\hostx64\x64/cl.exe" /nologo /showIncludes @obj/third_party/webrtc/modules/congestion_controller/estimators/probe_bitrate_estimator.obj.rsp /c ../../third_party/webrtc/modules/congestion_controller/probe_bitrate_estimator.cc /Foobj/third_party/webrtc/modules/congestion_controller/estimators/probe_bitrate_estimator.obj /Fd"obj/third_party/webrtc/modules/congestion_controller/estimators_cc.pdb" ../../third_party/webrtc/modules/congestion_controller/probe_bitrate_estimator.cc(68): error C2220: warning treated as error - no 'object' file generated ../../third_party/webrtc/modules/congestion_controller/probe_bitrate_estimator.cc(68): warning C4267: 'initializing': conversion from 'size_t' to 'int', possible loss of data [22699/51826] CXX obj/third_party/webrtc/modules/pacing/pacing/packet_router.obj [22700/51826] CXX obj/third_party/webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator/aimd_rate_control.obj [22701/51826] CXX obj/third_party/webrtc/modules/desktop_capture/desktop_capture_generic/dxgi_texture_mapping.obj [22702/51826] CXX obj/third_party/webrtc/modules/congestion_controller/estimators/acknowledged_bitrate_estimator.obj FAILED: obj/third_party/webrtc/modules/congestion_controller/estimators/acknowledged_bitrate_estimator.obj ninja -t msvc -e environment.x64 -- E:\b\c\goma_client/gomacc.exe "e:\b\c\win_toolchain\vs_files\1180cb75833ea365097e279efb2d5d7a42dee4b0\vc\tools\msvc\14.11.25503\bin\hostx64\x64/cl.exe" /nologo /showIncludes @obj/third_party/webrtc/modules/congestion_controller/estimators/acknowledged_bitrate_estimator.obj.rsp /c ../../third_party/webrtc/modules/congestion_controller/acknowledged_bitrate_estimator.cc /Foobj/third_party/webrtc/modules/congestion_controller/estimators/acknowledged_bitrate_estimator.obj /Fd"obj/third_party/webrtc/modules/congestion_controller/estimators_cc.pdb" ../../third_party/webrtc/modules/congestion_controller/acknowledged_bitrate_estimator.cc(41): error C2220: warning treated as error - no 'object' file generated ../../third_party/webrtc/modules/congestion_controller/acknowledged_bitrate_estimator.cc(41): warning C4267: 'argument': conversion from 'size_t' to 'int', possible loss of data https://logs.chromium.org/v/?s=chromium%2Fbb%2Ftryserver.chromium.win%2Fwin-msvc-dbg%2F1780%2F%2B%2Frecipes%2Fsteps%2Fcompile__with_patch_%2F0%2Fstdout Original change's description: > Fix circular dependency in BWE code. > > Bug: webrtc:6828 > Change-Id: I531ee5dea41140f085d82641253fadb9e997a378 > Reviewed-on: https://webrtc-review.googlesource.com/34641 > Reviewed-by: Stefan Holmer > Commit-Queue: Patrik Höglund > Cr-Commit-Position: refs/heads/master@{#21350} TBR=phoglund@webrtc.org,stefan@webrtc.org # Not skipping CQ checks because original CL landed > 1 day ago. Bug: webrtc:6828 Change-Id: I361050942cbd1d2b344b129c8c3a4e7b6e1c02f4 Reviewed-on: https://webrtc-review.googlesource.com/35240 Reviewed-by: Olga Sharonova Commit-Queue: Olga Sharonova Cr-Commit-Position: refs/heads/master@{#21384} --- modules/bitrate_controller/BUILD.gn | 14 ++-- modules/congestion_controller/BUILD.gn | 97 +++++++---------------- modules/remote_bitrate_estimator/BUILD.gn | 2 - rtc_tools/BUILD.gn | 1 - 4 files changed, 32 insertions(+), 82 deletions(-) diff --git a/modules/bitrate_controller/BUILD.gn b/modules/bitrate_controller/BUILD.gn index 09990b16f4..1439f54851 100644 --- a/modules/bitrate_controller/BUILD.gn +++ b/modules/bitrate_controller/BUILD.gn @@ -9,6 +9,11 @@ import("../../webrtc.gni") rtc_static_library("bitrate_controller") { + # TODO(mbonadei): Remove (bugs.webrtc.org/6828) + # Errors on cyclic dependency with: + # congestion_controller:congestion_controller if enabled. + check_includes = false + sources = [ "bitrate_controller_impl.cc", "bitrate_controller_impl.h", @@ -32,18 +37,9 @@ rtc_static_library("bitrate_controller") { } deps = [ - "..:module_api", - "../../logging:rtc_event_log_api", - "../../rtc_base:checks", "../../rtc_base:rtc_base_approved", "../../system_wrappers", - "../../system_wrappers:field_trial_api", - "../../system_wrappers:metrics_api", - "../congestion_controller:delay_based_bwe", - "../pacing", - "../remote_bitrate_estimator:remote_bitrate_estimator", "../rtp_rtcp", - "../rtp_rtcp:rtp_rtcp_format", ] } diff --git a/modules/congestion_controller/BUILD.gn b/modules/congestion_controller/BUILD.gn index a94082a2ba..c014934c0c 100644 --- a/modules/congestion_controller/BUILD.gn +++ b/modules/congestion_controller/BUILD.gn @@ -8,40 +8,55 @@ import("../../webrtc.gni") -config("bwe_test_logging") { - if (rtc_enable_bwe_test_logging) { - defines = [ "BWE_TEST_LOGGING_COMPILE_TIME_ENABLE=1" ] - } else { - defines = [ "BWE_TEST_LOGGING_COMPILE_TIME_ENABLE=0" ] - } -} - rtc_static_library("congestion_controller") { - configs += [ ":bwe_test_logging" ] sources = [ + "acknowledged_bitrate_estimator.cc", + "acknowledged_bitrate_estimator.h", + "bitrate_estimator.cc", + "bitrate_estimator.h", + "delay_based_bwe.cc", + "delay_based_bwe.h", "include/receive_side_congestion_controller.h", "include/send_side_congestion_controller.h", + "median_slope_estimator.cc", + "median_slope_estimator.h", + "probe_bitrate_estimator.cc", + "probe_bitrate_estimator.h", "probe_controller.cc", "probe_controller.h", "receive_side_congestion_controller.cc", "send_side_congestion_controller.cc", "transport_feedback_adapter.cc", "transport_feedback_adapter.h", + "trendline_estimator.cc", + "trendline_estimator.h", ] + if (rtc_enable_bwe_test_logging) { + defines = [ "BWE_TEST_LOGGING_COMPILE_TIME_ENABLE=1" ] + } else { + defines = [ "BWE_TEST_LOGGING_COMPILE_TIME_ENABLE=0" ] + } + + # TODO(jschuh): Bug 1348: fix this warning. + configs += [ "//build/config/compiler:no_size_t_to_int_warning" ] + if (!build_with_chromium && is_clang) { # Suppress warnings from the Chromium Clang plugin (bugs.webrtc.org/163). suppressed_configs += [ "//build/config/clang:find_bad_constructs" ] } deps = [ - ":delay_based_bwe", - ":estimators", "..:module_api", "../..:webrtc_common", + "../../:typedefs", + "../../api:optional", + "../../logging:rtc_event_log_api", "../../rtc_base:checks", "../../rtc_base:rate_limiter", "../../rtc_base:rtc_base", + "../../rtc_base:rtc_base_approved", + "../../rtc_base:rtc_numerics", "../../system_wrappers", "../../system_wrappers:field_trial_api", "../../system_wrappers:metrics_api", @@ -49,66 +64,10 @@ rtc_static_library("congestion_controller") { "../pacing", "../remote_bitrate_estimator", "../rtp_rtcp:rtp_rtcp_format", + "../utility", ] } -rtc_source_set("estimators") { - configs += [ ":bwe_test_logging" ] - sources = [ - "acknowledged_bitrate_estimator.cc", - "acknowledged_bitrate_estimator.h", - "bitrate_estimator.cc", - "bitrate_estimator.h", - "median_slope_estimator.cc", - "median_slope_estimator.h", - "probe_bitrate_estimator.cc", - "probe_bitrate_estimator.h", - "trendline_estimator.cc", - "trendline_estimator.h", - ] - - if (!build_with_chromium && is_clang) { - # Suppress warnings from the Chromium Clang plugin (bugs.webrtc.org/163). - suppressed_configs += [ "//build/config/clang:find_bad_constructs" ] - } - - deps = [ - "../../api:optional", - "../../logging:rtc_event_log_api", - "../../rtc_base:checks", - "../../rtc_base:rtc_base_approved", - "../../rtc_base:rtc_numerics", - "../../system_wrappers:field_trial_api", - "../../system_wrappers:metrics_api", - "../remote_bitrate_estimator:remote_bitrate_estimator", - "../rtp_rtcp:rtp_rtcp_format", - ] -} - -rtc_source_set("delay_based_bwe") { - configs += [ ":bwe_test_logging" ] - sources = [ - "delay_based_bwe.cc", - "delay_based_bwe.h", - ] - deps = [ - ":estimators", - "../../:typedefs", - "../../logging:rtc_event_log_api", - "../../rtc_base:checks", - "../../rtc_base:rtc_base_approved", - "../../system_wrappers:field_trial_api", - "../../system_wrappers:metrics_api", - "../pacing", - "../remote_bitrate_estimator", - ] - - if (!build_with_chromium && is_clang) { - # Suppress warnings from the Chromium Clang plugin (bugs.webrtc.org/163). - suppressed_configs += [ "//build/config/clang:find_bad_constructs" ] - } -} - if (rtc_include_tests) { rtc_source_set("congestion_controller_unittests") { testonly = true @@ -130,8 +89,6 @@ if (rtc_include_tests) { ] deps = [ ":congestion_controller", - ":delay_based_bwe", - ":estimators", ":mock_congestion_controller", "../../rtc_base:checks", "../../rtc_base:rtc_base", diff --git a/modules/remote_bitrate_estimator/BUILD.gn b/modules/remote_bitrate_estimator/BUILD.gn index b2ea7878ba..a5dd014e24 100644 --- a/modules/remote_bitrate_estimator/BUILD.gn +++ b/modules/remote_bitrate_estimator/BUILD.gn @@ -154,8 +154,6 @@ if (rtc_include_tests) { "../../voice_engine", "../bitrate_controller", "../congestion_controller", - "../congestion_controller:delay_based_bwe", - "../congestion_controller:estimators", "../pacing", "../rtp_rtcp", "../rtp_rtcp:rtp_rtcp_format", diff --git a/rtc_tools/BUILD.gn b/rtc_tools/BUILD.gn index 840d221b8b..771d6d90e4 100644 --- a/rtc_tools/BUILD.gn +++ b/rtc_tools/BUILD.gn @@ -231,7 +231,6 @@ if (!build_with_chromium) { "../modules/audio_coding:ana_debug_dump_proto", "../modules/audio_coding:audio_network_adaptor", "../modules/audio_coding:neteq_tools", - "../modules/congestion_controller:estimators", "../modules/rtp_rtcp:rtp_rtcp_format", "../rtc_base:checks", "../rtc_base:rtc_base_approved",