Switch to double for time calculations using playback rate.

Switch to double for time calculations using playback rate.

Multiplying an int64 against a float causes the float type to prevail which does not have enough precision for accurate time value computations.

BUG=475964
TBR=lcwu1, gunsch

Committed: https://crrev.com/338bf0048faab4c20b0767620c524b9636c31a04
Cr-Commit-Position: refs/heads/master@{#326269}

[a.berwal, 2015-04-17 03:14:52]

a.berwal@samsung.com changed reviewers:
+ xhwang@chromium.org

[a.berwal, 2015-04-17 05:43:55]

a.berwal@samsung.com changed reviewers:
+ dalecurtis@chromium.org

[DaleCurtis, 2015-04-17 18:21:37]

Thanks for doing this!

https://codereview.chromium.org/1094783002/diff/1/AUTHORS
File AUTHORS (right):

https://codereview.chromium.org/1094783002/diff/1/AUTHORS#newcode22
AUTHORS:22: Ajay Berwal <a.berwal@samsung.com>
Doesn't look like you need another entry here? Or is that a different Ajay
Berwal above?

https://codereview.chromium.org/1094783002/diff/1/media/blink/buffered_data_s...
File media/blink/buffered_data_source_unittest.cc (right):

https://codereview.chromium.org/1094783002/diff/1/media/blink/buffered_data_s...
media/blink/buffered_data_source_unittest.cc:237: float
data_source_playback_rate() { return data_source_->playback_rate_; }
double?

https://codereview.chromium.org/1094783002/diff/1/media/blink/buffered_data_s...
media/blink/buffered_data_source_unittest.cc:239: float loader_playback_rate() {
return loader()->playback_rate_; }
double?

[a.berwal, 2015-04-19 03:53:55]

https://codereview.chromium.org/1094783002/diff/1/AUTHORS
File AUTHORS (right):

https://codereview.chromium.org/1094783002/diff/1/AUTHORS#newcode22
AUTHORS:22: Ajay Berwal <a.berwal@samsung.com>
On 2015/04/17 18:21:37, DaleCurtis wrote:
> Doesn't look like you need another entry here? Or is that a different Ajay
> Berwal above?

That is different one..

https://codereview.chromium.org/1094783002/diff/1/media/blink/buffered_data_s...
File media/blink/buffered_data_source_unittest.cc (right):

https://codereview.chromium.org/1094783002/diff/1/media/blink/buffered_data_s...
media/blink/buffered_data_source_unittest.cc:237: float
data_source_playback_rate() { return data_source_->playback_rate_; }
On 2015/04/17 18:21:37, DaleCurtis wrote:
> double?

this function is not used anywhere for multiplying with int64, so there is no
harm to keep it float.

https://codereview.chromium.org/1094783002/diff/1/media/blink/buffered_data_s...
media/blink/buffered_data_source_unittest.cc:239: float loader_playback_rate() {
return loader()->playback_rate_; }
On 2015/04/17 18:21:37, DaleCurtis wrote:
> double?


this function is not used anywhere for multiplying with int64, so there is no
harm to keep it float.

[xhwang, 2015-04-20 21:47:07]

We have more places in media code that use float for the playback rate. We
should probably replace all of those in one refactoring CL, e.g.

https://code.google.com/p/chromium/codesearch#chromium/src/media/base/pipelin...

Dale: WDYT?

https://codereview.chromium.org/1094783002/diff/1/media/blink/buffered_data_s...
File media/blink/buffered_data_source_unittest.cc (right):

https://codereview.chromium.org/1094783002/diff/1/media/blink/buffered_data_s...
media/blink/buffered_data_source_unittest.cc:237: float
data_source_playback_rate() { return data_source_->playback_rate_; }
On 2015/04/19 03:53:55, a.berwal wrote:
> On 2015/04/17 18:21:37, DaleCurtis wrote:
> > double?
> 
> this function is not used anywhere for multiplying with int64, so there is no
> harm to keep it float.

Let's just keep using double for playback rates in all places to avoid
confusion.

https://codereview.chromium.org/1094783002/diff/1/media/blink/buffered_data_s...
media/blink/buffered_data_source_unittest.cc:239: float loader_playback_rate() {
return loader()->playback_rate_; }
On 2015/04/19 03:53:55, a.berwal wrote:
> On 2015/04/17 18:21:37, DaleCurtis wrote:
> > double?
> 
> 
> this function is not used anywhere for multiplying with int64, so there is no
> harm to keep it float.

ditto

[a.berwal, 2015-04-21 04:50:35]

On 2015/04/20 21:47:07, xhwang wrote:
> We have more places in media code that use float for the playback rate. We
> should probably replace all of those in one refactoring CL, e.g.
> 
>
https://code.google.com/p/chromium/codesearch#chromium/src/media/base/pipelin...
> 
> Dale: WDYT?
> 
>
https://codereview.chromium.org/1094783002/diff/1/media/blink/buffered_data_s...
> File media/blink/buffered_data_source_unittest.cc (right):
> 
>
https://codereview.chromium.org/1094783002/diff/1/media/blink/buffered_data_s...
> media/blink/buffered_data_source_unittest.cc:237: float
> data_source_playback_rate() { return data_source_->playback_rate_; }
> On 2015/04/19 03:53:55, a.berwal wrote:
> > On 2015/04/17 18:21:37, DaleCurtis wrote:
> > > double?
> > 
> > this function is not used anywhere for multiplying with int64, so there is
no
> > harm to keep it float.
> 
> Let's just keep using double for playback rates in all places to avoid
> confusion.
submitted new patchset for replacing all float playback_rate to double.
>
https://codereview.chromium.org/1094783002/diff/1/media/blink/buffered_data_s...
> media/blink/buffered_data_source_unittest.cc:239: float loader_playback_rate()
{
> return loader()->playback_rate_; }
> On 2015/04/19 03:53:55, a.berwal wrote:
> > On 2015/04/17 18:21:37, DaleCurtis wrote:
> > > double?
> > 
> > 
> > this function is not used anywhere for multiplying with int64, so there is
no
> > harm to keep it float.
> 
> ditto

[DaleCurtis, 2015-04-22 00:52:59]

Thanks for doing this. looks good % your CLA agreement.

https://codereview.chromium.org/1094783002/diff/20001/AUTHORS
File AUTHORS (right):

https://codereview.chromium.org/1094783002/diff/20001/AUTHORS#newcode22
AUTHORS:22: Ajay Berwal <a.berwal@samsung.com>
Have you filled out the CLA?
https://cla.developers.google.com/about/google-corporate?csw=1

[a.berwal, 2015-04-22 03:06:15]

On 2015/04/22 00:52:59, DaleCurtis wrote:

Yes, CLA has been filled.

[DaleCurtis, 2015-04-22 03:29:42]

Awesome, lgtm!

[xhwang, 2015-04-22 04:21:15]

lgtm % tiny nits

Thanks for doing this!

https://codereview.chromium.org/1094783002/diff/20001/media/base/pipeline.h
File media/base/pipeline.h (right):

https://codereview.chromium.org/1094783002/diff/20001/media/base/pipeline.h#n...
media/base/pipeline.h:147: // started, the playback rate will be 0.0f.  A rate
of 1.0f indicates
nit: here and below, s/0.0f/0.0

https://codereview.chromium.org/1094783002/diff/20001/media/base/pipeline_uni...
File media/base/pipeline_unittest.cc (right):

https://codereview.chromium.org/1094783002/diff/20001/media/base/pipeline_uni...
media/base/pipeline_unittest.cc:646: double playback_rate = 1.0f;
drop "f"

[a.berwal, 2015-04-22 05:24:34]

The CQ bit was checked by a.berwal@samsung.com

[a.berwal, 2015-04-22 05:24:34]

The patchset sent to the CQ was uploaded after l-g-t-m from xhwang@chromium.org,
dalecurtis@chromium.org
Link to the patchset: https://chromiumcodereview.appspot.com/1094783002/#ps40001
(title: "Incorporating Review Comments")

[commit-bot: I haz the power, 2015-04-22 05:24:46]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1094783002/40001

[commit-bot: I haz the power, 2015-04-22 05:37:38]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-04-22 05:37:39]

