Add audio_format_conversion to deps for audio_decoder_factory_interface.

Add audio_format_conversion to deps for audio_decoder_factory_interface.

This fix is made to remove the discrepancy between GYP and GN audio_decoder_factory_interface target.

BUG=webrtc:6412
NOTRY=True

Committed: https://crrev.com/1515e95329a96aa9d6e8cbc073cf4072959e6ac1
Cr-Commit-Position: refs/heads/master@{#14894}

[charujain, 2016-11-02 09:15:34]

Description was changed from

==========
Added audio_format_conversion


BUG=
==========

to

==========
Added audio_format_conversion in dependency of audio_decoder_factory_interface.

This fix is made to remove the discrepancy between GYP and GN
audio_decoder_factory_interface target.

BUG=webrtc:6440
==========

[charujain, 2016-11-02 09:15:34]

charujain@webrtc.org changed reviewers:
+ kjellander@webrtc.org

[kjellander_webrtc, 2016-11-02 09:22:48]

kjellander@webrtc.org changed reviewers:
+ kwiberg@webrtc.org

[kjellander_webrtc, 2016-11-02 09:22:48]

lgtm

Let's wait for kwiberg as well.

[kjellander_webrtc, 2016-11-02 09:32:27]

Description was changed from

==========
Added audio_format_conversion in dependency of audio_decoder_factory_interface.

This fix is made to remove the discrepancy between GYP and GN
audio_decoder_factory_interface target.

BUG=webrtc:6440
==========

to

==========
Add audio_format_conversion to deps for audio_decoder_factory_interface.

This fix is made to remove the discrepancy between GYP and GN
audio_decoder_factory_interface target.

BUG=webrtc:6440
==========

[kjellander_webrtc, 2016-11-02 09:32:47]

On 2016/11/02 09:22:48, kjellander_webrtc wrote:
> lgtm
> 
> Let's wait for kwiberg as well.

Run a few compile trybots and use NOTRY=True for landing.

[kwiberg-webrtc, 2016-11-02 10:00:42]

https://codereview.webrtc.org/2472643003/diff/1/webrtc/modules/audio_coding/B...
File webrtc/modules/audio_coding/BUILD.gn (right):

https://codereview.webrtc.org/2472643003/diff/1/webrtc/modules/audio_coding/B...
webrtc/modules/audio_coding/BUILD.gn:71: ":audio_format_conversion",
Thanks, but this doesn't look right. audio_decoder_factory.h, the only source
file in this build target, doesn't #include audio_format_conversion.h. It's the
build targets that contain the following files that ought to depend on
audio_format_conversion:

$ git grep -nH -l -e '#include.*audio_format_conversion.h'
webrtc/modules/audio_coding/acm2/acm_receive_test.cc
webrtc/modules/audio_coding/acm2/audio_coding_module_unittest.cc
webrtc/modules/audio_coding/codecs/audio_format_conversion.cc
webrtc/modules/audio_coding/test/APITest.cc
webrtc/modules/audio_coding/test/EncodeDecodeTest.cc
webrtc/modules/audio_coding/test/TestAllCodecs.cc
webrtc/modules/audio_coding/test/TestRedFec.cc
webrtc/modules/audio_coding/test/TestStereo.cc
webrtc/modules/audio_coding/test/TestVADDTX.cc
webrtc/modules/audio_coding/test/TwoWayCommunication.cc
webrtc/modules/audio_coding/test/delay_test.cc
webrtc/modules/audio_coding/test/iSACTest.cc
webrtc/modules/audio_coding/test/insert_packet_with_timing.cc
webrtc/modules/audio_coding/test/opus_test.cc
webrtc/modules/utility/source/coder.cc
webrtc/voice_engine/channel.cc

Or am I misunderstanding something?

[kjellander_webrtc, 2016-11-02 10:15:08]

Karl, let me know which of the two options I present you prefer.

https://codereview.webrtc.org/2472643003/diff/1/webrtc/modules/audio_coding/B...
File webrtc/modules/audio_coding/BUILD.gn (right):

https://codereview.webrtc.org/2472643003/diff/1/webrtc/modules/audio_coding/B...
webrtc/modules/audio_coding/BUILD.gn:71: ":audio_format_conversion",
On 2016/11/02 10:00:42, kwiberg-webrtc wrote:
> Thanks, but this doesn't look right. audio_decoder_factory.h, the only source
> file in this build target, doesn't #include audio_format_conversion.h. It's
the
> build targets that contain the following files that ought to depend on
> audio_format_conversion:
> 
> $ git grep -nH -l -e '#include.*audio_format_conversion.h'
> webrtc/modules/audio_coding/acm2/acm_receive_test.cc
> webrtc/modules/audio_coding/acm2/audio_coding_module_unittest.cc
> webrtc/modules/audio_coding/codecs/audio_format_conversion.cc
> webrtc/modules/audio_coding/test/APITest.cc
> webrtc/modules/audio_coding/test/EncodeDecodeTest.cc
> webrtc/modules/audio_coding/test/TestAllCodecs.cc
> webrtc/modules/audio_coding/test/TestRedFec.cc
> webrtc/modules/audio_coding/test/TestStereo.cc
> webrtc/modules/audio_coding/test/TestVADDTX.cc
> webrtc/modules/audio_coding/test/TwoWayCommunication.cc
> webrtc/modules/audio_coding/test/delay_test.cc
> webrtc/modules/audio_coding/test/iSACTest.cc
> webrtc/modules/audio_coding/test/insert_packet_with_timing.cc
> webrtc/modules/audio_coding/test/opus_test.cc
> webrtc/modules/utility/source/coder.cc
> webrtc/voice_engine/channel.cc
> 
> Or am I misunderstanding something?

You're right. But as there are now downstream projects depending on
audio_decoder_factory_interface target and its audio_format_conversion.cc
implementation, this would be one way to solve the GYP->GN switch quickly. I
think this is fine if we add a TODO to clean this up when downstream projects
are updated to properly depend on the audio_format_conversion target.

A better, but slower, way to solve this would be to:
1. Create the audio_format_conversion target in GYP
2. Update downstream projects to depend on that target.

[kwiberg-webrtc, 2016-11-02 10:23:14]

lgtm, provided you add a TODO comment that the dependency should be removed and
added to the right places instead.

https://codereview.webrtc.org/2472643003/diff/1/webrtc/modules/audio_coding/B...
File webrtc/modules/audio_coding/BUILD.gn (right):

https://codereview.webrtc.org/2472643003/diff/1/webrtc/modules/audio_coding/B...
webrtc/modules/audio_coding/BUILD.gn:71: ":audio_format_conversion",
On 2016/11/02 10:15:08, kjellander_webrtc wrote:
> On 2016/11/02 10:00:42, kwiberg-webrtc wrote:
> > Thanks, but this doesn't look right. audio_decoder_factory.h, the only
source
> > file in this build target, doesn't #include audio_format_conversion.h. It's
> the
> > build targets that contain the following files that ought to depend on
> > audio_format_conversion:
> > 
> > $ git grep -nH -l -e '#include.*audio_format_conversion.h'
> > webrtc/modules/audio_coding/acm2/acm_receive_test.cc
> > webrtc/modules/audio_coding/acm2/audio_coding_module_unittest.cc
> > webrtc/modules/audio_coding/codecs/audio_format_conversion.cc
> > webrtc/modules/audio_coding/test/APITest.cc
> > webrtc/modules/audio_coding/test/EncodeDecodeTest.cc
> > webrtc/modules/audio_coding/test/TestAllCodecs.cc
> > webrtc/modules/audio_coding/test/TestRedFec.cc
> > webrtc/modules/audio_coding/test/TestStereo.cc
> > webrtc/modules/audio_coding/test/TestVADDTX.cc
> > webrtc/modules/audio_coding/test/TwoWayCommunication.cc
> > webrtc/modules/audio_coding/test/delay_test.cc
> > webrtc/modules/audio_coding/test/iSACTest.cc
> > webrtc/modules/audio_coding/test/insert_packet_with_timing.cc
> > webrtc/modules/audio_coding/test/opus_test.cc
> > webrtc/modules/utility/source/coder.cc
> > webrtc/voice_engine/channel.cc
> > 
> > Or am I misunderstanding something?
> 
> You're right. But as there are now downstream projects depending on
> audio_decoder_factory_interface target and its audio_format_conversion.cc
> implementation, this would be one way to solve the GYP->GN switch quickly. I
> think this is fine if we add a TODO to clean this up when downstream projects
> are updated to properly depend on the audio_format_conversion target.
> 
> A better, but slower, way to solve this would be to:
> 1. Create the audio_format_conversion target in GYP
> 2. Update downstream projects to depend on that target.

Right. I'm not usually an "end justifies the means" kind of person, but for the
GYP->GN transition I can make an exception... It'll be less work in total to do
this quick fix to unblock the transition, and then clean up afterwards, is what
you're saying?

[kjellander_webrtc, 2016-11-02 10:29:17]

On 2016/11/02 10:23:14, kwiberg-webrtc wrote:
> lgtm, provided you add a TODO comment that the dependency should be removed
and
> added to the right places instead.
> 
>
https://codereview.webrtc.org/2472643003/diff/1/webrtc/modules/audio_coding/B...
> File webrtc/modules/audio_coding/BUILD.gn (right):
> 
>
https://codereview.webrtc.org/2472643003/diff/1/webrtc/modules/audio_coding/B...
> webrtc/modules/audio_coding/BUILD.gn:71: ":audio_format_conversion",
> On 2016/11/02 10:15:08, kjellander_webrtc wrote:
> > On 2016/11/02 10:00:42, kwiberg-webrtc wrote:
> > > Thanks, but this doesn't look right. audio_decoder_factory.h, the only
> source
> > > file in this build target, doesn't #include audio_format_conversion.h.
It's
> > the
> > > build targets that contain the following files that ought to depend on
> > > audio_format_conversion:
> > > 
> > > $ git grep -nH -l -e '#include.*audio_format_conversion.h'
> > > webrtc/modules/audio_coding/acm2/acm_receive_test.cc
> > > webrtc/modules/audio_coding/acm2/audio_coding_module_unittest.cc
> > > webrtc/modules/audio_coding/codecs/audio_format_conversion.cc
> > > webrtc/modules/audio_coding/test/APITest.cc
> > > webrtc/modules/audio_coding/test/EncodeDecodeTest.cc
> > > webrtc/modules/audio_coding/test/TestAllCodecs.cc
> > > webrtc/modules/audio_coding/test/TestRedFec.cc
> > > webrtc/modules/audio_coding/test/TestStereo.cc
> > > webrtc/modules/audio_coding/test/TestVADDTX.cc
> > > webrtc/modules/audio_coding/test/TwoWayCommunication.cc
> > > webrtc/modules/audio_coding/test/delay_test.cc
> > > webrtc/modules/audio_coding/test/iSACTest.cc
> > > webrtc/modules/audio_coding/test/insert_packet_with_timing.cc
> > > webrtc/modules/audio_coding/test/opus_test.cc
> > > webrtc/modules/utility/source/coder.cc
> > > webrtc/voice_engine/channel.cc
> > > 
> > > Or am I misunderstanding something?
> > 
> > You're right. But as there are now downstream projects depending on
> > audio_decoder_factory_interface target and its audio_format_conversion.cc
> > implementation, this would be one way to solve the GYP->GN switch quickly. I
> > think this is fine if we add a TODO to clean this up when downstream
projects
> > are updated to properly depend on the audio_format_conversion target.
> > 
> > A better, but slower, way to solve this would be to:
> > 1. Create the audio_format_conversion target in GYP
> > 2. Update downstream projects to depend on that target.
> 
> Right. I'm not usually an "end justifies the means" kind of person, but for
the
> GYP->GN transition I can make an exception... It'll be less work in total to
do
> this quick fix to unblock the transition, and then clean up afterwards, is
what
> you're saying?

Yes, but this fix should be tested downstream before submitted, since I'm not
entirely sure it'll solve the problem.
Charu: make sure we don't forget to do the cleanup after switching.

[kjellander_webrtc, 2016-11-02 10:31:09]

On 2016/11/02 10:29:17, kjellander_webrtc wrote:
> On 2016/11/02 10:23:14, kwiberg-webrtc wrote:
> > lgtm, provided you add a TODO comment that the dependency should be removed
> and
> > added to the right places instead.
> > 
> >
>
https://codereview.webrtc.org/2472643003/diff/1/webrtc/modules/audio_coding/B...
> > File webrtc/modules/audio_coding/BUILD.gn (right):
> > 
> >
>
https://codereview.webrtc.org/2472643003/diff/1/webrtc/modules/audio_coding/B...
> > webrtc/modules/audio_coding/BUILD.gn:71: ":audio_format_conversion",
> > On 2016/11/02 10:15:08, kjellander_webrtc wrote:
> > > On 2016/11/02 10:00:42, kwiberg-webrtc wrote:
> > > > Thanks, but this doesn't look right. audio_decoder_factory.h, the only
> > source
> > > > file in this build target, doesn't #include audio_format_conversion.h.
> It's
> > > the
> > > > build targets that contain the following files that ought to depend on
> > > > audio_format_conversion:
> > > > 
> > > > $ git grep -nH -l -e '#include.*audio_format_conversion.h'
> > > > webrtc/modules/audio_coding/acm2/acm_receive_test.cc
> > > > webrtc/modules/audio_coding/acm2/audio_coding_module_unittest.cc
> > > > webrtc/modules/audio_coding/codecs/audio_format_conversion.cc
> > > > webrtc/modules/audio_coding/test/APITest.cc
> > > > webrtc/modules/audio_coding/test/EncodeDecodeTest.cc
> > > > webrtc/modules/audio_coding/test/TestAllCodecs.cc
> > > > webrtc/modules/audio_coding/test/TestRedFec.cc
> > > > webrtc/modules/audio_coding/test/TestStereo.cc
> > > > webrtc/modules/audio_coding/test/TestVADDTX.cc
> > > > webrtc/modules/audio_coding/test/TwoWayCommunication.cc
> > > > webrtc/modules/audio_coding/test/delay_test.cc
> > > > webrtc/modules/audio_coding/test/iSACTest.cc
> > > > webrtc/modules/audio_coding/test/insert_packet_with_timing.cc
> > > > webrtc/modules/audio_coding/test/opus_test.cc
> > > > webrtc/modules/utility/source/coder.cc
> > > > webrtc/voice_engine/channel.cc
> > > > 
> > > > Or am I misunderstanding something?
> > > 
> > > You're right. But as there are now downstream projects depending on
> > > audio_decoder_factory_interface target and its audio_format_conversion.cc
> > > implementation, this would be one way to solve the GYP->GN switch quickly.
I
> > > think this is fine if we add a TODO to clean this up when downstream
> projects
> > > are updated to properly depend on the audio_format_conversion target.
> > > 
> > > A better, but slower, way to solve this would be to:
> > > 1. Create the audio_format_conversion target in GYP
> > > 2. Update downstream projects to depend on that target.
> > 
> > Right. I'm not usually an "end justifies the means" kind of person, but for
> the
> > GYP->GN transition I can make an exception... It'll be less work in total to
> do
> > this quick fix to unblock the transition, and then clean up afterwards, is
> what
> > you're saying?
> 
> Yes, but this fix should be tested downstream before submitted, since I'm not
> entirely sure it'll solve the problem.
> Charu: make sure we don't forget to do the cleanup after switching.

Also, 6440 seems like the wrong bug for this. Is 6412 a better choice?

[charujain, 2016-11-02 10:37:09]

Description was changed from

==========
Add audio_format_conversion to deps for audio_decoder_factory_interface.

This fix is made to remove the discrepancy between GYP and GN
audio_decoder_factory_interface target.

BUG=webrtc:6440
==========

to

==========
Add audio_format_conversion to deps for audio_decoder_factory_interface.

This fix is made to remove the discrepancy between GYP and GN
audio_decoder_factory_interface target.

BUG=webrtc:6412
==========

[charujain, 2016-11-02 15:41:46]

Description was changed from

==========
Add audio_format_conversion to deps for audio_decoder_factory_interface.

This fix is made to remove the discrepancy between GYP and GN
audio_decoder_factory_interface target.

BUG=webrtc:6412
==========

to

==========
Add audio_format_conversion to deps for audio_decoder_factory_interface.

This fix is made to remove the discrepancy between GYP and GN
audio_decoder_factory_interface target.

BUG=webrtc:6412
NOTRY=True
==========

[charujain, 2016-11-02 15:41:58]

The CQ bit was checked by charujain@webrtc.org

[charujain, 2016-11-02 15:41:58]

The patchset sent to the CQ was uploaded after l-g-t-m from
kjellander@webrtc.org, kwiberg@webrtc.org
Link to the patchset: https://codereview.webrtc.org/2472643003/#ps40001 (title:
"Spell fix")

