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

Issue 2101303004: Pass delay and timestamp to AudioSourceCallback::OnMoreData. (Closed)

Created:
3 years, 3 months ago by jameswest
Modified:
3 years ago
CC:
alokp+watch_chromium.org, avayvod+watch_chromium.org, chromium-reviews, darin-cc_chromium.org, feature-media-reviews_chromium.org, halliwell+watch_chromium.org, imcheng+watch_chromium.org, isheriff+watch_chromium.org, jam, jasonroberts+watch_google.com, lcwu+watch_chromium.org, mcasas+watch+vc_chromium.org, miu+watch_chromium.org, mlamouri+watch-media_chromium.org, xjz+watch_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Pass delay and timestamp to AudioSourceCallback::OnMoreData. Replace audio delay in bytes with audio delay in time and the timestamp at which the delay was measured. This is needed to accurately synchronize playback across Cast devices in a multizone group. BUG=internal b/29618700 TEST=base, cast, cast_media, content, and media unit tests Committed: https://crrev.com/3de19856c9d181a606df679384c6e0e84c4b0f39 Cr-Commit-Position: refs/heads/master@{#421999}

Patch Set 1 #

Patch Set 2 #

Total comments: 2

Patch Set 3 : Add comment. #

Total comments: 1

Patch Set 4 : Pass target playout time to AudioSourceCallback::OnMoreData. #

Total comments: 62

Patch Set 5 : Rebase #

Patch Set 6 : Changes based on comments #

Patch Set 7 : Changes based on comments #

Total comments: 28

Patch Set 8 : Changes based on comments #

Total comments: 19

Patch Set 9 : Change to delay and timestamp. #

Patch Set 10 : Rebase #

Patch Set 11 : Fix CQ errors. #

Patch Set 12 : Fix CQ errors. #

Total comments: 1

Patch Set 13 : Fix Windows CQ errors. #

Total comments: 21

Patch Set 14 : Rebase #

Patch Set 15 : Changes based on comments #

