Don't use libvpx if it is disabled in GN build

Don't use libvpx if it is disabled in GN build

This change excludes libvpx from linking if it is disabled. This is
needed to enable linking Chrome with GN, since libvpx is not building
with GN yet.

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

[Alpha Left Google, 2014-08-26 01:08:26]

hclam@chromium.org changed reviewers:
+ hubbe@chromium.org, sergeyu@chromium.org

[brettw, 2014-08-26 03:14:21]

brettw@chromium.org changed reviewers:
+ brettw@chromium.org

[brettw, 2014-08-26 03:14:21]

lgtm

https://codereview.chromium.org/501153003/diff/1/media/cast/sender/video_enco...
File media/cast/sender/video_encoder_impl.cc (right):

https://codereview.chromium.org/501153003/diff/1/media/cast/sender/video_enco...
media/cast/sender/video_encoder_impl.cc:89: #endif  // if
!defined(MEDIA_DISABLE_LIBVPX)
Did you mean to wrap the fake one in this block as well?

[hubbe, 2014-08-26 17:51:53]

On 2014/08/26 01:08:26, Alpha wrote:

What's the point of building cast without vpx?

[brettw, 2014-08-26 17:57:12]

On 2014/08/26 17:51:53, hubbe wrote:
> On 2014/08/26 01:08:26, Alpha wrote:
> 
> What's the point of building cast without vpx?

It's not converted to GN yet and will take some more time to convert.

[Alpha Left Google, 2014-08-26 17:58:23]

https://codereview.chromium.org/501153003/diff/1/media/cast/sender/video_enco...
File media/cast/sender/video_encoder_impl.cc (right):

https://codereview.chromium.org/501153003/diff/1/media/cast/sender/video_enco...
media/cast/sender/video_encoder_impl.cc:89: #endif  // if
!defined(MEDIA_DISABLE_LIBVPX)
On 2014/08/26 03:14:21, brettw wrote:
> Did you mean to wrap the fake one in this block as well?

Acknowledged.

[Alpha Left Google, 2014-08-26 17:59:21]

That's only for the GN build. GYP build has libvpx support. GN build is work in
progress.

[hubbe, 2014-08-26 18:33:26]

On 2014/08/26 17:59:21, Alpha wrote:
> That's only for the GN build. GYP build has libvpx support. GN build is work
in
> progress.

But building cast without VPX is like a hedgehog without spines: pointless.

[Alpha Left Google, 2014-08-26 18:40:26]

On 2014/08/26 18:33:26, hubbe wrote:
> On 2014/08/26 17:59:21, Alpha wrote:
> > That's only for the GN build. GYP build has libvpx support. GN build is work
> in
> > progress.
> 
> But building cast without VPX is like a hedgehog without spines: pointless.

Being able to build and link Chrome with GN is not pointless. GN is work in
progress. It's not going to have all features in one go. The alternative is to
exclude cast related source files from linking in Chrome but that affects a lot
more files.

[brettw, 2014-08-26 19:21:05]

On 2014/08/26 18:33:26, hubbe wrote:
> On 2014/08/26 17:59:21, Alpha wrote:
> > That's only for the GN build. GYP build has libvpx support. GN build is work
> in
> > progress.
> 
> But building cast without VPX is like a hedgehog without spines: pointless.

It's very important that we can link chrome ASAP so that the buildbots will
catch errors when people introduce linker errors in the GN build. Currently it's
easy to add a file to the GYP build and forget to add it to the GN build, and no
error will be thrown because there is no final target linking everything
together.

This and webrtc at the only things preventing this at this point.

[hubbe, 2014-08-26 19:26:25]

On 2014/08/26 19:21:05, brettw wrote:
> On 2014/08/26 18:33:26, hubbe wrote:
> > On 2014/08/26 17:59:21, Alpha wrote:
> > > That's only for the GN build. GYP build has libvpx support. GN build is
work
> > in
> > > progress.
> > 
> > But building cast without VPX is like a hedgehog without spines: pointless.
> 
> It's very important that we can link chrome ASAP so that the buildbots will
> catch errors when people introduce linker errors in the GN build. Currently
it's
> easy to add a file to the GYP build and forget to add it to the GN build, and
no
> error will be thrown because there is no final target linking everything
> together.
> 
> This and webrtc at the only things preventing this at this point.

LGTM
As long as this is reverted once VPX actually works, since it's not a
configuration we actually want to support.

[Alpha Left Google, 2014-08-26 19:29:26]

Thanks. This will be reverted once we have libvpx building with GN.

ping sergeyu@.

[Sergey Ulanov, 2014-08-26 20:32:41]

Ideally remoting/protocol/session_config.cc should be updated to disabled VP8
codec when libvpx is disabled (see CandidateSessionConfig::CreateDefault()).
LGTM with that change.

https://codereview.chromium.org/501153003/diff/20001/remoting/client/software...
File remoting/client/software_video_renderer.cc (right):

https://codereview.chromium.org/501153003/diff/20001/remoting/client/software...
remoting/client/software_video_renderer.cc:20: #include
"remoting/codec/video_decoder_vpx.h"
nit: remove if. It should look as follows
  #endif  // !defined(MEDIA_DISABLE_LIBVPX)