[commit-bot: I haz the power, 2016-11-02 15:42:00]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[commit-bot: I haz the power, 2016-11-02 15:43:36]

Description was changed from

==========
Add audio_format_conversion to deps for audio_decoder_factory_interface.

This fix is made to remove the discrepancy between GYP and GN
audio_decoder_factory_interface target.

BUG=webrtc:6412
NOTRY=True
==========

to

==========
Add audio_format_conversion to deps for audio_decoder_factory_interface.

This fix is made to remove the discrepancy between GYP and GN
audio_decoder_factory_interface target.

BUG=webrtc:6412
NOTRY=True
==========

[commit-bot: I haz the power, 2016-11-02 15:43:37]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-11-02 15:50:02]

Description was changed from

==========
Add audio_format_conversion to deps for audio_decoder_factory_interface.

This fix is made to remove the discrepancy between GYP and GN
audio_decoder_factory_interface target.

BUG=webrtc:6412
NOTRY=True
==========

to

==========
Add audio_format_conversion to deps for audio_decoder_factory_interface.

This fix is made to remove the discrepancy between GYP and GN
audio_decoder_factory_interface target.

BUG=webrtc:6412
NOTRY=True

Committed: https://crrev.com/1515e95329a96aa9d6e8cbc073cf4072959e6ac1
Cr-Commit-Position: refs/heads/master@{#14894}
==========

[commit-bot: I haz the power, 2016-11-02 15:50:03]

Patchset 3 (id:??) landed as
https://crrev.com/1515e95329a96aa9d6e8cbc073cf4072959e6ac1
Cr-Commit-Position: refs/heads/master@{#14894}