Express audio delay more precisely in frames rather than milliseconds.

Express audio delay more precisely in frames rather than milliseconds.

This is a first step to reduce drift in audio video synchronization.
With this change, drift in a 2 hour file will be mostly eliminated if
the media sample rate matches that of the hardware (no resampling).

Other sources of drift will be fixed in follow up cls.

BUG=564604, 587522, 586540
TEST=webm file from reporter in http://crbug.com/564604#c9

Committed: https://crrev.com/7bdc23eebf424b510e57e33d05603ced780808db
Cr-Commit-Position: refs/heads/master@{#375967}

[chcunningham, 2016-02-11 00:57:08]

Patchset #1 (id:1) has been deleted

[chcunningham, 2016-02-11 01:11:37]

Patchset #1 (id:20001) has been deleted

[chcunningham, 2016-02-11 01:25:41]

chcunningham@chromium.org changed reviewers:
+ dalecurtis@chromium.org, grunell@chromium.org

[chcunningham, 2016-02-11 01:25:42]

Hi reviewers.

This is the first of several cls to fix the av sync drift from
http://crbug.com/564604.

With this fix, the video from comment 9 in that bug should playback with nearly
imperceptible drift if you're not resampling (hw output rate must match 48000)
for at least the first 2 hours. By the end of hour 3 audio is slightly ahead of
video. 

Thanks!
Chris

https://codereview.chromium.org/1687213002/diff/40001/content/renderer/media/...
File content/renderer/media/renderer_webaudiodevice_impl.cc (right):

https://codereview.chromium.org/1687213002/diff/40001/content/renderer/media/...
content/renderer/media/renderer_webaudiodevice_impl.cc:103: uint32_t
frames_delayed,
This method isn't making use of delay... should it be?

https://codereview.chromium.org/1687213002/diff/40001/content/renderer/media/...
File content/renderer/media/webrtc_audio_renderer.cc (right):

https://codereview.chromium.org/1687213002/diff/40001/content/renderer/media/...
content/renderer/media/webrtc_audio_renderer.cc:433: // TODO(X): Converting from
frames to milliseconds will potentially lose
X should be one of the RTC folks. Henrik? 

Can you file a bug in RTC to do this cleanup?

https://codereview.chromium.org/1687213002/diff/40001/content/renderer/media/...
content/renderer/media/webrtc_audio_renderer.cc:492: // TODO(X): This integer
division by sample_rate will cause loss of partial
Ditto.

https://codereview.chromium.org/1687213002/diff/40001/media/audio/audio_devic...
File media/audio/audio_device_thread.h (right):

https://codereview.chromium.org/1687213002/diff/40001/media/audio/audio_devic...
media/audio/audio_device_thread.h:64: const double bytes_per_ms_;
audio_input_device is still using this.

https://codereview.chromium.org/1687213002/diff/40001/media/audio/audio_input...
File media/audio/audio_input_device.cc (left):

https://codereview.chromium.org/1687213002/diff/40001/media/audio/audio_input...
media/audio/audio_input_device.cc:342: buffer->params.hardware_delay_bytes /
bytes_per_ms_,  // Delay in ms
Another API expecing msec. Everyone OK to change this one later? Will file a bug
to track it.

https://codereview.chromium.org/1687213002/diff/40001/media/audio/audio_outpu...
File media/audio/audio_output_stream_sink.cc (right):

https://codereview.chromium.org/1687213002/diff/40001/media/audio/audio_outpu...
media/audio/audio_output_stream_sink.cc:89: return
active_render_callback_->Render(dest, frames_delayed, 0);
frames_skipped is not propagated? Anyone know why?

[DaleCurtis, 2016-02-11 01:57:16]

lgtm % suggestions and cleanup.

https://codereview.chromium.org/1687213002/diff/40001/media/audio/audio_outpu...
File media/audio/audio_output_device.cc (right):

https://codereview.chromium.org/1687213002/diff/40001/media/audio/audio_outpu...
media/audio/audio_output_device.cc:10: 
#include cmath for std::round (which is C++11) now.

https://codereview.chromium.org/1687213002/diff/40001/media/audio/audio_outpu...
media/audio/audio_output_device.cc:444:
render_callback_->Render(output_bus_.get(), std::round(frames_delayed),
You can also just use frames_delayed + 0.5 since that will truncate into an
integer correctly :)

https://codereview.chromium.org/1687213002/diff/40001/media/audio/audio_outpu...
File media/audio/audio_output_stream_sink.cc (right):

https://codereview.chromium.org/1687213002/diff/40001/media/audio/audio_outpu...
media/audio/audio_output_stream_sink.cc:89: return
active_render_callback_->Render(dest, frames_delayed, 0);
On 2016/02/11 01:25:42, chcunningham wrote:
> frames_skipped is not propagated? Anyone know why?

This is only used for Mojo demos currently I think. Feel free to forward it.

https://codereview.chromium.org/1687213002/diff/40001/media/base/audio_render...
File media/base/audio_renderer_mixer.cc (right):

https://codereview.chromium.org/1687213002/diff/40001/media/base/audio_render...
media/base/audio_renderer_mixer.cc:20: microseconds_per_frame_(
Should we instead have a AudioParameters::GetMicrosecondsPerSecond() call which
returns a double? Seems there might be quite a few call sites.

https://codereview.chromium.org/1687213002/diff/40001/media/base/audio_render...
media/base/audio_renderer_mixer.cc:140: 
Mark as TODO: Remove conversion once AudioConverter supports frames instead of
TimeDelta?

https://codereview.chromium.org/1687213002/diff/40001/media/base/audio_render...
File media/base/audio_renderer_mixer_input.cc (right):

https://codereview.chromium.org/1687213002/diff/40001/media/base/audio_render...
media/base/audio_renderer_mixer_input.cc:168: double microseconds_per_frame =
Ditto on the TODO.

[chcunningham, 2016-02-11 21:02:52]

Thanks Dale.

Still need to fix that unit test, but all other comments addressed.

https://codereview.chromium.org/1687213002/diff/40001/content/renderer/media/...
File content/renderer/media/renderer_webaudiodevice_impl.cc (right):

https://codereview.chromium.org/1687213002/diff/40001/content/renderer/media/...
content/renderer/media/renderer_webaudiodevice_impl.cc:103: uint32_t
frames_delayed,
On 2016/02/11 01:25:42, chcunningham wrote:
> This method isn't making use of delay... should it be?

@dale, your thoughts?

https://codereview.chromium.org/1687213002/diff/40001/media/audio/audio_outpu...
File media/audio/audio_output_device.cc (right):

https://codereview.chromium.org/1687213002/diff/40001/media/audio/audio_outpu...
media/audio/audio_output_device.cc:10: 
On 2016/02/11 01:57:15, DaleCurtis wrote:
> #include cmath for std::round (which is C++11) now.

Done.

https://codereview.chromium.org/1687213002/diff/40001/media/audio/audio_outpu...
media/audio/audio_output_device.cc:444:
render_callback_->Render(output_bus_.get(), std::round(frames_delayed),
On 2016/02/11 01:57:15, DaleCurtis wrote:
> You can also just use frames_delayed + 0.5 since that will truncate into an
> integer correctly :)

Ack. If you don't object I prefer round though... slightly more readable. Added
cmath for now.

https://codereview.chromium.org/1687213002/diff/40001/media/audio/audio_outpu...
File media/audio/audio_output_stream_sink.cc (right):

https://codereview.chromium.org/1687213002/diff/40001/media/audio/audio_outpu...
media/audio/audio_output_stream_sink.cc:89: return
active_render_callback_->Render(dest, frames_delayed, 0);
On 2016/02/11 01:57:15, DaleCurtis wrote:
> On 2016/02/11 01:25:42, chcunningham wrote:
> > frames_skipped is not propagated? Anyone know why?
> 
> This is only used for Mojo demos currently I think. Feel free to forward it.

Done.

https://codereview.chromium.org/1687213002/diff/40001/media/base/audio_render...
File media/base/audio_renderer_mixer.cc (right):

https://codereview.chromium.org/1687213002/diff/40001/media/base/audio_render...
media/base/audio_renderer_mixer.cc:20: microseconds_per_frame_(
On 2016/02/11 01:57:15, DaleCurtis wrote:
> Should we instead have a AudioParameters::GetMicrosecondsPerSecond() call
which
> returns a double? Seems there might be quite a few call sites.

Done.

https://codereview.chromium.org/1687213002/diff/40001/media/base/audio_render...
media/base/audio_renderer_mixer.cc:140: 
On 2016/02/11 01:57:15, DaleCurtis wrote:
> Mark as TODO: Remove conversion once AudioConverter supports frames instead of
> TimeDelta?

Done.

https://codereview.chromium.org/1687213002/diff/40001/media/base/audio_render...
File media/base/audio_renderer_mixer_input.cc (right):

https://codereview.chromium.org/1687213002/diff/40001/media/base/audio_render...
media/base/audio_renderer_mixer_input.cc:168: double microseconds_per_frame =
On 2016/02/11 01:57:16, DaleCurtis wrote:
> Ditto on the TODO.

Done.

[DaleCurtis, 2016-02-11 21:04:19]

dalecurtis@chromium.org changed reviewers:
+ rtoy@chromium.org

[DaleCurtis, 2016-02-11 21:04:19]

https://codereview.chromium.org/1687213002/diff/40001/content/renderer/media/...
File content/renderer/media/renderer_webaudiodevice_impl.cc (right):

https://codereview.chromium.org/1687213002/diff/40001/content/renderer/media/...
content/renderer/media/renderer_webaudiodevice_impl.cc:103: uint32_t
frames_delayed,
On 2016/02/11 21:02:52, chcunningham wrote:
> On 2016/02/11 01:25:42, chcunningham wrote:
> > This method isn't making use of delay... should it be?
> 
> @dale, your thoughts?

Oh didn't realize that question was for me. +rtoy

[Raymond Toy, 2016-02-11 21:12:41]

On 2016/02/11 21:04:19, DaleCurtis wrote:
>
https://codereview.chromium.org/1687213002/diff/40001/content/renderer/media/...
> File content/renderer/media/renderer_webaudiodevice_impl.cc (right):
> 
>
https://codereview.chromium.org/1687213002/diff/40001/content/renderer/media/...
> content/renderer/media/renderer_webaudiodevice_impl.cc:103: uint32_t
> frames_delayed,
> On 2016/02/11 21:02:52, chcunningham wrote:
> > On 2016/02/11 01:25:42, chcunningham wrote:
> > > This method isn't making use of delay... should it be?
> > 
> > @dale, your thoughts?
> 
> Oh didn't realize that question was for me. +rtoy

I don't actually understand what you're supposed to do with this delay (in
frames or microseconds), so I don't have any good comments on what this should
do.

I do, however, agree with using frames instead of milliseconds.  WebAudio's
internal clock counter used to be a double to count time in seconds.  Things got
much more predictable and consistent when the counter was changed to be a frame
counter.  No more roundoff issues with the clock (except when converting from
frames to seconds).

[chcunningham, 2016-02-11 22:26:05]

On 2016/02/11 21:12:41, Raymond Toy wrote:
> On 2016/02/11 21:04:19, DaleCurtis wrote:
> >
>
https://codereview.chromium.org/1687213002/diff/40001/content/renderer/media/...
> > File content/renderer/media/renderer_webaudiodevice_impl.cc (right):
> > 
> >
>
https://codereview.chromium.org/1687213002/diff/40001/content/renderer/media/...
> > content/renderer/media/renderer_webaudiodevice_impl.cc:103: uint32_t
> > frames_delayed,
> > On 2016/02/11 21:02:52, chcunningham wrote:
> > > On 2016/02/11 01:25:42, chcunningham wrote:
> > > > This method isn't making use of delay... should it be?
> > > 
> > > @dale, your thoughts?
> > 
> > Oh didn't realize that question was for me. +rtoy
> 
> I don't actually understand what you're supposed to do with this delay (in
> frames or microseconds), so I don't have any good comments on what this should
> do.
> 

This delay represents the number of frames since the last render call that have
not yet played out in the speaker. Its initially provided by the hardware api's
(e.g. pulse::GetHardwareLatencyInBytes) and we may add on additional delay as
part of mixing/resampling. For audio/video tag playback, the delay makes its way
to AudioRendererImpl::Render and ultimately into AudioClock::WroteAudio.
AudioClock uses the delay to refine its notion of current media playback time,
which is key to audio video synchronization. 

I'm not at all familiar with WebAudio though, so I can't say what roll it should
play for you guys. 

> I do, however, agree with using frames instead of milliseconds.  WebAudio's
> internal clock counter used to be a double to count time in seconds.  Things
got
> much more predictable and consistent when the counter was changed to be a
frame
> counter.  No more roundoff issues with the clock (except when converting from
> frames to seconds).

Sweet :)

[Raymond Toy, 2016-02-11 22:35:05]

On 2016/02/11 22:26:05, chcunningham wrote:
> On 2016/02/11 21:12:41, Raymond Toy wrote:
> > On 2016/02/11 21:04:19, DaleCurtis wrote:
> > >
> >
>
https://codereview.chromium.org/1687213002/diff/40001/content/renderer/media/...
> > > File content/renderer/media/renderer_webaudiodevice_impl.cc (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/1687213002/diff/40001/content/renderer/media/...
> > > content/renderer/media/renderer_webaudiodevice_impl.cc:103: uint32_t
> > > frames_delayed,
> > > On 2016/02/11 21:02:52, chcunningham wrote:
> > > > On 2016/02/11 01:25:42, chcunningham wrote:
> > > > > This method isn't making use of delay... should it be?
> > > > 
> > > > @dale, your thoughts?
> > > 
> > > Oh didn't realize that question was for me. +rtoy
> > 
> > I don't actually understand what you're supposed to do with this delay (in
> > frames or microseconds), so I don't have any good comments on what this
should
> > do.
> > 
> 
> This delay represents the number of frames since the last render call that
have
> not yet played out in the speaker. Its initially provided by the hardware
api's
> (e.g. pulse::GetHardwareLatencyInBytes) and we may add on additional delay as
> part of mixing/resampling. For audio/video tag playback, the delay makes its
way
> to AudioRendererImpl::Render and ultimately into AudioClock::WroteAudio.
> AudioClock uses the delay to refine its notion of current media playback time,
> which is key to audio video synchronization.

Ah, this is really apropos right now because the WebAudio group is trying to
spec 
a way to allow developers to synchronize performanceNow time and the audio
output
time (when the audio actually gets to the speakers).

But this isn't relevant to this CL. 
> 
> I'm not at all familiar with WebAudio though, so I can't say what roll it
should
> play for you guys. 
> 
> > I do, however, agree with using frames instead of milliseconds.  WebAudio's
> > internal clock counter used to be a double to count time in seconds.  Things
> got
> > much more predictable and consistent when the counter was changed to be a
> frame
> > counter.  No more roundoff issues with the clock (except when converting
from
> > frames to seconds).
> 
> Sweet :)

[Henrik Grunell, 2016-02-12 17:46:14]

On 2016/02/11 22:35:05, Raymond Toy wrote:
> On 2016/02/11 22:26:05, chcunningham wrote:
> > On 2016/02/11 21:12:41, Raymond Toy wrote:
> > > On 2016/02/11 21:04:19, DaleCurtis wrote:
> > > >
> > >
> >
>
https://codereview.chromium.org/1687213002/diff/40001/content/renderer/media/...
> > > > File content/renderer/media/renderer_webaudiodevice_impl.cc (right):
> > > > 
> > > >
> > >
> >
>
https://codereview.chromium.org/1687213002/diff/40001/content/renderer/media/...
> > > > content/renderer/media/renderer_webaudiodevice_impl.cc:103: uint32_t
> > > > frames_delayed,
> > > > On 2016/02/11 21:02:52, chcunningham wrote:
> > > > > On 2016/02/11 01:25:42, chcunningham wrote:
> > > > > > This method isn't making use of delay... should it be?
> > > > > 
> > > > > @dale, your thoughts?
> > > > 
> > > > Oh didn't realize that question was for me. +rtoy
> > > 
> > > I don't actually understand what you're supposed to do with this delay (in
> > > frames or microseconds), so I don't have any good comments on what this
> should
> > > do.
> > > 
> > 
> > This delay represents the number of frames since the last render call that
> have
> > not yet played out in the speaker. Its initially provided by the hardware
> api's
> > (e.g. pulse::GetHardwareLatencyInBytes) and we may add on additional delay
as
> > part of mixing/resampling. For audio/video tag playback, the delay makes its
> way
> > to AudioRendererImpl::Render and ultimately into AudioClock::WroteAudio.
> > AudioClock uses the delay to refine its notion of current media playback
time,
> > which is key to audio video synchronization.
> 
> Ah, this is really apropos right now because the WebAudio group is trying to
> spec 
> a way to allow developers to synchronize performanceNow time and the audio
> output
> time (when the audio actually gets to the speakers).

In the WebRTC case, it's also used as input to the echo canceler to inform about
the echo path delay.

> 
> But this isn't relevant to this CL. 
> > 
> > I'm not at all familiar with WebAudio though, so I can't say what roll it
> should
> > play for you guys. 
> > 
> > > I do, however, agree with using frames instead of milliseconds. 
WebAudio's
> > > internal clock counter used to be a double to count time in seconds. 
Things
> > got
> > > much more predictable and consistent when the counter was changed to be a
> > frame
> > > counter.  No more roundoff issues with the clock (except when converting
> from
> > > frames to seconds).
> > 
> > Sweet :)

[Henrik Grunell, 2016-02-12 17:47:01]

Nice change. :)

