Added support for keeping a buffer of the previous X seconds, to add to

webrtc/modules/audio_coding/main/acm2/acm_dump.cc

 /*
  *  Copyright (c) 2015 The WebRTC project authors. All Rights Reserved.
  *
  *  Use of this source code is governed by a BSD-style license
  *  that can be found in the LICENSE file in the root of the source
  *  tree. An additional intellectual property rights grant can be found
  *  in the file PATENTS.  All contributing project authors may
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
 #include "webrtc/modules/audio_coding/main/acm2/acm_dump.h"
 
-#include <sstream>
+#include <deque>
 
 #include "webrtc/base/checks.h"
 #include "webrtc/base/thread_annotations.h"
 #include "webrtc/system_wrappers/interface/clock.h"
 #include "webrtc/system_wrappers/interface/critical_section_wrapper.h"
 #include "webrtc/system_wrappers/interface/file_wrapper.h"
 
 // Files generated at build-time by the protobuf compiler.
 #ifdef WEBRTC_ANDROID_PLATFORM_BUILD
 #include "external/webrtc/webrtc/modules/audio_coding/dump.pb.h"
@@ skipping 36 matching lines @@
   void Clear() EXCLUSIVE_LOCKS_REQUIRED(crit_);
   // Returns true if the logging is currently active.
   bool CurrentlyLogging() const EXCLUSIVE_LOCKS_REQUIRED(crit_) {
     return active_ &&
            (clock_->TimeInMicroseconds() <= start_time_us_ + duration_us_);
   }
   // This function is identical to LogDebugEvent, but requires holding the lock.
   void LogDebugEventLocked(DebugEvent event_type,
                            const std::string& event_message)
       EXCLUSIVE_LOCKS_REQUIRED(crit_);
+  // Writes the event to the file. Note that this will destroy the state of the
+  // input argument.
+  void StoreToFile(ACMDumpEvent* event) EXCLUSIVE_LOCKS_REQUIRED(crit_);
+  // Adds the event to the list of recent events, and removes any events that
+  // are too old and no longer fall in the time window.
+  void AddRecentEvent(const ACMDumpEvent& event)
+      EXCLUSIVE_LOCKS_REQUIRED(crit_);
+
+  // Amount of time in microseconds to record log events, before starting the
+  // actual log.
+  const int recent_log_duration_us = 10000000;
 
   rtc::scoped_ptr<webrtc::CriticalSectionWrapper> crit_;
   rtc::scoped_ptr<webrtc::FileWrapper> file_ GUARDED_BY(crit_);
   rtc::scoped_ptr<ACMDumpEventStream> stream_ GUARDED_BY(crit_);
+  std::deque<ACMDumpEvent> recent_log_events_ GUARDED_BY(crit_);
   bool active_ GUARDED_BY(crit_);
   int64_t start_time_us_ GUARDED_BY(crit_);
   int64_t duration_us_ GUARDED_BY(crit_);
   const webrtc::Clock* clock_ GUARDED_BY(crit_);

[stefan-webrtc, 2015/06/25 14:55:44]

No need to guard this since it's const, right?

[kwiberg-webrtc, 2015/06/25 14:58:59]

Sounds reasonable. But then the actual pointer probably ought to be const too,
if possible.

[ivoc, 2015/06/25 15:14:48]

Okay, that makes sense, I made the pointer const as well like Karl suggested.

 };
 
 namespace {
 
 // Convert from AcmDump's debug event enum (runtime format) to the corresponding
 // protobuf enum (serialized format).
 ACMDumpDebugEvent_EventType convertDebugEvent(AcmDump::DebugEvent event_type) {
   switch (event_type) {
     case AcmDump::DebugEvent::kLogStart:
       return ACMDumpDebugEvent::LOG_START;
@@ skipping 17 matching lines @@
       duration_us_(0),
       clock_(webrtc::Clock::GetRealTimeClock()) {
 }
 
 void AcmDumpImpl::StartLogging(const std::string& file_name, int duration_ms) {
   CriticalSectionScoped lock(crit_.get());
   Clear();
   if (file_->OpenFile(file_name.c_str(), false) != 0) {
     return;
   }
-  // Add a single object to the stream that is reused at every log event.
-  stream_->add_stream();
+  // Add LOG_START event to the recent event list. This call will also remove
+  // any events that are too old from the recent event list.
+  LogDebugEventLocked(DebugEvent::kLogStart, "");
   active_ = true;
   start_time_us_ = clock_->TimeInMicroseconds();
   duration_us_ = static_cast<int64_t>(duration_ms) * 1000;
-  // Log the start event.
-  std::stringstream log_msg;
-  log_msg << "Initial timestamp: " << start_time_us_;
-  LogDebugEventLocked(DebugEvent::kLogStart, log_msg.str());
+  // Write all the recent events to the log file.
+  for (auto&& event : recent_log_events_) {
+    StoreToFile(&event);
+  }
+  recent_log_events_.clear();
 }
 
 void AcmDumpImpl::LogRtpPacket(bool incoming,
                                const uint8_t* packet,
                                size_t length) {
   CriticalSectionScoped lock(crit_.get());
-  if (!CurrentlyLogging()) {
+  ACMDumpEvent rtp_event;
+  rtp_event.clear_debug_event();

[stefan-webrtc, 2015/06/25 14:55:44]

Isn't the event clear when it is created?

[ivoc, 2015/06/25 15:14:48]

You're right, this is an artifact from when we reused the same event object each
time. Will remove.

+  const int64_t timestamp = clock_->TimeInMicroseconds();
+  rtp_event.set_timestamp_us(timestamp);
+  rtp_event.set_type(webrtc::ACMDumpEvent::RTP_EVENT);

[stefan-webrtc, 2015/06/25 14:55:44]

Would it make sense to have these in the constructor of the event instead?

[ivoc, 2015/06/25 15:14:48]

That would make sense, but this class is automatically generated from the
protobuf file, and it doesn't have a convenient constructor like that.

[stefan-webrtc, 2015/06/25 15:18:22]

Ah, that makes sense.

+  rtp_event.mutable_packet()->set_direction(
+      incoming ? ACMDumpRTPPacket::INCOMING : ACMDumpRTPPacket::OUTGOING);
+  rtp_event.mutable_packet()->set_rtp_data(packet, length);
+  if (CurrentlyLogging()) {
+    StoreToFile(&rtp_event);
+  } else {
     StopIfNecessary();
-    return;
+    AddRecentEvent(rtp_event);
   }
-  // Reuse the same object at every log event.
-  auto rtp_event = stream_->mutable_stream(0);
-  rtp_event->clear_debug_event();
-  const int64_t timestamp = clock_->TimeInMicroseconds() - start_time_us_;
-  rtp_event->set_timestamp_us(timestamp);
-  rtp_event->set_type(webrtc::ACMDumpEvent::RTP_EVENT);
-  rtp_event->mutable_packet()->set_direction(
-      incoming ? ACMDumpRTPPacket::INCOMING : ACMDumpRTPPacket::OUTGOING);
-  rtp_event->mutable_packet()->set_rtp_data(packet, length);
-  std::string dump_buffer;
-  stream_->SerializeToString(&dump_buffer);
-  file_->Write(dump_buffer.data(), dump_buffer.size());
-  file_->Flush();
 }
 
 void AcmDumpImpl::LogDebugEvent(DebugEvent event_type,
                                 const std::string& event_message) {
   CriticalSectionScoped lock(crit_.get());
   LogDebugEventLocked(event_type, event_message);
 }
 
 void AcmDumpImpl::LogDebugEvent(DebugEvent event_type) {
   CriticalSectionScoped lock(crit_.get());
@@ skipping 11 matching lines @@
 void AcmDumpImpl::Clear() {
   if (active_ || file_->Open()) {
     file_->CloseFile();
   }
   active_ = false;
   stream_->Clear();
 }
 
 void AcmDumpImpl::LogDebugEventLocked(DebugEvent event_type,
                                       const std::string& event_message) {
-  if (!CurrentlyLogging()) {
-    StopIfNecessary();
-    return;
-  }
-
-  // Reuse the same object at every log event.
-  auto event = stream_->mutable_stream(0);
-  int64_t timestamp = clock_->TimeInMicroseconds() - start_time_us_;
-  event->set_timestamp_us(timestamp);
-  event->set_type(webrtc::ACMDumpEvent::DEBUG_EVENT);
-  event->clear_packet();
-  auto debug_event = event->mutable_debug_event();
+  ACMDumpEvent event;
+  int64_t timestamp = clock_->TimeInMicroseconds();
+  event.set_timestamp_us(timestamp);
+  event.set_type(webrtc::ACMDumpEvent::DEBUG_EVENT);
+  event.clear_packet();
+  auto debug_event = event.mutable_debug_event();
   debug_event->set_type(convertDebugEvent(event_type));
   debug_event->set_message(event_message);
+  if (CurrentlyLogging()) {
+    StoreToFile(&event);
+  } else {
+    StopIfNecessary();
+    AddRecentEvent(event);
+  }
+}
+
+void AcmDumpImpl::StoreToFile(ACMDumpEvent* event) {
+  // Reuse the same object at every log event.
+  if (stream_->stream_size() < 1) {
+    stream_->add_stream();
+  }
+  stream_->mutable_stream(0)->Swap(event);

[terelius, 2015/06/25 16:21:03]

The stream must not contain more than one event, or we'll write a bunch of old
events to the ACMDump. Maybe add an assertion that this is the case?

[ivoc, 2015/06/26 08:08:56]

Done.

+
   std::string dump_buffer;
   stream_->SerializeToString(&dump_buffer);
   file_->Write(dump_buffer.data(), dump_buffer.size());
 }
 
+void AcmDumpImpl::AddRecentEvent(const ACMDumpEvent& event) {
+  recent_log_events_.push_back(event);
+  while (recent_log_events_.front().timestamp_us() <
+         event.timestamp_us() - recent_log_duration_us) {
+    recent_log_events_.pop_front();
+  }
+}
+
 #endif  // RTC_AUDIOCODING_DEBUG_DUMP
 
 // AcmDump member functions.
 rtc::scoped_ptr<AcmDump> AcmDump::Create() {
   return rtc::scoped_ptr<AcmDump>(new AcmDumpImpl());
 }
 
 bool AcmDump::ParseAcmDump(const std::string& file_name,
                            ACMDumpEventStream* result) {
   char tmp_buffer[1024];
   int bytes_read = 0;
   rtc::scoped_ptr<FileWrapper> dump_file(FileWrapper::Create());
   if (dump_file->OpenFile(file_name.c_str(), true) != 0) {
     return false;
   }
   std::string dump_buffer;
   while ((bytes_read = dump_file->Read(tmp_buffer, sizeof(tmp_buffer))) > 0) {
     dump_buffer.append(tmp_buffer, bytes_read);
   }
   dump_file->CloseFile();
   return result->ParseFromString(dump_buffer);
 }
 
 }  // namespace webrtc