Refactor MediaOptimization protection methods.

Makes MediaOptimization::EnableProtectionMethod significantly less
confusing. Also removing some dead methods in VideoSender.

BUG=
R=mflodman@webrtc.org
TBR=stefan@webrtc.org

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

Cr-Commit-Position: refs/heads/master@{#8693}
git-svn-id: http://webrtc.googlecode.com/svn/trunk@8693 4adac7df-926f-26a2-2b94-8c16560cd09d
diff --git a/webrtc/modules/video_coding/main/interface/video_coding.h b/webrtc/modules/video_coding/main/interface/video_coding.h
index 859f285..4d01da9 100644
--- a/webrtc/modules/video_coding/main/interface/video_coding.h
+++ b/webrtc/modules/video_coding/main/interface/video_coding.h
@@ -528,38 +528,6 @@
 
     // Robustness APIs
 
-    // Set the sender RTX/NACK mode.
-    // Input:
-    //      - mode       : the selected NACK mode.
-    //
-    // Return value      : VCM_OK, on success;
-    //                     < 0, on error.
-    virtual int SetSenderNackMode(SenderNackMode mode) = 0;
-
-    // Set the sender reference picture selection (RPS) mode.
-    // Input:
-    //      - enable     : true or false, for enable and disable, respectively.
-    //
-    // Return value      : VCM_OK, on success;
-    //                     < 0, on error.
-    virtual int SetSenderReferenceSelection(bool enable) = 0;
-
-    // Set the sender forward error correction (FEC) mode.
-    // Input:
-    //      - enable     : true or false, for enable and disable, respectively.
-    //
-    // Return value      : VCM_OK, on success;
-    //                     < 0, on error.
-    virtual int SetSenderFEC(bool enable) = 0;
-
-    // Set the key frame period, or disable periodic key frames (I-frames).
-    // Input:
-    //      - periodMs   : period in ms; <= 0 to disable periodic key frames.
-    //
-    // Return value      : VCM_OK, on success;
-    //                     < 0, on error.
-    virtual int SetSenderKeyFramePeriod(int periodMs) = 0;
-
     // Set the receiver robustness mode. The mode decides how the receiver
     // responds to losses in the stream. The type of counter-measure (soft or
     // hard NACK, dual decoder, RPS, etc.) is selected through the
diff --git a/webrtc/modules/video_coding/main/interface/video_coding_defines.h b/webrtc/modules/video_coding/main/interface/video_coding_defines.h
index ea47e31..50478ca 100644
--- a/webrtc/modules/video_coding/main/interface/video_coding_defines.h
+++ b/webrtc/modules/video_coding/main/interface/video_coding_defines.h
@@ -46,6 +46,7 @@
 enum { kDefaultStartBitrateKbps = 300 };
 
 enum VCMVideoProtection {
+  kProtectionNone,
   kProtectionNack,                // Both send-side and receive-side
   kProtectionNackSender,          // Send-side only
   kProtectionNackReceiver,        // Receive-side only
@@ -53,7 +54,6 @@
   kProtectionNackFEC,
   kProtectionKeyOnLoss,
   kProtectionKeyOnKeyLoss,
-  kProtectionPeriodicKeyFrames
 };
 
 enum VCMTemporalDecimation {
diff --git a/webrtc/modules/video_coding/main/source/media_opt_util.cc b/webrtc/modules/video_coding/main/source/media_opt_util.cc
index 79e1268..2aa599d 100644
--- a/webrtc/modules/video_coding/main/source/media_opt_util.cc
+++ b/webrtc/modules/video_coding/main/source/media_opt_util.cc
@@ -670,61 +670,30 @@
     Release();
 }
 
-bool
-VCMLossProtectionLogic::SetMethod(enum VCMProtectionMethodEnum newMethodType)
-{
-    if (_selectedMethod != NULL)
-    {
-        if (_selectedMethod->Type() == newMethodType)
-        {
-            // Nothing to update
-            return false;
-        }
-        // New method - delete existing one
-        delete _selectedMethod;
-    }
-    VCMProtectionMethod *newMethod = NULL;
-    switch (newMethodType)
-    {
-        case kNack:
-        {
-            newMethod = new VCMNackMethod();
-            break;
-        }
-        case kFec:
-        {
-            newMethod  = new VCMFecMethod();
-            break;
-        }
-        case kNackFec:
-        {
-            // Default to always having NACK enabled for the hybrid mode.
-            newMethod =  new VCMNackFecMethod(kLowRttNackMs, -1);
-            break;
-        }
-        default:
-        {
-          return false;
-          break;
-        }
+void VCMLossProtectionLogic::SetMethod(
+    enum VCMProtectionMethodEnum newMethodType) {
+  if (_selectedMethod != nullptr) {
+    if (_selectedMethod->Type() == newMethodType)
+      return;
+    // Remove old method.
+    delete _selectedMethod;
+  }
 
-    }
-    _selectedMethod = newMethod;
-    return true;
-}
-bool
-VCMLossProtectionLogic::RemoveMethod(enum VCMProtectionMethodEnum method)
-{
-    if (_selectedMethod == NULL)
-    {
-        return false;
-    }
-    else if (_selectedMethod->Type() == method)
-    {
-        delete _selectedMethod;
-        _selectedMethod = NULL;
-    }
-    return true;
+  switch(newMethodType) {
+    case kNack:
+      _selectedMethod = new VCMNackMethod();
+      break;
+    case kFec:
+      _selectedMethod = new VCMFecMethod();
+      break;
+    case kNackFec:
+      _selectedMethod = new VCMNackFecMethod(kLowRttNackMs, -1);
+      break;
+    case kNone:
+      _selectedMethod = nullptr;
+      break;
+  }
+  UpdateMethod();
 }
 
 float
@@ -924,10 +893,8 @@
     return _selectedMethod;
 }
 
-VCMProtectionMethodEnum
-VCMLossProtectionLogic::SelectedType() const
-{
-    return _selectedMethod->Type();
+VCMProtectionMethodEnum VCMLossProtectionLogic::SelectedType() const {
+  return _selectedMethod == nullptr ? kNone : _selectedMethod->Type();
 }
 
 void
diff --git a/webrtc/modules/video_coding/main/source/media_opt_util.h b/webrtc/modules/video_coding/main/source/media_opt_util.h
index 3b6fa8f..ac4e971 100644
--- a/webrtc/modules/video_coding/main/source/media_opt_util.h
+++ b/webrtc/modules/video_coding/main/source/media_opt_util.h
@@ -245,15 +245,7 @@
     // Input:
     //        - newMethodType    : New requested protection method type. If one
     //                           is already set, it will be deleted and replaced
-    // Return value:             Returns true on update
-    bool SetMethod(VCMProtectionMethodEnum newMethodType);
-
-    // Remove requested protection method
-    // Input:
-    //        - method          : method to be removed (if currently selected)
-    //
-    // Return value:             Returns true on update
-    bool RemoveMethod(VCMProtectionMethodEnum method);
+    void SetMethod(VCMProtectionMethodEnum newMethodType);
 
     // Return required bit rate per selected protectin method
     float RequiredBitRate() const;
diff --git a/webrtc/modules/video_coding/main/source/media_optimization.cc b/webrtc/modules/video_coding/main/source/media_optimization.cc
index 1151d5b..1f0eb5c 100644
--- a/webrtc/modules/video_coding/main/source/media_optimization.cc
+++ b/webrtc/modules/video_coding/main/source/media_optimization.cc
@@ -321,15 +321,11 @@
 void MediaOptimization::EnableProtectionMethod(bool enable,
                                                VCMProtectionMethodEnum method) {
   CriticalSectionScoped lock(crit_sect_.get());
-  bool updated = false;
-  if (enable) {
-    updated = loss_prot_logic_->SetMethod(method);
-  } else {
-    loss_prot_logic_->RemoveMethod(method);
-  }
-  if (updated) {
-    loss_prot_logic_->UpdateMethod();
-  }
+  if (!enable && loss_prot_logic_->SelectedType() != method)
+    return;
+  if (!enable)
+    method = kNone;
+  loss_prot_logic_->SetMethod(method);
 }
 
 uint32_t MediaOptimization::InputFrameRate() {
diff --git a/webrtc/modules/video_coding/main/source/video_coding_impl.cc b/webrtc/modules/video_coding/main/source/video_coding_impl.cc
index 4f86153..c2741ae 100644
--- a/webrtc/modules/video_coding/main/source/video_coding_impl.cc
+++ b/webrtc/modules/video_coding/main/source/video_coding_impl.cc
@@ -173,13 +173,8 @@
 
   int32_t SetVideoProtection(VCMVideoProtection videoProtection,
                              bool enable) override {
-    int32_t sender_return =
-        sender_->SetVideoProtection(videoProtection, enable);
-    int32_t receiver_return =
-        receiver_->SetVideoProtection(videoProtection, enable);
-    if (sender_return == VCM_OK)
-      return receiver_return;
-    return sender_return;
+    sender_->SetVideoProtection(enable, videoProtection);
+    return receiver_->SetVideoProtection(videoProtection, enable);
   }
 
   int32_t AddVideoFrame(const I420VideoFrame& videoFrame,
@@ -201,22 +196,6 @@
     return sender_->SentFrameCount(&frameCount);
   }
 
-  int SetSenderNackMode(SenderNackMode mode) override {
-    return sender_->SetSenderNackMode(mode);
-  }
-
-  int SetSenderReferenceSelection(bool enable) override {
-    return sender_->SetSenderReferenceSelection(enable);
-  }
-
-  int SetSenderFEC(bool enable) override {
-    return sender_->SetSenderFEC(enable);
-  }
-
-  int SetSenderKeyFramePeriod(int periodMs) override {
-    return sender_->SetSenderKeyFramePeriod(periodMs);
-  }
-
   int StartDebugRecording(const char* file_name_utf8) override {
     return sender_->StartDebugRecording(file_name_utf8);
   }
diff --git a/webrtc/modules/video_coding/main/source/video_coding_impl.h b/webrtc/modules/video_coding/main/source/video_coding_impl.h
index df38ad7..bb643bf 100644
--- a/webrtc/modules/video_coding/main/source/video_coding_impl.h
+++ b/webrtc/modules/video_coding/main/source/video_coding_impl.h
@@ -101,7 +101,7 @@
   int32_t RegisterSendStatisticsCallback(VCMSendStatisticsCallback* sendStats);
   int32_t RegisterVideoQMCallback(VCMQMSettingsCallback* videoQMSettings);
   int32_t RegisterProtectionCallback(VCMProtectionCallback* protection);
-  int32_t SetVideoProtection(VCMVideoProtection videoProtection, bool enable);
+  void SetVideoProtection(bool enable, VCMVideoProtection videoProtection);
 
   int32_t AddVideoFrame(const I420VideoFrame& videoFrame,
                         const VideoContentMetrics* _contentMetrics,
@@ -110,11 +110,6 @@
   int32_t IntraFrameRequest(int stream_index);
   int32_t EnableFrameDropper(bool enable);
 
-  int SetSenderNackMode(SenderNackMode mode);
-  int SetSenderReferenceSelection(bool enable);
-  int SetSenderFEC(bool enable);
-  int SetSenderKeyFramePeriod(int periodMs);
-
   int StartDebugRecording(const char* file_name_utf8);
   void StopDebugRecording();
 
diff --git a/webrtc/modules/video_coding/main/source/video_receiver.cc b/webrtc/modules/video_coding/main/source/video_receiver.cc
index 4411e11..6336e0e 100644
--- a/webrtc/modules/video_coding/main/source/video_receiver.cc
+++ b/webrtc/modules/video_coding/main/source/video_receiver.cc
@@ -236,9 +236,12 @@
     }
     case kProtectionNackSender:
     case kProtectionFEC:
-    case kProtectionPeriodicKeyFrames:
       // Ignore encoder modes.
       return VCM_OK;
+    case kProtectionNone:
+      // TODO(pbos): Implement like sender and remove enable parameter. Ignored
+      // for now.
+      break;
   }
   return VCM_OK;
 }
diff --git a/webrtc/modules/video_coding/main/source/video_sender.cc b/webrtc/modules/video_coding/main/source/video_sender.cc
index 032dab7..21999ad 100644
--- a/webrtc/modules/video_coding/main/source/video_sender.cc
+++ b/webrtc/modules/video_coding/main/source/video_sender.cc
@@ -330,43 +330,29 @@
 }
 
 // Enable or disable a video protection method.