https://codereview.chromium.org/1687213002/diff/40001/content/renderer/media/...
File content/renderer/media/webrtc_audio_renderer.cc (right):

https://codereview.chromium.org/1687213002/diff/40001/content/renderer/media/...
content/renderer/media/webrtc_audio_renderer.cc:433: // TODO(X): Converting from
frames to milliseconds will potentially lose
On 2016/02/11 01:25:42, chcunningham wrote:
> X should be one of the RTC folks. Henrik? 
> 
> Can you file a bug in RTC to do this cleanup?

Put me as reference. Filed 586540. Thanks for pointing this out!

https://codereview.chromium.org/1687213002/diff/40001/content/renderer/media/...
content/renderer/media/webrtc_audio_renderer.cc:492: // TODO(X): This integer
division by sample_rate will cause loss of partial
On 2016/02/11 01:25:42, chcunningham wrote:
> Ditto.

Use same bug.

https://codereview.chromium.org/1687213002/diff/40001/media/audio/audio_input...
File media/audio/audio_input_device.cc (left):

https://codereview.chromium.org/1687213002/diff/40001/media/audio/audio_input...
media/audio/audio_input_device.cc:342: buffer->params.hardware_delay_bytes /
bytes_per_ms_,  // Delay in ms
On 2016/02/11 01:25:42, chcunningham wrote:
> Another API expecing msec. Everyone OK to change this one later? Will file a
bug
> to track it.

