Simplify CodecManager::RegisterEncoder()

Simplify CodecManager::RegisterEncoder()

BUG=webrtc:5028
Committed: https://crrev.com/dfbb3a4bfc5761bdf55526beb9eb37b23d234839
Cr-Commit-Position: refs/heads/master@{#10855}

[kwiberg-webrtc, 2015-11-30 19:30:37]

Description was changed from

==========
Simplify CodecManager::RegisterEncoder()

BUG=webrtc:5028
==========

to

==========
Simplify CodecManager::RegisterEncoder()

BUG=webrtc:5028
==========

[kwiberg-webrtc, 2015-11-30 19:30:37]

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

[hlundin-webrtc, 2015-12-01 12:11:45]

lgtm with one suggestion.

https://codereview.webrtc.org/1483963002/diff/1/webrtc/modules/audio_coding/a...
File webrtc/modules/audio_coding/acm2/codec_manager.cc (right):

https://codereview.webrtc.org/1483963002/diff/1/webrtc/modules/audio_coding/a...
webrtc/modules/audio_coding/acm2/codec_manager.cc:172: send_codec_inst_ =
send_codec;
I'd like a DCHECK at this point to verify that all parameters but the bitrate
are unchanged. Maybe through an auxiliary function:

bool AllButRateEqual(const CodecInst& a, CodecInst b) {
  b.rate = a.rate;
  return a == b;
}

[kwiberg-webrtc, 2015-12-01 12:37:27]

https://codereview.webrtc.org/1483963002/diff/1/webrtc/modules/audio_coding/a...
File webrtc/modules/audio_coding/acm2/codec_manager.cc (right):

https://codereview.webrtc.org/1483963002/diff/1/webrtc/modules/audio_coding/a...
webrtc/modules/audio_coding/acm2/codec_manager.cc:172: send_codec_inst_ =
send_codec;
On 2015/12/01 12:11:45, hlundin-webrtc wrote:
> I'd like a DCHECK at this point to verify that all parameters but the bitrate
> are unchanged. Maybe through an auxiliary function:
> 
> bool AllButRateEqual(const CodecInst& a, CodecInst b) {
>   b.rate = a.rate;
>   return a == b;
> }

But they're not---we pass through here whether or not the bitrate was the only
thing that changed.

[kwiberg-webrtc, 2015-12-01 12:38:10]

The CQ bit was checked by kwiberg@webrtc.org

[commit-bot: I haz the power, 2015-12-01 12:38:20]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1483963002/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1483963002/1

[commit-bot: I haz the power, 2015-12-01 12:45:06]

Description was changed from

==========
Simplify CodecManager::RegisterEncoder()

BUG=webrtc:5028
==========

to

==========
Simplify CodecManager::RegisterEncoder()

BUG=webrtc:5028
==========

[commit-bot: I haz the power, 2015-12-01 12:45:06]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2015-12-01 12:45:18]

Description was changed from

==========
Simplify CodecManager::RegisterEncoder()

BUG=webrtc:5028
==========

to

==========
Simplify CodecManager::RegisterEncoder()

BUG=webrtc:5028

Committed: https://crrev.com/dfbb3a4bfc5761bdf55526beb9eb37b23d234839
Cr-Commit-Position: refs/heads/master@{#10855}
==========

[commit-bot: I haz the power, 2015-12-01 12:45:18]

Patchset 1 (id:??) landed as
https://crrev.com/dfbb3a4bfc5761bdf55526beb9eb37b23d234839
Cr-Commit-Position: refs/heads/master@{#10855}

[hlundin-webrtc, 2015-12-01 13:07:04]

https://codereview.webrtc.org/1483963002/diff/1/webrtc/modules/audio_coding/a...
File webrtc/modules/audio_coding/acm2/codec_manager.cc (right):

https://codereview.webrtc.org/1483963002/diff/1/webrtc/modules/audio_coding/a...
webrtc/modules/audio_coding/acm2/codec_manager.cc:172: send_codec_inst_ =
send_codec;
On 2015/12/01 12:37:27, kwiberg-webrtc wrote:
> On 2015/12/01 12:11:45, hlundin-webrtc wrote:
> > I'd like a DCHECK at this point to verify that all parameters but the
bitrate
> > are unchanged. Maybe through an auxiliary function:
> > 
> > bool AllButRateEqual(const CodecInst& a, CodecInst b) {
> >   b.rate = a.rate;
> >   return a == b;
> > }
> 
> But they're not---we pass through here whether or not the bitrate was the only
> thing that changed.

Oh, well, then I guess not...