Templated AudioDecoderFactory

Templated AudioDecoderFactory

No real encoder implements the correct API yet, so we're just testing
dummies.

BUG=webrtc:7824
TBR=ossu@webrtc.org, solenberg@webrtc.org

Review-Url: https://codereview.webrtc.org/2936773005
Cr-Commit-Position: refs/heads/master@{#18642}
Committed: https://chromium.googlesource.com/external/webrtc/+/0eacd88568dd520bb3a584caf64918642a696181

[kwiberg-webrtc, 2017-06-14 01:18:11]

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

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

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-14 01:41:43]

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

[kwiberg-webrtc, 2017-06-14 01:41:44]

This one is really similar to the "Templated AudioEncoderFactory" CL.

[commit-bot: I haz the power, 2017-06-14 01:52:59]

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

[commit-bot: I haz the power, 2017-06-14 01:52:59]

Dry run: This issue passed the CQ dry run.

[kwiberg-webrtc, 2017-06-15 13:24:40]

Description was changed from

==========
Templated AudioDecoderFactory

No real encoder implements the correct API yet, so we're just testing
dummies.

BUG=webrtc:5805
==========

to

==========
Templated AudioDecoderFactory

No real encoder implements the correct API yet, so we're just testing
dummies.

BUG=webrtc:7824
==========

[ossu, 2017-06-16 11:25:18]

Two comments, similar to the encoder factory side. Interface, etc., looks good!

https://codereview.webrtc.org/2936773005/diff/20001/webrtc/api/audio_codecs/a...
File webrtc/api/audio_codecs/audio_decoder_factory_template.h (right):

https://codereview.webrtc.org/2936773005/diff/20001/webrtc/api/audio_codecs/a...
webrtc/api/audio_codecs/audio_decoder_factory_template.h:47: template
<typename...>
As with the encoder factory, this one could do without the extra parameter.

https://codereview.webrtc.org/2936773005/diff/20001/webrtc/api/audio_codecs/a...
webrtc/api/audio_codecs/audio_decoder_factory_template.h:114: // NOTE: This
function is still under development and may change without notice.
Should probably mention that the order of the codecs matter here as well.

[the sun, 2017-06-16 12:52:21]

https://codereview.webrtc.org/2936773005/diff/20001/webrtc/api/audio_codecs/a...
File webrtc/api/audio_codecs/audio_decoder_factory_template.h (right):

https://codereview.webrtc.org/2936773005/diff/20001/webrtc/api/audio_codecs/a...
webrtc/api/audio_codecs/audio_decoder_factory_template.h:109: //  
std::unique_ptr<AudioDecoder> MakeAudioDecoder(const ConfigType& config);
Inconsistent with the decoder impls which use by-value semantics so far. Maybe
just stick to const& everywhere?

[kwiberg-webrtc, 2017-06-16 13:17:07]

https://codereview.webrtc.org/2936773005/diff/20001/webrtc/api/audio_codecs/a...
File webrtc/api/audio_codecs/audio_decoder_factory_template.h (right):

https://codereview.webrtc.org/2936773005/diff/20001/webrtc/api/audio_codecs/a...
webrtc/api/audio_codecs/audio_decoder_factory_template.h:47: template
<typename...>
On 2017/06/16 11:25:18, ossu wrote:
> As with the encoder factory, this one could do without the extra parameter.

Yes. The decoder factory is very similar to the encoder factory, so I'll
transfer over all the changes once we're done with the encoder factory.

https://codereview.webrtc.org/2936773005/diff/20001/webrtc/api/audio_codecs/a...
webrtc/api/audio_codecs/audio_decoder_factory_template.h:109: //  
std::unique_ptr<AudioDecoder> MakeAudioDecoder(const ConfigType& config);
On 2017/06/16 12:52:21, the sun wrote:
> Inconsistent with the decoder impls which use by-value semantics so far. Maybe
> just stick to const& everywhere?

It's just an implementation detail. const ref is probably the default we want
people to use if they aren't thinking about it, but by-value is a better fit for
passing around empty structs, so I used that in some implementations. If it
bothers you I can change the decoder implementations to const ref like you
suggest, though.

https://codereview.webrtc.org/2936773005/diff/20001/webrtc/api/audio_codecs/a...
webrtc/api/audio_codecs/audio_decoder_factory_template.h:114: // NOTE: This
function is still under development and may change without notice.
On 2017/06/16 11:25:18, ossu wrote:
> Should probably mention that the order of the codecs matter here as well.

Yes, that'll be among the things copied over from the encoder factory.

[kwiberg-webrtc, 2017-06-17 22:06:36]

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

[commit-bot: I haz the power, 2017-06-17 22:06:43]

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-17 22:36:49]

Description was changed from

==========
Templated AudioDecoderFactory

No real encoder implements the correct API yet, so we're just testing
dummies.

BUG=webrtc:7824
==========

to

==========
Templated AudioDecoderFactory

No real encoder implements the correct API yet, so we're just testing
dummies.

BUG=webrtc:7824
TBR=ossu@webrtc.org, solenberg@webrtc.org
==========

[commit-bot: I haz the power, 2017-06-17 22:37:55]

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

[commit-bot: I haz the power, 2017-06-17 22:37:56]

Dry run: This issue passed the CQ dry run.

[kwiberg-webrtc, 2017-06-17 22:38:04]

The CQ bit was checked by kwiberg@webrtc.org

[commit-bot: I haz the power, 2017-06-17 22:38:14]

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-17 22:45:29]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1497739084920790,
"parent_rev": "19b3a554e8dc9e484906060576a5151e794c34cf", "commit_rev":
"0eacd88568dd520bb3a584caf64918642a696181"}

[commit-bot: I haz the power, 2017-06-17 22:45:32]

Description was changed from

==========
Templated AudioDecoderFactory

No real encoder implements the correct API yet, so we're just testing
dummies.

BUG=webrtc:7824
TBR=ossu@webrtc.org, solenberg@webrtc.org
==========

to

==========
Templated AudioDecoderFactory

No real encoder implements the correct API yet, so we're just testing
dummies.

BUG=webrtc:7824
TBR=ossu@webrtc.org, solenberg@webrtc.org

Review-Url: https://codereview.webrtc.org/2936773005
Cr-Commit-Position: refs/heads/master@{#18642}
Committed:
https://chromium.googlesource.com/external/webrtc/+/0eacd88568dd520bb3a584caf...
==========

[commit-bot: I haz the power, 2017-06-17 22:45:33]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/external/webrtc/+/0eacd88568dd520bb3a584caf...

[kwiberg-webrtc, 2017-06-17 22:56:33]

I TBR'ed this CL after giving it exactly the same overhaul as the corresponding
encoder CL.

[ossu, 2017-06-19 08:44:45]

Sweet! LGTM!