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 2356763002: Adding debug dump to audio network adaptor. (Closed)

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

Description

Adding debug dump to audio network adaptor. BUG=webrtc:6303 Committed: https://crrev.com/7e4f8928062afc8d571bb69f3223711701cbaad6 Cr-Commit-Position: refs/heads/master@{#14361}

Patch Set 1 #

Total comments: 17

Patch Set 2 : on Karl's comments #

Total comments: 2

Patch Set 3 : some fixes #

Total comments: 8

Patch Set 4 : rebasing #

Patch Set 5 : some fixing #

Unified diffs Side-by-side diffs Delta from patch set Stats (+391 lines, -22 lines) Patch
M webrtc/modules/audio_coding/BUILD.gn View 1 2 3 3 chunks +16 lines, -0 lines 0 comments Download
M webrtc/modules/audio_coding/audio_network_adaptor/audio_network_adaptor.gypi View 1 2 3 4 2 chunks +28 lines, -1 line 0 comments Download
M webrtc/modules/audio_coding/audio_network_adaptor/audio_network_adaptor_impl.h View 1 2 3 chunks +17 lines, -0 lines 0 comments Download
M webrtc/modules/audio_coding/audio_network_adaptor/audio_network_adaptor_impl.cc View 1 2 5 chunks +31 lines, -7 lines 0 comments Download
M webrtc/modules/audio_coding/audio_network_adaptor/audio_network_adaptor_impl_unittest.cc View 1 2 4 chunks +76 lines, -1 line 0 comments Download
M webrtc/modules/audio_coding/audio_network_adaptor/controller.h View 1 chunk +1 line, -0 lines 0 comments Download
A webrtc/modules/audio_coding/audio_network_adaptor/debug_dump.proto View 1 chunk +30 lines, -0 lines 0 comments Download
A webrtc/modules/audio_coding/audio_network_adaptor/debug_dump_writer.h View 1 2 3 4 1 chunk +40 lines, -0 lines 0 comments Download
A webrtc/modules/audio_coding/audio_network_adaptor/debug_dump_writer.cc View 1 2 3 4 1 chunk +135 lines, -0 lines 0 comments Download
M webrtc/modules/audio_coding/audio_network_adaptor/include/audio_network_adaptor.h View 1 chunk +4 lines, -0 lines 0 comments Download
A + webrtc/modules/audio_coding/audio_network_adaptor/mock/mock_debug_dump_writer.h View 1 chunk +13 lines, -13 lines 0 comments Download

Depends on Patchset:

Messages

Total messages: 32 (17 generated)
minyue-webrtc
Hi Michael, Could you take a look at this CL?
4 years, 3 months ago (2016-09-20 14:48:03 UTC) #7
minyue-webrtc
Hi Karl, Michael has read the CL and had no major concerns. Now I add ...
4 years, 3 months ago (2016-09-21 09:16:30 UTC) #11
kwiberg-webrtc
https://codereview.webrtc.org/2356763002/diff/40001/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/2356763002/diff/40001/webrtc/modules/audio_coding/audio_network_adaptor/audio_network_adaptor_impl.cc#newcode34 webrtc/modules/audio_coding/audio_network_adaptor/audio_network_adaptor_impl.cc:34: } You can make just one constructor with the ...
4 years, 3 months ago (2016-09-21 10:54:09 UTC) #12
minyue-webrtc
Hi Karl, Thanks for the comments! Please take a look at the new patch. https://codereview.webrtc.org/2356763002/diff/40001/webrtc/modules/audio_coding/audio_network_adaptor/audio_network_adaptor_impl.cc ...
4 years, 3 months ago (2016-09-22 15:11:49 UTC) #13
minyue-webrtc
two additional self-comments on the new patch set. https://codereview.webrtc.org/2356763002/diff/60001/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/2356763002/diff/60001/webrtc/modules/audio_coding/audio_network_adaptor/audio_network_adaptor_impl.h#newcode28 webrtc/modules/audio_coding/audio_network_adaptor/audio_network_adaptor_impl.h:28: explicit ...
4 years, 3 months ago (2016-09-22 15:20:49 UTC) #14
minyue-webrtc
PS2 has some errors, if you have not started reviewing it, take PS3 instead. Thanks!
4 years, 3 months ago (2016-09-22 15:31:35 UTC) #15
kwiberg-webrtc
lgtm Comments are optional. https://codereview.webrtc.org/2356763002/diff/40001/webrtc/modules/audio_coding/audio_network_adaptor/controller.h File webrtc/modules/audio_coding/audio_network_adaptor/controller.h (right): https://codereview.webrtc.org/2356763002/diff/40001/webrtc/modules/audio_coding/audio_network_adaptor/controller.h#newcode27 webrtc/modules/audio_coding/audio_network_adaptor/controller.h:27: rtc::Optional<int> rtt_ms; On 2016/09/22 15:11:49, ...
4 years, 3 months ago (2016-09-22 15:54:16 UTC) #16
michaelt
lgtm
4 years, 3 months ago (2016-09-22 15:56:09 UTC) #17
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/2356763002/100001
4 years, 3 months ago (2016-09-22 17:50:46 UTC) #20
commit-bot: I haz the power
Try jobs failed on following builders: mac_gyp_dbg on master.tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/mac_gyp_dbg/builds/886) mac_gyp_rel on master.tryserver.webrtc (JOB_FAILED, ...
4 years, 3 months ago (2016-09-22 17:52:19 UTC) #22
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/2356763002/140001
4 years, 3 months ago (2016-09-22 18:45:28 UTC) #26
commit-bot: I haz the power
Committed patchset #5 (id:140001)
4 years, 3 months ago (2016-09-22 20:39:17 UTC) #28
commit-bot: I haz the power
Patchset 5 (id:??) landed as https://crrev.com/7e4f8928062afc8d571bb69f3223711701cbaad6 Cr-Commit-Position: refs/heads/master@{#14361}
4 years, 3 months ago (2016-09-22 20:39:25 UTC) #30
minyue-webrtc
https://codereview.webrtc.org/2356763002/diff/80001/webrtc/modules/audio_coding/audio_network_adaptor/debug_dump_writer.cc File webrtc/modules/audio_coding/audio_network_adaptor/debug_dump_writer.cc (right): https://codereview.webrtc.org/2356763002/diff/80001/webrtc/modules/audio_coding/audio_network_adaptor/debug_dump_writer.cc#newcode59 webrtc/modules/audio_coding/audio_network_adaptor/debug_dump_writer.cc:59: RTC_DISALLOW_COPY_AND_ASSIGN(DebugDumpWriterImpl); On 2016/09/22 15:54:16, kwiberg-webrtc wrote: > You can ...
4 years, 3 months ago (2016-09-22 20:40:32 UTC) #31
minyue-webrtc
4 years, 3 months ago (2016-09-22 21:16:44 UTC) #32
Message was sent while issue was closed.
A revert of this CL (patchset #5 id:140001) has been created in
https://codereview.webrtc.org/2362003002/ by minyue@webrtc.org.

The reason for reverting is: Chromium bot fails.

Powered by Google App Engine
This is Rietveld 408576698