Create BUILD.gn files for //media/formats.

Create BUILD.gn files for //media/formats.

Create new BUILD.gn files for media/formats and moved respective
media component changes from media/BUILD.gn file to new BUILD.gn
files.

BUG=613033
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel

Review-Url: https://codereview.chromium.org/2872853003
Cr-Commit-Position: refs/heads/master@{#474130}
Committed: https://chromium.googlesource.com/chromium/src/+/1cf16502586c8e7c6ba9e9293bda196c04ac4b0e

[a.suchit2, 2017-05-09 06:50:21]

The CQ bit was checked by a.suchit@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-09 06:51:01]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-09 06:54:44]

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

[commit-bot: I haz the power, 2017-05-09 06:54:45]

Dry run: Try jobs failed on following builders:
  cast_shell_android on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)

[a.suchit, 2017-05-10 06:03:08]

Description was changed from

==========
Create BUILD.gn files for subdirectries in //media.

Create new BUILD.gn files for media/cdm, media/filters, media/formats
and moved respective changes from media/BUILD.gn file to these new
BUILD.gn files.

BUG=613033
==========

to

==========
Create BUILD.gn files for subdirectries in //media.

Create new BUILD.gn files for media/cdm, media/filters, media/formats
and moved respective changes from media/BUILD.gn file to these new
BUILD.gn files.

BUG=613033
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

[a.suchit2, 2017-05-10 06:04:40]

The CQ bit was checked by a.suchit@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-10 06:05:29]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-10 06:10:54]

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

[commit-bot: I haz the power, 2017-05-10 06:10:55]

Dry run: Try jobs failed on following builders:
  android_compile_dbg on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)
  android_optional_gpu_tests_rel on master.tryserver.chromium.android
(JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_opti...)

[a.suchit2, 2017-05-10 11:13:56]

The CQ bit was checked by a.suchit@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-10 11:14:39]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-10 11:35:43]

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

[commit-bot: I haz the power, 2017-05-10 11:35:44]

Dry run: Try jobs failed on following builders:
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)

[a.suchit2, 2017-05-10 12:00:55]

a.suchit@chromium.org changed reviewers:
+ hubbe@chromium.org, jam@chromium.org, sgurun@chromium.org, xhwang@chromium.org

[a.suchit2, 2017-05-10 12:07:16]

Still have some build issues in this patch.

[xhwang, 2017-05-10 16:22:54]

+jrummell

Thanks for working on this!

Is it possible to do this in multiple smaller CLs for easier reviewing and
discussion?

[xhwang, 2017-05-10 16:23:11]

Description was changed from

==========
Create BUILD.gn files for subdirectries in //media.

Create new BUILD.gn files for media/cdm, media/filters, media/formats
and moved respective changes from media/BUILD.gn file to these new
BUILD.gn files.

BUG=613033
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

to

==========
Create BUILD.gn files for subdirectries in //media.

Create new BUILD.gn files for media/cdm, media/filters, media/formats
and moved respective changes from media/BUILD.gn file to these new
BUILD.gn files.

BUG=613033
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

[xhwang, 2017-05-10 16:23:11]

xhwang@chromium.org changed reviewers:
+ jrummell@chromium.org

[a.suchit2, 2017-05-10 23:49:33]

On 2017/05/10 16:22:54, xhwang wrote:
> +jrummell
> 
> Thanks for working on this!
> 
> Is it possible to do this in multiple smaller CLs for easier reviewing and
> discussion?

Yes, I can break it based on one subdirectory at a time in one patch.
Is it fine or you want me to break it to based on targets like formats and it's
unittest in different patches.

I know, second one is more reasonable but I need your opinion as well in it.

Thanks.

[xhwang, 2017-05-11 00:02:05]

On 2017/05/10 23:49:33, a.suchit2 wrote:
> On 2017/05/10 16:22:54, xhwang wrote:
> > +jrummell
> > 
> > Thanks for working on this!
> > 
> > Is it possible to do this in multiple smaller CLs for easier reviewing and
> > discussion?
> 
> Yes, I can break it based on one subdirectory at a time in one patch.
> Is it fine or you want me to break it to based on targets like formats and
it's
> unittest in different patches.
> 
> I know, second one is more reasonable but I need your opinion as well in it.
> 
> Thanks.

Either way seems reasonable. We might have more discussion in the first CL, then
hopefully the following CLs will go more smoothly.

[a.suchit2, 2017-05-11 11:29:43]

The CQ bit was checked by a.suchit@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-11 11:30:11]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[a.suchit2, 2017-05-11 11:39:11]

Description was changed from

==========
Create BUILD.gn files for subdirectries in //media.

