[Cast] EncodedAudioFrame+EncodedVideoFrame+reference_time --> EncodedFrame

media/cast/transport/cast_transport_config.h

Index: media/cast/transport/cast_transport_config.h
diff --git a/media/cast/transport/cast_transport_config.h b/media/cast/transport/cast_transport_config.h
index 283b5341935cdc82314aba4accb339120fc75807..25c516434ce93c2b13282ab077741632da0a2db9 100644
--- a/media/cast/transport/cast_transport_config.h
+++ b/media/cast/transport/cast_transport_config.h
@@ -12,6 +12,7 @@
 #include "base/callback.h"
 #include "base/memory/linked_ptr.h"
 #include "base/memory/ref_counted.h"
+#include "base/stl_util.h"
 #include "media/cast/transport/cast_transport_defines.h"
 #include "net/base/ip_endpoint.h"
 
@@ -69,27 +70,46 @@ struct CastTransportVideoConfig {
   VideoCodec codec;
 };
 
-struct EncodedVideoFrame {
-  EncodedVideoFrame();
-  ~EncodedVideoFrame();
-
-  VideoCodec codec;
-  bool key_frame;
+// A combination of metadata and data for one encoded frame.  This can contain
+// audio data or video data or other.
+struct EncodedFrame {
+  EncodedFrame();
+  ~EncodedFrame();
+
+  // Convenience accessors to data as an array of uint8 elements.
+  const uint8* bytes() const {
+    return reinterpret_cast<uint8*>(string_as_array(
+        const_cast<std::string*>(&data)));
+  }
+  uint8* mutable_bytes() {
+    return reinterpret_cast<uint8*>(string_as_array(&data));
+  }
+
+  // The label associated with this frame.  Implies an ordering relative to
+  // other frames in the same stream.
   uint32 frame_id;
-  uint32 last_referenced_frame_id;
-  uint32 rtp_timestamp;
-  std::string data;
-};
 
-struct EncodedAudioFrame {
-  EncodedAudioFrame();
-  ~EncodedAudioFrame();
+  // The label associated with the frame upon which this frame is based.  If
+  // this frame does not require any other frame in order to become decodable
+  // (e.g., key frames), |base_frame_id| must equal |frame_id|.
+  uint32 base_frame_id;

[Alpha Left Google, 2014/05/15 22:11:23]

I actually prefer the name before, i.e. referenced_frame_id. Calling this
|base_something| doesn't describe it more accurately.

[miu, 2014/05/16 04:27:25]

Done.  After looking around more in the codec APIs, it looks like "reference
frame" is best for naming consistency.

 
-  AudioCodec codec;
-  uint32 frame_id;  // Needed to release the frame.
+  // The stream timestamp, on the timeline of the signal data.  For example, RTP
+  // timestamps for audio are usually defined as the total number of audio
+  // samples encoded in all prior frames.  A playback system uses this value to
+  // detect gaps in the stream, and otherwise stretch the signal to match
+  // playout targets.
   uint32 rtp_timestamp;
-  // Support for max sampling rate of 48KHz, 2 channels, 100 ms duration.
-  static const int kMaxNumberOfSamples = 48 * 2 * 100;
+
+  // Reference time used for synchronization between streams.  In the sender

[Alpha Left Google, 2014/05/15 22:11:23]

In the sender the time is not used for synchronization. It is only used for
ordering between different streams in the pacer.

[miu, 2014/05/16 04:27:25]

I've clarified the comment (that the sender generates this value for the
purposes of lip sync in the receiver).

+  // implementation, this is the frame capture time.  In the receiver
+  // implementation, this is the target playout time.  Over a sequence of
+  // frames, this time value is expected to drift with respect to the elapsed
+  // time implied by the RTP timestamps; and it may not necessarily increment
+  // with precise regularity.
+  base::TimeTicks reference_time;

[Alpha Left Google, 2014/05/15 22:11:23]

Calling it reference_time doesn't seem to fit either of the purpose in sender
and receiver. Wouldn't |timestamp| be enough? 

[miu, 2014/05/16 04:27:25]

This is exactly the terminology used in Chapter 7 in the RTP book (see the
timing diagram on page 212).  I've reworked the comment a bit to clarify things,
but I think the naming of this field makes sense.

+
+  // The encoded signal data.
   std::string data;
 };