sgtm

https://codereview.chromium.org/1687213002/diff/40001/media/audio/audio_outpu...
File media/audio/audio_output_stream_sink.cc (right):

https://codereview.chromium.org/1687213002/diff/40001/media/audio/audio_outpu...
media/audio/audio_output_stream_sink.cc:89: return
active_render_callback_->Render(dest, frames_delayed, 0);
On 2016/02/11 21:02:52, chcunningham wrote:
> On 2016/02/11 01:57:15, DaleCurtis wrote:
> > On 2016/02/11 01:25:42, chcunningham wrote:
> > > frames_skipped is not propagated? Anyone know why?
> > 
> > This is only used for Mojo demos currently I think. Feel free to forward it.
> 
> Done.

|frames_skipped| is only used for WebRTC. Other clients may possibly use it in
the future.

https://codereview.chromium.org/1687213002/diff/60001/media/base/audio_render...
File media/base/audio_renderer_mixer_input.cc (right):

https://codereview.chromium.org/1687213002/diff/60001/media/base/audio_render...
media/base/audio_renderer_mixer_input.cc:172: uint32_t frames_delayed =
std::round(buffer_delay.InMicroseconds() /
Any reason to prefer std::round over + 0.5?

[chcunningham, 2016-02-17 01:56:52]

Thanks Henrik!

https://codereview.chromium.org/1687213002/diff/40001/content/renderer/media/...
File content/renderer/media/webrtc_audio_renderer.cc (right):

https://codereview.chromium.org/1687213002/diff/40001/content/renderer/media/...
content/renderer/media/webrtc_audio_renderer.cc:433: // TODO(X): Converting from
frames to milliseconds will potentially lose
On 2016/02/12 17:47:01, Henrik Grunell wrote:
> On 2016/02/11 01:25:42, chcunningham wrote:
> > X should be one of the RTC folks. Henrik? 
> > 
> > Can you file a bug in RTC to do this cleanup?
> 
> Put me as reference. Filed 586540. Thanks for pointing this out!

Done.

https://codereview.chromium.org/1687213002/diff/40001/content/renderer/media/...
content/renderer/media/webrtc_audio_renderer.cc:492: // TODO(X): This integer
division by sample_rate will cause loss of partial
On 2016/02/12 17:47:01, Henrik Grunell wrote:
> On 2016/02/11 01:25:42, chcunningham wrote:
> > Ditto.
> 
> Use same bug.

Done.

https://codereview.chromium.org/1687213002/diff/40001/media/audio/audio_input...
File media/audio/audio_input_device.cc (left):

https://codereview.chromium.org/1687213002/diff/40001/media/audio/audio_input...
media/audio/audio_input_device.cc:342: buffer->params.hardware_delay_bytes /
bytes_per_ms_,  // Delay in ms
On 2016/02/12 17:47:01, Henrik Grunell wrote:
> On 2016/02/11 01:25:42, chcunningham wrote:
> > Another API expecing msec. Everyone OK to change this one later? Will file a
> bug
> > to track it.
> 
> sgtm

Filed 587291. Added comment.

https://codereview.chromium.org/1687213002/diff/40001/media/audio/audio_outpu...
File media/audio/audio_output_stream_sink.cc (right):

https://codereview.chromium.org/1687213002/diff/40001/media/audio/audio_outpu...
media/audio/audio_output_stream_sink.cc:89: return
active_render_callback_->Render(dest, frames_delayed, 0);
On 2016/02/12 17:47:01, Henrik Grunell wrote:
> On 2016/02/11 21:02:52, chcunningham wrote:
> > On 2016/02/11 01:57:15, DaleCurtis wrote:
> > > On 2016/02/11 01:25:42, chcunningham wrote:
> > > > frames_skipped is not propagated? Anyone know why?
> > > 
> > > This is only used for Mojo demos currently I think. Feel free to forward
it.
> > 
> > Done.
> 
> |frames_skipped| is only used for WebRTC. Other clients may possibly use it in
> the future.

Acknowledged.

https://codereview.chromium.org/1687213002/diff/60001/media/base/audio_render...
File media/base/audio_renderer_mixer_input.cc (right):

https://codereview.chromium.org/1687213002/diff/60001/media/base/audio_render...
media/base/audio_renderer_mixer_input.cc:172: uint32_t frames_delayed =
std::round(buffer_delay.InMicroseconds() /
On 2016/02/12 17:47:01, Henrik Grunell wrote:
> Any reason to prefer std::round over + 0.5?

:) Dale asked the same. C-programmer style is not my favorite. I think this is
more readable and I don't see any clear advantage to the alternative.

[Henrik Grunell, 2016-02-17 08:13:48]

lgtm

https://codereview.chromium.org/1687213002/diff/60001/media/base/audio_render...
File media/base/audio_renderer_mixer_input.cc (right):

https://codereview.chromium.org/1687213002/diff/60001/media/base/audio_render...
media/base/audio_renderer_mixer_input.cc:172: uint32_t frames_delayed =
std::round(buffer_delay.InMicroseconds() /
On 2016/02/17 01:56:52, chcunningham wrote:
> On 2016/02/12 17:47:01, Henrik Grunell wrote:
> > Any reason to prefer std::round over + 0.5?
> 
> :) Dale asked the same. C-programmer style is not my favorite. I think this is
> more readable and I don't see any clear advantage to the alternative. 

Fine with me. :) I assume std::round is efficient (enough).

