Added protobuf message for loss-based BWE events, and wired it up to the send side bandwidth estima…

webrtc/call/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 55 matching lines @@
 
   // Logs an incoming or outgoing RTCP packet.
   virtual void LogRtcpPacket(bool incoming,
                              MediaType media_type,
                              const uint8_t* packet,
                              size_t length) = 0;
 
   // Logs an audio playout event
   virtual void LogAudioPlayout(uint32_t ssrc) = 0;
 
+  // Logs a bitrate update from the bandwidth estimator based on packet loss.
+  virtual void LogBwePacketLossEvent(int32_t bitrate,

[stefan-webrtc, 2015/10/19 08:43:21]

I think we can make this a regular int. Same for total_packets, and maybe even
for fraction_loss?

[terelius, 2015/10/26 17:40:18]

The type here should correspond to the protobuf I think. Since the bitrate is an
int32 in the protobuf, there will be an implicit typecast to int32_t when we
access the field.

The fraction lost field is specified in RFC 3550 as an 8 bit integer. (What we
are actually logging is an aggregate of several packets, but the variable still
seems to be an uint8_t in all places it is used.)

[stefan-webrtc, 2015/10/28 16:03:18]

That is true, although I think it's mostly legacy and there isn't any real
reason for using uint8_t or int32_t here. The styleguide suggests using int in
most cases except for sizes and in case you actually need a certain size.

I don't feel very strongly about this though, so if you think it makes sense to
have same types as in the protobuf, then go ahead.

[terelius, 2015/10/30 10:51:40]

It is mostly that I don't think it makes sense to have an unsigned variable,
cast it to signed only to immediately cast it back to unsigned to write it in
the protobuf.

In general, I prefer to make the type casts visible to the user by using the
same type in the logging API as in the protobuf. We could conceivably change the
protobuf to allow signed ints for the fraction loss though, but since the type
is specified in the RFC it  seemed fairly safe to assume that it would not
change.

[stefan-webrtc, 2015/10/30 10:58:04]

Agree it doesn't make sense to cast just to cast back later, so let's keep as is
then.

+                                     uint8_t fraction_loss,
+                                     int32_t total_packets) = 0;
+
   // 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_CALL_RTC_EVENT_LOG_H_