Create new BUILD.gn files for media/cdm, media/filters, media/formats
and moved respective changes from media/BUILD.gn file to these new
BUILD.gn files.

BUG=613033
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

to

==========
Create BUILD.gn files for //media/formats.

Create new BUILD.gn files for media/formats and moved respective
media component changes from media/BUILD.gn file to new BUILD.gn
files.

BUG=613033
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

[a.suchit2, 2017-05-11 11:39:22]

The CQ bit was checked by a.suchit@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-11 11:39:38]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[a.suchit2, 2017-05-11 11:45:18]

On 2017/05/11 00:02:05, xhwang wrote:
> On 2017/05/10 23:49:33, a.suchit2 wrote:
> > On 2017/05/10 16:22:54, xhwang wrote:
> > > +jrummell
> > > 
> > > Thanks for working on this!
> > > 
> > > Is it possible to do this in multiple smaller CLs for easier reviewing and
> > > discussion?
> > 
> > Yes, I can break it based on one subdirectory at a time in one patch.
> > Is it fine or you want me to break it to based on targets like formats and
> it's
> > unittest in different patches.
> > 
> > I know, second one is more reasonable but I need your opinion as well in it.
> > 
> > Thanks.
> 
> Either way seems reasonable. We might have more discussion in the first CL,
then
> hopefully the following CLs will go more smoothly.

Sorry, I needs to regularly use the BOTs because I would not able to test all
the build in local system. It is very slow on local system and I have only linux
system with me.
And it is Build files changes so all platform build required to test and fix
issues.

[commit-bot: I haz the power, 2017-05-11 11:58:46]

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

[commit-bot: I haz the power, 2017-05-11 11:58:47]

Dry run: Try jobs failed on following builders:
  win_optional_gpu_tests_rel on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_optional_gpu_...)

[a.suchit2, 2017-05-11 12:06:55]

The CQ bit was checked by a.suchit@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-11 12:07:53]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-11 12:25:20]

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

[commit-bot: I haz the power, 2017-05-11 12:25:21]

Dry run: Try jobs failed on following builders:
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)

[jrummell, 2017-05-11 17:36:15]

Welcome to the fun of trying to sort out the media files. Thanks for doing this.

https://codereview.chromium.org/2872853003/diff/180001/media/BUILD.gn
File media/BUILD.gn (right):

