G722 implementation of the AudioEncoderFactoryTemplate API

G722 implementation of the AudioEncoderFactoryTemplate API

Now the templated AudioEncoderFactory can create G722 encoders!

BUG=webrtc:7833
Review-Url: https://codereview.webrtc.org/2934833002
Cr-Commit-Position: refs/heads/master@{#18644}
Committed: https://chromium.googlesource.com/external/webrtc/+/b8727aebc161e94b818dd1323895fcd674990ad7

[kwiberg-webrtc, 2017-06-12 15:28:58]

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

[commit-bot: I haz the power, 2017-06-12 15:29:08]

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-06-12 15:31:42]

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

[commit-bot: I haz the power, 2017-06-12 15:31:43]

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/21056)
  ios_dbg on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/ios_dbg/builds/26376)

[kwiberg-webrtc, 2017-06-12 16:41:53]

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

[commit-bot: I haz the power, 2017-06-12 16:41:59]

Dry run: CQ is trying da patch.

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

[kwiberg-webrtc, 2017-06-12 16:42:22]

Patchset #1 (id:1) has been deleted

[commit-bot: I haz the power, 2017-06-12 16:50:56]

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

[commit-bot: I haz the power, 2017-06-12 16:50:57]

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

[kwiberg-webrtc, 2017-06-12 17:05:56]

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

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

Dry run: CQ is trying da patch.

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

[kwiberg-webrtc, 2017-06-12 17:06:43]

Patchset #1 (id:20001) has been deleted

[commit-bot: I haz the power, 2017-06-12 17:28:49]

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

[commit-bot: I haz the power, 2017-06-12 17:28:49]

Dry run: This issue passed the CQ dry run.

[kwiberg-webrtc, 2017-06-12 17:29:36]

kwiberg@webrtc.org changed reviewers:
+ ossu@webrtc.org, solenberg@webrtc.org

[the sun, 2017-06-13 09:53:43]

nice!

https://codereview.webrtc.org/2934833002/diff/40001/webrtc/api/audio_codecs/a...
File webrtc/api/audio_codecs/audio_encoder_g722.h (right):

