Decoder/Resampler clips ends of sounds about faudio HOT 5 CLOSED

Stared at dots in Audacity for a couple hours and this seems to be the only way to fix it:

diff --git a/src/FAudio_internal.c b/src/FAudio_internal.c
index 22d4ec7..98fdf86 100644
--- a/src/FAudio_internal.c
+++ b/src/FAudio_internal.c
@@ -815,6 +815,12 @@ static void FAudio_INTERNAL_MixSource(FAudioSourceVoice *voice)
        toResample -= voice->src.curBufferOffsetDec;
        /* ... undo step size, fixed to int. */
        toResample /= voice->src.resampleStep;
+
+       /* ... also round up. Because, uh, something
+        * Also do it in multiples of 4 because who the hell knows
+        */
+       toResample += ((toResample % voice->src.resampleStep) > 0) * 4;
+
        /* FIXME: I feel like this should be an assert but I suck */
        toResample = FAudio_min(toResample, voice->src.resampleSamples);

There's no way this is the right answer, but it does highlight the real problem. In short, we're not writing enough samples when resampling! I don't see how this makes sense though, because in our example we're well under the quantum size, and so we get this series of events:

• Source resample step is ~1.071773463
• Mixer quantum is 1024, asks for ~955 samples
• Buffer only has 359 samples, read all of that in
• Recalculate resample size, 359 / 1.071773463
• Resulting size is between 334/335 samples

Even when rounding up, we somehow come short by roughly 2-3 samples when comparing FAudio to XAudio output. Does this make sense to anybody, because it doesn't to me... should we be adding padding to the resample size or something? Maybe 2 samples, 1 for the beginning and end?

EDIT: Here's a patch version of that idea:

diff --git a/src/FAudio_internal.c b/src/FAudio_internal.c
index 22d4ec7..cccc15c 100644
--- a/src/FAudio_internal.c
+++ b/src/FAudio_internal.c
@@ -813,8 +813,12 @@ static void FAudio_INTERNAL_MixSource(FAudioSourceVoice *voice)
        toResample = toDecode << FIXED_PRECISION;
        /* ... round back down based on current offset... */
        toResample -= voice->src.curBufferOffsetDec;
+       /* ... but also ceil for any fraction value... */
+       toResample += FIXED_FRACTION_MASK;
        /* ... undo step size, fixed to int. */
        toResample /= voice->src.resampleStep;
+       /* Add the padding, for some reason this helps? */
+       toResample += EXTRA_DECODE_PADDING;
        /* FIXME: I feel like this should be an assert but I suck */
        toResample = FAudio_min(toResample, voice->src.resampleSamples);

As a reminder, here's where we calculate decode/resample sizes:

https://github.com/FNA-XNA/FAudio/blob/master/src/FAudio_internal.c#L755
https://github.com/FNA-XNA/FAudio/blob/master/src/FAudio_internal.c#L812

from faudio.

Messed with this some more, the EDIT patch above seems to work well too... but it really seems odd that we're adding resample samples when our dumbest hacks currently involve adding safety padding at the decode stage. Just seems really odd.

from faudio.

Just committed that patch: a770349

I still don't entirely know why this works, but in the end there were three numbers of interest:

1. The number of frames written by INTERNAL_DecodeBuffers (including padding)
2. The numbers for toDecode/toResample
3. The furthest float value read by the resamplers

I was focusing a lot on 2, but it turns out the numbers that matter a lot more are 1 and 3. In our example I was getting...

1. 361
2. 359/334
3. 357

But now, after the latest adjustments, I get...

1. 361
2. 359/334
3. 360

I think what's happening is that each end of the process needs to be padded, so even though toDecode/toResample are the theoretical sizes, toDecode needs padding so we have enough for the whole quantum, and toResample needs padding to read those extra samples to fill the quantum.

I tried this with a whole bunch of my usual ugly XACT samples and it seems to be okay, but I'm leaving this open for anyone to test their weirdest possible cases. @aeikum, @GloriousEggroll, can you check this out with your usual tests when you get a chance?

from faudio.

a756af4

FAudio's slowly becoming a drinking game where you drink on every off-by-one needed for padding. I feel like this particular commit could be sidestepped by just holding onto the last sample from the decoder and shoving it in at the start of DecodeBuffers, which would allow our in-between-buffers scenario to work, but I dunno when/where we'd grab that, so for now~

from faudio.

After playing with this a lot I think the patch covered the worst of it - that curBufferOffset == 0 case is still a thing, but I couldn't come up with a realistic example of it happening, as the sequence of events necessary to make it happen is extremely precise. We'll probably run into it some day, just not right now.

from faudio.