https://codereview.chromium.org/2872853003/diff/180001/media/BUILD.gn#newcode277
media/BUILD.gn:277: "formats",
Can you use the full name (//media/formats)?

https://codereview.chromium.org/2872853003/diff/180001/media/BUILD.gn#newcode489
media/BUILD.gn:489: "formats/common/offset_byte_queue_unittest.cc",
Are you going to move the test files in a separate CL?

https://codereview.chromium.org/2872853003/diff/180001/media/formats/BUILD.gn
File media/formats/BUILD.gn (right):

https://codereview.chromium.org/2872853003/diff/180001/media/formats/BUILD.gn...
media/formats/BUILD.gn:1: # Copyright 2014 The Chromium Authors. All rights
reserved.
Since this is a new file it should be 2017.

https://codereview.chromium.org/2872853003/diff/180001/media/formats/BUILD.gn...
media/formats/BUILD.gn:7: source_set("formats") {
Can you add "visibility = [ "//media/*" ]" so that nobody outside of //media can
use this directly.

https://codereview.chromium.org/2872853003/diff/180001/media/formats/BUILD.gn...
media/formats/BUILD.gn:7: source_set("formats") {
Based on the compile errors, you'll need something like:
  configs += [
    "//media:media_config",
    "//media:media_implementation",
  ]
or maybe just:
  configs += [ "//media/base:base_config" ]
(as it includes the 2 above plus some others that may also be necessary).

https://codereview.chromium.org/2872853003/diff/180001/media/formats/BUILD.gn...
media/formats/BUILD.gn:40: "//skia",
There should also be a dependency on //base, //media/base,
//media:media_features, //media:shared_memory_support, and probably some more.
However, this is where the dependency loops get challenging. You might have to
leave some out until all the directories are done.

https://codereview.chromium.org/2872853003/diff/180001/media/formats/BUILD.gn...
media/formats/BUILD.gn:85: if (enable_mse_mpeg2ts_stream_parser) {
Can this be it's own block (if (proprietary_codecs &&
enable_mse_mpeg2ts_stream_parser) ...)? I find it easier to understand when
looking to see what conditions trigger a file getting compiled.

And then I guess (proprietary_codecs) should be the first block, so move the 2
above it down to here as well.

if (proprietary_codecs) ...
if (proprietary_codecs && enable_hevc_demuxing) ...
if (proprietary_codecs && enable_dolby_vision_demuxing) ...
if (proprietary_codecs && enable_mse_mpeg2ts_stream_parser) ...
if (proprietary_codecs && enable_mse_mpeg2ts_stream_parser &&
enable_hls_sample_aes) ...

[a.suchit2, 2017-05-12 05:54:43]

https://codereview.chromium.org/2872853003/diff/180001/media/BUILD.gn
File media/BUILD.gn (right):

https://codereview.chromium.org/2872853003/diff/180001/media/BUILD.gn#newcode277
media/BUILD.gn:277: "formats",
On 2017/05/11 17:36:15, jrummell wrote:
> Can you use the full name (//media/formats)?

Done.

https://codereview.chromium.org/2872853003/diff/180001/media/BUILD.gn#newcode489
media/BUILD.gn:489: "formats/common/offset_byte_queue_unittest.cc",
On 2017/05/11 17:36:15, jrummell wrote:
> Are you going to move the test files in a separate CL?

I am thinking to take unittests in different CL because I am seeing more
dependencies with unittests.

https://codereview.chromium.org/2872853003/diff/180001/media/formats/BUILD.gn
File media/formats/BUILD.gn (right):

https://codereview.chromium.org/2872853003/diff/180001/media/formats/BUILD.gn...
media/formats/BUILD.gn:1: # Copyright 2014 The Chromium Authors. All rights
reserved.
On 2017/05/11 17:36:15, jrummell wrote:
> Since this is a new file it should be 2017.

Done.

https://codereview.chromium.org/2872853003/diff/180001/media/formats/BUILD.gn...
media/formats/BUILD.gn:7: source_set("formats") {
On 2017/05/11 17:36:15, jrummell wrote:
> Can you add "visibility = [ "//media/*" ]" so that nobody outside of //media
can
> use this directly.

Done.

https://codereview.chromium.org/2872853003/diff/180001/media/formats/BUILD.gn...
media/formats/BUILD.gn:85: if (enable_mse_mpeg2ts_stream_parser) {
On 2017/05/11 17:36:15, jrummell wrote:
> Can this be it's own block (if (proprietary_codecs &&
> enable_mse_mpeg2ts_stream_parser) ...)? I find it easier to understand when
> looking to see what conditions trigger a file getting compiled.
> 
> And then I guess (proprietary_codecs) should be the first block, so move the 2
> above it down to here as well.
> 
> if (proprietary_codecs) ...
> if (proprietary_codecs && enable_hevc_demuxing) ...
> if (proprietary_codecs && enable_dolby_vision_demuxing) ...
> if (proprietary_codecs && enable_mse_mpeg2ts_stream_parser) ...
> if (proprietary_codecs && enable_mse_mpeg2ts_stream_parser &&
> enable_hls_sample_aes) ...

Done.

[a.suchit2, 2017-05-12 05:54:51]

The CQ bit was checked by a.suchit@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-12 05:55:12]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-12 06:46:49]

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

[commit-bot: I haz the power, 2017-05-12 06:46:50]

Dry run: Try jobs failed on following builders:
  win_clang on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_clang/builds/...)

[a.suchit2, 2017-05-12 07:30:10]

The CQ bit was checked by a.suchit@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-12 07:30:31]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-12 08:19:49]

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

[commit-bot: I haz the power, 2017-05-12 08:19:50]

Dry run: Try jobs failed on following builders:
  win_clang on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_clang/builds/...)

[a.suchit2, 2017-05-12 08:57:42]

The CQ bit was checked by a.suchit@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-12 08:57:58]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-12 09:01:06]

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

[commit-bot: I haz the power, 2017-05-12 09:01:10]

Dry run: Try jobs failed on following builders:
  mac_optional_gpu_tests_rel on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_optional_gpu_...)

[a.suchit2, 2017-05-12 09:15:10]

The CQ bit was checked by a.suchit@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-12 09:15:22]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-12 10:02:01]

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

[commit-bot: I haz the power, 2017-05-12 10:02:02]

Dry run: Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[a.suchit2, 2017-05-12 11:29:34]

The CQ bit was checked by a.suchit@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-12 11:29:47]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[jrummell, 2017-05-13 00:30:40]

Looking good.

https://codereview.chromium.org/2872853003/diff/300001/media/BUILD.gn
File media/BUILD.gn (right):

https://codereview.chromium.org/2872853003/diff/300001/media/BUILD.gn#newcode532
media/BUILD.gn:532: "//media/formats",
Since you have this listed as a public_deps on :media and :media is referenced
above, this should not be needed.

