Adding build switch for Opus that supports 120ms ptime.

Adding build switch for Opus that supports 120ms ptime.

BUG=webrtc:7097
TEST=Set "ptime=120", try WebRTC calls over custom build Chromium with and without Opus 120ms. Try both Chromium w <-> Chromium w and Chromium w <-> Chromium w/o

Review-Url: https://codereview.webrtc.org/2668633004
Cr-Commit-Position: refs/heads/master@{#16408}
Committed: https://chromium.googlesource.com/external/webrtc/+/2e03c6611927af6ed08cf967b95342744ff329b6

[minyue-webrtc, 2017-02-01 09:32:59]

Description was changed from

==========
Adding build switch for Opus that supports 120ms ptime.

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

to

==========
Adding build switch for Opus that supports 120ms ptime.

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

[minyue-webrtc, 2017-02-01 09:32:59]

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

[minyue-webrtc, 2017-02-01 09:38:14]

Description was changed from

==========
Adding build switch for Opus that supports 120ms ptime.

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

to

==========
Adding build switch for Opus that supports 120ms ptime.

BUG=

TEST=Set "ptime=120", try WebRTC calls over custom build Chromium with and
without Opus 120ms. Try both Chromium w <-> Chromium w and Chromium w <->
Chromium w/o
==========

[minyue-webrtc, 2017-02-01 09:39:26]

Hi,

I will do the tests as written in TEST. But please start review and let me know
if you find flaws by reading it.

[hlundin-webrtc, 2017-02-01 14:12:34]

LG, but one nit and a question.

https://codereview.webrtc.org/2668633004/diff/1/webrtc/media/engine/webrtcvoi...
File webrtc/media/engine/webrtcvoiceengine.cc (right):

https://codereview.webrtc.org/2668633004/diff/1/webrtc/media/engine/webrtcvoi...
webrtc/media/engine/webrtcvoiceengine.cc:1480: max_packet_size_ms = 60;
Is this relevant to change?

https://codereview.webrtc.org/2668633004/diff/1/webrtc/webrtc.gni
File webrtc/webrtc.gni (right):

https://codereview.webrtc.org/2668633004/diff/1/webrtc/webrtc.gni#newcode20
webrtc/webrtc.gni:20: # Enable this if the Opus version upon which WebRTC is
built support direct
supports

[michaelt, 2017-02-01 15:01:08]

https://codereview.webrtc.org/2668633004/diff/1/webrtc/media/engine/webrtcvoi...
File webrtc/media/engine/webrtcvoiceengine.cc (right):

https://codereview.webrtc.org/2668633004/diff/1/webrtc/media/engine/webrtcvoi...
webrtc/media/engine/webrtcvoiceengine.cc:1480: max_packet_size_ms = 60;
Will be handled in https://codereview.webrtc.org/2669733002/

[hlundin-webrtc, 2017-02-01 15:11:00]

LGTM with my one remaining nit.

https://codereview.webrtc.org/2668633004/diff/1/webrtc/media/engine/webrtcvoi...
File webrtc/media/engine/webrtcvoiceengine.cc (right):

https://codereview.webrtc.org/2668633004/diff/1/webrtc/media/engine/webrtcvoi...
webrtc/media/engine/webrtcvoiceengine.cc:1480: max_packet_size_ms = 60;
On 2017/02/01 15:01:08, michaelt wrote:
> Will be handled in https://codereview.webrtc.org/2669733002/
> 

Acknowledged.

[minyue-webrtc, 2017-02-01 15:27:41]

On 2017/02/01 15:11:00, hlundin-webrtc wrote:
> LGTM with my one remaining nit.
> 
>
https://codereview.webrtc.org/2668633004/diff/1/webrtc/media/engine/webrtcvoi...
> File webrtc/media/engine/webrtcvoiceengine.cc (right):
> 
>
https://codereview.webrtc.org/2668633004/diff/1/webrtc/media/engine/webrtcvoi...
> webrtc/media/engine/webrtcvoiceengine.cc:1480: max_packet_size_ms = 60;
> On 2017/02/01 15:01:08, michaelt wrote:
> > Will be handled in https://codereview.webrtc.org/2669733002/
> > 
> 
> Acknowledged.

Thanks, but my tests show an error. I will fix. keep tuned.

[minyue-webrtc, 2017-02-01 16:09:43]

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

[minyue-webrtc, 2017-02-01 16:09:56]

https://codereview.webrtc.org/2668633004/diff/1/webrtc/webrtc.gni
File webrtc/webrtc.gni (right):

https://codereview.webrtc.org/2668633004/diff/1/webrtc/webrtc.gni#newcode20
webrtc/webrtc.gni:20: # Enable this if the Opus version upon which WebRTC is
built support direct
On 2017/02/01 14:12:34, hlundin-webrtc wrote:
> supports

will address it. 

Now I fix an error and PTAL again. and I will add Peter to review.

[minyue-webrtc, 2017-02-01 16:22:21]

Description was changed from

==========
Adding build switch for Opus that supports 120ms ptime.

BUG=

TEST=Set "ptime=120", try WebRTC calls over custom build Chromium with and
without Opus 120ms. Try both Chromium w <-> Chromium w and Chromium w <->
Chromium w/o
==========

to

==========
Adding build switch for Opus that supports 120ms ptime.

BUG=webrtc:7097

TEST=Set "ptime=120", try WebRTC calls over custom build Chromium with and
without Opus 120ms. Try both Chromium w <-> Chromium w and Chromium w <->
Chromium w/o
==========

[pthatcher2, 2017-02-01 19:47:38]

pthatcher@chromium.org changed reviewers:
+ pthatcher@chromium.org

[pthatcher2, 2017-02-01 19:47:38]

lgtm

[michaelt, 2017-02-01 19:48:26]

lgtm

[minyue-webrtc, 2017-02-01 20:34:35]

nit henrik found is fixed. commiting now

[minyue-webrtc, 2017-02-01 20:34:41]

The CQ bit was checked by minyue@webrtc.org

[minyue-webrtc, 2017-02-01 20:34:41]

The patchset sent to the CQ was uploaded after l-g-t-m from
henrik.lundin@webrtc.org, pthatcher@chromium.org, michaelt@webrtc.org
Link to the patchset: https://codereview.webrtc.org/2668633004/#ps40001 (title:
"fixing a nit")

