Refactoring: Merge VideoSenderConfig and AudioSenderConfig.

media/cast/cast_config.h

Index: media/cast/cast_config.h
diff --git a/media/cast/cast_config.h b/media/cast/cast_config.h
index 85ec3b917648bd2e3081f7fec74717ef2402bcde..4b9a4a3a6513991ac96f2760fea0ae9a25a82b43 100644
--- a/media/cast/cast_config.h
+++ b/media/cast/cast_config.h
@@ -44,65 +44,63 @@ enum SuggestedDefaults {
   // http://crbug.com/530839
   kDefaultRtpMaxDelayMs = 100,
 
-  // RTP payload types that identify an RTP stream as audio or video.
-  kDefaultRtpAudioPayloadType = 127,
-  kDefaultRtpVideoPayloadType = 96,
-
   // Suggested minimum and maximum video bitrates for general-purpose use (up to
   // 1080p, 30 FPS).
   kDefaultMinVideoKbps = 300,
   kDefaultMaxVideoKbps = 5000,
-};
 
-// TODO(miu): Merge AudioSenderConfig and VideoSenderConfig and make their
-// naming/documentation consistent with FrameReceiverConfig.
-// http://crbug.com/530839
-struct AudioSenderConfig {
-  AudioSenderConfig();
-  AudioSenderConfig(const AudioSenderConfig& other);
-  ~AudioSenderConfig();
+  // Minimum and Maximum VP8 quantizer in default configuration.
+  kDefaultMaxQp = 63,
+  kDefaultMinQp = 4,
 
-  // Identifier referring to the sender, used by the receiver.
-  uint32_t ssrc;
+  kDefaultMaxCpuSaverQp = 25,
 
-  // The receiver's SSRC identifier.
-  uint32_t receiver_ssrc;
+  // Number of video buffers in default configuration (applies only to certain
+  // external codecs).
+  kDefaultNumberOfVideoBuffers = 1,
+};
 
-  // The total amount of time between a frame's capture/recording on the sender
-  // and its playback on the receiver (i.e., shown to a user).  This should be
-  // set to a value large enough to give the system sufficient time to encode,
-  // transmit/retransmit, receive, decode, and render; given its run-time
-  // environment (sender/receiver hardware performance, network conditions,
-  // etc.).
-  base::TimeDelta min_playout_delay;
-  base::TimeDelta max_playout_delay;
+// These parameters are only for video encoders.
+struct CodecSpecificParams {
+  CodecSpecificParams();
+  CodecSpecificParams(const CodecSpecificParams& other);
+  ~CodecSpecificParams();
 
-  // Starting playout delay when streaming animated content.
-  base::TimeDelta animated_playout_delay;
+  int start_bitrate;
+  int max_qp;
+  int min_qp;
 
-  // RTP payload type enum: Specifies the type/encoding of frame data.
-  int rtp_payload_type;
+  // The maximum |min_quantizer| set to the encoder when CPU is constrained.
+  // This is a trade-off between higher resolution with lower encoding quality
+  // and lower resolution with higher encoding quality. The set value indicates
+  // the maximum quantizer that the encoder might produce better quality video
+  // at this resolution than lowering resolution with similar CPU usage and
+  // smaller quantizer. The set value has to be between |min_qp| and |max_qp|.
+  // Suggested value range: [4, 30]. It is only used by software VP8 codec.
+  int max_cpu_saver_qp;
 
-  bool use_external_encoder;
-  int frequency;
-  int channels;
-  int bitrate;  // Set to <= 0 for "auto variable bitrate" (libopus knows best).
-  Codec codec;
+  // This field is used differently by various encoders.
+  //
+  // It defaults to 1.
+  //
+  // For VP8, this field is ignored.
+  //
+  // For H.264 on Mac or iOS, it controls the max number of frames the encoder
+  // may hold before emitting a frame. A larger window may allow higher encoding
+  // efficiency at the cost of latency and memory. Set to 0 to let the encoder
+  // choose a suitable value for the platform and other encoding settings.
+  int max_number_of_video_buffers_used;
 
-  // The AES crypto key and initialization vector.  Each of these strings
-  // contains the data in binary form, of size kAesKeySize.  If they are empty
-  // strings, crypto is not being used.
-  std::string aes_key;
-  std::string aes_iv_mask;
+  int number_of_encode_threads;
 };
 
