|
|
Chromium Code Reviews|
Created:
4 years, 3 months ago by åsapersson Modified:
4 years, 3 months ago CC:
webrtc-reviews_webrtc.org, video-team_agora.io, yujie_mao (webrtc), zhengzhonghou_agora.io, stefan-webrtc, tterriberry_mozilla.com, perkj_webrtc, mflodman Target Ref:
refs/pending/heads/master Project:
webrtc Visibility:
Public. |
DescriptionAdd periodic logging of number of captured and dropped frames in VideoCaptureInput. Logged every minute.
BUG=
Committed: https://crrev.com/6ffb67d049e83e958a8cf90fc01395a60efdee58
Cr-Commit-Position: refs/heads/master@{#14179}
Patch Set 1 #
Total comments: 4
Patch Set 2 : address comment #Patch Set 3 : rebase, move logging from video_capture_input.cc to vie_encoder.cc #
Total comments: 4
Patch Set 4 : address comments #Patch Set 5 : rebase #Messages
Total messages: 29 (16 generated)
Description was changed from ========== Add periodic logging of number of dropped and encoded frames in VideoCaptureInput. Logged every minute. BUG= ========== to ========== Add periodic logging of number of dropped and encoded frames in VideoCaptureInput. Logged every minute. BUG= ==========
asapersson@webrtc.org changed reviewers: + mflodman@webrtc.org, mnilsson@chromium.org
Patchset #1 (id:1) has been deleted
Description was changed from ========== Add periodic logging of number of dropped and encoded frames in VideoCaptureInput. Logged every minute. BUG= ========== to ========== Add periodic logging of number of captured and dropped frames in VideoCaptureInput. Logged every minute. BUG= ==========
Patchset #1 (id:20001) has been deleted
https://codereview.webrtc.org/2298213002/diff/40001/webrtc/video/video_captur... File webrtc/video/video_capture_input.cc (right): https://codereview.webrtc.org/2298213002/diff/40001/webrtc/video/video_captur... webrtc/video/video_capture_input.cc:107: LOG(LS_INFO) << "Number of frames: captured " << captured_frame_count_ I'd prefer to log the number of frames for the last minute, then restart from zero to make it easier to compare log lines in test for longer calls. Mats, wdyt?
https://codereview.webrtc.org/2298213002/diff/40001/webrtc/video/video_captur... File webrtc/video/video_capture_input.cc (right): https://codereview.webrtc.org/2298213002/diff/40001/webrtc/video/video_captur... webrtc/video/video_capture_input.cc:107: LOG(LS_INFO) << "Number of frames: captured " << captured_frame_count_ On 2016/09/02 08:46:02, mflodman wrote: > I'd prefer to log the number of frames for the last minute, then restart from > zero to make it easier to compare log lines in test for longer calls. > > Mats, > wdyt? Yes, resetting the count will make it easier to see the behavior for the last window. We can also more easily aggregate several windows for a longer average.
https://codereview.webrtc.org/2298213002/diff/40001/webrtc/video/video_captur... File webrtc/video/video_capture_input.cc (right): https://codereview.webrtc.org/2298213002/diff/40001/webrtc/video/video_captur... webrtc/video/video_capture_input.cc:107: LOG(LS_INFO) << "Number of frames: captured " << captured_frame_count_ On 2016/09/02 09:21:10, mnilsson wrote: > On 2016/09/02 08:46:02, mflodman wrote: > > I'd prefer to log the number of frames for the last minute, then restart from > > zero to make it easier to compare log lines in test for longer calls. > > > > Mats, > > wdyt? > > Yes, resetting the count will make it easier to see the behavior for the last > window. We can also more easily aggregate several windows for a longer average. Done. https://codereview.webrtc.org/2298213002/diff/40001/webrtc/video/video_captur... webrtc/video/video_capture_input.cc:107: LOG(LS_INFO) << "Number of frames: captured " << captured_frame_count_ On 2016/09/02 08:46:02, mflodman wrote: > I'd prefer to log the number of frames for the last minute, then restart from > zero to make it easier to compare log lines in test for longer calls. > > Mats, > wdyt? Done.
lgtm Thanks!
Patchset #3 (id:80001) has been deleted
asapersson@webrtc.org changed reviewers: + perkj@webrtc.org
lgtm https://codereview.webrtc.org/2298213002/diff/100001/webrtc/video/vie_encoder.cc File webrtc/video/vie_encoder.cc (right): https://codereview.webrtc.org/2298213002/diff/100001/webrtc/video/vie_encoder... webrtc/video/vie_encoder.cc:30: const int64_t kFrameLogIntervalMs = 60000; Can you please add a comment about what this does? https://codereview.webrtc.org/2298213002/diff/100001/webrtc/video/vie_encoder... webrtc/video/vie_encoder.cc:223: << vie_encoder_->captured_frame_count_ << ", dropped " add the reason why the frames are dropped? A frame can be dropped later in the pipeline as well right?
Patchset #4 (id:120001) has been deleted
Patchset #4 (id:140001) has been deleted
https://codereview.webrtc.org/2298213002/diff/100001/webrtc/video/vie_encoder.cc File webrtc/video/vie_encoder.cc (right): https://codereview.webrtc.org/2298213002/diff/100001/webrtc/video/vie_encoder... webrtc/video/vie_encoder.cc:30: const int64_t kFrameLogIntervalMs = 60000; On 2016/09/08 07:34:40, perkj_webrtc wrote: > Can you please add a comment about what this does? Done. https://codereview.webrtc.org/2298213002/diff/100001/webrtc/video/vie_encoder... webrtc/video/vie_encoder.cc:223: << vie_encoder_->captured_frame_count_ << ", dropped " On 2016/09/08 07:34:40, perkj_webrtc wrote: > add the reason why the frames are dropped? A frame can be dropped later in the > pipeline as well right? > Done.
lgtm
The CQ bit was checked by asapersson@webrtc.org
The patchset sent to the CQ was uploaded after l-g-t-m from mnilsson@chromium.org, perkj@webrtc.org Link to the patchset: https://codereview.webrtc.org/2298213002/#ps160001 (title: "address comments")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: ios64_gyp_dbg on master.tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/ios64_gyp_dbg/builds/358) ios64_gyp_rel on master.tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/ios64_gyp_rel/builds/358) ios_rel on master.tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/ios_rel/builds/16667) linux_libfuzzer_rel on master.tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/linux_libfuzzer_rel/bui...) mac_compile_dbg on master.tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/mac_compile_dbg/builds/...)
The CQ bit was checked by asapersson@webrtc.org
The patchset sent to the CQ was uploaded after l-g-t-m from mnilsson@chromium.org, perkj@webrtc.org, mflodman@webrtc.org Link to the patchset: https://codereview.webrtc.org/2298213002/#ps180001 (title: "rebase")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...
Message was sent while issue was closed.
Description was changed from ========== Add periodic logging of number of captured and dropped frames in VideoCaptureInput. Logged every minute. BUG= ========== to ========== Add periodic logging of number of captured and dropped frames in VideoCaptureInput. Logged every minute. BUG= ==========
Message was sent while issue was closed.
Committed patchset #5 (id:180001)
Message was sent while issue was closed.
Description was changed from ========== Add periodic logging of number of captured and dropped frames in VideoCaptureInput. Logged every minute. BUG= ========== to ========== Add periodic logging of number of captured and dropped frames in VideoCaptureInput. Logged every minute. BUG= Committed: https://crrev.com/6ffb67d049e83e958a8cf90fc01395a60efdee58 Cr-Commit-Position: refs/heads/master@{#14179} ==========
Message was sent while issue was closed.
Patchset 5 (id:??) landed as https://crrev.com/6ffb67d049e83e958a8cf90fc01395a60efdee58 Cr-Commit-Position: refs/heads/master@{#14179} |
