Handle NaN in the Audio delay curves.

Handle NaN in the Audio delay curves.

Since switching from std::min to clampTo NaN has caused ASSERTs.
This restores the old behaviour of no ASSERT and a delay interpreted
as maxDelayTime.

BUG=582699
R=rtoy@chromium.org

Committed: https://crrev.com/f6e3d4665d9261e4fef2b5931e4c75ecb5e032bf
Cr-Commit-Position: refs/heads/master@{#372710}

[Daniel Bratell, 2016-02-01 16:08:12]

rtoy, can you take a look? This is just restoring the old behaviour but I think
there might be other options as well.

I see that you choose to just return early when handling NaN in
https://codereview.chromium.org/1657763002

[Daniel Bratell, 2016-02-01 16:11:25]

On 2016/02/01 16:08:12, Daniel Bratell wrote:
> rtoy, can you take a look? This is just restoring the old behaviour but I
think
> there might be other options as well.
> 
> I see that you choose to just return early when handling NaN in
> https://codereview.chromium.org/1657763002

I mean https://codereview.chromium.org/138943003

[Raymond Toy, 2016-02-01 17:13:26]

On 2016/02/01 16:11:25, Daniel Bratell wrote:
> On 2016/02/01 16:08:12, Daniel Bratell wrote:
> > rtoy, can you take a look? This is just restoring the old behaviour but I
> think
> > there might be other options as well.
> > 
> > I see that you choose to just return early when handling NaN in
> > https://codereview.chromium.org/1657763002
> 
> I mean https://codereview.chromium.org/138943003

lgtm.

But shouldn't clampTo be fixed not to ASSERT?  NaN is a perfectly valid number
that can't lie between the given limits.  I guess it's an issue on what value to
choose for the limit in this case, though.

[Raymond Toy, 2016-02-01 17:15:04]

Please fix the description before landing.  I'm not sure what

"This restores the old behaviour or no ASSERT and a delay interpreted"

means.  Did you mean "of" instead of "or"?

[Daniel Bratell, 2016-02-01 17:23:29]

Description was changed from

==========
Handle NaN in the Audio delay curves.

Since switching from std::min to clampTo NaN has caused ASSERTs.
This restores the old behaviour or no ASSERT and a delay interpreted
as maxDelayTime.

BUG=582699
R=rtoy@chromium.org
==========

to

==========
Handle NaN in the Audio delay curves.

Since switching from std::min to clampTo NaN has caused ASSERTs.
This restores the old behaviour of no ASSERT and a delay interpreted
as maxDelayTime.

BUG=582699
R=rtoy@chromium.org
==========

[Daniel Bratell, 2016-02-01 17:28:01]

On 2016/02/01 17:15:04, Raymond Toy wrote:
> Please fix the description before landing.  I'm not sure what
> 
> "This restores the old behaviour or no ASSERT and a delay interpreted"
> 
> means.  Did you mean "of" instead of "or"?

Yes. Thanks, fixed.

I think the problem is exactly what you mention, what to return for NaN. I had
to check for std::min, and std::min returns the first argument if either
argument is NaN. Well defined but I doubt many people know it (and I will have
forgotten tomorrow). I wonder how much code we have that depends on that without
knowing it. So forcing explicit handling of NaN is probably not bad.

[Daniel Bratell, 2016-02-01 17:28:09]

The CQ bit was checked by bratell@opera.com

[commit-bot: I haz the power, 2016-02-01 17:29:37]

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

[commit-bot: I haz the power, 2016-02-01 18:59:43]

Description was changed from

==========
Handle NaN in the Audio delay curves.

Since switching from std::min to clampTo NaN has caused ASSERTs.
This restores the old behaviour of no ASSERT and a delay interpreted
as maxDelayTime.

BUG=582699
R=rtoy@chromium.org
==========

to

==========
Handle NaN in the Audio delay curves.

Since switching from std::min to clampTo NaN has caused ASSERTs.
This restores the old behaviour of no ASSERT and a delay interpreted
as maxDelayTime.

BUG=582699
R=rtoy@chromium.org
==========

[commit-bot: I haz the power, 2016-02-01 18:59:44]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2016-02-01 19:00:52]

Description was changed from

==========
Handle NaN in the Audio delay curves.

Since switching from std::min to clampTo NaN has caused ASSERTs.
This restores the old behaviour of no ASSERT and a delay interpreted
as maxDelayTime.

BUG=582699
R=rtoy@chromium.org
==========

to

==========
Handle NaN in the Audio delay curves.

Since switching from std::min to clampTo NaN has caused ASSERTs.
This restores the old behaviour of no ASSERT and a delay interpreted
as maxDelayTime.

BUG=582699
R=rtoy@chromium.org

Committed: https://crrev.com/f6e3d4665d9261e4fef2b5931e4c75ecb5e032bf
Cr-Commit-Position: refs/heads/master@{#372710}
==========

[commit-bot: I haz the power, 2016-02-01 19:00:52]

Patchset 1 (id:??) landed as
https://crrev.com/f6e3d4665d9261e4fef2b5931e4c75ecb5e032bf
Cr-Commit-Position: refs/heads/master@{#372710}