Work on flexible mode and screen sharing.

webrtc/modules/video_coding/codecs/vp9/screenshare_layers.cc

+/* Copyright (c) 2013 The WebRTC project authors. All Rights Reserved.

[sprang_webrtc, 2015/09/07 14:39:04]

2015? :)

+*
+*  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.
+*/
+
+#include <algorithm>
+#include "webrtc/modules/video_coding/codecs/vp9/screenshare_layers.h"
+#include "webrtc/base/checks.h"
+
+namespace webrtc {
+
+ScreenshareLayersVP9::ScreenshareLayersVP9()
+    : threshold_kbps_(0.f), sl0_bits_used_(0.f), current_sl_(0), last_ts_(0) {
+}
+
+bool ScreenshareLayersVP9::ConfigureBitrate(int threshold_kbps) {
+  threshold_kbps_ = threshold_kbps;
+  return true;

[sprang_webrtc, 2015/09/07 14:39:04]

Why do we return a bool here?

[philipel, 2015/09/10 14:47:01]

Because I copied the interface from the temporal layers in VP8, now changed to
void.

+}
+
+void ScreenshareLayersVP9::FrameEncoded(unsigned int size_bytes) {
+  sl0_bits_used_ += current_sl_ == 0 ? size_bytes * 8 : 0;

[sprang_webrtc, 2015/09/07 14:39:04]

Would prefer
if (current_sl_ == 0)
  sl0_bits_used_ += size_bytes * 8;

[philipel, 2015/09/10 14:47:01]

Done.

+}
+
+int8_t ScreenshareLayersVP9::CurrentLayer() {

[sprang_webrtc, 2015/09/07 14:39:04]

const?

[philipel, 2015/09/10 14:47:01]

Done.

+  return current_sl_;
+}
+
+// TODO(philipel): suppose to be used for different spatial layers,

[sprang_webrtc, 2015/09/07 14:39:04]

Can we refer to a libvpx issue or some other reference point?

[philipel, 2015/09/10 14:47:01]

Comment removed.

+// but for now is used for different temporal layers.
+std::array<int8_t, 4> ScreenshareLayersVP9::BufferArguments(
+    uint32_t timestamp) {
+  std::array<int8_t, 4> res;

[sprang_webrtc, 2015/09/07 14:39:04]

Please comment how this res (result?) is to be interpreted and/or rename it.

[philipel, 2015/09/10 14:47:01]

Changed to new data structure

+  if (last_ts_ == 0) {
+    res[0] = 0;
+    res[1] = 0;
+  } else {
+    float diff = timestamp - last_ts_;
+    // A check to make sure the used bits calculation doesn't overflow,
+    // which can happen if you debug and use a real time clock
+    // (diff becomes to big).

[sprang_webrtc, 2015/09/07 14:39:04]

or too big?

[philipel, 2015/09/10 14:47:01]

Removed check and comment.

+    DCHECK(sl0_bits_used_ >= (sl0_bits_used_ - diff / 90.f * threshold_kbps_));
+    sl0_bits_used_ =
+        std::max(0.f, sl0_bits_used_ - diff / 90.f * threshold_kbps_);
+

[sprang_webrtc, 2015/09/07 14:39:04]

This means you are looking at the average bitrate over the entire lifetime of
this object?
The bitrate may be changed at runtime due to bandwidth estimates...

Could you instead reuse this part form the vp8 screenshare layers class? Minus
the qp boost and mode, which probably don't make much sense here...

[philipel, 2015/09/10 14:47:01]

No, this is not the bitrate over the entire lifetime of the object, if the
threshold_kbps_ is changed than so is the rate at which sl0_bits_used_ is
decreased.

+    if (sl0_bits_used_ > threshold_kbps_ * 1000) {
+      res[0] = 1;
+      res[1] = current_sl_;
+      current_sl_ = 1;
+    } else {
+      res[0] = 0;
+      res[1] = 0;
+      current_sl_ = 0;
+    }
+  }
+  last_ts_ = timestamp;
+  res[2] = res[3] = -1;
+  return res;
+}
+
+}  // namespace webrtc