Full impl of NnChannel::SetSendParameters and NnChannel::SetRecvParameters

talk/media/webrtc/webrtcvideoengine2.cc

Index: talk/media/webrtc/webrtcvideoengine2.cc
diff --git a/talk/media/webrtc/webrtcvideoengine2.cc b/talk/media/webrtc/webrtcvideoengine2.cc
index e94c0b1794919c854b26205fa26e912962db27b9..314bc3c5d66e59f23613680162d2807b8a9892f0 100644
--- a/talk/media/webrtc/webrtcvideoengine2.cc
+++ b/talk/media/webrtc/webrtcvideoengine2.cc
@@ -622,16 +622,9 @@ WebRtcVideoChannel2* WebRtcVideoEngine2::CreateChannel(
   LOG(LS_INFO) << "CreateChannel: "
                << (voice_channel != NULL ? "With" : "Without")
                << " voice channel. Options: " << options.ToString();
-  WebRtcVideoChannel2* channel =
-      new WebRtcVideoChannel2(call_factory_, voice_engine_,
-          static_cast<WebRtcVoiceMediaChannel*>(voice_channel), options,
-          external_encoder_factory_, external_decoder_factory_);
-  if (!channel->Init()) {
-    delete channel;
-    return NULL;
-  }
-  channel->SetRecvCodecs(video_codecs_);
-  return channel;
+  return new WebRtcVideoChannel2(call_factory_, voice_engine_,
+      static_cast<WebRtcVoiceMediaChannel*>(voice_channel), options,
+      video_codecs_, external_encoder_factory_, external_decoder_factory_);
 }
 
 const std::vector<VideoCodec>& WebRtcVideoEngine2::codecs() const {
@@ -792,6 +785,7 @@ WebRtcVideoChannel2::WebRtcVideoChannel2(
     WebRtcVoiceEngine* voice_engine,
     WebRtcVoiceMediaChannel* voice_channel,
     const VideoOptions& options,
+    const std::vector<VideoCodec>& recv_codecs,
     WebRtcVideoEncoderFactory* external_encoder_factory,
     WebRtcVideoDecoderFactory* external_decoder_factory)
     : unsignalled_ssrc_handler_(&default_unsignalled_ssrc_handler_),
@@ -817,6 +811,7 @@ WebRtcVideoChannel2::WebRtcVideoChannel2(
   rtcp_receiver_report_ssrc_ = kDefaultRtcpReceiverReportSsrc;
   sending_ = false;
   default_send_ssrc_ = 0;
+  SetRecvCodecs(recv_codecs);

[pthatcher1, 2015/09/14 19:42:02]

What if SetRecvCodecs fails?

 }
 
 void WebRtcVideoChannel2::SetDefaultOptions() {
@@ -835,8 +830,6 @@ WebRtcVideoChannel2::~WebRtcVideoChannel2() {
     delete kv.second;
 }
 
-bool WebRtcVideoChannel2::Init() { return true; }
-
 void WebRtcVideoChannel2::DetachVoiceChannel() {
   DCHECK(thread_checker_.CalledOnValidThread());
   if (voice_channel_) {
@@ -942,6 +935,9 @@ std::string WebRtcVideoChannel2::CodecSettingsVectorToString(
 bool WebRtcVideoChannel2::SetRecvCodecs(const std::vector<VideoCodec>& codecs) {
   TRACE_EVENT0("webrtc", "WebRtcVideoChannel2::SetRecvCodecs");
   LOG(LS_INFO) << "SetRecvCodecs: " << CodecVectorToString(codecs);
+  if (codecs.empty()) {
+    return true;
+  }

[pthatcher1, 2015/09/14 19:42:02]

I think this should be handled by the caller of SetRecvCodecs, not inside of
SetRecvCodecs.  Same with SetSendCodecs.

[the sun, 2015/09/14 19:58:00]

I was wondering about wether it should be at all legal to give an empty list of
codecs to Set[Send|Recv]Parameters(). Before this CL, none of the other state
will be changed if setting codecs fail, which it will in case of an empty list.

This was only changed once tests relied on it (the ones that do nothing but
SetOptions, etc). Another option would be to always set up a default codec in
the tests.

WDYT?

[pthatcher1, 2015/09/16 03:45:30]

I think leaving this here as-is just for unit tests is too risky.  I think we
have two options:

A.  The new contract is "you have to provide everything at once, including at
least one codec", then tests need to provide at least one codec.  

B.  The new contract is "you have to provide everything at once, but if there
are no codecs, we won't send anything".  

I could see a case for either.  I'm guessing it's easiest right now to do A and
update the tests to have at least one codec.

   if (!ValidateCodecFormats(codecs)) {
     return false;
   }
@@ -986,6 +982,9 @@ bool WebRtcVideoChannel2::SetRecvCodecs(const std::vector<VideoCodec>& codecs) {
 bool WebRtcVideoChannel2::SetSendCodecs(const std::vector<VideoCodec>& codecs) {
   TRACE_EVENT0("webrtc", "WebRtcVideoChannel2::SetSendCodecs");
   LOG(LS_INFO) << "SetSendCodecs: " << CodecVectorToString(codecs);
+  if (codecs.empty()) {
+    return true;
+  }
   if (!ValidateCodecFormats(codecs)) {
     return false;
   }