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

Issue 1267683002: Hooked up RtcEventLog. It lives in Voice Engine and pointers are propagated to ACM and Call. (Closed)

Created:
5 years, 4 months ago by ivoc
Modified:
5 years, 3 months ago
CC:
webrtc-reviews_webrtc.org, interface-changes_webrtc.org, rwolff_gocast.it, yujie_mao (webrtc), stefan-webrtc, Andrew MacDonald, henrika_webrtc, hlundin-webrtc, tlegrand-webrtc, tterriberry_mozilla.com, qiang.lu, niklas.enbom, mflodman, kwiberg-webrtc, perkj_webrtc, andresp
Base URL:
https://chromium.googlesource.com/external/webrtc.git@master
Target Ref:
refs/pending/heads/master
Project:
webrtc
Visibility:
Public.

Description

Hooked up RtcEventLog. It lives in Voice Engine and pointers are propagated to ACM and Call. An option was added to voe_cmd_test to make a RtcEventLog dump. BUG=webrtc:4741 Committed: https://crrev.com/b04965ccf83c2bc6e2758abab9bea0c18551a54c Cr-Commit-Position: refs/heads/master@{#9901}

Patch Set 1 : Initial version #

Total comments: 2

Patch Set 2 : Added nullptr check after call to GetInterface(), as suggested by Terelius@ #

Patch Set 3 : Moved GetEventLog from VoEBase to VoECodec, based on suggestion from Minyue@ #

Patch Set 4 : Added missing dependencies to gn/gyp build files and suppressed warning in ACM gn file. #

Patch Set 5 : Rebased to latest revision of webrtc to resolve patching conflict on try bots. #

Total comments: 19

Patch Set 6 : Addressed Henrik's review comments. #

Total comments: 2

Patch Set 7 : Processed more comments from Henrik. #

Total comments: 30

Patch Set 8 : All SetEventLog() functions have been removed, the pointer is now propagated through the constructo… #

Total comments: 12

Patch Set 9 : Moved RtcEventLog from SharedData to ChannelManager. #

Total comments: 5

Patch Set 10 : Added comment regarding pointer lifetime on the VoECodec sub-API. #

Total comments: 5

Patch Set 11 : Added a unittest in codec_test.cc, to test the integration in VoE. #

Total comments: 5

Patch Set 12 : Changed EXCEPT to ASSERT for the check to see if a logfile is created. #

Patch Set 13 : Rebase #

Total comments: 7

Patch Set 14 : Addressed Stefan's review comments. #

Patch Set 15 : Rebase #

Patch Set 16 : Another rebase #

