Move RtcEventLog object from inside VoiceEngine to Call.

webrtc/voice_engine/channel.cc

Index: webrtc/voice_engine/channel.cc
diff --git a/webrtc/voice_engine/channel.cc b/webrtc/voice_engine/channel.cc
index 9e27ce8b7b7162905a036f6d52a639c96d17bfa0..f19eee43f0d2540e0f9581e408ecf7da5f23fb4d 100644
--- a/webrtc/voice_engine/channel.cc
+++ b/webrtc/voice_engine/channel.cc
@@ -21,6 +21,7 @@
 #include "webrtc/base/timeutils.h"
 #include "webrtc/common.h"
 #include "webrtc/config.h"
+#include "webrtc/call/rtc_event_log.h"
 #include "webrtc/modules/audio_device/include/audio_device.h"
 #include "webrtc/modules/audio_processing/include/audio_processing.h"
 #include "webrtc/modules/include/module_common_types.h"
@@ -47,6 +48,106 @@
 namespace webrtc {
 namespace voe {
 
+class RtcEventLogProxy : public webrtc::RtcEventLog {

[the sun, 2016/03/21 13:03:09]

final

[ivoc, 2016/03/22 13:44:55]

Done.

+ public:
+  RtcEventLogProxy() : event_log_(nullptr) {}
+
+  void SetBufferDuration(int64_t buffer_duration_us) {

[the sun, 2016/03/21 13:03:09]

"override" here and below

[ivoc, 2016/03/22 13:44:55]

Done.

+    rtc::CritScope lock(&crit_);
+    if (event_log_) {
+      event_log_->SetBufferDuration(buffer_duration_us);
+    }
+  }
+
+  void StartLogging(const std::string& file_name, int duration_ms) {

[the sun, 2016/03/21 13:03:09]

It seems to me this method will never be called on the proxy object. Only the
Log... functions will ever be called from the components the proxy is give to,
right?

If that's correct, remove the code and add RTC_NOTREACHED.

[ivoc, 2016/03/22 13:44:55]

You're correct, done.

+    rtc::CritScope lock(&crit_);
+    if (event_log_) {
+      event_log_->StartLogging(file_name, duration_ms);
+    }
+  }
+
+  bool StartLogging(rtc::PlatformFile log_file, int64_t max_size_bytes) {
+    rtc::CritScope lock(&crit_);
+    if (event_log_) {
+      return event_log_->StartLogging(log_file, max_size_bytes);
+    }
+    return false;
+  }
+
+  bool StartLogging(rtc::PlatformFile log_file) {
+    rtc::CritScope lock(&crit_);
+    if (event_log_) {
+      return event_log_->StartLogging(log_file);
+    }
+    return false;
+  }
+
+  void StopLogging() {
+    rtc::CritScope lock(&crit_);
+    if (event_log_) {
+      event_log_->StopLogging();
+    }
+  }
+
+  void LogVideoReceiveStreamConfig(
+      const webrtc::VideoReceiveStream::Config& config) {
+    rtc::CritScope lock(&crit_);
+    RTC_DCHECK(event_log_);
+    event_log_->LogVideoReceiveStreamConfig(config);
+  }
+
+  void LogVideoSendStreamConfig(const webrtc::VideoSendStream::Config& config) {
+    rtc::CritScope lock(&crit_);
+    RTC_DCHECK(event_log_);
+    event_log_->LogVideoSendStreamConfig(config);
+  }
+
+  void LogRtpHeader(webrtc::PacketDirection direction,
+                    webrtc::MediaType media_type,
+                    const uint8_t* header,
+                    size_t packet_length) {
+    rtc::CritScope lock(&crit_);
+    RTC_DCHECK(event_log_);

[the sun, 2016/03/21 13:03:09]

I don't see why you can DCHECK here but have to use a conditional above. Please
explain?

[ivoc, 2016/03/22 13:44:55]

I was thinking about the lifetimes of the AudioReceiveStream compared to the
lifetime of the objects calling the LogX() functions (i.e. VoE Channel). I
thought it might be possible for the LogX functions to be called after the
AudioReceiveStream is destroyed (which replaces the event_log_ with a nullptr),
but before the VoE Channel is destroyed.
Thinking about it a bit more, there should probably be conditionals everywhere
to prevent this issue.

[the sun, 2016/03/23 10:26:44]

Well, that, and we must also account for the scenario where the Channel is used
without an AudioSend/ReceiveStream.

+    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) {
+    rtc::CritScope lock(&crit_);
+    RTC_DCHECK(event_log_);
+    event_log_->LogRtcpPacket(direction, media_type, packet, length);
+  }
+
+  void LogAudioPlayout(uint32_t ssrc) {
+    rtc::CritScope lock(&crit_);
+    if (event_log_) {
+      event_log_->LogAudioPlayout(ssrc);
+    }
+  }
+
+  void LogBwePacketLossEvent(int32_t bitrate,
+                             uint8_t fraction_loss,
+                             int32_t total_packets) {
+    rtc::CritScope lock(&crit_);
+    RTC_DCHECK(event_log_);
+    event_log_->LogBwePacketLossEvent(bitrate, fraction_loss, total_packets);
+  }
+
+  static bool ParseRtcEventLog(const std::string& file_name,

[the sun, 2016/03/21 13:03:09]

Remove

[ivoc, 2016/03/22 13:44:55]

Done.

+                               webrtc::rtclog::EventStream* result);
+
+  void SetEventLog(RtcEventLog* event_log) {
+    rtc::CritScope lock(&crit_);
+    event_log_ = event_log;
+  }
+
+ private:
+  rtc::CriticalSection crit_;
+  RtcEventLog* event_log_ GUARDED_BY(crit_);

[the sun, 2016/03/21 13:03:09]

RTC_DISALLOW_COPY_AND_ASSIGN

[ivoc, 2016/03/22 13:44:55]

Done.

+};
+
 class TransportFeedbackProxy : public TransportFeedbackObserver {
  public:
   TransportFeedbackProxy() : feedback_observer_(nullptr) {
@@ -494,11 +595,9 @@ bool Channel::OnRecoveredPacket(const uint8_t* rtp_packet,
 }
 
 int32_t Channel::GetAudioFrame(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_->LogAudioPlayout(ssrc);
   // Get 10ms raw PCM data from the ACM (mixer limits output frequency)
   if (audio_coding_->PlayoutData10Ms(audioFrame->sample_rate_hz_, audioFrame) ==
       -1) {
@@ -674,13 +773,12 @@ 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) {
   WEBRTC_TRACE(kTraceMemory, kTraceVoice, VoEId(instanceId, channelId),
                "Channel::CreateChannel(channelId=%d, instanceId=%d)", channelId,
                instanceId);
 
-  channel = new Channel(channelId, instanceId, event_log, config);
+  channel = new Channel(channelId, instanceId, config);
   if (channel == NULL) {
     WEBRTC_TRACE(kTraceMemory, kTraceVoice, VoEId(instanceId, channelId),
                  "Channel::CreateChannel() unable to allocate memory for"
@@ -737,13 +835,10 @@ void Channel::RecordFileEnded(int32_t id) {
                " shutdown");
 }
 
-Channel::Channel(int32_t channelId,
-                 uint32_t instanceId,
-                 RtcEventLog* const event_log,
-                 const Config& config)
+Channel::Channel(int32_t channelId, uint32_t instanceId, const Config& config)
     : _instanceId(instanceId),
       _channelId(channelId),
-      event_log_(event_log),
+      event_log_(new RtcEventLogProxy()),
       rtp_header_parser_(RtpHeaderParser::Create()),
       rtp_payload_registry_(
           new RTPPayloadRegistry(RTPPayloadStrategy::CreateStrategy(true))),
@@ -852,7 +947,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_;

[the sun, 2016/03/21 13:03:09]

nit: add () around *event_log_ for clarity.

[ivoc, 2016/03/22 13:44:55]

Done.

 
   _rtpRtcpModule.reset(RtpRtcp::CreateRtpRtcp(configuration));
 
@@ -3086,6 +3181,10 @@ void Channel::DisassociateSendChannel(int channel_id) {
   }
 }
 
+void Channel::SetRtcEventLog(RtcEventLog* event_log) {
+  event_log_->SetEventLog(event_log);
+}
+
 int Channel::RegisterExternalMediaProcessing(ProcessingTypes type,
                                              VoEMediaProcess& processObject) {
   WEBRTC_TRACE(kTraceInfo, kTraceVoice, VoEId(_instanceId, _channelId),