Build supported media targets on iOS.

Build supported media targets on iOS.

Makes the changes required to get media and media_unittests to build
successfully for iOS, building only the limited set of files that iOS requires.

BUG=None


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=154996

[blundell, 2012-09-05 09:50:27]

PTAL, thanks!

[stuartmorgan, 2012-09-05 11:10:19]

Since media_android.cc is essentially identical, I think we should rename it to
something like media_stub.cc, and have us and Android pull it in manually. It
seems silly to have two copies of the do-nothing implementation.

https://chromiumcodereview.appspot.com/10919092/diff/1/media/media.gyp
File media/media.gyp (right):

https://chromiumcodereview.appspot.com/10919092/diff/1/media/media.gyp#newcod...
media/media.gyp:328: # IOS support is limited to audio input only.
iOS

https://chromiumcodereview.appspot.com/10919092/diff/1/media/media.gyp#newcod...
media/media.gyp:332: ['include', '^audio/audio_input_controller\\.h$'],
Combine these into:
['include', '^audio/audio_input_controller\\.'],
Same for all other cc/h pairs.

https://chromiumcodereview.appspot.com/10919092/diff/1/media/media.gyp#newcod...
media/media.gyp:348: ],
Indent 2 more.

https://chromiumcodereview.appspot.com/10919092/diff/1/media/media.gyp#newcod...
media/media.gyp:356: # Android and iOS don't use ffmpeg, so make the dependency
conditional.
Remove everything from the ',' on.

https://chromiumcodereview.appspot.com/10919092/diff/1/media/media.gyp#newcod...
media/media.gyp:636: # Android and iOS don't use ffmpeg, so make the dependency
conditional.
Same as above.

[blundell, 2012-09-05 12:04:29]

Created media_stub.cc. Also added use_ffmpeg variable to media.gyp as discussed
offline.

https://chromiumcodereview.appspot.com/10919092/diff/1/media/media.gyp
File media/media.gyp (right):

https://chromiumcodereview.appspot.com/10919092/diff/1/media/media.gyp#newcod...
media/media.gyp:328: # IOS support is limited to audio input only.
On 2012/09/05 11:10:19, stuartmorgan wrote:
> iOS

Done.

https://chromiumcodereview.appspot.com/10919092/diff/1/media/media.gyp#newcod...
media/media.gyp:332: ['include', '^audio/audio_input_controller\\.h$'],
On 2012/09/05 11:10:19, stuartmorgan wrote:
> Combine these into:
> ['include', '^audio/audio_input_controller\\.'],
> Same for all other cc/h pairs.

Done.

https://chromiumcodereview.appspot.com/10919092/diff/1/media/media.gyp#newcod...
media/media.gyp:348: ],
On 2012/09/05 11:10:19, stuartmorgan wrote:
> Indent 2 more.

Done.

https://chromiumcodereview.appspot.com/10919092/diff/1/media/media.gyp#newcod...
media/media.gyp:356: # Android and iOS don't use ffmpeg, so make the dependency
conditional.
On 2012/09/05 11:10:19, stuartmorgan wrote:
> Remove everything from the ',' on.

Done.

https://chromiumcodereview.appspot.com/10919092/diff/1/media/media.gyp#newcod...
media/media.gyp:636: # Android and iOS don't use ffmpeg, so make the dependency
conditional.
On 2012/09/05 11:10:19, stuartmorgan wrote:
> Same as above.

Done.

[stuartmorgan, 2012-09-05 12:34:24]

LGTM with nits

https://chromiumcodereview.appspot.com/10919092/diff/5/media/base/media_stub.cc
File media/base/media_stub.cc (right):

https://chromiumcodereview.appspot.com/10919092/diff/5/media/base/media_stub....
media/base/media_stub.cc:6: #include "base/logging.h"
Blank line above this one (the Android file was wrong)

https://chromiumcodereview.appspot.com/10919092/diff/5/media/base/media_stub....
media/base/media_stub.cc:7: 
Put a short comment here explaining the file, like:

// This file is intended for platforms that don't need to load any media
libraries
// (e.g., Android and iOS).

https://chromiumcodereview.appspot.com/10919092/diff/5/media/base/media_stub....
media/base/media_stub.cc:14: void InitializeMediaLibraryForTesting() {}
I think this is more readable with a break before }, but we don't have a rule
AFAIK so you can leave it if you want.

https://chromiumcodereview.appspot.com/10919092/diff/5/media/media.gyp
File media/media.gyp (right):

