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

Issue 2918143003: Set overuse detector max frame interval based on target frame rate. (Closed)

Created:
3 years, 6 months ago by sprang_webrtc
Modified:
3 years, 6 months ago
Reviewers:
kthelgason, åsapersson
CC:
webrtc-reviews_webrtc.org, video-team_agora.io, yujie_mao (webrtc), zhengzhonghou_agora.io, stefan-webrtc, tterriberry_mozilla.com, mflodman
Target Ref:
refs/heads/master
Project:
webrtc
Visibility:
Public.

Description

Set overuse detector max frame interval based on target frame rate. Currently there is a hard limit for the estimated captured frame interval of 45ms. As the encoder utilization is calculated as (input frame interval)/(encode time), overuse signals can be triggered even though there is plenty of time to go around if the fps is low. However, in order to avoid falsly estimating low encode usage in case the capturer has a dynamic frame rate, set the frame interval based on the actual current max framerate. BUG=webrtc:4172 Review-Url: https://codereview.webrtc.org/2918143003 Cr-Commit-Position: refs/heads/master@{#18610} Committed: https://chromium.googlesource.com/external/webrtc/+/fda496a31e5c4d0aac98d3309eb288495748ba8f

Patch Set 1 #

Patch Set 2 : Rebase #

Patch Set 3 : Botched rebase #

Patch Set 4 : Bug fixes #

Patch Set 5 : Added tests #

Patch Set 6 : Rebase / merge fixes #

Patch Set 7 : Handle degradation preference change #

Total comments: 28

Patch Set 8 : Addressed comments, fixed last adaptation fps, fixed unit tests #

Total comments: 2

Patch Set 9 : Updates max fps handling #

Patch Set 10 : Active SinkWants #

Total comments: 4

Patch Set 11 : Cleanup #

Patch Set 12 : Rebase #

Unified diffs Side-by-side diffs Delta from patch set Stats (+499 lines, -79 lines) Patch
M webrtc/video/encoder_rtcp_feedback_unittest.cc View 1 2 3 4 5 6 7 2 chunks +4 lines, -1 line 0 comments Download
M webrtc/video/overuse_frame_detector.h View 1 2 3 4 5 6 7 3 chunks +9 lines, -1 line 0 comments Download
M webrtc/video/overuse_frame_detector.cc View 1 2 3 4 5 6 7 8 8 chunks +33 lines, -6 lines 0 comments Download
M webrtc/video/overuse_frame_detector_unittest.cc View 1 2 3 4 5 6 7 2 chunks +125 lines, -0 lines 0 comments Download
M webrtc/video/video_send_stream.cc View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +4 lines, -3 lines 0 comments Download
M webrtc/video/vie_encoder.h View 1 2 3 4 5 6 7 8 9 10 11 4 chunks +8 lines, -2 lines 0 comments Download
M webrtc/video/vie_encoder.cc View 1 2 3 4 5 6 7 8 9 10 11 18 chunks +111 lines, -62 lines 0 comments Download
M webrtc/video/vie_encoder_unittest.cc View 1 2 3 4 5 6 7 8 9 10 11 7 chunks +205 lines, -4 lines 0 comments Download

Messages

Total messages: 37 (24 generated)
sprang_webrtc
3 years, 6 months ago (2017-06-08 12:26:26 UTC) #12
sprang_webrtc
+kthelgason@ since asa is OOO
3 years, 6 months ago (2017-06-09 10:32:34 UTC) #14
kthelgason
A few comments. This seems like a good idea to me! https://codereview.webrtc.org/2918143003/diff/120001/webrtc/video/overuse_frame_detector.cc File webrtc/video/overuse_frame_detector.cc (right): ...
3 years, 6 months ago (2017-06-12 11:56:48 UTC) #15
åsapersson
https://codereview.webrtc.org/2918143003/diff/120001/webrtc/video/overuse_frame_detector.cc File webrtc/video/overuse_frame_detector.cc (right): https://codereview.webrtc.org/2918143003/diff/120001/webrtc/video/overuse_frame_detector.cc#newcode414 webrtc/video/overuse_frame_detector.cc:414: framerate_ = framerate_fps; Add a max limit for framerate_ ...
3 years, 6 months ago (2017-06-13 12:15:09 UTC) #16
sprang_webrtc
https://codereview.webrtc.org/2918143003/diff/120001/webrtc/video/overuse_frame_detector.cc File webrtc/video/overuse_frame_detector.cc (right): https://codereview.webrtc.org/2918143003/diff/120001/webrtc/video/overuse_frame_detector.cc#newcode408 webrtc/video/overuse_frame_detector.cc:408: OnTargetFramerateUpdated(framerate_); On 2017/06/12 11:56:47, kthelgason wrote: > is this ...
3 years, 6 months ago (2017-06-14 08:39:17 UTC) #19
åsapersson
https://codereview.webrtc.org/2918143003/diff/120001/webrtc/video/overuse_frame_detector.cc File webrtc/video/overuse_frame_detector.cc (right): https://codereview.webrtc.org/2918143003/diff/120001/webrtc/video/overuse_frame_detector.cc#newcode414 webrtc/video/overuse_frame_detector.cc:414: framerate_ = framerate_fps; On 2017/06/14 08:39:16, sprang_webrtc wrote: > ...
3 years, 6 months ago (2017-06-14 10:50:26 UTC) #22
sprang_webrtc
https://codereview.webrtc.org/2918143003/diff/120001/webrtc/video/overuse_frame_detector.cc File webrtc/video/overuse_frame_detector.cc (right): https://codereview.webrtc.org/2918143003/diff/120001/webrtc/video/overuse_frame_detector.cc#newcode414 webrtc/video/overuse_frame_detector.cc:414: framerate_ = framerate_fps; On 2017/06/14 10:50:26, åsapersson wrote: > ...
3 years, 6 months ago (2017-06-14 13:41:28 UTC) #23
åsapersson
https://codereview.webrtc.org/2918143003/diff/180001/webrtc/video/vie_encoder.cc File webrtc/video/vie_encoder.cc (right): https://codereview.webrtc.org/2918143003/diff/180001/webrtc/video/vie_encoder.cc#newcode586 webrtc/video/vie_encoder.cc:586: if (current_fps_want != std::numeric_limits<int>::max()) { remove if statement above? ...
3 years, 6 months ago (2017-06-14 14:46:22 UTC) #24
sprang_webrtc
https://codereview.webrtc.org/2918143003/diff/180001/webrtc/video/vie_encoder.cc File webrtc/video/vie_encoder.cc (right): https://codereview.webrtc.org/2918143003/diff/180001/webrtc/video/vie_encoder.cc#newcode586 webrtc/video/vie_encoder.cc:586: if (current_fps_want != std::numeric_limits<int>::max()) { On 2017/06/14 14:46:22, åsapersson ...
3 years, 6 months ago (2017-06-14 15:29:56 UTC) #25
sprang_webrtc
3 years, 6 months ago (2017-06-14 15:30:28 UTC) #26
åsapersson
lgtm
3 years, 6 months ago (2017-06-15 08:50:45 UTC) #27
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/2918143003/220001
3 years, 6 months ago (2017-06-15 11:19:03 UTC) #34
commit-bot: I haz the power
3 years, 6 months ago (2017-06-15 11:21:16 UTC) #37
Message was sent while issue was closed.
Committed patchset #12 (id:220001) as
https://chromium.googlesource.com/external/webrtc/+/fda496a31e5c4d0aac98d3309...

Powered by Google App Engine
This is Rietveld 408576698