https://codereview.chromium.org/1687213002/diff/100001/media/base/audio_rende...
File media/base/audio_renderer_mixer.cc (right):

https://codereview.chromium.org/1687213002/diff/100001/media/base/audio_rende...
media/base/audio_renderer_mixer.cc:138: // expect a count of frames delayed
instead of TimeDelta (less precise).
Did you have a bug for this?

https://codereview.chromium.org/1687213002/diff/100001/media/base/audio_rende...
File media/base/audio_renderer_mixer_input.cc (right):

https://codereview.chromium.org/1687213002/diff/100001/media/base/audio_rende...
media/base/audio_renderer_mixer_input.cc:171: // precisely describe delay as a
count of frames delayed instead of TimeDelta.
Is there a bug for this?

[chcunningham, 2016-02-17 18:32:37]

Thanks Henrik!

https://codereview.chromium.org/1687213002/diff/100001/media/base/audio_rende...
File media/base/audio_renderer_mixer.cc (right):

https://codereview.chromium.org/1687213002/diff/100001/media/base/audio_rende...
media/base/audio_renderer_mixer.cc:138: // expect a count of frames delayed
instead of TimeDelta (less precise).
On 2016/02/17 08:13:47, Henrik Grunell wrote:
> Did you have a bug for this?

Done. http://crbug.com/587522. Updated the comment

