Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(6)

Issue 2384063004: Move audio decoding to protocol layer (Closed)

Created:
2 years, 11 months ago by Sergey Ulanov
Modified:
2 years, 11 months ago
Reviewers:
Jamie
CC:
chromium-reviews, chromoting-reviews_chromium.org
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Move audio decoding to protocol layer Previously ChromotingClient was responsible for creation of AudioDecodeScheduler. Moved that logic to IceConnectionToHost because WebrtcConnectionToHost will not need it. Also moved AudioDecodeScheduler from remoting/client to remoting/protocol and simplified it. BUG=638505 Committed: https://crrev.com/77b42df81a9c802d91dd2575b57d4780df16bf80 Cr-Commit-Position: refs/heads/master@{#422671}

Patch Set 1 #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+205 lines, -458 lines) Patch
M remoting/client/BUILD.gn View 4 chunks +0 lines, -19 lines 0 comments Download
D remoting/client/audio_consumer.h View 1 chunk +0 lines, -24 lines 0 comments Download
D remoting/client/audio_decode_scheduler.h View 1 chunk +0 lines, -54 lines 0 comments Download
D remoting/client/audio_decode_scheduler.cc View 1 chunk +0 lines, -120 lines 0 comments Download
D remoting/client/audio_decode_scheduler_unittest.cc View 1 chunk +0 lines, -104 lines 0 comments Download
M remoting/client/audio_player.h View 2 chunks +5 lines, -4 lines 0 comments Download
M remoting/client/audio_player.cc View 3 chunks +6 lines, -3 lines 0 comments Download
M remoting/client/audio_player_unittest.cc View 7 chunks +19 lines, -12 lines 0 comments Download
M remoting/client/chromoting_client.h View 4 chunks +4 lines, -7 lines 0 comments Download
M remoting/client/chromoting_client.cc View 6 chunks +12 lines, -24 lines 0 comments Download
D remoting/client/fake_audio_consumer.h View 1 chunk +0 lines, -34 lines 0 comments Download
D remoting/client/fake_audio_consumer.cc View 1 chunk +0 lines, -21 lines 0 comments Download
M remoting/protocol/BUILD.gn View 2 chunks +3 lines, -0 lines 0 comments Download
A + remoting/protocol/audio_decode_scheduler.h View 3 chunks +24 lines, -14 lines 0 comments Download
A remoting/protocol/audio_decode_scheduler.cc View 1 chunk +64 lines, -0 lines 0 comments Download
A + remoting/protocol/audio_decode_scheduler_unittest.cc View 5 chunks +30 lines, -7 lines 0 comments Download
M remoting/protocol/connection_to_host.h View 2 chunks +12 lines, -2 lines 0 comments Download
M remoting/protocol/fake_connection_to_host.h View 1 chunk +3 lines, -1 line 0 comments Download
M remoting/protocol/fake_connection_to_host.cc View 1 chunk +3 lines, -1 line 0 comments Download
M remoting/protocol/ice_connection_to_host.h View 3 chunks +6 lines, -2 lines 0 comments Download
M remoting/protocol/ice_connection_to_host.cc View 3 chunks +8 lines, -3 lines 0 comments Download
M remoting/protocol/webrtc_connection_to_host.h View 1 chunk +3 lines, -1 line 0 comments Download
M remoting/protocol/webrtc_connection_to_host.cc View 1 chunk +3 lines, -1 line 2 comments Download

Messages

Total messages: 9 (3 generated)
Sergey Ulanov
2 years, 11 months ago (2016-10-03 23:36:42 UTC) #2
Jamie
lgtm https://codereview.chromium.org/2384063004/diff/1/remoting/protocol/webrtc_connection_to_host.cc File remoting/protocol/webrtc_connection_to_host.cc (right): https://codereview.chromium.org/2384063004/diff/1/remoting/protocol/webrtc_connection_to_host.cc#newcode78 remoting/protocol/webrtc_connection_to_host.cc:78: NOTIMPLEMENTED(); Is NOTIMPLEMENTED appropriate here? NOTIMPLEMENTED suggests to ...
2 years, 11 months ago (2016-10-04 00:25:38 UTC) #3
Sergey Ulanov
https://codereview.chromium.org/2384063004/diff/1/remoting/protocol/webrtc_connection_to_host.cc File remoting/protocol/webrtc_connection_to_host.cc (right): https://codereview.chromium.org/2384063004/diff/1/remoting/protocol/webrtc_connection_to_host.cc#newcode78 remoting/protocol/webrtc_connection_to_host.cc:78: NOTIMPLEMENTED(); On 2016/10/04 00:25:38, Jamie wrote: > Is NOTIMPLEMENTED ...
2 years, 11 months ago (2016-10-04 00:42:22 UTC) #4
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2384063004/1
2 years, 11 months ago (2016-10-04 00:43:19 UTC) #6
commit-bot: I haz the power
Committed patchset #1 (id:1)
2 years, 11 months ago (2016-10-04 02:08:57 UTC) #7
commit-bot: I haz the power
2 years, 11 months ago (2016-10-04 02:11:51 UTC) #9
Message was sent while issue was closed.
Patchset 1 (id:??) landed as
https://crrev.com/77b42df81a9c802d91dd2575b57d4780df16bf80
Cr-Commit-Position: refs/heads/master@{#422671}

Powered by Google App Engine
This is Rietveld 408576698