1228207 - Provide a mechanism to schedule smooth AudioParam changes immediately

Status: NEW

(Core :: Web Audio, defect, P3)

Description

[Karl Tomlinson (:karlt)]

Bug 1226481 comment 7 suggests using setValueAtTime(currentValue, currentTime)
and linearRampToValueAtTime(newValue, currentTime + fadeDuration) to fade
audio in a Web Audio graph, but this only works for very long fade durations,
due to the problems described in [1].

In bug 943138, the solution chosen was to use an AudioBufferSourceNode with the
desired envelope and connect to the gain.  This is a little complicated
however, and having to specify a buffer of sample values seems unfortunate
with longer transition durations.

[1] https://lists.w3.org/Archives/Public/public-audio/2013OctDec/0286.html

Comment 1

[Karl Tomlinson (:karlt)]

Attachment: linearRamp testcase

This demonstrates the problem with the linear ramp: the start of the ramp is
missed and so there is a jump in the envelope.

The ramp duration here is 100ms, which is similar to the intervals seem in
bug 1226481 comment 3.

Comment 2

[Karl Tomlinson (:karlt)]

Attachment: setTarget testcase

The spec says "Using the setTargetAtTime method with a low timeConstant allows
authors to perform a smooth transition", but this demonstrates the same
problem.

Sometimes, the setTargetAtTime seems to fail altogether and this testcase
continues to wait indefinitely for the decay.  That's probably a separate bug.

Also, beware of bug 1213313.

Comment 3

[Karl Tomlinson (:karlt)]

I think we may have some support [1][2] to modify the spec so that these methods
begin their transition curve when the methods are processed.  i.e. startTime is
clamped to be >= the time of processing the method. [1][2]
This would change the behavior in the test for bug 864171, but I think that is
OK.

The AudioBufferSourceNode envelope approach would still be required for short
transitions because otherwise the end of the transition (the event time) may
have passed when the method is processed).

Modifying the start of the transition curve as described above would mean that
AudioParam methods could be used for moderate transition durations of about
100 ms or more.  The approach of bug 956574 comment 23 could make this
effective for slightly shorter durations.

Still, having to use different approaches for different durations seems
messy, and so I wonder whether we should be looking for a different solution.
Perhaps that might be setValueCurveAtTime(), with a clamped start time.

There seems to be a complete difference in philosophy between AudioBufferSourceNode and AudioParam.  The former will delay the start time if necessary for the start to be rendered, while the latter waits for nothing.

https://github.com/WebAudio/web-audio-api/issues/652#issuecomment-158449946
https://github.com/WebAudio/web-audio-api/issues/573#issuecomment-149654221

Comment 4

[Paul Adenot (:padenot)]

Maybe it can be worth it to process the ControlMessages once per block, and not once per MSG iteration ? This is something we need to do regardless, but I think it can help here.

Comment 5

[Karl Tomlinson (:karlt)]

(In reply to Paul Adenot (:padenot) from comment #4)
> Maybe it can be worth it to process the ControlMessages once per block, and
> not once per MSG iteration ? This is something we need to do regardless, but
> I think it can help here.

That may be something to consider to improve latency.  To reduce the efficiency cost, I'd want to ensure that there are no unnecessary loops over streams to ExtractPendingInput(), set mStartBlocking, update current time, and send main thread updates, before doing that.

However, we cannot change the fact that currentTime is out of date when it is used.
The automation methods, as currently defined at least, do not support this use case.

Comment 7

[Karl Tomlinson (:karlt)]

I think the best way forward here is to fix up SetTargetAtTime() so that it
performs the smooth transition that the spec expects, even though this is not
exactly how SetTargetAtTime is currently spec'ed.  That merely involves
changing T_0 to be the greater of startTime and the time when the event is
applied.  There is some support for similar changes to the spec at
https://github.com/WebAudio/web-audio-api/pull/665#commitcomment-14720240

Because SetTargetAtTime has no end time, it does not suffer the problems of
linearRampToValueAtTime with short transitions.

We'd also need to fix bug 1213313.

We should also check that SetTargetAtTime() eventually reaches its target
without producing an infinite stream of subnormals.

Comment 9

[Paul Adenot (:padenot)]

This has been solved at the spec level [0]. I'm currently in the process of rewriting the automation code anyways, so that it's faster and implements the spec without bugs.

[0]: https://github.com/WebAudio/web-audio-api/commit/aa9af63e12fcb033b5721ab72d8b12df83941988

Comment 10

[Bulk Bug Changes for mreavy's org]

Mass change P1->P2 to align with new Mozilla triage process

Comment 11

[Sylvestre Ledru [:Sylvestre]]

Moving to p3 because no activity for at least 1 year(s).
See https://github.com/mozilla/bug-handling/blob/master/policy/triage-bugzilla.md#how-do-you-triage for more information

Comment 13

[Karl Tomlinson (:karlt)]

Some related spec discussion starting about https://github.com/WebAudio/web-audio-api/issues/1712#issuecomment-420455109

Comment 14

[BugBot [:suhaib / :marco/ :calixte]]

The severity field for this bug is relatively low, S3. However, the bug has 3 duplicates.
:kinetik, could you consider increasing the bug severity?

For more information, please visit auto_nag documentation.

Comment 15

[BugBot (nomail) [:suhaib / :marco/ :calixte]]

The last needinfo from me was triggered in error by recent activity on the bug. I'm clearing the needinfo since this is a very old bug and I don't know if it's still relevant.

Comment 16

[Karl Tomlinson (:karlt)]

Bug 1308433 isn't helpful here because the currentTime clamping is not on the rendering thread.
We could consider also doing the clamping on the rendering thread, even though that's not exactly what the spec says.