Implement VideoSendStream::SetCodec().

Removing assertion that SSRC count should be the same as the number of
streams in the codec. It makes sense that you don't always use the same
number of streams under one call. Dropping resolution due to CPU overuse
for instance can require less streams, but the SSRCs should stay
allocated so that operations can resume when not overusing any more.

This change also means we can get rid of the ugly SendStreamState whose
content wasn't defined. Instead we use SetCodec to change resolution
etc. on the fly. Should something else have to be replaced on the fly
then that functionality simply has to be implemented.

BUG=
R=mflodman@webrtc.org

Review URL: https://webrtc-codereview.appspot.com/3499005

git-svn-id: http://webrtc.googlecode.com/svn/trunk/webrtc@5123 4adac7df-926f-26a2-2b94-8c16560cd09d

6 files changed, 102 insertions, 47 deletions

diff --git a/call.cc b/call.cc
index 45f06eb1..ec42e3e4 100644
--- a/call.cc
+++ b/call.cc
@@ -41,8 +41,7 @@ class Call : public webrtc::Call, public PacketReceiver {
   virtual VideoSendStream* CreateSendStream(
       const VideoSendStream::Config& config) OVERRIDE;
 
-  virtual SendStreamState* DestroySendStream(
-      webrtc::VideoSendStream* send_stream) OVERRIDE;
+  virtual void DestroySendStream(webrtc::VideoSendStream* send_stream) OVERRIDE;
 
   virtual VideoReceiveStream::Config GetDefaultReceiveConfig() OVERRIDE;
 
@@ -228,7 +227,7 @@ VideoSendStream* Call::CreateSendStream(const VideoSendStream::Config& config) {
   return send_stream;
 }
 
-SendStreamState* Call::DestroySendStream(webrtc::VideoSendStream* send_stream) {
+void Call::DestroySendStream(webrtc::VideoSendStream* send_stream) {
   assert(send_stream != NULL);
 
   VideoSendStream* send_stream_impl = NULL;
@@ -248,9 +247,6 @@ SendStreamState* Call::DestroySendStream(webrtc::VideoSendStream* send_stream) {
 
   assert(send_stream_impl != NULL);
   delete send_stream_impl;
-
-  // TODO(pbos): Return its previous state
-  return NULL;
 }
 
 VideoReceiveStream::Config Call::GetDefaultReceiveConfig() {
diff --git a/call.h b/call.h
index 5239f5ac..5f4c7b61 100644
--- a/call.h
+++ b/call.h
@@ -66,7 +66,7 @@ class Call {
   // Returns the internal state of the send stream, for resume sending with a
   // new stream with different settings.
   // Note: Only the last returned send-stream state is valid.
-  virtual SendStreamState* DestroySendStream(VideoSendStream* send_stream) = 0;
+  virtual void DestroySendStream(VideoSendStream* send_stream) = 0;
 
   virtual VideoReceiveStream::Config GetDefaultReceiveConfig() = 0;
 
diff --git a/video/video_send_stream.cc b/video/video_send_stream.cc
index 8814c359..5ae1e941 100644
--- a/video/video_send_stream.cc
+++ b/video/video_send_stream.cc
@@ -82,14 +82,10 @@ VideoSendStream::VideoSendStream(newapi::Transport* transport,
                                  bool overuse_detection,
                                  webrtc::VideoEngine* video_engine,
                                  const VideoSendStream::Config& config)
-    : transport_adapter_(transport), config_(config), external_codec_(NULL) {
-
-  if (config_.codec.numberOfSimulcastStreams > 0) {
-    assert(config_.rtp.ssrcs.size() == config_.codec.numberOfSimulcastStreams);
-  } else {
-    assert(config_.rtp.ssrcs.size() == 1);
-  }
-
+    : transport_adapter_(transport),
+      codec_lock_(CriticalSectionWrapper::CreateCriticalSection()),
+      config_(config),
+      external_codec_(NULL) {
   video_engine_base_ = ViEBase::GetInterface(video_engine);
   video_engine_base_->CreateChannel(channel_);
   assert(channel_ != -1);
@@ -97,6 +93,7 @@ VideoSendStream::VideoSendStream(newapi::Transport* transport,
   rtp_rtcp_ = ViERTP_RTCP::GetInterface(video_engine);
   assert(rtp_rtcp_ != NULL);
 
+  assert(config_.rtp.ssrcs.size() > 0);
   if (config_.rtp.ssrcs.size() == 1) {
     rtp_rtcp_->SetLocalSSRC(channel_, config_.rtp.ssrcs[0]);
   } else {
@@ -186,9 +183,8 @@ VideoSendStream::VideoSendStream(newapi::Transport* transport,
   }
 
   codec_ = ViECodec::GetInterface(video_engine);
-  if (codec_->SetSendCodec(channel_, config_.codec) != 0) {
+  if (!SetCodec(config_.codec))
     abort();
-  }
 
   if (overuse_detection) {
     overuse_observer_.reset(
@@ -275,15 +271,21 @@ void VideoSendStream::StopSend() {
     abort();
 }
 
-bool VideoSendStream::SetTargetBitrate(
-    int min_bitrate,
-    int max_bitrate,
-    const std::vector<SimulcastStream>& streams) {
-  return false;
+bool VideoSendStream::SetCodec(const VideoCodec& codec) {
+  if (codec.numberOfSimulcastStreams > 0)
+    assert(config_.rtp.ssrcs.size() >= codec.numberOfSimulcastStreams);
+
+  CriticalSectionScoped crit(codec_lock_.get());
+  if (codec_->SetSendCodec(channel_, codec) != 0)
+    return false;
+
+  config_.codec = codec;
+  return true;
 }
 
-void VideoSendStream::GetSendCodec(VideoCodec* send_codec) {
-  *send_codec = config_.codec;
+VideoCodec VideoSendStream::GetCodec() {
+  CriticalSectionScoped crit(codec_lock_.get());
+  return config_.codec;
 }
 
 bool VideoSendStream::DeliverRtcp(const uint8_t* packet, size_t length) {
diff --git a/video/video_send_stream.h b/video/video_send_stream.h
index 304d825d..2d077e3a 100644
--- a/video/video_send_stream.h
+++ b/video/video_send_stream.h
@@ -17,6 +17,7 @@
 #include "webrtc/video/transport_adapter.h"
 #include "webrtc/video_receive_stream.h"
 #include "webrtc/video_send_stream.h"
+#include "webrtc/system_wrappers/interface/critical_section_wrapper.h"
 
 namespace webrtc {
 
@@ -53,17 +54,15 @@ class VideoSendStream : public webrtc::VideoSendStream,
 
   virtual void StopSend() OVERRIDE;
 
-  virtual bool SetTargetBitrate(int min_bitrate, int max_bitrate,
-                                const std::vector<SimulcastStream>& streams)
-      OVERRIDE;
-
-  virtual void GetSendCodec(VideoCodec* send_codec) OVERRIDE;
+  virtual bool SetCodec(const VideoCodec& codec) OVERRIDE;
+  virtual VideoCodec GetCodec() OVERRIDE;
 
  public:
   bool DeliverRtcp(const uint8_t* packet, size_t length);
 
  private:
   TransportAdapter transport_adapter_;
+  scoped_ptr<CriticalSectionWrapper> codec_lock_;
   VideoSendStream::Config config_;
 
   ViEBase* video_engine_base_;
diff --git a/video/video_send_stream_tests.cc b/video/video_send_stream_tests.cc
index 358ecf3f..f17d4a59 100644
--- a/video/video_send_stream_tests.cc
+++ b/video/video_send_stream_tests.cc
@@ -38,7 +38,9 @@ class SendTransportObserver : public test::NullTransport {
         send_test_complete_(EventWrapper::Create()),
         timeout_ms_(timeout_ms) {}
 
-  EventTypeWrapper Wait() { return send_test_complete_->Wait(timeout_ms_); }
+  virtual EventTypeWrapper Wait() {
+    return send_test_complete_->Wait(timeout_ms_);
+  }
 
   virtual void Stop() {}
 
@@ -58,19 +60,19 @@ class VideoSendStreamTest : public ::testing::Test {
   void RunSendTest(Call* call,
                    const VideoSendStream::Config& config,
                    SendTransportObserver* observer) {
-    VideoSendStream* send_stream = call->CreateSendStream(config);
+    send_stream_ = call->CreateSendStream(config);
     scoped_ptr<test::FrameGeneratorCapturer> frame_generator_capturer(
         test::FrameGeneratorCapturer::Create(
-            send_stream->Input(), 320, 240, 30, Clock::GetRealTimeClock()));
-    send_stream->StartSend();
+            send_stream_->Input(), 320, 240, 30, Clock::GetRealTimeClock()));
+    send_stream_->StartSend();
     frame_generator_capturer->Start();
 
     EXPECT_EQ(kEventSignaled, observer->Wait());
 
     observer->Stop();
     frame_generator_capturer->Stop();
-    send_stream->StopSend();
-    call->DestroySendStream(send_stream);
+    send_stream_->StopSend();
+    call->DestroySendStream(send_stream_);
   }
 
   VideoSendStream::Config GetSendTestConfig(Call* call) {
@@ -87,6 +89,7 @@ class VideoSendStreamTest : public ::testing::Test {
   static const uint32_t kSendSsrc;
   static const uint32_t kSendRtxSsrc;
 
+  VideoSendStream* send_stream_;
   test::FakeEncoder fake_encoder_;
 };
 
@@ -491,6 +494,71 @@ TEST_F(VideoSendStreamTest, MaxPacketSize) {
   RunSendTest(call.get(), send_config, &observer);
 }
 
+TEST_F(VideoSendStreamTest, CanChangeSendCodec) {
+  static const uint8_t kFirstPayloadType = 121;
+  static const uint8_t kSecondPayloadType = 122;
+
+  class CodecChangeObserver : public SendTransportObserver {
+   public:
+    CodecChangeObserver(VideoSendStream** send_stream_ptr)
+        : SendTransportObserver(30 * 1000),
+          received_first_payload_(EventWrapper::Create()),
+          send_stream_ptr_(send_stream_ptr) {}
+
+    virtual bool SendRTP(const uint8_t* packet, size_t length) OVERRIDE {
+      RTPHeader header;
+      EXPECT_TRUE(
+          rtp_header_parser_->Parse(packet, static_cast<int>(length), &header));
+
+      if (header.payloadType == kFirstPayloadType) {
+        received_first_payload_->Set();
+      } else if (header.payloadType == kSecondPayloadType) {
+        send_test_complete_->Set();
+      }
+
+      return true;
+    }
+
+    virtual EventTypeWrapper Wait() OVERRIDE {
+      EXPECT_EQ(kEventSignaled, received_first_payload_->Wait(30 * 1000))
+          << "Timed out while waiting for first payload.";
+
+      EXPECT_TRUE((*send_stream_ptr_)->SetCodec(second_codec_));
+
+      EXPECT_EQ(kEventSignaled, SendTransportObserver::Wait())
+          << "Timed out while waiting for second payload type.";
+
+      // Return OK regardless, prevents double error reporting.
+      return kEventSignaled;
+    }
+
+    void SetSecondCodec(const VideoCodec& codec) {
+      second_codec_ = codec;
+    }
+
+   private:
+    scoped_ptr<EventWrapper> received_first_payload_;
+    VideoSendStream** send_stream_ptr_;
+    VideoCodec second_codec_;
+  } observer(&send_stream_);
+
+  Call::Config call_config(&observer);
+  scoped_ptr<Call> call(Call::Create(call_config));
+
+  std::vector<VideoCodec> codecs = call->GetVideoCodecs();
+  ASSERT_GE(codecs.size(), 2u)
+      << "Test needs at least 2 separate codecs to work.";
+  codecs[0].plType = kFirstPayloadType;
+  codecs[1].plType = kSecondPayloadType;
+  observer.SetSecondCodec(codecs[1]);
+
+  VideoSendStream::Config send_config = GetSendTestConfig(call.get());
+  send_config.codec = codecs[0];
+  send_config.encoder = NULL;
+
+  RunSendTest(call.get(), send_config, &observer);
+}
+
 // The test will go through a number of phases.
 // 1. Start sending packets.
 // 2. As soon as the RTP stream has been detected, signal a low REMB value to
diff --git a/video_send_stream.h b/video_send_stream.h
index af33b7ca..2df282b4 100644
--- a/video_send_stream.h
+++ b/video_send_stream.h
@@ -23,8 +23,6 @@ namespace webrtc {
 
 class VideoEncoder;
 
-struct SendStreamState;
-
 // Class to deliver captured frame to the video send stream.
 class VideoSendStreamInput {
  public:
@@ -81,7 +79,6 @@ class VideoSendStream {
           target_delay_ms(0),
           pacing(false),
           stats_callback(NULL),
-          start_state(NULL),
           auto_mute(false) {}
     VideoCodec codec;
 
@@ -145,9 +142,6 @@ class VideoSendStream {
     // Callback for periodically receiving send stats.
     StatsCallback* stats_callback;
 
-    // Set to resume a previously destroyed send stream.
-    SendStreamState* start_state;
-
     // True if video should be muted when video goes under the minimum video
     // bitrate. Unless muted, video will be sent at a bitrate higher than
     // estimated available.
@@ -161,12 +155,8 @@ class VideoSendStream {
   virtual void StartSend() = 0;
   virtual void StopSend() = 0;
 
-  // TODO(mflodman) Change VideoCodec struct and use here.
-  virtual bool SetTargetBitrate(
-      int min_bitrate, int max_bitrate,
-      const std::vector<SimulcastStream>& streams) = 0;
-
-  virtual void GetSendCodec(VideoCodec* send_codec) = 0;
+  virtual bool SetCodec(const VideoCodec& codec) = 0;
+  virtual VideoCodec GetCodec() = 0;
 
  protected:
   virtual ~VideoSendStream() {}