# HG changeset patch # User Eric Wing # Date 1326918054 28800 # Node ID 53ee4253c925d8c2cf9aaf2bd322af25b10d2d26 # Parent d7da7270368184c93dbead742fffe0c6e33af56e# Parent e2687188aea5c087af57d65f401eeec070426957 merged diff -r d7da72703681 -r 53ee4253c925 ALmixer.c --- a/ALmixer.c Tue Sep 13 18:11:34 2011 -0700 +++ b/ALmixer.c Wed Jan 18 12:20:54 2012 -0800 @@ -1692,6 +1692,171 @@ } +static ALboolean Internal_DetachBuffersFromSource(ALuint source_id, ALboolean is_predecoded) +{ + ALboolean retval = AL_TRUE; + ALenum error; + /* Here's the situation. My old method of using + * alSourceUnqueueBuffers() seemed to be invalid in light + * of all the problems I suffered through with getting + * the CoreData backend to work with this code. + * As such, I'm changing all the code to set the buffer to + * AL_NONE. Furthermore, the queued vs. non-queued issue + * doesn't need to apply here. For non-queued, Loki, + * Creative Windows, and CoreAudio seem to leave the + * buffer queued (Old Mac didn't.) For queued, we need to + * remove the processed buffers and force remove the + * still-queued buffers. + * Update: This code is was changed agian due to a serious regression bug in iOS 5.0 + * Since the 1.1 spec, I think I can make some simplifying assumptions sans the iOS 5.0 bug. + * Before I looked at buffers_still_queued and buffers_processed. + * According to the spec, all buffers get marked processed when alSourceStop is called. + * Also, in addition, alSourcei(source, AL_BUFFER, AL_NONE) is supposed + * to detach buffers for both streamed and non-streamed so all the code + * should just go through that. + * Unfortunately, the iOS 5.0 bug doesn't detach/clear buffers with AL_NONE. + * So I need a special handler for iOS 5.0 which manually unqueues the buffers. + * Ironically, I think the original Mac version did not work reliably + * with unqueuing buffers which is why I moved the code to the AL_NONE + * solution in the first place. This means for safety, I need to + * conditionalize the workaround as not to risk breaking Mac or other * platforms. + */ +#ifdef __APPLE__ /* iOS 5.0 workaround */ + #if (TARGET_OS_IPHONE == 1) || (TARGET_IPHONE_SIMULATOR == 1) + /* + fprintf(stderr, "kCFCoreFoundationVersionNumber: %lf\n", kCFCoreFoundationVersionNumber); + */ + /* I needed a C way to get the iOS version at runtime. This is returning 674.0 iOS 5.0 Beta 7. Apple hasn't updated the headers + * for these constants since iOS 4.2, so I don't know if the value is also catching 4.3. + * iOS 5.0.1 final is returning 675.0. + * TODO: Once I learn which version Apple fixes the bug in, I need to update the range so this check is not run on fixed versions. + * iOS 5.1 Beta 1 is returning 690.0. + */ + if(kCFCoreFoundationVersionNumber >= 674.0 && kCFCoreFoundationVersionNumber < 690.0) + { + /* For OpenAL experts, this is contrary to what you know, but must be done because the OpenAL implementation is broken. + Instead of unqueuing buffers on only streaming sources, it appears that alSourcei(source, AL_BUFFER, AL_NONE) is not reliable at all. + In cases where I switch between stream and non-stream on the same source and then stream again, the bug breaks playback on the third playback + and only one buffer plays. + The workaround seems to be to always unqueue buffers regardless of whether the source is streamed or not. + And then avoid calling (source, AL_BUFFER, AL_NONE) + From past experience, I know it is a bad idea to try to unqueue buffers from a non-streamed source (which is the contrary to OpenAL part), + but this seems to work for this bug. + */ + ALint buffers_processed; + /* Crap. Another iOS 5.0 problem. It seems our loop workaround below can get in cases where the buffer never clears. + * It appears that in some cases (probably predecoded, but not always which makes it hard), that the buffer never can be unqueued. + * I think it may possibly happen if you never use a source for streaming, and it is also possible it happens after loading or using a certain number of sources. + * So to workaround, we need to abort after a certain amount of time to prevent an infinite loop check. + * Some testing on iPad 2, 122ms is the highest number I've seen so far. So maybe 200ms is the cap? + */ + ALuint timeout_counter = ALmixer_GetTicks(); + const ALuint MAX_NUMBER_OF_TICKS_TO_WAIT_IN_WORKAROUND = 200; + + /* Wow this is the bug that just keeps on sucking. There is even a race condition bug where the unqueue may not actually work. + * So I have to keep doing it until it does. + */ + do + { + ALint temp_count; + + /* Wow this is the bug that just keeps on sucking. There is even a race condition bug where the unqueue may not actually work. + * So I have to keep doing it until it does. + * Sleeping for 20ms seems to help. 10ms was not long enough. (iPad 2). + * Update: A user is hitting this in a tight loop and calling dozens/hundreds of times. The sleep some times seems to result in a stutter. + * My theory is that there is thread contention and sleeps may be queuing up. The workaround seems to be reduce the sleep or eliminate it. + * I prefer to reduce it because fundamentally I am waiting for the Core Audio/OpenAL thread to finish processing so yielding seems like a good idea. + * The flipside is that I hit a lot of OpenAL errors because the commands keep failing and I'm burning CPU unnecessarily. + * So far, the OpenAL errors seem harmless and there are no serious cascading failures I've encountered. + */ + ALmixer_Delay(0); + + alGetSourcei( + source_id, + AL_BUFFERS_PROCESSED, &buffers_processed + ); + if((error = alGetError()) != AL_NO_ERROR) + { + fprintf(stderr, "17aTesting Error with buffers_processed on Halt. (You may be seeing this because of a bad Apple OpenAL iOS 5.0 regression bug): %s", + alGetString(error)); + ALmixer_SetError("Failed detecting still processed buffers: %s", + alGetString(error) ); + /* This whole iOS 5.0 bug is so messed up that returning an error value probably isn't helpful. + retval = AL_FALSE; + */ + } + /* + fprintf(stderr, "Going to unqueue %d buffers\n", buffers_processed); + */ + for(temp_count=0; temp_count= Number_of_Channels_global) { @@ -1721,59 +1885,13 @@ fprintf(stderr, "14Testing error: %s\n", alGetString(error)); } - /* Here's the situation. My old method of using - * alSourceUnqueueBuffers() seemed to be invalid in light - * of all the problems I suffered through with getting - * the CoreData backend to work with this code. - * As such, I'm changing all the code to set the buffer to - * AL_NONE. Furthermore, the queued vs. non-queued issue - * doesn't need to apply here. For non-queued, Loki, - * Creative Windows, and CoreAudio seem to leave the - * buffer queued (Old Mac didn't.) For queued, we need to - * remove the processed buffers and force remove the - * still-queued buffers. - */ - alGetSourcei( - ALmixer_Channel_List[channel].alsource, - AL_BUFFERS_QUEUED, &buffers_still_queued - ); - if((error = alGetError()) != AL_NO_ERROR) + + clear_succeeded = Internal_DetachBuffersFromSource(ALmixer_Channel_List[channel].alsource, ALmixer_Channel_List[channel].almixer_data->decoded_all); + if(AL_FALSE == clear_succeeded) { - fprintf(stderr, "17Testing Error with buffers_still_queued: %s", - alGetString(error)); - ALmixer_SetError("Failed detecting still queued buffers: %s", - alGetString(error) ); retval = -1; } - alGetSourcei( - ALmixer_Channel_List[channel].alsource, - AL_BUFFERS_PROCESSED, &buffers_processed - ); - if((error = alGetError()) != AL_NO_ERROR) - { - fprintf(stderr, "17Testing Error with buffers_processed: %s", - alGetString(error)); - ALmixer_SetError("Failed detecting still processed buffers: %s", - alGetString(error) ); - retval = -1; - } - /* If either of these is greater than 0, it means we need - * to clear the source - */ - if((buffers_still_queued > 0) || (buffers_processed > 0)) - { - alSourcei(ALmixer_Channel_List[channel].alsource, - AL_BUFFER, - AL_NONE); - if((error = alGetError()) != AL_NO_ERROR) - { - fprintf(stderr, "17Testing Error with clearing buffer from source: %s", - alGetString(error)); - ALmixer_SetError("Failed to clear buffer from source: %s", - alGetString(error) ); - retval = -1; - } - } + ALmixer_Channel_List[channel].almixer_data->num_buffers_in_use = 0; @@ -1801,60 +1919,12 @@ alGetString(error)); } - /* Here's the situation. My old method of using - * alSourceUnqueueBuffers() seemed to be invalid in light - * of all the problems I suffered through with getting - * the CoreData backend to work with this code. - * As such, I'm changing all the code to set the buffer to - * AL_NONE. Furthermore, the queued vs. non-queued issue - * doesn't need to apply here. For non-queued, Loki, - * Creative Windows, and CoreAudio seem to leave the - * buffer queued (Old Mac didn't.) For queued, we need to - * remove the processed buffers and force remove the - * still-queued buffers. - */ - alGetSourcei( - ALmixer_Channel_List[i].alsource, - AL_BUFFERS_QUEUED, &buffers_still_queued - ); - if((error = alGetError()) != AL_NO_ERROR) + clear_succeeded = Internal_DetachBuffersFromSource(ALmixer_Channel_List[i].alsource, ALmixer_Channel_List[i].almixer_data->decoded_all); + if(AL_FALSE == clear_succeeded) { - fprintf(stderr, "17Testing Error with buffers_still_queued: %s", - alGetString(error)); - ALmixer_SetError("Failed detecting still queued buffers: %s", - alGetString(error) ); retval = -1; } - alGetSourcei( - ALmixer_Channel_List[i].alsource, - AL_BUFFERS_PROCESSED, &buffers_processed - ); - if((error = alGetError()) != AL_NO_ERROR) - { - fprintf(stderr, "17Testing Error with buffers_processed: %s", - alGetString(error)); - ALmixer_SetError("Failed detecting still processed buffers: %s", - alGetString(error) ); - retval = -1; - } - /* If either of these is greater than 0, it means we need - * to clear the source - */ - if((buffers_still_queued > 0) || (buffers_processed > 0)) - { - alSourcei(ALmixer_Channel_List[i].alsource, - AL_BUFFER, - AL_NONE); - if((error = alGetError()) != AL_NO_ERROR) - { - fprintf(stderr, "17Testing Error with clearing buffer from source: %s", - alGetString(error)); - ALmixer_SetError("Failed to clear buffer from source: %s", - alGetString(error) ); - retval = -1; - } - } - + ALmixer_Channel_List[i].almixer_data->num_buffers_in_use = 0; /* Launch callback for consistency? */ @@ -2179,7 +2249,7 @@ /* only need to process channel if in use */ if(ALmixer_Channel_List[channel].channel_in_use) { - + running_count = 1; /* What should I do? Do I just rewind the channel * or also rewind the data? Since the data is * shared, let's make it the user's responsibility @@ -2243,7 +2313,10 @@ * much data is queued. Recommend users call Halt * before rewind if they want immediate results. */ - retval = Internal_RewindData(ALmixer_Channel_List[channel].almixer_data); + if(AL_FALSE == Internal_RewindData(ALmixer_Channel_List[channel].almixer_data)) + { + retval = -1; + } } } } @@ -2256,6 +2329,7 @@ /* only need to process channel if in use */ if(ALmixer_Channel_List[i].channel_in_use) { + running_count++; /* What should I do? Do I just rewind the channel * or also rewind the data? Since the data is * shared, let's make it the user's responsibility @@ -2319,7 +2393,10 @@ * much data is queued. Recommend users call Halt * before rewind if they want immediate results. */ - running_count += Internal_RewindData(ALmixer_Channel_List[i].almixer_data); + if(AL_FALSE == Internal_RewindData(ALmixer_Channel_List[i].almixer_data)) + { + retval = -1; + } } } } @@ -2689,7 +2766,7 @@ ALuint queue_ret_flag; for(k=0; knum_buffers_in_use; k++) { -// fprintf(stderr, "56c: CircularQueue_PushBack.\n"); +/* fprintf(stderr, "56c: CircularQueue_PushBack.\n"); */ queue_ret_flag = CircularQueueUnsignedInt_PushBack(data->circular_buffer_queue, data->buffer[k]); if(0 == queue_ret_flag) { @@ -3291,18 +3368,8 @@ alGetString(error) ); retval = -1; } - /* Need to resume playback if it was originally playing */ - if(AL_PLAYING == state) - { - alSourcePlay(ALmixer_Channel_List[channel].alsource); - if((error = alGetError()) != AL_NO_ERROR) - { - ALmixer_SetError("%s", - alGetString(error) ); - retval = -1; - } - } - else if(AL_PAUSED == state) + /* OpenAL 1.1 spec says if this succeeds on a playing source, it will automatically jump */ + if(AL_PAUSED == state) { /* HACK: The problem is that when paused, after * the Rewind, I can't get it off the INITIAL @@ -3331,8 +3398,12 @@ * much data is queued. Recommend users call Halt * before rewind if they want immediate results. */ - retval = Internal_SeekData(ALmixer_Channel_List[channel].almixer_data, msec); + if(AL_FALSE == Internal_SeekData(ALmixer_Channel_List[channel].almixer_data, msec)) + { + retval = -1; + } } + running_count = 1; } } /* The user wants to rewind all channels */ @@ -3363,6 +3434,7 @@ alGetString(error)); } + /* OpenAL 1.1 spec says if this succeeds on a playing source, it will automatically jump */ alSourcef(ALmixer_Channel_List[channel].alsource, AL_SEC_OFFSET, sec_offset); if((error = alGetError()) != AL_NO_ERROR) { @@ -3370,18 +3442,7 @@ alGetString(error) ); retval = -1; } - /* Need to resume playback if it was originally playing */ - if(AL_PLAYING == state) - { - alSourcePlay(ALmixer_Channel_List[i].alsource); - if((error = alGetError()) != AL_NO_ERROR) - { - ALmixer_SetError("%s", - alGetString(error) ); - retval = -1; - } - } - else if(AL_PAUSED == state) + if(AL_PAUSED == state) { /* HACK: The problem is that when paused, after * the Rewind, I can't get it off the INITIAL @@ -3410,8 +3471,12 @@ * much data is queued. Recommend users call Halt * before rewind if they want immediate results. */ - running_count += Internal_SeekData(ALmixer_Channel_List[i].almixer_data, msec); + if(AL_FALSE == Internal_SeekData(ALmixer_Channel_List[i].almixer_data, msec)) + { + retval = -1; + } } + running_count++; } } } @@ -5173,36 +5238,11 @@ } if(buffers_still_queued > 0) { - -#if 0 /* This triggers an error in OS X Core Audio. */ - alSourceUnqueueBuffers( - ALmixer_Channel_List[i].alsource, - 1, - ALmixer_Channel_List[i].almixer_data->buffer - ); -#else -/* fprintf(stderr, "In the Bob Aron section...about to clear source\n"); - PrintQueueStatus(ALmixer_Channel_List[i].alsource); -*/ - /* Rather than force unqueuing the buffer, let's see if - * setting the buffer to none works (the OpenAL 1.0 - * Reference Annotation suggests this should work). - */ - alSourcei(ALmixer_Channel_List[i].alsource, - AL_BUFFER, AL_NONE); - /* - PrintQueueStatus(ALmixer_Channel_List[i].alsource); - */ -#endif - if((error = alGetError()) != AL_NO_ERROR) + ALboolean clear_succeeded = Internal_DetachBuffersFromSource(ALmixer_Channel_List[i].alsource, ALmixer_Channel_List[i].almixer_data->decoded_all); + if(AL_FALSE == clear_succeeded) { - fprintf(stderr, "Error with unqueue, after alSourceUnqueueBuffers, buffers_still_queued=%d, error is: %s", buffers_still_queued, - alGetString(error)); - ALmixer_SetError("Predecoded Unqueue buffer failed: %s", - alGetString(error) ); error_flag--; } - } /* Launch callback */ @@ -5276,7 +5316,7 @@ /* WARNING: It looks like Snow Leopard some times crashes on this call under x86_64 * typically when I suffer a lot of buffer underruns. */ -// fprintf(stderr, "calling AL_BUFFERS_PROCESSED on source:%d", ALmixer_Channel_List[i].alsource); +/* fprintf(stderr, "calling AL_BUFFERS_PROCESSED on source:%d", ALmixer_Channel_List[i].alsource); */ alGetSourcei( ALmixer_Channel_List[i].alsource, AL_BUFFERS_PROCESSED, &buffers_processed @@ -5286,7 +5326,7 @@ fprintf(stderr, "52Testing error: %s\n", alGetString(error)); } -// fprintf(stderr, "finished AL_BUFFERS_PROCESSED, buffers_processed=%d", buffers_processed); +/* fprintf(stderr, "finished AL_BUFFERS_PROCESSED, buffers_processed=%d", buffers_processed); */ /* WTF!!! The Nvidia distribution is failing on the alGetSourcei(source, AL_BUFFER, buf_id) call. * I need this call to figure out which buffer OpenAL is currently playing. @@ -5483,7 +5523,7 @@ if(AL_STOPPED == state) { number_of_buffers_to_queue_this_pass = ALmixer_Channel_List[i].almixer_data->num_startup_buffers; -// fprintf(stderr, "assuming underrun condition, using num_startup_buffers=%d\n", number_of_buffers_to_queue_this_pass); +/* fprintf(stderr, "assuming underrun condition, using num_startup_buffers=%d\n", number_of_buffers_to_queue_this_pass); */ } /* Don't bother to check to make sure the number_of_buffers_to_queue_this_pass does not exceed the maximum number of buffers because of the logic hack bug. */ @@ -5497,7 +5537,25 @@ } for(current_count_of_buffer_queue_passes=0; current_count_of_buffer_queue_passescircular_buffer_queue != NULL) { ALuint queue_ret_flag; - // fprintf(stderr, "56d: CircularQueue_PushBack.\n"); + /* fprintf(stderr, "56d: CircularQueue_PushBack.\n"); */ queue_ret_flag = CircularQueueUnsignedInt_PushBack( ALmixer_Channel_List[i].almixer_data->circular_buffer_queue, ALmixer_Channel_List[i].almixer_data->buffer[ALmixer_Channel_List[i].almixer_data->num_buffers_in_use] @@ -5966,7 +6024,7 @@ if(ALmixer_Channel_List[i].almixer_data->circular_buffer_queue != NULL) { ALuint queue_ret_flag; - // fprintf(stderr, "56e: CircularQueue_PushBack.\n"); + /* fprintf(stderr, "56e: CircularQueue_PushBack.\n"); */ queue_ret_flag = CircularQueueUnsignedInt_PushBack( ALmixer_Channel_List[i].almixer_data->circular_buffer_queue, unqueued_buffer_id @@ -6220,8 +6278,11 @@ * setting the buffer to none works (the OpenAL 1.0 * Reference Annotation suggests this should work). */ - alSourcei(ALmixer_Channel_List[i].alsource, - AL_BUFFER, AL_NONE); + ALboolean clear_succeeded = Internal_DetachBuffersFromSource(ALmixer_Channel_List[i].alsource, ALmixer_Channel_List[i].almixer_data->decoded_all); + if(AL_FALSE == clear_succeeded) + { + error_flag--; + } /* PrintQueueStatus(ALmixer_Channel_List[i].alsource); */ @@ -7737,7 +7798,7 @@ } va_list argp; va_start(argp, err_str); - // SDL_SetError which I'm emulating has no number parameter. + /* SDL_SetError which I'm emulating has no number parameter. */ TError_SetErrorv(s_ALmixerErrorPool, 1, err_str, argp); va_end(argp); } @@ -8268,7 +8329,7 @@ free(ret_data->buffer_map_list[j].data); j--; } - // Delete for j=0 because the while loop misses the last one + /* Delete for j=0 because the while loop misses the last one */ free(ret_data->buffer_map_list[j].data); free(ret_data->buffer_map_list); diff -r d7da72703681 -r 53ee4253c925 ALmixer.h --- a/ALmixer.h Tue Sep 13 18:11:34 2011 -0700 +++ b/ALmixer.h Wed Jan 18 12:20:54 2012 -0800 @@ -499,7 +499,7 @@ #define ALMIXER_DEFAULT_BUFFERSIZE 32768 #define ALMIXER_DEFAULT_BUFFERSIZE 16384 */ -#define ALMIXER_DEFAULT_BUFFERSIZE 4096 +#define ALMIXER_DEFAULT_BUFFERSIZE 8192 /* You probably never need to use these macros directly. */ #ifndef ALMIXER_DISABLE_PREDECODED_PRECOMPUTE_BUFFER_SIZE_OPTIMIZATION @@ -520,15 +520,15 @@ /* #define ALMIXER_DEFAULT_QUEUE_BUFFERS 5 */ -#define ALMIXER_DEFAULT_QUEUE_BUFFERS 25 +#define ALMIXER_DEFAULT_QUEUE_BUFFERS 12 /** * Specifies the number of queue buffers initially filled when first loading a stream. * Default startup buffers should be at least 1. */ -#define ALMIXER_DEFAULT_STARTUP_BUFFERS 8 +#define ALMIXER_DEFAULT_STARTUP_BUFFERS 4 /* #define ALMIXER_DEFAULT_STARTUP_BUFFERS 2 */ -#define ALMIXER_DEFAULT_BUFFERS_TO_QUEUE_PER_UPDATE_PASS 4 +#define ALMIXER_DEFAULT_BUFFERS_TO_QUEUE_PER_UPDATE_PASS 2 /* #define ALMIXER_DECODE_STREAM 0