Try jobs failed on following builders:
  linux_chromium_gn_dbg on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[a.berwal, 2015-04-22 08:17:56]

The CQ bit was checked by a.berwal@samsung.com

[a.berwal, 2015-04-22 08:17:57]

The patchset sent to the CQ was uploaded after l-g-t-m from xhwang@chromium.org,
dalecurtis@chromium.org
Link to the patchset: https://chromiumcodereview.appspot.com/1094783002/#ps60001
(title: "build fixes")

[commit-bot: I haz the power, 2015-04-22 08:18:32]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1094783002/60001

[commit-bot: I haz the power, 2015-04-22 08:52:00]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-04-22 08:52:01]

Try jobs failed on following builders:
  cast_shell_linux on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)

[a.berwal, 2015-04-22 09:45:56]

The CQ bit was checked by a.berwal@samsung.com

[a.berwal, 2015-04-22 09:45:57]

The patchset sent to the CQ was uploaded after l-g-t-m from xhwang@chromium.org,
dalecurtis@chromium.org
Link to the patchset: https://chromiumcodereview.appspot.com/1094783002/#ps80001
(title: "Making changes at chromecast side to fix trybots")

[commit-bot: I haz the power, 2015-04-22 09:46:29]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1094783002/80001

[commit-bot: I haz the power, 2015-04-22 09:57:36]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-04-22 09:57:37]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[a.berwal, 2015-04-22 10:02:30]

a.berwal@samsung.com changed reviewers:
+ gunsch@chromium.org, lcwu@chromium.org

[a.berwal, 2015-04-22 10:04:01]

@lcwu,@gunsch PTAL converting float playback_rate to double in chromecast/.

[a.berwal, 2015-04-22 11:10:52]

The CQ bit was checked by a.berwal@samsung.com

[commit-bot: I haz the power, 2015-04-22 11:11:00]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1094783002/80001

[commit-bot: I haz the power, 2015-04-22 11:15:00]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2015-04-22 11:15:55]

Patchset 5 (id:??) landed as
https://crrev.com/338bf0048faab4c20b0767620c524b9636c31a04
Cr-Commit-Position: refs/heads/master@{#326269}