Unified diffs Side-by-side diffs Delta from patch set Stats (+161 lines, -35 lines) Patch
M talk/media/webrtc/fakewebrtcvoiceengine.h View 1 2 3 4 5 6 7 8 9 10 11 12 1 chunk +1 line, -0 lines 0 comments Download
M webrtc/modules/audio_coding/BUILD.gn View 1 2 3 4 5 6 7 8 9 10 11 12 2 chunks +7 lines, -0 lines 0 comments Download
M webrtc/modules/audio_coding/main/acm2/audio_coding_module_impl.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 1 chunk +2 lines, -0 lines 0 comments Download
M webrtc/modules/audio_coding/main/acm2/audio_coding_module_impl.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 3 chunks +7 lines, -1 line 0 comments Download
M webrtc/modules/audio_coding/main/audio_coding_module.gypi View 1 2 3 4 1 chunk +1 line, -0 lines 0 comments Download
M webrtc/modules/audio_coding/main/interface/audio_coding_module.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 2 chunks +6 lines, -3 lines 0 comments Download
M webrtc/video/call.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 8 chunks +39 lines, -9 lines 0 comments Download
M webrtc/video/webrtc_video.gypi View 1 2 3 1 chunk +1 line, -0 lines 0 comments Download
M webrtc/voice_engine/BUILD.gn View 1 2 3 1 chunk +1 line, -0 lines 0 comments Download
M webrtc/voice_engine/channel.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 2 chunks +6 lines, -1 line 0 comments Download
M webrtc/voice_engine/channel.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 6 chunks +22 lines, -17 lines 0 comments Download
M webrtc/voice_engine/channel_manager.h View 1 2 3 4 5 6 7 8 3 chunks +5 lines, -0 lines 0 comments Download
M webrtc/voice_engine/channel_manager.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 3 chunks +8 lines, -2 lines 0 comments Download
M webrtc/voice_engine/include/voe_codec.h View 1 2 3 4 5 6 7 8 9 2 chunks +5 lines, -0 lines 0 comments Download
M webrtc/voice_engine/test/auto_test/standard/codec_test.cc View 1 2 3 4 5 6 7 8 9 10 11 2 chunks +30 lines, -0 lines 0 comments Download
M webrtc/voice_engine/test/cmd_test/voe_cmd_test.cc View 1 2 3 chunks +6 lines, -1 line 0 comments Download
M webrtc/voice_engine/voe_base_impl.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 1 chunk +0 lines, -1 line 0 comments Download
M webrtc/voice_engine/voe_codec_impl.h View 1 2 1 chunk +2 lines, -0 lines 0 comments Download
M webrtc/voice_engine/voe_codec_impl.cc View 1 2 3 4 5 6 7 8 1 chunk +4 lines, -0 lines 0 comments Download
M webrtc/voice_engine/voice_engine.gyp View 1 2 3 4 5 6 7 8 9 10 11 12 4 chunks +8 lines, -0 lines 0 comments Download

Messages