[commit-bot: I haz the power, 2017-02-01 20:34:47]

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-02-01 20:36:27]

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

[commit-bot: I haz the power, 2017-02-01 20:36:28]

Try jobs failed on following builders:
  ios64_sim_ios10_dbg on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/ios64_sim_ios10_dbg/bui...)
  ios64_sim_ios9_dbg on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/ios64_sim_ios9_dbg/buil...)
  ios_rel on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/ios_rel/builds/21037)

[minyue-webrtc, 2017-02-01 21:09:37]

The CQ bit was checked by minyue@webrtc.org

[minyue-webrtc, 2017-02-01 21:09:37]

The patchset sent to the CQ was uploaded after l-g-t-m from
henrik.lundin@webrtc.org, pthatcher@chromium.org, michaelt@webrtc.org
Link to the patchset: https://codereview.webrtc.org/2668633004/#ps60001 (title:
"my bad, a fix needed")

[commit-bot: I haz the power, 2017-02-01 21:09:45]

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-02-01 21:15:34]

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

[commit-bot: I haz the power, 2017-02-01 21:15:35]

Try jobs failed on following builders:
  presubmit on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/presubmit/builds/12926)

[minyue-webrtc, 2017-02-01 21:16:23]

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

[minyue-webrtc, 2017-02-01 21:16:56]

On 2017/02/01 21:15:35, commit-bot: I haz the power wrote:
> Try jobs failed on following builders:
>   presubmit on master.tryserver.webrtc (JOB_FAILED,
> http://build.chromium.org/p/tryserver.webrtc/builders/presubmit/builds/12926)

+the sun for VoE owner approval

[the sun, 2017-02-01 22:25:39]

On 2017/02/01 21:16:56, minyue-webrtc wrote:
> On 2017/02/01 21:15:35, commit-bot: I haz the power wrote:
> > Try jobs failed on following builders:
> >   presubmit on master.tryserver.webrtc (JOB_FAILED,
> >
http://build.chromium.org/p/tryserver.webrtc/builders/presubmit/builds/12926)
> 
> +the sun for VoE owner approval

Why the need for a compile time flag? Is WebRTC built with different versions of
Opus in native build vs. when part of Chromium?

[the sun, 2017-02-01 23:37:57]

lgtm

[minyue-webrtc, 2017-02-02 00:18:28]

The CQ bit was checked by minyue@webrtc.org

[minyue-webrtc, 2017-02-02 00:18:28]

The patchset sent to the CQ was uploaded after l-g-t-m from
solenberg@webrtc.org, henrik.lundin@webrtc.org, pthatcher@chromium.org,
michaelt@webrtc.org
Link to the patchset: https://codereview.webrtc.org/2668633004/#ps80001 (title:
"nit: undo unintended format")

[commit-bot: I haz the power, 2017-02-02 00:18:32]

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-02-02 01:31:12]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1485994708249930,
"parent_rev": "d3d3ba5159311296d1a190aa84d20cf1c785ba3d", "commit_rev":
"2e03c6611927af6ed08cf967b95342744ff329b6"}

[commit-bot: I haz the power, 2017-02-02 01:31:15]

Description was changed from

==========
Adding build switch for Opus that supports 120ms ptime.

BUG=webrtc:7097

TEST=Set "ptime=120", try WebRTC calls over custom build Chromium with and
without Opus 120ms. Try both Chromium w <-> Chromium w and Chromium w <->
Chromium w/o
==========

to

==========
Adding build switch for Opus that supports 120ms ptime.

BUG=webrtc:7097

TEST=Set "ptime=120", try WebRTC calls over custom build Chromium with and
without Opus 120ms. Try both Chromium w <-> Chromium w and Chromium w <->
Chromium w/o

Review-Url: https://codereview.webrtc.org/2668633004
Cr-Commit-Position: refs/heads/master@{#16408}
Committed:
https://chromium.googlesource.com/external/webrtc/+/2e03c6611927af6ed08cf967b...
==========

[commit-bot: I haz the power, 2017-02-02 01:31:18]

Committed patchset #5 (id:80001) as
https://chromium.googlesource.com/external/webrtc/+/2e03c6611927af6ed08cf967b...