Adds logging of configuration information for VideoReceiveStream

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

Index: webrtc/modules/audio_coding/main/acm2/acm_dump.h
diff --git a/webrtc/modules/audio_coding/main/acm2/acm_dump.h b/webrtc/modules/audio_coding/main/acm2/acm_dump.h
index c72c3870965f4883f68a0ca10d06ee91337f8463..2051bf486766f079fd8c9fda35adbdea8eb83df5 100644
--- a/webrtc/modules/audio_coding/main/acm2/acm_dump.h
+++ b/webrtc/modules/audio_coding/main/acm2/acm_dump.h
@@ -14,6 +14,9 @@
 #include <string>
 
 #include "webrtc/base/scoped_ptr.h"
+#include "webrtc/video_receive_stream.h"
+#include "webrtc/video_send_stream.h"
+#include "webrtc/call.h"  // For MediaType definition

[ivoc, 2015/07/14 12:13:14]

I think it would be a good idea to move these includes to the .cc file, and use
forward declarations for the types that are used on the interface. That should
lead to faster compilation.

[terelius, 2015/07/16 12:47:03]

Good idea, unfortunately it seems difficult to do. We actually use the
definition of MediaType from call.h. From the other headers we only need the
declarations of VideoReceiveStream::Config and VideoSendStream::Config, but
afaik it is not possible to forward declare nested classes outside of the class
where they are defined.

[ivoc, 2015/07/17 12:14:28]

I think forward declaring MediaType should work at least. Can you try it?

 
 namespace webrtc {
 
@@ -36,12 +39,30 @@ class AcmDump {
   // The logging will stop automatically after the specified duration.
   // If the file already exists it will be overwritten.
   // The function will return false on failure.
+  // TODO(terelius): Can't return false because declared as void. Fix
+  // documentation?

[ivoc, 2015/07/14 12:13:14]

Sounds like a good idea, I probably forgot to change it when changing the return
value at some point :-)

[terelius, 2015/07/16 12:47:03]

Done.

   virtual void StartLogging(const std::string& file_name, int duration_ms) = 0;
 
-  // Logs an incoming or outgoing RTP packet.
-  virtual void LogRtpPacket(bool incoming,
-                            const uint8_t* packet,
-                            size_t length) = 0;
+  // Logs configuration information for webrtc::VideoReceiveStream
+  virtual void LogVideoReceiveStreamConfig(
+      const webrtc::VideoReceiveStream::Config& config) = 0;
+
+  // Logs configuration information for webrtc::VideoSendStream
+  virtual void LogVideoSendStreamConfig(
+      const webrtc::VideoSendStream::Config& config) = 0;
+
+  // Logs the header of an incoming or outgoing RTP packet.
+  virtual void LogRtpHeader(bool incoming,
+                            MediaType media_type,

[ivoc, 2015/07/14 12:13:14]

media_type should be a const reference.

[stefan-webrtc, 2015/07/14 13:28:56]

Why would we do that? Seems like MediaType is a very simple type that wouldn't
be harmful to pass by value.

[ivoc, 2015/07/14 13:58:03]

Oh, I didn't actually look at the definition of MediaType. If it's small and
simple then it makes sense to pass by value.

[terelius, 2015/07/16 12:47:03]

I'll keep passing by value unless a change of types would motivate using
references.

+                            const uint8_t* header,
+                            size_t header_length,
+                            size_t total_length) = 0;
+
+  // Logs an incoming or outgoing RTCP packet.
+  virtual void LogRtcpPacket(bool incoming,
+                             MediaType media_type,

[ivoc, 2015/07/14 12:13:14]

Same here.

[terelius, 2015/07/16 12:47:02]

I'll keep it as-is for the time being.

+                             const uint8_t* packet,
+                             size_t length) = 0;
 
   // Logs a debug event, with optional message.
   virtual void LogDebugEvent(DebugEvent event_type,