warning: pointer of type 'void *' used in arithmetic
warning: enumeral and non-enumeral type in conditional expression
Change-Id: I7b8d626a636145ef648e3b5d0e77068216dd012e
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
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
"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
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>
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>
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
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
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
We can remove mExiting and use Thread::exitPending() instead.
The local sp<> on "this" in exit() is not needed, since the caller must
also hold an sp<> in order to be calling us. (Unless it was using a raw
pointer, but that would be dangerous for other reasons.)
Add comment explaining the mLock in exit().
Change-Id: I319e5107533a1a7cdbd13c292685f3e2be60f6c4
Unconditional delete for raw pointers.
Use "if (sp != 0)" not "if (sp.get() != 0)" or "if (sp != NULL)".
Use "if (raw != NULL)" not "if (raw)".
Change-Id: I531a8da7c37149261ed2f34b862ec4896a4b785b
Put IAudioFlinger methods in binder opcode order.
Move hardware call state closer to where it is used.
getMode() and btNrecIsOff() are private.
Change-Id: Ie50340b396c39c763f2b155cbc08da8a0d0f2424
Code was aliasing mBuffer as buffer, but continuing to use both buffer
and mBuffer after that point. This was at best misleading, and at worst
could confuse the compiler into generating bad code. There was no
performance advantage to the alias, in fact removing it saves 16 bytes.
Change-Id: I55023ddba465d9be82f66745b088d18af658ac60
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
stream_type_t is used by AudioFlinger class, so it should be declared there.
This way we don't have to peek into PlaybackThread to get the declaration.
Change-Id: Ie08bab1604699214d1e8df2d48d3fbfbbc436e96
This avoids possible confusion with thread's type().
Also remove redundant cast "(audio_stream_type_t)".
Change-Id: I320b9177b6c267a102d215f002228bcf988c437a
Other:
- add a comment to nextUniqueId
- made ThreadBase::mId const, since it is only assigned in constructor.
Change-Id: I4e8b7bec4e45badcde6274d574b8a9aabd046837
