Keep config events in RtcEventLog even if they are old.

webrtc/video/rtc_event_log.h

Index: webrtc/video/rtc_event_log.h
diff --git a/webrtc/video/rtc_event_log.h b/webrtc/video/rtc_event_log.h
index a6bf2e3d8836d2b7e0ee6f0ca702f56ffc48c366..5cf0a326d6f14881881ff608dcab1c7a66118e62 100644
--- a/webrtc/video/rtc_event_log.h
+++ b/webrtc/video/rtc_event_log.h
@@ -38,6 +38,10 @@ class 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.