almixer_isolated: ALmixer.c comparison

comparison ALmixer.c @ 51:e2687188aea5

The evil Apple iOS 5.0 OpenAL regression bug strikes again. A user managed to encounter a case where the OpenAL buffer never unqueues and the workaround gets stuck in an infinite loop which results in the app getting killed by watch dog (crashing). I've added a timeout that will abort the unqueue attempt after 200 milliseconds. But if the buffers could be unqueued but the timeout we picked is too short, then we will regress back to the original problem. All iOS 5 audio users need to test this to make sure their apps don't break again. Please remember to file your Apple bug report if you haven't (bug:10145018), test with iOS 5.1 beta to check for new problems, and report results as necessary. I've also constrained the iOS 5.0 workaround to not run on 5.1 so users can test to see if Apple has fixed the problem. I've also changed the default buffer size to 8k and all the other values accordingly (still targeting about 100k per stream). I encountered some bugs on Mac with 4k buffers. Also running under the memory profile, it appeared that the internal Apple overhead starts to flatten out around 8k which suggests 8k is a better value.

author	Eric Wing <ewing@anscamobile.com>
date	Wed, 18 Jan 2012 12:17:42 -0800
parents	db5bc1c80057
children	8063b19bd40e

comparison

equal deleted inserted replaced

-:db5bc1c80057
+:e2687188aea5
 	/*
 		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)
+		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.
 			   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)
+				 * 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(20);
+				ALmixer_Delay(0);
 				alGetSourcei(
 					source_id,
 					AL_BUFFERS_PROCESSED, &buffers_processed
 				);
 						source_id,
 						1, &unqueued_buffer_id
 					);
 					if((error = alGetError()) != AL_NO_ERROR)
 					{
-						fprintf(stderr, "17bTesting error with unqueuing buffers on Halt (You may be seeing this because of a bad Apple OpenAL iOS 5.0 regression bug): %s\n",
+						/* Disabling this print because we hit it way too much with the usleep(0) */
-							alGetString(error));
+/*						fprintf(stderr, "17bTesting error with unqueuing buffers on Halt (You may be seeing this because of a bad Apple OpenAL iOS 5.0 regression bug): %s\n",	alGetString(error));
+*/
 						/* This whole iOS 5.0 bug is so messed up that returning an error value probably isn't helpful.
 						 retval = AL_FALSE;
 						 */
 					}
 				}
 							 source_id,
 							 AL_BUFFERS_PROCESSED, &buffers_processed
 							 );
 				if((error = alGetError()) != AL_NO_ERROR)
 				{
-					fprintf(stderr, "17cTesting Error with buffers_processed on Halt. (You may be seeing this because of a bad Apple OpenAL iOS 5.0 regression bug): %s",
+					fprintf(stderr, "17cTesting 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));
-							alGetString(error));
+					ALmixer_SetError("Failed detecting still processed buffers: %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;
 					 */
 				}
 				/*
 				*/
 				/* Wow this is the bug that just keeps on sucking. There is an additional race condition bug where the unqueue may not actually work.
 				 * So I have to keep doing it until it does.
 				 * I hope this doesn't infinite loop.
 				 */
+				/* Disabling this print because we hit it way too much with the usleep(0) */
+				/*
 				if(0 != buffers_processed)
 				{
-					fprintf(stderr, "Evil Apple OpenAL iOS 5.0 race condition. Buffers didn't actually unqueue. Repeating unqueue loop.\n");
+					fprintf(stderr, "Evil Apple OpenAL iOS 5.0 race condition. Buffers didn't actually unqueue. Repeating unqueue loop. %d, %d %d\n", ALmixer_GetTicks(), timeout_counter, ALmixer_GetTicks()-timeout_counter);
 				}
-			} while(0 != buffers_processed);
+				*/
+			} while(0 != buffers_processed && ( (ALmixer_GetTicks()-timeout_counter) < MAX_NUMBER_OF_TICKS_TO_WAIT_IN_WORKAROUND) );
 			/* Avoid calling the normal cleanup because part of this bug seems to be triggered by alSourcei(source_id, AL_BUFFER, AL_NONE); */
 			return retval;
 		}
 	#endif
 #endif /* iOS 5.0 workaround */
 		/* According to the spec, this is the best way to clear a source.

Mercurial > almixer_isolated

comparison ALmixer.c @ 51:e2687188aea5