Add a new frame generator that cycles through randomly generated slides.

Add a new frame generator that cycles through randomly generated slides.

Like YuvFileGenerator, this also updates the display with a new slide on every Nth frame, but it generates the slides itself instead of reading them from files.



BUG=webrtc:8138
Review-Url: https://codereview.webrtc.org/3003193002
Cr-Commit-Position: refs/heads/master@{#19585}
Committed: https://chromium.googlesource.com/external/webrtc/+/579de6faef60ebe6e60a795041fecb92569d4f72

[erikvarga1, 2017-08-23 10:59:54]

Description was changed from

==========
Add a new frame generator that cycles through randomly generated slides.

Like YuvFileGenerator, this also updates the display with a new slide on every
Nth frame, but it generates the slides itself instead of reading them from
files.



BUG=webrtc:8138
==========

to

==========
Add a new frame generator that cycles through randomly generated slides.

Like YuvFileGenerator, this also updates the display with a new slide on every
Nth frame, but it generates the slides itself instead of reading them from
files.



BUG=webrtc:8138
==========

[erikvarga1, 2017-08-23 10:59:54]

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

[sprang_webrtc, 2017-08-23 12:59:25]

Some initial comments

https://codereview.webrtc.org/3003193002/diff/1/webrtc/test/frame_generator.cc
File webrtc/test/frame_generator.cc (right):

https://codereview.webrtc.org/3003193002/diff/1/webrtc/test/frame_generator.c...
webrtc/test/frame_generator.cc:214: const int kSquareNum = 20;
I think this will not properly catch the extreme variation in slide complexity
that can happen. I did some short prototyping, visually inspecting the results
and found this to be a reasonable replacement:

const int square_num = 1 << (4 + (random_generator_.Rand(0, 3) * 4));

together with 

int length = random_generator_.Rand(1, width_ > 4 ? width_ / 4 : 1);
length = (length * (square_num - i)) / square_num;

(so big squares towards the end don't overwrite too much detail)

https://codereview.webrtc.org/3003193002/diff/1/webrtc/video/full_stack_tests.cc
File webrtc/video/full_stack_tests.cc (right):

https://codereview.webrtc.org/3003193002/diff/1/webrtc/video/full_stack_tests...
webrtc/video/full_stack_tests.cc:371: 
What's the purpose of these tests? We're a little hesitant to add too many tests
here as they tend to be rather timing sensitive and may cause unnecessary burden
for perf sheriffs.

https://codereview.webrtc.org/3003193002/diff/1/webrtc/video/screenshare_loop...
File webrtc/video/screenshare_loopback.cc (right):

https://codereview.webrtc.org/3003193002/diff/1/webrtc/video/screenshare_loop...
webrtc/video/screenshare_loopback.cc:281: params.screenshare = {true, false,
flags::SlideChangeInterval(),
Maybe add command line flag for this?

https://codereview.webrtc.org/3003193002/diff/1/webrtc/video/video_quality_te...
File webrtc/video/video_quality_test.cc (right):

https://codereview.webrtc.org/3003193002/diff/1/webrtc/video/video_quality_te...
webrtc/video/video_quality_test.cc:1176: screenshare({false, 10, 0}),
Add the bool for |generate_slides| here as well

[erikvarga1, 2017-08-23 14:41:25]

https://codereview.webrtc.org/3003193002/diff/1/webrtc/test/frame_generator.cc
File webrtc/test/frame_generator.cc (right):

https://codereview.webrtc.org/3003193002/diff/1/webrtc/test/frame_generator.c...
webrtc/test/frame_generator.cc:214: const int kSquareNum = 20;
On 2017/08/23 12:59:24, sprang_webrtc wrote:
> I think this will not properly catch the extreme variation in slide complexity
> that can happen. I did some short prototyping, visually inspecting the results
> and found this to be a reasonable replacement:
> 
> const int square_num = 1 << (4 + (random_generator_.Rand(0, 3) * 4));
> 
> together with 
> 
> int length = random_generator_.Rand(1, width_ > 4 ? width_ / 4 : 1);
> length = (length * (square_num - i)) / square_num;
> 
> (so big squares towards the end don't overwrite too much detail)

Done. I didn't think to take the complexity change into account when writing
this.
I ran some benchmarks to see the frame size distribution over time and it seems
that this version is closer to the YuvFileGenerator than mine was:
https://docs.google.com/spreadsheets/d/1S2pjul33Eqeda5npesHWupqgTgVRwtTteVR8h...

(I got the numbers by running ScreenshareSlidesVP8_3TL_Simulcast with the three
frame generators and logging the delta frame sizes and send times in
rtp_sender.)

https://codereview.webrtc.org/3003193002/diff/1/webrtc/video/full_stack_tests.cc
File webrtc/video/full_stack_tests.cc (right):

https://codereview.webrtc.org/3003193002/diff/1/webrtc/video/full_stack_tests...
webrtc/video/full_stack_tests.cc:371: 
On 2017/08/23 12:59:24, sprang_webrtc wrote:
> What's the purpose of these tests? We're a little hesitant to add too many
tests
> here as they tend to be rather timing sensitive and may cause unnecessary
burden
> for perf sheriffs.

I added these to have some integration tests where the new generator is used,
but I guess the perf tests aren't really the best place for this.
I've removed them and instead added an option to choose between generators in
the screenshare_loopback.

https://codereview.webrtc.org/3003193002/diff/1/webrtc/video/screenshare_loop...
File webrtc/video/screenshare_loopback.cc (right):

https://codereview.webrtc.org/3003193002/diff/1/webrtc/video/screenshare_loop...
webrtc/video/screenshare_loopback.cc:281: params.screenshare = {true, false,
flags::SlideChangeInterval(),
On 2017/08/23 12:59:24, sprang_webrtc wrote:
> Maybe add command line flag for this?

Done.

https://codereview.webrtc.org/3003193002/diff/1/webrtc/video/video_quality_te...
File webrtc/video/video_quality_test.cc (right):

https://codereview.webrtc.org/3003193002/diff/1/webrtc/video/video_quality_te...
webrtc/video/video_quality_test.cc:1176: screenshare({false, 10, 0}),
On 2017/08/23 12:59:24, sprang_webrtc wrote:
> Add the bool for |generate_slides| here as well 

Done.

[sprang_webrtc, 2017-08-25 08:28:52]

lgtm

[stefan-webrtc, 2017-08-29 11:11:37]

lgtm

[erikvarga1, 2017-08-29 11:21:29]

The CQ bit was checked by erikvarga@webrtc.org

[commit-bot: I haz the power, 2017-08-29 11:21:41]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[commit-bot: I haz the power, 2017-08-29 11:38:32]

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

[commit-bot: I haz the power, 2017-08-29 11:38:33]

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

[erikvarga1, 2017-08-29 12:31:07]

The CQ bit was checked by erikvarga@webrtc.org to run a CQ dry run

[commit-bot: I haz the power, 2017-08-29 12:31:11]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[commit-bot: I haz the power, 2017-08-29 12:50:25]

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

[commit-bot: I haz the power, 2017-08-29 12:50:25]

Dry run: Try jobs failed on following builders:
  win_x64_dbg on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/win_x64_dbg/builds/11983)
  win_x64_rel on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/win_x64_rel/builds/28530)

[erikvarga1, 2017-08-29 12:53:48]

The CQ bit was checked by erikvarga@webrtc.org to run a CQ dry run

[commit-bot: I haz the power, 2017-08-29 12:53:51]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[commit-bot: I haz the power, 2017-08-29 13:47:41]

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

[commit-bot: I haz the power, 2017-08-29 13:47:42]

Dry run: Try jobs failed on following builders:
  win_x64_dbg on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/win_x64_dbg/builds/11991)

[erikvarga1, 2017-08-29 13:52:52]

The CQ bit was checked by erikvarga@webrtc.org to run a CQ dry run

[commit-bot: I haz the power, 2017-08-29 13:53:02]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[commit-bot: I haz the power, 2017-08-29 15:19:51]

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

[commit-bot: I haz the power, 2017-08-29 15:19:52]

Dry run: This issue passed the CQ dry run.

[erikvarga1, 2017-08-29 15:26:41]

https://codereview.webrtc.org/3003193002/diff/100001/webrtc/test/frame_genera...
File webrtc/test/frame_generator_capturer.cc (right):

https://codereview.webrtc.org/3003193002/diff/100001/webrtc/test/frame_genera...
webrtc/test/frame_generator_capturer.cc:119: int height,
Changed the width and height from size_t to int (like in the square generator)
to avoid type conversion warnings.

[erikvarga1, 2017-08-29 16:10:22]

The CQ bit was checked by erikvarga@webrtc.org

[erikvarga1, 2017-08-29 16:10:23]

The patchset sent to the CQ was uploaded after l-g-t-m from sprang@webrtc.org,
stefan@webrtc.org
Link to the patchset: https://codereview.webrtc.org/3003193002/#ps100001 (title:
"Patchset 3C.")

[commit-bot: I haz the power, 2017-08-29 16:10:33]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[commit-bot: I haz the power, 2017-08-29 16:12:58]

CQ is committing da patch.

Bot data: {"patchset_id": 100001, "attempt_start_ts": 1504023022718510,
"parent_rev": "c63f9345e723df57132405075107ee182a8fa8f9", "commit_rev":
"579de6faef60ebe6e60a795041fecb92569d4f72"}

[commit-bot: I haz the power, 2017-08-29 16:13:05]

Description was changed from

==========
Add a new frame generator that cycles through randomly generated slides.

Like YuvFileGenerator, this also updates the display with a new slide on every
Nth frame, but it generates the slides itself instead of reading them from
files.



BUG=webrtc:8138
==========

to

==========
Add a new frame generator that cycles through randomly generated slides.

Like YuvFileGenerator, this also updates the display with a new slide on every
Nth frame, but it generates the slides itself instead of reading them from
files.



BUG=webrtc:8138

Review-Url: https://codereview.webrtc.org/3003193002
Cr-Commit-Position: refs/heads/master@{#19585}
Committed:
https://chromium.googlesource.com/external/webrtc/+/579de6faef60ebe6e60a79504...
==========

[commit-bot: I haz the power, 2017-08-29 16:13:06]

Committed patchset #6 (id:100001) as
https://chromium.googlesource.com/external/webrtc/+/579de6faef60ebe6e60a79504...