Actually, since //media/formats is a source_set, it will link all the
media/format/ files with the test, which is what is probably causing the current
build problems.

[a.suchit2, 2017-05-13 02:18:12]

On 2017/05/13 00:30:40, jrummell wrote:
> Looking good.
> 
> https://codereview.chromium.org/2872853003/diff/300001/media/BUILD.gn
> File media/BUILD.gn (right):
> 
>
https://codereview.chromium.org/2872853003/diff/300001/media/BUILD.gn#newcode532
> media/BUILD.gn:532: "//media/formats",
> Since you have this listed as a public_deps on :media and :media is referenced
> above, this should not be needed.
> 
> Actually, since //media/formats is a source_set, it will link all the
> media/format/ files with the test, which is what is probably causing the
current
> build problems.

What I feel that all bots are happy accept windows bots and some dependencies
and
configs are already added for windows bots only and some more are reuiqred to
add.
So we should keep those dependencies under is_win condition.

[a.suchit2, 2017-05-15 05:38:44]

The CQ bit was checked by a.suchit@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-15 05:39:00]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-15 05:41:59]

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

[commit-bot: I haz the power, 2017-05-15 05:42:01]

Dry run: Try jobs failed on following builders:
  mac_optional_gpu_tests_rel on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_optional_gpu_...)

[a.suchit2, 2017-05-15 05:46:57]

The CQ bit was checked by a.suchit@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-15 05:47:07]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[a.suchit2, 2017-05-15 06:46:45]

The CQ bit was checked by a.suchit@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-15 06:47:04]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-15 06:51:23]

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

[commit-bot: I haz the power, 2017-05-15 06:51:24]

Dry run: Try jobs failed on following builders:
  android_clang_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_clan...)
  android_cronet on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_cron...)
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[a.suchit2, 2017-05-15 07:36:50]

The CQ bit was checked by a.suchit@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-15 07:37:11]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-15 08:37:53]

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

[commit-bot: I haz the power, 2017-05-15 08:37:54]

Dry run: Try jobs failed on following builders:
  win_chromium_compile_dbg_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...)

[a.suchit2, 2017-05-15 09:55:11]

The CQ bit was checked by a.suchit@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-15 09:55:23]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-15 11:41:26]

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

[commit-bot: I haz the power, 2017-05-15 11:41:27]

Dry run: Try jobs failed on following builders:
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[a.suchit2, 2017-05-16 00:16:41]

On 2017/05/13 02:18:12, a.suchit2 wrote:
> On 2017/05/13 00:30:40, jrummell wrote:
> > Looking good.
> > 
> > https://codereview.chromium.org/2872853003/diff/300001/media/BUILD.gn
> > File media/BUILD.gn (right):
> > 
> >
>
https://codereview.chromium.org/2872853003/diff/300001/media/BUILD.gn#newcode532
> > media/BUILD.gn:532: "//media/formats",
> > Since you have this listed as a public_deps on :media and :media is
referenced
> > above, this should not be needed.
> > 
> > Actually, since //media/formats is a source_set, it will link all the
> > media/format/ files with the test, which is what is probably causing the
> current
> > build problems.
> 
> What I feel that all bots are happy accept windows bots and some dependencies
> and
> configs are already added for windows bots only and some more are reuiqred to
> add.
> So we should keep those dependencies under is_win condition.

Please review It. Thanks

[a.suchit2, 2017-05-17 17:52:27]