Patch Set 16 : Fix Mac CQ errors. #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+676 lines, -410 lines) Patch
M base/time/time.h View 1 2 3 4 5 6 7 8 2 chunks +7 lines, -0 lines 0 comments Download
M base/time/time_mac.cc View 1 2 3 4 5 6 7 8 9 10 4 chunks +34 lines, -22 lines 0 comments Download
M base/time/time_posix.cc View 1 2 3 4 1 chunk +6 lines, -0 lines 0 comments Download
M base/time/time_unittest.cc View 1 2 3 1 chunk +12 lines, -5 lines 0 comments Download
M chromecast/media/audio/cast_audio_mixer.h View 1 2 3 4 5 6 7 8 3 chunks +8 lines, -4 lines 0 comments Download
M chromecast/media/audio/cast_audio_mixer.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 4 chunks +12 lines, -8 lines 0 comments Download
M chromecast/media/audio/cast_audio_mixer_unittest.cc View 1 2 3 4 5 6 7 8 7 chunks +21 lines, -20 lines 0 comments Download
M chromecast/media/audio/cast_audio_output_stream.h View 1 2 3 4 5 6 7 8 1 chunk +2 lines, -0 lines 2 comments Download
M chromecast/media/audio/cast_audio_output_stream.cc View 1 2 3 4 5 6 7 8 2 chunks +5 lines, -4 lines 0 comments Download
M chromecast/media/audio/cast_audio_output_stream_unittest.cc View 1 2 3 4 5 6 7 8 9 3 chunks +11 lines, -6 lines 0 comments Download
M content/browser/media/capture/web_contents_audio_input_stream_unittest.cc View 1 2 3 4 5 6 7 8 9 10 11 3 chunks +5 lines, -1 line 0 comments Download
M content/browser/media/capture/web_contents_audio_muter.cc View 1 2 3 4 5 6 7 8 2 chunks +6 lines, -1 line 0 comments Download
M content/renderer/media/audio_track_recorder_unittest.cc View 1 2 3 4 5 6 7 8 9 10 11 3 chunks +7 lines, -2 lines 0 comments Download
M content/renderer/media/media_recorder_handler_unittest.cc View 1 2 3 4 5 6 7 8 9 10 11 2 chunks +5 lines, -1 line 0 comments Download
M media/audio/alsa/alsa_output.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 5 chunks +8 lines, -2 lines 0 comments Download
M media/audio/alsa/alsa_output.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 9 chunks +23 lines, -17 lines 0 comments Download
M media/audio/alsa/alsa_output_unittest.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 12 chunks +27 lines, -7 lines 0 comments Download
M media/audio/android/audio_android_unittest.cc View 1 2 3 4 5 6 7 8 9 10 6 chunks +17 lines, -15 lines 0 comments Download
M media/audio/android/opensles_output.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 2 chunks +16 lines, -5 lines 0 comments Download
M media/audio/audio_io.h View 1 2 3 4 5 6 7 8 3 chunks +16 lines, -9 lines 0 comments Download
M media/audio/audio_low_latency_input_output_unittest.cc View 1 2 3 4 5 6 7 8 9 3 chunks +11 lines, -9 lines 0 comments Download
M media/audio/audio_output_controller.h View 1 2 3 4 5 6 7 8 2 chunks +6 lines, -3 lines 0 comments Download
M media/audio/audio_output_controller.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 4 chunks +10 lines, -9 lines 0 comments Download
M media/audio/audio_output_controller_unittest.cc View 1 2 3 4 5 6 7 8 4 chunks +6 lines, -3 lines 0 comments Download
M media/audio/audio_output_proxy_unittest.cc View 1 2 3 4 5 6 7 8 1 chunk +6 lines, -5 lines 0 comments Download
M media/audio/audio_output_resampler.h View 1 2 3 1 chunk +1 line, -2 lines 0 comments Download
M media/audio/audio_output_resampler.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 7 chunks +25 lines, -21 lines 0 comments Download
M media/audio/audio_output_stream_sink.h View 1 2 3 4 5 6 7 8 3 chunks +6 lines, -4 lines 0 comments Download
M media/audio/audio_output_stream_sink.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 3 chunks +11 lines, -7 lines 0 comments Download
M media/audio/cras/cras_unified.h View 1 2 3 4 5 6 7 8 2 chunks +1 line, -3 lines 0 comments Download
M media/audio/cras/cras_unified.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 4 chunks +8 lines, -19 lines 0 comments Download
M media/audio/cras/cras_unified_unittest.cc View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +1 line, -1 line 0 comments Download
M media/audio/fake_audio_input_stream.cc View 1 2 3 4 5 6 7 8 9 1 chunk +2 lines, -2 lines 0 comments Download
M media/audio/fake_audio_output_stream.cc View 1 2 3 4 5 6 7 8 2 chunks +5 lines, -3 lines 0 comments Download
M media/audio/mac/audio_auhal_mac.h View 1 2 3 4 5 6 7 8 4 chunks +9 lines, -9 lines 0 comments Download
M media/audio/mac/audio_auhal_mac.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 8 chunks +32 lines, -35 lines 0 comments Download
M media/audio/mac/audio_auhal_mac_unittest.cc View 1 2 3 4 5 6 7 8 9 10 11 2 chunks +2 lines, -2 lines 0 comments Download
M media/audio/mock_audio_source_callback.h View 1 2 3 4 5 6 7 8 2 chunks +4 lines, -5 lines 0 comments Download
M media/audio/pulse/pulse_output.cc View 1 2 3 4 5 6 7 8 2 chunks +4 lines, -4 lines 0 comments Download
M media/audio/pulse/pulse_util.h View 1 2 3 4 5 2 chunks +5 lines, -0 lines 0 comments Download
M media/audio/pulse/pulse_util.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 2 chunks +14 lines, -8 lines 0 comments Download
M media/audio/simple_sources.h View 1 2 3 4 5 6 7 8 4 chunks +14 lines, -10 lines 0 comments Download
M media/audio/simple_sources.cc View 1 2 3 4 5 6 7 8 7 chunks +24 lines, -20 lines 0 comments Download
M media/audio/simple_sources_unittest.cc View 1 2 3 4 5 6 7 8 7 chunks +25 lines, -10 lines 0 comments Download
M media/audio/sounds/audio_stream_handler.cc View 1 2 3 4 5 6 7 8 1 chunk +4 lines, -3 lines 0 comments Download
M media/audio/virtual_audio_input_stream_unittest.cc View 1 2 3 4 5 6 7 8 1 chunk +6 lines, -5 lines 0 comments Download
M media/audio/virtual_audio_output_stream.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 2 chunks +5 lines, -3 lines 0 comments Download
M media/audio/win/audio_low_latency_output_win.cc View 1 2 3 4 5 6 7 8 9 10 11 12 3 chunks +24 lines, -19 lines 0 comments Download
M media/audio/win/audio_low_latency_output_win_unittest.cc View 1 2 3 4 5 6 7 8 9 10 11 12 5 chunks +21 lines, -19 lines 0 comments Download
M media/audio/win/audio_output_win_unittest.cc View 1 2 3 4 5 6 7 8 9 10 11 12 11 chunks +45 lines, -31 lines 0 comments Download
M media/audio/win/waveout_output_win.cc View 1 2 3 4 5 6 7 8 2 chunks +6 lines, -3 lines 0 comments Download
M media/base/audio_renderer_sink.h View 1 2 3 4 5 6 7 8 1 chunk +2 lines, -0 lines 0 comments Download
M media/base/audio_timestamp_helper.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 1 chunk +8 lines, -0 lines 0 comments Download
M media/base/audio_timestamp_helper.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 1 chunk +14 lines, -0 lines 0 comments Download
M media/base/audio_timestamp_helper_unittest.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 2 chunks +52 lines, -0 lines 0 comments Download
M media/cast/test/receiver.cc View 1 2 3 4 5 6 7 8 9 10 2 chunks +5 lines, -3 lines 0 comments Download
M media/cast/test/utility/audio_utility.h View 1 2 3 4 5 6 7 8 9 10 1 chunk +2 lines, -0 lines 0 comments Download
M media/cast/test/utility/audio_utility.cc View 1 2 3 4 5 6 7 8 9 10 2 chunks +2 lines, -1 line 0 comments Download

