Yeah, I agree.  I'm going to hold off on this for a little bit until we have a clearer picture of how things are going to be moved around.

Do you need it done sometime soon?  We can do that too, if needed.

olka@

> we need AudioDestination to be able to render on a varying buffer size callbacks

I thought we were going to move the FIFO for varying buffer size out of WebAudio? I was hoping to use the FIFO in AudioDestination solely for the inter-thread communication. (e.g. AudioWorklet thread and the actual audio thread in the renderer)

Yes, let's clarify things first. Probably we will skip this change.

Now when it's decided to keep both FIFOs in AudioDestination, can we fix it? And are we sure it won't cause buffer underflow?

Okay, let me summarize where we are:

> we need AudioDestination to be able to render on a varying buffer size callbacks.

This is already happening. We are going to keep input and output FIFOs in AudioDestination, and we do not need to do anything further. The AudioWorklet change will require FIFO in the WebAudio side, so the output FIFO in AudioDestination can handle: 1) the inter-thread buffering and 2) the varying callback buffer size from the audio hardware.

> And are we sure it won't cause buffer underflow?

Not sure if I am following you. What causes the underflow and what prevents it from happening? WebAudio is based a dynamic audio graph (controlled with JS) so we cannot predict the latency or the required performance of the system. Also any special treatment might lead to the additional latency so we do want to avoid it.

The code you pointed out in the post - I am not sure if we should fix this. Is there any scenario these values can be different?

For the  issue 564472 , WebAudio has 3 latency category: interactive, balanced and playback. The first two types must provide the fixed callback size. 

We have not decided what to do with the `playback`, but the fixed callback size (something large enough) seems to be a reasonable design because the web audio graph can change anytime. (Measuring the latency of a dynamic graph is almost impossible due to cycles and filters.)

>Not sure if I am following you. What causes the underflow and what prevents it from happening? WebAudio is based a dynamic audio graph (controlled with JS) so we cannot predict the latency or the required performance of the system. Also any special treatment might lead to the additional latency so we do want to avoid it.

As I understood, WebAudio code does not work without those two FIFOs, and they are used for Input somehow as well. My impression was that input pushes data into them.

> The code you pointed out in the post - I am not sure if we should fix this. 
Let's switch to mail to cover the audience for the discussion :)

It turns out we do not handle the input in AudioDestination at all. The new FIFO will be landed shortly, and it is designed to support any hardware callback buffer size.

Now I am not sure which "check" prevented us from having a variable callback buffer size?

olka@

This CL might fix this bug as well. WDYT?

https://codereview.chromium.org/2549093009

If so, how do we verify it? Perhaps via the unit test included in that CL?

The check is still there, isn't it?
In any case it's not a priority, since the mixing experiment is closed for now.

olka@

Could you clarify which check are you referring to? The link above does not work anymore.

This one: https://cs.chromium.org/chromium/src/third_party/WebKit/Source/platform/audio/AudioDestination.cpp?cl=GROK&l=104

So |numberOfFrames| can be changed dynamically each callback? I know the experiment is closed, but what was the range of the flexible callback size?

Certainly the new WebAudio FIFO can handle the variable callback size, but the pre-defined range would be nice to know.

It cannot, otherwise DCHECK would be hit.

What I meant was; if this mixing experiment is deployed, can |numberOfFrames| be changed dynamically? I thought that was implication of this bug entry, but wanted to confirm.

It was deployed, it did not change the buffer size because of DCHECKs, there are no plans to re-deploy it for now. Basically, the bug is obsolete.

Got it. Then I am closing this.