Closed Bug 1152359 Opened 7 years ago Closed 7 years ago

Incorrect calculation of doppler shift value

Categories

(Core :: Web Audio, defect)

x86_64
Linux
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla40
Tracking Status
firefox40 --- fixed

People

(Reporter: egorov, Assigned: egorov)

References

Details

Attachments

(1 file)

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
Blocks: 710966
Attachment #8589686 - Flags: review?(gavin.sharp)
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 :)
Attachment #8589686 - Flags: review?(gavin.sharp) → review?(padenot)
Component: Untriaged → Video/Audio
Product: Firefox → Core
(In reply to :Gavin Sharp [email: gavin@gavinsharp.com] 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 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.
Attachment #8589686 - Flags: review?(padenot) → review+
Component: Video/Audio → Web Audio
Assignee: nobody → egorov
https://hg.mozilla.org/mozilla-central/rev/fa63bc4130b9
Status: UNCONFIRMED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla40
You need to log in before you can comment on or make changes to this bug.