Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(83)

Issue 3007983002: Implement ANA statistics. (Closed)

Created:
3 years, 3 months ago by ivoc
Modified:
3 years, 3 months ago
CC:
webrtc-reviews_webrtc.org, AleBzk, peah-webrtc, tlegrand-webrtc, tterriberry_mozilla.com, audio-team_agora.io, hlundin-webrtc, kwiberg-webrtc, minyue-webrtc
Target Ref:
refs/heads/master
Project:
webrtc
Visibility:
Public.

Description

Implement ANA statistics. This CL also makes it possible to enable/prevent ANA controllers from making adaptations using field trials. BUG=webrtc:8127 Review-Url: https://codereview.webrtc.org/3007983002 Cr-Commit-Position: refs/heads/master@{#19761} Committed: https://chromium.googlesource.com/external/webrtc/+/17289097f08280b377c4ca550e8d75e46ffcede3

Patch Set 1 #

Total comments: 8

Patch Set 2 : Review comments. #

Patch Set 3 : Changed upstream dependency. #

Patch Set 4 : Implemented additional stats. #

Total comments: 7

Patch Set 5 : Review comments. #

Patch Set 6 : Fix for failing unittests. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+97 lines, -19 lines) Patch
M webrtc/modules/audio_coding/audio_network_adaptor/audio_network_adaptor_impl.h View 1 2 3 1 chunk +10 lines, -0 lines 0 comments Download
M webrtc/modules/audio_coding/audio_network_adaptor/audio_network_adaptor_impl.cc View 1 2 3 4 4 chunks +62 lines, -4 lines 0 comments Download
M webrtc/modules/audio_coding/audio_network_adaptor/audio_network_adaptor_impl_unittest.cc View 1 2 3 4 5 5 chunks +25 lines, -15 lines 0 comments Download

Depends on Patchset:

Messages

