Renaming AudioEncoder::SetTargetBitrate and SetProjectedPacketLossRate.

Renaming AudioEncoder::SetTargetBitrate and SetProjectedPacketLossRate.

BUG=webrtc:6303
Committed: https://crrev.com/84e56d576806635c966093d5421c5d04c9b90746
Cr-Commit-Position: refs/heads/master@{#15310}

[minyue-webrtc, 2016-10-11 11:38:51]

Description was changed from

==========
Renaming AudioEncoder::SetTargetBitrate and SetProjectedPacketLossRate.

BUG=
==========

to

==========
Renaming AudioEncoder::SetTargetBitrate and SetProjectedPacketLossRate.

BUG=
==========

[minyue-webrtc, 2016-10-11 11:38:51]

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

[minyue-webrtc, 2016-10-20 11:58:00]

Hi Karl,

It has been almost 2 weeks since I sent out a PSA on this issue.

Now it can be good time to start reviewing it.

[kwiberg-webrtc, 2016-10-20 21:46:23]

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

https://codereview.chromium.org/2411613002/diff/1/webrtc/modules/audio_coding...
webrtc/modules/audio_coding/codecs/opus/audio_encoder_opus.cc:221:
SetProjectedPacketLossRate(uplink_packet_loss_fraction);
Shouldn't there be an early retirn here? Or an else?

[minyue-webrtc, 2016-11-08 13:21:16]

Patchset #2 (id:20001) has been deleted

[minyue-webrtc, 2016-11-08 13:27:05]

Patchset #2 (id:40001) has been deleted

[minyue-webrtc, 2016-11-08 13:37:24]

Hi Karl,

Now this CL is rebased on master (as opposed to an unfinished CL as it was). It
looks ok to me. please take a look.

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

https://codereview.webrtc.org/2411613002/diff/1/webrtc/modules/audio_coding/c...
webrtc/modules/audio_coding/codecs/opus/audio_encoder_opus.cc:221:
SetProjectedPacketLossRate(uplink_packet_loss_fraction);
On 2016/10/20 21:46:23, kwiberg-webrtc wrote:
> Shouldn't there be an early retirn here? Or an else?

Yes, it should early return :)

This is self-solved after rebasing.

[kwiberg-webrtc, 2016-11-08 16:10:35]

lgtm

[minyue-webrtc, 2016-11-08 17:04:21]

The CQ bit was checked by minyue@webrtc.org

[commit-bot: I haz the power, 2016-11-08 17:04:31]

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, 2016-11-08 17:06:41]

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

[commit-bot: I haz the power, 2016-11-08 17:06:42]

Try jobs failed on following builders:
  android_compile_x64_dbg on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/android_compile_x64_dbg...)
  linux_libfuzzer_rel on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/linux_libfuzzer_rel/bui...)

[minyue-webrtc, 2016-11-09 07:01:54]

Patchset #3 (id:80001) has been deleted

[minyue-webrtc, 2016-11-09 07:02:05]

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

[commit-bot: I haz the power, 2016-11-09 07:02:11]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[minyue-webrtc, 2016-11-09 07:03:37]

Hi Karl,

Some errors were introduced by rebasing. Sorry for missing them earlier. I have
fixed them. They are fairly simple to fix.

[minyue-webrtc, 2016-11-09 07:04:09]

Description was changed from

==========
Renaming AudioEncoder::SetTargetBitrate and SetProjectedPacketLossRate.

BUG=
==========

to

==========
Renaming AudioEncoder::SetTargetBitrate and SetProjectedPacketLossRate.

BUG=webrtc:6303
==========

[commit-bot: I haz the power, 2016-11-09 07:08:41]

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

[commit-bot: I haz the power, 2016-11-09 07:08:42]

Dry run: Try jobs failed on following builders:
  win_x64_rel on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/win_x64_rel/builds/19419)

[minyue-webrtc, 2016-11-09 08:08:10]

