iSAC: Untangle some cyclic dependencies

iSAC: Untangle some cyclic dependencies

BUG=webrtc:6828, webrtc:7042
Review-Url: https://codereview.webrtc.org/2656823005
Cr-Commit-Position: refs/heads/master@{#16358}
Committed: https://chromium.googlesource.com/external/webrtc/+/a6ca518dd5767fbd7fa3af53a4ec91da401e1f48

[kwiberg-webrtc, 2017-01-26 00:32:12]

The CQ bit was checked by kwiberg@webrtc.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-26 00:32:19]

Dry run: 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, 2017-01-26 00:34:08]

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

[commit-bot: I haz the power, 2017-01-26 00:34:09]

Dry run: Try jobs failed on following builders:
  ios64_sim_ios10_dbg on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/ios64_sim_ios10_dbg/bui...)
  ios_arm64_dbg on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/ios_arm64_dbg/builds/16840)
  ios_dbg on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/ios_dbg/builds/22099)
  mac_asan on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/mac_asan/builds/21315)
  mac_baremetal on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/mac_baremetal/builds/18130)

[kwiberg-webrtc, 2017-01-26 00:34:47]

Description was changed from

==========
iSAC: Untangle some cyclic dependencies

BUG=webrtc:7042
==========

to

==========
iSAC: Untangle some cyclic dependencies

BUG=webrtc:6828, webrtc:7042
==========

[kwiberg-webrtc, 2017-01-26 00:39:48]

The CQ bit was checked by kwiberg@webrtc.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-26 00:39:59]

Dry run: 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, 2017-01-26 01:46:59]

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

[commit-bot: I haz the power, 2017-01-26 01:46:59]

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

[kwiberg-webrtc, 2017-01-26 12:30:30]

kwiberg@webrtc.org changed reviewers:
+ henrik.lundin@webrtc.org, kjellander@webrtc.org

[kwiberg-webrtc, 2017-01-26 12:30:47]

Patchset #1 (id:1) has been deleted

[kjellander_webrtc, 2017-01-26 15:22:00]

lgtm with a question.

https://codereview.webrtc.org/2656823005/diff/20001/webrtc/modules/audio_codi...
File webrtc/modules/audio_coding/codecs/builtin_audio_decoder_factory.cc
(right):

https://codereview.webrtc.org/2656823005/diff/20001/webrtc/modules/audio_codi...
webrtc/modules/audio_coding/codecs/builtin_audio_decoder_factory.cc:27: #include
"webrtc/modules/audio_coding/codecs/isac/fix/include/audio_decoder_isacfix.h" 
// nogncheck
This is not possible to solve, or you're punting it to a future CL? If so, can
you add a reference to the bug here?

[hlundin-webrtc, 2017-01-30 08:46:19]

RS-LGTM

[kwiberg-webrtc, 2017-01-30 09:44:46]

https://codereview.webrtc.org/2656823005/diff/20001/webrtc/modules/audio_codi...
File webrtc/modules/audio_coding/codecs/builtin_audio_decoder_factory.cc
(right):

https://codereview.webrtc.org/2656823005/diff/20001/webrtc/modules/audio_codi...
webrtc/modules/audio_coding/codecs/builtin_audio_decoder_factory.cc:27: #include
"webrtc/modules/audio_coding/codecs/isac/fix/include/audio_decoder_isacfix.h" 
// nogncheck
On 2017/01/26 15:22:00, kjellander_webrtc wrote:
> This is not possible to solve, or you're punting it to a future CL? If so, can
> you add a reference to the bug here?

This is how the gn docs recommend that you handle conditional #includes, since
gn doesn't actually understand the C preprocessor and always pretends that all
#include lines are in effect. So not possible to solve with current gn, and
"standard" enough that I'd argue a comment may not be needed. (Feel free to
disagree; I can add a comment if you still think one is called for.)

[kjellander_webrtc, 2017-01-30 10:45:34]

https://codereview.webrtc.org/2656823005/diff/20001/webrtc/modules/audio_codi...
File webrtc/modules/audio_coding/codecs/builtin_audio_decoder_factory.cc
(right):

https://codereview.webrtc.org/2656823005/diff/20001/webrtc/modules/audio_codi...
webrtc/modules/audio_coding/codecs/builtin_audio_decoder_factory.cc:27: #include
"webrtc/modules/audio_coding/codecs/isac/fix/include/audio_decoder_isacfix.h" 
// nogncheck
On 2017/01/30 09:44:45, kwiberg-webrtc wrote:
> On 2017/01/26 15:22:00, kjellander_webrtc wrote:
> > This is not possible to solve, or you're punting it to a future CL? If so,
can
> > you add a reference to the bug here?
> 
> This is how the gn docs recommend that you handle conditional #includes, since
> gn doesn't actually understand the C preprocessor and always pretends that all
> #include lines are in effect. So not possible to solve with current gn, and
> "standard" enough that I'd argue a comment may not be needed. (Feel free to
> disagree; I can add a comment if you still think one is called for.)

I hadn't seen it this before and I didn't know the check had that limitation.
Then this is of course fine. I wonder if we can document this somewhere
though... I'll make a comment in the GN check bug..

[kwiberg-webrtc, 2017-01-30 12:48:44]

The CQ bit was checked by kwiberg@webrtc.org

[commit-bot: I haz the power, 2017-01-30 12:48:55]

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, 2017-01-30 13:28:55]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1485780524133510,
"parent_rev": "4fb97462a8443fd0879953f0b30d59811828f851", "commit_rev":
"a6ca518dd5767fbd7fa3af53a4ec91da401e1f48"}

[commit-bot: I haz the power, 2017-01-30 13:28:57]

Description was changed from

==========
iSAC: Untangle some cyclic dependencies

BUG=webrtc:6828, webrtc:7042
==========

to

==========
iSAC: Untangle some cyclic dependencies

BUG=webrtc:6828, webrtc:7042

Review-Url: https://codereview.webrtc.org/2656823005
Cr-Commit-Position: refs/heads/master@{#16358}
Committed:
https://chromium.googlesource.com/external/webrtc/+/a6ca518dd5767fbd7fa3af53a...
==========

[commit-bot: I haz the power, 2017-01-30 13:28:59]

Committed patchset #1 (id:20001) as
https://chromium.googlesource.com/external/webrtc/+/a6ca518dd5767fbd7fa3af53a...