Let AudioCodingModule::SendCodec return Maybe<CodecInst>

Let AudioCodingModule::SendCodec return Maybe<CodecInst>

And deal with the consequences thereof...

Committed: https://crrev.com/1fd4a4ab35f1fcb08b551befca180de9a1f55dfc
Cr-Commit-Position: refs/heads/master@{#10497}

[kwiberg-webrtc, 2015-11-03 08:13:58]

Description was changed from

==========
Let AudioCodingModule::SendCodec return Maybe<CodecInst>

And deal with the consequences thereof...
==========

to

==========
Let AudioCodingModule::SendCodec return Maybe<CodecInst>

And deal with the consequences thereof...
==========

[kwiberg-webrtc, 2015-11-03 08:13:58]

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

[kwiberg-webrtc, 2015-11-03 08:15:28]

Please have a look. I've tried to divide the load evenly between you.

Henrik: audio_coding
Fredrik: voice_engine

[hlundin-webrtc, 2015-11-03 08:56:13]

Nice!

audio_coding: lgtm

[the sun, 2015-11-03 10:13:42]

lgtm, save for one nit

https://codereview.webrtc.org/1406123011/diff/1/webrtc/voice_engine/channel.cc
File webrtc/voice_engine/channel.cc (right):

https://codereview.webrtc.org/1406123011/diff/1/webrtc/voice_engine/channel.c...
webrtc/voice_engine/channel.cc:1247: codec = *send_codec;
nit: less indent + braxies

[kwiberg-webrtc, 2015-11-03 12:23:53]

https://codereview.webrtc.org/1406123011/diff/1/webrtc/voice_engine/channel.cc
File webrtc/voice_engine/channel.cc (right):

https://codereview.webrtc.org/1406123011/diff/1/webrtc/voice_engine/channel.c...
webrtc/voice_engine/channel.cc:1247: codec = *send_codec;
On 2015/11/03 10:13:42, the sun wrote:
> nit: less indent + braxies

I was trying to keep with the style of the rest of the file. Couldn't bring
myself to imitate the brace style it's using, though, so I picked an acceptable
alternative.

Would you prefer clang-format-compatible formatting instead?

[the sun, 2015-11-03 12:38:21]

https://codereview.webrtc.org/1406123011/diff/1/webrtc/voice_engine/channel.cc
File webrtc/voice_engine/channel.cc (right):

https://codereview.webrtc.org/1406123011/diff/1/webrtc/voice_engine/channel.c...
webrtc/voice_engine/channel.cc:1247: codec = *send_codec;
On 2015/11/03 12:23:53, kwiberg-webrtc wrote:
> On 2015/11/03 10:13:42, the sun wrote:
> > nit: less indent + braxies
> 
> I was trying to keep with the style of the rest of the file. Couldn't bring
> myself to imitate the brace style it's using, though, so I picked an
acceptable
> alternative.
> 
> Would you prefer clang-format-compatible formatting instead?

just fix the indent, but this function would read better as:

auto send_codec = audio_coding_->SendCodec();
if (send_codec) {
  codec = *send_codec;
  return 0;
}
return -1;

No need to repeat the condition

[kwiberg-webrtc, 2015-11-03 13:08:19]

https://codereview.webrtc.org/1406123011/diff/1/webrtc/voice_engine/channel.cc
File webrtc/voice_engine/channel.cc (right):

https://codereview.webrtc.org/1406123011/diff/1/webrtc/voice_engine/channel.c...
webrtc/voice_engine/channel.cc:1247: codec = *send_codec;
On 2015/11/03 12:38:21, the sun wrote:
> On 2015/11/03 12:23:53, kwiberg-webrtc wrote:
> > On 2015/11/03 10:13:42, the sun wrote:
> > > nit: less indent + braxies
> > 
> > I was trying to keep with the style of the rest of the file. Couldn't bring
> > myself to imitate the brace style it's using, though, so I picked an
> acceptable
> > alternative.
> > 
> > Would you prefer clang-format-compatible formatting instead?
> 
> just fix the indent, but this function would read better as:
> 
> auto send_codec = audio_coding_->SendCodec();
> if (send_codec) {
>   codec = *send_codec;
>   return 0;
> }
> return -1;
> 
> No need to repeat the condition

Done.

[kwiberg-webrtc, 2015-11-03 19:19:18]

The CQ bit was checked by kwiberg@webrtc.org

[kwiberg-webrtc, 2015-11-03 19:19:18]

The patchset sent to the CQ was uploaded after l-g-t-m from
solenberg@webrtc.org, henrik.lundin@webrtc.org
Link to the patchset: https://codereview.webrtc.org/1406123011/#ps20001 (title:
"review fix")

[commit-bot: I haz the power, 2015-11-03 19:19:29]

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

[commit-bot: I haz the power, 2015-11-03 19:20:54]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-11-03 19:21:13]

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