summaryrefslogtreecommitdiffstats
path: root/services/audioflinger
Commit message (Collapse)AuthorAgeFilesLines
* threadLoop mergeGlenn Kasten2012-03-052-392/+329
| | | | Change-Id: Id8e6330ac6be76f9c2debba94f856de87e2d98f7
* renamed audio policy output flag.Eric Laurent2012-03-011-1/+1
| | | | | | | Renamed AUDIO_POLICY_OUTPUT_FLAG_INDIRECT to AUDIO_POLICY_OUTPUT_FLAG_NONE which is more appropriate. Change-Id: Ia14d60397df0f2dcd9bea0186400a09da35bc104
* Merge "Shorten thread names"Glenn Kasten2012-02-293-5/+5
|\
| * Shorten thread namesGlenn Kasten2012-02-283-5/+5
| | | | | | | | | | | | | | | | prctl(PR_SET_NAME) limits to 15 characters. Before we had names like "Binder Thread #" and the counter was cut off :-( Also remove redundant "thread" at end of name; it's always a thread. Change-Id: I1f99c2730ba0787ed9b59c15914356cddf698e2f
* | Prepare for threadLoop merge - active tracksGlenn Kasten2012-02-292-17/+19
| | | | | | | | | | | | | | | | | | | | Continued work on making the copies of threadLoop more similar: - Remove alias for mActiveTracks in MixerThread and DuplicatingThread. - Pull in declaration of activeTrack in DirectOutputThread. - Remove redundant parameter of prepareTracks_l(). - Comment prepareTracks_l(). Change-Id: If1087c1902b454acec01ddfdd9f055f0ca7abf04
* | Merge "Update AudioFlinger comments"Glenn Kasten2012-02-291-4/+7
|\ \
| * | Update AudioFlinger commentsGlenn Kasten2012-02-291-4/+7
| |/ | | | | | | | | | | | | | | Add comments to enum mixer_state Note side-effect of lockEffectChains_l Fix a typo Change-Id: Ibd51678bac2193201cbcbe081ff5664046fbc494
* | Merge "Pull in declaration of effectChains to inner block"Glenn Kasten2012-02-291-2/+13
|\ \
| * | Pull in declaration of effectChains to inner blockGlenn Kasten2012-02-291-2/+13
| | | | | | | | | | | | Change-Id: I09eacf72124942abd604132b9f4e774b1236fcf3
* | | mSuspend comments and usageGlenn Kasten2012-02-292-11/+11
|/ / | | | | | | | | | | Emphasize that playbackthread::mSuspend is a counter, not a bool Change-Id: I7188e56814e1c54dbc65e560f3627f138257d644
* | Mark similar and different sections in threadLoopGlenn Kasten2012-02-291-9/+78
|/ | | | | | | | | Most of these comments will be removed after the threadLoop merge. Note: the trivial change in assignments to mixBufferSize, and the comments about "tracks to remove" is to make them all identical. Change-Id: I3b1a33a7f2cd12ad557a1986bb71f6171161974a
* Merge "Unlock effect chains in the middle of two if's"Glenn Kasten2012-02-281-13/+25
|\
| * Unlock effect chains in the middle of two if'sGlenn Kasten2012-02-241-13/+25
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | As part of the upcoming threadLoop() merge, this CL makes it clearer what are the similar and different parts before and after unlocking effect chains. In each threadLoop(), the old code was: if (sleepTime == 0) { // A unlockEffectChains(effectChains); // B } else { unlockEffectChains(effectChains); // C } The new code is: if (sleepTime == 0) { // A } unlockEffectChains(effectChains); if (sleepTime == 0) { // B } else { // C } Also this is slightly slower by one "if", it has the advantage of making it much more obvious about what is done before and after the unlock, and also to see the similarities and differences among the various copies of threadLoop(). Change-Id: I7bf4369d2dcb072573ec43b7e52c637f0097dc00
* | Merge "Simplify removeNotificationClient"Glenn Kasten2012-02-281-6/+1
|\ \
| * | Simplify removeNotificationClientGlenn Kasten2012-02-241-6/+1
| | | | | | | | | | | | | | | | | | | | | No need to check for presence of item before removing (but we do lose the log of the previous value). Change-Id: I2838430824de5f257f2ee15db0c22b1920c67d08
* | | Merge "AudioFlinger const methods and parameters"Glenn Kasten2012-02-282-10/+11
|\ \ \
| * | | AudioFlinger const methods and parametersGlenn Kasten2012-02-242-10/+11
| |/ / | | | | | | | | | Change-Id: I93ec28024005ed23aa141518092a012a4a7c44c5
* | | Merge "Fix theoretical race condition in addOutputTrack"Glenn Kasten2012-02-281-0/+1
|\ \ \
| * | | Fix theoretical race condition in addOutputTrackGlenn Kasten2012-02-241-0/+1
| | |/ | |/| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This is not a real race, because addOutputTrack was only called in two places, and in both places there could be no other threads referencing the DuplicatingThread instance. Those two places are: - the DuplicatingThread constructor, which is of course safe - openDuplicateOutput - this is safe because it's called immediately after the new DuplicatingThread, and there are no sp<> either in the constructor or here which could cause onFirstRef() to do Thread::run(). But for safety in case addOutputTrack is ever called somewhere else, or there are sp<> created earlier, it is safer to take the thread lock. Change-Id: I1502d014fa37ec5dbf4bf40d3e2884af311cd5e9
* | | Merge "AudioBufferProvider comments and cleanup"Glenn Kasten2012-02-286-53/+31
|\ \ \
| * | | AudioBufferProvider comments and cleanupGlenn Kasten2012-02-246-53/+31
| |/ / | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Add comments about which methods implement the AudioBufferProvider interface. Simplified the definition of kInvalidPts. <stdint.h> is very hard to work with, there seems to be no way to use it reliably to get INT64_MAX without having a separate source file, which is ugly because it means kInvalidPts is not a compile-time constant. So I just deleted AudioBufferProvider.cpp and used a hard-coded constant instead. Added a default constructor for Buffer so that the fields aren't random (especially .raw which is used to determine if the buffer is valid). Make the pts for getNextBuffer default to kInvalidPTS so code that doesn't need a pts doesn't have to specify a value. Rename the parameter to AudioMixer::setBufferProvider to make it clearer. Change-Id: I87e7290884d4ed975b019f62d1ab6ae2bc5065a5
* | | Merge "Fix tracking of hardware state for dump"Glenn Kasten2012-02-272-29/+37
|\ \ \
| * | | Fix tracking of hardware state for dumpGlenn Kasten2012-02-242-29/+37
| |/ / | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | At end of AudioFlinger::onFirstRef(), the hardware status was being left in wrong state. It should be AUDIO_HW_IDLE but was AUDIO_HW_INIT. mHardwareStatus was being set to AUDIO_HW_OUTPUT_OPEN too early, and so a return would leave it in the wrong state until next hardware operation. Take the hardware lock for dev->get_parameters, and update mHardwareStatus before and after. Keep hardware lock only for the duration of the dev->set_parameters. Rename two constants in enum hardware_call_state to have the prefix AUDIO_HW so they follow the naming conventions. Add comments. Change-Id: I6c7450b11f9b13adaeef9cec874333e478a58fc0
* | | Merge "Make threadLoop() logs identical"Glenn Kasten2012-02-271-9/+11
|\ \ \
| * | | Make threadLoop() logs identicalGlenn Kasten2012-02-241-9/+11
| | |/ | |/| | | | | | | | | | | | | | | | Change the wording of the logs in the various copies of threadLoop() to be identical. This will make it easier to merge them soon. Change-Id: Idfa181e437738712c784dc7f746cac79f83d2931
* | | Merge "Move declaration of mixerStatus to inner block"Glenn Kasten2012-02-271-7/+3
|\ \ \ | |/ / |/| |
| * | Move declaration of mixerStatus to inner blockGlenn Kasten2012-02-241-7/+3
| |/ | | | | | | | | | | | | | | mixerStatus was being declared (and initialized) too early, which also resulted in a duplicate initialization. Moved the declaration into the block where it is actually used. Change-Id: Ifdcfefe362a5efe3493dd616cdb44645c6f9aed5
* | Pull out duplicated copies of silent mode checkGlenn Kasten2012-02-242-25/+23
| | | | | | | | | | | | | | | | Also fix the error handling for the property_get. This is part of preparation for the threadLoop() merge. Change-Id: I6405190ea18146d1271575e1dfe9f279e8f36b17
* | Merge "Pull CPU statistics code out of threadLoop()"Glenn Kasten2012-02-241-31/+42
|\ \
| * | Pull CPU statistics code out of threadLoop()Glenn Kasten2012-02-241-31/+42
| |/ | | | | | | | | | | This is to prepare for the threadLoop() merge Change-Id: I118c7d5c6b011b5d5b95ec7d63fb03feb166a9cf
* | Remove TrackBase::mFlagsGlenn Kasten2012-02-242-24/+13
|/ | | | | | | | | | | | | | | | | | | | The bit-field TrackBase::mFlags was supposed to have track-specific flags in the upper 16 bits, and system flags in the lower 16 bits. The upper 16 bits of mFlags were initialized in the TrackBase constructor from the flags parameter of IAudioFlinger::createTrack() and IAudioFlinger::openRecord(), and the lower 16 bits were cleared. However, the upper 16 bits of mFlags were never acccessed again. So really there are no track-specific flags. I left the flags in the parameter list of createTrack() and openRecord() but made a note that these should be removed eventually as they are dead. This leaves only the one system flag "step server failed". I replaced the bit-field mFlags by bool mStepServerFailed, which is simpler and slightly faster. Change-Id: I6650f5487be72791b4a67d73adcd10ffa04e2aa5
* Merge "Avoid wp<>::unsafe_get() with a few exceptions"Glenn Kasten2012-02-222-38/+25
|\
| * Avoid wp<>::unsafe_get() with a few exceptionsGlenn Kasten2012-02-222-38/+25
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Avoid using wp<>::unsafe_get() except in a log, and other specific cases when it's known to be safe. Use more specific subclass types for parameters to avoid down-casts. When a constructor or method parameter is "this" of an object that is currently being constructed, it's better to use a raw pointer rather than either sp<> or wp<>. Using the raw pointer is safe, provided either: - it is "this" of an object being constructed (which has sp<> refcount of 0), - or the caller already holds an sp<> The raw pointer is simpler and faster, and it avoids the problem of the sp<> reference count being incremented and then decremented to zero on scope exit, which would cause the object's destructor to run while the object is still being constructed. Also removed some dead code per a review comment. Change-Id: I7375f64da3aec11b928c33cb01faff186252ef5e
* | Fix build warningGlenn Kasten2012-02-221-1/+1
|/ | | | | | | warning: pointer of type 'void *' used in arithmetic warning: enumeral and non-enumeral type in conditional expression Change-Id: I7b8d626a636145ef648e3b5d0e77068216dd012e
* Remove bit fields to improve performanceGlenn Kasten2012-02-173-21/+32
| | | | | | uint16_t enabled is (mostly) changed to bool in a separate CL Change-Id: Ied9f8c034b2479cee9a8778cee7b8ff92ae75b7b
* Merge "Simplify code"Glenn Kasten2012-02-175-45/+20
|\
| * Simplify codeGlenn Kasten2012-02-175-45/+20
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Use DefaultKeyedVector::valueFor to avoid extra test Make local variables as local as possible No double parentheses No typedef for single use No parentheses around indirect function call No AudioFlinger:: prefix when not needed Remove unnecessary casts Remove block with only one line Saves 128 bytes Change-Id: I3a87430eeb01b81e7b81a1c38f6fdd3274ec48f3
* | Merge "Put a bandaid on a segfault in timed audio track handling."Mike Lockwood2012-02-171-4/+17
|\ \
| * | Put a bandaid on a segfault in timed audio track handling.John Grossman2012-02-161-4/+17
| |/ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Add a bandaid to prevent a segfault which can occur while handling timed audio buffers. There is a deeper problem which should eventually be addressed, but for now this fix should prevent any crashing. The deeper problem is as follows. When the AudioFlinger mixer gets data to mix from an AudioTrack, it ends up getting a structure filled out which points into an IMemory region owned by the AudioTrack. Unfortunately, this structure is not holding a refcount on the IMemory which it points into. If the IMemory refcount hits 0 and the chunk of RAM is retuned to the binder heap it came from, there can still be a Buffer object being held by the AudioFlinger mixer which points into the region of memory which was retuned to the binfer heap. If AF reads from this buffer, it could read corrupt data (if the region of memory gets handed back out to a writer), or it could segfault (if the heap has been freed and the pages unmapped). Similar problems could happen if AF attempts to write to the buffer, heap corruption in one case, segfaulting in the other. In the past, this has not been an issue for AF, because tracks allocate a single IMemory (which serves as a ring buffer) and the IMemory lives for as long as the track lives. As an artifact of the way the code came out, the mixer cannot be holding a Buffer structure pointing into the IMemory which used to be owned by a track if the track no longer exists. Tracks cannot come into or out of existence during a mix operation, which is the only thing which makes this safe. TimedTracks work differently, however. Timed tracks each allocate a small binder heap, and then hand out IMemory instances broken out of this heap. The heap lives as long as the track, so the worst which could happen here is that a TimedTrack's IMemory gets returned to the heap while there is still a buffer structure in flight pointing into the memory region, then the region gets handed out again and overwritten by new data causing the mixer to mix the wrong audio. The timing to cause this to happen is very difficult to encounter, and you to generate the timing conditions required, you need to be in a pretty bad failure state where audio is already breaking up and skipping, so its unlikely that anyone would notice (which is why I'm band-aiding the segfault and letting the deeper issue slide for now). In general, however, it might be a good idea to revisit this buffering design. On principal, if someone is going to hold pointers into a refcounted object, they should be holding a ref on the object at the same time. Failure to do this will usually lead to a situation where there are corruption or segfault issues, or to a system where the refcounted object's lifetime must be implicitly managed very carefully in ways which are usually non-obvious and are easy to break by new engineers on a project. Change-Id: Ib391075395ed0ef46a03c37aa38a82d09e88abeb
* | Fixed possible heap corruption in EffectDescGlenn Kasten2012-02-162-22/+35
|/ | | | | | | | | | | | | | | | | | | | | | | | | "EffectDesc *effect = new EffectDesc(*effects[i]);" was relying on the default copy constructor for EffectDesc, but the default copy constructor does a member-by-member copy. This works OK for mUuid, but a member copy of mName and mParams shares pointers. This could result in heap corruption later on due to a double free. Changed to add an explicit copy constructor that does a deep copy of both mName and mParams. A malloc() and strdup() were being freed by delete, but the correct matching API for these is free(). Fortunately our current memory runtime implementation ignores the difference. Changed to use free(). EffectDesc and InputSourceDesc member fields were being torn down by the code that does delete. Changed to do the tear-down in ~EffectDesc() and ~InputSourceDesc(). Added constructor EffectDesc() with name and UUID parameters, rather than having caller fill in the object after construction. Made ~EffectDesc() and ~InputSourceDesc() non-virtual to save memory, since they have no subclasses. Change-Id: Ibb5cc2e6760d72e0c4cf537068ac4432c717bafd
* Fix a segfault in AudioFlinger.John Grossman2012-02-161-1/+1
| | | | | | | | | Check the string returned by a HAL's implementation of get_parameters for NULL before attempting to make use of it. That way, we won't bring down the mediaserver because of a poorly written HAL. Change-Id: Ic99d7b004520d7d6347842a681c0595e889b68ea Signed-off-by: John Grossman <johngro@google.com>
* Upintegrate Audio Flinger changes from ICS_AAHJohn Grossman2012-02-1611-80/+874
| | | | | | | | Bring in changes to audio flinger made to support timed audio tracks and HW master volume control. Change-Id: Ide52d48809bdbed13acf35fd59b24637e35064ae Signed-off-by: John Grossman <johngro@google.com>
* Merge "Fix races related to volume and mute"Glenn Kasten2012-02-142-31/+31
|\
| * Fix races related to volume and muteGlenn Kasten2012-02-082-31/+31
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Fix race conditions when setting master volume, master mute, stream volume, stream mute for a playback thread, and when reading stream volume of a playback thread. Lock order is AudioFlinger, then thread. Rename streamVolumeInternal to streamVolume_l, comment, and use it to implement streamVolume(). Code size reduction: - Remove dead code: AudioFlinger::PlaybackThread::masterVolume, masterMute, streamMute. - Change return type of non-binder methods that always succeed from status_t to void. - Remove virtual from volume and mute methods that don't need it. This change saves 228 bytes but decreases performance of binder operations due to the added locks. Change-Id: Iac75abc1f54784873a667d1981b2e08f8f31e5c9
* | Merge "Update comments"Glenn Kasten2012-02-144-8/+27
|\ \
| * | Update commentsGlenn Kasten2012-02-144-8/+27
| | | | | | | | | | | | | | | | | | We no longer put the filename at start of file. Change-Id: Ic435b159a23105681e3d4a6cb1ac097bc853302e
* | | Use size_t and ssize_t with VectorGlenn Kasten2012-02-142-27/+27
|/ / | | | | | | | | | | | | | | Use size_t with size() and ssize_t with indexOfKey(). Exception: use ssize_t for backwards loops, and indices that are overloaded as a marker or error code. Change-Id: Ibf2a360af4539b72b09c818dda22ea2a0de92431
* | AudioRecord and AudioTrack client tidGlenn Kasten2012-02-142-20/+21
| | | | | | | | | | | | Inform AudioFlinger of the tid of the callback thread. Change-Id: I670df92dd06749b057238b48ed1094b13aab720b
* | Factor out and speed up permission-checking codeGlenn Kasten2012-02-135-33/+97
| | | | | | | | | | | | | | | | | | | | | | | | | | Use the caching permission check for dump to save IPC. Cache getpid() to save kernel call for other permission checks. The C runtime library getpid() can't cache due to a fork race condition, but we know that mediaserver doesn't fork. Don't construct String16 on the stack. Change-Id: I6be6161dae5155d39ba6ed6228e7683e67be34ed
* | mAudioHwDevs and related cleanupGlenn Kasten2012-02-102-23/+27
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Inline AudioFlinger::initCheck and remove unnecessary lock. Remove redundant check of mAudioHwDevs.size(). No need to lock mHardwareLock for each device separately during initialization. Use size_t not int to loop through Vector, since size() returns size_t. Add missing hardware lock for get_mic_mute() and get_input_buffer_size(). Add comments. Change-Id: Iafae78ef78bbf65f703d99fcc27c2f4ff221aedc