https://codereview.chromium.org/1687213002/diff/100001/media/base/audio_rende...
File media/base/audio_renderer_mixer_input.cc (right):

https://codereview.chromium.org/1687213002/diff/100001/media/base/audio_rende...
media/base/audio_renderer_mixer_input.cc:171: // precisely describe delay as a
count of frames delayed instead of TimeDelta.
On 2016/02/17 08:13:47, Henrik Grunell wrote:
> Is there a bug for this?

Done. http://crbug.com/587522. Updated comment.

[chcunningham, 2016-02-17 18:39:07]

The CQ bit was checked by chcunningham@chromium.org

[chcunningham, 2016-02-17 18:39:07]

The patchset sent to the CQ was uploaded after l-g-t-m from
dalecurtis@chromium.org, grunell@chromium.org
Link to the patchset: https://codereview.chromium.org/1687213002/#ps140001
(title: "Rebase")

[chcunningham, 2016-02-17 18:39:09]

The CQ bit was unchecked by chcunningham@chromium.org

[chcunningham, 2016-02-17 18:40:13]

Description was changed from

==========
Express audio delay more precisely in frames rather than milliseconds.

This is a first step to reduce drift in audio video synchronization.
With this change, drift in a 2 hour file will be mostly eliminated if
the media sample rate matches that of the hardware (no resampling).

