Make sure temporal layered screenshare frames are sent in at least 2s.

Make sure temporal layered screenshare frames are sent in at least 2s.

If a very large frame is sent (high res slide change) when the available
send bitrate is very low, the it might take many seconds before any new
frames are emitted as the accrued debt will take time to pay off.

Add a bailout, so that if a frame hasn't been sent for 2 seconds, cancel
the debt immediately, even if the target bitrate is then exceeded.

BUG=webrtc:5750
Committed: https://crrev.com/afe1f74c04d908f8bd8ad5b15c6adf6ce6250969
Cr-Commit-Position: refs/heads/master@{#12328}

[sprang_webrtc, 2016-04-07 12:47:56]

sprang@webrtc.org changed reviewers:
+ stefan@webrtc.org

[stefan-webrtc, 2016-04-07 12:53:12]

https://codereview.webrtc.org/1869003002/diff/20001/webrtc/modules/video_codi...
File webrtc/modules/video_coding/codecs/vp8/screenshare_layers.cc (right):

https://codereview.webrtc.org/1869003002/diff/20001/webrtc/modules/video_codi...
webrtc/modules/video_coding/codecs/vp8/screenshare_layers.cc:97:
layers_[1].CheckFrameInterval(unwrapped_timestamp);
Do we want the same behavior in TL1, or should we only enforce 0.5 fps in TL0? I
think TL0 would be enough?

https://codereview.webrtc.org/1869003002/diff/20001/webrtc/modules/video_codi...
webrtc/modules/video_coding/codecs/vp8/screenshare_layers.cc:112:
layers_[0].last_emitted_timsetamp_ = unwrapped_timestamp;
last_emitted_timestamp

[sprang_webrtc, 2016-04-07 13:03:32]

https://codereview.webrtc.org/1869003002/diff/20001/webrtc/modules/video_codi...
File webrtc/modules/video_coding/codecs/vp8/screenshare_layers.cc (right):

https://codereview.webrtc.org/1869003002/diff/20001/webrtc/modules/video_codi...
webrtc/modules/video_coding/codecs/vp8/screenshare_layers.cc:97:
layers_[1].CheckFrameInterval(unwrapped_timestamp);
On 2016/04/07 12:53:12, stefan-webrtc (holmer) wrote:
> Do we want the same behavior in TL1, or should we only enforce 0.5 fps in TL0?
I
> think TL0 would be enough?

Yeah, I think only doing it for TL0 makes sense.

https://codereview.webrtc.org/1869003002/diff/20001/webrtc/modules/video_codi...
webrtc/modules/video_coding/codecs/vp8/screenshare_layers.cc:112:
layers_[0].last_emitted_timsetamp_ = unwrapped_timestamp;
On 2016/04/07 12:53:12, stefan-webrtc (holmer) wrote:
> last_emitted_timestamp

Done.

[stefan-webrtc, 2016-04-11 10:29:22]

https://codereview.webrtc.org/1869003002/diff/60001/webrtc/modules/video_codi...
File webrtc/modules/video_coding/codecs/vp8/screenshare_layers.cc (right):

https://codereview.webrtc.org/1869003002/diff/60001/webrtc/modules/video_codi...
webrtc/modules/video_coding/codecs/vp8/screenshare_layers.cc:99:
layers_[0].debt_bytes_ = 0;
Is it correct to cancel the whole debt? Instead we could set it to
max_debt_bytes_ - 1 so that we don't end up sending a burst of TL0 frames after
this has triggered?

[sprang_webrtc, 2016-04-11 11:02:53]

https://codereview.webrtc.org/1869003002/diff/60001/webrtc/modules/video_codi...
File webrtc/modules/video_coding/codecs/vp8/screenshare_layers.cc (right):

https://codereview.webrtc.org/1869003002/diff/60001/webrtc/modules/video_codi...
webrtc/modules/video_coding/codecs/vp8/screenshare_layers.cc:99:
layers_[0].debt_bytes_ = 0;
On 2016/04/11 10:29:22, stefan-webrtc (holmer) wrote:
> Is it correct to cancel the whole debt? Instead we could set it to
> max_debt_bytes_ - 1 so that we don't end up sending a burst of TL0 frames
after
> this has triggered?

You mean if the frames would end up being reeealy tiny? I find that unlikely :)
But sure, we can add that as a precaution.

[stefan-webrtc, 2016-04-12 07:24:55]

lgtm

[sprang_webrtc, 2016-04-12 07:29:55]

The CQ bit was checked by sprang@webrtc.org

[commit-bot: I haz the power, 2016-04-12 07:30:02]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1869003002/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1869003002/80001

[commit-bot: I haz the power, 2016-04-12 07:42:56]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-04-12 07:42:57]

Try jobs failed on following builders:
  linux_ubsan on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/linux_ubsan/builds/1148)

[sprang_webrtc, 2016-04-12 08:21:34]

The CQ bit was checked by sprang@webrtc.org

[sprang_webrtc, 2016-04-12 08:21:35]

The patchset sent to the CQ was uploaded after l-g-t-m from stefan@webrtc.org
Link to the patchset: https://codereview.webrtc.org/1869003002/#ps120001 (title:
"Cleanup/rebase")

[commit-bot: I haz the power, 2016-04-12 08:21:39]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1869003002/120001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1869003002/120001

[commit-bot: I haz the power, 2016-04-12 09:45:17]

Committed patchset #7 (id:120001)

[commit-bot: I haz the power, 2016-04-12 09:45:27]

Description was changed from

==========
Make sure temporal layered screenshare frames are sent in at least 2s.

If a very large frame is sent (high res slide change) when the available
send bitrate is very low, the it might take many seconds before any new
frames are emitted as the accrued debt will take time to pay off.

Add a bailout, so that if a frame hasn't been sent for 2 seconds, cancel
the debt immediately, even if the target bitrate is then exceeded.

BUG=webrtc:5750
==========

to

==========
Make sure temporal layered screenshare frames are sent in at least 2s.

If a very large frame is sent (high res slide change) when the available
send bitrate is very low, the it might take many seconds before any new
frames are emitted as the accrued debt will take time to pay off.

Add a bailout, so that if a frame hasn't been sent for 2 seconds, cancel
the debt immediately, even if the target bitrate is then exceeded.

BUG=webrtc:5750

Committed: https://crrev.com/afe1f74c04d908f8bd8ad5b15c6adf6ce6250969
Cr-Commit-Position: refs/heads/master@{#12328}
==========

[commit-bot: I haz the power, 2016-04-12 09:45:28]

Patchset 7 (id:??) landed as
https://crrev.com/afe1f74c04d908f8bd8ad5b15c6adf6ce6250969
Cr-Commit-Position: refs/heads/master@{#12328}

[sprang_webrtc, 2016-05-02 18:59:05]

Patchset #8 (id:140001) has been deleted