https://codereview.webrtc.org/2934833002/diff/40001/webrtc/api/audio_codecs/a...
webrtc/api/audio_codecs/audio_encoder_g722.h:28: struct AudioEncoderG722Api {
"Api" is a non-standard suffix. I guess it is to disambiguate from the impl?

Could we change the implementation to use the Impl suffix and drop Api here?

https://codereview.webrtc.org/2934833002/diff/40001/webrtc/api/audio_codecs/a...
File webrtc/api/audio_codecs/audio_encoder_g722_config.h (right):

https://codereview.webrtc.org/2934833002/diff/40001/webrtc/api/audio_codecs/a...
webrtc/api/audio_codecs/audio_encoder_g722_config.h:14: #include <stddef.h>
why?

[kwiberg-webrtc, 2017-06-13 12:13:25]

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

[commit-bot: I haz the power, 2017-06-13 12:13:32]

Dry run: CQ is trying da patch.

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

[kwiberg-webrtc, 2017-06-13 12:13:37]

Patchset #2 (id:60001) has been deleted

[kwiberg-webrtc, 2017-06-13 12:15:03]

New patch set. Moved the g722 files in api to a subdirectory, and renamed
AudioEncoderG722 to AudioEncoderG722Impl and AudioEncoderG722Api to
AudioEncoderG722. Also fixed the unnecessary #include.

https://codereview.webrtc.org/2934833002/diff/40001/webrtc/api/audio_codecs/a...
File webrtc/api/audio_codecs/audio_encoder_g722.h (right):

https://codereview.webrtc.org/2934833002/diff/40001/webrtc/api/audio_codecs/a...
webrtc/api/audio_codecs/audio_encoder_g722.h:28: struct AudioEncoderG722Api {
On 2017/06/13 09:53:43, the sun wrote:
> "Api" is a non-standard suffix. I guess it is to disambiguate from the impl?
> 
> Could we change the implementation to use the Impl suffix and drop Api here?

Yes, I guess we could. I'll do it!

https://codereview.webrtc.org/2934833002/diff/40001/webrtc/api/audio_codecs/a...
File webrtc/api/audio_codecs/audio_encoder_g722_config.h (right):

https://codereview.webrtc.org/2934833002/diff/40001/webrtc/api/audio_codecs/a...
webrtc/api/audio_codecs/audio_encoder_g722_config.h:14: #include <stddef.h>
On 2017/06/13 09:53:43, the sun wrote:
> why?

Because num_channels was a size_t. But I changed it to an int, so we don't need
this anymore.

[commit-bot: I haz the power, 2017-06-13 12:16:02]

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

[commit-bot: I haz the power, 2017-06-13 12:16:02]

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

[kwiberg-webrtc, 2017-06-13 12:24:30]

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

[commit-bot: I haz the power, 2017-06-13 12:24:42]

Dry run: CQ is trying da patch.

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

[kwiberg-webrtc, 2017-06-13 12:24:59]

Patchset #3 (id:100001) has been deleted

[commit-bot: I haz the power, 2017-06-13 12:54:27]

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

[commit-bot: I haz the power, 2017-06-13 12:54:28]

Dry run: This issue passed the CQ dry run.

[the sun, 2017-06-13 14:49:30]

lgtm

https://codereview.webrtc.org/2934833002/diff/120001/webrtc/api/audio_codecs/...
File webrtc/api/audio_codecs/test/audio_encoder_factory_template_unittest.cc
(right):

https://codereview.webrtc.org/2934833002/diff/120001/webrtc/api/audio_codecs/...
webrtc/api/audio_codecs/test/audio_encoder_factory_template_unittest.cc:123:
TEST(AudioEncoderFactoryTemplateTest, G722) {
I guess the idea is to eventually move this test case to the unit test for
BuiltInEncoderFactory?

[kwiberg-webrtc, 2017-06-13 19:12:10]

https://codereview.webrtc.org/2934833002/diff/120001/webrtc/api/audio_codecs/...
File webrtc/api/audio_codecs/test/audio_encoder_factory_template_unittest.cc
(right):

https://codereview.webrtc.org/2934833002/diff/120001/webrtc/api/audio_codecs/...
webrtc/api/audio_codecs/test/audio_encoder_factory_template_unittest.cc:123:
TEST(AudioEncoderFactoryTemplateTest, G722) {
On 2017/06/13 14:49:30, the sun wrote:
> I guess the idea is to eventually move this test case to the unit test for
> BuiltInEncoderFactory?

I hadn't in fact thought that far ahead, but yes, that sounds like a good idea.

[kwiberg-webrtc, 2017-06-16 09:29:12]

Description was changed from

==========
G722 implementation of the AudioEncoderFactoryTemplate API

Now the templated AudioEncoderFactory can create G722 encoders!

BUG=webrtc:5806
==========

to

==========
G722 implementation of the AudioEncoderFactoryTemplate API

Now the templated AudioEncoderFactory can create G722 encoders!

BUG=webrtc:7833
==========

[ossu, 2017-06-16 10:01:45]

Sweet! LGTM.

[the sun, 2017-06-16 17:52:28]

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

[commit-bot: I haz the power, 2017-06-16 17:52:32]

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-06-16 18:18:42]

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

[commit-bot: I haz the power, 2017-06-16 18:18:43]

Dry run: This issue passed the CQ dry run.

[kwiberg-webrtc, 2017-06-18 00:13:48]

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

[commit-bot: I haz the power, 2017-06-18 00:14:00]

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-06-18 00:35:27]

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

[commit-bot: I haz the power, 2017-06-18 00:35:28]

Dry run: This issue passed the CQ dry run.

[kwiberg-webrtc, 2017-06-18 00:39:47]

The CQ bit was checked by kwiberg@webrtc.org

[kwiberg-webrtc, 2017-06-18 00:39:48]

The patchset sent to the CQ was uploaded after l-g-t-m from solenberg@webrtc.org
Link to the patchset: https://codereview.webrtc.org/2934833002/#ps140001 (title:
"rebase")

[commit-bot: I haz the power, 2017-06-18 00:39:57]

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-06-18 00:42:00]

CQ is committing da patch.

Bot data: {"patchset_id": 140001, "attempt_start_ts": 1497746387780800,
"parent_rev": "b1ed7f09c0ed3dfdac88d4b5d4c28629c373ac5f", "commit_rev":
"b8727aebc161e94b818dd1323895fcd674990ad7"}

[commit-bot: I haz the power, 2017-06-18 00:42:04]

Description was changed from

==========
G722 implementation of the AudioEncoderFactoryTemplate API

Now the templated AudioEncoderFactory can create G722 encoders!

BUG=webrtc:7833
==========

to

==========
G722 implementation of the AudioEncoderFactoryTemplate API

Now the templated AudioEncoderFactory can create G722 encoders!

BUG=webrtc:7833

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

[commit-bot: I haz the power, 2017-06-18 00:42:07]

Committed patchset #4 (id:140001) as
https://chromium.googlesource.com/external/webrtc/+/b8727aebc161e94b818dd1323...