On 2016/11/09 07:03:37, minyue-webrtc wrote:
> Hi Karl,
> 
> Some errors were introduced by rebasing. Sorry for missing them earlier. I
have
> fixed them. They are fairly simple to fix.

Hi, wait a second. An Opus unittest fails since the packet loss rate is smoothed
when switching to the new API. I need to fix that, too.

[minyue-webrtc, 2016-11-09 14:17:39]

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

[commit-bot: I haz the power, 2016-11-09 14:17:43]

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, 2016-11-09 14:20:53]

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

[commit-bot: I haz the power, 2016-11-09 14:20:54]

Dry run: Try jobs failed on following builders:
  win_rel on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/win_rel/builds/19816)

[minyue-webrtc, 2016-11-09 14:34:34]

Patchset #4 (id:120001) has been deleted

[minyue-webrtc, 2016-11-09 14:34:44]

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

[commit-bot: I haz the power, 2016-11-09 14:34:51]

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, 2016-11-09 14:38:05]

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

[commit-bot: I haz the power, 2016-11-09 14:38:06]

Dry run: Try jobs failed on following builders:
  win_x64_dbg on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/win_x64_dbg/builds/2973)
  win_x64_rel on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/win_x64_rel/builds/19431)

[minyue-webrtc, 2016-11-09 14:39:41]

Patchset #4 (id:140001) has been deleted

[minyue-webrtc, 2016-11-09 15:03:39]

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

[commit-bot: I haz the power, 2016-11-09 15:03:41]

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, 2016-11-09 15:28:33]

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

[commit-bot: I haz the power, 2016-11-09 15:28:36]

Dry run: Try jobs failed on following builders:
  android_arm64_rel on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/android_arm64_rel/build...)

[minyue-webrtc, 2016-11-10 14:47:21]

Patchset #4 (id:160001) has been deleted

[minyue-webrtc, 2016-11-10 14:47:28]

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

[commit-bot: I haz the power, 2016-11-10 14:47:37]

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, 2016-11-10 15:14:53]

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

[commit-bot: I haz the power, 2016-11-10 15:14:54]

Dry run: Try jobs failed on following builders:
  android_rel on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/android_rel/builds/18290)

[minyue-webrtc, 2016-11-10 15:28:10]

Patchset #4 (id:180001) has been deleted

[minyue-webrtc, 2016-11-10 15:28:21]

Patchset #4 (id:200001) has been deleted

[minyue-webrtc, 2016-11-10 15:28:59]

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

[commit-bot: I haz the power, 2016-11-10 15:29:03]

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, 2016-11-10 16:43:06]

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

[commit-bot: I haz the power, 2016-11-10 16:43:06]

Dry run: This issue passed the CQ dry run.

[minyue-webrtc, 2016-11-10 16:45:15]

On 2016/11/09 08:08:10, minyue-webrtc wrote:
> On 2016/11/09 07:03:37, minyue-webrtc wrote:
> > Hi Karl,
> > 
> > Some errors were introduced by rebasing. Sorry for missing them earlier. I
> have
> > fixed them. They are fairly simple to fix.
> 
> Hi, wait a second. An Opus unittest fails since the packet loss rate is
smoothed
> when switching to the new API. I need to fix that, too.

Hi Karl,

I finally managed to fix the unit test. Would you please take a look at it?

[minyue-webrtc, 2016-11-10 16:45:30]

https://codereview.webrtc.org/2411613002/diff/220001/webrtc/modules/audio_cod...
File webrtc/modules/audio_coding/codecs/opus/audio_encoder_opus_unittest.cc
(right):

https://codereview.webrtc.org/2411613002/diff/220001/webrtc/modules/audio_cod...
webrtc/modules/audio_coding/codecs/opus/audio_encoder_opus_unittest.cc:187:
points.push_back(a);
This is to avoid numerical errors like 0.0f becomes -0.0f.

https://codereview.webrtc.org/2411613002/diff/220001/webrtc/modules/audio_cod...
webrtc/modules/audio_coding/codecs/opus/audio_encoder_opus_unittest.cc:216:
constexpr float eps = 1e-8f;
I changed this because

