[Cast] Refactor ExternalVideoEncoder for cleaner/simpler encapsulation.

[Cast] Refactor ExternalVideoEncoder for cleaner/simpler encapsulation.

There was a lot of hopping around between two classes, each class having
code that executes on two threads.  This change makes
ExternalVideoEncoder live entirely on the cast MAIN thread, and the VEA
client live entirely on the VEA's own thread.  This greatly simplified
the code and improved readability (especially the create/init sequence).

Also added a new FakeVideoEncodeAcceleratorFactory class to provide
common functionality for multiple unit test modules.  In a soon-upcoming
change, the video_sender_unittest.cc code will start using it.

BUG=325998, 451277
Committed: https://crrev.com/7ccf0ef39c36bed75b19ad08a87a0e93c60893ba
Cr-Commit-Position: refs/heads/master@{#313852}

[miu, 2015-01-29 01:27:03]

miu@chromium.org changed reviewers:
+ hubbe@chromium.org

[miu, 2015-01-29 01:27:04]

hubbe: PTAL.

[hubbe, 2015-01-29 23:29:15]

LGTM

https://codereview.chromium.org/868803007/diff/20001/media/cast/sender/extern...
File media/cast/sender/external_video_encoder.cc (right):

https://codereview.chromium.org/868803007/diff/20001/media/cast/sender/extern...
media/cast/sender/external_video_encoder.cc:102: base::Bind(initialization_cb,
Looks like there is a change in behavior when the codec is reported as
initialized. Is it always permissible to start posting frames to the codec even
if the shared memory hasn't been allocated yet?

https://codereview.chromium.org/868803007/diff/20001/media/cast/sender/extern...
media/cast/sender/external_video_encoder.cc:397: if (!encoder_task_runner ||
!vea) {
Seems like these should be CHECK/DCHECKs?
(At least the first one, when would it be NULL?)

[miu, 2015-01-30 00:20:47]

Thanks.  Addressed your comments:

https://codereview.chromium.org/868803007/diff/20001/media/cast/sender/extern...
File media/cast/sender/external_video_encoder.cc (right):

https://codereview.chromium.org/868803007/diff/20001/media/cast/sender/extern...
media/cast/sender/external_video_encoder.cc:102: base::Bind(initialization_cb,
On 2015/01/29 23:29:15, hubbe wrote:
> Looks like there is a change in behavior when the codec is reported as
> initialized. Is it always permissible to start posting frames to the codec
even
> if the shared memory hasn't been allocated yet?

I looked at the code.  It's safe, since the impl won't try to download from the
GPU until it is provided the shared memory.

https://codereview.chromium.org/868803007/diff/20001/media/cast/sender/extern...
media/cast/sender/external_video_encoder.cc:397: if (!encoder_task_runner ||
!vea) {
On 2015/01/29 23:29:15, hubbe wrote:
> Seems like these should be CHECK/DCHECKs?
> (At least the first one, when would it be NULL?)

The implementation can return nulls when support is lacking or GPU
support/resources are unavailable, or during a renderer shutdown.  The
|initialization_cb| will be run to indicate the initialization failure (to
CastSession and upstream).

I added a comment to explain this.

[miu, 2015-01-30 00:20:51]

The CQ bit was checked by miu@chromium.org

[commit-bot: I haz the power, 2015-01-30 00:22:01]

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

[commit-bot: I haz the power, 2015-01-30 01:24:56]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-01-30 01:25:39]

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