Other sources of drift will be fixed in follow up cls.

BUG=564604
TEST=webm file from reporter in http://crbug.com/564604#c9
==========

to

==========
Express audio delay more precisely in frames rather than milliseconds.

This is a first step to reduce drift in audio video synchronization.
With this change, drift in a 2 hour file will be mostly eliminated if
the media sample rate matches that of the hardware (no resampling).

Other sources of drift will be fixed in follow up cls.

BUG=564604,587522,586540
TEST=webm file from reporter in http://crbug.com/564604#c9
==========

[chcunningham, 2016-02-17 18:40:46]

The CQ bit was checked by chcunningham@chromium.org

[commit-bot: I haz the power, 2016-02-17 18:45:19]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1687213002/140001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1687213002/140001

[commit-bot: I haz the power, 2016-02-17 20:24:41]

Description was changed from

==========
Express audio delay more precisely in frames rather than milliseconds.

This is a first step to reduce drift in audio video synchronization.
With this change, drift in a 2 hour file will be mostly eliminated if
the media sample rate matches that of the hardware (no resampling).

Other sources of drift will be fixed in follow up cls.

BUG=564604,587522,586540
TEST=webm file from reporter in http://crbug.com/564604#c9
==========

to

==========
Express audio delay more precisely in frames rather than milliseconds.