-// Note: This API should be deprecated, as it does not offer a distinction
-// between the protection method and decoding with or without errors. If such a
-// behavior is desired, use the following API: SetReceiverRobustnessMode.
-int32_t VideoSender::SetVideoProtection(VCMVideoProtection videoProtection,
-                                        bool enable) {
+void VideoSender::SetVideoProtection(bool enable,
+                                     VCMVideoProtection videoProtection) {
+  CriticalSectionScoped cs(_sendCritSect);
   switch (videoProtection) {
+    case kProtectionNone:
+      _mediaOpt.EnableProtectionMethod(enable, media_optimization::kNone);
+      break;
     case kProtectionNack:
-    case kProtectionNackSender: {
-      CriticalSectionScoped cs(_sendCritSect);
+    case kProtectionNackSender:
       _mediaOpt.EnableProtectionMethod(enable, media_optimization::kNack);
       break;
-    }
-
-    case kProtectionNackFEC: {
-      CriticalSectionScoped cs(_sendCritSect);
+    case kProtectionNackFEC:
       _mediaOpt.EnableProtectionMethod(enable, media_optimization::kNackFec);
       break;
-    }
-
-    case kProtectionFEC: {
-      CriticalSectionScoped cs(_sendCritSect);
+    case kProtectionFEC:
       _mediaOpt.EnableProtectionMethod(enable, media_optimization::kFec);
       break;
-    }
-
-    case kProtectionPeriodicKeyFrames: {
-      CriticalSectionScoped cs(_sendCritSect);
-      return _codecDataBase.SetPeriodicKeyFrames(enable) ? 0 : -1;
-      break;
-    }
     case kProtectionNackReceiver:
     case kProtectionKeyOnLoss:
     case kProtectionKeyOnKeyLoss:
-      // Ignore decoder modes.
-      return VCM_OK;
+      // Ignore receiver modes.
+      return;
   }
-  return VCM_OK;
 }
 // Add one raw video frame to the encoder, blocking.
 int32_t VideoSender::AddVideoFrame(const I420VideoFrame& videoFrame,
@@ -429,33 +415,6 @@
   return VCM_OK;
 }
 
-int VideoSender::SetSenderNackMode(SenderNackMode mode) {
-  switch (mode) {
-    case VideoCodingModule::kNackNone:
-      _mediaOpt.EnableProtectionMethod(false, media_optimization::kNack);
-      break;
-    case VideoCodingModule::kNackAll:
-      _mediaOpt.EnableProtectionMethod(true, media_optimization::kNack);
-      break;
-    case VideoCodingModule::kNackSelective:
-      return VCM_NOT_IMPLEMENTED;
-  }
-  return VCM_OK;
-}
-
-int VideoSender::SetSenderReferenceSelection(bool enable) {
-  return VCM_NOT_IMPLEMENTED;
-}
-
-int VideoSender::SetSenderFEC(bool enable) {
-  _mediaOpt.EnableProtectionMethod(enable, media_optimization::kFec);
-  return VCM_OK;
-}
-
-int VideoSender::SetSenderKeyFramePeriod(int periodMs) {
-  return VCM_NOT_IMPLEMENTED;
-}
-
 int VideoSender::StartDebugRecording(const char* file_name_utf8) {
   return recorder_->Start(file_name_utf8);
 }