Fix a numerical issue in NetEq delay plotting

Fix a numerical issue in NetEq delay plotting

Imprecisions in floating point representation caused noise in the
graphs. The integer division is in fact exact.

BUG=webrtc:7467
Review-Url: https://codereview.webrtc.org/2933053002
Cr-Commit-Position: refs/heads/master@{#18592}
Committed: https://chromium.googlesource.com/external/webrtc/+/4eccdaa31415375f18e927420119ab197737be69

[hlundin-webrtc, 2017-06-12 08:22:42]

henrik.lundin@webrtc.org changed reviewers:
+ ivoc@webrtc.org

[hlundin-webrtc, 2017-06-12 08:22:43]

Ivo,

PTAL at this minor CL.

Thanks!

[ivoc, 2017-06-12 10:19:20]

https://codereview.webrtc.org/2933053002/diff/1/webrtc/modules/audio_coding/n...
File webrtc/modules/audio_coding/neteq/tools/neteq_delay_analyzer.cc (left):

https://codereview.webrtc.org/2933053002/diff/1/webrtc/modules/audio_coding/n...
webrtc/modules/audio_coding/neteq/tools/neteq_delay_analyzer.cc:119:
(last_sample_rate_hz_ / 1000.f));
Are you sure this change has any effect? I tried this division for 8000.f,
16000.f, 32000.f and 48000.f, but the results are always exact (floats can
exactly represent all integers up to 16 million).

[hlundin-webrtc, 2017-06-12 11:54:58]

https://codereview.webrtc.org/2933053002/diff/1/webrtc/modules/audio_coding/n...
File webrtc/modules/audio_coding/neteq/tools/neteq_delay_analyzer.cc (left):

https://codereview.webrtc.org/2933053002/diff/1/webrtc/modules/audio_coding/n...
webrtc/modules/audio_coding/neteq/tools/neteq_delay_analyzer.cc:119:
(last_sample_rate_hz_ / 1000.f));
On 2017/06/12 10:19:19, ivoc wrote:
> Are you sure this change has any effect? I tried this division for 8000.f,
> 16000.f, 32000.f and 48000.f, but the results are always exact (floats can
> exactly represent all integers up to 16 million).

The machine instructions change quite a bit: https://godbolt.org/g/kfXdqX

If I understand correctly, the inner division by 1000 is transformed from a
floating point divss to a multiplication and shifts. The outer division is
changed from a floating point divss to an idiv. So, it does make a change.

With the old code, I get an alternating +/- offset of the results, which causes
a saw-tooth graph, without motivation. This change resolves that.

[ivoc, 2017-06-12 12:01:28]

On 2017/06/12 11:54:58, hlundin-webrtc wrote:
>
https://codereview.webrtc.org/2933053002/diff/1/webrtc/modules/audio_coding/n...
> File webrtc/modules/audio_coding/neteq/tools/neteq_delay_analyzer.cc (left):
> 
>
https://codereview.webrtc.org/2933053002/diff/1/webrtc/modules/audio_coding/n...
> webrtc/modules/audio_coding/neteq/tools/neteq_delay_analyzer.cc:119:
> (last_sample_rate_hz_ / 1000.f));
> On 2017/06/12 10:19:19, ivoc wrote:
> > Are you sure this change has any effect? I tried this division for 8000.f,
> > 16000.f, 32000.f and 48000.f, but the results are always exact (floats can
> > exactly represent all integers up to 16 million).
> 
> The machine instructions change quite a bit: https://godbolt.org/g/kfXdqX
> 
> If I understand correctly, the inner division by 1000 is transformed from a
> floating point divss to a multiplication and shifts. The outer division is
> changed from a floating point divss to an idiv. So, it does make a change.
> 
> With the old code, I get an alternating +/- offset of the results, which
causes
> a saw-tooth graph, without motivation. This change resolves that.

Ok, perhaps it depends on the applied optimizations. lgtm!

[hlundin-webrtc, 2017-06-14 13:34:56]

The CQ bit was checked by henrik.lundin@webrtc.org

[commit-bot: I haz the power, 2017-06-14 13:35:02]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[commit-bot: I haz the power, 2017-06-14 14:02:17]

CQ is committing da patch.

Bot data: {"patchset_id": 1, "attempt_start_ts": 1497447296834350, "parent_rev":
"7a721e84f84b5205ff260a34fb87bf8e56c85cd1", "commit_rev":
"4eccdaa31415375f18e927420119ab197737be69"}

[commit-bot: I haz the power, 2017-06-14 14:02:21]

Description was changed from

==========
Fix a numerical issue in NetEq delay plotting

Imprecisions in floating point representation caused noise in the
graphs. The integer division is in fact exact.

BUG= webrtc:7467
==========

to

==========
Fix a numerical issue in NetEq delay plotting

Imprecisions in floating point representation caused noise in the
graphs. The integer division is in fact exact.

BUG= webrtc:7467

Review-Url: https://codereview.webrtc.org/2933053002
Cr-Commit-Position: refs/heads/master@{#18592}
Committed:
https://chromium.googlesource.com/external/webrtc/+/4eccdaa31415375f18e927420...
==========

[commit-bot: I haz the power, 2017-06-14 14:02:22]

Committed patchset #1 (id:1) as
https://chromium.googlesource.com/external/webrtc/+/4eccdaa31415375f18e927420...