Add VTSupport for dynamically linking VideoToolbox.

Add VTSupport for dynamically linking VideoToolbox.

BUG=133828
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=279593

[DaleCurtis, 2014-06-18 21:55:33]

Over to rsesek@ for osx review. I'll stamp after he's done. cc:mcasas who went
through a lot of this for AVFoundation support.

[mcasas, 2014-06-19 12:04:24]

Drive-by.

https://codereview.chromium.org/340933002/diff/1/content/common/gpu/media/vt_...
File content/common/gpu/media/vt_support.cc (right):

https://codereview.chromium.org/340933002/diff/1/content/common/gpu/media/vt_...
content/common/gpu/media/vt_support.cc:29: RTLD_LAZY | RTLD_LOCAL);
First create the Bundle and then dlopen() it:

NSBundle* bundle = [NSBundle bundleWithPath:
    @"/System/Library/Frameworks/VideoToolbox.framework/VideoToolbox"];

const char* path = [[bundle executablePath] fileSystemRepresentation];
CHECK(path);
void* library_handle = dlopen(path, RTLD_LAZY | RTLD_LOCAL);
(D)CHECK(library_handle) << dlerror();

https://codereview.chromium.org/340933002/diff/1/content/common/gpu/media/vt_...
content/common/gpu/media/vt_support.cc:35: dlsym(cm_handle_,
"CMBlockBufferCreateWithMemoryBlock"));
CHECK(CMBlockBufferCreateWithMemoryBlock) << dlerror(); ?

https://codereview.chromium.org/340933002/diff/1/content/common/gpu/media/vt_...
File content/common/gpu/media/vt_support.h (right):