OnReceivedUplinkPacketLossFraction takes a float argument, instead of double as
the old API did.

The numerical precision has to be reduced.

https://codereview.webrtc.org/2411613002/diff/220001/webrtc/modules/audio_cod...
webrtc/modules/audio_coding/codecs/opus/audio_encoder_opus_unittest.cc:330:
constexpr int64_t kSecondSampleTimeMs = 6931;
There was an error (float->int64_t) in old code. fixed here too.

[kwiberg-webrtc, 2016-11-11 10:25:41]

lgtm

But I don't really see why you'd want to change the Opus interface functions to
take floats instead of doubles.

https://codereview.webrtc.org/2411613002/diff/220001/webrtc/modules/audio_cod...
File webrtc/modules/audio_coding/codecs/opus/audio_encoder_opus_unittest.cc
(right):

https://codereview.webrtc.org/2411613002/diff/220001/webrtc/modules/audio_cod...
webrtc/modules/audio_coding/codecs/opus/audio_encoder_opus_unittest.cc:187:
points.push_back(a);
On 2016/11/10 16:45:30, minyue-webrtc wrote:
> This is to avoid numerical errors like 0.0f becomes -0.0f.

Acknowledged.

https://codereview.webrtc.org/2411613002/diff/220001/webrtc/modules/audio_cod...
webrtc/modules/audio_coding/codecs/opus/audio_encoder_opus_unittest.cc:216:
constexpr float eps = 1e-8f;
On 2016/11/10 16:45:30, minyue-webrtc wrote:
> I changed this because
> 
> OnReceivedUplinkPacketLossFraction takes a float argument, instead of double
as
> the old API did.
> 
> The numerical precision has to be reduced.

Acknowledged.

https://codereview.webrtc.org/2411613002/diff/220001/webrtc/modules/audio_cod...
webrtc/modules/audio_coding/codecs/opus/audio_encoder_opus_unittest.cc:330:
constexpr int64_t kSecondSampleTimeMs = 6931;
On 2016/11/10 16:45:30, minyue-webrtc wrote:
> There was an error (float->int64_t) in old code. fixed here too.

Acknowledged.

[minyue-webrtc, 2016-11-11 14:02:02]

On 2016/11/11 10:25:41, kwiberg-webrtc wrote:
> lgtm
> 
> But I don't really see why you'd want to change the Opus interface functions
to
> take floats instead of doubles.

Actually no strong reasons for that, but two small benefits
1.  precision-wise, float should be enough, since 1) the value came from
integer, and 2) it is mapped back to integer.
2. the expfilter (used as packet loss smoothing filter) takes float and outputs
float


>
https://codereview.webrtc.org/2411613002/diff/220001/webrtc/modules/audio_cod...
> File webrtc/modules/audio_coding/codecs/opus/audio_encoder_opus_unittest.cc
> (right):
> 
>
https://codereview.webrtc.org/2411613002/diff/220001/webrtc/modules/audio_cod...
> webrtc/modules/audio_coding/codecs/opus/audio_encoder_opus_unittest.cc:187:
> points.push_back(a);
> On 2016/11/10 16:45:30, minyue-webrtc wrote:
> > This is to avoid numerical errors like 0.0f becomes -0.0f.
> 
> Acknowledged.
> 
>
https://codereview.webrtc.org/2411613002/diff/220001/webrtc/modules/audio_cod...
> webrtc/modules/audio_coding/codecs/opus/audio_encoder_opus_unittest.cc:216:
> constexpr float eps = 1e-8f;
> On 2016/11/10 16:45:30, minyue-webrtc wrote:
> > I changed this because
> > 
> > OnReceivedUplinkPacketLossFraction takes a float argument, instead of double
> as
> > the old API did.
> > 
> > The numerical precision has to be reduced.
> 
> Acknowledged.
> 
>
https://codereview.webrtc.org/2411613002/diff/220001/webrtc/modules/audio_cod...
> webrtc/modules/audio_coding/codecs/opus/audio_encoder_opus_unittest.cc:330:
> constexpr int64_t kSecondSampleTimeMs = 6931;
> On 2016/11/10 16:45:30, minyue-webrtc wrote:
> > There was an error (float->int64_t) in old code. fixed here too.
> 
> Acknowledged.

