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

Issue 11260053: Use correct thread for audio capturing and encoding. (Closed)

Created:
8 years, 1 month ago by Sergey Ulanov
Modified:
8 years, 1 month ago
CC:
chromium-reviews, jamiewalch+watch_chromium.org, dcaiafa+watch_chromium.org, simonmorris+watch_chromium.org, hclam+watch_chromium.org, wez+watch_chromium.org, amit, sanjeevr, garykac+watch_chromium.org, lambroslambrou+watch_chromium.org, rmsousa+watch_chromium.org, alexeypa+watch_chromium.org, sergeyu+watch_chromium.org, alexeypa (please no reviews)
Visibility:
Public.

Description

Use correct thread for audio capturing and encoding. Previously video capture thread was passed to AudioScheduler instead of the dedicated audio thread. Fixed ChromotingHost so that the correct thread is given to AudioScheduler. BUG=157992 Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=165426

Patch Set 1 #

Patch Set 2 : #

Patch Set 3 : #

Total comments: 16

Patch Set 4 : #

Patch Set 5 : #

Unified diffs Side-by-side diffs Delta from patch set Stats (+15 lines, -1 line) Patch
M remoting/host/chromoting_host.h View 2 chunks +2 lines, -0 lines 0 comments Download
M remoting/host/chromoting_host.cc View 2 chunks +3 lines, -0 lines 0 comments Download
M remoting/host/chromoting_host_unittest.cc View 1 2 3 4 2 chunks +2 lines, -0 lines 0 comments Download
M remoting/host/client_session.h View 1 2 3 2 chunks +2 lines, -0 lines 0 comments Download
M remoting/host/client_session.cc View 1 2 3 4 3 chunks +3 lines, -1 line 0 comments Download
M remoting/host/client_session_unittest.cc View 1 2 3 4 1 chunk +1 line, -0 lines 0 comments Download
M remoting/host/plugin/host_script_object.cc View 1 2 3 1 chunk +1 line, -0 lines 0 comments Download
M remoting/host/remoting_me2me_host.cc View 1 2 3 4 1 chunk +1 line, -0 lines 0 comments Download

Messages

Total messages: 15 (0 generated)
Sergey Ulanov
8 years, 1 month ago (2012-10-26 01:52:08 UTC) #1
Wez
We have the same issue, in principle, with VideoFrameCapturer, don't we? Do you really need ...
8 years, 1 month ago (2012-10-26 03:30:57 UTC) #2
Sergey Ulanov
On 2012/10/26 03:30:57, Wez wrote: > We have the same issue, in principle, with VideoFrameCapturer, ...
8 years, 1 month ago (2012-10-26 18:38:51 UTC) #3
Sergey Ulanov
On 2012/10/26 18:38:51, sergeyu wrote: > On 2012/10/26 03:30:57, Wez wrote: > > We have ...
8 years, 1 month ago (2012-10-26 21:07:01 UTC) #4
Sergey Ulanov
Done. PTAL. On 2012/10/26 21:07:01, sergeyu wrote: > On 2012/10/26 18:38:51, sergeyu wrote: > > ...
8 years, 1 month ago (2012-10-26 21:23:12 UTC) #5
Wez
http://codereview.chromium.org/11260053/diff/7001/remoting/host/audio_scheduler.cc File remoting/host/audio_scheduler.cc (right): http://codereview.chromium.org/11260053/diff/7001/remoting/host/audio_scheduler.cc#newcode37 remoting/host/audio_scheduler.cc:37: desktop_environment)); base::Unretained()? http://codereview.chromium.org/11260053/diff/7001/remoting/host/audio_scheduler.cc#newcode37 remoting/host/audio_scheduler.cc:37: desktop_environment)); Add a comment reminding ...
8 years, 1 month ago (2012-10-27 03:43:15 UTC) #6
Sergey Ulanov
http://codereview.chromium.org/11260053/diff/7001/remoting/host/audio_scheduler.cc File remoting/host/audio_scheduler.cc (right): http://codereview.chromium.org/11260053/diff/7001/remoting/host/audio_scheduler.cc#newcode37 remoting/host/audio_scheduler.cc:37: desktop_environment)); On 2012/10/27 03:43:15, Wez wrote: > base::Unretained()? AudioScheduler ...
8 years, 1 month ago (2012-10-29 23:43:04 UTC) #7
Wez
The AudioCapturer model is currently that we create and destroy it on the main/network thread, ...
8 years, 1 month ago (2012-10-30 03:43:04 UTC) #8
Sergey Ulanov
Reduced this CL to the changes that just pass correct audio thread to the scheduler. ...
8 years, 1 month ago (2012-10-30 18:47:13 UTC) #9
Sergey Ulanov
http://codereview.chromium.org/11260053/diff/7001/remoting/host/desktop_environment.cc File remoting/host/desktop_environment.cc (right): http://codereview.chromium.org/11260053/diff/7001/remoting/host/desktop_environment.cc#newcode31 remoting/host/desktop_environment.cc:31: scoped_ptr<AudioCapturer> DesktopEnvironment::CreateAudioCapturer() { On 2012/10/30 03:43:04, Wez wrote: > ...
8 years, 1 month ago (2012-10-30 18:47:24 UTC) #10
alexeypa (please no reviews)
This CL seems to fix http://crbug.com/158804.
8 years, 1 month ago (2012-10-31 23:24:15 UTC) #11
Wez
LGTM. Let's follow-up by re-naming capture_task_runner to video_task_runner to avoid it being mis-used for any ...
8 years, 1 month ago (2012-10-31 23:34:39 UTC) #12
Sergey Ulanov
On 2012/10/31 23:34:39, Wez wrote: > LGTM. > > Let's follow-up by re-naming capture_task_runner to ...
8 years, 1 month ago (2012-10-31 23:43:35 UTC) #13
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/sergeyu@chromium.org/11260053/19002
8 years, 1 month ago (2012-10-31 23:44:34 UTC) #14
commit-bot: I haz the power
8 years, 1 month ago (2012-11-01 07:20:30 UTC) #15
Step "update" is always a major failure.
Look at the try server FAQ for more details.

Powered by Google App Engine
This is Rietveld 408576698