Change pre_encode_callback to get a const frame.

Change pre_encode_callback to get a const frame.

Used only by tests. Deleted the EndToEndTest.UsesFrameCallbacks, which
modified pixel data. Change callback from in EndToEndTest.GetStats to call SleepMs, rath than
modifying the timestamp.

BUG=

Committed: https://crrev.com/d30a1117f842f43e089404981361350b53b4f159
Cr-Commit-Position: refs/heads/master@{#12406}

[nisse-webrtc, 2016-04-14 11:05:15]

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

[commit-bot: I haz the power, 2016-04-14 11:05:22]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1891733002/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1891733002/1

[commit-bot: I haz the power, 2016-04-14 12:26:30]

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

[commit-bot: I haz the power, 2016-04-14 12:26:31]

Dry run: This issue passed the CQ dry run.

[nisse-webrtc, 2016-04-15 08:10:08]

nisse@webrtc.org changed reviewers:
+ pbos@webrtc.org, perkj@webrtc.org

[nisse-webrtc, 2016-04-15 08:10:09]

This cl is a bit speculative, I'd like to know what you think.

The reason I try it now is that it eliminates a call to CreateFrame. It would be
even better to delete pre_encode_callback completely; this is only a small step
in that direction, dumbing it down so that it can't modify the frame.

In the tests, I deleted the testcase UsesFrameCallbacks, which sets up callback
which to real modifications of the pixels.

In the GetStats test, a pre_encode_callback was used to subtract 5 ms from the
frame timestamp. Unclear to me why that was done. The tests seems to still pass
when I delete that... pbos, are you familiar with these tests? It seems you
refactored them one and a half year ago.

[perkj_webrtc, 2016-04-15 08:22:22]

lgtm

https://codereview.webrtc.org/1891733002/diff/20001/webrtc/video/vie_encoder.h
File webrtc/video/vie_encoder.h (right):

https://codereview.webrtc.org/1891733002/diff/20001/webrtc/video/vie_encoder....
webrtc/video/vie_encoder.h:52: // TODO(nisse): Used only for tests, delete?
I think you can leave it for now. I am working on refactoring this class.

[pbos-webrtc, 2016-04-15 09:02:03]

https://codereview.webrtc.org/1891733002/diff/20001/webrtc/video/end_to_end_t...
File webrtc/video/end_to_end_tests.cc (left):

https://codereview.webrtc.org/1891733002/diff/20001/webrtc/video/end_to_end_t...
webrtc/video/end_to_end_tests.cc:2607: // Ensure that we have at least 5ms send
side delay.
This part of the code was added to make it not flaky (encoding time always > 0),
we need encoding to not be instant, hence adding the delay.

https://codereview.webrtc.org/1891733002/diff/20001/webrtc/video/vie_encoder.cc
File webrtc/video/vie_encoder.cc (right):

https://codereview.webrtc.org/1891733002/diff/20001/webrtc/video/vie_encoder....
webrtc/video/vie_encoder.cc:346: if (pre_encode_callback_) {
remove {}s

https://codereview.webrtc.org/1891733002/diff/20001/webrtc/video_send_stream.h
File webrtc/video_send_stream.h (right):

https://codereview.webrtc.org/1891733002/diff/20001/webrtc/video_send_stream....
webrtc/video_send_stream.h:141: // Called for each I420 frame before encoding
the frame. Can be used for
This comment is blatantly not true anymore.

[pbos-webrtc, 2016-04-15 09:03:43]

https://codereview.webrtc.org/1891733002/diff/20001/webrtc/video/end_to_end_t...
File webrtc/video/end_to_end_tests.cc (left):

https://codereview.webrtc.org/1891733002/diff/20001/webrtc/video/end_to_end_t...
webrtc/video/end_to_end_tests.cc:2607: // Ensure that we have at least 5ms send
side delay.
On 2016/04/15 09:02:02, pbos-webrtc wrote:
> This part of the code was added to make it not flaky (encoding time always >
0),
> we need encoding to not be instant, hence adding the delay.

There exists a test::DelayedEncoder, can we use that in this test instead? I'm
not 100% on why we did this part. Is this on the sender or receiver even?

[nisse-webrtc, 2016-04-15 12:29:07]

https://codereview.webrtc.org/1891733002/diff/20001/webrtc/video/end_to_end_t...
File webrtc/video/end_to_end_tests.cc (left):

https://codereview.webrtc.org/1891733002/diff/20001/webrtc/video/end_to_end_t...
webrtc/video/end_to_end_tests.cc:2607: // Ensure that we have at least 5ms send
side delay.
On 2016/04/15 09:03:43, pbos-webrtc wrote:
> There exists a test::DelayedEncoder, can we use that in this test instead?

You tell me... One of the other tests in this file uses a pre_encode_callback
calling 

  SleepMs(kDelayRenderCallbackMs);

Would that work?

> Is this on the sender or receiver even?

I guess sender, since its an encode callback installed in send_config.

[pbos-webrtc, 2016-04-15 15:03:15]

https://codereview.webrtc.org/1891733002/diff/20001/webrtc/video/end_to_end_t...
File webrtc/video/end_to_end_tests.cc (left):

https://codereview.webrtc.org/1891733002/diff/20001/webrtc/video/end_to_end_t...
webrtc/video/end_to_end_tests.cc:2607: // Ensure that we have at least 5ms send
side delay.
On 2016/04/15 12:29:06, nisse-webrtc wrote:
> On 2016/04/15 09:03:43, pbos-webrtc wrote:
> > There exists a test::DelayedEncoder, can we use that in this test instead?
> 
> You tell me... One of the other tests in this file uses a pre_encode_callback
> calling 
> 
>   SleepMs(kDelayRenderCallbackMs);
> 
> Would that work?
> 
> > Is this on the sender or receiver even?
> 
> I guess sender, since its an encode callback installed in send_config.

I believe so, go for it. :)

[nisse-webrtc, 2016-04-18 11:15:42]

Description was changed from

==========
Change pre_encode_callback to get a const frame.

Used only by tests. Deleted the EndToEndTest.UsesFrameCallbacks, which
modified pixel data. Drop callback from EndToEndTest.GetStats, where
it modified the timestamp for unclear reasons.

BUG=
==========

to

==========
Change pre_encode_callback to get a const frame.

Used only by tests. Deleted the EndToEndTest.UsesFrameCallbacks, which
modified pixel data. Change callback from in EndToEndTest.GetStats to call
SleepMs, rath than
modifying the timestamp.

BUG=
==========

[nisse-webrtc, 2016-04-18 11:15:56]

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

[commit-bot: I haz the power, 2016-04-18 11:15:58]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1891733002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1891733002/40001

[pbos-webrtc, 2016-04-18 11:50:22]

lgtm, hopefully this fixes the
https://build.chromium.org/p/client.webrtc/builders/Win%20DrMemory%20Full/bui...
timeout and a probable perf regression

[pbos-webrtc, 2016-04-18 11:57:36]

The CQ bit was unchecked by pbos@webrtc.org

[pbos-webrtc, 2016-04-18 11:57:41]

The CQ bit was checked by pbos@webrtc.org

[pbos-webrtc, 2016-04-18 11:57:42]

The patchset sent to the CQ was uploaded after l-g-t-m from perkj@webrtc.org
Link to the patchset: https://codereview.webrtc.org/1891733002/#ps40001 (title:
"Do per-frame delay by calling SleepMs.")

