Bug 222098 – Regression: Raw AudioBufferSourceNode playback causes repeated crackling sound

• Component: Web Audio
• Status: RESOLVED FIXED

STR: Visit and play https://smashkarts.io/webgl2/ for some minutes

Audio playback will begin to crackle.

Earlier Safari versions work ok.

More information can be found in this thread: https://forum.unity.com/threads/request-for-help-does-your-webgl-2-game-render-correctly-in-safari-tech-preview.1050890/#post-6839972 along with a video of the issue.

Unity uses Web Audio with uncompressed AudioBufferSourceNodes to play back audio, so scheduling audio samples is done inside the browser. The uncompressed audio nodes contain the "full" audio clips without JS/Wasm software mixing that would be applied.

[Radar WebKit Bug Importer]

<rdar://problem/74546471>

[Chris Dumez]

I can reproduce with the latest build on WebKit indeed.

[Chris Dumez]

It does not appear to be a super recent WebKit regression. I went as far back as r269041 (from Oct 2020) and could already reproduce the issue. Older WebKit builds do not run on my build of macOS.

This issue has been since reported a number of times, it looks like it affects a lot of Unity users on the web. Any chance any updates on this?

[Chris Dumez]

(In reply to jujjyl from comment #4)
> This issue has been since reported a number of times, it looks like it
> affects a lot of Unity users on the web. Any chance any updates on this?

A reduced test case would be very helpful.

The projects at https://github.com/Unity-Technologies/ProjectTinySamples/ have also been reported to exhibit the same issue. Those are somewhat smaller than the classic Unity builds.

[kris]

Our JS web game (not Unity-based) is also having this issues since Safari 14.1 on both Mac and IOS devices.

Here's a simple JSFiddle that shows the issue by playing a ton of sounds. Click on the 'Start Adding Sounds' button, and after roughly 10 seconds the crackling sounds begins. Note - this issue does not occur on MacOS Chrome browsers.

https://jsfiddle.net/KrisJohnson/s5vL24o1/123/

I've reduced the code as much as possible and it appears to happen when using panner objects. Let me know if I can be of help.

[Eric]

Same on latest iOS 14.5.1

[Chris Dumez]

(In reply to kris from comment #7)
> Our JS web game (not Unity-based) is also having this issues since Safari
> 14.1 on both Mac and IOS devices.
> 
> Here's a simple JSFiddle that shows the issue by playing a ton of sounds.
> Click on the 'Start Adding Sounds' button, and after roughly 10 seconds the
> crackling sounds begins. Note - this issue does not occur on MacOS Chrome
> browsers.
> 
> https://jsfiddle.net/KrisJohnson/s5vL24o1/123/
> 
> I've reduced the code as much as possible and it appears to happen when
> using panner objects. Let me know if I can be of help.

Thanks for the reduced test case, I'll look into it soon.

[Chris Dumez]

(In reply to Chris Dumez from comment #9)
> (In reply to kris from comment #7)
> > Our JS web game (not Unity-based) is also having this issues since Safari
> > 14.1 on both Mac and IOS devices.
> > 
> > Here's a simple JSFiddle that shows the issue by playing a ton of sounds.
> > Click on the 'Start Adding Sounds' button, and after roughly 10 seconds the
> > crackling sounds begins. Note - this issue does not occur on MacOS Chrome
> > browsers.
> > 
> > https://jsfiddle.net/KrisJohnson/s5vL24o1/123/
> > 
> > I've reduced the code as much as possible and it appears to happen when
> > using panner objects. Let me know if I can be of help.
> 
> Thanks for the reduced test case, I'll look into it soon.

I am totally able to reproduce with the provided test case on my system Safari. This should help me a lot.

[Chris Dumez]

(In reply to Chris Dumez from comment #10)
> (In reply to Chris Dumez from comment #9)
> > (In reply to kris from comment #7)
> > > Our JS web game (not Unity-based) is also having this issues since Safari
> > > 14.1 on both Mac and IOS devices.
> > > 
> > > Here's a simple JSFiddle that shows the issue by playing a ton of sounds.
> > > Click on the 'Start Adding Sounds' button, and after roughly 10 seconds the
> > > crackling sounds begins. Note - this issue does not occur on MacOS Chrome
> > > browsers.
> > > 
> > > https://jsfiddle.net/KrisJohnson/s5vL24o1/123/
> > > 
> > > I've reduced the code as much as possible and it appears to happen when
> > > using panner objects. Let me know if I can be of help.
> > 
> > Thanks for the reduced test case, I'll look into it soon.
> 
> I am totally able to reproduce with the provided test case on my system
> Safari. This should help me a lot.

Interestingly, the issue seems to stop reproducing when dropping the PannerNode from the graph so maybe the issue is not strictly related to AudioBufferSourceNode.

[Chris Dumez]

I initially see us destroying AudioBufferSourceNode / PannerNode objects as they finish playing. However, after a little while, we stop destroying them (or at least we can't keep up with the rate at which they are created). When we end up with ~2000 AudioBufferSourceNode, the audio gets bad.

I haven't figured out yet why the nodes aren't getting deallocated promptly but it is a start.

[kris]

It makes me wonder if my test case identifies a separate issue or if it's another way to expose the problem, as our application and I imagine others are using far fewer than 2000 simultaneous AudioBufferSourceNodes. In our case (and it appears similar to the OP), we play various audio samples of varying lengths, some of which are looping. Then after roughly ~10 seconds, the crackling starts.

[kris]

And to clarify, the crackling only happens when using PannerNodes.

[Chris Dumez]

The javascript wrappers are no longer getting garbage collected in a timely fashion after a while. As a result, our implementation nodes aren't either.

It could be a JSC garbage collection issue or an issue with AudioScheduledSourceNode::virtualHasPendingActivity() as the wrapper stays alive as long as this function returns true.

The implementation of AudioScheduledSourceNode::virtualHasPendingActivity() seems sensible though:
```
bool AudioScheduledSourceNode::virtualHasPendingActivity() const
{
    return m_hasEndedEventListener && m_playbackState != FINISHED_STATE && !isMarkedForDeletion() && !context().isClosed();
}
```

And from my logging, I do see m_playbackState becoming FINISHED_STATE in a timely fashion.

[Chris Dumez]

It definitely seems there is a garbage collection issue here. When I start playing, I see some calls to AudioScheduledSourceNode::virtualHasPendingActivity(), which return false and I see JS wrappers getting garbage collection. HOWEVER, after this initial period, we go for an extended period of time without AudioScheduledSourceNode::virtualHasPendingActivity() getting called (which seems to indicate the JS wrapper aren't getting visited). Since the JS is constructing JS wrappers very aggressively and since we are not collecting them, the number of JS wrappers grows pretty quickly, once we reach ~12k-15k JS wrappers, the audio quality starts to deteriorate and craters very quickly. 

I could use some help from our GC experts here to understand why our nodes aren't getting visited in a timely fashion, allowing the number of wrappers to grow so quickly.

[Chris Dumez]

(In reply to Chris Dumez from comment #16)
> It definitely seems there is a garbage collection issue here. When I start
> playing, I see some calls to
> AudioScheduledSourceNode::virtualHasPendingActivity(), which return false
> and I see JS wrappers getting garbage collection. HOWEVER, after this
> initial period, we go for an extended period of time without
> AudioScheduledSourceNode::virtualHasPendingActivity() getting called (which
> seems to indicate the JS wrapper aren't getting visited). Since the JS is
> constructing JS wrappers very aggressively and since we are not collecting
> them, the number of JS wrappers grows pretty quickly, once we reach ~12k-15k
> JS wrappers, the audio quality starts to deteriorate and craters very
> quickly. 
> 
> I could use some help from our GC experts here to understand why our nodes
> aren't getting visited in a timely fashion, allowing the number of wrappers
> to grow so quickly.

The visitor logic is in generated bindings code, here:
```
bool JSAudioScheduledSourceNodeOwner::isReachableFromOpaqueRoots(JSC::Handle<JSC::Unknown> handle, void*, AbstractSlotVisitor& visitor, const char** reason)
{
    auto* jsAudioScheduledSourceNode = jsCast<JSAudioScheduledSourceNode*>(handle.slot()->asCell());
    auto& wrapped = jsAudioScheduledSourceNode->wrapped();
    if (!wrapped.isContextStopped() && wrapped.hasPendingActivity()) {
        if (UNLIKELY(reason))
            *reason = "ActiveDOMObject with pending activity";
        return true;
     }
    if (jsAudioScheduledSourceNode->wrapped().isFiringEventListeners()) {
        if (UNLIKELY(reason))
            *reason = "EventTarget firing event listeners";
        return true;
    }
    UNUSED_PARAM(visitor);
    UNUSED_PARAM(reason);
    return false;
}
```

[Chris Dumez]

(In reply to Chris Dumez from comment #17)
> (In reply to Chris Dumez from comment #16)
> > It definitely seems there is a garbage collection issue here. When I start
> > playing, I see some calls to
> > AudioScheduledSourceNode::virtualHasPendingActivity(), which return false
> > and I see JS wrappers getting garbage collection. HOWEVER, after this
> > initial period, we go for an extended period of time without
> > AudioScheduledSourceNode::virtualHasPendingActivity() getting called (which
> > seems to indicate the JS wrapper aren't getting visited). Since the JS is
> > constructing JS wrappers very aggressively and since we are not collecting
> > them, the number of JS wrappers grows pretty quickly, once we reach ~12k-15k
> > JS wrappers, the audio quality starts to deteriorate and craters very
> > quickly. 
> > 
> > I could use some help from our GC experts here to understand why our nodes
> > aren't getting visited in a timely fashion, allowing the number of wrappers
> > to grow so quickly.
> 
> The visitor logic is in generated bindings code, here:
> ```
> bool
> JSAudioScheduledSourceNodeOwner::isReachableFromOpaqueRoots(JSC::Handle<JSC::
> Unknown> handle, void*, AbstractSlotVisitor& visitor, const char** reason)
> {
>     auto* jsAudioScheduledSourceNode =
> jsCast<JSAudioScheduledSourceNode*>(handle.slot()->asCell());
>     auto& wrapped = jsAudioScheduledSourceNode->wrapped();
>     if (!wrapped.isContextStopped() && wrapped.hasPendingActivity()) {
>         if (UNLIKELY(reason))
>             *reason = "ActiveDOMObject with pending activity";
>         return true;
>      }
>     if (jsAudioScheduledSourceNode->wrapped().isFiringEventListeners()) {
>         if (UNLIKELY(reason))
>             *reason = "EventTarget firing event listeners";
>         return true;
>     }
>     UNUSED_PARAM(visitor);
>     UNUSED_PARAM(reason);
>     return false;
> }
> ```

Thinking about this more, it may be acceptable from GC's point of view to not collect if the memory usage doesn't get too high. I think part of the issue may be that our WebAudio implementation relies on the AudioNodes's JS wrappers to get collected promptly to maintain performance. I suspect the issue may be that AudioScheduledSourceNodes don't break their connections to other nodes when finishing (until their ref count goes to 0, which cannot happen while the JS wrapper is alive). As a result, the audio graph becomes very large very quickly and we have to traverse this very large audio graph for every render quantum, thus the big perf hit.

I'll see if I can take the AudioScheduledSourceNode out of the graph as soon as it's finished playing.

[Chris Dumez]

(In reply to kris from comment #14)
> And to clarify, the crackling only happens when using PannerNodes.

Actually, you can actually replace the PannerNode in your demo with a DelayNode and the same issue will happen. I think it is about the AudioBufferSourceNode not being connected directly to the listenerGain somehow.

[Chris Dumez]

(In reply to Chris Dumez from comment #19)
> (In reply to kris from comment #14)
> > And to clarify, the crackling only happens when using PannerNodes.
> 
> Actually, you can actually replace the PannerNode in your demo with a
> DelayNode and the same issue will happen. I think it is about the
> AudioBufferSourceNode not being connected directly to the listenerGain
> somehow.

Ok, some progress, if I make PannerNode::requiresTailProcessing() return false then the issue no longer reproduces. So the issue likely only reproduces if the AudioBufferSource is connected to a node that requires tail processing (Both Delay and Panner nodes) require tail processing.

I think it is related to this code in AudioNode::disableOutputsIfNecessary():
```
    if (m_connectionRefCount <= 1 && !m_isDisabled) {
        if (!requiresTailProcessing())
            disableOutputs();
    }
```

disableOutputs() is not getting called because requiresTailProcessing() returns true.
disableOutputsIfNecessary() gets called when the AudioNode loses its last connection.

[Chris Dumez]

(In reply to Chris Dumez from comment #20)
> (In reply to Chris Dumez from comment #19)
> > (In reply to kris from comment #14)
> > > And to clarify, the crackling only happens when using PannerNodes.
> > 
> > Actually, you can actually replace the PannerNode in your demo with a
> > DelayNode and the same issue will happen. I think it is about the
> > AudioBufferSourceNode not being connected directly to the listenerGain
> > somehow.
> 
> Ok, some progress, if I make PannerNode::requiresTailProcessing() return
> false then the issue no longer reproduces. So the issue likely only
> reproduces if the AudioBufferSource is connected to a node that requires
> tail processing (Both Delay and Panner nodes) require tail processing.
> 
> I think it is related to this code in AudioNode::disableOutputsIfNecessary():
> ```
>     if (m_connectionRefCount <= 1 && !m_isDisabled) {
>         if (!requiresTailProcessing())
>             disableOutputs();
>     }
> ```
> 
> disableOutputs() is not getting called because requiresTailProcessing()
> returns true.
> disableOutputsIfNecessary() gets called when the AudioNode loses its last
> connection.

Seems to indicate we need to keep track of the nodes doing tail processing and disable their outputs once they're done processing their tail. I am working on a patch.

[Chris Dumez]

Created attachment 430049 [details]
Patch

[Chris Dumez]

A temporary workaround for people until this fix ships would likely be to manually remove nodes from the graph once they are no longer useful.

[Eric]

(In reply to Chris Dumez from comment #22)
> Created attachment 430049 [details]
> Patch

Great news, congrats for the patch Chris!

Looking forward to testing it on iOS too.

[Darin Adler]

Comment on attachment 430049 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=430049&action=review

The WinCairo build failure looks real.

> Source/WebCore/Modules/webaudio/BaseAudioContext.cpp:629
> +    // Heap allocations are forbidden on the audio thread for performance reasons so we need to
> +    // explicitly allow the following allocation(s).

What makes the following allocation OK? Just that it’s one of only a few? Comment should make that clear too. This restriction isn’t great if we are constantly disabling it. I’d like clearer explanation when we do of what’s special.

> Source/WebCore/Modules/webaudio/BaseAudioContext.cpp:661
> +        // Heap allocations are forbidden on the audio thread for performance reasons so we need to
> +        // explicitly allow the following allocation(s).
> +        DisableMallocRestrictionsForCurrentThreadScope disableMallocRestrictions;

Ditto.

> Source/WebCore/Modules/webaudio/BaseAudioContext.cpp:677
> +    // Heap allocations are forbidden on the audio thread for performance reasons so we need to
> +    // explicitly allow the following allocation(s).
> +    DisableMallocRestrictionsForCurrentThreadScope disableMallocRestrictions;

Ditto.

> Source/WebCore/Modules/webaudio/BaseAudioContext.h:325
> +        TailProcessingNode(TailProcessingNode&& other)
> +            : m_node(std::exchange(other.m_node, nullptr))
> +        { }

If we used Ref instead of RefPtr, I think this exact move constructor would be generated for us, automatically without us having to write this code.

We probably need to delete the copy constructor, assignment operator, and move-assignment operator. If any of them were used, setIsTailProcessing would get out of whack. Maybe the copy constructor is automatically deleted because we are explicitly providing a move constructor. The same is not true of the assignment operators, though.

> Source/WebCore/Modules/webaudio/BaseAudioContext.h:329
> +            if (m_node)
> +                m_node->setIsTailProcessing(false);

This would be a little harder to write if we did that, but we could just use m_node.ptr(). I think it’s good on balance to switch to Ref.

[Chris Dumez]

Comment on attachment 430049 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=430049&action=review

>> Source/WebCore/Modules/webaudio/BaseAudioContext.cpp:629
>> +    // explicitly allow the following allocation(s).
> 
> What makes the following allocation OK? Just that it’s one of only a few? Comment should make that clear too. This restriction isn’t great if we are constantly disabling it. I’d like clearer explanation when we do of what’s special.

Vector appends don't always allocate. The assertions help us make sure we keep allocations to a minimum.

>> Source/WebCore/Modules/webaudio/BaseAudioContext.h:325
>> +        { }
> 
> If we used Ref instead of RefPtr, I think this exact move constructor would be generated for us, automatically without us having to write this code.
> 
> We probably need to delete the copy constructor, assignment operator, and move-assignment operator. If any of them were used, setIsTailProcessing would get out of whack. Maybe the copy constructor is automatically deleted because we are explicitly providing a move constructor. The same is not true of the assignment operators, though.

Since we have a custom destructor so no move constructor would get generated by default unless I forced it. I guess `TailProcessingNode(TailProcessingNode&&) = default;` would do the trick.

>> Source/WebCore/Modules/webaudio/BaseAudioContext.h:329
>> +                m_node->setIsTailProcessing(false);
> 
> This would be a little harder to write if we did that, but we could just use m_node.ptr(). I think it’s good on balance to switch to Ref.

Are you suggested I use a Ref<> that's already been moved out here? If so, I don't like the idea and Ref::ptr() would hit an assertion.

[Peng Liu]

Comment on attachment 430049 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=430049&action=review

> Source/WebCore/ChangeLog:25
> +        The issue was that once those nodes had finished processing their tail, we wouldn't come

Can we let the node which just finished processing its tail to put itself into some list? I guess that will make the job of clean up the list a bit easier. Not sure it is better/feasible though.

[Darin Adler]

Comment on attachment 430049 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=430049&action=review

>>> Source/WebCore/Modules/webaudio/BaseAudioContext.h:325
>>> +        { }
>> 
>> If we used Ref instead of RefPtr, I think this exact move constructor would be generated for us, automatically without us having to write this code.
>> 
>> We probably need to delete the copy constructor, assignment operator, and move-assignment operator. If any of them were used, setIsTailProcessing would get out of whack. Maybe the copy constructor is automatically deleted because we are explicitly providing a move constructor. The same is not true of the assignment operators, though.
> 
> Since we have a custom destructor so no move constructor would get generated by default unless I forced it. I guess `TailProcessingNode(TailProcessingNode&&) = default;` would do the trick.

Makes sense. Maybe the copy constructor is not a problem, and the  assignment operator is the remaining problem. I think it should be deleted for completeness even though there’s little practical risk of it being used by accident. Not sure if you need to delete one or both of the assignment operators. Probably only one.

>>> Source/WebCore/Modules/webaudio/BaseAudioContext.h:329
>>> +                m_node->setIsTailProcessing(false);
>> 
>> This would be a little harder to write if we did that, but we could just use m_node.ptr(). I think it’s good on balance to switch to Ref.
> 
> Are you suggested I use a Ref<> that's already been moved out here? If so, I don't like the idea and Ref::ptr() would hit an assertion.

OK, then Ref<> makes the move constructor automatically correct, but makes the destructor impossible to write. Too bad!

[Chris Dumez]

Comment on attachment 430049 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=430049&action=review

>>>> Source/WebCore/Modules/webaudio/BaseAudioContext.h:325
>>>> +        { }
>>> 
>>> If we used Ref instead of RefPtr, I think this exact move constructor would be generated for us, automatically without us having to write this code.
>>> 
>>> We probably need to delete the copy constructor, assignment operator, and move-assignment operator. If any of them were used, setIsTailProcessing would get out of whack. Maybe the copy constructor is automatically deleted because we are explicitly providing a move constructor. The same is not true of the assignment operators, though.
>> 
>> Since we have a custom destructor so no move constructor would get generated by default unless I forced it. I guess `TailProcessingNode(TailProcessingNode&&) = default;` would do the trick.
> 
> Makes sense. Maybe the copy constructor is not a problem, and the  assignment operator is the remaining problem. I think it should be deleted for completeness even though there’s little practical risk of it being used by accident. Not sure if you need to delete one or both of the assignment operators. Probably only one.

You said switching to Ref<> would make the generated move constructor correct. However, I don't understand what would be the issue the the default move constructor generated if my data member is a RefPtr<>. My understanding is that it would be identical to what I wrote. I still decided to write it out because I thought it was clearer.

[Darin Adler]

Comment on attachment 430049 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=430049&action=review

>>>>> Source/WebCore/Modules/webaudio/BaseAudioContext.h:325
>>>>> +        { }
>>>> 
>>>> If we used Ref instead of RefPtr, I think this exact move constructor would be generated for us, automatically without us having to write this code.
>>>> 
>>>> We probably need to delete the copy constructor, assignment operator, and move-assignment operator. If any of them were used, setIsTailProcessing would get out of whack. Maybe the copy constructor is automatically deleted because we are explicitly providing a move constructor. The same is not true of the assignment operators, though.
>>> 
>>> Since we have a custom destructor so no move constructor would get generated by default unless I forced it. I guess `TailProcessingNode(TailProcessingNode&&) = default;` would do the trick.
>> 
>> Makes sense. Maybe the copy constructor is not a problem, and the  assignment operator is the remaining problem. I think it should be deleted for completeness even though there’s little practical risk of it being used by accident. Not sure if you need to delete one or both of the assignment operators. Probably only one.
> 
> You said switching to Ref<> would make the generated move constructor correct. However, I don't understand what would be the issue the the default move constructor generated if my data member is a RefPtr<>. My understanding is that it would be identical to what I wrote. I still decided to write it out because I thought it was clearer.

I agree that it’s clearer, because we require the "exchange" semantic here, where RefPtr is making a design choice that could change some day in theory (although in practice we won’t change it). I missed that angle before, but now I see it, thanks to your patient explanation.

[kris]

(In reply to Chris Dumez from comment #23)
> A temporary workaround for people until this fix ships would likely be to
> manually remove nodes from the graph once they are no longer useful.

Thanks Chris, your workaround fixes the crackle for us!

[Chris Dumez]

(In reply to kris from comment #31)
> (In reply to Chris Dumez from comment #23)
> > A temporary workaround for people until this fix ships would likely be to
> > manually remove nodes from the graph once they are no longer useful.
> 
> Thanks Chris, your workaround fixes the crackle for us!

Great news! I understand people want to get things working ASAP :)

[Chris Dumez]

Created attachment 430076 [details]
Patch

[EWS]

Committed r278233 (238270@main): <https://commits.webkit.org/238270@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 430076 [details].

[Chris Dumez]

Tiny follow-up assertion fix in <https://commits.webkit.org/238300@main>.

[Aakash Jain]

(In reply to EWS from comment #34)
> Committed r278233 (238270@main): <https://commits.webkit.org/238270@main>
This broke imported/w3c/web-platform-tests/webaudio/the-audio-api/the-convolvernode-interface/active-processing.https.html which still seems to be broken on iOS after the follow-up fix.

History: https://results.webkit.org/?suite=layout-tests&test=imported%2Fw3c%2Fweb-platform-tests%2Fwebaudio%2Fthe-audio-api%2Fthe-convolvernode-interface%2Factive-processing.https.html

[Chris Dumez]

(In reply to Aakash Jain from comment #36)
> (In reply to EWS from comment #34)
> > Committed r278233 (238270@main): <https://commits.webkit.org/238270@main>
> This broke
> imported/w3c/web-platform-tests/webaudio/the-audio-api/the-convolvernode-
> interface/active-processing.https.html which still seems to be broken on iOS
> after the follow-up fix.
> 
> History:
> https://results.webkit.org/?suite=layout-tests&test=imported%2Fw3c%2Fweb-
> platform-tests%2Fwebaudio%2Fthe-audio-api%2Fthe-convolvernode-
> interface%2Factive-processing.https.html

Ok, looking now

[Chris Dumez]

(In reply to Chris Dumez from comment #37)
> (In reply to Aakash Jain from comment #36)
> > (In reply to EWS from comment #34)
> > > Committed r278233 (238270@main): <https://commits.webkit.org/238270@main>
> > This broke
> > imported/w3c/web-platform-tests/webaudio/the-audio-api/the-convolvernode-
> > interface/active-processing.https.html which still seems to be broken on iOS
> > after the follow-up fix.
> > 
> > History:
> > https://results.webkit.org/?suite=layout-tests&test=imported%2Fw3c%2Fweb-
> > platform-tests%2Fwebaudio%2Fthe-audio-api%2Fthe-convolvernode-
> > interface%2Factive-processing.https.html
> 
> Ok, looking now

Looks like a potential progression:
--- /Volumes/Data/worker/ios-simulator-14-debug-tests-wk2/build/layout-test-results/imported/w3c/web-platform-tests/webaudio/the-audio-api/the-convolvernode-interface/active-processing.https-expected.txt
+++ /Volumes/Data/worker/ios-simulator-14-debug-tests-wk2/build/layout-test-results/imported/w3c/web-platform-tests/webaudio/the-audio-api/the-convolvernode-interface/active-processing.https-actual.txt
@@ -8,7 +8,8 @@
 PASS < [initialize] All assertions passed. (total 1 assertions)
 PASS > [test]
 PASS   Test 0: Number of convolver output channels is equal to 2.
-FAIL X Number of distinct values is not equal to 2. Got 1. assert_true: expected true got false
-FAIL < [test] 1 out of 2 assertions were failed. assert_true: expected true got false
+FAIL X Test 1: Number of convolver output channels is not equal to 1. Got 0. assert_true: expected true got false
+PASS   Number of distinct values is equal to 2.
+FAIL < [test] 1 out of 3 assertions were failed. assert_true: expected true got false
 FAIL # AUDIT TASK RUNNER FINISHED: 1 out of 2 tasks were failed. assert_true: expected true got false

[Chris Dumez]

(In reply to Chris Dumez from comment #38)
> (In reply to Chris Dumez from comment #37)
> > (In reply to Aakash Jain from comment #36)
> > > (In reply to EWS from comment #34)
> > > > Committed r278233 (238270@main): <https://commits.webkit.org/238270@main>
> > > This broke
> > > imported/w3c/web-platform-tests/webaudio/the-audio-api/the-convolvernode-
> > > interface/active-processing.https.html which still seems to be broken on iOS
> > > after the follow-up fix.
> > > 
> > > History:
> > > https://results.webkit.org/?suite=layout-tests&test=imported%2Fw3c%2Fweb-
> > > platform-tests%2Fwebaudio%2Fthe-audio-api%2Fthe-convolvernode-
> > > interface%2Factive-processing.https.html
> > 
> > Ok, looking now
> 
> Looks like a potential progression:
> ---
> /Volumes/Data/worker/ios-simulator-14-debug-tests-wk2/build/layout-test-
> results/imported/w3c/web-platform-tests/webaudio/the-audio-api/the-
> convolvernode-interface/active-processing.https-expected.txt
> +++
> /Volumes/Data/worker/ios-simulator-14-debug-tests-wk2/build/layout-test-
> results/imported/w3c/web-platform-tests/webaudio/the-audio-api/the-
> convolvernode-interface/active-processing.https-actual.txt
> @@ -8,7 +8,8 @@
>  PASS < [initialize] All assertions passed. (total 1 assertions)
>  PASS > [test]
>  PASS   Test 0: Number of convolver output channels is equal to 2.
> -FAIL X Number of distinct values is not equal to 2. Got 1. assert_true:
> expected true got false
> -FAIL < [test] 1 out of 2 assertions were failed. assert_true: expected true
> got false
> +FAIL X Test 1: Number of convolver output channels is not equal to 1. Got
> 0. assert_true: expected true got false
> +PASS   Number of distinct values is equal to 2.
> +FAIL < [test] 1 out of 3 assertions were failed. assert_true: expected true
> got false
>  FAIL # AUDIT TASK RUNNER FINISHED: 1 out of 2 tasks were failed.
> assert_true: expected true got false

We didn't see it on Mac because this test is already marked as flaky on that platform:
LayoutTests/platform/mac/TestExpectations:webkit.org/b/225421 imported/w3c/web-platform-tests/webaudio/the-audio-api/the-convolvernode-interface/active-processing.https.html [ Pass Failure ]

[Chris Dumez]

(In reply to Chris Dumez from comment #39)
> (In reply to Chris Dumez from comment #38)
> > (In reply to Chris Dumez from comment #37)
> > > (In reply to Aakash Jain from comment #36)
> > > > (In reply to EWS from comment #34)
> > > > > Committed r278233 (238270@main): <https://commits.webkit.org/238270@main>
> > > > This broke
> > > > imported/w3c/web-platform-tests/webaudio/the-audio-api/the-convolvernode-
> > > > interface/active-processing.https.html which still seems to be broken on iOS
> > > > after the follow-up fix.
> > > > 
> > > > History:
> > > > https://results.webkit.org/?suite=layout-tests&test=imported%2Fw3c%2Fweb-
> > > > platform-tests%2Fwebaudio%2Fthe-audio-api%2Fthe-convolvernode-
> > > > interface%2Factive-processing.https.html
> > > 
> > > Ok, looking now
> > 
> > Looks like a potential progression:
> > ---
> > /Volumes/Data/worker/ios-simulator-14-debug-tests-wk2/build/layout-test-
> > results/imported/w3c/web-platform-tests/webaudio/the-audio-api/the-
> > convolvernode-interface/active-processing.https-expected.txt
> > +++
> > /Volumes/Data/worker/ios-simulator-14-debug-tests-wk2/build/layout-test-
> > results/imported/w3c/web-platform-tests/webaudio/the-audio-api/the-
> > convolvernode-interface/active-processing.https-actual.txt
> > @@ -8,7 +8,8 @@
> >  PASS < [initialize] All assertions passed. (total 1 assertions)
> >  PASS > [test]
> >  PASS   Test 0: Number of convolver output channels is equal to 2.
> > -FAIL X Number of distinct values is not equal to 2. Got 1. assert_true:
> > expected true got false
> > -FAIL < [test] 1 out of 2 assertions were failed. assert_true: expected true
> > got false
> > +FAIL X Test 1: Number of convolver output channels is not equal to 1. Got
> > 0. assert_true: expected true got false
> > +PASS   Number of distinct values is equal to 2.
> > +FAIL < [test] 1 out of 3 assertions were failed. assert_true: expected true
> > got false
> >  FAIL # AUDIT TASK RUNNER FINISHED: 1 out of 2 tasks were failed.
> > assert_true: expected true got false
> 
> We didn't see it on Mac because this test is already marked as flaky on that
> platform:
> LayoutTests/platform/mac/TestExpectations:webkit.org/b/225421
> imported/w3c/web-platform-tests/webaudio/the-audio-api/the-convolvernode-
> interface/active-processing.https.html [ Pass Failure ]

Rebaselined test in <https://commits.webkit.org/r278293>.

[kris]

(In reply to Chris Dumez from comment #32)
> (In reply to kris from comment #31)
> > (In reply to Chris Dumez from comment #23)
> > > A temporary workaround for people until this fix ships would likely be to
> > > manually remove nodes from the graph once they are no longer useful.
> > 
> > Thanks Chris, your workaround fixes the crackle for us!
> 
> Great news! I understand people want to get things working ASAP :)

Hey Chris, we're hearing the crackling issue again using your workaround in our product. Was the originally reported link tested and working correctly with your patch? This problem is such a show stopper for us I just want to confirm it's not a separate issue. Thanks!

Original reported issue:

STR: Visit and play https://smashkarts.io/webgl2/ for some minutes