https://chromiumcodereview.appspot.com/10919092/diff/5/media/media.gyp#newcode13
media/media.gyp:13: ['OS=="android" or OS=="ios"', {
Spaces around the == (we aren't consistent, but it's more readable so unless a
file already uses the no-space form exclusively, it's preferred).

https://chromiumcodereview.appspot.com/10919092/diff/5/media/media.gyp#newcode16
media/media.gyp:16: }, {  # 'OS!="android" and OS!="ios"'
Same

https://chromiumcodereview.appspot.com/10919092/diff/5/media/media.gyp#newcod...
media/media.gyp:322: ['OS == "ios"', {
Move this down to at least after the use_ffmpeg block; it's helpful to have
specific-OS blocks grouped together after things related to a flag, or things
that apply to all but one OS.

https://chromiumcodereview.appspot.com/10919092/diff/5/media/media.gyp#newcod...
media/media.gyp:629: ['OS == "ios"', {
Same here.

[blundell, 2012-09-05 12:51:08]

+scherkus for OWNERS

https://chromiumcodereview.appspot.com/10919092/diff/5/media/base/media_stub.cc
File media/base/media_stub.cc (right):

https://chromiumcodereview.appspot.com/10919092/diff/5/media/base/media_stub....
media/base/media_stub.cc:6: #include "base/logging.h"
On 2012/09/05 12:34:24, stuartmorgan wrote:
> Blank line above this one (the Android file was wrong)

Done.

https://chromiumcodereview.appspot.com/10919092/diff/5/media/base/media_stub....
media/base/media_stub.cc:7: 
On 2012/09/05 12:34:24, stuartmorgan wrote:
> Put a short comment here explaining the file, like:
> 
> // This file is intended for platforms that don't need to load any media
> libraries
> // (e.g., Android and iOS).

Done.

https://chromiumcodereview.appspot.com/10919092/diff/5/media/base/media_stub....
media/base/media_stub.cc:14: void InitializeMediaLibraryForTesting() {}
On 2012/09/05 12:34:24, stuartmorgan wrote:
> I think this is more readable with a break before }, but we don't have a rule
> AFAIK so you can leave it if you want.

Done.

https://chromiumcodereview.appspot.com/10919092/diff/5/media/media.gyp
File media/media.gyp (right):

https://chromiumcodereview.appspot.com/10919092/diff/5/media/media.gyp#newcode13
media/media.gyp:13: ['OS=="android" or OS=="ios"', {
On 2012/09/05 12:34:24, stuartmorgan wrote:
> Spaces around the == (we aren't consistent, but it's more readable so unless a
> file already uses the no-space form exclusively, it's preferred).

Done.

https://chromiumcodereview.appspot.com/10919092/diff/5/media/media.gyp#newcode16
media/media.gyp:16: }, {  # 'OS!="android" and OS!="ios"'
On 2012/09/05 12:34:24, stuartmorgan wrote:
> Same

Done.

https://chromiumcodereview.appspot.com/10919092/diff/5/media/media.gyp#newcod...
media/media.gyp:322: ['OS == "ios"', {
On 2012/09/05 12:34:24, stuartmorgan wrote:
> Move this down to at least after the use_ffmpeg block; it's helpful to have
> specific-OS blocks grouped together after things related to a flag, or things
> that apply to all but one OS.

Done.

https://chromiumcodereview.appspot.com/10919092/diff/5/media/media.gyp#newcod...
media/media.gyp:629: ['OS == "ios"', {
On 2012/09/05 12:34:24, stuartmorgan wrote:
> Same here.

Done.

[scherkus (not reviewing), 2012-09-05 15:39:22]

https://chromiumcodereview.appspot.com/10919092/diff/7/media/base/media_stub.cc
File media/base/media_stub.cc (right):

https://chromiumcodereview.appspot.com/10919092/diff/7/media/base/media_stub....
media/base/media_stub.cc:1: // Copyright 2012 The Chromium Authors. All rights
reserved.
where did the (c) go? :)

https://chromiumcodereview.appspot.com/10919092/diff/7/media/media.gyp
File media/media.gyp (right):

https://chromiumcodereview.appspot.com/10919092/diff/7/media/media.gyp#newcod...
media/media.gyp:370: # iOS support is limited to audio input only.
should audio/mac/audio_input_mac\\. be listed here or is that covered by another
rule?

[blundell, 2012-09-05 15:54:49]

Thanks for the review.

https://chromiumcodereview.appspot.com/10919092/diff/7/media/base/media_stub.cc
File media/base/media_stub.cc (right):

https://chromiumcodereview.appspot.com/10919092/diff/7/media/base/media_stub....
media/base/media_stub.cc:1: // Copyright 2012 The Chromium Authors. All rights
reserved.
The new Chromium convention as of quite recently is that the "(c)" is no longer
included. :)

On 2012/09/05 15:39:22, scherkus wrote:
> where did the (c) go? :)

https://chromiumcodereview.appspot.com/10919092/diff/7/media/media.gyp
File media/media.gyp (right):

https://chromiumcodereview.appspot.com/10919092/diff/7/media/media.gyp#newcod...
media/media.gyp:370: # iOS support is limited to audio input only.
We are not using audio_input_mac. Stay tuned for an exciting future installment
wherein audio_input_ios is upstreamed :).
On 2012/09/05 15:39:22, scherkus wrote:
> should audio/mac/audio_input_mac\\. be listed here or is that covered by
another
> rule?

[scherkus (not reviewing), 2012-09-05 15:56:56]

lgtm

https://chromiumcodereview.appspot.com/10919092/diff/7/media/media.gyp
File media/media.gyp (right):

https://chromiumcodereview.appspot.com/10919092/diff/7/media/media.gyp#newcod...
media/media.gyp:370: # iOS support is limited to audio input only.
SGTM!

On 2012/09/05 15:54:49, blundell wrote:
> We are not using audio_input_mac. Stay tuned for an exciting future
installment
> wherein audio_input_ios is upstreamed :).
> On 2012/09/05 15:39:22, scherkus wrote:
> > should audio/mac/audio_input_mac\\. be listed here or is that covered by
> another
> > rule?
>

[commit-bot: I haz the power, 2012-09-05 16:37:55]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/blundell@chromium.org/10919092/7

[commit-bot: I haz the power, 2012-09-05 18:53:53]

Change committed as 154996