From 3d00aa6de95fb46e36f2bab4e3facdf0b96acf06 Mon Sep 17 00:00:00 2001 From: Eric Laurent Date: Tue, 24 Sep 2013 09:53:27 -0700 Subject: soundpool: allocate shared memory heap by client Current SoundPool implementation allocates the shared memory heap containing decoded PCM samples in mediaserver process. When mediaserver process crashes, the shared memory heaps allocated by AudioCache cannot be mapped anymore in the new instance of mediaserver. This causes a silent failure to end playback of new sounds because AudioFlinger believes the new AudioTracks are opened in streaming mode and not static mode: it sees a NULL shared memory pointer when the track is created. The fix consists in allocating the memory heap in the client process. Thus the heap is not lost when mediaserver restarts. The global memory usage is the same as this is shared memory. Also added a way to detect that a shared memory is passed when the track is created but cannot be mapped on mediaserver side. Also fix a crash in SoundPool when ALOGV is enabled. Bug: 10894793. Change-Id: Ice6c66ec3b2a409d75dc903a508b6c6fbfb2e8a7 --- include/media/IMediaPlayerService.h | 8 ++- include/media/SoundPool.h | 3 + include/media/mediaplayer.h | 8 ++- media/libmedia/IAudioFlinger.cpp | 27 ++++++-- media/libmedia/IMediaPlayerService.cpp | 73 +++++++++++++++------- media/libmedia/SoundPool.cpp | 40 ++++++------ media/libmedia/mediaplayer.cpp | 24 ++++--- media/libmediaplayerservice/MediaPlayerService.cpp | 49 ++++++++------- media/libmediaplayerservice/MediaPlayerService.h | 13 ++-- 9 files changed, 162 insertions(+), 83 deletions(-) diff --git a/include/media/IMediaPlayerService.h b/include/media/IMediaPlayerService.h index fef7af2..2998b37 100644 --- a/include/media/IMediaPlayerService.h +++ b/include/media/IMediaPlayerService.h @@ -49,8 +49,12 @@ public: virtual sp createMetadataRetriever() = 0; virtual sp create(const sp& client, int audioSessionId = 0) = 0; - virtual sp decode(const char* url, uint32_t *pSampleRate, int* pNumChannels, audio_format_t* pFormat) = 0; - virtual sp decode(int fd, int64_t offset, int64_t length, uint32_t *pSampleRate, int* pNumChannels, audio_format_t* pFormat) = 0; + virtual status_t decode(const char* url, uint32_t *pSampleRate, int* pNumChannels, + audio_format_t* pFormat, + const sp& heap, size_t *pSize) = 0; + virtual status_t decode(int fd, int64_t offset, int64_t length, uint32_t *pSampleRate, + int* pNumChannels, audio_format_t* pFormat, + const sp& heap, size_t *pSize) = 0; virtual sp getOMX() = 0; virtual sp makeCrypto() = 0; virtual sp makeDrm() = 0; diff --git a/include/media/SoundPool.h b/include/media/SoundPool.h index 9e5654f..2dd78cc 100644 --- a/include/media/SoundPool.h +++ b/include/media/SoundPool.h @@ -22,6 +22,8 @@ #include #include #include +#include +#include namespace android { @@ -85,6 +87,7 @@ private: int64_t mLength; char* mUrl; sp mData; + sp mHeap; }; // stores pending events for stolen channels diff --git a/include/media/mediaplayer.h b/include/media/mediaplayer.h index 923c8b2..2177c4c 100644 --- a/include/media/mediaplayer.h +++ b/include/media/mediaplayer.h @@ -223,8 +223,12 @@ public: bool isLooping(); status_t setVolume(float leftVolume, float rightVolume); void notify(int msg, int ext1, int ext2, const Parcel *obj = NULL); - static sp decode(const char* url, uint32_t *pSampleRate, int* pNumChannels, audio_format_t* pFormat); - static sp decode(int fd, int64_t offset, int64_t length, uint32_t *pSampleRate, int* pNumChannels, audio_format_t* pFormat); + static status_t decode(const char* url, uint32_t *pSampleRate, int* pNumChannels, + audio_format_t* pFormat, + const sp& heap, size_t *pSize); + static status_t decode(int fd, int64_t offset, int64_t length, uint32_t *pSampleRate, + int* pNumChannels, audio_format_t* pFormat, + const sp& heap, size_t *pSize); status_t invoke(const Parcel& request, Parcel *reply); status_t setMetadataFilter(const Parcel& filter); status_t getMetadata(bool update_only, bool apply_filter, Parcel *metadata); diff --git a/media/libmedia/IAudioFlinger.cpp b/media/libmedia/IAudioFlinger.cpp index be818c6..68928f1 100644 --- a/media/libmedia/IAudioFlinger.cpp +++ b/media/libmedia/IAudioFlinger.cpp @@ -108,7 +108,12 @@ public: data.writeInt32(frameCount); track_flags_t lFlags = flags != NULL ? *flags : (track_flags_t) TRACK_DEFAULT; data.writeInt32(lFlags); - data.writeStrongBinder(sharedBuffer->asBinder()); + if (sharedBuffer != 0) { + data.writeInt32(true); + data.writeStrongBinder(sharedBuffer->asBinder()); + } else { + data.writeInt32(false); + } data.writeInt32((int32_t) output); data.writeInt32((int32_t) tid); int lSessionId = 0; @@ -738,15 +743,27 @@ status_t BnAudioFlinger::onTransact( audio_channel_mask_t channelMask = data.readInt32(); size_t frameCount = data.readInt32(); track_flags_t flags = (track_flags_t) data.readInt32(); - sp buffer = interface_cast(data.readStrongBinder()); + bool haveSharedBuffer = data.readInt32() != 0; + sp buffer; + if (haveSharedBuffer) { + buffer = interface_cast(data.readStrongBinder()); + } audio_io_handle_t output = (audio_io_handle_t) data.readInt32(); pid_t tid = (pid_t) data.readInt32(); int sessionId = data.readInt32(); String8 name; status_t status; - sp track = createTrack( - (audio_stream_type_t) streamType, sampleRate, format, - channelMask, frameCount, &flags, buffer, output, tid, &sessionId, name, &status); + sp track; + if ((haveSharedBuffer && (buffer == 0)) || + ((buffer != 0) && (buffer->pointer() == NULL))) { + ALOGW("CREATE_TRACK: cannot retrieve shared memory"); + status = DEAD_OBJECT; + } else { + track = createTrack( + (audio_stream_type_t) streamType, sampleRate, format, + channelMask, frameCount, &flags, buffer, output, tid, + &sessionId, name, &status); + } reply->writeInt32(flags); reply->writeInt32(sessionId); reply->writeString8(name); diff --git a/media/libmedia/IMediaPlayerService.cpp b/media/libmedia/IMediaPlayerService.cpp index 74f574d..3c22b4c 100644 --- a/media/libmedia/IMediaPlayerService.cpp +++ b/media/libmedia/IMediaPlayerService.cpp @@ -86,30 +86,48 @@ public: return interface_cast(reply.readStrongBinder()); } - virtual sp decode(const char* url, uint32_t *pSampleRate, int* pNumChannels, audio_format_t* pFormat) + virtual status_t decode(const char* url, uint32_t *pSampleRate, int* pNumChannels, + audio_format_t* pFormat, + const sp& heap, size_t *pSize) { Parcel data, reply; data.writeInterfaceToken(IMediaPlayerService::getInterfaceDescriptor()); data.writeCString(url); - remote()->transact(DECODE_URL, data, &reply); - *pSampleRate = uint32_t(reply.readInt32()); - *pNumChannels = reply.readInt32(); - *pFormat = (audio_format_t) reply.readInt32(); - return interface_cast(reply.readStrongBinder()); + data.writeStrongBinder(heap->asBinder()); + status_t status = remote()->transact(DECODE_URL, data, &reply); + if (status == NO_ERROR) { + status = (status_t)reply.readInt32(); + if (status == NO_ERROR) { + *pSampleRate = uint32_t(reply.readInt32()); + *pNumChannels = reply.readInt32(); + *pFormat = (audio_format_t)reply.readInt32(); + *pSize = (size_t)reply.readInt32(); + } + } + return status; } - virtual sp decode(int fd, int64_t offset, int64_t length, uint32_t *pSampleRate, int* pNumChannels, audio_format_t* pFormat) + virtual status_t decode(int fd, int64_t offset, int64_t length, uint32_t *pSampleRate, + int* pNumChannels, audio_format_t* pFormat, + const sp& heap, size_t *pSize) { Parcel data, reply; data.writeInterfaceToken(IMediaPlayerService::getInterfaceDescriptor()); data.writeFileDescriptor(fd); data.writeInt64(offset); data.writeInt64(length); - remote()->transact(DECODE_FD, data, &reply); - *pSampleRate = uint32_t(reply.readInt32()); - *pNumChannels = reply.readInt32(); - *pFormat = (audio_format_t) reply.readInt32(); - return interface_cast(reply.readStrongBinder()); + data.writeStrongBinder(heap->asBinder()); + status_t status = remote()->transact(DECODE_FD, data, &reply); + if (status == NO_ERROR) { + status = (status_t)reply.readInt32(); + if (status == NO_ERROR) { + *pSampleRate = uint32_t(reply.readInt32()); + *pNumChannels = reply.readInt32(); + *pFormat = (audio_format_t)reply.readInt32(); + *pSize = (size_t)reply.readInt32(); + } + } + return status; } virtual sp getOMX() { @@ -205,14 +223,19 @@ status_t BnMediaPlayerService::onTransact( case DECODE_URL: { CHECK_INTERFACE(IMediaPlayerService, data, reply); const char* url = data.readCString(); + sp heap = interface_cast(data.readStrongBinder()); uint32_t sampleRate; int numChannels; audio_format_t format; - sp player = decode(url, &sampleRate, &numChannels, &format); - reply->writeInt32(sampleRate); - reply->writeInt32(numChannels); - reply->writeInt32((int32_t) format); - reply->writeStrongBinder(player->asBinder()); + size_t size; + status_t status = decode(url, &sampleRate, &numChannels, &format, heap, &size); + reply->writeInt32(status); + if (status == NO_ERROR) { + reply->writeInt32(sampleRate); + reply->writeInt32(numChannels); + reply->writeInt32((int32_t)format); + reply->writeInt32((int32_t)size); + } return NO_ERROR; } break; case DECODE_FD: { @@ -220,14 +243,20 @@ status_t BnMediaPlayerService::onTransact( int fd = dup(data.readFileDescriptor()); int64_t offset = data.readInt64(); int64_t length = data.readInt64(); + sp heap = interface_cast(data.readStrongBinder()); uint32_t sampleRate; int numChannels; audio_format_t format; - sp player = decode(fd, offset, length, &sampleRate, &numChannels, &format); - reply->writeInt32(sampleRate); - reply->writeInt32(numChannels); - reply->writeInt32((int32_t) format); - reply->writeStrongBinder(player->asBinder()); + size_t size; + status_t status = decode(fd, offset, length, &sampleRate, &numChannels, &format, + heap, &size); + reply->writeInt32(status); + if (status == NO_ERROR) { + reply->writeInt32(sampleRate); + reply->writeInt32(numChannels); + reply->writeInt32((int32_t)format); + reply->writeInt32((int32_t)size); + } return NO_ERROR; } break; case CREATE_MEDIA_RECORDER: { diff --git a/media/libmedia/SoundPool.cpp b/media/libmedia/SoundPool.cpp index 5239b2f..8434d43 100644 --- a/media/libmedia/SoundPool.cpp +++ b/media/libmedia/SoundPool.cpp @@ -32,6 +32,8 @@ int kDefaultBufferCount = 4; uint32_t kMaxSampleRate = 48000; uint32_t kDefaultSampleRate = 44100; uint32_t kDefaultFrameCount = 1200; +size_t kDefaultHeapSize = 1024 * 1024; // 1MB + SoundPool::SoundPool(int maxChannels, audio_stream_type_t streamType, int srcQuality) { @@ -464,7 +466,6 @@ Sample::Sample(int sampleID, int fd, int64_t offset, int64_t length) void Sample::init() { - mData = 0; mSize = 0; mRefCount = 0; mSampleID = 0; @@ -482,7 +483,6 @@ Sample::~Sample() ALOGV("close(%d)", mFd); ::close(mFd); } - mData.clear(); free(mUrl); } @@ -491,44 +491,48 @@ status_t Sample::doLoad() uint32_t sampleRate; int numChannels; audio_format_t format; - sp p; + status_t status; + mHeap = new MemoryHeapBase(kDefaultHeapSize); + ALOGV("Start decode"); if (mUrl) { - p = MediaPlayer::decode(mUrl, &sampleRate, &numChannels, &format); + status = MediaPlayer::decode(mUrl, &sampleRate, &numChannels, &format, mHeap, &mSize); } else { - p = MediaPlayer::decode(mFd, mOffset, mLength, &sampleRate, &numChannels, &format); + status = MediaPlayer::decode(mFd, mOffset, mLength, &sampleRate, &numChannels, &format, + mHeap, &mSize); ALOGV("close(%d)", mFd); ::close(mFd); mFd = -1; } - if (p == 0) { + if (status != NO_ERROR) { ALOGE("Unable to load sample: %s", mUrl); - return -1; + goto error; } ALOGV("pointer = %p, size = %u, sampleRate = %u, numChannels = %d", - p->pointer(), p->size(), sampleRate, numChannels); + mHeap->getBase(), mSize, sampleRate, numChannels); if (sampleRate > kMaxSampleRate) { ALOGE("Sample rate (%u) out of range", sampleRate); - return - 1; + status = BAD_VALUE; + goto error; } if ((numChannels < 1) || (numChannels > 2)) { ALOGE("Sample channel count (%d) out of range", numChannels); - return - 1; + status = BAD_VALUE; + goto error; } - //_dumpBuffer(p->pointer(), p->size()); - uint8_t* q = static_cast(p->pointer()) + p->size() - 10; - //_dumpBuffer(q, 10, 10, false); - - mData = p; - mSize = p->size(); + mData = new MemoryBase(mHeap, 0, mSize); mSampleRate = sampleRate; mNumChannels = numChannels; mFormat = format; mState = READY; - return 0; + return NO_ERROR; + +error: + mHeap.clear(); + return status; } @@ -744,7 +748,7 @@ void SoundChannel::process(int event, void *info, unsigned long toggle) ALOGV("process %p channel %d EVENT_UNDERRUN or EVENT_BUFFER_END", this, mChannelID); mSoundPool->addToStopList(this); } else if (event == AudioTrack::EVENT_LOOP_END) { - ALOGV("End loop %p channel %d count %d", this, mChannelID, *(int *)info); + ALOGV("End loop %p channel %d", this, mChannelID); } } diff --git a/media/libmedia/mediaplayer.cpp b/media/libmedia/mediaplayer.cpp index 4323d0c..0f6d897 100644 --- a/media/libmedia/mediaplayer.cpp +++ b/media/libmedia/mediaplayer.cpp @@ -776,17 +776,20 @@ void MediaPlayer::notify(int msg, int ext1, int ext2, const Parcel *obj) } } -/*static*/ sp MediaPlayer::decode(const char* url, uint32_t *pSampleRate, int* pNumChannels, audio_format_t* pFormat) +/*static*/ status_t MediaPlayer::decode(const char* url, uint32_t *pSampleRate, + int* pNumChannels, audio_format_t* pFormat, + const sp& heap, size_t *pSize) { ALOGV("decode(%s)", url); - sp p; + status_t status; const sp& service = getMediaPlayerService(); if (service != 0) { - p = service->decode(url, pSampleRate, pNumChannels, pFormat); + status = service->decode(url, pSampleRate, pNumChannels, pFormat, heap, pSize); } else { ALOGE("Unable to locate media service"); + status = DEAD_OBJECT; } - return p; + return status; } @@ -796,17 +799,22 @@ void MediaPlayer::died() notify(MEDIA_ERROR, MEDIA_ERROR_SERVER_DIED, 0); } -/*static*/ sp MediaPlayer::decode(int fd, int64_t offset, int64_t length, uint32_t *pSampleRate, int* pNumChannels, audio_format_t* pFormat) +/*static*/ status_t MediaPlayer::decode(int fd, int64_t offset, int64_t length, + uint32_t *pSampleRate, int* pNumChannels, + audio_format_t* pFormat, + const sp& heap, size_t *pSize) { ALOGV("decode(%d, %lld, %lld)", fd, offset, length); - sp p; + status_t status; const sp& service = getMediaPlayerService(); if (service != 0) { - p = service->decode(fd, offset, length, pSampleRate, pNumChannels, pFormat); + status = service->decode(fd, offset, length, pSampleRate, + pNumChannels, pFormat, heap, pSize); } else { ALOGE("Unable to locate media service"); + status = DEAD_OBJECT; } - return p; + return status; } diff --git a/media/libmediaplayerservice/MediaPlayerService.cpp b/media/libmediaplayerservice/MediaPlayerService.cpp index 0dabd37..9553458 100644 --- a/media/libmediaplayerservice/MediaPlayerService.cpp +++ b/media/libmediaplayerservice/MediaPlayerService.cpp @@ -319,8 +319,8 @@ status_t MediaPlayerService::AudioCache::dump(int fd, const Vector& ar result.append(" AudioCache\n"); if (mHeap != 0) { - snprintf(buffer, 255, " heap base(%p), size(%d), flags(%d), device(%s)\n", - mHeap->getBase(), mHeap->getSize(), mHeap->getFlags(), mHeap->getDevice()); + snprintf(buffer, 255, " heap base(%p), size(%d), flags(%d)\n", + mHeap->getBase(), mHeap->getSize(), mHeap->getFlags()); result.append(buffer); } snprintf(buffer, 255, " msec per frame(%f), channel count(%d), format(%d), frame count(%zd)\n", @@ -1176,13 +1176,13 @@ int Antagonizer::callbackThread(void* user) } #endif -static size_t kDefaultHeapSize = 1024 * 1024; // 1MB - -sp MediaPlayerService::decode(const char* url, uint32_t *pSampleRate, int* pNumChannels, audio_format_t* pFormat) +status_t MediaPlayerService::decode(const char* url, uint32_t *pSampleRate, int* pNumChannels, + audio_format_t* pFormat, + const sp& heap, size_t *pSize) { ALOGV("decode(%s)", url); - sp mem; sp player; + status_t status = BAD_VALUE; // Protect our precious, precious DRMd ringtones by only allowing // decoding of http, but not filesystem paths or content Uris. @@ -1190,7 +1190,7 @@ sp MediaPlayerService::decode(const char* url, uint32_t *pSampleRate, i // filedescriptor for them and use that. if (url != NULL && strncmp(url, "http://", 7) != 0) { ALOGD("Can't decode %s by path, use filedescriptor instead", url); - return mem; + return BAD_VALUE; } player_type playerType = @@ -1198,7 +1198,7 @@ sp MediaPlayerService::decode(const char* url, uint32_t *pSampleRate, i ALOGV("player type = %d", playerType); // create the right type of player - sp cache = new AudioCache(url); + sp cache = new AudioCache(heap); player = MediaPlayerFactory::createPlayer(playerType, cache.get(), cache->notify); if (player == NULL) goto Exit; if (player->hardwareOutput()) goto Exit; @@ -1224,22 +1224,27 @@ sp MediaPlayerService::decode(const char* url, uint32_t *pSampleRate, i goto Exit; } - mem = new MemoryBase(cache->getHeap(), 0, cache->size()); + *pSize = cache->size(); *pSampleRate = cache->sampleRate(); *pNumChannels = cache->channelCount(); *pFormat = cache->format(); - ALOGV("return memory @ %p, sampleRate=%u, channelCount = %d, format = %d", mem->pointer(), *pSampleRate, *pNumChannels, *pFormat); + ALOGV("return size %d sampleRate=%u, channelCount = %d, format = %d", + *pSize, *pSampleRate, *pNumChannels, *pFormat); + status = NO_ERROR; Exit: if (player != 0) player->reset(); - return mem; + return status; } -sp MediaPlayerService::decode(int fd, int64_t offset, int64_t length, uint32_t *pSampleRate, int* pNumChannels, audio_format_t* pFormat) +status_t MediaPlayerService::decode(int fd, int64_t offset, int64_t length, + uint32_t *pSampleRate, int* pNumChannels, + audio_format_t* pFormat, + const sp& heap, size_t *pSize) { ALOGV("decode(%d, %lld, %lld)", fd, offset, length); - sp mem; sp player; + status_t status = BAD_VALUE; player_type playerType = MediaPlayerFactory::getPlayerType(NULL /* client */, fd, @@ -1248,7 +1253,7 @@ sp MediaPlayerService::decode(int fd, int64_t offset, int64_t length, u ALOGV("player type = %d", playerType); // create the right type of player - sp cache = new AudioCache("decode_fd"); + sp cache = new AudioCache(heap); player = MediaPlayerFactory::createPlayer(playerType, cache.get(), cache->notify); if (player == NULL) goto Exit; if (player->hardwareOutput()) goto Exit; @@ -1274,16 +1279,18 @@ sp MediaPlayerService::decode(int fd, int64_t offset, int64_t length, u goto Exit; } - mem = new MemoryBase(cache->getHeap(), 0, cache->size()); + *pSize = cache->size(); *pSampleRate = cache->sampleRate(); *pNumChannels = cache->channelCount(); *pFormat = cache->format(); - ALOGV("return memory @ %p, sampleRate=%u, channelCount = %d, format = %d", mem->pointer(), *pSampleRate, *pNumChannels, *pFormat); + ALOGV("return size %d, sampleRate=%u, channelCount = %d, format = %d", + *pSize, *pSampleRate, *pNumChannels, *pFormat); + status = NO_ERROR; Exit: if (player != 0) player->reset(); ::close(fd); - return mem; + return status; } @@ -1803,12 +1810,10 @@ int MediaPlayerService::AudioOutput::getSessionId() const #undef LOG_TAG #define LOG_TAG "AudioCache" -MediaPlayerService::AudioCache::AudioCache(const char* name) : - mChannelCount(0), mFrameCount(1024), mSampleRate(0), mSize(0), - mError(NO_ERROR), mCommandComplete(false) +MediaPlayerService::AudioCache::AudioCache(const sp& heap) : + mHeap(heap), mChannelCount(0), mFrameCount(1024), mSampleRate(0), mSize(0), + mError(NO_ERROR), mCommandComplete(false) { - // create ashmem heap - mHeap = new MemoryHeapBase(kDefaultHeapSize, 0, name); } uint32_t MediaPlayerService::AudioCache::latency () const diff --git a/media/libmediaplayerservice/MediaPlayerService.h b/media/libmediaplayerservice/MediaPlayerService.h index 7d27944..21f4117 100644 --- a/media/libmediaplayerservice/MediaPlayerService.h +++ b/media/libmediaplayerservice/MediaPlayerService.h @@ -177,7 +177,7 @@ class MediaPlayerService : public BnMediaPlayerService class AudioCache : public MediaPlayerBase::AudioSink { public: - AudioCache(const char* name); + AudioCache(const sp& heap); virtual ~AudioCache() {} virtual bool ready() const { return (mChannelCount > 0) && (mHeap->getHeapID() > 0); } @@ -224,7 +224,7 @@ class MediaPlayerService : public BnMediaPlayerService Mutex mLock; Condition mSignal; - sp mHeap; + sp mHeap; float mMsecsPerFrame; uint16_t mChannelCount; audio_format_t mFormat; @@ -247,8 +247,13 @@ public: virtual sp create(const sp& client, int audioSessionId); - virtual sp decode(const char* url, uint32_t *pSampleRate, int* pNumChannels, audio_format_t* pFormat); - virtual sp decode(int fd, int64_t offset, int64_t length, uint32_t *pSampleRate, int* pNumChannels, audio_format_t* pFormat); + virtual status_t decode(const char* url, uint32_t *pSampleRate, int* pNumChannels, + audio_format_t* pFormat, + const sp& heap, size_t *pSize); + virtual status_t decode(int fd, int64_t offset, int64_t length, + uint32_t *pSampleRate, int* pNumChannels, + audio_format_t* pFormat, + const sp& heap, size_t *pSize); virtual sp getOMX(); virtual sp makeCrypto(); virtual sp makeDrm(); -- cgit v1.1