On 2017/05/16 00:16:41, a.suchit2 wrote:
> On 2017/05/13 02:18:12, a.suchit2 wrote:
> > On 2017/05/13 00:30:40, jrummell wrote:
> > > Looking good.
> > > 
> > > https://codereview.chromium.org/2872853003/diff/300001/media/BUILD.gn
> > > File media/BUILD.gn (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/2872853003/diff/300001/media/BUILD.gn#newcode532
> > > media/BUILD.gn:532: "//media/formats",
> > > Since you have this listed as a public_deps on :media and :media is
> referenced
> > > above, this should not be needed.
> > > 
> > > Actually, since //media/formats is a source_set, it will link all the
> > > media/format/ files with the test, which is what is probably causing the
> > current
> > > build problems.
> > 
> > What I feel that all bots are happy accept windows bots and some
dependencies
> > and
> > configs are already added for windows bots only and some more are reuiqred
to
> > add.
> > So we should keep those dependencies under is_win condition.
> 
> Please review It. Thanks

@jrummell
Please review it. Thanks

[a.suchit2, 2017-05-18 00:36:15]

Please take a look for it. Patch is ready now. All bots are happy.
Thanks

https://codereview.chromium.org/2872853003/diff/180001/media/formats/BUILD.gn
File media/formats/BUILD.gn (right):

https://codereview.chromium.org/2872853003/diff/180001/media/formats/BUILD.gn...
media/formats/BUILD.gn:7: source_set("formats") {
On 2017/05/11 17:36:15, jrummell wrote:
> Based on the compile errors, you'll need something like:
>   configs += [
>     "//media:media_config",
>     "//media:media_implementation",
>   ]
> or maybe just:
>   configs += [ "//media/base:base_config" ]
> (as it includes the 2 above plus some others that may also be necessary).

Done.

https://codereview.chromium.org/2872853003/diff/180001/media/formats/BUILD.gn...
media/formats/BUILD.gn:40: "//skia",
On 2017/05/11 17:36:15, jrummell wrote:
> There should also be a dependency on //base, //media/base,
> //media:media_features, //media:shared_memory_support, and probably some more.
> However, this is where the dependency loops get challenging. You might have to
> leave some out until all the directories are done.

Done.

[a.suchit2, 2017-05-19 23:38:15]

On 2017/05/18 00:36:15, a.suchit2 wrote:
> Please take a look for it. Patch is ready now. All bots are happy.
> Thanks
> 
> https://codereview.chromium.org/2872853003/diff/180001/media/formats/BUILD.gn
> File media/formats/BUILD.gn (right):
> 
>
https://codereview.chromium.org/2872853003/diff/180001/media/formats/BUILD.gn...
> media/formats/BUILD.gn:7: source_set("formats") {
> On 2017/05/11 17:36:15, jrummell wrote:
> > Based on the compile errors, you'll need something like:
> >   configs += [
> >     "//media:media_config",
> >     "//media:media_implementation",
> >   ]
> > or maybe just:
> >   configs += [ "//media/base:base_config" ]
> > (as it includes the 2 above plus some others that may also be necessary).
> 
> Done.
> 
>
https://codereview.chromium.org/2872853003/diff/180001/media/formats/BUILD.gn...
> media/formats/BUILD.gn:40: "//skia",
> On 2017/05/11 17:36:15, jrummell wrote:
> > There should also be a dependency on //base, //media/base,
> > //media:media_features, //media:shared_memory_support, and probably some
more.
> > However, this is where the dependency loops get challenging. You might have
to
> > leave some out until all the directories are done.
> 
> Done.

Please review it. Thanks

[a.suchit2, 2017-05-23 03:48:53]

The CQ bit was checked by a.suchit@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-23 03:49:10]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-23 05:46:04]

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

[commit-bot: I haz the power, 2017-05-23 05:46:05]

Dry run: This issue passed the CQ dry run.

[DaleCurtis, 2017-05-23 23:54:49]

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

[DaleCurtis, 2017-05-23 23:54:51]

a.suchit@ sorry for the delay, sometimes reviews with lots of OWNERS can get
lost. this lgtm

[a.suchit2, 2017-05-24 02:13:12]

The CQ bit was checked by a.suchit@chromium.org

[commit-bot: I haz the power, 2017-05-24 02:14:10]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[a.suchit2, 2017-05-24 02:16:56]

On 2017/05/23 23:54:51, DaleCurtis wrote:
> a.suchit@ sorry for the delay, sometimes reviews with lots of OWNERS can get
> lost. this lgtm

Thanks

[commit-bot: I haz the power, 2017-05-24 02:27:54]

CQ is committing da patch.

Bot data: {"patchset_id": 410001, "attempt_start_ts": 1495591992771110,
"parent_rev": "480e803afdf8b9a1b0623b8af3575c244ddd6f3b", "commit_rev":
"1cf16502586c8e7c6ba9e9293bda196c04ac4b0e"}

[commit-bot: I haz the power, 2017-05-24 02:28:06]

Description was changed from

==========
Create BUILD.gn files for //media/formats.

Create new BUILD.gn files for media/formats and moved respective
media component changes from media/BUILD.gn file to new BUILD.gn
files.

BUG=613033
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

to

==========
Create BUILD.gn files for //media/formats.

Create new BUILD.gn files for media/formats and moved respective
media component changes from media/BUILD.gn file to new BUILD.gn
files.

BUG=613033
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel

Review-Url: https://codereview.chromium.org/2872853003
Cr-Commit-Position: refs/heads/master@{#474130}
Committed:
https://chromium.googlesource.com/chromium/src/+/1cf16502586c8e7c6ba9e9293bda...
==========

[commit-bot: I haz the power, 2017-05-24 02:28:08]

Committed patchset #22 (id:410001) as
https://chromium.googlesource.com/chromium/src/+/1cf16502586c8e7c6ba9e9293bda...