Total messages: 25 (11 generated)
ivoc
Hi guys, PTAL at this CL to implement ANA statistics and add support for field ...
3 years, 3 months ago (2017-09-01 14:34:37 UTC) #3
minyue-webrtc
https://codereview.webrtc.org/3007983002/diff/1/webrtc/modules/audio_coding/audio_network_adaptor/audio_network_adaptor_impl.h File webrtc/modules/audio_coding/audio_network_adaptor/audio_network_adaptor_impl.h (right): https://codereview.webrtc.org/3007983002/diff/1/webrtc/modules/audio_coding/audio_network_adaptor/audio_network_adaptor_impl.h#newcode82 webrtc/modules/audio_coding/audio_network_adaptor/audio_network_adaptor_impl.h:82: bool enable_bitrate_adaptation_ = true; My comment: add const, then ...
3 years, 3 months ago (2017-09-01 14:52:13 UTC) #5
alexnarest
https://codereview.webrtc.org/3007983002/diff/1/webrtc/modules/audio_coding/audio_network_adaptor/audio_network_adaptor_impl.cc File webrtc/modules/audio_coding/audio_network_adaptor/audio_network_adaptor_impl.cc (right): https://codereview.webrtc.org/3007983002/diff/1/webrtc/modules/audio_coding/audio_network_adaptor/audio_network_adaptor_impl.cc#newcode147 webrtc/modules/audio_coding/audio_network_adaptor/audio_network_adaptor_impl.cc:147: if (config.enable_fec != prev_config_->enable_fec) { In addition I think ...
3 years, 3 months ago (2017-09-04 11:24:02 UTC) #6
ivoc
Thanks for the comments. https://codereview.webrtc.org/3007983002/diff/1/webrtc/modules/audio_coding/audio_network_adaptor/audio_network_adaptor_impl.cc File webrtc/modules/audio_coding/audio_network_adaptor/audio_network_adaptor_impl.cc (right): https://codereview.webrtc.org/3007983002/diff/1/webrtc/modules/audio_coding/audio_network_adaptor/audio_network_adaptor_impl.cc#newcode147 webrtc/modules/audio_coding/audio_network_adaptor/audio_network_adaptor_impl.cc:147: if (config.enable_fec != prev_config_->enable_fec) { ...
3 years, 3 months ago (2017-09-05 09:01:40 UTC) #7
alexnarest
https://codereview.webrtc.org/3007983002/diff/1/webrtc/modules/audio_coding/audio_network_adaptor/audio_network_adaptor_impl.cc File webrtc/modules/audio_coding/audio_network_adaptor/audio_network_adaptor_impl.cc (right): https://codereview.webrtc.org/3007983002/diff/1/webrtc/modules/audio_coding/audio_network_adaptor/audio_network_adaptor_impl.cc#newcode147 webrtc/modules/audio_coding/audio_network_adaptor/audio_network_adaptor_impl.cc:147: if (config.enable_fec != prev_config_->enable_fec) { On 2017/09/05 09:01:40, ivoc ...
3 years, 3 months ago (2017-09-05 10:43:28 UTC) #8
ivoc
I just added the implementation of the new stats that are introduced in https://codereview.webrtc.org/3007243002/ to ...
3 years, 3 months ago (2017-09-07 14:43:40 UTC) #9
alexnarest
lgtm
3 years, 3 months ago (2017-09-07 17:17:25 UTC) #10
ossu
A few comments https://codereview.webrtc.org/3007983002/diff/60001/webrtc/modules/audio_coding/audio_network_adaptor/audio_network_adaptor_impl.cc File webrtc/modules/audio_coding/audio_network_adaptor/audio_network_adaptor_impl.cc (right): https://codereview.webrtc.org/3007983002/diff/60001/webrtc/modules/audio_coding/audio_network_adaptor/audio_network_adaptor_impl.cc#newcode137 webrtc/modules/audio_coding/audio_network_adaptor/audio_network_adaptor_impl.cc:137: a = rtc::Optional<uint32_t>(1); I think you ...
3 years, 3 months ago (2017-09-08 10:56:01 UTC) #11
ivoc
Thanks! https://codereview.webrtc.org/3007983002/diff/60001/webrtc/modules/audio_coding/audio_network_adaptor/audio_network_adaptor_impl.cc File webrtc/modules/audio_coding/audio_network_adaptor/audio_network_adaptor_impl.cc (right): https://codereview.webrtc.org/3007983002/diff/60001/webrtc/modules/audio_coding/audio_network_adaptor/audio_network_adaptor_impl.cc#newcode137 webrtc/modules/audio_coding/audio_network_adaptor/audio_network_adaptor_impl.cc:137: a = rtc::Optional<uint32_t>(1); On 2017/09/08 10:56:01, ossu wrote: ...
3 years, 3 months ago (2017-09-08 11:43:47 UTC) #12
ossu
LGTM! https://codereview.webrtc.org/3007983002/diff/60001/webrtc/modules/audio_coding/audio_network_adaptor/audio_network_adaptor_impl.cc File webrtc/modules/audio_coding/audio_network_adaptor/audio_network_adaptor_impl.cc (right): https://codereview.webrtc.org/3007983002/diff/60001/webrtc/modules/audio_coding/audio_network_adaptor/audio_network_adaptor_impl.cc#newcode137 webrtc/modules/audio_coding/audio_network_adaptor/audio_network_adaptor_impl.cc:137: a = rtc::Optional<uint32_t>(1); On 2017/09/08 11:43:47, ivoc wrote: ...
3 years, 3 months ago (2017-09-08 12:05:21 UTC) #13
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/3007983002/80001
3 years, 3 months ago (2017-09-09 05:37:19 UTC) #17
commit-bot: I haz the power
Try jobs failed on following builders: linux_ubsan_vptr on master.tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/linux_ubsan_vptr/builds/16086)
3 years, 3 months ago (2017-09-09 05:44:49 UTC) #19
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/3007983002/120001
3 years, 3 months ago (2017-09-09 15:16:29 UTC) #22
commit-bot: I haz the power
3 years, 3 months ago (2017-09-09 15:45:48 UTC) #25
Message was sent while issue was closed.
Committed patchset #6 (id:120001) as
https://chromium.googlesource.com/external/webrtc/+/17289097f08280b377c4ca550...

Powered by Google App Engine
This is Rietveld 408576698