Extend media glue code to support the VideoToolbox cast video encoder.

Extend media glue code to support the VideoToolbox cast video encoder.

BUG=409897
R=rsesek, DaleCurtis

Committed: https://crrev.com/79fae2e8bf9f295e70b9dba9744121faae076c37
Cr-Commit-Position: refs/heads/master@{#294654}

[jfroy, 2014-09-10 00:27:09]

This is the final set of media glue functions for the VideoToolbox encoder.
Thanks for the review :)

[jfroy, 2014-09-11 01:16:11]

I significantly reworked the VideoToolbox glue to

1) reduce function call overhead
2) allow load failure.

On 1, I changed the glue to call functions through a library member struct that
contains the symbol pointers, instead of getting the lazy library handle
instance. Getting a lazy instance is costly on armv7 because of the full barrier
that's part of the atomic access. This revised version gets the lazy instance
once in VideoToolboxGlue::Get().

On 2, I allow the Get() function to return NULL, indicating that
VideoToolboxGlue is not available. The previous implementation provided no safe
means of testing (it had a static function prototype that was not implemented;
calling any of the glue-ed functions would fail a CHECK).

I'm not sure how good this code is w/r to Chromium coding standards, especially
on returning a naked pointer from VideoToolboxGlue::Get(). Feedback is welcome
of course :)

[Robert Sesek, 2014-09-12 16:16:17]

LGTM

https://codereview.chromium.org/531863002/diff/80001/media/base/mac/coremedia...
File media/base/mac/coremedia_glue.h (right):

https://codereview.chromium.org/531863002/diff/80001/media/base/mac/coremedia...
media/base/mac/coremedia_glue.h:111:
/*__OSX_AVAILABLE_STARTING(__MAC_10_9,__IPHONE_7_0)*/;
None of the other functions carry these comments, so I'd remove.

https://codereview.chromium.org/531863002/diff/80001/media/base/mac/videotool...
File media/base/mac/videotoolbox_glue.h (right):

https://codereview.chromium.org/531863002/diff/80001/media/base/mac/videotool...
media/base/mac/videotoolbox_glue.h:19: class Loader;
Move to private: ?

[jfroy, 2014-09-12 17:11:47]

https://codereview.chromium.org/531863002/diff/80001/media/base/mac/coremedia...
File media/base/mac/coremedia_glue.h (right):

https://codereview.chromium.org/531863002/diff/80001/media/base/mac/coremedia...
media/base/mac/coremedia_glue.h:111:
/*__OSX_AVAILABLE_STARTING(__MAC_10_9,__IPHONE_7_0)*/;
On 2014/09/12 16:16:16, rsesek wrote:
> None of the other functions carry these comments, so I'd remove.

Unlike every other function in the CoreMedia glue, this function is only
available on 10.9 and 7.0. If you try to call it on an unsupported OS, you will
crash on a null dereference. There has to be some indication of this. Any
suggestion?

Also as a note, no one seems to be using the CoreMedia glue directly, or
primarily. The AVF glue and the H.264 VT encoder do, but they check for AVF or
VT availability first, which both guarantee that CM is available. The CM glue
itself provides no mechanism to check for availability. On first use the lazy
instance is created and if CM is not there you CHECK assert.

https://codereview.chromium.org/531863002/diff/80001/media/base/mac/videotool...
File media/base/mac/videotoolbox_glue.h (right):

https://codereview.chromium.org/531863002/diff/80001/media/base/mac/videotool...
media/base/mac/videotoolbox_glue.h:19: class Loader;
On 2014/09/12 16:16:16, rsesek wrote:
> Move to private: ?

Loader is the lazy instance class in the implementation and needs to be a public
member to declare the static lazy instance variable. I wasn't sure if this was
better or worse than declaring Loader in the private section along with the
static lazy instance variable (i.e. a class static vs a file static). Both are
ugly exposé of the guts.

[DaleCurtis, 2014-09-12 17:51:57]

lgtm

[jfroy, 2014-09-12 19:22:27]

The CQ bit was checked by jfroy@chromium.org

[commit-bot: I haz the power, 2014-09-12 19:38:47]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patchset/531863002/80001

[commit-bot: I haz the power, 2014-09-12 20:50:43]

Committed patchset #5 (id:80001) as fb215fc2e43f9d77dd1bb77ab4c1e54ba1209054

[commit-bot: I haz the power, 2014-09-12 20:55:16]

Patchset 5 (id:??) landed as
https://crrev.com/79fae2e8bf9f295e70b9dba9744121faae076c37
Cr-Commit-Position: refs/heads/master@{#294654}