https://codereview.chromium.org/501153003/diff/20001/remoting/client/software...
remoting/client/software_video_renderer.cc:158: #endif  // if
!defined(MEDIA_DISABLE_LIBVPX)
Same here

[Alpha Left Google, 2014-08-26 21:03:55]

On 2014/08/26 20:32:41, Sergey Ulanov wrote:
> Ideally remoting/protocol/session_config.cc should be updated to disabled VP8
> codec when libvpx is disabled (see CandidateSessionConfig::CreateDefault()).
> LGTM with that change.
> 
>
https://codereview.chromium.org/501153003/diff/20001/remoting/client/software...
> File remoting/client/software_video_renderer.cc (right):
> 
>
https://codereview.chromium.org/501153003/diff/20001/remoting/client/software...
> remoting/client/software_video_renderer.cc:20: #include
> "remoting/codec/video_decoder_vpx.h"
> nit: remove if. It should look as follows
>   #endif  // !defined(MEDIA_DISABLE_LIBVPX)
> 
>
https://codereview.chromium.org/501153003/diff/20001/remoting/client/software...
> remoting/client/software_video_renderer.cc:158: #endif  // if
> !defined(MEDIA_DISABLE_LIBVPX)
> Same here

Done all.

[Alpha Left Google, 2014-08-26 21:03:59]

The CQ bit was checked by hclam@chromium.org

[commit-bot: I haz the power, 2014-08-26 21:05:52]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/hclam@chromium.org/501153003/40001

[commit-bot: I haz the power, 2014-08-26 22:27:19]

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

[commit-bot: I haz the power, 2014-08-26 22:35:24]

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

[commit-bot: I haz the power, 2014-08-26 22:35:25]

Try jobs failed on following builders:
  android_clang_dbg on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_clang_d...)

[Alpha Left Google, 2014-08-26 22:36:26]

The CQ bit was checked by hclam@chromium.org

[commit-bot: I haz the power, 2014-08-26 22:38:48]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/hclam@chromium.org/501153003/40001

[commit-bot: I haz the power, 2014-08-26 22:46:47]

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

[commit-bot: I haz the power, 2014-08-26 22:54:46]

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

[commit-bot: I haz the power, 2014-08-26 22:54:47]

Try jobs failed on following builders:
  android_clang_dbg on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_clang_d...)

[Alpha Left Google, 2014-08-26 22:55:26]

The CQ bit was checked by hclam@chromium.org

[commit-bot: I haz the power, 2014-08-26 22:57:19]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/hclam@chromium.org/501153003/40001

[commit-bot: I haz the power, 2014-08-26 23:03:27]

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

[commit-bot: I haz the power, 2014-08-26 23:10:11]

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

[commit-bot: I haz the power, 2014-08-26 23:10:12]

Try jobs failed on following builders:
  android_clang_dbg on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_clang_d...)

[Alpha Left Google, 2014-08-28 18:06:23]

The CQ bit was checked by hclam@chromium.org

[commit-bot: I haz the power, 2014-08-28 18:07:36]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/hclam@chromium.org/501153003/40001

[commit-bot: I haz the power, 2014-08-28 18:13:52]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  android_arm64_dbg_recipe on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_arm64_d...)
  android_clang_dbg_recipe on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_clang_d...)

[commit-bot: I haz the power, 2014-08-28 18:21:15]

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

[commit-bot: I haz the power, 2014-08-28 18:21:16]

Try jobs failed on following builders:
  android_arm64_dbg_recipe on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_arm64_d...)
  android_clang_dbg_recipe on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_clang_d...)

[Alpha Left Google, 2014-08-28 18:22:59]

The CQ bit was checked by hclam@chromium.org

[commit-bot: I haz the power, 2014-08-28 18:23:43]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/hclam@chromium.org/501153003/40001

[commit-bot: I haz the power, 2014-08-28 18:33:02]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  android_arm64_dbg_recipe on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_arm64_d...)
  android_clang_dbg_recipe on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_clang_d...)

[commit-bot: I haz the power, 2014-08-28 18:40:47]

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

[commit-bot: I haz the power, 2014-08-28 18:40:49]

Try jobs failed on following builders:
  android_arm64_dbg_recipe on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_arm64_d...)
  android_clang_dbg_recipe on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_clang_d...)

[Alpha Left Google, 2014-08-28 18:42:04]

The CQ bit was checked by hclam@chromium.org

[Alpha Left Google, 2014-08-28 18:42:29]

The CQ bit was unchecked by hclam@chromium.org

[Alpha Left Google, 2014-08-28 19:53:57]

The CQ bit was checked by hclam@chromium.org

[commit-bot: I haz the power, 2014-08-28 19:55:19]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/hclam@chromium.org/501153003/60001

[commit-bot: I haz the power, 2014-08-28 20:52:07]

Committed patchset #4 (id:60001) as 70b18bc45630f53206b6b464d8516d6e4a958efd

[commit-bot: I haz the power, 2014-09-10 03:02:04]

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