This is a first step to reduce drift in audio video synchronization.
With this change, drift in a 2 hour file will be mostly eliminated if
the media sample rate matches that of the hardware (no resampling).

Other sources of drift will be fixed in follow up cls.

BUG=564604,587522,586540
TEST=webm file from reporter in http://crbug.com/564604#c9
==========

[commit-bot: I haz the power, 2016-02-17 20:24:42]

Committed patchset #6 (id:140001)

[commit-bot: I haz the power, 2016-02-17 20:25:44]

Description was changed from

==========
Express audio delay more precisely in frames rather than milliseconds.

This is a first step to reduce drift in audio video synchronization.
With this change, drift in a 2 hour file will be mostly eliminated if
the media sample rate matches that of the hardware (no resampling).

Other sources of drift will be fixed in follow up cls.

BUG=564604,587522,586540
TEST=webm file from reporter in http://crbug.com/564604#c9
==========

to

==========
Express audio delay more precisely in frames rather than milliseconds.

This is a first step to reduce drift in audio video synchronization.
With this change, drift in a 2 hour file will be mostly eliminated if
the media sample rate matches that of the hardware (no resampling).

Other sources of drift will be fixed in follow up cls.

BUG=564604,587522,586540
TEST=webm file from reporter in http://crbug.com/564604#c9

Committed: https://crrev.com/7bdc23eebf424b510e57e33d05603ced780808db
Cr-Commit-Position: refs/heads/master@{#375967}
==========

[commit-bot: I haz the power, 2016-02-17 20:25:45]

Patchset 6 (id:??) landed as
https://crrev.com/7bdc23eebf424b510e57e33d05603ced780808db
Cr-Commit-Position: refs/heads/master@{#375967}