Keep config events in RtcEventLog even if they are old.

webrtc/video/rtc_event_log.h

 /*
  *  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.
  */
 
@@ skipping 20 matching lines @@
 
 class RtcEventLog {
  public:
   // The types of debug events that are currently supported for logging.
   enum class DebugEvent { kLogStart, kLogEnd, kAudioPlayout };
 
   virtual ~RtcEventLog() {}
 
   static rtc::scoped_ptr<RtcEventLog> Create();
 
+  // Sets the time that events are stored in the internal event buffer
+  // before the user calls StartLogging.  The default is 10'000'000 us = 10 s
+  virtual void SetBufferDuration(int64_t recent_log_duration_us) = 0;

[ivoc, 2015/08/25 08:03:21]

How useful is it to be able to change the size of the buffer at runtime? In
which cases would this function be called?

[terelius, 2015/08/25 16:41:44]

At the moment it is only used for testing purposes. I want to construct a unit
test where old events are removed from the queue, and waiting 10 seconds would
be impractical. In the future, there might be real use cases e.g. allocating
more memory for logging on high end devices.

I considered passing the logging duration to the constructor, but as we
discussed before we might have to change to a separate ring buffer for the RTP
and RTCP packets. (Using a separate buffer would avoid constructing protobuf
messages when not logging, would avoid dynamic memory allocation when queueing
an event, and would avoid lock contention between the audio and video threads.)
In that case, logging "duration" will be set based on memory usage rather than
time, and there is no good way to change the usage of constructor arguments.

[ivoc, 2015/08/26 08:15:28]

Hmm, I don't really see why there no good way to change constructor arguments,
compared to changing function arguments. To me it seems to be pretty similar. 

[terelius, 2015/08/26 08:43:37]

Using a function you can choose a new name, e.g. SetBufferMemorySize.
You can then either remove SetBufferDuration to cause compilation errors in code
that don't use the new API, or you can change SetBufferDuration to estimate the
required memory like this

void SetBufferDuration(seconds) {
  memory_size = seconds * packets_per_second * avg_packet_size
  SetBufferMemorySize(memory_size)
}

[ivoc, 2015/09/25 15:30:23]

How about creating multiple variants of the Create function? For example a
default one with a certain default buffer size, and a CreateWithBufferDuration
to set a certain buffer size explicitly. If the meaning should be changed later,
a new CreateWithBufferSize function can be added, and the old one can be
removed. This would have similar advantages as having a function, while still
having the buffer size/duration as a constant in the RtcEventLogImpl class
(which I would personally prefer). 

[terelius, 2015/10/15 13:28:21]

Following the discussions we had last week with Jiayang, I think we will have to
move to a memory limit rather than a time limit. With the current implementation
there is no way to construct the object based on a memory limit, so the
constructor would still have to take a duration as a parameter.

I suggest we hold off on this until we have the new buffers. Does this sound
reasonable?

[ivoc, 2015/10/15 15:52:06]

Okay, sounds good.

+
   // Starts logging for the specified duration to the specified file.
   // The logging will stop automatically after the specified duration.
   // If the file already exists it will be overwritten.
   // If the file cannot be opened, the RtcEventLog will not start logging.
   virtual void StartLogging(const std::string& file_name, int duration_ms) = 0;
 
   virtual void StopLogging() = 0;
 
   // Logs configuration information for webrtc::VideoReceiveStream
   virtual void LogVideoReceiveStreamConfig(
@@ skipping 21 matching lines @@
 
   // Reads an RtcEventLog file and returns true when reading was successful.
   // The result is stored in the given EventStream object.
   static bool ParseRtcEventLog(const std::string& file_name,
                                rtclog::EventStream* result);
 };
 
 }  // namespace webrtc
 
 #endif  // WEBRTC_VIDEO_RTC_EVENT_LOG_H_