Total messages: 83 (21 generated)
ivoc
Please have a look at this CL to hook up RtcEventLog.
5 years, 4 months ago (2015-07-30 12:06:55 UTC) #2
terelius
Nice to have this hooked up! https://codereview.webrtc.org/1267683002/diff/1/webrtc/video/call.cc File webrtc/video/call.cc (right): https://codereview.webrtc.org/1267683002/diff/1/webrtc/video/call.cc#newcode182 webrtc/video/call.cc:182: voe_base->Release(); I don't ...
5 years, 4 months ago (2015-07-30 12:34:43 UTC) #3
ivoc
https://codereview.webrtc.org/1267683002/diff/1/webrtc/video/call.cc File webrtc/video/call.cc (right): https://codereview.webrtc.org/1267683002/diff/1/webrtc/video/call.cc#newcode182 webrtc/video/call.cc:182: voe_base->Release(); On 2015/07/30 12:34:43, terelius wrote: > I don't ...
5 years, 4 months ago (2015-07-30 15:10:23 UTC) #4
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1267683002/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1267683002/40001
5 years, 4 months ago (2015-08-03 07:06:20 UTC) #6
commit-bot: I haz the power
Dry run: Try jobs failed on following builders: linux_gn_rel on tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/linux_gn_rel/builds/4818)
5 years, 4 months ago (2015-08-03 07:07:50 UTC) #8
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1267683002/60001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1267683002/60001
5 years, 4 months ago (2015-08-03 12:51:07 UTC) #10
commit-bot: I haz the power
Dry run: Try jobs failed on following builders: android_clang on tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/android_clang/builds/6860) android_gn_rel on ...
5 years, 4 months ago (2015-08-03 12:52:02 UTC) #12
hlundin-webrtc
Looks good in general. I only have a few minor comments. https://codereview.webrtc.org/1267683002/diff/80001/webrtc/modules/audio_coding/BUILD.gn File webrtc/modules/audio_coding/BUILD.gn (right): ...
5 years, 4 months ago (2015-08-04 09:14:47 UTC) #13
ivoc
https://codereview.webrtc.org/1267683002/diff/80001/webrtc/modules/audio_coding/BUILD.gn File webrtc/modules/audio_coding/BUILD.gn (right): https://codereview.webrtc.org/1267683002/diff/80001/webrtc/modules/audio_coding/BUILD.gn#newcode62 webrtc/modules/audio_coding/BUILD.gn:62: # Suppress warnings from Chrome's Clang plugins. On 2015/08/04 ...
5 years, 4 months ago (2015-08-05 07:39:04 UTC) #14
hlundin-webrtc
Good. A few more comments, then I will be happy. https://codereview.webrtc.org/1267683002/diff/80001/webrtc/video/call.cc File webrtc/video/call.cc (right): https://codereview.webrtc.org/1267683002/diff/80001/webrtc/video/call.cc#newcode529 ...
5 years, 4 months ago (2015-08-05 08:32:13 UTC) #15
ivoc
https://codereview.webrtc.org/1267683002/diff/80001/webrtc/video/call.cc File webrtc/video/call.cc (right): https://codereview.webrtc.org/1267683002/diff/80001/webrtc/video/call.cc#newcode529 webrtc/video/call.cc:529: event_log_->LogRtcpPacket(false, media_type, packet, length); On 2015/08/05 08:32:13, hlundin-webrtc wrote: ...
5 years, 4 months ago (2015-08-05 09:12:41 UTC) #16
hlundin-webrtc
webrtc/modules/audio_coding: lgtm
5 years, 4 months ago (2015-08-05 09:17:09 UTC) #17
Henrik Grunell WebRTC
https://codereview.webrtc.org/1267683002/diff/120001/webrtc/voice_engine/channel.h File webrtc/voice_engine/channel.h (right): https://codereview.webrtc.org/1267683002/diff/120001/webrtc/voice_engine/channel.h#newcode455 webrtc/voice_engine/channel.h:455: void SetEventLog(RtcEventLog* event_log); Use a separate function for disabling ...
5 years, 4 months ago (2015-08-05 09:44:36 UTC) #18
terelius
https://codereview.webrtc.org/1267683002/diff/120001/webrtc/modules/audio_coding/main/acm2/audio_coding_module_impl.cc File webrtc/modules/audio_coding/main/acm2/audio_coding_module_impl.cc (right): https://codereview.webrtc.org/1267683002/diff/120001/webrtc/modules/audio_coding/main/acm2/audio_coding_module_impl.cc#newcode150 webrtc/modules/audio_coding/main/acm2/audio_coding_module_impl.cc:150: event_log_ptr_crit_sect_(CriticalSectionWrapper::CreateCriticalSection()), I got the following comment from pbos@ concering ...
5 years, 4 months ago (2015-08-05 10:35:42 UTC) #19
hlundin-webrtc
https://codereview.webrtc.org/1267683002/diff/120001/webrtc/modules/audio_coding/main/acm2/audio_coding_module_impl.cc File webrtc/modules/audio_coding/main/acm2/audio_coding_module_impl.cc (right): https://codereview.webrtc.org/1267683002/diff/120001/webrtc/modules/audio_coding/main/acm2/audio_coding_module_impl.cc#newcode150 webrtc/modules/audio_coding/main/acm2/audio_coding_module_impl.cc:150: event_log_ptr_crit_sect_(CriticalSectionWrapper::CreateCriticalSection()), On 2015/08/05 10:35:42, terelius wrote: > I got ...
5 years, 4 months ago (2015-08-05 11:02:06 UTC) #20
ivoc
https://codereview.webrtc.org/1267683002/diff/120001/webrtc/modules/audio_coding/main/acm2/audio_coding_module_impl.cc File webrtc/modules/audio_coding/main/acm2/audio_coding_module_impl.cc (right): https://codereview.webrtc.org/1267683002/diff/120001/webrtc/modules/audio_coding/main/acm2/audio_coding_module_impl.cc#newcode150 webrtc/modules/audio_coding/main/acm2/audio_coding_module_impl.cc:150: event_log_ptr_crit_sect_(CriticalSectionWrapper::CreateCriticalSection()), On 2015/08/05 11:02:06, hlundin-webrtc wrote: > On 2015/08/05 ...
5 years, 4 months ago (2015-08-06 10:57:52 UTC) #21
Henrik Grunell WebRTC
https://codereview.webrtc.org/1267683002/diff/120001/webrtc/voice_engine/channel.cc File webrtc/voice_engine/channel.cc (right): https://codereview.webrtc.org/1267683002/diff/120001/webrtc/voice_engine/channel.cc#newcode3457 webrtc/voice_engine/channel.cc:3457: void Channel::SetEventLog(RtcEventLog* event_log) { Where is RtcEventLog defined? https://codereview.webrtc.org/1267683002/diff/120001/webrtc/voice_engine/include/voe_codec.h ...
5 years, 4 months ago (2015-08-06 15:30:45 UTC) #22
Henrik Grunell WebRTC
https://codereview.webrtc.org/1267683002/diff/120001/webrtc/voice_engine/channel.cc File webrtc/voice_engine/channel.cc (right): https://codereview.webrtc.org/1267683002/diff/120001/webrtc/voice_engine/channel.cc#newcode3457 webrtc/voice_engine/channel.cc:3457: void Channel::SetEventLog(RtcEventLog* event_log) { On 2015/08/06 15:30:45, Henrik Grunell ...
5 years, 4 months ago (2015-08-07 11:30:03 UTC) #23
terelius
https://codereview.webrtc.org/1267683002/diff/120001/webrtc/modules/audio_coding/main/acm2/audio_coding_module_impl.cc File webrtc/modules/audio_coding/main/acm2/audio_coding_module_impl.cc (right): https://codereview.webrtc.org/1267683002/diff/120001/webrtc/modules/audio_coding/main/acm2/audio_coding_module_impl.cc#newcode744 webrtc/modules/audio_coding/main/acm2/audio_coding_module_impl.cc:744: if (event_log_) On 2015/08/06 10:57:51, ivoc wrote: > On ...
5 years, 4 months ago (2015-08-11 09:04:34 UTC) #24
terelius
I fear we are over-designing this. Right now we have an abstract base, but no ...
5 years, 4 months ago (2015-08-11 09:54:01 UTC) #25
ivoc
Thanks for the feedback. I agree with the general sentiment of your comment, so I ...
5 years, 4 months ago (2015-08-11 12:29:27 UTC) #26
Henrik Grunell WebRTC
https://codereview.webrtc.org/1267683002/diff/120001/webrtc/voice_engine/channel.h File webrtc/voice_engine/channel.h (right): https://codereview.webrtc.org/1267683002/diff/120001/webrtc/voice_engine/channel.h#newcode455 webrtc/voice_engine/channel.h:455: void SetEventLog(RtcEventLog* event_log); On 2015/08/11 09:04:34, terelius wrote: > ...
5 years, 4 months ago (2015-08-12 12:38:51 UTC) #27
Henrik Grunell WebRTC
https://codereview.webrtc.org/1267683002/diff/140001/webrtc/voice_engine/channel_manager.cc File webrtc/voice_engine/channel_manager.cc (right): https://codereview.webrtc.org/1267683002/diff/140001/webrtc/voice_engine/channel_manager.cc#newcode63 webrtc/voice_engine/channel_manager.cc:63: RtcEventLog* const event_log) { Will we ever use another ...
5 years, 4 months ago (2015-08-12 12:43:45 UTC) #28
terelius
https://codereview.webrtc.org/1267683002/diff/120001/webrtc/voice_engine/shared_data.cc File webrtc/voice_engine/shared_data.cc (right): https://codereview.webrtc.org/1267683002/diff/120001/webrtc/voice_engine/shared_data.cc#newcode34 webrtc/voice_engine/shared_data.cc:34: _eventLog(RtcEventLog::Create()), On 2015/08/12 12:38:50, Henrik Grunell (webrtc) wrote: > ...
5 years, 4 months ago (2015-08-12 13:05:05 UTC) #29
ivoc
All the SetEventLog functions have been removed now, the pointer to the RtcEventLog object is ...
5 years, 4 months ago (2015-08-12 13:16:41 UTC) #30
Henrik Grunell WebRTC
I can't find any tests for this except the command line test. We need some ...
5 years, 4 months ago (2015-08-12 15:29:53 UTC) #31
ivoc
There are some pretty extensive unittests for the RtcEventLog class in webrtc/video/rtc_event_log_unittest.cc . Do you ...
5 years, 4 months ago (2015-08-13 07:49:24 UTC) #32
Henrik Grunell WebRTC
https://codereview.webrtc.org/1267683002/diff/140001/webrtc/voice_engine/channel_manager.cc File webrtc/voice_engine/channel_manager.cc (right): https://codereview.webrtc.org/1267683002/diff/140001/webrtc/voice_engine/channel_manager.cc#newcode63 webrtc/voice_engine/channel_manager.cc:63: RtcEventLog* const event_log) { On 2015/08/13 07:49:23, ivoc wrote: ...
5 years, 4 months ago (2015-08-13 08:13:55 UTC) #33
ivoc
https://codereview.webrtc.org/1267683002/diff/140001/webrtc/voice_engine/channel_manager.cc File webrtc/voice_engine/channel_manager.cc (right): https://codereview.webrtc.org/1267683002/diff/140001/webrtc/voice_engine/channel_manager.cc#newcode63 webrtc/voice_engine/channel_manager.cc:63: RtcEventLog* const event_log) { On 2015/08/13 08:13:55, Henrik Grunell ...
5 years, 4 months ago (2015-08-13 13:33:43 UTC) #34
terelius
Ivo, do you have any more changes pending to this CL? https://codereview.webrtc.org/1267683002/diff/140001/webrtc/modules/audio_coding/BUILD.gn File webrtc/modules/audio_coding/BUILD.gn (right): ...
5 years, 4 months ago (2015-08-14 13:00:44 UTC) #35
Henrik Grunell WebRTC
Final two comments. https://codereview.webrtc.org/1267683002/diff/160001/webrtc/voice_engine/include/voe_codec.h File webrtc/voice_engine/include/voe_codec.h (right): https://codereview.webrtc.org/1267683002/diff/160001/webrtc/voice_engine/include/voe_codec.h#newcode136 webrtc/voice_engine/include/voe_codec.h:136: // Engine. Also document the lifetime ...
5 years, 4 months ago (2015-08-14 13:11:20 UTC) #36
ivoc
No changes are pending on my side for this CL, I'm merely trying to appease ...
5 years, 4 months ago (2015-08-14 13:23:13 UTC) #37
Henrik Grunell WebRTC
I think you should add a test in voice_engine/test/auto_test/standard/codec_test.cc. At least check that we get ...
5 years, 4 months ago (2015-08-14 13:41:44 UTC) #38
terelius
https://codereview.webrtc.org/1267683002/diff/180001/webrtc/modules/audio_coding/main/interface/audio_coding_module.h File webrtc/modules/audio_coding/main/interface/audio_coding_module.h (right): https://codereview.webrtc.org/1267683002/diff/180001/webrtc/modules/audio_coding/main/interface/audio_coding_module.h#newcode94 webrtc/modules/audio_coding/main/interface/audio_coding_module.h:94: RtcEventLog* event_log; What's the difference between AudioCodingModule and AudioCodingModuleImpl? ...
5 years, 4 months ago (2015-08-14 19:12:48 UTC) #39
hlundin-webrtc
https://codereview.webrtc.org/1267683002/diff/180001/webrtc/modules/audio_coding/main/interface/audio_coding_module.h File webrtc/modules/audio_coding/main/interface/audio_coding_module.h (right): https://codereview.webrtc.org/1267683002/diff/180001/webrtc/modules/audio_coding/main/interface/audio_coding_module.h#newcode94 webrtc/modules/audio_coding/main/interface/audio_coding_module.h:94: RtcEventLog* event_log; On 2015/08/14 19:12:47, terelius wrote: > What's ...
5 years, 4 months ago (2015-08-17 11:04:02 UTC) #40
ivoc
I added a simple unittest to codec_test.cc, it tests if the GetEventLog() function returns a ...
5 years, 4 months ago (2015-08-21 11:45:52 UTC) #41
terelius
lgtm https://codereview.webrtc.org/1267683002/diff/180001/webrtc/modules/audio_coding/main/interface/audio_coding_module.h File webrtc/modules/audio_coding/main/interface/audio_coding_module.h (right): https://codereview.webrtc.org/1267683002/diff/180001/webrtc/modules/audio_coding/main/interface/audio_coding_module.h#newcode94 webrtc/modules/audio_coding/main/interface/audio_coding_module.h:94: RtcEventLog* event_log; On 2015/08/21 11:45:51, ivoc wrote: > ...
5 years, 4 months ago (2015-08-21 11:50:34 UTC) #42
hlundin-webrtc
Still lgtm.
5 years, 4 months ago (2015-08-21 12:09:43 UTC) #43
Henrik Grunell WebRTC
https://codereview.webrtc.org/1267683002/diff/200001/webrtc/voice_engine/test/auto_test/standard/codec_test.cc File webrtc/voice_engine/test/auto_test/standard/codec_test.cc (right): https://codereview.webrtc.org/1267683002/diff/200001/webrtc/voice_engine/test/auto_test/standard/codec_test.cc#newcode204 webrtc/voice_engine/test/auto_test/standard/codec_test.cc:204: event_log->StopLogging(); Is there maybe any VoE function call that ...
5 years, 4 months ago (2015-08-24 09:03:03 UTC) #44
ivoc
https://codereview.webrtc.org/1267683002/diff/200001/webrtc/voice_engine/test/auto_test/standard/codec_test.cc File webrtc/voice_engine/test/auto_test/standard/codec_test.cc (right): https://codereview.webrtc.org/1267683002/diff/200001/webrtc/voice_engine/test/auto_test/standard/codec_test.cc#newcode204 webrtc/voice_engine/test/auto_test/standard/codec_test.cc:204: event_log->StopLogging(); On 2015/08/24 09:03:03, Henrik Grunell (webrtc) wrote: > ...
5 years, 4 months ago (2015-08-24 12:17:29 UTC) #45
Henrik Grunell WebRTC
LGTM. Thanks for adding the test! https://codereview.webrtc.org/1267683002/diff/200001/webrtc/voice_engine/test/auto_test/standard/codec_test.cc File webrtc/voice_engine/test/auto_test/standard/codec_test.cc (right): https://codereview.webrtc.org/1267683002/diff/200001/webrtc/voice_engine/test/auto_test/standard/codec_test.cc#newcode204 webrtc/voice_engine/test/auto_test/standard/codec_test.cc:204: event_log->StopLogging(); On 2015/08/24 ...
5 years, 4 months ago (2015-08-24 12:23:34 UTC) #46
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1267683002/220001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1267683002/220001
5 years, 3 months ago (2015-08-26 14:03:16 UTC) #49
commit-bot: I haz the power
Dry run: Try jobs failed on following builders: linux on tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/linux/builds/9247) linux_gn on ...
5 years, 3 months ago (2015-08-26 14:03:58 UTC) #51
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1267683002/240001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1267683002/240001
5 years, 3 months ago (2015-08-26 14:56:00 UTC) #53
commit-bot: I haz the power
Dry run: Try jobs failed on following builders: android on tryserver.webrtc (JOB_TIMED_OUT, no build URL) ...
5 years, 3 months ago (2015-08-26 16:56:13 UTC) #55
stefan-webrtc
https://codereview.webrtc.org/1267683002/diff/240001/webrtc/video/call.cc File webrtc/video/call.cc (right): https://codereview.webrtc.org/1267683002/diff/240001/webrtc/video/call.cc#newcode529 webrtc/video/call.cc:529: event_log_->LogRtcpPacket(false, media_type, packet, length); Maybe move these two logs ...
5 years, 3 months ago (2015-09-07 11:04:37 UTC) #56
ivoc
Thanks for the comments. See my responses below. https://codereview.webrtc.org/1267683002/diff/240001/webrtc/video/call.cc File webrtc/video/call.cc (right): https://codereview.webrtc.org/1267683002/diff/240001/webrtc/video/call.cc#newcode529 webrtc/video/call.cc:529: event_log_->LogRtcpPacket(false, ...
5 years, 3 months ago (2015-09-07 11:51:14 UTC) #57
stefan-webrtc
lgtm
5 years, 3 months ago (2015-09-07 12:16:49 UTC) #58
terelius
https://codereview.webrtc.org/1267683002/diff/240001/webrtc/video/call.cc File webrtc/video/call.cc (right): https://codereview.webrtc.org/1267683002/diff/240001/webrtc/video/call.cc#newcode529 webrtc/video/call.cc:529: event_log_->LogRtcpPacket(false, media_type, packet, length); On 2015/09/07 11:51:14, ivoc wrote: ...
5 years, 3 months ago (2015-09-07 12:32:44 UTC) #59
stefan-webrtc
https://codereview.webrtc.org/1267683002/diff/240001/webrtc/video/call.cc File webrtc/video/call.cc (right): https://codereview.webrtc.org/1267683002/diff/240001/webrtc/video/call.cc#newcode548 webrtc/video/call.cc:548: event_log_->LogRtpHeader(true, media_type, packet, 20, length); On 2015/09/07 12:32:44, terelius ...
5 years, 3 months ago (2015-09-07 12:42:37 UTC) #60
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1267683002/260001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1267683002/260001
5 years, 3 months ago (2015-09-07 14:19:04 UTC) #62
commit-bot: I haz the power
Dry run: Try jobs failed on following builders: android on tryserver.webrtc (JOB_TIMED_OUT, no build URL) ...
5 years, 3 months ago (2015-09-07 16:19:21 UTC) #64
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1267683002/260001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1267683002/260001
5 years, 3 months ago (2015-09-08 07:45:46 UTC) #67
commit-bot: I haz the power
Try jobs failed on following builders: presubmit on tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/presubmit/builds/656)
5 years, 3 months ago (2015-09-08 07:49:31 UTC) #69
pbos-webrtc
lgtm for talk/
5 years, 3 months ago (2015-09-08 08:42:26 UTC) #70
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1267683002/280001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1267683002/280001
5 years, 3 months ago (2015-09-08 09:47:09 UTC) #72
commit-bot: I haz the power
Dry run: Try jobs failed on following builders: android on tryserver.webrtc (JOB_TIMED_OUT, no build URL) ...
5 years, 3 months ago (2015-09-08 11:47:34 UTC) #74
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1267683002/300001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1267683002/300001
5 years, 3 months ago (2015-09-08 14:54:13 UTC) #76
commit-bot: I haz the power
Dry run: Try jobs failed on following builders: android on tryserver.webrtc (JOB_TIMED_OUT, no build URL) ...
5 years, 3 months ago (2015-09-08 16:54:29 UTC) #78
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1267683002/300001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1267683002/300001
5 years, 3 months ago (2015-09-09 07:08:38 UTC) #81
commit-bot: I haz the power
Committed patchset #16 (id:300001)
5 years, 3 months ago (2015-09-09 07:09:48 UTC) #82
commit-bot: I haz the power
5 years, 3 months ago (2015-09-09 07:10:08 UTC) #83
Message was sent while issue was closed.
Patchset 16 (id:??) landed as
https://crrev.com/b04965ccf83c2bc6e2758abab9bea0c18551a54c
Cr-Commit-Position: refs/heads/master@{#9901}

Powered by Google App Engine
This is Rietveld 408576698