-struct VideoSenderConfig {
-  VideoSenderConfig();
-  VideoSenderConfig(const VideoSenderConfig& other);
-  ~VideoSenderConfig();
+struct FrameSenderConfig {
+  FrameSenderConfig();
+  FrameSenderConfig(const FrameSenderConfig& other);
+  ~FrameSenderConfig();
 
-  // Identifier referring to the sender, used by the receiver.
-  uint32_t ssrc;
+  // The sender's SSRC identifier.
+  uint32_t sender_ssrc;
 
   // The receiver's SSRC identifier.
   uint32_t receiver_ssrc;
@@ -120,46 +118,30 @@ struct VideoSenderConfig {
   base::TimeDelta animated_playout_delay;
 
   // RTP payload type enum: Specifies the type/encoding of frame data.
-  int rtp_payload_type;
+  RtpPayloadType rtp_payload_type;
 
   bool use_external_encoder;

[miu, 2016/06/30 21:59:43]

Let's add a comment like, "If true, use an external encoder rather than the
built-in software-based one."

[xjz, 2016/07/01 23:52:09]

Done.

 
-  float congestion_control_back_off;
-  int max_bitrate;
-  int min_bitrate;
-  int start_bitrate;
-  int max_qp;
-  int min_qp;
-
-  // The maximum |min_quantizer| set to the encoder when CPU is constrained.
-  // This is a trade-off between higher resolution with lower encoding quality
-  // and lower resolution with higher encoding quality. The set value indicates
-  // the maximum quantizer that the encoder might produce better quality video
-  // at this resolution than lowering resolution with similar CPU usage and
-  // smaller quantizer. The set value has to be between |min_qp| and |max_qp|.
-  // Suggested value range: [4, 30].
-  int max_cpu_saver_qp;
+  int frequency;

[miu, 2016/06/30 21:59:43]

Now might be a good time to rename this to rtp_timebase (like in
FrameReceiverConfig below).

Also, feel free to copy the comment from FrameReceiverConfig to here, and also
for other things like 'channels' and 'codec'.

[xjz, 2016/07/01 23:52:09]

Done.

 
-  int max_frame_rate;  // TODO(miu): Should be double, not int.
+  int channels;
 
-  // This field is used differently by various encoders. It defaults to 1.
-  //
-  // For VP8, this field is ignored.
-  //
-  // For H.264 on Mac or iOS, it controls the max number of frames the encoder
-  // may hold before emitting a frame. A larger window may allow higher encoding
-  // efficiency at the cost of latency and memory. Set to 0 to let the encoder
-  // choose a suitable value for the platform and other encoding settings.
-  int max_number_of_video_buffers_used;
+  // Audio streams only use |max_bitrate|.

[miu, 2016/06/30 21:59:43]

Probably not worth mentioning that audio uses only |max_bitrate|. In fact, maybe
we should add a TODO in AudioSender::InsertAudioFrame() to allow
dynamically-adjusting the bitrate based on congestion (just like we do for
VideoSender)?

Also, consider moving start_bitrate back to here.

[xjz, 2016/07/01 23:52:09]

Moved |start_bitrate| back to here.
Added comments for audio streams.

+  // Audio only: Set to <= 0 for "auto variable bitrate" (libopus knows best).
+  int max_bitrate;
+  int min_bitrate;
+  double max_frame_rate;

[miu, 2016/06/30 21:59:43]

Nice! Thanks for resolving the TODO! :)

[xjz, 2016/07/01 23:52:09]

:)

 
   Codec codec;
-  int number_of_encode_threads;
 
   // The AES crypto key and initialization vector.  Each of these strings
   // contains the data in binary form, of size kAesKeySize.  If they are empty
   // strings, crypto is not being used.
   std::string aes_key;
   std::string aes_iv_mask;
+
+  // These are codec specific parameters for video streams only.
+  CodecSpecificParams codec_specific_params;

[miu, 2016/06/30 21:59:43]

Please rename (for precision). Suggestion:

s/CodecSpecificParams/VideoCodecParams/

and

s/codec_specific_params/video_codec_params/

[xjz, 2016/07/01 23:52:09]

Done.

 };
 
 // TODO(miu): Naming and minor type changes are badly needed in a later CL.
@@ -183,7 +165,7 @@ struct FrameReceiverConfig {
   int rtp_max_delay_ms;  // TODO(miu): Change to TimeDelta target_playout_delay.
 
   // RTP payload type enum: Specifies the type/encoding of frame data.
-  int rtp_payload_type;
+  RtpPayloadType rtp_payload_type;
 
   // RTP timebase: The number of RTP units advanced per one second.  For audio,
   // this is the sampling rate.  For video, by convention, this is 90 kHz.