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

Issue 2079253003: Move frame scheduling logic from WebrtcFrameScheduler to WebrtcVideoStream. (Closed)

Created:
4 years, 6 months ago by Sergey Ulanov
Modified:
3 years, 7 months ago
Reviewers:
Irfan, Jamie
CC:
chromium-reviews, posciak+watch_chromium.org, chromoting-reviews_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Move frame scheduling logic from WebrtcFrameScheduler to WebrtcVideoStream. Move all code from WebrtcFrameScheduler to WebrtcVideoStream and removed WebrtcFrameScheduler. This makes the code simpler. E.g. it's no longer necessary to pass size_callback from the stream to the scheduler. Also made some minor cleanups in the moved code: - Added thread checker. - Removed lock. bitrate and keyframe callbacks are posted to the main thread. Committed: https://crrev.com/4e10ab19860691855b0688007ebfdc82d72033ae Cr-Commit-Position: refs/heads/master@{#401150}

Patch Set 1 #

Patch Set 2 : . #

Unified diffs Side-by-side diffs Delta from patch set Stats (+274 lines, -392 lines) Patch
M remoting/protocol/webrtc_dummy_video_capturer.h View 2 chunks +3 lines, -6 lines 0 comments Download
M remoting/protocol/webrtc_dummy_video_capturer.cc View 1 chunk +1 line, -8 lines 0 comments Download
D remoting/protocol/webrtc_frame_scheduler.h View 1 chunk +0 lines, -117 lines 0 comments Download
D remoting/protocol/webrtc_frame_scheduler.cc View 1 chunk +0 lines, -229 lines 0 comments Download
M remoting/protocol/webrtc_video_encoder_factory.cc View 2 chunks +2 lines, -2 lines 0 comments Download
M remoting/protocol/webrtc_video_stream.h View 4 chunks +60 lines, -8 lines 0 comments Download
M remoting/protocol/webrtc_video_stream.cc View 1 5 chunks +208 lines, -20 lines 0 comments Download
M remoting/remoting_srcs.gypi View 1 chunk +0 lines, -2 lines 0 comments Download

Dependent Patchsets:

Messages

Total messages: 14 (5 generated)
Sergey Ulanov
4 years, 6 months ago (2016-06-20 21:58:30 UTC) #3
Sergey Ulanov
Jamie, please take a look at this CL while Irfan is on vacation. Thanks!
4 years, 6 months ago (2016-06-21 21:17:51 UTC) #4
Jamie
I don't feel like I have enough context to review this. Specifically, aside from removing ...
4 years, 6 months ago (2016-06-21 21:46:56 UTC) #5
Sergey Ulanov
On 2016/06/21 21:46:56, Jamie wrote: > I don't feel like I have enough context to ...
4 years, 6 months ago (2016-06-21 22:11:15 UTC) #6
Jamie
lgtm
4 years, 6 months ago (2016-06-21 23:30:17 UTC) #7
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/2079253003/20001
4 years, 6 months ago (2016-06-21 23:36:29 UTC) #9
commit-bot: I haz the power
Committed patchset #2 (id:20001)
4 years, 6 months ago (2016-06-22 00:56:41 UTC) #11
commit-bot: I haz the power
Patchset 2 (id:??) landed as https://crrev.com/4e10ab19860691855b0688007ebfdc82d72033ae Cr-Commit-Position: refs/heads/master@{#401150}
4 years, 6 months ago (2016-06-22 00:59:21 UTC) #13
albl11021223
3 years, 7 months ago (2017-05-13 11:11:16 UTC) #14
Message was sent while issue was closed.
On 2016/06/22 00:59:21, commit-bot: I haz the power wrote:
> Patchset 2 (id:??) landed as
> https://crrev.com/4e10ab19860691855b0688007ebfdc82d72033ae
> Cr-Commit-Position: refs/heads/master@{#401150}

What and the fuck is this?

Powered by Google App Engine
This is Rietveld 408576698