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

Issue 2781433002: Reland of Add framerate to VideoSinkWants and ability to signal on overuse (Closed)

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

Description

Reland of Add framerate to VideoSinkWants and ability to signal on overuse (patchset #1 id:1 of https://codereview.webrtc.org/2764133002/ ) Reason for revert: Found issue with test case, will add fix to reland cl. Original issue's description: > Revert of Add framerate to VideoSinkWants and ability to signal on overuse (patchset #14 id:250001 of https://codereview.webrtc.org/2716643002/ ) > > Reason for revert: > Breaks perf tests: > https://build.chromium.org/p/client.webrtc.perf/builders/Win7/builds/1679 > https://build.chromium.org/p/client.webrtc.perf/builders/Android32%20Tests%20%28L%20Nexus5%29/builds/2325 > > Original issue's description: > > Add framerate to VideoSinkWants and ability to signal on overuse > > > > In ViEEncoder, try to reduce framerate instead of resolution if the > > current degradation preference is maintain-resolution rather than > > balanced. > > > > BUG=webrtc:4172 > > > > Review-Url: https://codereview.webrtc.org/2716643002 > > Cr-Commit-Position: refs/heads/master@{#17327} > > Committed: https://chromium.googlesource.com/external/webrtc/+/72acf2526177bb4dbb5103cd6e165eb4361a5ae6 > > TBR=nisse@webrtc.org,magjed@webrtc.org,kthelgason@webrtc.org,ilnik@webrtc.org,stefan@webrtc.org,sprang@webrtc.org > # Skipping CQ checks because original CL landed less than 1 days ago. > NOPRESUBMIT=true > NOTREECHECKS=true > NOTRY=true > BUG=webrtc:4172 > > Review-Url: https://codereview.webrtc.org/2764133002 > Cr-Commit-Position: refs/heads/master@{#17331} > Committed: https://chromium.googlesource.com/external/webrtc/+/8b45b11144c968b4173215c76f78c710c9a2ed0b TBR=nisse@webrtc.org,magjed@webrtc.org,kthelgason@webrtc.org,ilnik@webrtc.org,stefan@webrtc.org,skvlad@webrtc.org # Not skipping CQ checks because original CL landed more than 1 days ago. BUG=webrtc:4172 Review-Url: https://codereview.webrtc.org/2781433002 Cr-Commit-Position: refs/heads/master@{#17474} Committed: https://chromium.googlesource.com/external/webrtc/+/3ea3c77e93121b1ab9d5e46641e6764f2cca0d51

Patch Set 1 #

Patch Set 2 : Rebase #

Patch Set 3 : Fix for failing CallPerfTest #

Patch Set 4 : Rmoved accidentally partially included unrelated diff #

Total comments: 2

Patch Set 5 : Time limit #

Total comments: 3

Patch Set 6 : DegradationPreference allows scaling wasn't handled porperly #

Patch Set 7 : Rebase #

Patch Set 8 : Rebase error, initial scaling fix #

Unified diffs Side-by-side diffs Delta from patch set Stats (+1356 lines, -397 lines) Patch
M webrtc/call/bitrate_estimator_tests.cc View 1 1 chunk +1 line, -1 line 0 comments Download
M webrtc/call/call_perf_tests.cc View 1 2 3 4 5 6 3 chunks +40 lines, -17 lines 0 comments Download
M webrtc/media/base/adaptedvideotracksource.cc View 1 1 chunk +2 lines, -2 lines 0 comments Download
M webrtc/media/base/fakevideocapturer.h View 1 2 chunks +8 lines, -4 lines 0 comments Download
M webrtc/media/base/fakevideorenderer.h View 1 chunk +1 line, -0 lines 0 comments Download
M webrtc/media/base/videoadapter.h View 1 3 chunks +14 lines, -7 lines 0 comments Download
M webrtc/media/base/videoadapter.cc View 1 4 chunks +25 lines, -10 lines 0 comments Download
M webrtc/media/base/videoadapter_unittest.cc View 1 29 chunks +117 lines, -50 lines 0 comments Download
M webrtc/media/base/videobroadcaster.cc View 1 2 chunks +8 lines, -6 lines 0 comments Download
M webrtc/media/base/videobroadcaster_unittest.cc View 1 2 chunks +29 lines, -6 lines 0 comments Download
M webrtc/media/base/videocapturer.cc View 1 1 chunk +3 lines, -2 lines 0 comments Download
M webrtc/media/base/videocapturer_unittest.cc View 1 3 chunks +4 lines, -5 lines 0 comments Download
M webrtc/media/base/videosourceinterface.h View 1 1 chunk +3 lines, -1 line 0 comments Download
M webrtc/media/engine/fakewebrtccall.h View 1 2 chunks +2 lines, -0 lines 0 comments Download
M webrtc/media/engine/fakewebrtccall.cc View 1 3 chunks +22 lines, -4 lines 0 comments Download
M webrtc/media/engine/webrtcvideoengine2.h View 1 1 chunk +3 lines, -0 lines 0 comments Download
M webrtc/media/engine/webrtcvideoengine2.cc View 1 2 3 4 5 6 3 chunks +24 lines, -23 lines 0 comments Download
M webrtc/media/engine/webrtcvideoengine2_unittest.cc View 1 2 3 4 5 6 8 chunks +53 lines, -23 lines 0 comments Download
M webrtc/test/BUILD.gn View 1 2 chunks +2 lines, -0 lines 0 comments Download
M webrtc/test/call_test.cc View 1 2 3 4 5 6 2 chunks +2 lines, -2 lines 0 comments Download
M webrtc/test/frame_generator_capturer.h View 1 2 3 4 5 6 2 chunks +3 lines, -1 line 0 comments Download
M webrtc/test/frame_generator_capturer.cc View 1 6 chunks +85 lines, -48 lines 0 comments Download
M webrtc/test/vcm_capturer.h View 1 1 chunk +2 lines, -0 lines 0 comments Download
M webrtc/test/vcm_capturer.cc View 1 6 chunks +16 lines, -12 lines 0 comments Download
M webrtc/test/video_capturer.h View 1 1 chunk +24 lines, -5 lines 0 comments Download
A webrtc/test/video_capturer.cc View 1 chunk +58 lines, -0 lines 0 comments Download
M webrtc/video/end_to_end_tests.cc View 1 2 3 4 5 6 4 chunks +6 lines, -4 lines 0 comments Download
M webrtc/video/overuse_frame_detector.h View 1 2 chunks +4 lines, -0 lines 0 comments Download
M webrtc/video/overuse_frame_detector.cc View 1 7 chunks +93 lines, -4 lines 0 comments Download
M webrtc/video/send_statistics_proxy.h View 1 2 3 2 chunks +2 lines, -1 line 0 comments Download
M webrtc/video/video_quality_test.h View 1 1 chunk +1 line, -1 line 0 comments Download
M webrtc/video/video_send_stream_tests.cc View 1 3 chunks +3 lines, -3 lines 0 comments Download
M webrtc/video/vie_encoder.h View 1 5 chunks +16 lines, -3 lines 0 comments Download
M webrtc/video/vie_encoder.cc View 1 2 3 4 5 6 7 11 chunks +265 lines, -75 lines 0 comments Download
M webrtc/video/vie_encoder_unittest.cc View 1 2 3 4 5 6 7 32 chunks +404 lines, -75 lines 0 comments Download
M webrtc/video_send_stream.h View 1 1 chunk +11 lines, -2 lines 0 comments Download

Messages

Total messages: 41 (26 generated)
sprang_webrtc
Created Reland of Add framerate to VideoSinkWants and ability to signal on overuse
3 years, 9 months ago (2017-03-27 07:45:30 UTC) #1
sprang_webrtc
ptal Only change since reverted cl (apart from rebase with screwup) is test case fixed ...
3 years, 9 months ago (2017-03-27 09:21:33 UTC) #8
nisse-webrtc
I don't quite understand this test, but change looks reasonable. I'm happy if someone more ...
3 years, 9 months ago (2017-03-27 10:04:17 UTC) #11
ilnik
On 2017/03/27 10:04:17, nisse-webrtc wrote: > I don't quite understand this test, but change looks ...
3 years, 9 months ago (2017-03-27 10:11:00 UTC) #12
ilnik
https://codereview.webrtc.org/2781433002/diff/330001/webrtc/call/call_perf_tests.cc File webrtc/call/call_perf_tests.cc (right): https://codereview.webrtc.org/2781433002/diff/330001/webrtc/call/call_perf_tests.cc#newcode497 webrtc/call/call_perf_tests.cc:497: switch (test_phase_) { In some tests there were 2 ...
3 years, 9 months ago (2017-03-27 10:11:10 UTC) #13
sprang_webrtc
https://codereview.webrtc.org/2781433002/diff/330001/webrtc/call/call_perf_tests.cc File webrtc/call/call_perf_tests.cc (right): https://codereview.webrtc.org/2781433002/diff/330001/webrtc/call/call_perf_tests.cc#newcode497 webrtc/call/call_perf_tests.cc:497: switch (test_phase_) { On 2017/03/27 10:11:10, ilnik wrote: > ...
3 years, 9 months ago (2017-03-27 11:18:38 UTC) #14
ilnik
On 2017/03/27 11:18:38, språng wrote: > https://codereview.webrtc.org/2781433002/diff/330001/webrtc/call/call_perf_tests.cc > File webrtc/call/call_perf_tests.cc (right): > > https://codereview.webrtc.org/2781433002/diff/330001/webrtc/call/call_perf_tests.cc#newcode497 > ...
3 years, 9 months ago (2017-03-27 11:37:45 UTC) #15
sprang_webrtc
Ping? It's only webrtc/call/call_perf_tests.cc that needs approval. Moving most owner to cc as this doesn't ...
3 years, 9 months ago (2017-03-27 14:27:53 UTC) #17
åsapersson
https://codereview.webrtc.org/2781433002/diff/350001/webrtc/video/vie_encoder.cc File webrtc/video/vie_encoder.cc (left): https://codereview.webrtc.org/2781433002/diff/350001/webrtc/video/vie_encoder.cc#oldcode764 webrtc/video/vie_encoder.cc:764: current_pixel_count, AdaptationRequest::Mode::kAdaptUp}); Should last_adaptation_request_ be updated?
3 years, 8 months ago (2017-03-29 08:59:41 UTC) #19
sprang_webrtc
https://codereview.webrtc.org/2781433002/diff/350001/webrtc/video/vie_encoder.cc File webrtc/video/vie_encoder.cc (left): https://codereview.webrtc.org/2781433002/diff/350001/webrtc/video/vie_encoder.cc#oldcode764 webrtc/video/vie_encoder.cc:764: current_pixel_count, AdaptationRequest::Mode::kAdaptUp}); On 2017/03/29 08:59:41, åsapersson wrote: > Should ...
3 years, 8 months ago (2017-03-29 09:03:08 UTC) #20
stefan-webrtc
lgtm
3 years, 8 months ago (2017-03-29 15:05:26 UTC) #21
sprang_webrtc
Thanks Åsa for catching mistake in vie_encoder. Updated. https://codereview.webrtc.org/2781433002/diff/350001/webrtc/video/vie_encoder.cc File webrtc/video/vie_encoder.cc (left): https://codereview.webrtc.org/2781433002/diff/350001/webrtc/video/vie_encoder.cc#oldcode764 webrtc/video/vie_encoder.cc:764: current_pixel_count, ...
3 years, 8 months ago (2017-03-30 09:15:41 UTC) #32
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/2781433002/410001
3 years, 8 months ago (2017-03-30 14:20:31 UTC) #37
commit-bot: I haz the power
Committed patchset #8 (id:410001) as https://chromium.googlesource.com/external/webrtc/+/3ea3c77e93121b1ab9d5e46641e6764f2cca0d51
3 years, 8 months ago (2017-03-30 14:23:54 UTC) #40
lliuu
3 years, 8 months ago (2017-03-30 17:44:25 UTC) #41
Message was sent while issue was closed.
A revert of this CL (patchset #8 id:410001) has been created in
https://codereview.webrtc.org/2783183003/ by lliuu@webrtc.org.

The reason for reverting is: This has resulted in failure of
CallPerfTest.ReceivesCpuOveruseAndUnderuse test on the Win7 build bot
https://build.chromium.org/p/client.webrtc.perf/builders/Win7/builds/1780.

Powered by Google App Engine
This is Rietveld 408576698