Remove the doppler effect from the PannerNode

NEW
Assigned to

Status

()

Core
Web Audio
P4
normal
3 years ago
7 months ago

People

(Reporter: padenot, Assigned: padenot)

Tracking

({leave-open})

32 Branch
x86_64
Linux
leave-open
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Assignee)

Description

3 years ago
This has always been a bit broken, and does not work anyways in Gecko because of a bug.

The WG resolution was during TPAC, Blink currently marks those members as "deprecated".

The spec bug is: https://github.com/WebAudio/web-audio-api/issues/455
(Assignee)

Comment 1

3 years ago
Created attachment 8584507 [details] [diff] [review]
Remove the doppler effect from the PannerNode. r=

This removes quite a bit of annoying code. I also believe this is not widely
used, but I don't have numbers, only anecdotal evidence (me searching github for
relevant keywords).
Attachment #8584507 - Flags: review?(ehsan)

Comment 2

3 years ago
Comment on attachment 8584507 [details] [diff] [review]
Remove the doppler effect from the PannerNode. r=

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

I think we should start by deprecating this with issuing warnings in the Web Console.  Preferrably we'd uplift that to get it into all channels as fast as possible, but please check the l10n requirements on that with the release managers (I think we're OK with English only messages targeted to web devs, not completely sure though.)
Attachment #8584507 - Flags: review?(ehsan) → review-
(Assignee)

Comment 3

3 years ago
Created attachment 8589603 [details] [diff] [review]
Deprecate the doppler effect from the PannerNode. r=

This is built on top of bug 1148496.
Attachment #8589603 - Flags: review?(ehsan)

Comment 4

3 years ago
Comment on attachment 8589603 [details] [diff] [review]
Deprecate the doppler effect from the PannerNode. r=

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

Very nice!

::: dom/locales/en-US/chrome/dom/dom.properties
@@ +162,5 @@
>  # LOCALIZATION NOTE (WillChangeBudgetWarning): Do not translate Will-change, %1$S,%2$S,%3$S are numbers.
>  WillChangeBudgetWarning=Will-change memory consumption is too high. Surface area covers %1$S pixels, budget is the document surface area multiplied by %2$S (%3$S pixels). All occurences of will-change in the document are ignored when over budget.
>  # LOCALIZATION NOTE: Do not translate "ServiceWorker".
>  HittingMaxWorkersPerDomain=A ServiceWorker could not be started immediately because other documents in the same origin are already using the maximum number of workers. The ServiceWorker is now queued and will be started after some of the other workers have completed.
> +# LOCALIZATION: Do no translate "setVelocity", "PannerNode", "AudioListener", "speedOfSound" and "dopplerFactor"

Nit: LOCALIZATION NOTE
Attachment #8589603 - Flags: review?(ehsan) → review+
(Assignee)

Comment 5

3 years ago
I landed with with a revised error message (including a link to a MDN page).
Keywords: leave-open
Should we set a target for eventual removal?
Flags: needinfo?(padenot)
Priority: -- → P3
(Assignee)

Comment 9

3 years ago
I was going to ask the Chrome folks at TPAC (next week). I'll post again here with a tentative date.
Flags: needinfo?(padenot)
One thing to consider may be to first remove the implementation leaving the methods as no-ops.  It would make it hard to test for the feature, but that may be less problematic than generating exceptions in existing code.
(Assignee)

Comment 11

2 years ago
I was talking to rtoy from Google, and he looked up the method usage in their statistics, and it was actually called 0% of the time in the last month or so [0], it's probably safe to proceed.

[0]: https://www.chromestatus.com/metrics/feature/timeline/popularity/1251
(Assignee)

Comment 12

2 years ago
I've sent an email to the google folks, we're going to sync up on removing those.
(Assignee)

Updated

2 years ago
Depends on: 1269741
(Assignee)

Comment 13

2 years ago
Chromium people have removed the processing bits, but have left the API in place, being no-op. They see very very small but non-null calls on the API, and ultimately want to remove the methods altogether in M55.

I suggest we proceed along the same timeline, starting by removing the processing code first, and making the deprecation message stronger, indicating a release.
Mass change P3->P4 to align with new Mozilla triage process.
Priority: P3 → P4
You need to log in before you can comment on or make changes to this bug.