Audio processing crashes - bug in audio cache

[Gavino]

Doing some audio processing with Avisynth, I have been experiencing strange crashes. So strange that even changing a variable name affected the result - strongly suggesting memory corruption.

Greatly simplifying the script to home in on the source of the problem, I can reproduce the fault with just this:
# was originally NicMPG123Source(" ... .mp3")
BlankClip(600, fps=25, audio_rate=44100, channels=2, sample_type="float")
KillVideo()
c=MixAudio(last, last)
MixAudio(c, c) # also fails with MergeChannels
When played, this crashes both MPC and WMP.

After much head-scratching, I have tracked it down to a bug in the operation of the audio cache, which can lead to data being written beyond the end of the cache buffer in some circumstances.
The fix is to replace the following code in Cache::GetAudio
if (shiftsamples >= cache_count) {
shiftsamples = cache_count; // Preserve linear access
}
else {
memmove(cache, cache+shiftsamples*samplesize,(cache_count-shiftsamples)*samplesize);
}
cache_start = cache_start + shiftsamples;
cache_count = cache_count - shiftsamples;
by
if (shiftsamples >= cache_count) {
cache_start = start; // Preserve linear access
cache_count = 0;
}
else {
memmove(cache, cache+shiftsamples*samplesize,(cache_count-shiftsamples)*samplesize);
cache_start = cache_start + shiftsamples;
cache_count = cache_count - shiftsamples;
}
The bug affects both version 2.58 and 2.60 alpha1, but not 2.57 or earlier.
It can potentially afflict scripts that mix or merge two or more audio channels from the same source (eg 5.1 mixdown).

For users affected, a viable (but inefficient) workaround is to use a separate instance of the source filter for each channel (that way, the cache would not be used). Or revert to Avisynth 2.57.

[IanB]

Yes, well spotted.

But if (shiftsamples >= cache_count) { // Can we save any existing data
cache_start = start+count - maxsamplecount; // Preserve linear access
cache_count = 0;
}

[Gavino]

That will work too. It doesn't exactly 'save' existing data, though - it forces it to be re-fetched (indeed it may never have been there in the first place).

[IanB]

Okay the comment is barse ackwards, but you got the idea.

The audio cache sucks as a piece of code anyway.

[Gavino]

LOL - it certainly gave me a few headaches trying to figure out what was going wrong. :)

I guess there are no threading issues with the audio cache?
AIUI, each thread has its own cache instance and there is no data shared between instances?

EDIT: Just checked the MT docs, etc, and now see that it doesn't multithread audio at all. So in that sense there can be no issue with the audio cache. ;)

[tin3tin]

I wonder if this bug also might cause these problems?
http://forum.doom9.org/showthread.php?p=1023539#post1023539

[Gavino]

I wonder if this bug also might cause these problems?
I don't think so, I believe they are due to a different problem - see this post (http://forum.doom9.org/showthread.php?p=1205423#post1205423) and IanB's reply to it.

Also, the audio cache problem was introduced in Avisynth 2.58 and I believe the problems you mention date from before that.