From 1895526c6130e3d0e9b154f95079b8eda7567016 Mon Sep 17 00:00:00 2001 From: Ivo Creusen Date: Wed, 29 Jun 2016 13:56:45 +0200 Subject: [PATCH] Move RtcEventLog object from inside VoiceEngine to Call. In addition to moving the logging object itself, this also moves the interface from PeerConnectionFactory to PeerConnection, which makes more sense for this functionality. An API parameter to set an upper limit to the size of the logfile is introduced. The old interface on PeerConnectionFactory is not removed in this CL, because it is called from Chrome, it will be removed after Chrome is updated to use the PeerConnection interface. BUG=webrtc:4741,webrtc:5603,chromium:609749 R=solenberg@webrtc.org, stefan@webrtc.org, terelius@webrtc.org, tommi@webrtc.org Review URL: https://codereview.webrtc.org/1748403002 . Cr-Commit-Position: refs/heads/master@{#13321} --- .../java/src/org/webrtc/PeerConnection.java | 23 ++++ .../src/org/webrtc/PeerConnectionFactory.java | 28 ----- webrtc/api/peerconnection.cc | 22 ++++ webrtc/api/peerconnection.h | 11 ++ webrtc/api/peerconnectionfactory.cc | 11 -- webrtc/api/peerconnectionfactory.h | 13 ++- webrtc/api/peerconnectionfactoryproxy.h | 2 + webrtc/api/peerconnectioninterface.h | 35 +++--- webrtc/api/peerconnectionproxy.h | 2 + webrtc/audio/DEPS | 9 ++ webrtc/audio/audio_receive_stream.cc | 5 +- webrtc/audio/audio_receive_stream.h | 4 +- webrtc/audio/audio_receive_stream_unittest.cc | 27 ++++- webrtc/audio/audio_send_stream_unittest.cc | 5 +- webrtc/call.h | 5 + webrtc/call/call.cc | 39 ++++--- webrtc/call/rtc_event_log.cc | 10 +- webrtc/call/rtc_event_log.h | 3 + webrtc/media/base/mediaengine.h | 16 --- webrtc/media/engine/fakewebrtccall.cc | 7 ++ webrtc/media/engine/fakewebrtccall.h | 4 + webrtc/media/engine/fakewebrtcvoiceengine.h | 1 - webrtc/media/engine/webrtcvoiceengine.cc | 22 ---- webrtc/media/engine/webrtcvoiceengine.h | 8 -- .../bitrate_controller_impl.cc | 24 ++-- .../bitrate_controller_impl.h | 7 +- .../bitrate_controller_unittest.cc | 15 ++- .../include/bitrate_controller.h | 9 +- .../send_side_bandwidth_estimation.cc | 27 ++--- .../send_side_bandwidth_estimation.h | 5 +- ...send_side_bandwidth_estimation_unittest.cc | 10 +- webrtc/modules/congestion_controller/DEPS | 1 + .../congestion_controller.cc | 32 ++---- .../congestion_controller_unittest.cc | 8 +- .../include/congestion_controller.h | 15 +-- .../include/mock/mock_congestion_controller.h | 8 +- webrtc/modules/remote_bitrate_estimator/DEPS | 6 + .../test/estimators/remb.cc | 4 +- .../test/estimators/remb.h | 2 + .../test/estimators/send_side.cc | 4 +- .../test/estimators/send_side.h | 2 + webrtc/pc/channelmanager.cc | 13 --- webrtc/pc/channelmanager.h | 6 - webrtc/test/mock_voe_channel_proxy.h | 1 + webrtc/test/mock_voice_engine.h | 1 - webrtc/voice_engine/channel.cc | 106 +++++++++++++++--- webrtc/voice_engine/channel.h | 9 +- webrtc/voice_engine/channel_manager.cc | 13 +-- webrtc/voice_engine/channel_manager.h | 5 - webrtc/voice_engine/channel_proxy.cc | 5 + webrtc/voice_engine/channel_proxy.h | 3 + webrtc/voice_engine/include/voe_codec.h | 5 - .../test/auto_test/standard/codec_test.cc | 24 ---- .../test/cmd_test/voe_cmd_test.cc | 5 - webrtc/voice_engine/voe_codec_impl.cc | 4 - webrtc/voice_engine/voe_codec_impl.h | 2 - 56 files changed, 380 insertions(+), 313 deletions(-) diff --git a/webrtc/api/java/src/org/webrtc/PeerConnection.java b/webrtc/api/java/src/org/webrtc/PeerConnection.java index ad8362d485..3808eb4893 100644 --- a/webrtc/api/java/src/org/webrtc/PeerConnection.java +++ b/webrtc/api/java/src/org/webrtc/PeerConnection.java @@ -253,6 +253,23 @@ public class PeerConnection { return nativeGetStats(observer, (track == null) ? 0 : track.nativeTrack); } + // Starts recording an RTC event log. Ownership of the file is transfered to + // the native code. If an RTC event log is already being recorded, it will be + // stopped and a new one will start using the provided file. Logging will + // continue until the stopRtcEventLog function is called. The max_size_bytes + // argument is ignored, it is added for future use. + public boolean startRtcEventLog( + int file_descriptor, long max_size_bytes) { + return nativeStartRtcEventLog( + nativePeerConnection, file_descriptor, max_size_bytes); + } + + // Stops recording an RTC event log. If no RTC event log is currently being + // recorded, this call will have no effect. + public void stopRtcEventLog() { + nativeStopRtcEventLog(nativePeerConnection); + } + // TODO(fischman): add support for DTMF-related methods once that API // stabilizes. public native SignalingState signalingState(); @@ -303,4 +320,10 @@ public class PeerConnection { private native List nativeGetSenders(); private native List nativeGetReceivers(); + + private static native boolean nativeStartRtcEventLog( + long nativePeerConnection, int file_descriptor, long max_size_bytes); + + private static native void nativeStopRtcEventLog(long nativePeerConnection); + } diff --git a/webrtc/api/java/src/org/webrtc/PeerConnectionFactory.java b/webrtc/api/java/src/org/webrtc/PeerConnectionFactory.java index 0c1ef3ca88..2edd56e841 100644 --- a/webrtc/api/java/src/org/webrtc/PeerConnectionFactory.java +++ b/webrtc/api/java/src/org/webrtc/PeerConnectionFactory.java @@ -148,28 +148,6 @@ public class PeerConnectionFactory { nativeStopAecDump(nativeFactory); } - // Starts recording an RTC event log. Ownership of the file is transfered to - // the native code. If an RTC event log is already being recorded, it will be - // stopped and a new one will start using the provided file. - public boolean startRtcEventLog(int file_descriptor) { - return startRtcEventLog(file_descriptor, -1); - } - - // Same as above, but allows setting an upper limit to the size of the - // generated logfile. - public boolean startRtcEventLog(int file_descriptor, - int filesize_limit_bytes) { - return nativeStartRtcEventLog(nativeFactory, - file_descriptor, - filesize_limit_bytes); - } - - // Stops recording an RTC event log. If no RTC event log is currently being - // recorded, this call will have no effect. - public void stopRtcEventLog() { - nativeStopRtcEventLog(nativeFactory); - } - @Deprecated public void setOptions(Options options) { nativeSetOptions(nativeFactory, options); @@ -275,12 +253,6 @@ public class PeerConnectionFactory { private static native void nativeStopAecDump(long nativeFactory); - private static native boolean nativeStartRtcEventLog(long nativeFactory, - int file_descriptor, - int filesize_limit_bytes); - - private static native void nativeStopRtcEventLog(long nativeFactory); - @Deprecated public native void nativeSetOptions(long nativeFactory, Options options); diff --git a/webrtc/api/peerconnection.cc b/webrtc/api/peerconnection.cc index e0b4fca48d..a337d003dc 100644 --- a/webrtc/api/peerconnection.cc +++ b/webrtc/api/peerconnection.cc @@ -31,10 +31,12 @@ #include "webrtc/api/videocapturertracksource.h" #include "webrtc/api/videotrack.h" #include "webrtc/base/arraysize.h" +#include "webrtc/base/bind.h" #include "webrtc/base/logging.h" #include "webrtc/base/stringencode.h" #include "webrtc/base/stringutils.h" #include "webrtc/base/trace_event.h" +#include "webrtc/call.h" #include "webrtc/media/sctp/sctpdataengine.h" #include "webrtc/pc/channelmanager.h" #include "webrtc/system_wrappers/include/field_trial.h" @@ -1263,6 +1265,18 @@ void PeerConnection::RegisterUMAObserver(UMAObserver* observer) { } } +bool PeerConnection::StartRtcEventLog(rtc::PlatformFile file, + int64_t max_size_bytes) { + return factory_->worker_thread()->Invoke( + RTC_FROM_HERE, rtc::Bind(&PeerConnection::StartRtcEventLog_w, this, file, + max_size_bytes)); +} + +void PeerConnection::StopRtcEventLog() { + factory_->worker_thread()->Invoke( + RTC_FROM_HERE, rtc::Bind(&PeerConnection::StopRtcEventLog_w, this)); +} + const SessionDescriptionInterface* PeerConnection::local_description() const { return session_->local_description(); } @@ -2232,4 +2246,12 @@ bool PeerConnection::ReconfigurePortAllocator_n( return true; } +bool PeerConnection::StartRtcEventLog_w(rtc::PlatformFile file, + int64_t max_size_bytes) { + return media_controller_->call_w()->StartEventLog(file, max_size_bytes); +} + +void PeerConnection::StopRtcEventLog_w() { + media_controller_->call_w()->StopEventLog(); +} } // namespace webrtc diff --git a/webrtc/api/peerconnection.h b/webrtc/api/peerconnection.h index fc754b9c83..c4a9a60053 100644 --- a/webrtc/api/peerconnection.h +++ b/webrtc/api/peerconnection.h @@ -136,6 +136,10 @@ class PeerConnection : public PeerConnectionInterface, void RegisterUMAObserver(UMAObserver* observer) override; + bool StartRtcEventLog(rtc::PlatformFile file, + int64_t max_size_bytes) override; + void StopRtcEventLog() override; + void Close() override; // Virtual for unit tests. @@ -360,6 +364,13 @@ class PeerConnection : public PeerConnectionInterface, // is applied. bool ReconfigurePortAllocator_n(const RTCConfiguration& configuration); + // Starts recording an Rtc EventLog using the supplied platform file. + // This function should only be called from the worker thread. + bool StartRtcEventLog_w(rtc::PlatformFile file, int64_t max_size_bytes); + // Starts recording an Rtc EventLog using the supplied platform file. + // This function should only be called from the worker thread. + void StopRtcEventLog_w(); + // Storing the factory as a scoped reference pointer ensures that the memory // in the PeerConnectionFactoryImpl remains available as long as the // PeerConnection is running. It is passed to PeerConnection as a raw pointer. diff --git a/webrtc/api/peerconnectionfactory.cc b/webrtc/api/peerconnectionfactory.cc index 745de3f214..b79e7a2544 100644 --- a/webrtc/api/peerconnectionfactory.cc +++ b/webrtc/api/peerconnectionfactory.cc @@ -220,17 +220,6 @@ void PeerConnectionFactory::StopAecDump() { channel_manager_->StopAecDump(); } -bool PeerConnectionFactory::StartRtcEventLog(rtc::PlatformFile file, - int64_t max_size_bytes) { - RTC_DCHECK(signaling_thread_->IsCurrent()); - return channel_manager_->StartRtcEventLog(file, max_size_bytes); -} - -void PeerConnectionFactory::StopRtcEventLog() { - RTC_DCHECK(signaling_thread_->IsCurrent()); - channel_manager_->StopRtcEventLog(); -} - rtc::scoped_refptr PeerConnectionFactory::CreatePeerConnection( const PeerConnectionInterface::RTCConfiguration& configuration_in, diff --git a/webrtc/api/peerconnectionfactory.h b/webrtc/api/peerconnectionfactory.h index d4bc0da62b..39a64027b6 100644 --- a/webrtc/api/peerconnectionfactory.h +++ b/webrtc/api/peerconnectionfactory.h @@ -80,12 +80,15 @@ class PeerConnectionFactory : public PeerConnectionFactoryInterface { bool StartAecDump(rtc::PlatformFile file, int64_t max_size_bytes) override; void StopAecDump() override; - bool StartRtcEventLog(rtc::PlatformFile file) override { - return StartRtcEventLog(file, -1); - } + // TODO(ivoc) Remove after Chrome is updated. + bool StartRtcEventLog(rtc::PlatformFile file) override { return false; } + // TODO(ivoc) Remove after Chrome is updated. bool StartRtcEventLog(rtc::PlatformFile file, - int64_t max_size_bytes) override; - void StopRtcEventLog() override; + int64_t max_size_bytes) override { + return false; + } + // TODO(ivoc) Remove after Chrome is updated. + void StopRtcEventLog() override {} virtual webrtc::MediaControllerInterface* CreateMediaController( const cricket::MediaConfig& config) const; diff --git a/webrtc/api/peerconnectionfactoryproxy.h b/webrtc/api/peerconnectionfactoryproxy.h index aad97e82d4..227a685b7e 100644 --- a/webrtc/api/peerconnectionfactoryproxy.h +++ b/webrtc/api/peerconnectionfactoryproxy.h @@ -70,6 +70,8 @@ BEGIN_SIGNALING_PROXY_MAP(PeerConnectionFactory) CreateAudioTrack, const std::string&, AudioSourceInterface*) PROXY_METHOD2(bool, StartAecDump, rtc::PlatformFile, int64_t) PROXY_METHOD0(void, StopAecDump) + // TODO(ivoc): Remove the StartRtcEventLog and StopRtcEventLog functions as + // soon as they are removed from PeerConnectionFactoryInterface. PROXY_METHOD1(bool, StartRtcEventLog, rtc::PlatformFile) PROXY_METHOD2(bool, StartRtcEventLog, rtc::PlatformFile, int64_t) PROXY_METHOD0(void, StopRtcEventLog) diff --git a/webrtc/api/peerconnectioninterface.h b/webrtc/api/peerconnectioninterface.h index 0ac37e1564..e57435559e 100644 --- a/webrtc/api/peerconnectioninterface.h +++ b/webrtc/api/peerconnectioninterface.h @@ -489,6 +489,17 @@ class PeerConnectionInterface : public rtc::RefCountInterface { virtual IceConnectionState ice_connection_state() = 0; virtual IceGatheringState ice_gathering_state() = 0; + // Starts RtcEventLog using existing file. Takes ownership of |file| and + // passes it on to Call, which will take the ownership. If the + // operation fails the file will be closed. The logging will stop + // automatically after 10 minutes have passed, or when the StopRtcEventLog + // function is called. + virtual bool StartRtcEventLog(rtc::PlatformFile file, + int64_t max_size_bytes) = 0; + + // Stops logging the RtcEventLog. + virtual void StopRtcEventLog() = 0; + // Terminates all media and closes the transport. virtual void Close() = 0; @@ -655,25 +666,19 @@ class PeerConnectionFactoryInterface : public rtc::RefCountInterface { // Stops logging the AEC dump. virtual void StopAecDump() = 0; - // Starts RtcEventLog using existing file. Takes ownership of |file| and - // passes it on to VoiceEngine, which will take the ownership. If the - // operation fails the file will be closed. The logging will stop - // automatically after 10 minutes have passed, or when the StopRtcEventLog - // function is called. A maximum filesize in bytes can be set, the logging - // will be stopped before exceeding this limit. If max_size_bytes is set to a - // value <= 0, no limit will be used. - // This function as well as the StopRtcEventLog don't really belong on this - // interface, this is a temporary solution until we move the logging object - // from inside voice engine to webrtc::Call, which will happen when the VoE - // restructuring effort is further along. - // TODO(ivoc): Move this into being: - // PeerConnection => MediaController => webrtc::Call. + // This function is deprecated and will be removed when Chrome is updated to + // use the equivalent function on PeerConnectionInterface. + // TODO(ivoc) Remove after Chrome is updated. virtual bool StartRtcEventLog(rtc::PlatformFile file, int64_t max_size_bytes) = 0; - // Deprecated, use the version above. + // This function is deprecated and will be removed when Chrome is updated to + // use the equivalent function on PeerConnectionInterface. + // TODO(ivoc) Remove after Chrome is updated. virtual bool StartRtcEventLog(rtc::PlatformFile file) = 0; - // Stops logging the RtcEventLog. + // This function is deprecated and will be removed when Chrome is updated to + // use the equivalent function on PeerConnectionInterface. + // TODO(ivoc) Remove after Chrome is updated. virtual void StopRtcEventLog() = 0; protected: diff --git a/webrtc/api/peerconnectionproxy.h b/webrtc/api/peerconnectionproxy.h index d35d5bacde..37f2e89f9b 100644 --- a/webrtc/api/peerconnectionproxy.h +++ b/webrtc/api/peerconnectionproxy.h @@ -74,6 +74,8 @@ BEGIN_SIGNALING_PROXY_MAP(PeerConnection) PROXY_METHOD0(IceState, ice_state) PROXY_METHOD0(IceConnectionState, ice_connection_state) PROXY_METHOD0(IceGatheringState, ice_gathering_state) + PROXY_METHOD2(bool, StartRtcEventLog, rtc::PlatformFile, int64_t) + PROXY_METHOD0(void, StopRtcEventLog) PROXY_METHOD0(void, Close) END_SIGNALING_PROXY() diff --git a/webrtc/audio/DEPS b/webrtc/audio/DEPS index b9605bb611..82c3165366 100644 --- a/webrtc/audio/DEPS +++ b/webrtc/audio/DEPS @@ -9,3 +9,12 @@ include_rules = [ "+webrtc/modules/rtp_rtcp", "+webrtc/system_wrappers", ] + +specific_include_rules = { + "audio_receive_stream_unittest\.cc": [ + "+webrtc/call/mock", + ], + "audio_send_stream_unittest\.cc": [ + "+webrtc/call/mock", + ], +} diff --git a/webrtc/audio/audio_receive_stream.cc b/webrtc/audio/audio_receive_stream.cc index a684003326..0a2bc2b4c6 100644 --- a/webrtc/audio/audio_receive_stream.cc +++ b/webrtc/audio/audio_receive_stream.cc @@ -81,7 +81,8 @@ namespace internal { AudioReceiveStream::AudioReceiveStream( CongestionController* congestion_controller, const webrtc::AudioReceiveStream::Config& config, - const rtc::scoped_refptr& audio_state) + const rtc::scoped_refptr& audio_state, + webrtc::RtcEventLog* event_log) : config_(config), audio_state_(audio_state), rtp_header_parser_(RtpHeaderParser::Create()) { @@ -93,6 +94,7 @@ AudioReceiveStream::AudioReceiveStream( VoiceEngineImpl* voe_impl = static_cast(voice_engine()); channel_proxy_ = voe_impl->GetChannelProxy(config_.voe_channel_id); + channel_proxy_->SetRtcEventLog(event_log); channel_proxy_->SetLocalSSRC(config.rtp.local_ssrc); // TODO(solenberg): Config NACK history window (which is a packet count), // using the actual packet size for the configured codec. @@ -144,6 +146,7 @@ AudioReceiveStream::~AudioReceiveStream() { LOG(LS_INFO) << "~AudioReceiveStream: " << config_.ToString(); channel_proxy_->DeRegisterExternalTransport(); channel_proxy_->ResetCongestionControlObjects(); + channel_proxy_->SetRtcEventLog(nullptr); if (remote_bitrate_estimator_) { remote_bitrate_estimator_->RemoveStream(config_.rtp.remote_ssrc); } diff --git a/webrtc/audio/audio_receive_stream.h b/webrtc/audio/audio_receive_stream.h index 284f98e42d..24924c9a6d 100644 --- a/webrtc/audio/audio_receive_stream.h +++ b/webrtc/audio/audio_receive_stream.h @@ -22,6 +22,7 @@ namespace webrtc { class CongestionController; class RemoteBitrateEstimator; +class RtcEventLog; namespace voe { class ChannelProxy; @@ -33,7 +34,8 @@ class AudioReceiveStream final : public webrtc::AudioReceiveStream { public: AudioReceiveStream(CongestionController* congestion_controller, const webrtc::AudioReceiveStream::Config& config, - const rtc::scoped_refptr& audio_state); + const rtc::scoped_refptr& audio_state, + webrtc::RtcEventLog* event_log); ~AudioReceiveStream() override; // webrtc::AudioReceiveStream implementation. diff --git a/webrtc/audio/audio_receive_stream_unittest.cc b/webrtc/audio/audio_receive_stream_unittest.cc index aed1d1ad20..dd66cc67d8 100644 --- a/webrtc/audio/audio_receive_stream_unittest.cc +++ b/webrtc/audio/audio_receive_stream_unittest.cc @@ -15,6 +15,7 @@ #include "webrtc/audio/audio_receive_stream.h" #include "webrtc/audio/conversion.h" +#include "webrtc/call/mock/mock_rtc_event_log.h" #include "webrtc/modules/audio_coding/codecs/mock/mock_audio_decoder_factory.h" #include "webrtc/modules/bitrate_controller/include/mock/mock_bitrate_controller.h" #include "webrtc/modules/congestion_controller/include/mock/mock_congestion_controller.h" @@ -70,7 +71,8 @@ struct ConfigHelper { decoder_factory_(new rtc::RefCountedObject), congestion_controller_(&simulated_clock_, &bitrate_observer_, - &remote_bitrate_observer_) { + &remote_bitrate_observer_, + &event_log_) { using testing::Invoke; EXPECT_CALL(voice_engine_, @@ -109,6 +111,12 @@ struct ConfigHelper { .Times(1); EXPECT_CALL(*channel_proxy_, GetAudioDecoderFactory()) .WillOnce(ReturnRef(decoder_factory_)); + testing::Expectation expect_set = + EXPECT_CALL(*channel_proxy_, SetRtcEventLog(&event_log_)) + .Times(1); + EXPECT_CALL(*channel_proxy_, SetRtcEventLog(testing::IsNull())) + .Times(1) + .After(expect_set); return channel_proxy_; })); stream_config_.voe_channel_id = kChannelId; @@ -130,6 +138,7 @@ struct ConfigHelper { MockRemoteBitrateEstimator* remote_bitrate_estimator() { return &remote_bitrate_estimator_; } + MockRtcEventLog* event_log() { return &event_log_; } AudioReceiveStream::Config& config() { return stream_config_; } rtc::scoped_refptr audio_state() { return audio_state_; } MockVoiceEngine& voice_engine() { return voice_engine_; } @@ -171,6 +180,7 @@ struct ConfigHelper { rtc::scoped_refptr decoder_factory_; MockCongestionController congestion_controller_; MockRemoteBitrateEstimator remote_bitrate_estimator_; + MockRtcEventLog event_log_; testing::StrictMock voice_engine_; rtc::scoped_refptr audio_state_; AudioReceiveStream::Config stream_config_; @@ -248,7 +258,8 @@ TEST(AudioReceiveStreamTest, ConfigToString) { TEST(AudioReceiveStreamTest, ConstructDestruct) { ConfigHelper helper; internal::AudioReceiveStream recv_stream( - helper.congestion_controller(), helper.config(), helper.audio_state()); + helper.congestion_controller(), helper.config(), helper.audio_state(), + helper.event_log()); } MATCHER_P(VerifyHeaderExtension, expected_extension, "") { @@ -267,7 +278,8 @@ TEST(AudioReceiveStreamTest, ReceiveRtpPacket) { helper.config().rtp.transport_cc = true; helper.SetupMockForBweFeedback(true); internal::AudioReceiveStream recv_stream( - helper.congestion_controller(), helper.config(), helper.audio_state()); + helper.congestion_controller(), helper.config(), helper.audio_state(), + helper.event_log()); const int kTransportSequenceNumberValue = 1234; std::vector rtp_packet = CreateRtpHeaderWithOneByteExtension( kTransportSequenceNumberId, kTransportSequenceNumberValue, 2); @@ -295,7 +307,8 @@ TEST(AudioReceiveStreamTest, ReceiveRtcpPacket) { helper.config().rtp.transport_cc = true; helper.SetupMockForBweFeedback(true); internal::AudioReceiveStream recv_stream( - helper.congestion_controller(), helper.config(), helper.audio_state()); + helper.congestion_controller(), helper.config(), helper.audio_state(), + helper.event_log()); std::vector rtcp_packet = CreateRtcpSenderReport(); EXPECT_CALL(*helper.channel_proxy(), @@ -307,7 +320,8 @@ TEST(AudioReceiveStreamTest, ReceiveRtcpPacket) { TEST(AudioReceiveStreamTest, GetStats) { ConfigHelper helper; internal::AudioReceiveStream recv_stream( - helper.congestion_controller(), helper.config(), helper.audio_state()); + helper.congestion_controller(), helper.config(), helper.audio_state(), + helper.event_log()); helper.SetupMockForGetStats(); AudioReceiveStream::Stats stats = recv_stream.GetStats(); EXPECT_EQ(kRemoteSsrc, stats.remote_ssrc); @@ -349,7 +363,8 @@ TEST(AudioReceiveStreamTest, GetStats) { TEST(AudioReceiveStreamTest, SetGain) { ConfigHelper helper; internal::AudioReceiveStream recv_stream( - helper.congestion_controller(), helper.config(), helper.audio_state()); + helper.congestion_controller(), helper.config(), helper.audio_state(), + helper.event_log()); EXPECT_CALL(*helper.channel_proxy(), SetChannelOutputVolumeScaling(FloatEq(0.765f))); recv_stream.SetGain(0.765f); diff --git a/webrtc/audio/audio_send_stream_unittest.cc b/webrtc/audio/audio_send_stream_unittest.cc index b9535543e2..ea70d304d7 100644 --- a/webrtc/audio/audio_send_stream_unittest.cc +++ b/webrtc/audio/audio_send_stream_unittest.cc @@ -17,6 +17,7 @@ #include "webrtc/audio/audio_state.h" #include "webrtc/audio/conversion.h" #include "webrtc/modules/congestion_controller/include/mock/mock_congestion_controller.h" +#include "webrtc/call/mock/mock_rtc_event_log.h" #include "webrtc/modules/congestion_controller/include/congestion_controller.h" #include "webrtc/modules/pacing/paced_sender.h" #include "webrtc/modules/remote_bitrate_estimator/include/mock/mock_remote_bitrate_estimator.h" @@ -55,7 +56,8 @@ struct ConfigHelper { stream_config_(nullptr), congestion_controller_(&simulated_clock_, &bitrate_observer_, - &remote_bitrate_observer_) { + &remote_bitrate_observer_, + &event_log_) { using testing::Invoke; using testing::StrEq; @@ -167,6 +169,7 @@ struct ConfigHelper { testing::NiceMock bitrate_observer_; testing::NiceMock remote_bitrate_observer_; CongestionController congestion_controller_; + MockRtcEventLog event_log_; }; } // namespace diff --git a/webrtc/call.h b/webrtc/call.h index f89af93b51..ff20a1ec1e 100644 --- a/webrtc/call.h +++ b/webrtc/call.h @@ -18,6 +18,7 @@ #include "webrtc/audio_send_stream.h" #include "webrtc/audio_state.h" #include "webrtc/base/networkroute.h" +#include "webrtc/base/platform_file.h" #include "webrtc/base/socket.h" #include "webrtc/video_receive_stream.h" #include "webrtc/video_send_stream.h" @@ -147,6 +148,10 @@ class Call { virtual void OnSentPacket(const rtc::SentPacket& sent_packet) = 0; + virtual bool StartEventLog(rtc::PlatformFile log_file, + int64_t max_size_bytes) = 0; + virtual void StopEventLog() = 0; + virtual ~Call() {} }; diff --git a/webrtc/call/call.cc b/webrtc/call/call.cc index a6a7978941..cbf9da7494 100644 --- a/webrtc/call/call.cc +++ b/webrtc/call/call.cc @@ -35,6 +35,7 @@ #include "webrtc/modules/rtp_rtcp/include/rtp_header_parser.h" #include "webrtc/modules/rtp_rtcp/source/byte_io.h" #include "webrtc/modules/utility/include/process_thread.h" +#include "webrtc/system_wrappers/include/clock.h" #include "webrtc/system_wrappers/include/cpu_info.h" #include "webrtc/system_wrappers/include/critical_section_wrapper.h" #include "webrtc/system_wrappers/include/metrics.h" @@ -107,6 +108,13 @@ class Call : public webrtc::Call, void OnAllocationLimitsChanged(uint32_t min_send_bitrate_bps, uint32_t max_padding_bitrate_bps) override; + bool StartEventLog(rtc::PlatformFile log_file, + int64_t max_size_bytes) override { + return event_log_->StartLogging(log_file, max_size_bytes); + } + + void StopEventLog() override { event_log_->StopLogging(); } + private: DeliveryStatus DeliverRtcp(MediaType media_type, const uint8_t* packet, size_t length); @@ -162,7 +170,7 @@ class Call : public webrtc::Call, VideoSendStream::RtpStateMap suspended_video_send_ssrcs_; - RtcEventLog* event_log_ = nullptr; + std::unique_ptr event_log_; // The following members are only accessed (exclusively) from one thread and // from the destructor, and therefore doesn't need any explicit @@ -210,6 +218,7 @@ Call::Call(const Call::Config& config) video_network_state_(kNetworkUp), receive_crit_(RWLockWrapper::CreateRWLock()), send_crit_(RWLockWrapper::CreateRWLock()), + event_log_(RtcEventLog::Create(webrtc::Clock::GetRealTimeClock())), received_video_bytes_(0), received_audio_bytes_(0), received_rtcp_bytes_(0), @@ -221,7 +230,8 @@ Call::Call(const Call::Config& config) min_allocated_send_bitrate_bps_(0), num_bitrate_updates_(0), remb_(clock_), - congestion_controller_(new CongestionController(clock_, this, &remb_)), + congestion_controller_( + new CongestionController(clock_, this, &remb_, event_log_.get())), video_send_delay_stats_(new SendDelayStats(clock_)) { RTC_DCHECK(configuration_thread_checker_.CalledOnValidThread()); RTC_DCHECK_GE(config.bitrate_config.min_bitrate_bps, 0); @@ -231,10 +241,6 @@ Call::Call(const Call::Config& config) RTC_DCHECK_GE(config.bitrate_config.max_bitrate_bps, config.bitrate_config.start_bitrate_bps); } - if (config.audio_state.get()) { - ScopedVoEInterface voe_codec(voice_engine()); - event_log_ = voe_codec->GetEventLog(); - } Trace::CreateTrace(); call_stats_->RegisterStatsObserver(congestion_controller_.get()); @@ -243,7 +249,6 @@ Call::Call(const Call::Config& config) config_.bitrate_config.min_bitrate_bps, config_.bitrate_config.start_bitrate_bps, config_.bitrate_config.max_bitrate_bps); - congestion_controller_->GetBitrateController()->SetEventLog(event_log_); module_process_thread_->Start(); module_process_thread_->RegisterModule(call_stats_.get()); @@ -371,8 +376,9 @@ webrtc::AudioReceiveStream* Call::CreateAudioReceiveStream( const webrtc::AudioReceiveStream::Config& config) { TRACE_EVENT0("webrtc", "Call::CreateAudioReceiveStream"); RTC_DCHECK(configuration_thread_checker_.CalledOnValidThread()); - AudioReceiveStream* receive_stream = new AudioReceiveStream( - congestion_controller_.get(), config, config_.audio_state); + AudioReceiveStream* receive_stream = + new AudioReceiveStream(congestion_controller_.get(), config, + config_.audio_state, event_log_.get()); { WriteLockScoped write_lock(*receive_crit_); RTC_DCHECK(audio_receive_ssrcs_.find(config.rtp.remote_ssrc) == @@ -421,8 +427,8 @@ webrtc::VideoSendStream* Call::CreateVideoSendStream( VideoSendStream* send_stream = new VideoSendStream( num_cpu_cores_, module_process_thread_.get(), call_stats_.get(), congestion_controller_.get(), bitrate_allocator_.get(), - video_send_delay_stats_.get(), &remb_, event_log_, config, encoder_config, - suspended_video_send_ssrcs_); + video_send_delay_stats_.get(), &remb_, event_log_.get(), config, + encoder_config, suspended_video_send_ssrcs_); { WriteLockScoped write_lock(*send_crit_); for (uint32_t ssrc : config.rtp.ssrcs) { @@ -433,8 +439,7 @@ webrtc::VideoSendStream* Call::CreateVideoSendStream( } send_stream->SignalNetworkState(video_network_state_); UpdateAggregateNetworkState(); - if (event_log_) - event_log_->LogVideoSendStreamConfig(config); + event_log_->LogVideoSendStreamConfig(config); return send_stream; } @@ -493,13 +498,11 @@ webrtc::VideoReceiveStream* Call::CreateVideoReceiveStream( if (it != config.rtp.rtx.end()) video_receive_ssrcs_[it->second.ssrc] = receive_stream; video_receive_streams_.insert(receive_stream); - ConfigureSync(config.sync_group); } receive_stream->SignalNetworkState(video_network_state_); UpdateAggregateNetworkState(); - if (event_log_) - event_log_->LogVideoReceiveStreamConfig(config); + event_log_->LogVideoReceiveStreamConfig(config); return receive_stream; } @@ -827,7 +830,7 @@ PacketReceiver::DeliveryStatus Call::DeliverRtp(MediaType media_type, auto status = it->second->DeliverRtp(packet, length, packet_time) ? DELIVERY_OK : DELIVERY_PACKET_ERROR; - if (status == DELIVERY_OK && event_log_) + if (status == DELIVERY_OK) event_log_->LogRtpHeader(kIncomingPacket, media_type, packet, length); return status; } @@ -839,7 +842,7 @@ PacketReceiver::DeliveryStatus Call::DeliverRtp(MediaType media_type, auto status = it->second->DeliverRtp(packet, length, packet_time) ? DELIVERY_OK : DELIVERY_PACKET_ERROR; - if (status == DELIVERY_OK && event_log_) + if (status == DELIVERY_OK) event_log_->LogRtpHeader(kIncomingPacket, media_type, packet, length); return status; } diff --git a/webrtc/call/rtc_event_log.cc b/webrtc/call/rtc_event_log.cc index 7ea1e167a1..840b210d15 100644 --- a/webrtc/call/rtc_event_log.cc +++ b/webrtc/call/rtc_event_log.cc @@ -38,9 +38,7 @@ namespace webrtc { -#ifndef ENABLE_RTC_EVENT_LOG - -// No-op implementation if flag is not set. +// No-op implementation is used if flag is not set, or in tests. class RtcEventLogNullImpl final : public RtcEventLog { public: bool StartLogging(const std::string& file_name, @@ -74,7 +72,7 @@ class RtcEventLogNullImpl final : public RtcEventLog { int32_t total_packets) override {} }; -#else // ENABLE_RTC_EVENT_LOG is defined +#ifdef ENABLE_RTC_EVENT_LOG class RtcEventLogImpl final : public RtcEventLog { public: @@ -465,4 +463,8 @@ std::unique_ptr RtcEventLog::Create(const Clock* clock) { #endif // ENABLE_RTC_EVENT_LOG } +std::unique_ptr RtcEventLog::CreateNull() { + return std::unique_ptr(new RtcEventLogNullImpl()); +} + } // namespace webrtc diff --git a/webrtc/call/rtc_event_log.h b/webrtc/call/rtc_event_log.h index bea57b01cd..7c72dd5ce9 100644 --- a/webrtc/call/rtc_event_log.h +++ b/webrtc/call/rtc_event_log.h @@ -40,6 +40,9 @@ class RtcEventLog { // Factory method to create an RtcEventLog object. static std::unique_ptr Create(const Clock* clock); + // Create an RtcEventLog object that does nothing. + static std::unique_ptr CreateNull(); + // Starts logging a maximum of max_size_bytes bytes to the specified file. // If the file already exists it will be overwritten. // If max_size_bytes <= 0, logging will be active until StopLogging is called. diff --git a/webrtc/media/base/mediaengine.h b/webrtc/media/base/mediaengine.h index 0e2b7efa86..dd1f5272a0 100644 --- a/webrtc/media/base/mediaengine.h +++ b/webrtc/media/base/mediaengine.h @@ -88,15 +88,6 @@ class MediaEngineInterface { // Stops recording AEC dump. virtual void StopAecDump() = 0; - - // Starts RtcEventLog using existing file. A maximum file size in bytes can be - // specified. Logging is stopped just before the size limit is exceeded. - // If max_size_bytes is set to a value <= 0, no limit will be used. - virtual bool StartRtcEventLog(rtc::PlatformFile file, - int64_t max_size_bytes) = 0; - - // Stops recording an RtcEventLog. - virtual void StopRtcEventLog() = 0; }; @@ -175,13 +166,6 @@ class CompositeMediaEngine : public MediaEngineInterface { voice_.StopAecDump(); } - virtual bool StartRtcEventLog(rtc::PlatformFile file, - int64_t max_size_bytes) { - return voice_.StartRtcEventLog(file, max_size_bytes); - } - - virtual void StopRtcEventLog() { voice_.StopRtcEventLog(); } - protected: VOICE voice_; VIDEO video_; diff --git a/webrtc/media/engine/fakewebrtccall.cc b/webrtc/media/engine/fakewebrtccall.cc index 914a403f41..fdf7cf36fc 100644 --- a/webrtc/media/engine/fakewebrtccall.cc +++ b/webrtc/media/engine/fakewebrtccall.cc @@ -478,4 +478,11 @@ void FakeCall::OnSentPacket(const rtc::SentPacket& sent_packet) { } } +bool FakeCall::StartEventLog(rtc::PlatformFile log_file, + int64_t max_size_bytes) { + return false; +} + +void FakeCall::StopEventLog() {} + } // namespace cricket diff --git a/webrtc/media/engine/fakewebrtccall.h b/webrtc/media/engine/fakewebrtccall.h index f703b157d8..a2ac079956 100644 --- a/webrtc/media/engine/fakewebrtccall.h +++ b/webrtc/media/engine/fakewebrtccall.h @@ -231,6 +231,10 @@ class FakeCall final : public webrtc::Call, public webrtc::PacketReceiver { webrtc::NetworkState state) override; void OnSentPacket(const rtc::SentPacket& sent_packet) override; + bool StartEventLog(rtc::PlatformFile log_file, + int64_t max_size_bytes) override; + void StopEventLog() override; + webrtc::Call::Config config_; webrtc::NetworkState audio_network_state_; webrtc::NetworkState video_network_state_; diff --git a/webrtc/media/engine/fakewebrtcvoiceengine.h b/webrtc/media/engine/fakewebrtcvoiceengine.h index 4e3ced6c35..e745f13dc4 100644 --- a/webrtc/media/engine/fakewebrtcvoiceengine.h +++ b/webrtc/media/engine/fakewebrtcvoiceengine.h @@ -272,7 +272,6 @@ class FakeWebRtcVoiceEngine channels_[channel]->associate_send_channel = accociate_send_channel; return 0; } - webrtc::RtcEventLog* GetEventLog() override { return nullptr; } // webrtc::VoECodec WEBRTC_STUB(NumOfCodecs, ()); diff --git a/webrtc/media/engine/webrtcvoiceengine.cc b/webrtc/media/engine/webrtcvoiceengine.cc index 2ddf67dd3a..d329270ce5 100644 --- a/webrtc/media/engine/webrtcvoiceengine.cc +++ b/webrtc/media/engine/webrtcvoiceengine.cc @@ -28,7 +28,6 @@ #include "webrtc/base/stringencode.h" #include "webrtc/base/stringutils.h" #include "webrtc/base/trace_event.h" -#include "webrtc/call/rtc_event_log.h" #include "webrtc/common.h" #include "webrtc/media/base/audiosource.h" #include "webrtc/media/base/mediaconstants.h" @@ -1040,27 +1039,6 @@ void WebRtcVoiceEngine::StopAecDump() { } } -bool WebRtcVoiceEngine::StartRtcEventLog(rtc::PlatformFile file, - int64_t max_size_bytes) { - RTC_DCHECK(worker_thread_checker_.CalledOnValidThread()); - webrtc::RtcEventLog* event_log = voe_wrapper_->codec()->GetEventLog(); - if (event_log) { - return event_log->StartLogging(file, max_size_bytes); - } - LOG_RTCERR0(StartRtcEventLog); - return false; -} - -void WebRtcVoiceEngine::StopRtcEventLog() { - RTC_DCHECK(worker_thread_checker_.CalledOnValidThread()); - webrtc::RtcEventLog* event_log = voe_wrapper_->codec()->GetEventLog(); - if (event_log) { - event_log->StopLogging(); - return; - } - LOG_RTCERR0(StopRtcEventLog); -} - int WebRtcVoiceEngine::CreateVoEChannel() { RTC_DCHECK(worker_thread_checker_.CalledOnValidThread()); return voe_wrapper_->base()->CreateChannel(voe_config_); diff --git a/webrtc/media/engine/webrtcvoiceengine.h b/webrtc/media/engine/webrtcvoiceengine.h index 52983fad2d..16da9ef29e 100644 --- a/webrtc/media/engine/webrtcvoiceengine.h +++ b/webrtc/media/engine/webrtcvoiceengine.h @@ -109,14 +109,6 @@ class WebRtcVoiceEngine final : public webrtc::TraceCallback { // Stops AEC dump. void StopAecDump(); - // Starts recording an RtcEventLog using an existing file until the log file - // reaches the maximum filesize or the StopRtcEventLog function is called. - // If the value of max_size_bytes is <= 0, no limit is used. - bool StartRtcEventLog(rtc::PlatformFile file, int64_t max_size_bytes); - - // Stops recording the RtcEventLog. - void StopRtcEventLog(); - private: // Every option that is "set" will be applied. Every option not "set" will be // ignored. This allows us to selectively turn on and off different options diff --git a/webrtc/modules/bitrate_controller/bitrate_controller_impl.cc b/webrtc/modules/bitrate_controller/bitrate_controller_impl.cc index c99bd53218..8a2464d09c 100644 --- a/webrtc/modules/bitrate_controller/bitrate_controller_impl.cc +++ b/webrtc/modules/bitrate_controller/bitrate_controller_impl.cc @@ -79,20 +79,25 @@ class BitrateControllerImpl::RtcpBandwidthObserverImpl BitrateController* BitrateController::CreateBitrateController( Clock* clock, - BitrateObserver* observer) { - return new BitrateControllerImpl(clock, observer); + BitrateObserver* observer, + RtcEventLog* event_log) { + return new BitrateControllerImpl(clock, observer, event_log); } -BitrateController* BitrateController::CreateBitrateController(Clock* clock) { - return new BitrateControllerImpl(clock, nullptr); +BitrateController* BitrateController::CreateBitrateController( + Clock* clock, + RtcEventLog* event_log) { + return CreateBitrateController(clock, nullptr, event_log); } BitrateControllerImpl::BitrateControllerImpl(Clock* clock, - BitrateObserver* observer) + BitrateObserver* observer, + RtcEventLog* event_log) : clock_(clock), observer_(observer), last_bitrate_update_ms_(clock_->TimeInMilliseconds()), - bandwidth_estimation_(), + event_log_(event_log), + bandwidth_estimation_(event_log), reserved_bitrate_bps_(0), last_bitrate_bps_(0), last_fraction_loss_(0), @@ -143,7 +148,7 @@ void BitrateControllerImpl::ResetBitrates(int bitrate_bps, int max_bitrate_bps) { { rtc::CritScope cs(&critsect_); - bandwidth_estimation_ = SendSideBandwidthEstimation(); + bandwidth_estimation_ = SendSideBandwidthEstimation(event_log_); bandwidth_estimation_.SetBitrates(bitrate_bps, min_bitrate_bps, max_bitrate_bps); } @@ -158,11 +163,6 @@ void BitrateControllerImpl::SetReservedBitrate(uint32_t reserved_bitrate_bps) { MaybeTriggerOnNetworkChanged(); } -void BitrateControllerImpl::SetEventLog(RtcEventLog* event_log) { - rtc::CritScope cs(&critsect_); - bandwidth_estimation_.SetEventLog(event_log); -} - void BitrateControllerImpl::OnReceivedEstimatedBitrate(uint32_t bitrate) { { rtc::CritScope cs(&critsect_); diff --git a/webrtc/modules/bitrate_controller/bitrate_controller_impl.h b/webrtc/modules/bitrate_controller/bitrate_controller_impl.h index 91f0902cb2..114e1a6789 100644 --- a/webrtc/modules/bitrate_controller/bitrate_controller_impl.h +++ b/webrtc/modules/bitrate_controller/bitrate_controller_impl.h @@ -30,7 +30,9 @@ class BitrateControllerImpl : public BitrateController { public: // TODO(perkj): BitrateObserver has been deprecated and is not used in WebRTC. // |observer| is left for project that is not yet updated. - BitrateControllerImpl(Clock* clock, BitrateObserver* observer); + BitrateControllerImpl(Clock* clock, + BitrateObserver* observer, + RtcEventLog* event_log); virtual ~BitrateControllerImpl() {} bool AvailableBandwidth(uint32_t* bandwidth) const override; @@ -54,8 +56,6 @@ class BitrateControllerImpl : public BitrateController { void SetReservedBitrate(uint32_t reserved_bitrate_bps) override; - void SetEventLog(RtcEventLog* event_log) override; - // Returns true if the parameters have changed since the last call. bool GetNetworkParameters(uint32_t* bitrate, uint8_t* fraction_loss, @@ -86,6 +86,7 @@ class BitrateControllerImpl : public BitrateController { Clock* const clock_; BitrateObserver* const observer_; int64_t last_bitrate_update_ms_; + RtcEventLog* const event_log_; rtc::CriticalSection critsect_; SendSideBandwidthEstimation bandwidth_estimation_ GUARDED_BY(critsect_); diff --git a/webrtc/modules/bitrate_controller/bitrate_controller_unittest.cc b/webrtc/modules/bitrate_controller/bitrate_controller_unittest.cc index 4f92a3884b..f2a14ea9cf 100644 --- a/webrtc/modules/bitrate_controller/bitrate_controller_unittest.cc +++ b/webrtc/modules/bitrate_controller/bitrate_controller_unittest.cc @@ -13,6 +13,7 @@ #include "testing/gtest/include/gtest/gtest.h" +#include "webrtc/call/mock/mock_rtc_event_log.h" #include "webrtc/modules/bitrate_controller/include/bitrate_controller.h" #include "webrtc/modules/pacing/mock/mock_paced_sender.h" #include "webrtc/modules/rtp_rtcp/include/rtp_rtcp_defines.h" @@ -66,8 +67,8 @@ class BitrateControllerTest : public ::testing::Test { ~BitrateControllerTest() {} virtual void SetUp() { - controller_ = - BitrateController::CreateBitrateController(&clock_, &bitrate_observer_); + controller_ = BitrateController::CreateBitrateController( + &clock_, &bitrate_observer_, &event_log_); controller_->SetStartBitrate(kStartBitrateBps); EXPECT_EQ(kStartBitrateBps, bitrate_observer_.last_bitrate_); controller_->SetMinMaxBitrate(kMinBitrateBps, kMaxBitrateBps); @@ -91,6 +92,7 @@ class BitrateControllerTest : public ::testing::Test { TestBitrateObserver bitrate_observer_; BitrateController* controller_; RtcpBandwidthObserver* bandwidth_observer_; + webrtc::MockRtcEventLog event_log_; }; TEST_F(BitrateControllerTest, DefaultMinMaxBitrate) { @@ -107,6 +109,7 @@ TEST_F(BitrateControllerTest, DefaultMinMaxBitrate) { TEST_F(BitrateControllerTest, OneBitrateObserverOneRtcpObserver) { // First REMB applies immediately. + EXPECT_CALL(event_log_, LogBwePacketLossEvent(testing::Gt(0), 0, 0)).Times(8); int64_t time_ms = 1001; webrtc::ReportBlockList report_blocks; report_blocks.push_back(CreateReportBlock(1, 2, 0, 1)); @@ -183,6 +186,7 @@ TEST_F(BitrateControllerTest, OneBitrateObserverOneRtcpObserver) { TEST_F(BitrateControllerTest, OneBitrateObserverTwoRtcpObservers) { // REMBs during the first 2 seconds apply immediately. + EXPECT_CALL(event_log_, LogBwePacketLossEvent(testing::Gt(0), 0, 0)).Times(9); int64_t time_ms = 1; webrtc::ReportBlockList report_blocks; report_blocks.push_back(CreateReportBlock(1, 2, 0, 1)); @@ -278,6 +282,13 @@ TEST_F(BitrateControllerTest, OneBitrateObserverTwoRtcpObservers) { } TEST_F(BitrateControllerTest, OneBitrateObserverMultipleReportBlocks) { + testing::Expectation first_calls = + EXPECT_CALL(event_log_, LogBwePacketLossEvent(testing::Gt(0), 0, 0)) + .Times(7); + EXPECT_CALL(event_log_, + LogBwePacketLossEvent(testing::Gt(0), testing::Gt(0), 0)) + .Times(2) + .After(first_calls); uint32_t sequence_number[2] = {0, 0xFF00}; const int kStartBitrate = 200000; const int kMinBitrate = 100000; diff --git a/webrtc/modules/bitrate_controller/include/bitrate_controller.h b/webrtc/modules/bitrate_controller/include/bitrate_controller.h index 4165f066b2..f06314cd4a 100644 --- a/webrtc/modules/bitrate_controller/include/bitrate_controller.h +++ b/webrtc/modules/bitrate_controller/include/bitrate_controller.h @@ -55,8 +55,11 @@ class BitrateController : public Module { // TODO(perkj): BitrateObserver has been deprecated and is not used in WebRTC. // Remove this method once other other projects does not use it. static BitrateController* CreateBitrateController(Clock* clock, - BitrateObserver* observer); - static BitrateController* CreateBitrateController(Clock* clock); + BitrateObserver* observer, + RtcEventLog* event_log); + + static BitrateController* CreateBitrateController(Clock* clock, + RtcEventLog* event_log); virtual ~BitrateController() {} @@ -76,8 +79,6 @@ class BitrateController : public Module { virtual void UpdateDelayBasedEstimate(uint32_t bitrate_bps) = 0; - virtual void SetEventLog(RtcEventLog* event_log) = 0; - // Gets the available payload bandwidth in bits per second. Note that // this bandwidth excludes packet headers. virtual bool AvailableBandwidth(uint32_t* bandwidth) const = 0; diff --git a/webrtc/modules/bitrate_controller/send_side_bandwidth_estimation.cc b/webrtc/modules/bitrate_controller/send_side_bandwidth_estimation.cc index a1b78a257c..e47d4917e9 100644 --- a/webrtc/modules/bitrate_controller/send_side_bandwidth_estimation.cc +++ b/webrtc/modules/bitrate_controller/send_side_bandwidth_estimation.cc @@ -44,7 +44,7 @@ const size_t kNumUmaRampupMetrics = } // namespace -SendSideBandwidthEstimation::SendSideBandwidthEstimation() +SendSideBandwidthEstimation::SendSideBandwidthEstimation(RtcEventLog* event_log) : lost_packets_since_last_loss_update_Q8_(0), expected_packets_since_last_loss_update_(0), bitrate_(0), @@ -63,7 +63,9 @@ SendSideBandwidthEstimation::SendSideBandwidthEstimation() bitrate_at_2_seconds_kbps_(0), uma_update_state_(kNoUpdate), rampup_uma_stats_updated_(kNumUmaRampupMetrics, false), - event_log_(nullptr) {} + event_log_(event_log) { + RTC_DCHECK(event_log); +} SendSideBandwidthEstimation::~SendSideBandwidthEstimation() {} @@ -227,11 +229,9 @@ void SendSideBandwidthEstimation::UpdateEstimate(int64_t now_ms) { // rates). bitrate_ += 1000; - if (event_log_) { - event_log_->LogBwePacketLossEvent( - bitrate_, last_fraction_loss_, - expected_packets_since_last_loss_update_); - } + event_log_->LogBwePacketLossEvent( + bitrate_, last_fraction_loss_, + expected_packets_since_last_loss_update_); } else if (last_fraction_loss_ <= 26) { // Loss between 2% - 10%: Do nothing. } else { @@ -250,11 +250,9 @@ void SendSideBandwidthEstimation::UpdateEstimate(int64_t now_ms) { 512.0); has_decreased_since_last_fraction_loss_ = true; } - if (event_log_) { - event_log_->LogBwePacketLossEvent( - bitrate_, last_fraction_loss_, - expected_packets_since_last_loss_update_); - } + event_log_->LogBwePacketLossEvent( + bitrate_, last_fraction_loss_, + expected_packets_since_last_loss_update_); } } bitrate_ = CapBitrateToThresholds(now_ms, bitrate_); @@ -308,9 +306,4 @@ uint32_t SendSideBandwidthEstimation::CapBitrateToThresholds( } return bitrate; } - -void SendSideBandwidthEstimation::SetEventLog(RtcEventLog* event_log) { - event_log_ = event_log; -} - } // namespace webrtc diff --git a/webrtc/modules/bitrate_controller/send_side_bandwidth_estimation.h b/webrtc/modules/bitrate_controller/send_side_bandwidth_estimation.h index 402d22a6bf..c97714f7e8 100644 --- a/webrtc/modules/bitrate_controller/send_side_bandwidth_estimation.h +++ b/webrtc/modules/bitrate_controller/send_side_bandwidth_estimation.h @@ -26,7 +26,8 @@ class RtcEventLog; class SendSideBandwidthEstimation { public: - SendSideBandwidthEstimation(); + SendSideBandwidthEstimation() = delete; + explicit SendSideBandwidthEstimation(RtcEventLog* event_log); virtual ~SendSideBandwidthEstimation(); void CurrentEstimate(int* bitrate, uint8_t* loss, int64_t* rtt) const; @@ -53,8 +54,6 @@ class SendSideBandwidthEstimation { void SetMinMaxBitrate(int min_bitrate, int max_bitrate); int GetMinBitrate() const; - void SetEventLog(RtcEventLog* event_log); - private: enum UmaState { kNoUpdate, kFirstDone, kDone }; diff --git a/webrtc/modules/bitrate_controller/send_side_bandwidth_estimation_unittest.cc b/webrtc/modules/bitrate_controller/send_side_bandwidth_estimation_unittest.cc index a6fda5be35..64cb2fee51 100644 --- a/webrtc/modules/bitrate_controller/send_side_bandwidth_estimation_unittest.cc +++ b/webrtc/modules/bitrate_controller/send_side_bandwidth_estimation_unittest.cc @@ -12,12 +12,14 @@ #include #include "testing/gtest/include/gtest/gtest.h" +#include "webrtc/call/mock/mock_rtc_event_log.h" #include "webrtc/modules/bitrate_controller/send_side_bandwidth_estimation.h" namespace webrtc { void TestProbing(bool use_delay_based) { - SendSideBandwidthEstimation bwe; + MockRtcEventLog event_log; + SendSideBandwidthEstimation bwe(&event_log); bwe.SetMinMaxBitrate(100000, 1500000); bwe.SetSendBitrate(200000); @@ -62,7 +64,11 @@ TEST(SendSideBweTest, InitialDelayBasedBweWithProbing) { } TEST(SendSideBweTest, DoesntReapplyBitrateDecreaseWithoutFollowingRemb) { - SendSideBandwidthEstimation bwe; + MockRtcEventLog event_log; + EXPECT_CALL(event_log, + LogBwePacketLossEvent(testing::Gt(0), testing::Gt(0), 0)) + .Times(3); + SendSideBandwidthEstimation bwe(&event_log); static const int kMinBitrateBps = 100000; static const int kInitialBitrateBps = 1000000; bwe.SetMinMaxBitrate(kMinBitrateBps, 1500000); diff --git a/webrtc/modules/congestion_controller/DEPS b/webrtc/modules/congestion_controller/DEPS index d72e34db6e..675f103fb9 100644 --- a/webrtc/modules/congestion_controller/DEPS +++ b/webrtc/modules/congestion_controller/DEPS @@ -1,5 +1,6 @@ include_rules = [ "+webrtc/base", + "+webrtc/call/mock", "+webrtc/system_wrappers", "+webrtc/video", ] diff --git a/webrtc/modules/congestion_controller/congestion_controller.cc b/webrtc/modules/congestion_controller/congestion_controller.cc index c836f40f5e..d42ad4acc5 100644 --- a/webrtc/modules/congestion_controller/congestion_controller.cc +++ b/webrtc/modules/congestion_controller/congestion_controller.cc @@ -151,39 +151,19 @@ class WrappingBitrateEstimator : public RemoteBitrateEstimator { } // namespace -CongestionController::CongestionController( - Clock* clock, - BitrateObserver* bitrate_observer, - RemoteBitrateObserver* remote_bitrate_observer) - : clock_(clock), - observer_(nullptr), - packet_router_(new PacketRouter()), - pacer_(new PacedSender(clock_, packet_router_.get())), - remote_bitrate_estimator_( - new WrappingBitrateEstimator(remote_bitrate_observer, clock_)), - bitrate_controller_( - BitrateController::CreateBitrateController(clock_, bitrate_observer)), - remote_estimator_proxy_(clock_, packet_router_.get()), - transport_feedback_adapter_(bitrate_controller_.get(), clock_), - min_bitrate_bps_(RemoteBitrateEstimator::kDefaultMinBitrateBps), - last_reported_bitrate_bps_(0), - last_reported_fraction_loss_(0), - last_reported_rtt_(0), - network_state_(kNetworkUp) { - Init(); -} - CongestionController::CongestionController( Clock* clock, Observer* observer, - RemoteBitrateObserver* remote_bitrate_observer) + RemoteBitrateObserver* remote_bitrate_observer, + RtcEventLog* event_log) : clock_(clock), observer_(observer), packet_router_(new PacketRouter()), pacer_(new PacedSender(clock_, packet_router_.get())), remote_bitrate_estimator_( new WrappingBitrateEstimator(remote_bitrate_observer, clock_)), - bitrate_controller_(BitrateController::CreateBitrateController(clock_)), + bitrate_controller_( + BitrateController::CreateBitrateController(clock_, event_log)), remote_estimator_proxy_(clock_, packet_router_.get()), transport_feedback_adapter_(bitrate_controller_.get(), clock_), min_bitrate_bps_(RemoteBitrateEstimator::kDefaultMinBitrateBps), @@ -198,6 +178,7 @@ CongestionController::CongestionController( Clock* clock, Observer* observer, RemoteBitrateObserver* remote_bitrate_observer, + RtcEventLog* event_log, std::unique_ptr packet_router, std::unique_ptr pacer) : clock_(clock), @@ -208,7 +189,8 @@ CongestionController::CongestionController( new WrappingBitrateEstimator(remote_bitrate_observer, clock_)), // Constructed last as this object calls the provided callback on // construction. - bitrate_controller_(BitrateController::CreateBitrateController(clock_)), + bitrate_controller_( + BitrateController::CreateBitrateController(clock_, event_log)), remote_estimator_proxy_(clock_, packet_router_.get()), transport_feedback_adapter_(bitrate_controller_.get(), clock_), min_bitrate_bps_(RemoteBitrateEstimator::kDefaultMinBitrateBps), diff --git a/webrtc/modules/congestion_controller/congestion_controller_unittest.cc b/webrtc/modules/congestion_controller/congestion_controller_unittest.cc index 1a265824bc..aa4a5d3330 100644 --- a/webrtc/modules/congestion_controller/congestion_controller_unittest.cc +++ b/webrtc/modules/congestion_controller/congestion_controller_unittest.cc @@ -10,6 +10,7 @@ #include "testing/gmock/include/gmock/gmock.h" #include "testing/gtest/include/gtest/gtest.h" +#include "webrtc/call/mock/mock_rtc_event_log.h" #include "webrtc/modules/pacing/mock/mock_paced_sender.h" #include "webrtc/modules/congestion_controller/include/congestion_controller.h" #include "webrtc/modules/congestion_controller/include/mock/mock_congestion_controller.h" @@ -34,9 +35,9 @@ class CongestionControllerTest : public ::testing::Test { pacer_ = new NiceMock(); std::unique_ptr pacer(pacer_); // Passes ownership. std::unique_ptr packet_router(new PacketRouter()); - controller_.reset( - new CongestionController(&clock_, &observer_, &remote_bitrate_observer_, - std::move(packet_router), std::move(pacer))); + controller_.reset(new CongestionController( + &clock_, &observer_, &remote_bitrate_observer_, &event_log_, + std::move(packet_router), std::move(pacer))); bandwidth_observer_.reset( controller_->GetBitrateController()->CreateRtcpBandwidthObserver()); @@ -51,6 +52,7 @@ class CongestionControllerTest : public ::testing::Test { StrictMock observer_; NiceMock* pacer_; NiceMock remote_bitrate_observer_; + MockRtcEventLog event_log_; std::unique_ptr bandwidth_observer_; std::unique_ptr controller_; const uint32_t kInitialBitrateBps = 60000; diff --git a/webrtc/modules/congestion_controller/include/congestion_controller.h b/webrtc/modules/congestion_controller/include/congestion_controller.h index 82906efc1d..a48f0008cc 100644 --- a/webrtc/modules/congestion_controller/include/congestion_controller.h +++ b/webrtc/modules/congestion_controller/include/congestion_controller.h @@ -29,11 +29,11 @@ struct SentPacket; namespace webrtc { class BitrateController; -class BitrateObserver; class Clock; class ProcessThread; class RemoteBitrateEstimator; class RemoteBitrateObserver; +class RtcEventLog; class TransportFeedbackObserver; class CongestionController : public CallStatsObserver, public Module { @@ -52,17 +52,14 @@ class CongestionController : public CallStatsObserver, public Module { protected: virtual ~Observer() {} }; - // Deprecated - // TODO(perkj): Remove once no other clients use this ctor. - CongestionController(Clock* clock, - BitrateObserver* bitrate_observer, - RemoteBitrateObserver* remote_bitrate_observer); - CongestionController(Clock* clock, - Observer* observer, - RemoteBitrateObserver* remote_bitrate_observer); CongestionController(Clock* clock, Observer* observer, RemoteBitrateObserver* remote_bitrate_observer, + RtcEventLog* event_log); + CongestionController(Clock* clock, + Observer* observer, + RemoteBitrateObserver* remote_bitrate_observer, + RtcEventLog* event_log, std::unique_ptr packet_router, std::unique_ptr pacer); virtual ~CongestionController(); diff --git a/webrtc/modules/congestion_controller/include/mock/mock_congestion_controller.h b/webrtc/modules/congestion_controller/include/mock/mock_congestion_controller.h index 20955ea81a..f7d4aaa951 100644 --- a/webrtc/modules/congestion_controller/include/mock/mock_congestion_controller.h +++ b/webrtc/modules/congestion_controller/include/mock/mock_congestion_controller.h @@ -31,8 +31,12 @@ class MockCongestionController : public CongestionController { public: MockCongestionController(Clock* clock, Observer* observer, - RemoteBitrateObserver* remote_bitrate_observer) - : CongestionController(clock, observer, remote_bitrate_observer) {} + RemoteBitrateObserver* remote_bitrate_observer, + RtcEventLog* event_log) + : CongestionController(clock, + observer, + remote_bitrate_observer, + event_log) {} MOCK_METHOD3(SetBweBitrates, void(int min_bitrate_bps, int start_bitrate_bps, diff --git a/webrtc/modules/remote_bitrate_estimator/DEPS b/webrtc/modules/remote_bitrate_estimator/DEPS index 9a863d94a9..fc31fa7038 100644 --- a/webrtc/modules/remote_bitrate_estimator/DEPS +++ b/webrtc/modules/remote_bitrate_estimator/DEPS @@ -7,4 +7,10 @@ specific_include_rules = { "nada\.h": [ "+webrtc/voice_engine", ], + "remb\.h": [ + "+webrtc/call/mock", + ], + "send_side\.h": [ + "+webrtc/call/mock", + ], } diff --git a/webrtc/modules/remote_bitrate_estimator/test/estimators/remb.cc b/webrtc/modules/remote_bitrate_estimator/test/estimators/remb.cc index e2d3da9632..1c878b9f15 100644 --- a/webrtc/modules/remote_bitrate_estimator/test/estimators/remb.cc +++ b/webrtc/modules/remote_bitrate_estimator/test/estimators/remb.cc @@ -25,7 +25,9 @@ namespace bwe { RembBweSender::RembBweSender(int kbps, BitrateObserver* observer, Clock* clock) : bitrate_controller_( - BitrateController::CreateBitrateController(clock, observer)), + BitrateController::CreateBitrateController(clock, + observer, + &event_log_)), feedback_observer_(bitrate_controller_->CreateRtcpBandwidthObserver()), clock_(clock) { assert(kbps >= kMinBitrateKbps); diff --git a/webrtc/modules/remote_bitrate_estimator/test/estimators/remb.h b/webrtc/modules/remote_bitrate_estimator/test/estimators/remb.h index 3dc4f388c8..5840aef963 100644 --- a/webrtc/modules/remote_bitrate_estimator/test/estimators/remb.h +++ b/webrtc/modules/remote_bitrate_estimator/test/estimators/remb.h @@ -16,6 +16,7 @@ #include #include "webrtc/base/constructormagic.h" +#include "webrtc/call/mock/mock_rtc_event_log.h" #include "webrtc/modules/remote_bitrate_estimator/test/bwe.h" namespace webrtc { @@ -45,6 +46,7 @@ class RembBweSender : public BweSender { private: Clock* clock_; + MockRtcEventLog event_log_; RTC_DISALLOW_IMPLICIT_CONSTRUCTORS(RembBweSender); }; diff --git a/webrtc/modules/remote_bitrate_estimator/test/estimators/send_side.cc b/webrtc/modules/remote_bitrate_estimator/test/estimators/send_side.cc index 7a31b542e3..6d79e09815 100644 --- a/webrtc/modules/remote_bitrate_estimator/test/estimators/send_side.cc +++ b/webrtc/modules/remote_bitrate_estimator/test/estimators/send_side.cc @@ -22,7 +22,9 @@ const int kFeedbackIntervalMs = 50; FullBweSender::FullBweSender(int kbps, BitrateObserver* observer, Clock* clock) : bitrate_controller_( - BitrateController::CreateBitrateController(clock, observer)), + BitrateController::CreateBitrateController(clock, + observer, + &event_log_)), rbe_(new RemoteBitrateEstimatorAbsSendTime(this)), feedback_observer_(bitrate_controller_->CreateRtcpBandwidthObserver()), clock_(clock), diff --git a/webrtc/modules/remote_bitrate_estimator/test/estimators/send_side.h b/webrtc/modules/remote_bitrate_estimator/test/estimators/send_side.h index ca7f6dbddc..9eb0c1a1d3 100644 --- a/webrtc/modules/remote_bitrate_estimator/test/estimators/send_side.h +++ b/webrtc/modules/remote_bitrate_estimator/test/estimators/send_side.h @@ -14,6 +14,7 @@ #include #include +#include "webrtc/call/mock/mock_rtc_event_log.h" #include "webrtc/modules/remote_bitrate_estimator/include/send_time_history.h" #include "webrtc/modules/remote_bitrate_estimator/test/bwe.h" @@ -46,6 +47,7 @@ class FullBweSender : public BweSender, public RemoteBitrateObserver { bool has_received_ack_; uint16_t last_acked_seq_num_; int64_t last_log_time_ms_; + MockRtcEventLog event_log_; RTC_DISALLOW_IMPLICIT_CONSTRUCTORS(FullBweSender); }; diff --git a/webrtc/pc/channelmanager.cc b/webrtc/pc/channelmanager.cc index fba1040317..c2ce1cc604 100644 --- a/webrtc/pc/channelmanager.cc +++ b/webrtc/pc/channelmanager.cc @@ -389,17 +389,4 @@ void ChannelManager::StopAecDump() { Bind(&MediaEngineInterface::StopAecDump, media_engine_.get())); } -bool ChannelManager::StartRtcEventLog(rtc::PlatformFile file, - int64_t max_size_bytes) { - return worker_thread_->Invoke( - RTC_FROM_HERE, Bind(&MediaEngineInterface::StartRtcEventLog, - media_engine_.get(), file, max_size_bytes)); -} - -void ChannelManager::StopRtcEventLog() { - worker_thread_->Invoke( - RTC_FROM_HERE, - Bind(&MediaEngineInterface::StopRtcEventLog, media_engine_.get())); -} - } // namespace cricket diff --git a/webrtc/pc/channelmanager.h b/webrtc/pc/channelmanager.h index 34188e6e58..c6a67dfb07 100644 --- a/webrtc/pc/channelmanager.h +++ b/webrtc/pc/channelmanager.h @@ -138,12 +138,6 @@ class ChannelManager { // Stops recording AEC dump. void StopAecDump(); - // Starts RtcEventLog using existing file. - bool StartRtcEventLog(rtc::PlatformFile file, int64_t max_size_bytes); - - // Stops logging RtcEventLog. - void StopRtcEventLog(); - private: typedef std::vector VoiceChannels; typedef std::vector VideoChannels; diff --git a/webrtc/test/mock_voe_channel_proxy.h b/webrtc/test/mock_voe_channel_proxy.h index dc2a961b3c..fef8b7d899 100644 --- a/webrtc/test/mock_voe_channel_proxy.h +++ b/webrtc/test/mock_voe_channel_proxy.h @@ -57,6 +57,7 @@ class MockVoEChannelProxy : public voe::ChannelProxy { MOCK_CONST_METHOD0(GetAudioDecoderFactory, const rtc::scoped_refptr&()); MOCK_METHOD1(SetChannelOutputVolumeScaling, void(float scaling)); + MOCK_METHOD1(SetRtcEventLog, void(RtcEventLog* event_log)); }; } // namespace test } // namespace webrtc diff --git a/webrtc/test/mock_voice_engine.h b/webrtc/test/mock_voice_engine.h index be72e0d631..469028dbf6 100644 --- a/webrtc/test/mock_voice_engine.h +++ b/webrtc/test/mock_voice_engine.h @@ -157,7 +157,6 @@ class MockVoiceEngine : public VoiceEngineImpl { int(int channel, bool& enabled, VadModes& mode, bool& disabledDTX)); MOCK_METHOD2(SetOpusMaxPlaybackRate, int(int channel, int frequency_hz)); MOCK_METHOD2(SetOpusDtx, int(int channel, bool enable_dtx)); - MOCK_METHOD0(GetEventLog, RtcEventLog*()); // VoEExternalMedia MOCK_METHOD3(RegisterExternalMediaProcessing, diff --git a/webrtc/voice_engine/channel.cc b/webrtc/voice_engine/channel.cc index 943a04572b..8287668258 100644 --- a/webrtc/voice_engine/channel.cc +++ b/webrtc/voice_engine/channel.cc @@ -19,6 +19,7 @@ #include "webrtc/base/logging.h" #include "webrtc/base/thread_checker.h" #include "webrtc/base/timeutils.h" +#include "webrtc/call/rtc_event_log.h" #include "webrtc/common.h" #include "webrtc/config.h" #include "webrtc/modules/audio_coding/codecs/builtin_audio_decoder_factory.h" @@ -58,6 +59,87 @@ bool RegisterReceiveCodec(std::unique_ptr* acm, const int kTelephoneEventAttenuationdB = 10; +class RtcEventLogProxy final : public webrtc::RtcEventLog { + public: + RtcEventLogProxy() : event_log_(nullptr) {} + + bool StartLogging(const std::string& file_name, + int64_t max_size_bytes) override { + RTC_NOTREACHED(); + return false; + } + + bool StartLogging(rtc::PlatformFile log_file, + int64_t max_size_bytes) override { + RTC_NOTREACHED(); + return false; + } + + void StopLogging() override { RTC_NOTREACHED(); } + + void LogVideoReceiveStreamConfig( + const webrtc::VideoReceiveStream::Config& config) override { + rtc::CritScope lock(&crit_); + if (event_log_) { + event_log_->LogVideoReceiveStreamConfig(config); + } + } + + void LogVideoSendStreamConfig( + const webrtc::VideoSendStream::Config& config) override { + rtc::CritScope lock(&crit_); + if (event_log_) { + event_log_->LogVideoSendStreamConfig(config); + } + } + + void LogRtpHeader(webrtc::PacketDirection direction, + webrtc::MediaType media_type, + const uint8_t* header, + size_t packet_length) override { + rtc::CritScope lock(&crit_); + if (event_log_) { + event_log_->LogRtpHeader(direction, media_type, header, packet_length); + } + } + + void LogRtcpPacket(webrtc::PacketDirection direction, + webrtc::MediaType media_type, + const uint8_t* packet, + size_t length) override { + rtc::CritScope lock(&crit_); + if (event_log_) { + event_log_->LogRtcpPacket(direction, media_type, packet, length); + } + } + + void LogAudioPlayout(uint32_t ssrc) override { + rtc::CritScope lock(&crit_); + if (event_log_) { + event_log_->LogAudioPlayout(ssrc); + } + } + + void LogBwePacketLossEvent(int32_t bitrate, + uint8_t fraction_loss, + int32_t total_packets) override { + rtc::CritScope lock(&crit_); + if (event_log_) { + event_log_->LogBwePacketLossEvent(bitrate, fraction_loss, total_packets); + } + } + + void SetEventLog(RtcEventLog* event_log) { + rtc::CritScope lock(&crit_); + event_log_ = event_log; + } + + private: + rtc::CriticalSection crit_; + RtcEventLog* event_log_ GUARDED_BY(crit_); + RTC_DISALLOW_COPY_AND_ASSIGN(RtcEventLogProxy); +}; + class TransportFeedbackProxy : public TransportFeedbackObserver { public: TransportFeedbackProxy() : feedback_observer_(nullptr) { @@ -480,11 +562,9 @@ bool Channel::OnRecoveredPacket(const uint8_t* rtp_packet, MixerParticipant::AudioFrameInfo Channel::GetAudioFrameWithMuted( int32_t id, AudioFrame* audioFrame) { - if (event_log_) { - unsigned int ssrc; - RTC_CHECK_EQ(GetLocalSSRC(ssrc), 0); - event_log_->LogAudioPlayout(ssrc); - } + unsigned int ssrc; + RTC_CHECK_EQ(GetLocalSSRC(ssrc), 0); + event_log_proxy_->LogAudioPlayout(ssrc); // Get 10ms raw PCM data from the ACM (mixer limits output frequency) bool muted; if (audio_coding_->PlayoutData10Ms(audioFrame->sample_rate_hz_, audioFrame, @@ -671,9 +751,8 @@ int32_t Channel::NeededFrequency(int32_t id) const { int32_t Channel::CreateChannel(Channel*& channel, int32_t channelId, uint32_t instanceId, - RtcEventLog* const event_log, const Config& config) { - return CreateChannel(channel, channelId, instanceId, event_log, config, + return CreateChannel(channel, channelId, instanceId, config, CreateBuiltinAudioDecoderFactory()); } @@ -681,15 +760,13 @@ int32_t Channel::CreateChannel( Channel*& channel, int32_t channelId, uint32_t instanceId, - RtcEventLog* const event_log, const Config& config, const rtc::scoped_refptr& decoder_factory) { WEBRTC_TRACE(kTraceMemory, kTraceVoice, VoEId(instanceId, channelId), "Channel::CreateChannel(channelId=%d, instanceId=%d)", channelId, instanceId); - channel = - new Channel(channelId, instanceId, event_log, config, decoder_factory); + channel = new Channel(channelId, instanceId, config, decoder_factory); if (channel == NULL) { WEBRTC_TRACE(kTraceMemory, kTraceVoice, VoEId(instanceId, channelId), "Channel::CreateChannel() unable to allocate memory for" @@ -748,12 +825,11 @@ void Channel::RecordFileEnded(int32_t id) { Channel::Channel(int32_t channelId, uint32_t instanceId, - RtcEventLog* const event_log, const Config& config, const rtc::scoped_refptr& decoder_factory) : _instanceId(instanceId), _channelId(channelId), - event_log_(event_log), + event_log_proxy_(new RtcEventLogProxy()), rtp_header_parser_(RtpHeaderParser::Create()), rtp_payload_registry_( new RTPPayloadRegistry(RTPPayloadStrategy::CreateStrategy(true))), @@ -856,7 +932,7 @@ Channel::Channel(int32_t channelId, seq_num_allocator_proxy_.get(); configuration.transport_feedback_callback = feedback_observer_proxy_.get(); } - configuration.event_log = event_log; + configuration.event_log = &(*event_log_proxy_); _rtpRtcpModule.reset(RtpRtcp::CreateRtpRtcp(configuration)); _rtpRtcpModule->SetSendingMediaStatus(false); @@ -3008,6 +3084,10 @@ void Channel::DisassociateSendChannel(int channel_id) { } } +void Channel::SetRtcEventLog(RtcEventLog* event_log) { + event_log_proxy_->SetEventLog(event_log); +} + int Channel::RegisterExternalMediaProcessing(ProcessingTypes type, VoEMediaProcess& processObject) { WEBRTC_TRACE(kTraceInfo, kTraceVoice, VoEId(_instanceId, _channelId), diff --git a/webrtc/voice_engine/channel.h b/webrtc/voice_engine/channel.h index 9b6a2a136f..68211a605a 100644 --- a/webrtc/voice_engine/channel.h +++ b/webrtc/voice_engine/channel.h @@ -66,6 +66,7 @@ struct SenderInfo; namespace voe { class OutputMixer; +class RtcEventLogProxy; class RtpPacketSenderProxy; class Statistics; class StatisticsProxy; @@ -173,18 +174,15 @@ class Channel static int32_t CreateChannel(Channel*& channel, int32_t channelId, uint32_t instanceId, - RtcEventLog* const event_log, const Config& config); static int32_t CreateChannel( Channel*& channel, int32_t channelId, uint32_t instanceId, - RtcEventLog* const event_log, const Config& config, const rtc::scoped_refptr& decoder_factory); Channel(int32_t channelId, uint32_t instanceId, - RtcEventLog* const event_log, const Config& config, const rtc::scoped_refptr& decoder_factory); int32_t Init(); @@ -451,6 +449,9 @@ class Channel // Disassociate a send channel if it was associated. void DisassociateSendChannel(int channel_id); + // Set a RtcEventLog logging object. + void SetRtcEventLog(RtcEventLog* event_log); + protected: void OnIncomingFractionLoss(int fraction_lost); @@ -486,7 +487,7 @@ class Channel ChannelState channel_state_; - RtcEventLog* const event_log_; + std::unique_ptr event_log_proxy_; std::unique_ptr rtp_header_parser_; std::unique_ptr rtp_payload_registry_; diff --git a/webrtc/voice_engine/channel_manager.cc b/webrtc/voice_engine/channel_manager.cc index 47350b59c9..5f20b2ed16 100644 --- a/webrtc/voice_engine/channel_manager.cc +++ b/webrtc/voice_engine/channel_manager.cc @@ -47,10 +47,7 @@ ChannelOwner::ChannelRef::ChannelRef(class Channel* channel) : channel(channel), ref_count(1) {} ChannelManager::ChannelManager(uint32_t instance_id, const Config& config) - : instance_id_(instance_id), - last_channel_id_(-1), - config_(config), - event_log_(RtcEventLog::Create(Clock::GetRealTimeClock())) {} + : instance_id_(instance_id), last_channel_id_(-1), config_(config) {} ChannelOwner ChannelManager::CreateChannel() { return CreateChannel(CreateBuiltinAudioDecoderFactory()); @@ -75,8 +72,8 @@ ChannelOwner ChannelManager::CreateChannelInternal( const Config& config, const rtc::scoped_refptr& decoder_factory) { Channel* channel; - Channel::CreateChannel(channel, ++last_channel_id_, instance_id_, - event_log_.get(), config, decoder_factory); + Channel::CreateChannel(channel, ++last_channel_id_, instance_id_, config, + decoder_factory); ChannelOwner channel_owner(channel); rtc::CritScope crit(&lock_); @@ -143,10 +140,6 @@ size_t ChannelManager::NumOfChannels() const { return channels_.size(); } -RtcEventLog* ChannelManager::GetEventLog() const { - return event_log_.get(); -} - ChannelManager::Iterator::Iterator(ChannelManager* channel_manager) : iterator_pos_(0) { channel_manager->GetAllChannels(&channels_); diff --git a/webrtc/voice_engine/channel_manager.h b/webrtc/voice_engine/channel_manager.h index 213a48f769..c59ee5dc97 100644 --- a/webrtc/voice_engine/channel_manager.h +++ b/webrtc/voice_engine/channel_manager.h @@ -17,7 +17,6 @@ #include "webrtc/base/constructormagic.h" #include "webrtc/base/criticalsection.h" #include "webrtc/base/scoped_ref_ptr.h" -#include "webrtc/call/rtc_event_log.h" #include "webrtc/system_wrappers/include/atomic32.h" #include "webrtc/typedefs.h" @@ -119,9 +118,6 @@ class ChannelManager { size_t NumOfChannels() const; - // Returns a pointer to the event log object stored within the ChannelManager. - RtcEventLog* GetEventLog() const; - private: // Create a channel given a configuration, |config|. ChannelOwner CreateChannelInternal( @@ -136,7 +132,6 @@ class ChannelManager { std::vector channels_; const Config& config_; - std::unique_ptr event_log_; RTC_DISALLOW_COPY_AND_ASSIGN(ChannelManager); }; diff --git a/webrtc/voice_engine/channel_proxy.cc b/webrtc/voice_engine/channel_proxy.cc index f60728aadc..215d931f1a 100644 --- a/webrtc/voice_engine/channel_proxy.cc +++ b/webrtc/voice_engine/channel_proxy.cc @@ -204,6 +204,11 @@ void ChannelProxy::SetChannelOutputVolumeScaling(float scaling) { RTC_DCHECK_EQ(0, error); } +void ChannelProxy::SetRtcEventLog(RtcEventLog* event_log) { + RTC_DCHECK(thread_checker_.CalledOnValidThread()); + channel()->SetRtcEventLog(event_log); +} + Channel* ChannelProxy::channel() const { RTC_DCHECK(channel_owner_.channel()); return channel_owner_.channel(); diff --git a/webrtc/voice_engine/channel_proxy.h b/webrtc/voice_engine/channel_proxy.h index 39eed67d5d..0c26b00986 100644 --- a/webrtc/voice_engine/channel_proxy.h +++ b/webrtc/voice_engine/channel_proxy.h @@ -24,6 +24,7 @@ namespace webrtc { class AudioSinkInterface; class PacketRouter; +class RtcEventLog; class RtpPacketSender; class Transport; class TransportFeedbackObserver; @@ -87,6 +88,8 @@ class ChannelProxy { virtual void SetChannelOutputVolumeScaling(float scaling); + virtual void SetRtcEventLog(RtcEventLog* event_log); + private: Channel* channel() const; diff --git a/webrtc/voice_engine/include/voe_codec.h b/webrtc/voice_engine/include/voe_codec.h index 6c4fb38a2f..5d94ac2736 100644 --- a/webrtc/voice_engine/include/voe_codec.h +++ b/webrtc/voice_engine/include/voe_codec.h @@ -35,7 +35,6 @@ namespace webrtc { -class RtcEventLog; class VoiceEngine; class WEBRTC_DLLEXPORT VoECodec { @@ -132,10 +131,6 @@ class WEBRTC_DLLEXPORT VoECodec { // success, and -1 if failed. virtual int SetOpusDtx(int channel, bool enable_dtx) = 0; - // Get a pointer to the event logging object associated with this Voice - // Engine. This pointer will remain valid until VoiceEngine is destroyed. - virtual RtcEventLog* GetEventLog() = 0; - protected: VoECodec() {} virtual ~VoECodec() {} diff --git a/webrtc/voice_engine/test/auto_test/standard/codec_test.cc b/webrtc/voice_engine/test/auto_test/standard/codec_test.cc index 3a3d83031d..5a500af2df 100644 --- a/webrtc/voice_engine/test/auto_test/standard/codec_test.cc +++ b/webrtc/voice_engine/test/auto_test/standard/codec_test.cc @@ -176,30 +176,6 @@ TEST_F(CodecTest, OpusDtxCannotBeSetForNonOpus) { } } -#ifdef ENABLE_RTC_EVENT_LOG -TEST_F(CodecTest, RtcEventLogIntegrationTest) { - webrtc::RtcEventLog* event_log = voe_codec_->GetEventLog(); - ASSERT_TRUE(event_log); - - // Find the name of the current test, in order to use it as a temporary - // filename. - auto test_info = ::testing::UnitTest::GetInstance()->current_test_info(); - const std::string temp_filename = webrtc::test::OutputPath() + - test_info->test_case_name() + - test_info->name(); - // Create a log file. - event_log->StartLogging(temp_filename, 1000); - event_log->StopLogging(); - - // Check if the file has been created. - FILE* event_file = fopen(temp_filename.c_str(), "r"); - ASSERT_TRUE(event_file); - fclose(event_file); - // Remove the temporary file. - remove(temp_filename.c_str()); -} -#endif // ENABLE_RTC_EVENT_LOG - // TODO(xians, phoglund): Re-enable when issue 372 is resolved. TEST_F(CodecTest, DISABLED_ManualVerifySendCodecsForAllPacketSizes) { for (int i = 0; i < voe_codec_->NumOfCodecs(); ++i) { diff --git a/webrtc/voice_engine/test/cmd_test/voe_cmd_test.cc b/webrtc/voice_engine/test/cmd_test/voe_cmd_test.cc index d3d2a2d790..674a05c4cb 100644 --- a/webrtc/voice_engine/test/cmd_test/voe_cmd_test.cc +++ b/webrtc/voice_engine/test/cmd_test/voe_cmd_test.cc @@ -21,7 +21,6 @@ #include "gflags/gflags.h" #include "testing/gtest/include/gtest/gtest.h" #include "webrtc/base/format_macros.h" -#include "webrtc/call/rtc_event_log.h" #include "webrtc/engine_configurations.h" #include "webrtc/modules/audio_processing/include/audio_processing.h" #include "webrtc/test/channel_transport/channel_transport.h" @@ -448,7 +447,6 @@ void RunTest(std::string out_path) { printf("%i. Set bit rate (only take effect on codecs that allow the " "change) \n", option_index++); printf("%i. Toggle AECdump recording \n", option_index++); - printf("%i. Record RtcEventLog file of 30 seconds \n", option_index++); printf("Select action or %i to stop the call: ", option_index); int option_selection; @@ -795,9 +793,6 @@ void RunTest(std::string out_path) { printf("Debug recording named %s started\n", kDebugFileName); } debug_recording_started = !debug_recording_started; - } else if (option_selection == option_index++) { - const char* kDebugFileName = "eventlog.rel"; - codec->GetEventLog()->StartLogging(kDebugFileName, 30000); } else { break; } diff --git a/webrtc/voice_engine/voe_codec_impl.cc b/webrtc/voice_engine/voe_codec_impl.cc index 5a16592e41..19891bc39f 100644 --- a/webrtc/voice_engine/voe_codec_impl.cc +++ b/webrtc/voice_engine/voe_codec_impl.cc @@ -376,10 +376,6 @@ int VoECodecImpl::SetOpusDtx(int channel, bool enable_dtx) { return channelPtr->SetOpusDtx(enable_dtx); } -RtcEventLog* VoECodecImpl::GetEventLog() { - return _shared->channel_manager().GetEventLog(); -} - #endif // WEBRTC_VOICE_ENGINE_CODEC_API } // namespace webrtc diff --git a/webrtc/voice_engine/voe_codec_impl.h b/webrtc/voice_engine/voe_codec_impl.h index 5095f6e232..5519232055 100644 --- a/webrtc/voice_engine/voe_codec_impl.h +++ b/webrtc/voice_engine/voe_codec_impl.h @@ -58,8 +58,6 @@ class VoECodecImpl : public VoECodec { int SetOpusDtx(int channel, bool enable_dtx) override; - RtcEventLog* GetEventLog() override; - protected: VoECodecImpl(voe::SharedData* shared); ~VoECodecImpl() override;