Messages

Total messages: 151 (60 generated)
jameswest
3 years, 3 months ago (2016-06-30 23:35:43 UTC) #3
alokp
https://codereview.chromium.org/2101303004/diff/20001/media/audio/audio_io.h File media/audio/audio_io.h (right): https://codereview.chromium.org/2101303004/diff/20001/media/audio/audio_io.h#newcode69 media/audio/audio_io.h:69: base::TimeDelta delay_timestamp, please add comments
3 years, 3 months ago (2016-06-30 23:37:39 UTC) #4
DaleCurtis
Can you elaborate on why we need two delay values? I don't understand the use ...
3 years, 3 months ago (2016-06-30 23:46:30 UTC) #6
jameswest
On 2016/06/30 23:46:30, DaleCurtis wrote: > Can you elaborate on why we need two delay ...
3 years, 3 months ago (2016-07-01 00:08:26 UTC) #7
jameswest
https://codereview.chromium.org/2101303004/diff/20001/media/audio/audio_io.h File media/audio/audio_io.h (right): https://codereview.chromium.org/2101303004/diff/20001/media/audio/audio_io.h#newcode69 media/audio/audio_io.h:69: base::TimeDelta delay_timestamp, On 2016/06/30 23:37:39, alokp wrote: > please ...
3 years, 3 months ago (2016-07-01 00:08:45 UTC) #8
DaleCurtis
So you need both a timestamp and a delay value? Why can't the output stream ...
3 years, 3 months ago (2016-07-01 00:11:15 UTC) #9
alokp
+kmackay@ who has more context about the usage. James has now added comment about the ...
3 years, 3 months ago (2016-07-01 00:22:48 UTC) #11
kmackay
Some more context: if you just have the delay value and not the time when ...
3 years, 3 months ago (2016-07-01 00:32:32 UTC) #12
DaleCurtis
Okay that makes sense, we don't generally use a TimeDelta for timestamps though. Instead we ...
3 years, 3 months ago (2016-07-01 00:39:34 UTC) #13
DaleCurtis
Also, can you support your claims that this is necessary? Is this a cast only ...
3 years, 3 months ago (2016-07-01 00:41:54 UTC) #14
kmackay
On 2016/07/01 00:39:34, DaleCurtis wrote: > Okay that makes sense, we don't generally use a ...
3 years, 3 months ago (2016-07-01 00:44:15 UTC) #15
kmackay
On 2016/07/01 00:41:54, DaleCurtis wrote: > Also, can you support your claims that this is ...
3 years, 3 months ago (2016-07-01 00:45:35 UTC) #16
jameswest
We use CLOCK_MONOTONIC_RAW, which TimeTicks doesn't support as far as I can tell. 10 ms ...
3 years, 3 months ago (2016-07-01 00:46:11 UTC) #17
kmackay
On 2016/07/01 00:46:11, James West wrote: > We use CLOCK_MONOTONIC_RAW, which TimeTicks doesn't support as ...
3 years, 3 months ago (2016-07-01 00:47:07 UTC) #18
DaleCurtis
+miu who knows all things timeticks :)
3 years, 3 months ago (2016-07-01 02:07:21 UTC) #20
DaleCurtis
Thanks for the explanation, lets see if we can find a way to get TimeTicks ...
3 years, 3 months ago (2016-07-01 02:07:57 UTC) #21
DaleCurtis
ping miu for thoughts :)
3 years, 3 months ago (2016-07-07 20:26:04 UTC) #22
alokp
On 2016/07/07 20:26:04, DaleCurtis wrote: > ping miu for thoughts :) TimeTicks currently uses CLOCK_MONOTONIC: ...
3 years, 3 months ago (2016-07-13 19:41:26 UTC) #23
miu
On 2016/07/01 00:46:11, James West wrote: > We use CLOCK_MONOTONIC_RAW, which TimeTicks doesn't support as ...
3 years, 3 months ago (2016-07-14 20:53:09 UTC) #24
miu
On 2016/07/14 20:53:09, miu wrote: > On 2016/07/01 00:46:11, James West wrote: > > We ...
3 years, 3 months ago (2016-07-14 21:06:28 UTC) #25
miu
https://codereview.chromium.org/2101303004/diff/40001/media/audio/audio_io.h File media/audio/audio_io.h (right): https://codereview.chromium.org/2101303004/diff/40001/media/audio/audio_io.h#newcode70 media/audio/audio_io.h:70: base::TimeDelta delay_timestamp, If "delay_timestamp is the time at which ...
3 years, 3 months ago (2016-07-14 21:08:55 UTC) #26
kmackay
On 2016/07/14 21:06:28, miu wrote: > On 2016/07/14 20:53:09, miu wrote: > > On 2016/07/01 ...
3 years, 3 months ago (2016-07-14 21:12:50 UTC) #27
kmackay
On 2016/07/14 21:08:55, miu wrote: > Putting it all together, this would be a much ...
3 years, 3 months ago (2016-07-14 21:18:52 UTC) #28
miu
On 2016/07/14 21:12:50, kmackay wrote: > We picked CLOCK_MONOTONIC_RAW specifically so that the frequency doesn't ...
3 years, 3 months ago (2016-07-19 02:04:17 UTC) #29
miu
On 2016/07/14 21:18:52, kmackay wrote: > Now() + delay_until_playout isn't accurate enough for our use ...
3 years, 3 months ago (2016-07-19 02:15:14 UTC) #30
kmackay
On 2016/07/19 02:04:17, miu wrote: > On 2016/07/14 21:12:50, kmackay wrote: > > We picked ...
3 years, 3 months ago (2016-07-19 21:07:24 UTC) #31
kmackay
On 2016/07/19 02:15:14, miu wrote: > So, what do you think of: > > virtual ...
3 years, 3 months ago (2016-07-19 21:09:59 UTC) #32
alokp
To summarize, we need to: 1. Switch base::TimeTicks to use CLOCK_MONOTONIC_RAW 2. Change OnMoreData interface ...
3 years, 3 months ago (2016-07-20 04:53:01 UTC) #33
miu
On 2016/07/20 04:53:01, alokp wrote: > To summarize, we need to: > > 1. Switch ...
3 years, 3 months ago (2016-07-20 19:21:10 UTC) #34
miu
On 2016/07/19 21:07:24, kmackay wrote: > Our current audio synchronization across wifi is within 200 ...
3 years, 3 months ago (2016-07-20 19:27:51 UTC) #35
jameswest
I updated the interface to use target_playout_time. I created a separate CL for changing TimeTicks ...
3 years, 2 months ago (2016-07-26 23:36:47 UTC) #36
DaleCurtis
=> chcunningham to do media/ review.
3 years, 2 months ago (2016-07-27 18:22:39 UTC) #39
DaleCurtis
https://codereview.chromium.org/2101303004/diff/60001/media/audio/audio_output_controller.cc File media/audio/audio_output_controller.cc (right): https://codereview.chromium.org/2101303004/diff/60001/media/audio/audio_output_controller.cc#newcode310 media/audio/audio_output_controller.cc:310: const base::TimeDelta delay = target_playout_time - base::TimeTicks::Now(); Hmm, if ...
3 years, 2 months ago (2016-07-27 18:25:55 UTC) #40
chcunningham1
On 2016/07/27 18:25:55, DaleCurtis wrote: > https://codereview.chromium.org/2101303004/diff/60001/media/audio/audio_output_controller.cc > File media/audio/audio_output_controller.cc (right): > > https://codereview.chromium.org/2101303004/diff/60001/media/audio/audio_output_controller.cc#newcode310 > ...
3 years, 2 months ago (2016-07-27 21:34:43 UTC) #41
jameswest
https://codereview.chromium.org/2101303004/diff/60001/media/audio/audio_output_controller.cc File media/audio/audio_output_controller.cc (right): https://codereview.chromium.org/2101303004/diff/60001/media/audio/audio_output_controller.cc#newcode310 media/audio/audio_output_controller.cc:310: const base::TimeDelta delay = target_playout_time - base::TimeTicks::Now(); We don't ...
3 years, 2 months ago (2016-07-27 23:07:50 UTC) #42
DaleCurtis
Ah, in that case this seems okay to me. For http://crbug.com/619533 and https://codereview.chromium.org/2060833002 they'll have ...
3 years, 2 months ago (2016-07-28 01:21:47 UTC) #43
jameswest
https://codereview.chromium.org/2101303004/diff/60001/media/audio/audio_output_controller.cc File media/audio/audio_output_controller.cc (right): https://codereview.chromium.org/2101303004/diff/60001/media/audio/audio_output_controller.cc#newcode310 media/audio/audio_output_controller.cc:310: const base::TimeDelta delay = target_playout_time - base::TimeTicks::Now(); On 2016/07/28 ...
3 years, 2 months ago (2016-07-28 02:09:39 UTC) #44
kmackay
On 2016/07/28 02:09:39, James West wrote: > https://codereview.chromium.org/2101303004/diff/60001/media/audio/audio_output_controller.cc > File media/audio/audio_output_controller.cc (right): > > https://codereview.chromium.org/2101303004/diff/60001/media/audio/audio_output_controller.cc#newcode310 ...
3 years, 2 months ago (2016-07-28 02:17:19 UTC) #45
chcunningham
Comments incoming ;) https://codereview.chromium.org/2101303004/diff/60001/content/browser/media/capture/web_contents_audio_muter.cc File content/browser/media/capture/web_contents_audio_muter.cc (right): https://codereview.chromium.org/2101303004/diff/60001/content/browser/media/capture/web_contents_audio_muter.cc#newcode56 content/browser/media/capture/web_contents_audio_muter.cc:56: callback->OnMoreData(base::TimeTicks(), 0, audio_bus_.get()); Should the be ...
3 years, 2 months ago (2016-07-29 01:21:10 UTC) #46
chcunningham
https://codereview.chromium.org/2101303004/diff/60001/content/browser/media/capture/web_contents_audio_muter.cc File content/browser/media/capture/web_contents_audio_muter.cc (right): https://codereview.chromium.org/2101303004/diff/60001/content/browser/media/capture/web_contents_audio_muter.cc#newcode56 content/browser/media/capture/web_contents_audio_muter.cc:56: callback->OnMoreData(base::TimeTicks(), 0, audio_bus_.get()); On 2016/07/29 01:21:08, chcunningham wrote: > ...
3 years, 2 months ago (2016-07-29 01:24:12 UTC) #47
miu
Catching up on code reviews. Sorry for the delay. However, it's not clear whether we ...
3 years, 2 months ago (2016-07-30 21:17:02 UTC) #48
miu
Bringing over the comment from https://codereview.chromium.org/2172483002... On 2016/07/28 01:31:31, James West wrote: > I think ...
3 years, 2 months ago (2016-07-30 22:45:00 UTC) #49
kmackay
On 2016/07/30 22:45:00, miu wrote: > Bringing over the comment from https://codereview.chromium.org/2172483002... > > On ...
3 years, 2 months ago (2016-07-31 06:16:38 UTC) #50
kmackay
On 2016/07/31 06:16:38, kmackay wrote: > CastAudioOutputStream is not used in general for casted audio ...
3 years, 2 months ago (2016-07-31 06:48:46 UTC) #51
jameswest
https://codereview.chromium.org/2101303004/diff/60001/content/browser/media/capture/web_contents_audio_muter.cc File content/browser/media/capture/web_contents_audio_muter.cc (right): https://codereview.chromium.org/2101303004/diff/60001/content/browser/media/capture/web_contents_audio_muter.cc#newcode56 content/browser/media/capture/web_contents_audio_muter.cc:56: callback->OnMoreData(base::TimeTicks(), 0, audio_bus_.get()); On 2016/07/29 01:24:12, chcunningham wrote: > ...
3 years, 1 month ago (2016-08-26 02:08:48 UTC) #52
chcunningham1
This LGTM - Dale/Yuri should probably take a last pass though. https://codereview.chromium.org/2101303004/diff/60001/media/audio/alsa/alsa_output_unittest.cc File media/audio/alsa/alsa_output_unittest.cc (right): ...
3 years, 1 month ago (2016-08-27 00:35:00 UTC) #54
chcunningham
Arg, I mixed my accounts up. Here's some extra nits. lgtm https://codereview.chromium.org/2101303004/diff/60001/media/audio/audio_output_controller.cc File media/audio/audio_output_controller.cc (right): ...
3 years, 1 month ago (2016-08-27 00:36:19 UTC) #55
miu
hubbe: PTAL at changes to audio_shifter.* There was some question about the old and new ...
3 years, 1 month ago (2016-08-31 23:26:55 UTC) #57
miu
On 2016/08/31 23:26:55, miu wrote: > https://codereview.chromium.org/2101303004/diff/120001/media/audio/audio_output_resampler.cc#newcode392 > media/audio/audio_output_resampler.cc:392: base::TimeTicks > new_target_playout_time = > On ...
3 years, 1 month ago (2016-08-31 23:29:35 UTC) #58
miu
-hubbe@
3 years, 1 month ago (2016-08-31 23:29:55 UTC) #60
DaleCurtis
https://codereview.chromium.org/2101303004/diff/120001/media/audio/audio_output_resampler.cc File media/audio/audio_output_resampler.cc (right): https://codereview.chromium.org/2101303004/diff/120001/media/audio/audio_output_resampler.cc#newcode392 media/audio/audio_output_resampler.cc:392: base::TimeTicks new_target_playout_time = On 2016/08/31 at 23:26:54, miu wrote: ...
3 years, 1 month ago (2016-09-01 18:46:14 UTC) #61
jameswest
https://codereview.chromium.org/2101303004/diff/120001/media/audio/audio_output_controller.cc File media/audio/audio_output_controller.cc (right): https://codereview.chromium.org/2101303004/diff/120001/media/audio/audio_output_controller.cc#newcode310 media/audio/audio_output_controller.cc:310: const base::TimeDelta delay = On 2016/08/31 23:26:54, miu wrote: ...
3 years, 1 month ago (2016-09-07 21:52:16 UTC) #62
miu
https://codereview.chromium.org/2101303004/diff/120001/media/audio/audio_output_controller.cc File media/audio/audio_output_controller.cc (right): https://codereview.chromium.org/2101303004/diff/120001/media/audio/audio_output_controller.cc#newcode310 media/audio/audio_output_controller.cc:310: const base::TimeDelta delay = On 2016/09/07 21:52:16, James West ...
3 years, 1 month ago (2016-09-07 22:19:37 UTC) #63
Mikhail
https://codereview.chromium.org/2101303004/diff/120001/media/audio/win/audio_low_latency_output_win.cc File media/audio/win/audio_low_latency_output_win.cc (right): https://codereview.chromium.org/2101303004/diff/120001/media/audio/win/audio_low_latency_output_win.cc#newcode515 media/audio/win/audio_low_latency_output_win.cc:515: base::TimeTicks target_playout_time = base::TimeTicks::Now(); wouldn't it be more accurate ...
3 years, 1 month ago (2016-09-08 07:44:12 UTC) #65
miu
https://codereview.chromium.org/2101303004/diff/120001/media/audio/win/audio_low_latency_output_win.cc File media/audio/win/audio_low_latency_output_win.cc (right): https://codereview.chromium.org/2101303004/diff/120001/media/audio/win/audio_low_latency_output_win.cc#newcode515 media/audio/win/audio_low_latency_output_win.cc:515: base::TimeTicks target_playout_time = base::TimeTicks::Now(); On 2016/09/08 07:44:12, Mikhail wrote: ...
3 years, 1 month ago (2016-09-08 20:46:48 UTC) #66
miu
https://codereview.chromium.org/2101303004/diff/120001/media/audio/mac/audio_auhal_mac.cc File media/audio/mac/audio_auhal_mac.cc (right): https://codereview.chromium.org/2101303004/diff/120001/media/audio/mac/audio_auhal_mac.cc#newcode342 media/audio/mac/audio_auhal_mac.cc:342: return 0; Looking at this again, I think it ...
3 years, 1 month ago (2016-09-08 20:56:23 UTC) #67
Mikhail
If I understand the intention of this CL correctly you provide a system time estimation ...
3 years, 1 month ago (2016-09-09 18:19:42 UTC) #68
James West
https://codereview.chromium.org/2101303004/diff/120001/media/audio/android/opensles_output.cc File media/audio/android/opensles_output.cc (right): https://codereview.chromium.org/2101303004/diff/120001/media/audio/android/opensles_output.cc#newcode344 media/audio/android/opensles_output.cc:344: // Calculate the delay in frames. On 2016/08/31 23:26:54, ...
3 years, 1 month ago (2016-09-13 07:40:51 UTC) #70
James West
On 2016/09/09 18:19:42, Mikhail wrote: > If I understand the intention of this CL correctly ...
3 years, 1 month ago (2016-09-13 08:10:33 UTC) #71
Mikhail
On 2016/09/13 08:10:33, jameswest wrote: > On 2016/09/09 18:19:42, Mikhail wrote: > > If I ...
3 years, 1 month ago (2016-09-13 08:17:17 UTC) #72
jameswest
On 2016/09/13 08:17:17, Mikhail wrote: > On 2016/09/13 08:10:33, jameswest wrote: > > On 2016/09/09 ...
3 years, 1 month ago (2016-09-16 03:26:52 UTC) #73
Mikhail
On 2016/09/16 03:26:52, James West wrote: > Mikhail, > > Is it possible for you ...
3 years, 1 month ago (2016-09-16 08:42:29 UTC) #74
miu
Nice work! :) Just a few minor considerations left: https://codereview.chromium.org/2101303004/diff/120001/media/audio/audio_output_controller.cc File media/audio/audio_output_controller.cc (right): https://codereview.chromium.org/2101303004/diff/120001/media/audio/audio_output_controller.cc#newcode310 media/audio/audio_output_controller.cc:310: ...
3 years, 1 month ago (2016-09-16 18:35:58 UTC) #75
miu
On 2016/09/16 08:42:29, Mikhail wrote: > I think I could surviuve with 'stream_position = written_audio ...
3 years, 1 month ago (2016-09-16 18:38:03 UTC) #76
chcunningham
On 2016/09/16 18:38:03, miu wrote: > On 2016/09/16 08:42:29, Mikhail wrote: > > I think ...
3 years, 1 month ago (2016-09-16 20:52:44 UTC) #77
jameswest
On 2016/09/16 20:52:44, chcunningham wrote: > On 2016/09/16 18:38:03, miu wrote: > > On 2016/09/16 ...
3 years, 1 month ago (2016-09-16 21:10:31 UTC) #78
jameswest
On 2016/09/16 21:10:31, James West wrote: > On 2016/09/16 20:52:44, chcunningham wrote: > > On ...
3 years, 1 month ago (2016-09-16 21:26:12 UTC) #79
jameswest
https://codereview.chromium.org/2101303004/diff/140001/media/audio/cras/cras_unified.cc File media/audio/cras/cras_unified.cc (right): https://codereview.chromium.org/2101303004/diff/140001/media/audio/cras/cras_unified.cc#newcode253 media/audio/cras/cras_unified.cc:253: const timespec* output_ts, Do you know if |output_ts| is ...
3 years, 1 month ago (2016-09-16 21:59:43 UTC) #80
miu
On 2016/09/16 21:26:12, James West wrote: > On 2016/09/16 21:10:31, James West wrote: > > ...
3 years, 1 month ago (2016-09-16 22:56:10 UTC) #81
miu
On 2016/09/16 20:52:44, chcunningham wrote: > Sorry, I'm confused now. Why is delay needed? Is ...
3 years, 1 month ago (2016-09-16 22:58:15 UTC) #82
miu
https://codereview.chromium.org/2101303004/diff/140001/media/audio/cras/cras_unified.cc File media/audio/cras/cras_unified.cc (right): https://codereview.chromium.org/2101303004/diff/140001/media/audio/cras/cras_unified.cc#newcode253 media/audio/cras/cras_unified.cc:253: const timespec* output_ts, On 2016/09/16 21:59:43, James West wrote: ...
3 years, 1 month ago (2016-09-16 23:38:28 UTC) #83
jameswest
https://codereview.chromium.org/2101303004/diff/140001/media/audio/mac/audio_auhal_mac.cc File media/audio/mac/audio_auhal_mac.cc (right): https://codereview.chromium.org/2101303004/diff/140001/media/audio/mac/audio_auhal_mac.cc#newcode263 media/audio/mac/audio_auhal_mac.cc:263: current_target_playout_time_ + delay; On 2016/09/16 23:38:28, miu wrote: > ...
3 years, 1 month ago (2016-09-17 00:06:50 UTC) #84
miu
https://codereview.chromium.org/2101303004/diff/140001/media/audio/mac/audio_auhal_mac.cc File media/audio/mac/audio_auhal_mac.cc (right): https://codereview.chromium.org/2101303004/diff/140001/media/audio/mac/audio_auhal_mac.cc#newcode263 media/audio/mac/audio_auhal_mac.cc:263: current_target_playout_time_ + delay; On 2016/09/17 00:06:50, James West wrote: ...
3 years, 1 month ago (2016-09-17 00:33:56 UTC) #85
jameswest
https://codereview.chromium.org/2101303004/diff/140001/base/time/time.h File base/time/time.h (right): https://codereview.chromium.org/2101303004/diff/140001/base/time/time.h#newcode738 base/time/time.h:738: #if defined(OS_MACOSX) On 2016/09/16 18:35:58, miu wrote: > To ...
3 years, 1 month ago (2016-09-19 23:32:36 UTC) #86
Mikhail
https://codereview.chromium.org/2101303004/diff/220001/media/audio/audio_io.h File media/audio/audio_io.h (right): https://codereview.chromium.org/2101303004/diff/220001/media/audio/audio_io.h#newcode72 media/audio/audio_io.h:72: virtual int OnMoreData(base::TimeDelta delay, Thanks for providing this API!
3 years, 1 month ago (2016-09-20 09:39:02 UTC) #103
jameswest
I fixed all the CQ errors, so it's ready for everyone to take a final ...
3 years ago (2016-09-21 00:54:57 UTC) #117
chcunningham
Some cases of integer division in computing delay where the truncation will accumulate across the ...
3 years ago (2016-09-23 20:53:30 UTC) #118
jameswest
On 2016/09/23 20:53:30, chcunningham wrote: > Some cases of integer division in computing delay where ...
3 years ago (2016-09-23 22:56:19 UTC) #119
jameswest
Is there a good place to put utility functions to convert frames to TimeDelta and ...
3 years ago (2016-09-23 23:00:28 UTC) #120
chcunningham
On 2016/09/23 22:56:19, James West wrote: > On 2016/09/23 20:53:30, chcunningham wrote: > > Some ...
3 years ago (2016-09-28 19:39:14 UTC) #121
chcunningham
On 2016/09/23 23:00:28, James West wrote: > Is there a good place to put utility ...
3 years ago (2016-09-28 19:40:53 UTC) #122
miu
Patch Set 8 LGTM % nits: https://codereview.chromium.org/2101303004/diff/240001/chromecast/media/audio/cast_audio_mixer.cc File chromecast/media/audio/cast_audio_mixer.cc (right): https://codereview.chromium.org/2101303004/diff/240001/chromecast/media/audio/cast_audio_mixer.cc#newcode257 chromecast/media/audio/cast_audio_mixer.cc:257: uint32_t frames_delayed = ...
3 years ago (2016-09-28 23:04:41 UTC) #123
miu
On 2016/09/28 23:04:41, miu wrote: > Patch Set 8 LGTM % nits: Whoops! I mean, ...
3 years ago (2016-09-28 23:05:16 UTC) #124
jameswest
https://codereview.chromium.org/2101303004/diff/240001/chromecast/media/audio/cast_audio_mixer.cc File chromecast/media/audio/cast_audio_mixer.cc (right): https://codereview.chromium.org/2101303004/diff/240001/chromecast/media/audio/cast_audio_mixer.cc#newcode146 chromecast/media/audio/cast_audio_mixer.cc:146: input_params_.sample_rate()); On 2016/09/23 20:53:29, chcunningham wrote: > int division ...
3 years ago (2016-09-29 00:52:24 UTC) #130
chcunningham
media LGTM
3 years ago (2016-09-29 21:30:16 UTC) #137
kmackay
chromecast changes lgtm (need alokp for owner's approval)
3 years ago (2016-09-29 21:53:38 UTC) #142
alokp
chromecast/ lgtm https://codereview.chromium.org/2101303004/diff/300001/chromecast/media/audio/cast_audio_output_stream.h File chromecast/media/audio/cast_audio_output_stream.h (right): https://codereview.chromium.org/2101303004/diff/300001/chromecast/media/audio/cast_audio_output_stream.h#newcode8 chromecast/media/audio/cast_audio_output_stream.h:8: #include <memory> is this due to restructuring ...
3 years ago (2016-09-30 00:12:21 UTC) #143
jameswest
https://codereview.chromium.org/2101303004/diff/300001/chromecast/media/audio/cast_audio_output_stream.h File chromecast/media/audio/cast_audio_output_stream.h (right): https://codereview.chromium.org/2101303004/diff/300001/chromecast/media/audio/cast_audio_output_stream.h#newcode8 chromecast/media/audio/cast_audio_output_stream.h:8: #include <memory> On 2016/09/30 00:12:21, alokp wrote: > is ...
3 years ago (2016-09-30 00:30:16 UTC) #144
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/2101303004/300001
3 years ago (2016-09-30 00:36:27 UTC) #147
commit-bot: I haz the power
Committed patchset #16 (id:300001)
3 years ago (2016-09-30 00:46:07 UTC) #149
commit-bot: I haz the power
3 years ago (2016-09-30 00:50:19 UTC) #151
Message was sent while issue was closed.
Patchset 16 (id:??) landed as
https://crrev.com/3de19856c9d181a606df679384c6e0e84c4b0f39
Cr-Commit-Position: refs/heads/master@{#421999}

Powered by Google App Engine
This is Rietveld 408576698