L16 implementation of the Audio{En,De}coderFactoryTemplate APIs

L16 implementation of the Audio{En,De}coderFactoryTemplate APIs

BUG=webrtc:7836, webrtc:7842
Review-Url: https://codereview.webrtc.org/2995523002
Cr-Commit-Position: refs/heads/master@{#19354}
Committed: https://chromium.googlesource.com/external/webrtc/+/edff94df62d5cc0563430ed8d5e2ba8c9f88f7dd

[kwiberg-webrtc, 2017-08-04 14:24:09]

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

[commit-bot: I haz the power, 2017-08-04 14:24:21]

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-08-04 14:55:48]

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

[commit-bot: I haz the power, 2017-08-04 14:55:49]

Dry run: This issue passed the CQ dry run.

[kwiberg-webrtc, 2017-08-04 17:16:41]

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

[kwiberg-webrtc, 2017-08-10 12:37:51]

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

[commit-bot: I haz the power, 2017-08-10 12:38:01]

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-08-10 13:13:15]

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

[commit-bot: I haz the power, 2017-08-10 13:13:16]

Dry run: This issue passed the CQ dry run.

[ossu, 2017-08-14 15:54:33]

https://codereview.webrtc.org/2995523002/diff/20001/webrtc/api/audio_codecs/L...
File webrtc/api/audio_codecs/L16/audio_decoder_L16.h (right):

https://codereview.webrtc.org/2995523002/diff/20001/webrtc/api/audio_codecs/L...
webrtc/api/audio_codecs/L16/audio_decoder_L16.h:32: (num_channels == 1 ||
num_channels == 2);
AudioDecoderPcm16B only enforces num_channels >= 1. There seems to be at least
some cursory support for multi-channel audio (at least for four channels), so
perhaps let this "decoder" support any positive number of channels?

https://codereview.webrtc.org/2995523002/diff/20001/webrtc/api/audio_codecs/L...
File webrtc/api/audio_codecs/L16/audio_encoder_L16.h (right):

https://codereview.webrtc.org/2995523002/diff/20001/webrtc/api/audio_codecs/L...
webrtc/api/audio_codecs/L16/audio_encoder_L16.h:32: (num_channels == 1 ||
num_channels == 2) && frame_size_ms > 0 &&
As with the decoder, the encoder implementation seems to support any number of
channels really.

https://codereview.webrtc.org/2995523002/diff/20001/webrtc/modules/audio_codi...
File webrtc/modules/audio_coding/codecs/pcm16b/pcm16b_common.h (right):

https://codereview.webrtc.org/2995523002/diff/20001/webrtc/modules/audio_codi...
webrtc/modules/audio_coding/codecs/pcm16b/pcm16b_common.h:19: void
Pcm16BAppendSupportedEncoders(std::vector<AudioCodecSpec>* specs);
Perhaps an unfortunate name, seeing as it's used by both encoders and decoders.
AppendSupportedCodecs? CodecSpecs?

[kwiberg-webrtc, 2017-08-15 11:00:53]

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

[commit-bot: I haz the power, 2017-08-15 11:01: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-08-15 11:01:59]

Thanks for the review! New patch set posted.

https://codereview.webrtc.org/2995523002/diff/20001/webrtc/api/audio_codecs/L...
File webrtc/api/audio_codecs/L16/audio_decoder_L16.h (right):

https://codereview.webrtc.org/2995523002/diff/20001/webrtc/api/audio_codecs/L...
webrtc/api/audio_codecs/L16/audio_decoder_L16.h:32: (num_channels == 1 ||
num_channels == 2);
On 2017/08/14 15:54:32, ossu wrote:
> AudioDecoderPcm16B only enforces num_channels >= 1. There seems to be at least
> some cursory support for multi-channel audio (at least for four channels), so
> perhaps let this "decoder" support any positive number of channels?

I initially did that, but IIRC the decoder factory test complained because it
used to restrict to 1 or 2 channels. Oh well---I'll just do num_channels >= 1
and update the test.

https://codereview.webrtc.org/2995523002/diff/20001/webrtc/modules/audio_codi...
File webrtc/modules/audio_coding/codecs/pcm16b/pcm16b_common.h (right):

https://codereview.webrtc.org/2995523002/diff/20001/webrtc/modules/audio_codi...
webrtc/modules/audio_coding/codecs/pcm16b/pcm16b_common.h:19: void
Pcm16BAppendSupportedEncoders(std::vector<AudioCodecSpec>* specs);
On 2017/08/14 15:54:32, ossu wrote:
> Perhaps an unfortunate name, seeing as it's used by both encoders and
decoders.
> AppendSupportedCodecs? CodecSpecs?

Done.

[commit-bot: I haz the power, 2017-08-15 11:27:09]

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

[commit-bot: I haz the power, 2017-08-15 11:27:10]

Dry run: This issue passed the CQ dry run.

[ossu, 2017-08-15 11:53:30]

Great! LGTM!

[kwiberg-webrtc, 2017-08-15 12:03:27]

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

[commit-bot: I haz the power, 2017-08-15 12:03:31]

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-08-15 12:37:41]

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

[commit-bot: I haz the power, 2017-08-15 12:37:42]

Dry run: This issue passed the CQ dry run.

[kwiberg-webrtc, 2017-08-15 13:27:05]

The CQ bit was checked by kwiberg@webrtc.org

[kwiberg-webrtc, 2017-08-15 13:27:06]

The patchset sent to the CQ was uploaded after l-g-t-m from ossu@webrtc.org
Link to the patchset: https://codereview.webrtc.org/2995523002/#ps80001 (title:
"avoid a narrowing conversion")

[commit-bot: I haz the power, 2017-08-15 13:27:16]

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-08-15 13:30:19]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1502803625585570,
"parent_rev": "cf5d485e147f7d7b3081692f101e496ce9e1d257", "commit_rev":
"edff94df62d5cc0563430ed8d5e2ba8c9f88f7dd"}

[commit-bot: I haz the power, 2017-08-15 13:30:25]

Description was changed from

==========
L16 implementation of the Audio{En,De}coderFactoryTemplate APIs

BUG=webrtc:7836, webrtc:7842
==========

to

==========
L16 implementation of the Audio{En,De}coderFactoryTemplate APIs

BUG=webrtc:7836, webrtc:7842

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

[commit-bot: I haz the power, 2017-08-15 13:30:26]

Committed patchset #5 (id:80001) as
https://chromium.googlesource.com/external/webrtc/+/edff94df62d5cc0563430ed8d...

[charujain, 2017-08-15 14:49:57]

A revert of this CL (patchset #5 id:80001) has been created in
https://codereview.webrtc.org/2996993002/ by charujain@webrtc.org.

The reason for reverting is: Breaks compilation in google3..