BWE allocation strategy

webrtc/rtc_base/bitrateallocationstrategy.h

Index: webrtc/rtc_base/bitrateallocationstrategy.h
diff --git a/webrtc/rtc_base/bitrateallocationstrategy.h b/webrtc/rtc_base/bitrateallocationstrategy.h
new file mode 100644
index 0000000000000000000000000000000000000000..47c426cff0f15c75b4651456dcfc47de26da772a
--- /dev/null
+++ b/webrtc/rtc_base/bitrateallocationstrategy.h
@@ -0,0 +1,84 @@
+/*
+ *  Copyright 2017 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 WEBRTC_RTC_BASE_BITRATEALLOCATIONSTRATEGY_H_
+#define WEBRTC_RTC_BASE_BITRATEALLOCATIONSTRATEGY_H_
+
+#include <map>
+#include <string>
+#include <vector>
+#include "webrtc/rtc_base/checks.h"
+#include "webrtc/rtc_base/refcountedobject.h"
+
+namespace rtc {
+
+// Plugable strategy allows configuration of bitrate allocation per media track
+class BitrateAllocationStrategyConfig {
+ public:
+  struct TrackConfig {
+    TrackConfig(uint32_t min_bitrate_bps,
+                uint32_t max_bitrate_bps,
+                bool enforce_min_bitrate,
+                std::string track_id)
+        : min_bitrate_bps(min_bitrate_bps),
+          max_bitrate_bps(max_bitrate_bps),
+          enforce_min_bitrate(enforce_min_bitrate),
+          track_id(track_id) {}
+    TrackConfig(const TrackConfig& track_config)

[nisse-webrtc, 2017/08/22 11:46:05]

Use "= default" for the copy constructor, if possible.

[alexnarest, 2017/08/22 14:52:17]

Done.

+        : min_bitrate_bps(track_config.min_bitrate_bps),
+          max_bitrate_bps(track_config.max_bitrate_bps),
+          enforce_min_bitrate(track_config.enforce_min_bitrate),
+          track_id(track_config.track_id) {}
+    TrackConfig() {}
+
+    uint32_t min_bitrate_bps;
+    uint32_t max_bitrate_bps;
+    bool enforce_min_bitrate;
+    std::string track_id;

[nisse-webrtc, 2017/08/22 11:46:06]

Track ids are unclear to me. How are they assigned, and at which level do they
belong?

[alexnarest, 2017/08/22 14:52:17]

Track IDs are assigned by the app, for example please look at
https://cs.corp.google.com/piper///depot/google3/third_party/webrtc/files/sta....

+  };
+  // Track ID to track bitrate allocation
+  typedef std::map<std::string, uint32_t> TrackAllocations;
+  // Track ID to track config
+  typedef std::map<std::string, TrackConfig> TrackConfigs;

[nisse-webrtc, 2017/08/22 11:46:05]

Seems redundant to have track_ids inside TrackConfig, and also as keys in this
mapping. If the ids are just to associate passed in config with returned
allocation, it would be simpler to pass and return vectors, and drop the ids.

[alexnarest, 2017/08/22 14:52:17]

It can be done with the vectors but the map saves few lines at
https://codereview.webrtc.org/2996643002/diff/100001/webrtc/call/bitrate_allo...
line 276

[nisse-webrtc, 2017/08/22 15:23:34]

Not sure. See my comments on that place.

[alexnarest, 2017/08/31 18:40:26]

Done.

+
+  virtual ~BitrateAllocationStrategyConfig() = default;
+};
+
+// The base class future allocation strategies will inherit
+class BitrateAllocationStrategy
+    : public RefCountedObject<BitrateAllocationStrategyConfig> {

[nisse-webrtc, 2017/08/22 11:46:06]

This inheritance is puzzling to me, a strategy and a strategy conf sounds like
very different things.

I would have expected a BitrateAllocationStrategyInterface, one or two
implementations of that interface, and perhaps some convenience base class in
between, for code sharing.

[alexnarest, 2017/08/22 14:52:17]

Will renaming BitrateAllocationStrategyConfig to
BitrateAllocationStrategyInterface and BitrateAllocationStrategy to
BitrateAllocationStrategyBase make things clear?

[nisse-webrtc, 2017/08/22 15:23:34]

Sounds good. But don't do the Base class unless there really is a lot code it
makes sense to share between implementations of the interface.

[alexnarest, 2017/08/31 18:40:27]

Done.

+ public:
+  // Will be called by bitrate allocator, should return allocated bitrate
+  // for every track.
+  virtual TrackAllocations AllocateBitrates(
+      uint32_t available_bitrate,
+      const TrackConfigs& track_configs) = 0;
+
+  TrackAllocations SetAllBitratesToMinimum(const TrackConfigs& track_configs);
+  TrackAllocations DistributeBitratesEvenly(const TrackConfigs& track_configs,
+                                            uint32_t available_bitrate);
+};
+
+// Simple allocation strategy giving priority to audio at low bitrates
+class AudioPriorityBitrateAllocationStrategy
+    : public BitrateAllocationStrategy {
+ public:
+  AudioPriorityBitrateAllocationStrategy(std::string audio_track_id,
+                                         uint32_t sufficient_audio_bitrate);
+  TrackAllocations AllocateBitrates(uint32_t available_bitrate,
+                                    const TrackConfigs& track_configs) override;
+
+ private:
+  std::string audio_track_id_;
+  uint32_t sufficient_audio_bitrate_;
+};
+}  // namespace rtc
+
+#endif  // WEBRTC_RTC_BASE_BITRATEALLOCATIONSTRATEGY_H_