1152359 - Incorrect calculation of doppler shift value

Status: RESOLVED FIXED

(Core :: Web Audio, defect)

Description

[Boris Egorov]

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Firefox/38.0 Iceweasel/38.0a2
Build ID: 20150227004023

Steps to reproduce:

From http://www.viva64.com/en/b/0262/

float
PannerNode::ComputeDopplerShift()
{
  ....
  double scaledSpeedOfSound = listener->DopplerFactor() /
                              listener->DopplerFactor();
  ....
}

V501 There are identical sub-expressions 'listener->DopplerFactor()' to the left and to the right of the '/' operator. pannernode.cpp 529


--

I've made an investigation and found detailed description of doppler shift calculation here: http://redmine.spatdif.org/projects/spatdif/wiki/Doppler_Extension. I didn't found any other source, so I'm not sure it is a correct formula. Anyway, I've made a patch to fix calculation. I believe Paul Adenot (:padenot) can check this calculation since he implemented original code (see #853551).

Blocks: #710966

Comment 1

[Boris Egorov]

Attachment: fix_doppler_shift_calculation.diff

Comment 2

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 8589686 [details] [diff] [review]
fix_doppler_shift_calculation.diff

If you think Paul should review this, you can ask him directly by setting the review flag appropriately :)

Comment 3

[Boris Egorov]

(In reply to :Gavin Sharp [email: [e-mail]] from comment #2)
> Comment on attachment 8589686 [details] [diff] [review]
> fix_doppler_shift_calculation.diff
> 
> If you think Paul should review this, you can ask him directly by setting
> the review flag appropriately :)

Ok. I set it to him first, and then I saw a list of 'recommended reviewers'. I selected you, thinking it will add a reviewer, not replace the first one.

Comment 4

[Paul Adenot (:padenot)]

Comment on attachment 8589686 [details] [diff] [review]
fix_doppler_shift_calculation.diff

Review of attachment 8589686 [details] [diff] [review]:
-----------------------------------------------------------------

This is indeed correct. Please not that we are deprecating the doppler effect in bug 1148354.

Comment 5

[Paul Adenot (:padenot)]

Thanks!

https://hg.mozilla.org/integration/mozilla-inbound/rev/fa63bc4130b9

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/fa63bc4130b9