Revert "Revert "Revert "Add ProtectionBitrateCalculator as an abstract class. ProtectionBitrateCalculatorDefault implements ProtectionBitrateCalculator. Register VideoSendStream to packet feedback"""
This reverts commit 718d8631b0294a8bdc56366b68c51e2f04cd0c9e.
Reason for revert: <INSERT REASONING HERE>
Original change's description:
> Revert "Revert "Add ProtectionBitrateCalculator as an abstract class. ProtectionBitrateCalculatorDefault implements ProtectionBitrateCalculator. Register VideoSendStream to packet feedback""
>
> This reverts commit 53d901332c2eb43cad0da5768c6f7a8c4aeb9590.
>
> Reason for revert: root cause has been found and will be addressed in the patch.The root cause was protection_bitrate_calculator_ is now destructed before worker_queue_, and worker_queue_ may contain tasks which involves protection_bitrate_calculator_, so they need to be destructed in the opposite order.
> That was not an issue since before this cl we didn't allocate protection_bitrate_calculator_ on the heap.
>
> Original change's description:
> > Revert "Add ProtectionBitrateCalculator as an abstract class. ProtectionBitrateCalculatorDefault implements ProtectionBitrateCalculator. Register VideoSendStream to packet feedback"
> >
> > This reverts commit e58e91b6d143ef847f8df24b19de4ba98cdb6f72.
> >
> > Reason for revert: Breaks downstream project b/70848177
> >
> > Original change's description:
> > > Add ProtectionBitrateCalculator as an abstract class. ProtectionBitrateCalculatorDefault implements ProtectionBitrateCalculator. Register VideoSendStream to packet feedback
> > >
> > > Bug: webrtc:8656
> > > Change-Id: Iab4f6ab8997cb082762218afc8580e9985ac2522
> > > Reviewed-on: https://webrtc-review.googlesource.com/33010
> > > Commit-Queue: Ying Wang <yinwa@webrtc.org>
> > > Reviewed-by: Stefan Holmer <stefan@webrtc.org>
> > > Cr-Commit-Position: refs/heads/master@{#21348}
> >
> > TBR=stefan@webrtc.org,philipel@webrtc.org,yinwa@webrtc.org
> >
> > Change-Id: Ic186ba78be429bd1046ceac15051a3382b6ffc4f
> > No-Presubmit: true
> > No-Tree-Checks: true
> > No-Try: true
> > Bug: webrtc:8656
> > Reviewed-on: https://webrtc-review.googlesource.com/35080
> > Commit-Queue: Lu Liu <lliuu@webrtc.org>
> > Reviewed-by: Lu Liu <lliuu@webrtc.org>
> > Cr-Commit-Position: refs/heads/master@{#21374}
>
> TBR=stefan@webrtc.org,philipel@webrtc.org,lliuu@webrtc.org,yujo@chromium.org,yinwa@webrtc.org
>
> Change-Id: Ie2b5a2a2ead0f20ac67c1ea9b8d192af66bddf8d
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug: webrtc:8656
> Reviewed-on: https://webrtc-review.googlesource.com/35320
> Reviewed-by: Stefan Holmer <stefan@webrtc.org>
> Reviewed-by: Ying Wang <yinwa@webrtc.org>
> Commit-Queue: Ying Wang <yinwa@webrtc.org>
> Cr-Commit-Position: refs/heads/master@{#21409}
TBR=stefan@webrtc.org,philipel@webrtc.org,lliuu@webrtc.org,yujo@chromium.org,yinwa@webrtc.org
Change-Id: I9773aaa942054dcfbab6002a5d713ab3526b0534
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: webrtc:8656
Reviewed-on: https://webrtc-review.googlesource.com/35700
Reviewed-by: Ying Wang <yinwa@webrtc.org>
Commit-Queue: Ying Wang <yinwa@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#21410}diff --git a/modules/video_coding/BUILD.gn b/modules/video_coding/BUILD.gn
index 71e9333..59e8eb9 100644
--- a/modules/video_coding/BUILD.gn
+++ b/modules/video_coding/BUILD.gn
@@ -57,9 +57,8 @@
"packet.h",
"packet_buffer.cc",
"packet_buffer.h",
+ "protection_bitrate_calculator.cc",
"protection_bitrate_calculator.h",
- "protection_bitrate_calculator_default.cc",
- "protection_bitrate_calculator_default.h",
"qp_parser.cc",
"qp_parser.h",
"receiver.cc",
diff --git a/modules/video_coding/protection_bitrate_calculator_default.cc b/modules/video_coding/protection_bitrate_calculator.cc
similarity index 89%
rename from modules/video_coding/protection_bitrate_calculator_default.cc
rename to modules/video_coding/protection_bitrate_calculator.cc
index f0db4c2..3c1da23 100644
--- a/modules/video_coding/protection_bitrate_calculator_default.cc
+++ b/modules/video_coding/protection_bitrate_calculator.cc
@@ -8,13 +8,13 @@
* be found in the AUTHORS file in the root of the source tree.
*/
-#include "modules/video_coding/protection_bitrate_calculator_default.h"
+#include "modules/video_coding/protection_bitrate_calculator.h"
namespace webrtc {
using rtc::CritScope;
-ProtectionBitrateCalculatorDefault::ProtectionBitrateCalculatorDefault(
+ProtectionBitrateCalculator::ProtectionBitrateCalculator(
Clock* clock,
VCMProtectionCallback* protection_callback)
: clock_(clock),
@@ -23,22 +23,21 @@
clock_->TimeInMilliseconds())),
max_payload_size_(1460) {}
-ProtectionBitrateCalculatorDefault::~ProtectionBitrateCalculatorDefault(void) {
+ProtectionBitrateCalculator::~ProtectionBitrateCalculator(void) {
loss_prot_logic_->Release();
}
-void ProtectionBitrateCalculatorDefault::SetEncodingData(
- size_t width,
- size_t height,
- size_t num_temporal_layers,
- size_t max_payload_size) {
+void ProtectionBitrateCalculator::SetEncodingData(size_t width,
+ size_t height,
+ size_t num_temporal_layers,
+ size_t max_payload_size) {
CritScope lock(&crit_sect_);
loss_prot_logic_->UpdateFrameSize(width, height);
loss_prot_logic_->UpdateNumLayers(num_temporal_layers);
max_payload_size_ = max_payload_size;
}
-uint32_t ProtectionBitrateCalculatorDefault::SetTargetRates(
+uint32_t ProtectionBitrateCalculator::SetTargetRates(
uint32_t estimated_bitrate_bps,
int actual_framerate_fps,
uint8_t fraction_lost,
@@ -139,8 +138,8 @@
return estimated_bitrate_bps * (1.0 - protection_overhead_rate);
}
-void ProtectionBitrateCalculatorDefault::SetProtectionMethod(bool enable_fec,
- bool enable_nack) {
+void ProtectionBitrateCalculator::SetProtectionMethod(bool enable_fec,
+ bool enable_nack) {
media_optimization::VCMProtectionMethodEnum method(media_optimization::kNone);
if (enable_fec && enable_nack) {
method = media_optimization::kNackFec;
@@ -153,7 +152,7 @@
loss_prot_logic_->SetMethod(method);
}
-void ProtectionBitrateCalculatorDefault::UpdateWithEncodedData(
+void ProtectionBitrateCalculator::UpdateWithEncodedData(
const EncodedImage& encoded_image) {
const size_t encoded_length = encoded_image._length;
CritScope lock(&crit_sect_);
diff --git a/modules/video_coding/protection_bitrate_calculator.h b/modules/video_coding/protection_bitrate_calculator.h
index a309ec4..bf50589 100644
--- a/modules/video_coding/protection_bitrate_calculator.h
+++ b/modules/video_coding/protection_bitrate_calculator.h
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2017 The WebRTC project authors. All Rights Reserved.
+ * Copyright (c) 2016 The WebRTC project authors. All Rights Reserved.
*
* Use of this source code is governed by a BSD-style license
* that can be found in the LICENSE file in the root of the source
@@ -13,7 +13,6 @@
#include <list>
#include <memory>
-#include <vector>
#include "modules/include/module_common_types.h"
#include "modules/video_coding/include/video_coding.h"
@@ -23,38 +22,57 @@
namespace webrtc {
+// ProtectionBitrateCalculator calculates how much of the allocated network
+// capacity that can be used by an encoder and how much that
+// is needed for redundant packets such as FEC and NACK. It uses an
+// implementation of |VCMProtectionCallback| to set new FEC parameters and get
+// the bitrate currently used for FEC and NACK.
+// Usage:
+// Setup by calling SetProtectionMethod and SetEncodingData.
+// For each encoded image, call UpdateWithEncodedData.
+// Each time the bandwidth estimate change, call SetTargetRates. SetTargetRates
+// will return the bitrate that can be used by an encoder.
+// A lock is used to protect internal states, so methods can be called on an
+// arbitrary thread.
class ProtectionBitrateCalculator {
public:
- virtual ~ProtectionBitrateCalculator() {}
+ ProtectionBitrateCalculator(Clock* clock,
+ VCMProtectionCallback* protection_callback);
+ ~ProtectionBitrateCalculator();
- virtual void SetProtectionMethod(bool enable_fec, bool enable_nack) = 0;
+ void SetProtectionMethod(bool enable_fec, bool enable_nack);
// Informs media optimization of initial encoding state.
- virtual void SetEncodingData(size_t width,
- size_t height,
- size_t num_temporal_layers,
- size_t max_payload_size) = 0;
+ void SetEncodingData(size_t width,
+ size_t height,
+ size_t num_temporal_layers,
+ size_t max_payload_size);
// Returns target rate for the encoder given the channel parameters.
// Inputs: estimated_bitrate_bps - the estimated network bitrate in bits/s.
// actual_framerate - encoder frame rate.
// fraction_lost - packet loss rate in % in the network.
// round_trip_time_ms - round trip time in milliseconds.
- virtual uint32_t SetTargetRates(uint32_t estimated_bitrate_bps,
- int actual_framerate,
- uint8_t fraction_lost,
- int64_t round_trip_time_ms) = 0;
-
- virtual uint32_t SetTargetRates(uint32_t estimated_bitrate_bps,
- std::vector<uint8_t> loss_mask_vector,
- int64_t round_trip_time_ms) = 0;
-
+ uint32_t SetTargetRates(uint32_t estimated_bitrate_bps,
+ int actual_framerate,
+ uint8_t fraction_lost,
+ int64_t round_trip_time_ms);
// Informs of encoded output.
- virtual void UpdateWithEncodedData(const EncodedImage& encoded_image) = 0;
+ void UpdateWithEncodedData(const EncodedImage& encoded_image);
- virtual void OnLossMaskVector(const std::vector<bool> loss_mask_vector) = 0;
+ private:
+ enum { kBitrateAverageWinMs = 1000 };
+
+ Clock* const clock_;
+ VCMProtectionCallback* const protection_callback_;
+
+ rtc::CriticalSection crit_sect_;
+ std::unique_ptr<media_optimization::VCMLossProtectionLogic> loss_prot_logic_
+ RTC_GUARDED_BY(crit_sect_);
+ size_t max_payload_size_ RTC_GUARDED_BY(crit_sect_);
+
+ RTC_DISALLOW_COPY_AND_ASSIGN(ProtectionBitrateCalculator);
};
} // namespace webrtc
-
#endif // MODULES_VIDEO_CODING_PROTECTION_BITRATE_CALCULATOR_H_
diff --git a/modules/video_coding/protection_bitrate_calculator_default.h b/modules/video_coding/protection_bitrate_calculator_default.h
deleted file mode 100644
index 19fa7e1..0000000
--- a/modules/video_coding/protection_bitrate_calculator_default.h
+++ /dev/null
@@ -1,87 +0,0 @@
-/*
- * Copyright (c) 2016 The WebRTC project authors. All Rights Reserved.
- *
- * Use of this source code is governed by a BSD-style license
- * that can be found in the LICENSE file in the root of the source
- * tree. An additional intellectual property rights grant can be found
- * in the file PATENTS. All contributing project authors may
- * be found in the AUTHORS file in the root of the source tree.
- */
-
-#ifndef MODULES_VIDEO_CODING_PROTECTION_BITRATE_CALCULATOR_DEFAULT_H_
-#define MODULES_VIDEO_CODING_PROTECTION_BITRATE_CALCULATOR_DEFAULT_H_
-
-#include <list>
-#include <memory>
-#include <vector>
-
-#include "modules/include/module_common_types.h"
-#include "modules/video_coding/include/video_coding.h"
-#include "modules/video_coding/media_opt_util.h"
-#include "modules/video_coding/protection_bitrate_calculator.h"
-#include "rtc_base/criticalsection.h"
-#include "system_wrappers/include/clock.h"
-
-namespace webrtc {
-
-// ProtectionBitrateCalculator calculates how much of the allocated network
-// capacity that can be used by an encoder and how much that
-// is needed for redundant packets such as FEC and NACK. It uses an
-// implementation of |VCMProtectionCallback| to set new FEC parameters and get
-// the bitrate currently used for FEC and NACK.
-// Usage:
-// Setup by calling SetProtectionMethod and SetEncodingData.
-// For each encoded image, call UpdateWithEncodedData.
-// Each time the bandwidth estimate change, call SetTargetRates. SetTargetRates
-// will return the bitrate that can be used by an encoder.
-// A lock is used to protect internal states, so methods can be called on an
-// arbitrary thread.
-class ProtectionBitrateCalculatorDefault : public ProtectionBitrateCalculator {
- public:
- ProtectionBitrateCalculatorDefault(
- Clock* clock,
- VCMProtectionCallback* protection_callback);
- ~ProtectionBitrateCalculatorDefault();
-
- void SetProtectionMethod(bool enable_fec, bool enable_nack) override;
-
- // Informs media optimization of initial encoding state.
- void SetEncodingData(size_t width,
- size_t height,
- size_t num_temporal_layers,
- size_t max_payload_size) override;
-
- uint32_t SetTargetRates(uint32_t estimated_bitrate_bps,
- std::vector<uint8_t> loss_mask_vector,
- int64_t round_trip_time_ms) override {
- return 0;
- }
-
- void OnLossMaskVector(const std::vector<bool> loss_mask_vector) override {}
-
- // Returns target rate for the encoder given the channel parameters.
- // Inputs: estimated_bitrate_bps - the estimated network bitrate in bits/s.
- // actual_framerate - encoder frame rate.
- // fraction_lost - packet loss rate in % in the network.
- // round_trip_time_ms - round trip time in milliseconds.
- uint32_t SetTargetRates(uint32_t estimated_bitrate_bps,
- int actual_framerate,
- uint8_t fraction_lost,
- int64_t round_trip_time_ms) override;
- // Informs of encoded output.
- void UpdateWithEncodedData(const EncodedImage& encoded_image) override;
-
- private:
- enum { kBitrateAverageWinMs = 1000 };
- Clock* const clock_;
- VCMProtectionCallback* const protection_callback_;
- rtc::CriticalSection crit_sect_;
- std::unique_ptr<media_optimization::VCMLossProtectionLogic> loss_prot_logic_
- RTC_GUARDED_BY(crit_sect_);
- size_t max_payload_size_ RTC_GUARDED_BY(crit_sect_);
-
- RTC_DISALLOW_COPY_AND_ASSIGN(ProtectionBitrateCalculatorDefault);
-};
-
-} // namespace webrtc
-#endif // MODULES_VIDEO_CODING_PROTECTION_BITRATE_CALCULATOR_DEFAULT_H_
diff --git a/modules/video_coding/protection_bitrate_calculator_unittest.cc b/modules/video_coding/protection_bitrate_calculator_unittest.cc
index dd58bc2..66e2fa6 100644
--- a/modules/video_coding/protection_bitrate_calculator_unittest.cc
+++ b/modules/video_coding/protection_bitrate_calculator_unittest.cc
@@ -8,7 +8,7 @@
* be found in the AUTHORS file in the root of the source tree.
*/
-#include "modules/video_coding/protection_bitrate_calculator_default.h"
+#include "modules/video_coding/protection_bitrate_calculator.h"
#include "system_wrappers/include/clock.h"
#include "test/gtest.h"
@@ -46,7 +46,7 @@
SimulatedClock clock_;
ProtectionCallback protection_callback_;
- ProtectionBitrateCalculatorDefault media_opt_;
+ ProtectionBitrateCalculator media_opt_;
};
TEST_F(ProtectionBitrateCalculatorTest, ProtectsUsingFecBitrate) {