Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2934983002:
[Chromoting] Update comments in AudioCapturerWin (Closed)
Description[Chromoting] Update comments in AudioCapturerWin
In change http://codereview.chromium.org/2903153004/, Chris (ChCunningham@)
suggested to update the comments. But I forgot to publish the updated version.
BUG=669070
Review-Url: https://codereview.chromium.org/2934983002
Cr-Commit-Position: refs/heads/master@{#478887}
Committed: https://chromium.googlesource.com/chromium/src/+/2fc3d053221101434dcb5049c3cc90dd9c15d73a
Patch Set 1 #
Total comments: 2
Patch Set 2 : Resolve review comments #Messages
Total messages: 21 (15 generated)
The CQ bit was checked by zijiehe@chromium.org to run a CQ dry run
Description was changed from ========== [Chromoting] Update comments in AudioCapturerWin In change http://codereview.chromium.org/2903153004/, Chris (ChCunningham@) suggested to update the comments. But I forgot to publish the updated version. BUG=669070 ========== to ========== [Chromoting] Update comments in AudioCapturerWin In change http://codereview.chromium.org/2903153004/, Chris (ChCunningham@) suggested to update the comments. But I forgot to publish the updated version. BUG=669070 ==========
zijiehe@chromium.org changed reviewers: + jamiewalch@chromium.org
zijiehe@chromium.org changed reviewers: + sergeyu@chromium.org
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
lgtm
lgtm when my comment is addressed https://codereview.chromium.org/2934983002/diff/1/remoting/host/audio_capture... File remoting/host/audio_capturer_win.cc (right): https://codereview.chromium.org/2934983002/diff/1/remoting/host/audio_capture... remoting/host/audio_capturer_win.cc:277: // TODO(zijiehe): Support various layouts of speakers. This TODO is confusing since technically the current code works with various speaker layouts, it just doesn't map correctly all them to media::ChannelLayout when downmixing. Maybe rephrase it as "Convert dwChannelMask to layout and pass it to AudioPump, so the stream can be downmixed properly, accounting for the actual speaker layout, not just number of channels."
The CQ bit was checked by zijiehe@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
https://codereview.chromium.org/2934983002/diff/1/remoting/host/audio_capture... File remoting/host/audio_capturer_win.cc (right): https://codereview.chromium.org/2934983002/diff/1/remoting/host/audio_capture... remoting/host/audio_capturer_win.cc:277: // TODO(zijiehe): Support various layouts of speakers. On 2017/06/12 23:59:18, Sergey Ulanov wrote: > This TODO is confusing since technically the current code works with various > speaker layouts, it just doesn't map correctly all them to media::ChannelLayout > when downmixing. > Maybe rephrase it as "Convert dwChannelMask to layout and pass it to AudioPump, > so the stream can be downmixed properly, accounting for the actual speaker > layout, not just number of channels." Done.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by zijiehe@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from sergeyu@chromium.org, jamiewalch@chromium.org Link to the patchset: https://codereview.chromium.org/2934983002/#ps20001 (title: "Resolve review comments")
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 20001, "attempt_start_ts": 1497324179723000,
"parent_rev": "5af9b8213c9a95a05d452b56b39f96cf33ceca42", "commit_rev":
"2fc3d053221101434dcb5049c3cc90dd9c15d73a"}
Message was sent while issue was closed.
Description was changed from ========== [Chromoting] Update comments in AudioCapturerWin In change http://codereview.chromium.org/2903153004/, Chris (ChCunningham@) suggested to update the comments. But I forgot to publish the updated version. BUG=669070 ========== to ========== [Chromoting] Update comments in AudioCapturerWin In change http://codereview.chromium.org/2903153004/, Chris (ChCunningham@) suggested to update the comments. But I forgot to publish the updated version. BUG=669070 Review-Url: https://codereview.chromium.org/2934983002 Cr-Commit-Position: refs/heads/master@{#478887} Committed: https://chromium.googlesource.com/chromium/src/+/2fc3d053221101434dcb5049c3cc... ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as https://chromium.googlesource.com/chromium/src/+/2fc3d053221101434dcb5049c3cc... |