https://codereview.chromium.org/340933002/diff/1/content/common/gpu/media/vt_...
content/common/gpu/media/vt_support.h:28: struct CMTime {
This is also defined here:
https://code.google.com/p/chromium/codesearch#chromium/src/media/video/captur...

https://codereview.chromium.org/340933002/diff/1/content/common/gpu/media/vt_...
content/common/gpu/media/vt_support.h:57: class VTSupport {
- Looking at src/media/video/capture/mac/, this class should be called
VideoToolboxGlue.

- There is already code in [1] for lazily loading and manipulating CoreMedia,
could we use it from here or refactor somehow?

- In [1] the idiom is a public CoreMediaGlue class offering data types and
static class methods and in the .mm file a singleton CoreMediaGlueInternal is
manipulated via lazy initialiser (good for startup time). Function pointer
typedefs and their pointer are not exposed. I think you should take the same
approach here for the VT parts.

[1]
https://code.google.com/p/chromium/codesearch#chromium/src/media/video/captur...

[Robert Sesek, 2014-06-19 14:17:58]

I'd consider using //tools/generate_stubs for this. I recently got a chance to
use it, and it would eliminate most of this boilerplate. You create a .sig file
that defines all the C function signatures you want to dlsym and a .fragment
header file to forward-declare the types used. A gyp action then generates the
header and stub loader automatically.

[sandersd (OOO until July 31), 2014-06-19 17:31:22]

On 2014/06/19 14:17:58, rsesek wrote:
> I'd consider using //tools/generate_stubs for this. I recently got a chance to
> use it, and it would eliminate most of this boilerplate. You create a .sig
file
> that defines all the C function signatures you want to dlsym and a .fragment
> header file to forward-declare the types used. A gyp action then generates the
> header and stub loader automatically.

In general I have two specific requirements that make this harder than usual:
  - I would like the option to fallback to the PrivateFramework versions of the
frameworks on Mac OS X < 10.8.
  - CMVideoFormatDescriptionCreateFromH264ParameterSets is 10.9+, I need the
ability to implement a fallback.

Using the generator looks like the best option, but it will take some work to
implement these features.

[sandersd (OOO until July 31), 2014-06-20 00:54:13]

I've rewritten using the generate_stubs.py tool, PTAL.

The big drawback of this approach is that it requires 10.8 SDK headers to
compile. If needed, we can work around that by copying the typedefs from my
original version into the fragment file.

It also does not support allowing symbols to be missing. That's okay for now, I
can worry about it later.

re merge with src/media/video/capture/mac/: This is not currently a safe idea,
as it would prevent that code from working on OS X < 10.9, while it currently
works fine.

[mcasas, 2014-06-23 06:55:44]

On 2014/06/20 00:54:13, sandersd wrote:
> I've rewritten using the generate_stubs.py tool, PTAL.
> 
> The big drawback of this approach is that it requires 10.8 SDK headers to
> compile. If needed, we can work around that by copying the typedefs from my
> original version into the fragment file.
> 
> It also does not support allowing symbols to be missing. That's okay for now,
I
> can worry about it later.
> 
> re merge with src/media/video/capture/mac/: This is not currently a safe idea,
> as it would prevent that code from working on OS X < 10.9, while it currently
> works fine.

Not sure I understand. CoreMedia is available in OS > 10.6,
and so is the glue.

[sandersd (OOO until July 31), 2014-06-23 17:16:04]

On 2014/06/23 06:55:44, mcasas wrote:
> On 2014/06/20 00:54:13, sandersd wrote:
> > I've rewritten using the generate_stubs.py tool, PTAL.
> > 
> > The big drawback of this approach is that it requires 10.8 SDK headers to
> > compile. If needed, we can work around that by copying the typedefs from my
> > original version into the fragment file.
> > 
> > It also does not support allowing symbols to be missing. That's okay for
now,
> I
> > can worry about it later.
> > 
> > re merge with src/media/video/capture/mac/: This is not currently a safe
idea,
> > as it would prevent that code from working on OS X < 10.9, while it
currently
> > works fine.
> 
> Not sure I understand. CoreMedia is available in OS > 10.6,
> and so is the glue.

CMVideoFormatDescriptionCreateFromH264ParameterSets is not.

[Robert Sesek, 2014-06-23 21:54:19]

On 2014/06/20 00:54:13, sandersd wrote:
> I've rewritten using the generate_stubs.py tool, PTAL.
> 
> The big drawback of this approach is that it requires 10.8 SDK headers to
> compile. If needed, we can work around that by copying the typedefs from my
> original version into the fragment file.

That's not going to fly, since official builds require the 10.6 SDK. If you'd
like to test this locally, follow these instructions:

https://docs.google.com/a/google.com/document/d/1af7kmgO0gRiKZT3CBSzk0qGxxsdr...

You'll have to forward-declare the types. That's what I did in //sandbox/mac, if
you want an example.

> It also does not support allowing symbols to be missing. That's okay for now,
I
> can worry about it later.
> 
> re merge with src/media/video/capture/mac/: This is not currently a safe idea,
> as it would prevent that code from working on OS X < 10.9, while it currently
> works fine.

[sandersd (OOO until July 31), 2014-06-23 23:49:04]

> You'll have to forward-declare the types. That's what I did in //sandbox/mac,
if you want an example.

Will do.

How do you get the .sig and .fragment files to be available at build time?

[sandersd (OOO until July 31), 2014-06-24 00:29:41]

On 2014/06/23 at 23:49:04, sandersd wrote:
> > You'll have to forward-declare the types. That's what I did in
//sandbox/mac, if you want an example.
> 
> Will do.
> 
> How do you get the .sig and .fragment files to be available at build time?

Ignore that, I just had typos in some path names.

I've added forward declarations, PTAL.

[Robert Sesek, 2014-06-24 19:05:02]

LGTM

https://codereview.chromium.org/340933002/diff/60001/content/common/gpu/media...
File content/common/gpu/media/vt.h (right):

https://codereview.chromium.org/340933002/diff/60001/content/common/gpu/media...
content/common/gpu/media/vt.h:20: 
nit: extra blank line

[sandersd (OOO until July 31), 2014-06-24 20:04:25]

dalecurtis: approve for content/common/gpu/media?

https://codereview.chromium.org/340933002/diff/60001/content/common/gpu/media...
File content/common/gpu/media/vt.h (right):

https://codereview.chromium.org/340933002/diff/60001/content/common/gpu/media...
content/common/gpu/media/vt.h:20: 
On 2014/06/24 19:05:02, rsesek wrote:
> nit: extra blank line

Done.

[DaleCurtis, 2014-06-24 20:17:39]

lgtm

[sandersd (OOO until July 31), 2014-06-24 20:22:26]

The CQ bit was checked by sandersd@chromium.org

[commit-bot: I haz the power, 2014-06-24 20:23:34]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/sandersd@chromium.org/340933002/80001

[commit-bot: I haz the power, 2014-06-25 00:06:21]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  android_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_dbg/builds/19...)

[commit-bot: I haz the power, 2014-06-25 04:56:09]

Change committed as 279593