[minyue-webrtc, 2016-11-29 22:18:14]

The CQ bit was checked by minyue@webrtc.org

[commit-bot: I haz the power, 2016-11-29 22:18:19]

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, 2016-11-29 22:19:28]

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

[commit-bot: I haz the power, 2016-11-29 22:19:29]

Try jobs failed on following builders:
  android_more_configs on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/android_more_configs/bu...)
  ios32_sim_dbg on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/ios32_sim_dbg/builds/12919)
  ios_arm64_dbg on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/ios_arm64_dbg/builds/15292)
  ios_arm64_rel on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/ios_arm64_rel/builds/15152)
  ios_dbg on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/ios_dbg/builds/20510)
  ios_rel on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/ios_rel/builds/19225)

[minyue-webrtc, 2016-11-29 22:42:07]

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

[minyue-webrtc, 2016-11-29 22:43:22]

I forgot to commit this one. Now the rebasing may affect Henrik's planned
changes.

Add Henrik to take a look and suggest when to commit this.

[minyue-webrtc, 2016-11-29 22:43:33]

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

[commit-bot: I haz the power, 2016-11-29 22:43:42]

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, 2016-11-29 23:08:36]

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

[commit-bot: I haz the power, 2016-11-29 23:08:37]

Dry run: This issue passed the CQ dry run.

[hlundin-webrtc, 2016-11-30 08:07:40]

I see no conflicts with my previous work here. LGTM.

[minyue-webrtc, 2016-11-30 08:26:20]

The CQ bit was checked by minyue@webrtc.org

[minyue-webrtc, 2016-11-30 08:26:20]

The patchset sent to the CQ was uploaded after l-g-t-m from kwiberg@webrtc.org
Link to the patchset: https://codereview.webrtc.org/2411613002/#ps240001 (title:
"rebasing")

[commit-bot: I haz the power, 2016-11-30 08:26:24]

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, 2016-11-30 08:28:02]

CQ is committing da patch.

Bot data: {"patchset_id": 240001, "attempt_start_ts": 1480494380258480,
"parent_rev": "6ec9bff240971f81901248488fb94627b04bbbe2", "commit_rev":
"751b7089f46aa2dab8037d5803dad05daf005cfb"}

[commit-bot: I haz the power, 2016-11-30 08:28:05]

Description was changed from

==========
Renaming AudioEncoder::SetTargetBitrate and SetProjectedPacketLossRate.

BUG=webrtc:6303
==========

to

==========
Renaming AudioEncoder::SetTargetBitrate and SetProjectedPacketLossRate.

BUG=webrtc:6303
==========

[commit-bot: I haz the power, 2016-11-30 08:28:07]

Committed patchset #5 (id:240001)

[commit-bot: I haz the power, 2016-11-30 08:28:16]

Description was changed from

==========
Renaming AudioEncoder::SetTargetBitrate and SetProjectedPacketLossRate.

BUG=webrtc:6303
==========

to

==========
Renaming AudioEncoder::SetTargetBitrate and SetProjectedPacketLossRate.

BUG=webrtc:6303

Committed: https://crrev.com/84e56d576806635c966093d5421c5d04c9b90746
Cr-Commit-Position: refs/heads/master@{#15310}
==========

[commit-bot: I haz the power, 2016-11-30 08:28:18]

Patchset 5 (id:??) landed as
https://crrev.com/84e56d576806635c966093d5421c5d04c9b90746
Cr-Commit-Position: refs/heads/master@{#15310}

[minyue-webrtc, 2016-11-30 09:18:34]

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

The reason for reverting is: internal bot failure.