[commit-bot: I haz the power, 2016-04-18 11:57:47]

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

[commit-bot: I haz the power, 2016-04-18 12:15:25]

Description was changed from

==========
Change pre_encode_callback to get a const frame.

Used only by tests. Deleted the EndToEndTest.UsesFrameCallbacks, which
modified pixel data. Change callback from in EndToEndTest.GetStats to call
SleepMs, rath than
modifying the timestamp.

BUG=
==========

to

==========
Change pre_encode_callback to get a const frame.

Used only by tests. Deleted the EndToEndTest.UsesFrameCallbacks, which
modified pixel data. Change callback from in EndToEndTest.GetStats to call
SleepMs, rath than
modifying the timestamp.

BUG=
==========

[commit-bot: I haz the power, 2016-04-18 12:15:26]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-04-18 12:15:39]

Description was changed from

==========
Change pre_encode_callback to get a const frame.

Used only by tests. Deleted the EndToEndTest.UsesFrameCallbacks, which
modified pixel data. Change callback from in EndToEndTest.GetStats to call
SleepMs, rath than
modifying the timestamp.

BUG=
==========

to

==========
Change pre_encode_callback to get a const frame.

Used only by tests. Deleted the EndToEndTest.UsesFrameCallbacks, which
modified pixel data. Change callback from in EndToEndTest.GetStats to call
SleepMs, rath than
modifying the timestamp.

BUG=

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

[commit-bot: I haz the power, 2016-04-18 12:15:41]

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