Fix AudioEncoderOpus::RecreateEncoderInstance() referring to old config_

Fix AudioEncoderOpus::RecreateEncoderInstance() referring to old config_

BUG=webrtc:7334
Review-Url: https://codereview.webrtc.org/2742383002
Cr-Commit-Position: refs/heads/master@{#17222}
Committed: https://chromium.googlesource.com/external/webrtc/+/cfd88bbe80c41a4ec3d1eb529f60e06e8bad3de8

[elad.alon_webrtc.org, 2017-03-13 14:40:55]

Description was changed from

==========
Fix AudioEncoderOpus::RecreateEncoderInstance() referring to old config_

BUG=wbertc:7334
==========

to

==========
Fix AudioEncoderOpus::RecreateEncoderInstance() referring to old config_

BUG=webrtc:7334
==========

[elad.alon_webrtc.org, 2017-03-13 14:50:31]

elad.alon@webrtc.org changed reviewers:
+ minyue@webrtc.org

[elad.alon_webrtc.org, 2017-03-13 14:50:32]

Please have a look.

https://codereview.webrtc.org/2742383002/diff/1/webrtc/modules/audio_coding/c...
File webrtc/modules/audio_coding/codecs/opus/audio_encoder_opus.cc (right):

https://codereview.webrtc.org/2742383002/diff/1/webrtc/modules/audio_coding/c...
webrtc/modules/audio_coding/codecs/opus/audio_encoder_opus.cc:451: config_ =
config;
Saving this for last would make more sense if we had a rollback mechanism for
problems that might arise later in the function. But since we either reach this
or crash the application, we might as well do it first.

https://codereview.webrtc.org/2742383002/diff/1/webrtc/modules/audio_coding/c...
webrtc/modules/audio_coding/codecs/opus/audio_encoder_opus.cc:455:
input_buffer_.reserve(Num10msFramesPerPacket() * SamplesPer10msFrame());
These functions read from |config_|.

[kwiberg-webrtc, 2017-03-14 07:04:35]

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

[kwiberg-webrtc, 2017-03-14 07:04:35]

lgtm

https://codereview.webrtc.org/2742383002/diff/1/webrtc/modules/audio_coding/c...
File webrtc/modules/audio_coding/codecs/opus/audio_encoder_opus.cc (right):

https://codereview.webrtc.org/2742383002/diff/1/webrtc/modules/audio_coding/c...
webrtc/modules/audio_coding/codecs/opus/audio_encoder_opus.cc:451: config_ =
config;
On 2017/03/13 14:50:31, elad.alon wrote:
> Saving this for last would make more sense if we had a rollback mechanism for
> problems that might arise later in the function. But since we either reach
this
> or crash the application, we might as well do it first.

Acknowledged.

https://codereview.webrtc.org/2742383002/diff/1/webrtc/modules/audio_coding/c...
webrtc/modules/audio_coding/codecs/opus/audio_encoder_opus.cc:455:
input_buffer_.reserve(Num10msFramesPerPacket() * SamplesPer10msFrame());
On 2017/03/13 14:50:32, elad.alon wrote:
> These functions read from |config_|.

Indeed. It should be harmless, since reserve() only impacts performance, but it
certainly isn't nice.

Another acceptable solution (which I probably would have picked) is to simply
remove the reserve() call.

[minyue-webrtc, 2017-03-14 07:46:50]

lgtm

good find

[elad.alon_webrtc.org, 2017-03-14 09:25:35]

The CQ bit was checked by elad.alon@webrtc.org

[commit-bot: I haz the power, 2017-03-14 09:25:38]

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-03-14 09:50:47]

CQ is committing da patch.

Bot data: {"patchset_id": 1, "attempt_start_ts": 1489483535004260, "parent_rev":
"07b8388234624c31aad7a7f4f5be2ecb2ee76ff8", "commit_rev":
"cfd88bbe80c41a4ec3d1eb529f60e06e8bad3de8"}

[commit-bot: I haz the power, 2017-03-14 09:50:49]

Description was changed from

==========
Fix AudioEncoderOpus::RecreateEncoderInstance() referring to old config_

BUG=webrtc:7334
==========

to

==========
Fix AudioEncoderOpus::RecreateEncoderInstance() referring to old config_

BUG=webrtc:7334

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

[commit-bot: I haz the power, 2017-03-14 09:50:51]

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