From ad0c79ec27d6534ef7f4496859c5e9cb261153a8 Mon Sep 17 00:00:00 2001 From: Sridhar Vashist Date: Thu, 23 Jul 2015 13:48:54 -0500 Subject: DO NOT MERGE AwesomePlayer: Stop posting buffering events once at EOS Stop posting buffering events once at end of stream to avoid perpetually holding the 'TimedEventQueue' wakelock in libstagefright. Change-Id: I3b8012886f2c27e830ce215b14090c35825635cd Signed-off-by: Sridhar Vashist --- media/libstagefright/AwesomePlayer.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/media/libstagefright/AwesomePlayer.cpp b/media/libstagefright/AwesomePlayer.cpp index 007c090..7e5f8c3 100644 --- a/media/libstagefright/AwesomePlayer.cpp +++ b/media/libstagefright/AwesomePlayer.cpp @@ -826,7 +826,8 @@ void AwesomePlayer::onBufferingUpdate() { } } - if (mFlags & (PLAYING | PREPARING | CACHE_UNDERRUN)) { + if ( ((mFlags & PLAYING) && !eos) || + (mFlags & (PREPARING | CACHE_UNDERRUN)) ) { postBufferingEvent_l(); } } -- cgit v1.1 From e90bf7573f4c420705285a3b2666cb07c73b1cbc Mon Sep 17 00:00:00 2001 From: Sridhar Vashist Date: Thu, 23 Jul 2015 16:43:10 -0500 Subject: DO NOT MERGE AwesomePlayer: Separate cache buffer watermarks for offload audio - Using normal playback cache watermarks for offload playback leads to cache underruns & buffering pauses resulting in choppy audio. - Add new properties to define cache hi/low watermarks for offload audio. - Calculate cache buffer levels only based on size for offload audio. Change-Id: Idb8c1be351678d57490939187079f452a65aebc3 Signed-off-by: Sridhar Vashist --- media/libstagefright/AwesomePlayer.cpp | 39 +++++++++++++++++++++++----------- 1 file changed, 27 insertions(+), 12 deletions(-) diff --git a/media/libstagefright/AwesomePlayer.cpp b/media/libstagefright/AwesomePlayer.cpp index 007c090..23623a4 100644 --- a/media/libstagefright/AwesomePlayer.cpp +++ b/media/libstagefright/AwesomePlayer.cpp @@ -65,6 +65,8 @@ #define USE_SURFACE_ALLOC 1 #define FRAME_DROP_FREQ 0 +#define PROPERTY_OFFLOAD_HIWATERMARK "audio.offload.hiwatermark" +#define PROPERTY_OFFLOAD_LOWATERMARK "audio.offload.lowatermark" namespace android { @@ -72,7 +74,8 @@ static int64_t kLowWaterMarkUs = 2000000ll; // 2secs static int64_t kHighWaterMarkUs = 5000000ll; // 5secs static const size_t kLowWaterMarkBytes = 40000; static const size_t kHighWaterMarkBytes = 200000; - +static size_t kOffloadLowWaterMarkBytes = kLowWaterMarkBytes; +static size_t kOffloadHighWaterMarkBytes = kHighWaterMarkBytes; // maximum time in paused state when offloading audio decompression. When elapsed, the AudioPlayer // is destroyed to allow the audio DSP to power down. static int64_t kOffloadPauseMaxUs = 10000000ll; @@ -636,6 +639,11 @@ void AwesomePlayer::reset_l() { mMediaRenderingStartGeneration = 0; mStartGeneration = 0; + + kOffloadLowWaterMarkBytes = + property_get_int32(PROPERTY_OFFLOAD_LOWATERMARK, kLowWaterMarkBytes); + kOffloadHighWaterMarkBytes = + property_get_int32(PROPERTY_OFFLOAD_HIWATERMARK, kHighWaterMarkBytes); } void AwesomePlayer::notifyListener_l(int msg, int ext1, int ext2) { @@ -726,6 +734,7 @@ void AwesomePlayer::onBufferingUpdate() { size_t cachedDataRemaining = mCachedSource->approxDataRemaining(&finalStatus); bool eos = (finalStatus != OK); + ALOGV("cachedDataRemaining = %zu b, eos=%d", cachedDataRemaining, eos); if (eos) { if (finalStatus == ERROR_END_OF_STREAM) { notifyListener_l(MEDIA_BUFFERING_UPDATE, 100); @@ -736,36 +745,42 @@ void AwesomePlayer::onBufferingUpdate() { } } else { bool eos2; + bool knownDuration = false; int64_t cachedDurationUs; if (getCachedDuration_l(&cachedDurationUs, &eos2) && mDurationUs > 0) { + knownDuration = true; int percentage = 100.0 * (double)cachedDurationUs / mDurationUs; if (percentage > 100) { percentage = 100; } notifyListener_l(MEDIA_BUFFERING_UPDATE, percentage); - } else { - // We don't know the bitrate/duration of the stream, use absolute size - // limits to maintain the cache. + } + if (!knownDuration || mOffloadAudio) { + // If we don't know the bitrate/duration of the stream, or are offloading + // decode, use absolute size limits to maintain the cache. + + size_t lowWatermark = + mOffloadAudio ? kOffloadLowWaterMarkBytes : kLowWaterMarkBytes; + size_t highWatermark = + mOffloadAudio ? kOffloadHighWaterMarkBytes : kHighWaterMarkBytes; - if ((mFlags & PLAYING) && !eos - && (cachedDataRemaining < kLowWaterMarkBytes)) { - ALOGI("cache is running low (< %zu) , pausing.", - kLowWaterMarkBytes); + if ((mFlags & PLAYING) && !eos && (cachedDataRemaining < lowWatermark)) { + ALOGI("cache is running low (< %zu) , pausing.", lowWatermark); modifyFlags(CACHE_UNDERRUN, SET); pause_l(); ensureCacheIsFetching_l(); sendCacheStats(); notifyListener_l(MEDIA_INFO, MEDIA_INFO_BUFFERING_START); - } else if (eos || cachedDataRemaining > kHighWaterMarkBytes) { + } else if (eos || cachedDataRemaining > highWatermark) { if (mFlags & CACHE_UNDERRUN) { ALOGI("cache has filled up (> %zu), resuming.", - kHighWaterMarkBytes); + highWatermark); modifyFlags(CACHE_UNDERRUN, CLEAR); play_l(); } else if (mFlags & PREPARING) { ALOGV("cache has filled up (> %zu), prepare is done", - kHighWaterMarkBytes); + highWatermark); finishAsyncPrepare_l(); } } @@ -799,7 +814,7 @@ void AwesomePlayer::onBufferingUpdate() { int64_t cachedDurationUs; bool eos; - if (getCachedDuration_l(&cachedDurationUs, &eos)) { + if (!mOffloadAudio && getCachedDuration_l(&cachedDurationUs, &eos)) { ALOGV("cachedDurationUs = %.2f secs, eos=%d", cachedDurationUs / 1E6, eos); -- cgit v1.1 From b0063fecf524e9fc0ab08e288ae2c8075ecb66c0 Mon Sep 17 00:00:00 2001 From: Sridhar Vashist Date: Mon, 27 Jul 2015 12:30:54 -0500 Subject: DO NOT MERGE audioflinger: dont release wakelock during offload drain sequence workaround to avoid lost offload audio buffers during a compress_drain when a wakelock is not held. reference internal commit# 078538cfc6c4682889ed52de460d29c0d15bb9eb Change-Id: I91fe76f985dfb619610f92f946022184564598cb Signed-off-by: Sridhar Vashist --- services/audioflinger/Threads.cpp | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/services/audioflinger/Threads.cpp b/services/audioflinger/Threads.cpp index 9fccda1..98bdbd9 100644 --- a/services/audioflinger/Threads.cpp +++ b/services/audioflinger/Threads.cpp @@ -2467,13 +2467,23 @@ bool AudioFlinger::PlaybackThread::threadLoop() if (exitPending()) { break; } - releaseWakeLock_l(); + bool released = false; + // The following works around a bug in the offload driver. Ideally we would release + // the wake lock every time, but that causes the last offload buffer(s) to be + // dropped while the device is on battery, so we need to hold a wake lock during + // the drain phase. + if (mBytesRemaining && !(mDrainSequence & 1)) { + releaseWakeLock_l(); + released = true; + } mWakeLockUids.clear(); mActiveTracksGeneration++; ALOGV("wait async completion"); mWaitWorkCV.wait(mLock); ALOGV("async completion/wake"); - acquireWakeLock_l(); + if (released) { + acquireWakeLock_l(); + } standbyTime = systemTime() + standbyDelay; sleepTime = 0; -- cgit v1.1 From d005c5ddb4842369979df7b76f1d0f5f1380fcd9 Mon Sep 17 00:00:00 2001 From: Wei Jia Date: Tue, 13 Oct 2015 10:58:07 -0700 Subject: NuPlayerRenderer: always update MediaClock with max media duration. Bug: 24345295 Change-Id: I868c9c44ea22de98a083432262e485d0f134203f --- .../libmediaplayerservice/nuplayer/NuPlayerRenderer.cpp | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/media/libmediaplayerservice/nuplayer/NuPlayerRenderer.cpp b/media/libmediaplayerservice/nuplayer/NuPlayerRenderer.cpp index 776dba8..8053245 100644 --- a/media/libmediaplayerservice/nuplayer/NuPlayerRenderer.cpp +++ b/media/libmediaplayerservice/nuplayer/NuPlayerRenderer.cpp @@ -910,6 +910,13 @@ bool NuPlayer::Renderer::onDrainAudioQueue() { { Mutex::Autolock autoLock(mLock); + int64_t maxTimeMedia; + maxTimeMedia = + mAnchorTimeMediaUs + + (int64_t)(max((long long)mNumFramesWritten - mAnchorNumFramesWritten, 0LL) + * 1000LL * mAudioSink->msecsPerFrame()); + mMediaClock->updateMaxTimeMedia(maxTimeMedia); + notifyIfMediaRenderingStarted_l(); } @@ -936,15 +943,6 @@ bool NuPlayer::Renderer::onDrainAudioQueue() { break; } } - int64_t maxTimeMedia; - { - Mutex::Autolock autoLock(mLock); - maxTimeMedia = - mAnchorTimeMediaUs + - (int64_t)(max((long long)mNumFramesWritten - mAnchorNumFramesWritten, 0LL) - * 1000LL * mAudioSink->msecsPerFrame()); - } - mMediaClock->updateMaxTimeMedia(maxTimeMedia); // calculate whether we need to reschedule another write. bool reschedule = !mAudioQueue.empty() -- cgit v1.1 From 2be5af3d34fe614c078864aef936352af80d965f Mon Sep 17 00:00:00 2001 From: Eric Laurent Date: Thu, 2 Apr 2015 13:49:15 -0700 Subject: DO NOT MERGE - IAudioFlinger: add checks on binder calls Limit number of ports and patches listed by LIST_AUDIO_PATCHES and LIST_AUDIO_PORTS. Also fix typo causing wring pointer to be used when writing to Parcel. Bug: 19573085. Change-Id: I41a9c710e45738a4f11990160587856c429a4646 (cherry picked from commit 9ef830c6dbd4f6000b94abee3df14b9e27a38294) --- media/libmedia/IAudioFlinger.cpp | 46 +++++++++++++++++++++++++++++++--------- 1 file changed, 36 insertions(+), 10 deletions(-) diff --git a/media/libmedia/IAudioFlinger.cpp b/media/libmedia/IAudioFlinger.cpp index 346a192..d7ca425 100644 --- a/media/libmedia/IAudioFlinger.cpp +++ b/media/libmedia/IAudioFlinger.cpp @@ -83,6 +83,8 @@ enum { GET_AUDIO_HW_SYNC }; +#define MAX_ITEMS_PER_LIST 1024 + class BpAudioFlinger : public BpInterface { public: @@ -1289,15 +1291,27 @@ status_t BnAudioFlinger::onTransact( } break; case LIST_AUDIO_PORTS: { CHECK_INTERFACE(IAudioFlinger, data, reply); - unsigned int num_ports = data.readInt32(); + unsigned int numPortsReq = data.readInt32(); + if (numPortsReq > MAX_ITEMS_PER_LIST) { + numPortsReq = MAX_ITEMS_PER_LIST; + } + unsigned int numPorts = numPortsReq; struct audio_port *ports = - (struct audio_port *)calloc(num_ports, + (struct audio_port *)calloc(numPortsReq, sizeof(struct audio_port)); - status_t status = listAudioPorts(&num_ports, ports); + if (ports == NULL) { + reply->writeInt32(NO_MEMORY); + reply->writeInt32(0); + return NO_ERROR; + } + status_t status = listAudioPorts(&numPorts, ports); reply->writeInt32(status); + reply->writeInt32(numPorts); if (status == NO_ERROR) { - reply->writeInt32(num_ports); - reply->write(&ports, num_ports * sizeof(struct audio_port)); + if (numPortsReq > numPorts) { + numPortsReq = numPorts; + } + reply->write(ports, numPortsReq * sizeof(struct audio_port)); } free(ports); return NO_ERROR; @@ -1336,15 +1350,27 @@ status_t BnAudioFlinger::onTransact( } break; case LIST_AUDIO_PATCHES: { CHECK_INTERFACE(IAudioFlinger, data, reply); - unsigned int num_patches = data.readInt32(); + unsigned int numPatchesReq = data.readInt32(); + if (numPatchesReq > MAX_ITEMS_PER_LIST) { + numPatchesReq = MAX_ITEMS_PER_LIST; + } + unsigned int numPatches = numPatchesReq; struct audio_patch *patches = - (struct audio_patch *)calloc(num_patches, + (struct audio_patch *)calloc(numPatchesReq, sizeof(struct audio_patch)); - status_t status = listAudioPatches(&num_patches, patches); + if (patches == NULL) { + reply->writeInt32(NO_MEMORY); + reply->writeInt32(0); + return NO_ERROR; + } + status_t status = listAudioPatches(&numPatches, patches); reply->writeInt32(status); + reply->writeInt32(numPatches); if (status == NO_ERROR) { - reply->writeInt32(num_patches); - reply->write(&patches, num_patches * sizeof(struct audio_patch)); + if (numPatchesReq > numPatches) { + numPatchesReq = numPatches; + } + reply->write(patches, numPatchesReq * sizeof(struct audio_patch)); } free(patches); return NO_ERROR; -- cgit v1.1 From e4b4cf1b6d8f99da91f287837185b27f919050a1 Mon Sep 17 00:00:00 2001 From: Eric Laurent Date: Fri, 8 May 2015 10:50:03 -0700 Subject: DO NOT MERGE - audio flinger: fix fuzz test crash Clear output stream pointer in duplicating thread when the main output to which it is attached is closed. Also do not forward master mute and volume commands to duplicating threads as this is not applicable. Also fix logic in AudioFlinger::primaryPlaybackThread_l() that could accidentally return a duplicating thread. This never happens because the primary thread is always first in the list. Bug: 20731946. Change-Id: Ic8869699836920351b23d09544c50a258d3fb585 (cherry picked from commit f6870aefc5e31d4220f3778c4e79ff34a61f48ad) --- services/audioflinger/AudioFlinger.cpp | 22 ++++++++++++++++------ services/audioflinger/Threads.cpp | 5 ++++- services/audioflinger/Threads.h | 2 ++ 3 files changed, 22 insertions(+), 7 deletions(-) diff --git a/services/audioflinger/AudioFlinger.cpp b/services/audioflinger/AudioFlinger.cpp index ccc05a1..b4c9905 100644 --- a/services/audioflinger/AudioFlinger.cpp +++ b/services/audioflinger/AudioFlinger.cpp @@ -751,8 +751,12 @@ status_t AudioFlinger::setMasterVolume(float value) // assigned to HALs which do not have master volume support will apply // master volume during the mix operation. Threads with HALs which do // support master volume will simply ignore the setting. - for (size_t i = 0; i < mPlaybackThreads.size(); i++) + for (size_t i = 0; i < mPlaybackThreads.size(); i++) { + if (mPlaybackThreads.valueAt(i)->isDuplicating()) { + continue; + } mPlaybackThreads.valueAt(i)->setMasterVolume(value); + } return NO_ERROR; } @@ -863,8 +867,12 @@ status_t AudioFlinger::setMasterMute(bool muted) // assigned to HALs which do not have master mute support will apply master // mute during the mix operation. Threads with HALs which do support master // mute will simply ignore the setting. - for (size_t i = 0; i < mPlaybackThreads.size(); i++) + for (size_t i = 0; i < mPlaybackThreads.size(); i++) { + if (mPlaybackThreads.valueAt(i)->isDuplicating()) { + continue; + } mPlaybackThreads.valueAt(i)->setMasterMute(muted); + } return NO_ERROR; } @@ -1859,11 +1867,10 @@ status_t AudioFlinger::closeOutput_nonvirtual(audio_io_handle_t output) if (thread->type() == ThreadBase::MIXER) { for (size_t i = 0; i < mPlaybackThreads.size(); i++) { - if (mPlaybackThreads.valueAt(i)->type() == ThreadBase::DUPLICATING) { + if (mPlaybackThreads.valueAt(i)->isDuplicating()) { DuplicatingThread *dupThread = (DuplicatingThread *)mPlaybackThreads.valueAt(i).get(); dupThread->removeOutputTrack((MixerThread *)thread.get()); - } } } @@ -1890,7 +1897,7 @@ status_t AudioFlinger::closeOutput_nonvirtual(audio_io_handle_t output) // The thread entity (active unit of execution) is no longer running here, // but the ThreadBase container still exists. - if (thread->type() != ThreadBase::DUPLICATING) { + if (!thread->isDuplicating()) { closeOutputFinish(thread); } @@ -2336,6 +2343,9 @@ AudioFlinger::PlaybackThread *AudioFlinger::primaryPlaybackThread_l() const { for (size_t i = 0; i < mPlaybackThreads.size(); i++) { PlaybackThread *thread = mPlaybackThreads.valueAt(i).get(); + if(thread->isDuplicating()) { + continue; + } AudioStreamOut *output = thread->getOutput(); if (output != NULL && output->audioHwDev == mPrimaryHardwareDev) { return thread; @@ -2649,7 +2659,7 @@ status_t AudioFlinger::moveEffectChain_l(int sessionId, // Check whether the destination thread has a channel count of FCC_2, which is // currently required for (most) effects. Prevent moving the effect chain here rather // than disabling the addEffect_l() call in dstThread below. - if ((dstThread->type() == ThreadBase::MIXER || dstThread->type() == ThreadBase::DUPLICATING) && + if ((dstThread->type() == ThreadBase::MIXER || dstThread->isDuplicating()) && dstThread->mChannelCount != FCC_2) { ALOGW("moveEffectChain_l() effect chain failed because" " destination thread %p channel count(%u) != %u", diff --git a/services/audioflinger/Threads.cpp b/services/audioflinger/Threads.cpp index 98bdbd9..31e85c7 100644 --- a/services/audioflinger/Threads.cpp +++ b/services/audioflinger/Threads.cpp @@ -4874,10 +4874,13 @@ void AudioFlinger::DuplicatingThread::removeOutputTrack(MixerThread *thread) mOutputTracks[i]->destroy(); mOutputTracks.removeAt(i); updateWaitTime_l(); + if (thread->getOutput() == mOutput) { + mOutput = NULL; + } return; } } - ALOGV("removeOutputTrack(): unkonwn thread: %p", thread); + ALOGV("removeOutputTrack(): unknown thread: %p", thread); } // caller must hold mLock diff --git a/services/audioflinger/Threads.h b/services/audioflinger/Threads.h index 1088843..56a42a8 100644 --- a/services/audioflinger/Threads.h +++ b/services/audioflinger/Threads.h @@ -229,6 +229,8 @@ public: // static externally-visible type_t type() const { return mType; } + bool isDuplicating() const { return (mType == DUPLICATING); } + audio_io_handle_t id() const { return mId;} // dynamic externally-visible -- cgit v1.1 From b49c385232adfc7f82d7194c7b19b5966499f66b Mon Sep 17 00:00:00 2001 From: Ronghua Wu Date: Mon, 26 Oct 2015 10:17:37 -0700 Subject: ALooper::awaitResponse gets reply and returns immediately if the looper is stopped. Bug: 25088488 Change-Id: Id33d5d75f1173db52d00f4ff71d4c2c4f27f72f5 --- media/libstagefright/foundation/ALooper.cpp | 20 ++++---------------- 1 file changed, 4 insertions(+), 16 deletions(-) diff --git a/media/libstagefright/foundation/ALooper.cpp b/media/libstagefright/foundation/ALooper.cpp index 5c2e9f9..9921636 100644 --- a/media/libstagefright/foundation/ALooper.cpp +++ b/media/libstagefright/foundation/ALooper.cpp @@ -234,31 +234,19 @@ sp ALooper::createReplyToken() { // to be called by AMessage::postAndAwaitResponse only status_t ALooper::awaitResponse(const sp &replyToken, sp *response) { - { - Mutex::Autolock autoLock(mLock); - if (mThread == NULL) { - return -ENOENT; - } - } - // return status in case we want to handle an interrupted wait Mutex::Autolock autoLock(mRepliesLock); CHECK(replyToken != NULL); - bool gotReply; - bool shouldContinue = true; - while (!(gotReply = replyToken->retrieveReply(response)) && shouldContinue) { - mRepliesCondition.wait(mRepliesLock); - + while (!replyToken->retrieveReply(response)) { { Mutex::Autolock autoLock(mLock); if (mThread == NULL) { - shouldContinue = false; - // continue and try to get potential reply one more time before break the loop + return -ENOENT; } } + mRepliesCondition.wait(mRepliesLock); } - - return gotReply ? OK : -ENOENT; + return OK; } status_t ALooper::postReply(const sp &replyToken, const sp &reply) { -- cgit v1.1 From 76d4c7ffa9cdeed39c93d685c9c03b0915262a8b Mon Sep 17 00:00:00 2001 From: Ronghua Wu Date: Fri, 23 Oct 2015 15:01:53 -0700 Subject: Reduce lock time for dump to make sure not locked when calling back to IResourceManagerClient. Bug: 25166048 Change-Id: I35f9917079c4b783a7cf4cef94b3c7112760c0b8 --- .../ResourceManagerService.cpp | 37 +++++++++++++++------- 1 file changed, 26 insertions(+), 11 deletions(-) diff --git a/services/mediaresourcemanager/ResourceManagerService.cpp b/services/mediaresourcemanager/ResourceManagerService.cpp index 4790754..6781a36 100644 --- a/services/mediaresourcemanager/ResourceManagerService.cpp +++ b/services/mediaresourcemanager/ResourceManagerService.cpp @@ -90,11 +90,7 @@ static ResourceInfo& getResourceInfoForEdit( } status_t ResourceManagerService::dump(int fd, const Vector& /* args */) { - Mutex::Autolock lock(mLock); - String8 result; - const size_t SIZE = 256; - char buffer[SIZE]; if (checkCallingPermission(String16("android.permission.DUMP")) == false) { result.format("Permission Denial: " @@ -105,20 +101,35 @@ status_t ResourceManagerService::dump(int fd, const Vector& /* args */ return PERMISSION_DENIED; } + PidResourceInfosMap mapCopy; + bool supportsMultipleSecureCodecs; + bool supportsSecureWithNonSecureCodec; + String8 serviceLog; + { + Mutex::Autolock lock(mLock); + mapCopy = mMap; // Shadow copy, real copy will happen on write. + supportsMultipleSecureCodecs = mSupportsMultipleSecureCodecs; + supportsSecureWithNonSecureCodec = mSupportsSecureWithNonSecureCodec; + serviceLog = mServiceLog->toString(" " /* linePrefix */); + } + + const size_t SIZE = 256; + char buffer[SIZE]; snprintf(buffer, SIZE, "ResourceManagerService: %p\n", this); result.append(buffer); result.append(" Policies:\n"); - snprintf(buffer, SIZE, " SupportsMultipleSecureCodecs: %d\n", mSupportsMultipleSecureCodecs); + snprintf(buffer, SIZE, " SupportsMultipleSecureCodecs: %d\n", supportsMultipleSecureCodecs); result.append(buffer); - snprintf(buffer, SIZE, " SupportsSecureWithNonSecureCodec: %d\n", mSupportsSecureWithNonSecureCodec); + snprintf(buffer, SIZE, " SupportsSecureWithNonSecureCodec: %d\n", + supportsSecureWithNonSecureCodec); result.append(buffer); result.append(" Processes:\n"); - for (size_t i = 0; i < mMap.size(); ++i) { - snprintf(buffer, SIZE, " Pid: %d\n", mMap.keyAt(i)); + for (size_t i = 0; i < mapCopy.size(); ++i) { + snprintf(buffer, SIZE, " Pid: %d\n", mapCopy.keyAt(i)); result.append(buffer); - const ResourceInfos &infos = mMap.valueAt(i); + const ResourceInfos &infos = mapCopy.valueAt(i); for (size_t j = 0; j < infos.size(); ++j) { result.append(" Client:\n"); snprintf(buffer, SIZE, " Id: %lld\n", (long long)infos[j].clientId); @@ -136,7 +147,7 @@ status_t ResourceManagerService::dump(int fd, const Vector& /* args */ } } result.append(" Events logs (most recent at top):\n"); - result.append(mServiceLog->toString(" " /* linePrefix */)); + result.append(serviceLog); write(fd, result.string(), result.size()); return OK; @@ -307,6 +318,10 @@ bool ResourceManagerService::reclaimResource( } } + if (failedClient == NULL) { + return true; + } + { Mutex::Autolock lock(mLock); bool found = false; @@ -329,7 +344,7 @@ bool ResourceManagerService::reclaimResource( } } - return (failedClient == NULL); + return false; } bool ResourceManagerService::getAllClients_l( -- cgit v1.1 From 75c82b50951b21190f710a638c6a26ff7ee6d86d Mon Sep 17 00:00:00 2001 From: Zach Jang Date: Tue, 27 Oct 2015 01:24:55 +0000 Subject: Revert "AudioPolicyService: fix race in AudioCommandThread" This reverts commit 74ce88ff0f24a8c08fdab3a1140212183089c2b5. Change-Id: I39114c8cdd3021951ba93716aaa0c1c03e68538d --- .../audiopolicy/service/AudioPolicyService.cpp | 24 ++++++++-------------- 1 file changed, 8 insertions(+), 16 deletions(-) diff --git a/services/audiopolicy/service/AudioPolicyService.cpp b/services/audiopolicy/service/AudioPolicyService.cpp index c77cc45..eefff3d 100644 --- a/services/audiopolicy/service/AudioPolicyService.cpp +++ b/services/audiopolicy/service/AudioPolicyService.cpp @@ -577,28 +577,22 @@ bool AudioPolicyService::AudioCommandThread::threadLoop() } } waitTime = INT64_MAX; - // release mLock before releasing strong reference on the service as - // AudioPolicyService destructor calls AudioCommandThread::exit() which - // acquires mLock. - mLock.unlock(); - svc.clear(); - mLock.lock(); } else { waitTime = mAudioCommands[0]->mTime - curTime; break; } } - - // release delayed commands wake lock if the queue is empty - if (mAudioCommands.isEmpty()) { + // release mLock before releasing strong reference on the service as + // AudioPolicyService destructor calls AudioCommandThread::exit() which acquires mLock. + mLock.unlock(); + svc.clear(); + mLock.lock(); + if (!exitPending() && (mAudioCommands.isEmpty() || waitTime != INT64_MAX)) { + // release delayed commands wake lock release_wake_lock(mName.string()); - } - - // At this stage we have either an empty command queue or the first command in the queue - // has a finite delay. So unless we are exiting it is safe to wait. - if (!exitPending()) { ALOGV("AudioCommandThread() going to sleep"); mWaitWorkCV.waitRelative(mLock, waitTime); + ALOGV("AudioCommandThread() waking up"); } } // release delayed commands wake lock before quitting @@ -1009,8 +1003,6 @@ void AudioPolicyService::AudioCommandThread::exit() requestExit(); mWaitWorkCV.signal(); } - // Note that we can call it from the thread loop if all other references have been released - // but it will safely return WOULD_BLOCK in this case requestExitAndWait(); } -- cgit v1.1 From 26ce11a1dd2a9b70865aec1b42c365dd19172511 Mon Sep 17 00:00:00 2001 From: Zach Jang Date: Tue, 27 Oct 2015 01:25:27 +0000 Subject: Revert "audio policy: bind setMode() and setPhoneState() operations" This reverts commit 9ddf1c76121caef55a05c537d6a9a1d76c1d17be. Change-Id: I860ecc288a1798605dff46f39107f4450ca5cd56 --- services/audiopolicy/service/AudioPolicyInterfaceImpl.cpp | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/services/audiopolicy/service/AudioPolicyInterfaceImpl.cpp b/services/audiopolicy/service/AudioPolicyInterfaceImpl.cpp index ca365a5..793c26a 100644 --- a/services/audiopolicy/service/AudioPolicyInterfaceImpl.cpp +++ b/services/audiopolicy/service/AudioPolicyInterfaceImpl.cpp @@ -76,14 +76,10 @@ status_t AudioPolicyService::setPhoneState(audio_mode_t state) ALOGV("setPhoneState()"); - // acquire lock before calling setMode() so that setMode() + setPhoneState() are an atomic - // operation from policy manager standpoint (no other operation (e.g track start or stop) - // can be interleaved). - Mutex::Autolock _l(mLock); - // TODO: check if it is more appropriate to do it in platform specific policy manager AudioSystem::setMode(state); + Mutex::Autolock _l(mLock); mAudioPolicyManager->setPhoneState(state); mPhoneState = state; return NO_ERROR; -- cgit v1.1 From a754b4fa874f97a51ed2bee9257f2a870effe619 Mon Sep 17 00:00:00 2001 From: Zach Jang Date: Tue, 27 Oct 2015 01:29:34 +0000 Subject: Revert "Revert "AudioPolicyService: fix race in AudioCommandThread"" This reverts commit 75c82b50951b21190f710a638c6a26ff7ee6d86d. Change-Id: I1b1f147bedf205636ec20b84faf6ef597a781c0d --- .../audiopolicy/service/AudioPolicyService.cpp | 24 ++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/services/audiopolicy/service/AudioPolicyService.cpp b/services/audiopolicy/service/AudioPolicyService.cpp index eefff3d..c77cc45 100644 --- a/services/audiopolicy/service/AudioPolicyService.cpp +++ b/services/audiopolicy/service/AudioPolicyService.cpp @@ -577,22 +577,28 @@ bool AudioPolicyService::AudioCommandThread::threadLoop() } } waitTime = INT64_MAX; + // release mLock before releasing strong reference on the service as + // AudioPolicyService destructor calls AudioCommandThread::exit() which + // acquires mLock. + mLock.unlock(); + svc.clear(); + mLock.lock(); } else { waitTime = mAudioCommands[0]->mTime - curTime; break; } } - // release mLock before releasing strong reference on the service as - // AudioPolicyService destructor calls AudioCommandThread::exit() which acquires mLock. - mLock.unlock(); - svc.clear(); - mLock.lock(); - if (!exitPending() && (mAudioCommands.isEmpty() || waitTime != INT64_MAX)) { - // release delayed commands wake lock + + // release delayed commands wake lock if the queue is empty + if (mAudioCommands.isEmpty()) { release_wake_lock(mName.string()); + } + + // At this stage we have either an empty command queue or the first command in the queue + // has a finite delay. So unless we are exiting it is safe to wait. + if (!exitPending()) { ALOGV("AudioCommandThread() going to sleep"); mWaitWorkCV.waitRelative(mLock, waitTime); - ALOGV("AudioCommandThread() waking up"); } } // release delayed commands wake lock before quitting @@ -1003,6 +1009,8 @@ void AudioPolicyService::AudioCommandThread::exit() requestExit(); mWaitWorkCV.signal(); } + // Note that we can call it from the thread loop if all other references have been released + // but it will safely return WOULD_BLOCK in this case requestExitAndWait(); } -- cgit v1.1 From 3994ffdd2c280aa5fad9f3c41255371cd545c7cf Mon Sep 17 00:00:00 2001 From: Zach Jang Date: Tue, 27 Oct 2015 01:29:45 +0000 Subject: Revert "Revert "audio policy: bind setMode() and setPhoneState() operations"" This reverts commit 26ce11a1dd2a9b70865aec1b42c365dd19172511. Change-Id: I0affb97e7f2eb541ebd6f26c33e8f32261e9e221 --- services/audiopolicy/service/AudioPolicyInterfaceImpl.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/services/audiopolicy/service/AudioPolicyInterfaceImpl.cpp b/services/audiopolicy/service/AudioPolicyInterfaceImpl.cpp index 793c26a..ca365a5 100644 --- a/services/audiopolicy/service/AudioPolicyInterfaceImpl.cpp +++ b/services/audiopolicy/service/AudioPolicyInterfaceImpl.cpp @@ -76,10 +76,14 @@ status_t AudioPolicyService::setPhoneState(audio_mode_t state) ALOGV("setPhoneState()"); + // acquire lock before calling setMode() so that setMode() + setPhoneState() are an atomic + // operation from policy manager standpoint (no other operation (e.g track start or stop) + // can be interleaved). + Mutex::Autolock _l(mLock); + // TODO: check if it is more appropriate to do it in platform specific policy manager AudioSystem::setMode(state); - Mutex::Autolock _l(mLock); mAudioPolicyManager->setPhoneState(state); mPhoneState = state; return NO_ERROR; -- cgit v1.1 From 7845a1f0790a1de9e26e99578f6ce3219cf8efc3 Mon Sep 17 00:00:00 2001 From: Erik Wolsheimer Date: Fri, 30 Oct 2015 12:07:52 -0700 Subject: fix deadlock in MediaPlayerService BUG: 25263909 Change-Id: I3f08c02a851b67ab269e9aef7b2fb17eda09ea5d --- media/libmediaplayerservice/MediaPlayerService.cpp | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/media/libmediaplayerservice/MediaPlayerService.cpp b/media/libmediaplayerservice/MediaPlayerService.cpp index bcfd83a..f0baf69 100644 --- a/media/libmediaplayerservice/MediaPlayerService.cpp +++ b/media/libmediaplayerservice/MediaPlayerService.cpp @@ -1894,8 +1894,13 @@ void MediaPlayerService::AudioOutput::pause() void MediaPlayerService::AudioOutput::close() { ALOGV("close"); - Mutex::Autolock lock(mLock); - close_l(); + sp track; + { + Mutex::Autolock lock(mLock); + track = mTrack; + close_l(); // clears mTrack + } + // destruction of the track occurs outside of mutex. } void MediaPlayerService::AudioOutput::setVolume(float left, float right) -- cgit v1.1 From 87f8cbb223ee516803dbb99699320c2484cbf3ba Mon Sep 17 00:00:00 2001 From: Wei Jia Date: Fri, 20 Nov 2015 10:34:35 -0800 Subject: libstagefright: check requested memory size before allocation for SoftMPEG4Encoder and SoftVPXEncoder. Bug: 25812794 Change-Id: I96dc74734380d462583f6efa33d09946f9532809 --- media/libstagefright/codecs/m4v_h263/enc/SoftMPEG4Encoder.cpp | 9 +++++++++ media/libstagefright/codecs/on2/enc/SoftVPXEncoder.cpp | 9 +++++++++ 2 files changed, 18 insertions(+) diff --git a/media/libstagefright/codecs/m4v_h263/enc/SoftMPEG4Encoder.cpp b/media/libstagefright/codecs/m4v_h263/enc/SoftMPEG4Encoder.cpp index 8240f83..f2a4e65 100644 --- a/media/libstagefright/codecs/m4v_h263/enc/SoftMPEG4Encoder.cpp +++ b/media/libstagefright/codecs/m4v_h263/enc/SoftMPEG4Encoder.cpp @@ -37,6 +37,10 @@ #include +#ifndef INT32_MAX +#define INT32_MAX 2147483647 +#endif + namespace android { template @@ -137,6 +141,11 @@ OMX_ERRORTYPE SoftMPEG4Encoder::initEncParams() { if (mColorFormat != OMX_COLOR_FormatYUV420Planar || mInputDataIsMeta) { // Color conversion is needed. free(mInputFrameData); + mInputFrameData = NULL; + if (((uint64_t)mWidth * mHeight) > ((uint64_t)INT32_MAX / 3)) { + ALOGE("b/25812794, Buffer size is too big."); + return OMX_ErrorBadParameter; + } mInputFrameData = (uint8_t *) malloc((mWidth * mHeight * 3 ) >> 1); CHECK(mInputFrameData != NULL); diff --git a/media/libstagefright/codecs/on2/enc/SoftVPXEncoder.cpp b/media/libstagefright/codecs/on2/enc/SoftVPXEncoder.cpp index e654843..410f9d0 100644 --- a/media/libstagefright/codecs/on2/enc/SoftVPXEncoder.cpp +++ b/media/libstagefright/codecs/on2/enc/SoftVPXEncoder.cpp @@ -26,6 +26,10 @@ #include #include +#ifndef INT32_MAX +#define INT32_MAX 2147483647 +#endif + namespace android { template @@ -315,6 +319,11 @@ status_t SoftVPXEncoder::initEncoder() { if (mColorFormat != OMX_COLOR_FormatYUV420Planar || mInputDataIsMeta) { free(mConversionBuffer); + mConversionBuffer = NULL; + if (((uint64_t)mWidth * mHeight) > ((uint64_t)INT32_MAX / 3)) { + ALOGE("b/25812794, Buffer size is too big."); + return UNKNOWN_ERROR; + } mConversionBuffer = (uint8_t *)malloc(mWidth * mHeight * 3 / 2); if (mConversionBuffer == NULL) { ALOGE("Allocating conversion buffer failed."); -- cgit v1.1 From 701cac1716d5ddb867a5444ea152d26741f3b397 Mon Sep 17 00:00:00 2001 From: Sridhar Vashist Date: Thu, 23 Jul 2015 13:48:54 -0500 Subject: AwesomePlayer: Stop posting buffering events once at EOS Stop posting buffering events once at end of stream to avoid perpetually holding the 'TimedEventQueue' wakelock in libstagefright. Change-Id: I3b8012886f2c27e830ce215b14090c35825635cd Signed-off-by: Sridhar Vashist --- media/libstagefright/AwesomePlayer.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/media/libstagefright/AwesomePlayer.cpp b/media/libstagefright/AwesomePlayer.cpp index 3cd0b0e..d7be07d 100644 --- a/media/libstagefright/AwesomePlayer.cpp +++ b/media/libstagefright/AwesomePlayer.cpp @@ -828,7 +828,8 @@ void AwesomePlayer::onBufferingUpdate() { } } - if (mFlags & (PLAYING | PREPARING | CACHE_UNDERRUN)) { + if ( ((mFlags & PLAYING) && !eos) || + (mFlags & (PREPARING | CACHE_UNDERRUN)) ) { postBufferingEvent_l(); } } -- cgit v1.1 From ddbebb92f831784dc143d2cd8492ce690e238744 Mon Sep 17 00:00:00 2001 From: Sridhar Vashist Date: Thu, 23 Jul 2015 16:43:10 -0500 Subject: AwesomePlayer: Separate cache buffer watermarks for offload audio - Using normal playback cache watermarks for offload playback leads to cache underruns & buffering pauses resulting in choppy audio. - Add new properties to define cache hi/low watermarks for offload audio. - Calculate cache buffer levels only based on size for offload audio. Change-Id: Idb8c1be351678d57490939187079f452a65aebc3 Signed-off-by: Sridhar Vashist --- media/libstagefright/AwesomePlayer.cpp | 39 +++++++++++++++++++++++----------- 1 file changed, 27 insertions(+), 12 deletions(-) diff --git a/media/libstagefright/AwesomePlayer.cpp b/media/libstagefright/AwesomePlayer.cpp index d7be07d..15506ef 100644 --- a/media/libstagefright/AwesomePlayer.cpp +++ b/media/libstagefright/AwesomePlayer.cpp @@ -65,6 +65,8 @@ #define USE_SURFACE_ALLOC 1 #define FRAME_DROP_FREQ 0 +#define PROPERTY_OFFLOAD_HIWATERMARK "audio.offload.hiwatermark" +#define PROPERTY_OFFLOAD_LOWATERMARK "audio.offload.lowatermark" namespace android { @@ -72,7 +74,8 @@ static int64_t kLowWaterMarkUs = 2000000ll; // 2secs static int64_t kHighWaterMarkUs = 5000000ll; // 5secs static const size_t kLowWaterMarkBytes = 40000; static const size_t kHighWaterMarkBytes = 200000; - +static size_t kOffloadLowWaterMarkBytes = kLowWaterMarkBytes; +static size_t kOffloadHighWaterMarkBytes = kHighWaterMarkBytes; // maximum time in paused state when offloading audio decompression. When elapsed, the AudioPlayer // is destroyed to allow the audio DSP to power down. static int64_t kOffloadPauseMaxUs = 10000000ll; @@ -638,6 +641,11 @@ void AwesomePlayer::reset_l() { mMediaRenderingStartGeneration = 0; mStartGeneration = 0; + + kOffloadLowWaterMarkBytes = + property_get_int32(PROPERTY_OFFLOAD_LOWATERMARK, kLowWaterMarkBytes); + kOffloadHighWaterMarkBytes = + property_get_int32(PROPERTY_OFFLOAD_HIWATERMARK, kHighWaterMarkBytes); } void AwesomePlayer::notifyListener_l(int msg, int ext1, int ext2) { @@ -728,6 +736,7 @@ void AwesomePlayer::onBufferingUpdate() { size_t cachedDataRemaining = mCachedSource->approxDataRemaining(&finalStatus); bool eos = (finalStatus != OK); + ALOGV("cachedDataRemaining = %zu b, eos=%d", cachedDataRemaining, eos); if (eos) { if (finalStatus == ERROR_END_OF_STREAM) { notifyListener_l(MEDIA_BUFFERING_UPDATE, 100); @@ -738,36 +747,42 @@ void AwesomePlayer::onBufferingUpdate() { } } else { bool eos2; + bool knownDuration = false; int64_t cachedDurationUs; if (getCachedDuration_l(&cachedDurationUs, &eos2) && mDurationUs > 0) { + knownDuration = true; int percentage = 100.0 * (double)cachedDurationUs / mDurationUs; if (percentage > 100) { percentage = 100; } notifyListener_l(MEDIA_BUFFERING_UPDATE, percentage); - } else { - // We don't know the bitrate/duration of the stream, use absolute size - // limits to maintain the cache. + } + if (!knownDuration || mOffloadAudio) { + // If we don't know the bitrate/duration of the stream, or are offloading + // decode, use absolute size limits to maintain the cache. + + size_t lowWatermark = + mOffloadAudio ? kOffloadLowWaterMarkBytes : kLowWaterMarkBytes; + size_t highWatermark = + mOffloadAudio ? kOffloadHighWaterMarkBytes : kHighWaterMarkBytes; - if ((mFlags & PLAYING) && !eos - && (cachedDataRemaining < kLowWaterMarkBytes)) { - ALOGI("cache is running low (< %zu) , pausing.", - kLowWaterMarkBytes); + if ((mFlags & PLAYING) && !eos && (cachedDataRemaining < lowWatermark)) { + ALOGI("cache is running low (< %zu) , pausing.", lowWatermark); modifyFlags(CACHE_UNDERRUN, SET); pause_l(); ensureCacheIsFetching_l(); sendCacheStats(); notifyListener_l(MEDIA_INFO, MEDIA_INFO_BUFFERING_START); - } else if (eos || cachedDataRemaining > kHighWaterMarkBytes) { + } else if (eos || cachedDataRemaining > highWatermark) { if (mFlags & CACHE_UNDERRUN) { ALOGI("cache has filled up (> %zu), resuming.", - kHighWaterMarkBytes); + highWatermark); modifyFlags(CACHE_UNDERRUN, CLEAR); play_l(); } else if (mFlags & PREPARING) { ALOGV("cache has filled up (> %zu), prepare is done", - kHighWaterMarkBytes); + highWatermark); finishAsyncPrepare_l(); } } @@ -801,7 +816,7 @@ void AwesomePlayer::onBufferingUpdate() { int64_t cachedDurationUs; bool eos; - if (getCachedDuration_l(&cachedDurationUs, &eos)) { + if (!mOffloadAudio && getCachedDuration_l(&cachedDurationUs, &eos)) { ALOGV("cachedDurationUs = %.2f secs, eos=%d", cachedDurationUs / 1E6, eos); -- cgit v1.1 From 92e41514344227f0c0cf09e9a989b455c8490fda Mon Sep 17 00:00:00 2001 From: Chris Elliott Date: Wed, 2 Dec 2015 13:22:51 -0800 Subject: DO NOT MERGE Revert "AwesomePlayer: Stop posting buffering events once at EOS" This reverts commit 701cac1716d5ddb867a5444ea152d26741f3b397. --- media/libstagefright/AwesomePlayer.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/media/libstagefright/AwesomePlayer.cpp b/media/libstagefright/AwesomePlayer.cpp index 15506ef..9a2c7a7 100644 --- a/media/libstagefright/AwesomePlayer.cpp +++ b/media/libstagefright/AwesomePlayer.cpp @@ -843,8 +843,7 @@ void AwesomePlayer::onBufferingUpdate() { } } - if ( ((mFlags & PLAYING) && !eos) || - (mFlags & (PREPARING | CACHE_UNDERRUN)) ) { + if (mFlags & (PLAYING | PREPARING | CACHE_UNDERRUN)) { postBufferingEvent_l(); } } -- cgit v1.1 From 35997452876b7ec164534b5267535076597495cd Mon Sep 17 00:00:00 2001 From: Chris Elliott Date: Wed, 2 Dec 2015 13:24:15 -0800 Subject: DO NOT MERGE Revert "AwesomePlayer: Separate cache buffer watermarks for offload audio" This reverts commit ddbebb92f831784dc143d2cd8492ce690e238744. --- media/libstagefright/AwesomePlayer.cpp | 39 +++++++++++----------------------- 1 file changed, 12 insertions(+), 27 deletions(-) diff --git a/media/libstagefright/AwesomePlayer.cpp b/media/libstagefright/AwesomePlayer.cpp index 9a2c7a7..3cd0b0e 100644 --- a/media/libstagefright/AwesomePlayer.cpp +++ b/media/libstagefright/AwesomePlayer.cpp @@ -65,8 +65,6 @@ #define USE_SURFACE_ALLOC 1 #define FRAME_DROP_FREQ 0 -#define PROPERTY_OFFLOAD_HIWATERMARK "audio.offload.hiwatermark" -#define PROPERTY_OFFLOAD_LOWATERMARK "audio.offload.lowatermark" namespace android { @@ -74,8 +72,7 @@ static int64_t kLowWaterMarkUs = 2000000ll; // 2secs static int64_t kHighWaterMarkUs = 5000000ll; // 5secs static const size_t kLowWaterMarkBytes = 40000; static const size_t kHighWaterMarkBytes = 200000; -static size_t kOffloadLowWaterMarkBytes = kLowWaterMarkBytes; -static size_t kOffloadHighWaterMarkBytes = kHighWaterMarkBytes; + // maximum time in paused state when offloading audio decompression. When elapsed, the AudioPlayer // is destroyed to allow the audio DSP to power down. static int64_t kOffloadPauseMaxUs = 10000000ll; @@ -641,11 +638,6 @@ void AwesomePlayer::reset_l() { mMediaRenderingStartGeneration = 0; mStartGeneration = 0; - - kOffloadLowWaterMarkBytes = - property_get_int32(PROPERTY_OFFLOAD_LOWATERMARK, kLowWaterMarkBytes); - kOffloadHighWaterMarkBytes = - property_get_int32(PROPERTY_OFFLOAD_HIWATERMARK, kHighWaterMarkBytes); } void AwesomePlayer::notifyListener_l(int msg, int ext1, int ext2) { @@ -736,7 +728,6 @@ void AwesomePlayer::onBufferingUpdate() { size_t cachedDataRemaining = mCachedSource->approxDataRemaining(&finalStatus); bool eos = (finalStatus != OK); - ALOGV("cachedDataRemaining = %zu b, eos=%d", cachedDataRemaining, eos); if (eos) { if (finalStatus == ERROR_END_OF_STREAM) { notifyListener_l(MEDIA_BUFFERING_UPDATE, 100); @@ -747,42 +738,36 @@ void AwesomePlayer::onBufferingUpdate() { } } else { bool eos2; - bool knownDuration = false; int64_t cachedDurationUs; if (getCachedDuration_l(&cachedDurationUs, &eos2) && mDurationUs > 0) { - knownDuration = true; int percentage = 100.0 * (double)cachedDurationUs / mDurationUs; if (percentage > 100) { percentage = 100; } notifyListener_l(MEDIA_BUFFERING_UPDATE, percentage); - } - if (!knownDuration || mOffloadAudio) { - // If we don't know the bitrate/duration of the stream, or are offloading - // decode, use absolute size limits to maintain the cache. - - size_t lowWatermark = - mOffloadAudio ? kOffloadLowWaterMarkBytes : kLowWaterMarkBytes; - size_t highWatermark = - mOffloadAudio ? kOffloadHighWaterMarkBytes : kHighWaterMarkBytes; + } else { + // We don't know the bitrate/duration of the stream, use absolute size + // limits to maintain the cache. - if ((mFlags & PLAYING) && !eos && (cachedDataRemaining < lowWatermark)) { - ALOGI("cache is running low (< %zu) , pausing.", lowWatermark); + if ((mFlags & PLAYING) && !eos + && (cachedDataRemaining < kLowWaterMarkBytes)) { + ALOGI("cache is running low (< %zu) , pausing.", + kLowWaterMarkBytes); modifyFlags(CACHE_UNDERRUN, SET); pause_l(); ensureCacheIsFetching_l(); sendCacheStats(); notifyListener_l(MEDIA_INFO, MEDIA_INFO_BUFFERING_START); - } else if (eos || cachedDataRemaining > highWatermark) { + } else if (eos || cachedDataRemaining > kHighWaterMarkBytes) { if (mFlags & CACHE_UNDERRUN) { ALOGI("cache has filled up (> %zu), resuming.", - highWatermark); + kHighWaterMarkBytes); modifyFlags(CACHE_UNDERRUN, CLEAR); play_l(); } else if (mFlags & PREPARING) { ALOGV("cache has filled up (> %zu), prepare is done", - highWatermark); + kHighWaterMarkBytes); finishAsyncPrepare_l(); } } @@ -816,7 +801,7 @@ void AwesomePlayer::onBufferingUpdate() { int64_t cachedDurationUs; bool eos; - if (!mOffloadAudio && getCachedDuration_l(&cachedDurationUs, &eos)) { + if (getCachedDuration_l(&cachedDurationUs, &eos)) { ALOGV("cachedDurationUs = %.2f secs, eos=%d", cachedDurationUs / 1E6, eos); -- cgit v1.1 From 19c47afbc402542720ddd280e1bbde3b2277b586 Mon Sep 17 00:00:00 2001 From: Andy Hung Date: Wed, 2 Dec 2015 15:55:23 -0800 Subject: DO NOT MERGE SoundPool: add lock for findSample access from SoundPoolThread Sample decoding still occurs in SoundPoolThread without holding the SoundPool lock. Bug: 25781119 Change-Id: I11fde005aa9cf5438e0390a0d2dfe0ec1dd282e8 --- include/media/SoundPool.h | 4 +-- media/libmedia/SoundPool.cpp | 61 ++++++++++++++++++++++++++++++-------------- 2 files changed, 44 insertions(+), 21 deletions(-) diff --git a/include/media/SoundPool.h b/include/media/SoundPool.h index 5830475..f57313c 100644 --- a/include/media/SoundPool.h +++ b/include/media/SoundPool.h @@ -187,6 +187,7 @@ public: // called from SoundPoolThread void sampleLoaded(int sampleID); + sp findSample(int sampleID); // called from AudioTrack thread void done_l(SoundChannel* channel); @@ -198,8 +199,7 @@ public: private: SoundPool() {} // no default constructor bool startThreads(); - void doLoad(sp& sample); - sp findSample(int sampleID) { return mSamples.valueFor(sampleID); } + sp findSample_l(int sampleID); SoundChannel* findChannel (int channelID); SoundChannel* findNextChannel (int channelID); SoundChannel* allocateChannel_l(int priority); diff --git a/media/libmedia/SoundPool.cpp b/media/libmedia/SoundPool.cpp index d2e381b..29ad7ea 100644 --- a/media/libmedia/SoundPool.cpp +++ b/media/libmedia/SoundPool.cpp @@ -183,6 +183,17 @@ bool SoundPool::startThreads() return mDecodeThread != NULL; } +sp SoundPool::findSample(int sampleID) +{ + Mutex::Autolock lock(&mLock); + return findSample_l(sampleID); +} + +sp SoundPool::findSample_l(int sampleID) +{ + return mSamples.valueFor(sampleID); +} + SoundChannel* SoundPool::findChannel(int channelID) { for (int i = 0; i < mMaxChannels; ++i) { @@ -206,29 +217,42 @@ SoundChannel* SoundPool::findNextChannel(int channelID) int SoundPool::load(const char* path, int priority __unused) { ALOGV("load: path=%s, priority=%d", path, priority); - Mutex::Autolock lock(&mLock); - sp sample = new Sample(++mNextSampleID, path); - mSamples.add(sample->sampleID(), sample); - doLoad(sample); - return sample->sampleID(); + int sampleID; + { + Mutex::Autolock lock(&mLock); + sampleID = ++mNextSampleID; + sp sample = new Sample(sampleID, path); + mSamples.add(sampleID, sample); + sample->startLoad(); + } + // mDecodeThread->loadSample() must be called outside of mLock. + // mDecodeThread->loadSample() may block on mDecodeThread message queue space; + // the message queue emptying may block on SoundPool::findSample(). + // + // It theoretically possible that sample loads might decode out-of-order. + mDecodeThread->loadSample(sampleID); + return sampleID; } int SoundPool::load(int fd, int64_t offset, int64_t length, int priority __unused) { ALOGV("load: fd=%d, offset=%" PRId64 ", length=%" PRId64 ", priority=%d", fd, offset, length, priority); - Mutex::Autolock lock(&mLock); - sp sample = new Sample(++mNextSampleID, fd, offset, length); - mSamples.add(sample->sampleID(), sample); - doLoad(sample); - return sample->sampleID(); -} - -void SoundPool::doLoad(sp& sample) -{ - ALOGV("doLoad: loading sample sampleID=%d", sample->sampleID()); - sample->startLoad(); - mDecodeThread->loadSample(sample->sampleID()); + int sampleID; + { + Mutex::Autolock lock(&mLock); + sampleID = ++mNextSampleID; + sp sample = new Sample(sampleID, fd, offset, length); + mSamples.add(sampleID, sample); + sample->startLoad(); + } + // mDecodeThread->loadSample() must be called outside of mLock. + // mDecodeThread->loadSample() may block on mDecodeThread message queue space; + // the message queue emptying may block on SoundPool::findSample(). + // + // It theoretically possible that sample loads might decode out-of-order. + mDecodeThread->loadSample(sampleID); + return sampleID; } bool SoundPool::unload(int sampleID) @@ -243,7 +267,6 @@ int SoundPool::play(int sampleID, float leftVolume, float rightVolume, { ALOGV("play sampleID=%d, leftVolume=%f, rightVolume=%f, priority=%d, loop=%d, rate=%f", sampleID, leftVolume, rightVolume, priority, loop, rate); - sp sample; SoundChannel* channel; int channelID; @@ -253,7 +276,7 @@ int SoundPool::play(int sampleID, float leftVolume, float rightVolume, return 0; } // is sample ready? - sample = findSample(sampleID); + sp sample(findSample_l(sampleID)); if ((sample == 0) || (sample->state() != Sample::READY)) { ALOGW(" sample %d not READY", sampleID); return 0; -- cgit v1.1 From 69bd1cf225328e64a5b4ae6935d2b7fe0b7b6400 Mon Sep 17 00:00:00 2001 From: Wei Jia Date: Fri, 20 Nov 2015 10:34:35 -0800 Subject: libstagefright: check requested memory size before allocation for SoftMPEG4Encoder and SoftVPXEncoder. Bug: 25812794 Change-Id: I96dc74734380d462583f6efa33d09946f9532809 (cherry picked from commit 87f8cbb223ee516803dbb99699320c2484cbf3ba) --- media/libstagefright/codecs/m4v_h263/enc/SoftMPEG4Encoder.cpp | 9 +++++++++ media/libstagefright/codecs/on2/enc/SoftVPXEncoder.cpp | 9 +++++++++ 2 files changed, 18 insertions(+) diff --git a/media/libstagefright/codecs/m4v_h263/enc/SoftMPEG4Encoder.cpp b/media/libstagefright/codecs/m4v_h263/enc/SoftMPEG4Encoder.cpp index 8240f83..f2a4e65 100644 --- a/media/libstagefright/codecs/m4v_h263/enc/SoftMPEG4Encoder.cpp +++ b/media/libstagefright/codecs/m4v_h263/enc/SoftMPEG4Encoder.cpp @@ -37,6 +37,10 @@ #include +#ifndef INT32_MAX +#define INT32_MAX 2147483647 +#endif + namespace android { template @@ -137,6 +141,11 @@ OMX_ERRORTYPE SoftMPEG4Encoder::initEncParams() { if (mColorFormat != OMX_COLOR_FormatYUV420Planar || mInputDataIsMeta) { // Color conversion is needed. free(mInputFrameData); + mInputFrameData = NULL; + if (((uint64_t)mWidth * mHeight) > ((uint64_t)INT32_MAX / 3)) { + ALOGE("b/25812794, Buffer size is too big."); + return OMX_ErrorBadParameter; + } mInputFrameData = (uint8_t *) malloc((mWidth * mHeight * 3 ) >> 1); CHECK(mInputFrameData != NULL); diff --git a/media/libstagefright/codecs/on2/enc/SoftVPXEncoder.cpp b/media/libstagefright/codecs/on2/enc/SoftVPXEncoder.cpp index e654843..410f9d0 100644 --- a/media/libstagefright/codecs/on2/enc/SoftVPXEncoder.cpp +++ b/media/libstagefright/codecs/on2/enc/SoftVPXEncoder.cpp @@ -26,6 +26,10 @@ #include #include +#ifndef INT32_MAX +#define INT32_MAX 2147483647 +#endif + namespace android { template @@ -315,6 +319,11 @@ status_t SoftVPXEncoder::initEncoder() { if (mColorFormat != OMX_COLOR_FormatYUV420Planar || mInputDataIsMeta) { free(mConversionBuffer); + mConversionBuffer = NULL; + if (((uint64_t)mWidth * mHeight) > ((uint64_t)INT32_MAX / 3)) { + ALOGE("b/25812794, Buffer size is too big."); + return UNKNOWN_ERROR; + } mConversionBuffer = (uint8_t *)malloc(mWidth * mHeight * 3 / 2); if (mConversionBuffer == NULL) { ALOGE("Allocating conversion buffer failed."); -- cgit v1.1 From 0462975291796e414891e04bcec9da993914e458 Mon Sep 17 00:00:00 2001 From: Wei Jia Date: Fri, 20 Nov 2015 10:34:35 -0800 Subject: DO NOT MERGE - libstagefright: check requested memory size before allocation for SoftMPEG4Encoder and SoftVPXEncoder. Bug: 25812794 Change-Id: I96dc74734380d462583f6efa33d09946f9532809 (cherry picked from commit 87f8cbb223ee516803dbb99699320c2484cbf3ba) --- media/libstagefright/codecs/m4v_h263/enc/SoftMPEG4Encoder.cpp | 9 +++++++++ media/libstagefright/codecs/on2/enc/SoftVPXEncoder.cpp | 9 +++++++++ 2 files changed, 18 insertions(+) diff --git a/media/libstagefright/codecs/m4v_h263/enc/SoftMPEG4Encoder.cpp b/media/libstagefright/codecs/m4v_h263/enc/SoftMPEG4Encoder.cpp index c87d19c..d68c682 100644 --- a/media/libstagefright/codecs/m4v_h263/enc/SoftMPEG4Encoder.cpp +++ b/media/libstagefright/codecs/m4v_h263/enc/SoftMPEG4Encoder.cpp @@ -35,6 +35,10 @@ #include +#ifndef INT32_MAX +#define INT32_MAX 2147483647 +#endif + namespace android { template @@ -123,6 +127,11 @@ OMX_ERRORTYPE SoftMPEG4Encoder::initEncParams() { || mStoreMetaDataInBuffers) { // Color conversion is needed. free(mInputFrameData); + mInputFrameData = NULL; + if (((uint64_t)mVideoWidth * mVideoHeight) > ((uint64_t)INT32_MAX / 3)) { + ALOGE("b/25812794, Buffer size is too big."); + return OMX_ErrorBadParameter; + } mInputFrameData = (uint8_t *) malloc((mVideoWidth * mVideoHeight * 3 ) >> 1); CHECK(mInputFrameData != NULL); diff --git a/media/libstagefright/codecs/on2/enc/SoftVPXEncoder.cpp b/media/libstagefright/codecs/on2/enc/SoftVPXEncoder.cpp index eb621d5..43f99d4 100644 --- a/media/libstagefright/codecs/on2/enc/SoftVPXEncoder.cpp +++ b/media/libstagefright/codecs/on2/enc/SoftVPXEncoder.cpp @@ -25,6 +25,10 @@ #include #include +#ifndef INT32_MAX +#define INT32_MAX 2147483647 +#endif + namespace android { template @@ -369,6 +373,11 @@ status_t SoftVPXEncoder::initEncoder() { if (mColorFormat != OMX_COLOR_FormatYUV420Planar || mInputDataIsMeta) { free(mConversionBuffer); + mConversionBuffer = NULL; + if (((uint64_t)mWidth * mHeight) > ((uint64_t)INT32_MAX / 3)) { + ALOGE("b/25812794, Buffer size is too big."); + return UNKNOWN_ERROR; + } mConversionBuffer = (uint8_t *)malloc(mWidth * mHeight * 3 / 2); if (mConversionBuffer == NULL) { ALOGE("Allocating conversion buffer failed."); -- cgit v1.1 From 6afc659b00c3f4a83b9f5f3c744b7119b33340b4 Mon Sep 17 00:00:00 2001 From: Wei Jia Date: Fri, 20 Nov 2015 10:34:35 -0800 Subject: DO NOT MERGE - libstagefright: check requested memory size before allocation for SoftMPEG4Encoder and SoftVPXEncoder. Bug: 25812794 Change-Id: I96dc74734380d462583f6efa33d09946f9532809 (cherry picked from commit 87f8cbb223ee516803dbb99699320c2484cbf3ba) --- media/libstagefright/codecs/m4v_h263/enc/SoftMPEG4Encoder.cpp | 11 ++++++++++- media/libstagefright/codecs/on2/enc/SoftVPXEncoder.cpp | 8 ++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/media/libstagefright/codecs/m4v_h263/enc/SoftMPEG4Encoder.cpp b/media/libstagefright/codecs/m4v_h263/enc/SoftMPEG4Encoder.cpp index e02af90..9f03502 100644 --- a/media/libstagefright/codecs/m4v_h263/enc/SoftMPEG4Encoder.cpp +++ b/media/libstagefright/codecs/m4v_h263/enc/SoftMPEG4Encoder.cpp @@ -33,6 +33,10 @@ #include "SoftMPEG4Encoder.h" +#ifndef INT32_MAX +#define INT32_MAX 2147483647 +#endif + namespace android { template @@ -149,7 +153,12 @@ OMX_ERRORTYPE SoftMPEG4Encoder::initEncParams() { if (mVideoColorFormat == OMX_COLOR_FormatYUV420SemiPlanar) { // Color conversion is needed. - CHECK(mInputFrameData == NULL); + free(mInputFrameData); + mInputFrameData = NULL; + if (((uint64_t)mVideoWidth * mVideoHeight) > ((uint64_t)INT32_MAX / 3)) { + ALOGE("b/25812794, Buffer size is too big."); + return OMX_ErrorBadParameter; + } mInputFrameData = (uint8_t *) malloc((mVideoWidth * mVideoHeight * 3 ) >> 1); CHECK(mInputFrameData != NULL); diff --git a/media/libstagefright/codecs/on2/enc/SoftVPXEncoder.cpp b/media/libstagefright/codecs/on2/enc/SoftVPXEncoder.cpp index 8375cac..50eb6bf 100644 --- a/media/libstagefright/codecs/on2/enc/SoftVPXEncoder.cpp +++ b/media/libstagefright/codecs/on2/enc/SoftVPXEncoder.cpp @@ -25,6 +25,10 @@ #include #include +#ifndef INT32_MAX +#define INT32_MAX 2147483647 +#endif + namespace android { @@ -300,6 +304,10 @@ status_t SoftVPXEncoder::initEncoder() { if (mColorFormat == OMX_COLOR_FormatYUV420SemiPlanar || mInputDataIsMeta) { if (mConversionBuffer == NULL) { + if (((uint64_t)mWidth * mHeight) > ((uint64_t)INT32_MAX / 3)) { + ALOGE("b/25812794, Buffer size is too big."); + return UNKNOWN_ERROR; + } mConversionBuffer = (uint8_t *)malloc(mWidth * mHeight * 3 / 2); if (mConversionBuffer == NULL) { ALOGE("Allocating conversion buffer failed."); -- cgit v1.1 From 3d6a7149802928ecf3f58b7218b0e82699b492df Mon Sep 17 00:00:00 2001 From: Andy Hung Date: Wed, 2 Dec 2015 15:55:23 -0800 Subject: DO NOT MERGE SoundPool: add lock for findSample access from SoundPoolThread Sample decoding still occurs in SoundPoolThread without holding the SoundPool lock. Bug: 25781119 Change-Id: I11fde005aa9cf5438e0390a0d2dfe0ec1dd282e8 --- include/media/SoundPool.h | 4 +-- media/libmedia/SoundPool.cpp | 61 ++++++++++++++++++++++++++++++-------------- 2 files changed, 44 insertions(+), 21 deletions(-) diff --git a/include/media/SoundPool.h b/include/media/SoundPool.h index 2dd78cc..1b21a36 100644 --- a/include/media/SoundPool.h +++ b/include/media/SoundPool.h @@ -188,6 +188,7 @@ public: // called from SoundPoolThread void sampleLoaded(int sampleID); + sp findSample(int sampleID); // called from AudioTrack thread void done_l(SoundChannel* channel); @@ -199,8 +200,7 @@ public: private: SoundPool() {} // no default constructor bool startThreads(); - void doLoad(sp& sample); - sp findSample(int sampleID) { return mSamples.valueFor(sampleID); } + sp findSample_l(int sampleID); SoundChannel* findChannel (int channelID); SoundChannel* findNextChannel (int channelID); SoundChannel* allocateChannel_l(int priority); diff --git a/media/libmedia/SoundPool.cpp b/media/libmedia/SoundPool.cpp index 22e9fad..a2b5e30 100644 --- a/media/libmedia/SoundPool.cpp +++ b/media/libmedia/SoundPool.cpp @@ -179,6 +179,17 @@ bool SoundPool::startThreads() return mDecodeThread != NULL; } +sp SoundPool::findSample(int sampleID) +{ + Mutex::Autolock lock(&mLock); + return findSample_l(sampleID); +} + +sp SoundPool::findSample_l(int sampleID) +{ + return mSamples.valueFor(sampleID); +} + SoundChannel* SoundPool::findChannel(int channelID) { for (int i = 0; i < mMaxChannels; ++i) { @@ -202,29 +213,42 @@ SoundChannel* SoundPool::findNextChannel(int channelID) int SoundPool::load(const char* path, int priority) { ALOGV("load: path=%s, priority=%d", path, priority); - Mutex::Autolock lock(&mLock); - sp sample = new Sample(++mNextSampleID, path); - mSamples.add(sample->sampleID(), sample); - doLoad(sample); - return sample->sampleID(); + int sampleID; + { + Mutex::Autolock lock(&mLock); + sampleID = ++mNextSampleID; + sp sample = new Sample(sampleID, path); + mSamples.add(sampleID, sample); + sample->startLoad(); + } + // mDecodeThread->loadSample() must be called outside of mLock. + // mDecodeThread->loadSample() may block on mDecodeThread message queue space; + // the message queue emptying may block on SoundPool::findSample(). + // + // It theoretically possible that sample loads might decode out-of-order. + mDecodeThread->loadSample(sampleID); + return sampleID; } int SoundPool::load(int fd, int64_t offset, int64_t length, int priority) { ALOGV("load: fd=%d, offset=%lld, length=%lld, priority=%d", fd, offset, length, priority); - Mutex::Autolock lock(&mLock); - sp sample = new Sample(++mNextSampleID, fd, offset, length); - mSamples.add(sample->sampleID(), sample); - doLoad(sample); - return sample->sampleID(); -} - -void SoundPool::doLoad(sp& sample) -{ - ALOGV("doLoad: loading sample sampleID=%d", sample->sampleID()); - sample->startLoad(); - mDecodeThread->loadSample(sample->sampleID()); + int sampleID; + { + Mutex::Autolock lock(&mLock); + sampleID = ++mNextSampleID; + sp sample = new Sample(sampleID, fd, offset, length); + mSamples.add(sampleID, sample); + sample->startLoad(); + } + // mDecodeThread->loadSample() must be called outside of mLock. + // mDecodeThread->loadSample() may block on mDecodeThread message queue space; + // the message queue emptying may block on SoundPool::findSample(). + // + // It theoretically possible that sample loads might decode out-of-order. + mDecodeThread->loadSample(sampleID); + return sampleID; } bool SoundPool::unload(int sampleID) @@ -239,7 +263,6 @@ int SoundPool::play(int sampleID, float leftVolume, float rightVolume, { ALOGV("play sampleID=%d, leftVolume=%f, rightVolume=%f, priority=%d, loop=%d, rate=%f", sampleID, leftVolume, rightVolume, priority, loop, rate); - sp sample; SoundChannel* channel; int channelID; @@ -249,7 +272,7 @@ int SoundPool::play(int sampleID, float leftVolume, float rightVolume, return 0; } // is sample ready? - sample = findSample(sampleID); + sp sample(findSample_l(sampleID)); if ((sample == 0) || (sample->state() != Sample::READY)) { ALOGW(" sample %d not READY", sampleID); return 0; -- cgit v1.1 From 91a23ed95cda558a3c31e8ef34f420924f4d6d7d Mon Sep 17 00:00:00 2001 From: Chong Zhang Date: Thu, 19 Feb 2015 16:39:59 -0800 Subject: DO NOT MERGE nuplayer: do not use cached source for wvm content bug: 18730095, 25563255 Change-Id: Ibd4f54907949daae1d095fa0922050310d16698f (cherry picked from commit fc6cfd8343ae8919e85ec22efed9df626fe8854b) --- .../nuplayer/GenericSource.cpp | 55 +++++++++++++--------- 1 file changed, 32 insertions(+), 23 deletions(-) diff --git a/media/libmediaplayerservice/nuplayer/GenericSource.cpp b/media/libmediaplayerservice/nuplayer/GenericSource.cpp index cdd2670..7c38edb 100644 --- a/media/libmediaplayerservice/nuplayer/GenericSource.cpp +++ b/media/libmediaplayerservice/nuplayer/GenericSource.cpp @@ -130,23 +130,37 @@ sp NuPlayer::GenericSource::getFileFormatMeta() const { status_t NuPlayer::GenericSource::initFromDataSource() { sp extractor; + String8 mimeType; + float confidence; + sp dummy; + bool isWidevineStreaming = false; CHECK(mDataSource != NULL); if (mIsWidevine) { - String8 mimeType; - float confidence; - sp dummy; - bool success; - - success = SniffWVM(mDataSource, &mimeType, &confidence, &dummy); - if (!success - || strcasecmp( + isWidevineStreaming = SniffWVM( + mDataSource, &mimeType, &confidence, &dummy); + if (!isWidevineStreaming || + strcasecmp( mimeType.string(), MEDIA_MIMETYPE_CONTAINER_WVM)) { ALOGE("unsupported widevine mime: %s", mimeType.string()); return UNKNOWN_ERROR; } + } else if (mIsStreaming) { + if (mSniffedMIME.empty()) { + if (!mDataSource->sniff(&mimeType, &confidence, &dummy)) { + return UNKNOWN_ERROR; + } + mSniffedMIME = mimeType.string(); + } + isWidevineStreaming = !strcasecmp( + mSniffedMIME.c_str(), MEDIA_MIMETYPE_CONTAINER_WVM); + } + if (isWidevineStreaming) { + // we don't want cached source for widevine streaming. + mCachedSource.clear(); + mDataSource = mHttpSource; mWVMExtractor = new WVMExtractor(mDataSource); mWVMExtractor->setAdaptiveStreamingMode(true); if (mUIDValid) { @@ -181,14 +195,6 @@ status_t NuPlayer::GenericSource::initFromDataSource() { if (mFileMeta->findCString(kKeyMIMEType, &fileMime) && !strncasecmp(fileMime, "video/wvm", 9)) { mIsWidevine = true; - if (!mUri.empty()) { - // streaming, but the app forgot to specify widevine:// url - mWVMExtractor = static_cast(extractor.get()); - mWVMExtractor->setAdaptiveStreamingMode(true); - if (mUIDValid) { - mWVMExtractor->setUID(mUID); - } - } } } } @@ -704,10 +710,10 @@ void NuPlayer::GenericSource::sendCacheStats() { int32_t kbps = 0; status_t err = UNKNOWN_ERROR; - if (mCachedSource != NULL) { - err = mCachedSource->getEstimatedBandwidthKbps(&kbps); - } else if (mWVMExtractor != NULL) { + if (mWVMExtractor != NULL) { err = mWVMExtractor->getEstimatedBandwidthKbps(&kbps); + } else if (mCachedSource != NULL) { + err = mCachedSource->getEstimatedBandwidthKbps(&kbps); } if (err == OK) { @@ -729,7 +735,13 @@ void NuPlayer::GenericSource::onPollBuffering() { int64_t cachedDurationUs = -1ll; ssize_t cachedDataRemaining = -1; - if (mCachedSource != NULL) { + ALOGW_IF(mWVMExtractor != NULL && mCachedSource != NULL, + "WVMExtractor and NuCachedSource both present"); + + if (mWVMExtractor != NULL) { + cachedDurationUs = + mWVMExtractor->getCachedDurationUs(&finalStatus); + } else if (mCachedSource != NULL) { cachedDataRemaining = mCachedSource->approxDataRemaining(&finalStatus); @@ -745,9 +757,6 @@ void NuPlayer::GenericSource::onPollBuffering() { cachedDurationUs = cachedDataRemaining * 8000000ll / bitrate; } } - } else if (mWVMExtractor != NULL) { - cachedDurationUs - = mWVMExtractor->getCachedDurationUs(&finalStatus); } if (finalStatus != OK) { -- cgit v1.1 From 22f824feac43d5758f9a70b77f2aca840ba62c3b Mon Sep 17 00:00:00 2001 From: Jeff Tinker Date: Fri, 4 Dec 2015 16:29:16 -0800 Subject: Fix security vulnerability in ICrypto DO NOT MERGE b/25800375 Change-Id: I03c9395f7c7de4ac5813a1207452aac57aa39484 --- media/libmedia/ICrypto.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/media/libmedia/ICrypto.cpp b/media/libmedia/ICrypto.cpp index a398ff7..22f8af7 100644 --- a/media/libmedia/ICrypto.cpp +++ b/media/libmedia/ICrypto.cpp @@ -321,7 +321,9 @@ status_t BnCrypto::onTransact( if (overflow || sumSubsampleSizes != totalSize) { result = -EINVAL; - } else if (offset + totalSize > sharedBuffer->size()) { + } else if (totalSize > sharedBuffer->size()) { + result = -EINVAL; + } else if ((size_t)offset > sharedBuffer->size() - totalSize) { result = -EINVAL; } else { result = decrypt( -- cgit v1.1 From 50270d98e26fa18b20ca88216c3526667b724ba7 Mon Sep 17 00:00:00 2001 From: Wei Jia Date: Fri, 20 Nov 2015 10:34:35 -0800 Subject: DO NOT MERGE - libstagefright: check requested memory size before allocation for SoftMPEG4Encoder and SoftVPXEncoder. Bug: 25812794 Change-Id: I96dc74734380d462583f6efa33d09946f9532809 (cherry picked from commit 87f8cbb223ee516803dbb99699320c2484cbf3ba) (cherry picked from commit 0462975291796e414891e04bcec9da993914e458) --- media/libstagefright/codecs/m4v_h263/enc/SoftMPEG4Encoder.cpp | 9 +++++++++ media/libstagefright/codecs/on2/enc/SoftVPXEncoder.cpp | 9 +++++++++ 2 files changed, 18 insertions(+) diff --git a/media/libstagefright/codecs/m4v_h263/enc/SoftMPEG4Encoder.cpp b/media/libstagefright/codecs/m4v_h263/enc/SoftMPEG4Encoder.cpp index fa3486c..bd4d623 100644 --- a/media/libstagefright/codecs/m4v_h263/enc/SoftMPEG4Encoder.cpp +++ b/media/libstagefright/codecs/m4v_h263/enc/SoftMPEG4Encoder.cpp @@ -37,6 +37,10 @@ #include +#ifndef INT32_MAX +#define INT32_MAX 2147483647 +#endif + namespace android { template @@ -137,6 +141,11 @@ OMX_ERRORTYPE SoftMPEG4Encoder::initEncParams() { if (mColorFormat != OMX_COLOR_FormatYUV420Planar || mInputDataIsMeta) { // Color conversion is needed. free(mInputFrameData); + mInputFrameData = NULL; + if (((uint64_t)mWidth * mHeight) > ((uint64_t)INT32_MAX / 3)) { + ALOGE("b/25812794, Buffer size is too big."); + return OMX_ErrorBadParameter; + } mInputFrameData = (uint8_t *) malloc((mWidth * mHeight * 3 ) >> 1); CHECK(mInputFrameData != NULL); diff --git a/media/libstagefright/codecs/on2/enc/SoftVPXEncoder.cpp b/media/libstagefright/codecs/on2/enc/SoftVPXEncoder.cpp index 970acf3..ef94946 100644 --- a/media/libstagefright/codecs/on2/enc/SoftVPXEncoder.cpp +++ b/media/libstagefright/codecs/on2/enc/SoftVPXEncoder.cpp @@ -26,6 +26,10 @@ #include #include +#ifndef INT32_MAX +#define INT32_MAX 2147483647 +#endif + namespace android { template @@ -315,6 +319,11 @@ status_t SoftVPXEncoder::initEncoder() { if (mColorFormat != OMX_COLOR_FormatYUV420Planar || mInputDataIsMeta) { free(mConversionBuffer); + mConversionBuffer = NULL; + if (((uint64_t)mWidth * mHeight) > ((uint64_t)INT32_MAX / 3)) { + ALOGE("b/25812794, Buffer size is too big."); + return UNKNOWN_ERROR; + } mConversionBuffer = (uint8_t *)malloc(mWidth * mHeight * 3 / 2); if (mConversionBuffer == NULL) { ALOGE("Allocating conversion buffer failed."); -- cgit v1.1 From 9e29523b9537983b4c4b205ff868d0b3bca0383b Mon Sep 17 00:00:00 2001 From: Eric Laurent Date: Fri, 8 Jan 2016 10:52:38 -0800 Subject: fix possible overflow in effect wrappers. Add checks on parameter size field in effect command handlers to avoid overflow leading to invalid comparison with min allowed size for command and reply buffers. Bug: 26347509. Change-Id: I20e6a9b6de8e5172b957caa1ac9410b9752efa4d (cherry picked from commit ad1bd92a49d78df6bc6e75bee68c517c1326f3cf) --- media/libeffects/lvm/wrapper/Bundle/EffectBundle.cpp | 5 ++++- media/libeffects/lvm/wrapper/Reverb/EffectReverb.cpp | 5 ++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/media/libeffects/lvm/wrapper/Bundle/EffectBundle.cpp b/media/libeffects/lvm/wrapper/Bundle/EffectBundle.cpp index 9fcfba3..5befff8 100644 --- a/media/libeffects/lvm/wrapper/Bundle/EffectBundle.cpp +++ b/media/libeffects/lvm/wrapper/Bundle/EffectBundle.cpp @@ -2809,7 +2809,10 @@ int Effect_command(effect_handle_t self, //ALOGV("\tEffect_command cmdCode Case: EFFECT_CMD_GET_PARAM start"); effect_param_t *p = (effect_param_t *)pCmdData; - + if (SIZE_MAX - sizeof(effect_param_t) < (size_t)p->psize) { + android_errorWriteLog(0x534e4554, "26347509"); + return -EINVAL; + } if (pCmdData == NULL || cmdSize < sizeof(effect_param_t) || cmdSize < (sizeof(effect_param_t) + p->psize) || pReplyData == NULL || replySize == NULL || diff --git a/media/libeffects/lvm/wrapper/Reverb/EffectReverb.cpp b/media/libeffects/lvm/wrapper/Reverb/EffectReverb.cpp index 2e22532..7ab16a1 100644 --- a/media/libeffects/lvm/wrapper/Reverb/EffectReverb.cpp +++ b/media/libeffects/lvm/wrapper/Reverb/EffectReverb.cpp @@ -1953,7 +1953,10 @@ int Reverb_command(effect_handle_t self, //ALOGV("\tReverb_command cmdCode Case: " // "EFFECT_CMD_GET_PARAM start"); effect_param_t *p = (effect_param_t *)pCmdData; - + if (SIZE_MAX - sizeof(effect_param_t) < (size_t)p->psize) { + android_errorWriteLog(0x534e4554, "26347509"); + return -EINVAL; + } if (pCmdData == NULL || cmdSize < sizeof(effect_param_t) || cmdSize < (sizeof(effect_param_t) + p->psize) || pReplyData == NULL || replySize == NULL || -- cgit v1.1 From 9cebd7cfba272117522617661cf9d4985880921e Mon Sep 17 00:00:00 2001 From: Eric Laurent Date: Tue, 12 Jan 2016 18:03:24 +0000 Subject: DO NOT MERGE ANYWHERE Revert "fix possible overflow in effect wrappers." This reverts commit 9e29523b9537983b4c4b205ff868d0b3bca0383b. Change-Id: Ic9a97d1a98165500dd444b97629349cf082ced94 --- media/libeffects/lvm/wrapper/Bundle/EffectBundle.cpp | 5 +---- media/libeffects/lvm/wrapper/Reverb/EffectReverb.cpp | 5 +---- 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/media/libeffects/lvm/wrapper/Bundle/EffectBundle.cpp b/media/libeffects/lvm/wrapper/Bundle/EffectBundle.cpp index 86ce27a..40c7fef 100644 --- a/media/libeffects/lvm/wrapper/Bundle/EffectBundle.cpp +++ b/media/libeffects/lvm/wrapper/Bundle/EffectBundle.cpp @@ -3053,10 +3053,7 @@ int Effect_command(effect_handle_t self, //ALOGV("\tEffect_command cmdCode Case: EFFECT_CMD_GET_PARAM start"); effect_param_t *p = (effect_param_t *)pCmdData; - if (SIZE_MAX - sizeof(effect_param_t) < (size_t)p->psize) { - android_errorWriteLog(0x534e4554, "26347509"); - return -EINVAL; - } + if (pCmdData == NULL || cmdSize < sizeof(effect_param_t) || cmdSize < (sizeof(effect_param_t) + p->psize) || pReplyData == NULL || replySize == NULL || diff --git a/media/libeffects/lvm/wrapper/Reverb/EffectReverb.cpp b/media/libeffects/lvm/wrapper/Reverb/EffectReverb.cpp index 4dc8b45..a48a4e3 100644 --- a/media/libeffects/lvm/wrapper/Reverb/EffectReverb.cpp +++ b/media/libeffects/lvm/wrapper/Reverb/EffectReverb.cpp @@ -1956,10 +1956,7 @@ int Reverb_command(effect_handle_t self, //ALOGV("\tReverb_command cmdCode Case: " // "EFFECT_CMD_GET_PARAM start"); effect_param_t *p = (effect_param_t *)pCmdData; - if (SIZE_MAX - sizeof(effect_param_t) < (size_t)p->psize) { - android_errorWriteLog(0x534e4554, "26347509"); - return -EINVAL; - } + if (pCmdData == NULL || cmdSize < sizeof(effect_param_t) || cmdSize < (sizeof(effect_param_t) + p->psize) || pReplyData == NULL || replySize == NULL || -- cgit v1.1 From 113efbb3ac8e7306ea8645cb53a96d1e81c2d041 Mon Sep 17 00:00:00 2001 From: Eric Laurent Date: Fri, 8 Jan 2016 17:16:42 -0800 Subject: audioflinger: fix standby delay on A2DP output Make sure that standby delay is never less than the audio flinger default on A2DP output. Due to variable latency and amount of buffering in A2DP sinks, an agressive standby delay could lead to truncated audio. Bug: 25830539. Change-Id: I38be37ad346f5f4bf8303d3db4e3e911bf637968 (cherry picked from commit 42537be61479e59c4718e1304364551c1454f63c) --- services/audioflinger/Threads.cpp | 23 ++++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/services/audioflinger/Threads.cpp b/services/audioflinger/Threads.cpp index 71fc498..7d2d550 100644 --- a/services/audioflinger/Threads.cpp +++ b/services/audioflinger/Threads.cpp @@ -1589,6 +1589,7 @@ void AudioFlinger::PlaybackThread::dumpInternals(int fd, const Vector& dprintf(fd, " Mixer buffer: %p\n", mMixerBuffer); dprintf(fd, " Effect buffer: %p\n", mEffectBuffer); dprintf(fd, " Fast track availMask=%#x\n", mFastTrackAvailMask); + dprintf(fd, " Standby delay ns=%lld\n", (long long)mStandbyDelayNs); AudioStreamOut *output = mOutput; audio_output_flags_t flags = output != NULL ? output->flags : AUDIO_OUTPUT_FLAG_NONE; String8 flagsAsString = outputFlagsToString(flags); @@ -2513,7 +2514,8 @@ The derived values that are cached: - mSinkBufferSize from frame count * frame size - mActiveSleepTimeUs from activeSleepTimeUs() - mIdleSleepTimeUs from idleSleepTimeUs() - - mStandbyDelayNs from mActiveSleepTimeUs (DIRECT only) + - mStandbyDelayNs from mActiveSleepTimeUs (DIRECT only) or forced to at least + kDefaultStandbyTimeInNsecs when connected to an A2DP device. - maxPeriod from frame count and sample rate (MIXER only) The parameters that affect these derived values are: @@ -2532,6 +2534,15 @@ void AudioFlinger::PlaybackThread::cacheParameters_l() mSinkBufferSize = mNormalFrameCount * mFrameSize; mActiveSleepTimeUs = activeSleepTimeUs(); mIdleSleepTimeUs = idleSleepTimeUs(); + + // make sure standby delay is not too short when connected to an A2DP sink to avoid + // truncating audio when going to standby. + mStandbyDelayNs = AudioFlinger::mStandbyTimeInNsecs; + if ((mOutDevice & AUDIO_DEVICE_OUT_ALL_A2DP) != 0) { + if (mStandbyDelayNs < kDefaultStandbyTimeInNsecs) { + mStandbyDelayNs = kDefaultStandbyTimeInNsecs; + } + } } void AudioFlinger::PlaybackThread::invalidateTracks(audio_stream_type_t streamType) @@ -4248,6 +4259,7 @@ bool AudioFlinger::MixerThread::checkForNewParameter_l(const String8& keyValuePa status_t& status) { bool reconfig = false; + bool a2dpDeviceChanged = false; status = NO_ERROR; @@ -4324,6 +4336,8 @@ bool AudioFlinger::MixerThread::checkForNewParameter_l(const String8& keyValuePa // forward device change to effects that have requested to be // aware of attached audio device. if (value != AUDIO_DEVICE_NONE) { + a2dpDeviceChanged = + (mOutDevice & AUDIO_DEVICE_OUT_ALL_A2DP) != (value & AUDIO_DEVICE_OUT_ALL_A2DP); mOutDevice = value; for (size_t i = 0; i < mEffectChains.size(); i++) { mEffectChains[i]->setDevice_l(mOutDevice); @@ -4367,7 +4381,7 @@ bool AudioFlinger::MixerThread::checkForNewParameter_l(const String8& keyValuePa sq->push(FastMixerStateQueue::BLOCK_UNTIL_PUSHED); } - return reconfig; + return reconfig || a2dpDeviceChanged; } @@ -4803,6 +4817,7 @@ bool AudioFlinger::DirectOutputThread::checkForNewParameter_l(const String8& key status_t& status) { bool reconfig = false; + bool a2dpDeviceChanged = false; status = NO_ERROR; @@ -4812,6 +4827,8 @@ bool AudioFlinger::DirectOutputThread::checkForNewParameter_l(const String8& key // forward device change to effects that have requested to be // aware of attached audio device. if (value != AUDIO_DEVICE_NONE) { + a2dpDeviceChanged = + (mOutDevice & AUDIO_DEVICE_OUT_ALL_A2DP) != (value & AUDIO_DEVICE_OUT_ALL_A2DP); mOutDevice = value; for (size_t i = 0; i < mEffectChains.size(); i++) { mEffectChains[i]->setDevice_l(mOutDevice); @@ -4844,7 +4861,7 @@ bool AudioFlinger::DirectOutputThread::checkForNewParameter_l(const String8& key } } - return reconfig; + return reconfig || a2dpDeviceChanged; } uint32_t AudioFlinger::DirectOutputThread::activeSleepTimeUs() const -- cgit v1.1 From 5403587a74aee2fb57076528c3927851531c8afb Mon Sep 17 00:00:00 2001 From: Marco Nelissen Date: Tue, 12 Jan 2016 12:37:36 -0800 Subject: Fix out-of-bounds write Bug: 26365349 Change-Id: Ia363d9f8c231cf255dea852e0bbf5ca466c7990b --- media/libstagefright/MPEG4Extractor.cpp | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/media/libstagefright/MPEG4Extractor.cpp b/media/libstagefright/MPEG4Extractor.cpp index f0988eb..5d8be84 100644 --- a/media/libstagefright/MPEG4Extractor.cpp +++ b/media/libstagefright/MPEG4Extractor.cpp @@ -3682,7 +3682,15 @@ status_t MPEG4Source::fragmentedRead( continue; } - CHECK(dstOffset + 4 <= mBuffer->size()); + if (dstOffset > SIZE_MAX - 4 || + dstOffset + 4 > SIZE_MAX - nalLength || + dstOffset + 4 + nalLength > mBuffer->size()) { + ALOGE("b/26365349 : %zu %zu", dstOffset, mBuffer->size()); + android_errorWriteLog(0x534e4554, "26365349"); + mBuffer->release(); + mBuffer = NULL; + return ERROR_MALFORMED; + } dstData[dstOffset++] = 0; dstData[dstOffset++] = 0; -- cgit v1.1 From 6dd6c546513aa18dc1d7fba0f72d670edce34f77 Mon Sep 17 00:00:00 2001 From: Yin-Chia Yeh Date: Mon, 7 Dec 2015 12:20:11 -0800 Subject: Camera: set mNumberOfNormalCameras correctly Need to set the number correctly when a camera HAL is not present. Bug: 25951590 Change-Id: I666acf7a2a523c51f2c2ae88ff690ca9dccda08c --- services/camera/libcameraservice/CameraService.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/services/camera/libcameraservice/CameraService.cpp b/services/camera/libcameraservice/CameraService.cpp index 3deb396..897fa1b 100644 --- a/services/camera/libcameraservice/CameraService.cpp +++ b/services/camera/libcameraservice/CameraService.cpp @@ -153,6 +153,7 @@ void CameraService::onFirstRef() ALOGE("Could not load camera HAL module: %d (%s)", err, strerror(-err)); logServiceError("Could not load camera HAL module", err); mNumberOfCameras = 0; + mNumberOfNormalCameras = 0; return; } -- cgit v1.1 From c9ab2b0bb05a7e19fb057e79b36e232809d70122 Mon Sep 17 00:00:00 2001 From: Eino-Ville Talvala Date: Wed, 13 Jan 2016 10:07:04 -0800 Subject: Camera: Disallow dumping clients directly Camera service dumps should only be initiated through ICameraService::dump. Bug: 26265403 Change-Id: If3ca4718ed74bf33ad8a416192689203029e2803 --- services/camera/libcameraservice/CameraService.cpp | 10 +++++++++- services/camera/libcameraservice/CameraService.h | 8 +++++++- services/camera/libcameraservice/api1/Camera2Client.cpp | 4 ++++ services/camera/libcameraservice/api1/Camera2Client.h | 2 ++ services/camera/libcameraservice/api1/CameraClient.cpp | 4 ++++ services/camera/libcameraservice/api1/CameraClient.h | 4 +++- services/camera/libcameraservice/api2/CameraDeviceClient.cpp | 4 ++++ services/camera/libcameraservice/api2/CameraDeviceClient.h | 2 ++ services/camera/libcameraservice/api_pro/ProCamera2Client.cpp | 4 ++++ services/camera/libcameraservice/api_pro/ProCamera2Client.h | 2 ++ services/camera/libcameraservice/common/Camera2ClientBase.cpp | 2 +- services/camera/libcameraservice/common/Camera2ClientBase.h | 2 +- 12 files changed, 43 insertions(+), 5 deletions(-) diff --git a/services/camera/libcameraservice/CameraService.cpp b/services/camera/libcameraservice/CameraService.cpp index 06c1626..4e3bb19 100644 --- a/services/camera/libcameraservice/CameraService.cpp +++ b/services/camera/libcameraservice/CameraService.cpp @@ -985,6 +985,14 @@ void CameraService::BasicClient::disconnect() { mClientPid = 0; } +status_t CameraService::BasicClient::dump(int, const Vector&) { + // No dumping of clients directly over Binder, + // must go through CameraService::dump + android_errorWriteWithInfoLog(SN_EVENT_LOG_ID, "26265403", + IPCThreadState::self()->getCallingUid(), NULL, 0); + return OK; +} + status_t CameraService::BasicClient::startCameraOps() { int32_t res; @@ -1222,7 +1230,7 @@ status_t CameraService::dump(int fd, const Vector& args) { hasClient = true; result = String8::format(" Device is open. Client instance dump:\n"); write(fd, result.string(), result.size()); - client->dump(fd, args); + client->dumpClient(fd, args); } if (!hasClient) { result = String8::format("\nNo active camera clients yet.\n"); diff --git a/services/camera/libcameraservice/CameraService.h b/services/camera/libcameraservice/CameraService.h index ad6a582..7f3691a 100644 --- a/services/camera/libcameraservice/CameraService.h +++ b/services/camera/libcameraservice/CameraService.h @@ -55,6 +55,9 @@ public: class Client; class BasicClient; + // Event log ID + static const int SN_EVENT_LOG_ID = 0x534e4554; + // Implementation of BinderService static char const* getServiceName() { return "media.camera"; } @@ -144,7 +147,10 @@ public: return mRemoteBinder; } - virtual status_t dump(int fd, const Vector& args) = 0; + // Disallows dumping over binder interface + virtual status_t dump(int fd, const Vector& args); + // Internal dump method to be called by CameraService + virtual status_t dumpClient(int fd, const Vector& args) = 0; protected: BasicClient(const sp& cameraService, diff --git a/services/camera/libcameraservice/api1/Camera2Client.cpp b/services/camera/libcameraservice/api1/Camera2Client.cpp index b093946..131048e 100644 --- a/services/camera/libcameraservice/api1/Camera2Client.cpp +++ b/services/camera/libcameraservice/api1/Camera2Client.cpp @@ -158,6 +158,10 @@ Camera2Client::~Camera2Client() { } status_t Camera2Client::dump(int fd, const Vector& args) { + return BasicClient::dump(fd, args); +} + +status_t Camera2Client::dumpClient(int fd, const Vector& args) { String8 result; result.appendFormat("Client2[%d] (%p) Client: %s PID: %d, dump:\n", mCameraId, diff --git a/services/camera/libcameraservice/api1/Camera2Client.h b/services/camera/libcameraservice/api1/Camera2Client.h index fe0bf74..dfcf836 100644 --- a/services/camera/libcameraservice/api1/Camera2Client.h +++ b/services/camera/libcameraservice/api1/Camera2Client.h @@ -98,6 +98,8 @@ public: virtual status_t dump(int fd, const Vector& args); + virtual status_t dumpClient(int fd, const Vector& args); + /** * Interface used by CameraDeviceBase */ diff --git a/services/camera/libcameraservice/api1/CameraClient.cpp b/services/camera/libcameraservice/api1/CameraClient.cpp index bd6805d..2e3a2ed 100644 --- a/services/camera/libcameraservice/api1/CameraClient.cpp +++ b/services/camera/libcameraservice/api1/CameraClient.cpp @@ -112,6 +112,10 @@ CameraClient::~CameraClient() { } status_t CameraClient::dump(int fd, const Vector& args) { + return BasicClient::dump(fd, args); +} + +status_t CameraClient::dumpClient(int fd, const Vector& args) { const size_t SIZE = 256; char buffer[SIZE]; diff --git a/services/camera/libcameraservice/api1/CameraClient.h b/services/camera/libcameraservice/api1/CameraClient.h index 4b89564..a270f90 100644 --- a/services/camera/libcameraservice/api1/CameraClient.h +++ b/services/camera/libcameraservice/api1/CameraClient.h @@ -69,7 +69,9 @@ public: status_t initialize(camera_module_t *module); - status_t dump(int fd, const Vector& args); + virtual status_t dump(int fd, const Vector& args); + + virtual status_t dumpClient(int fd, const Vector& args); private: diff --git a/services/camera/libcameraservice/api2/CameraDeviceClient.cpp b/services/camera/libcameraservice/api2/CameraDeviceClient.cpp index 1cdf8dc..ab7057f 100644 --- a/services/camera/libcameraservice/api2/CameraDeviceClient.cpp +++ b/services/camera/libcameraservice/api2/CameraDeviceClient.cpp @@ -480,6 +480,10 @@ status_t CameraDeviceClient::flush() { } status_t CameraDeviceClient::dump(int fd, const Vector& args) { + return BasicClient::dump(fd, args); +} + +status_t CameraDeviceClient::dumpClient(int fd, const Vector& args) { String8 result; result.appendFormat("CameraDeviceClient[%d] (%p) PID: %d, dump:\n", mCameraId, diff --git a/services/camera/libcameraservice/api2/CameraDeviceClient.h b/services/camera/libcameraservice/api2/CameraDeviceClient.h index b9c16aa..99df386 100644 --- a/services/camera/libcameraservice/api2/CameraDeviceClient.h +++ b/services/camera/libcameraservice/api2/CameraDeviceClient.h @@ -109,6 +109,8 @@ public: virtual status_t dump(int fd, const Vector& args); + virtual status_t dumpClient(int fd, const Vector& args); + /** * Device listener interface */ diff --git a/services/camera/libcameraservice/api_pro/ProCamera2Client.cpp b/services/camera/libcameraservice/api_pro/ProCamera2Client.cpp index 1a7a7a7..27156b1 100644 --- a/services/camera/libcameraservice/api_pro/ProCamera2Client.cpp +++ b/services/camera/libcameraservice/api_pro/ProCamera2Client.cpp @@ -331,6 +331,10 @@ status_t ProCamera2Client::getCameraInfo(int cameraId, } status_t ProCamera2Client::dump(int fd, const Vector& args) { + return BasicClient::dump(fd, args); +} + +status_t ProCamera2Client::dumpClient(int fd, const Vector& args) { String8 result; result.appendFormat("ProCamera2Client[%d] (%p) PID: %d, dump:\n", mCameraId, diff --git a/services/camera/libcameraservice/api_pro/ProCamera2Client.h b/services/camera/libcameraservice/api_pro/ProCamera2Client.h index 8a0f547..137c272 100644 --- a/services/camera/libcameraservice/api_pro/ProCamera2Client.h +++ b/services/camera/libcameraservice/api_pro/ProCamera2Client.h @@ -88,6 +88,8 @@ public: virtual status_t dump(int fd, const Vector& args); + virtual status_t dumpClient(int fd, const Vector& args); + // Callbacks from camera service virtual void onExclusiveLockStolen(); diff --git a/services/camera/libcameraservice/common/Camera2ClientBase.cpp b/services/camera/libcameraservice/common/Camera2ClientBase.cpp index 2d1253f..b6ad042 100644 --- a/services/camera/libcameraservice/common/Camera2ClientBase.cpp +++ b/services/camera/libcameraservice/common/Camera2ClientBase.cpp @@ -117,7 +117,7 @@ Camera2ClientBase::~Camera2ClientBase() { } template -status_t Camera2ClientBase::dump(int fd, +status_t Camera2ClientBase::dumpClient(int fd, const Vector& args) { String8 result; result.appendFormat("Camera2ClientBase[%d] (%p) PID: %d, dump:\n", diff --git a/services/camera/libcameraservice/common/Camera2ClientBase.h b/services/camera/libcameraservice/common/Camera2ClientBase.h index 61e44f0..c12793a 100644 --- a/services/camera/libcameraservice/common/Camera2ClientBase.h +++ b/services/camera/libcameraservice/common/Camera2ClientBase.h @@ -55,7 +55,7 @@ public: virtual ~Camera2ClientBase(); virtual status_t initialize(camera_module_t *module); - virtual status_t dump(int fd, const Vector& args); + virtual status_t dumpClient(int fd, const Vector& args); /** * CameraDeviceBase::NotificationListener implementation -- cgit v1.1 From c4003965258404a19b99280ac0f475e2f290bf27 Mon Sep 17 00:00:00 2001 From: Eino-Ville Talvala Date: Wed, 13 Jan 2016 10:07:04 -0800 Subject: Camera: Disallow dumping clients directly Camera service dumps should only be initiated through ICameraService::dump. Bug: 26265403 Change-Id: If3ca4718ed74bf33ad8a416192689203029e2803 --- services/camera/libcameraservice/CameraService.cpp | 10 +++++++++- services/camera/libcameraservice/CameraService.h | 8 +++++++- services/camera/libcameraservice/api1/Camera2Client.cpp | 4 ++++ services/camera/libcameraservice/api1/Camera2Client.h | 2 ++ services/camera/libcameraservice/api1/CameraClient.cpp | 4 ++++ services/camera/libcameraservice/api1/CameraClient.h | 4 +++- services/camera/libcameraservice/api2/CameraDeviceClient.cpp | 5 ++++- services/camera/libcameraservice/api2/CameraDeviceClient.h | 2 ++ services/camera/libcameraservice/common/Camera2ClientBase.cpp | 2 +- services/camera/libcameraservice/common/Camera2ClientBase.h | 2 +- 10 files changed, 37 insertions(+), 6 deletions(-) diff --git a/services/camera/libcameraservice/CameraService.cpp b/services/camera/libcameraservice/CameraService.cpp index 9a1101a..8391f26 100644 --- a/services/camera/libcameraservice/CameraService.cpp +++ b/services/camera/libcameraservice/CameraService.cpp @@ -1896,6 +1896,14 @@ void CameraService::BasicClient::disconnect() { mClientPid = 0; } +status_t CameraService::BasicClient::dump(int, const Vector&) { + // No dumping of clients directly over Binder, + // must go through CameraService::dump + android_errorWriteWithInfoLog(SN_EVENT_LOG_ID, "26265403", + IPCThreadState::self()->getCallingUid(), NULL, 0); + return OK; +} + String16 CameraService::BasicClient::getPackageName() const { return mClientPackageName; } @@ -2328,7 +2336,7 @@ status_t CameraService::dump(int fd, const Vector& args) { String8(client->getPackageName()).string()); write(fd, result.string(), result.size()); - client->dump(fd, args); + client->dumpClient(fd, args); } if (stateLocked) mCameraStatesLock.unlock(); diff --git a/services/camera/libcameraservice/CameraService.h b/services/camera/libcameraservice/CameraService.h index b56c161..3905d62 100644 --- a/services/camera/libcameraservice/CameraService.h +++ b/services/camera/libcameraservice/CameraService.h @@ -84,6 +84,9 @@ public: // Default number of messages to store in eviction log static const size_t DEFAULT_EVENT_LOG_LENGTH = 100; + // Event log ID + static const int SN_EVENT_LOG_ID = 0x534e4554; + // Implementation of BinderService static char const* getServiceName() { return "media.camera"; } @@ -189,7 +192,10 @@ public: return mRemoteBinder; } - virtual status_t dump(int fd, const Vector& args) = 0; + // Disallows dumping over binder interface + virtual status_t dump(int fd, const Vector& args); + // Internal dump method to be called by CameraService + virtual status_t dumpClient(int fd, const Vector& args) = 0; // Return the package name for this client virtual String16 getPackageName() const; diff --git a/services/camera/libcameraservice/api1/Camera2Client.cpp b/services/camera/libcameraservice/api1/Camera2Client.cpp index 36e99dd..1695309 100644 --- a/services/camera/libcameraservice/api1/Camera2Client.cpp +++ b/services/camera/libcameraservice/api1/Camera2Client.cpp @@ -163,6 +163,10 @@ Camera2Client::~Camera2Client() { } status_t Camera2Client::dump(int fd, const Vector& args) { + return BasicClient::dump(fd, args); +} + +status_t Camera2Client::dumpClient(int fd, const Vector& args) { String8 result; result.appendFormat("Client2[%d] (%p) PID: %d, dump:\n", mCameraId, (getRemoteCallback() != NULL ? diff --git a/services/camera/libcameraservice/api1/Camera2Client.h b/services/camera/libcameraservice/api1/Camera2Client.h index d50bf63..7e7a284 100644 --- a/services/camera/libcameraservice/api1/Camera2Client.h +++ b/services/camera/libcameraservice/api1/Camera2Client.h @@ -100,6 +100,8 @@ public: virtual status_t dump(int fd, const Vector& args); + virtual status_t dumpClient(int fd, const Vector& args); + /** * Interface used by CameraDeviceBase */ diff --git a/services/camera/libcameraservice/api1/CameraClient.cpp b/services/camera/libcameraservice/api1/CameraClient.cpp index e552633..9e6ed4e 100644 --- a/services/camera/libcameraservice/api1/CameraClient.cpp +++ b/services/camera/libcameraservice/api1/CameraClient.cpp @@ -108,6 +108,10 @@ CameraClient::~CameraClient() { } status_t CameraClient::dump(int fd, const Vector& args) { + return BasicClient::dump(fd, args); +} + +status_t CameraClient::dumpClient(int fd, const Vector& args) { const size_t SIZE = 256; char buffer[SIZE]; diff --git a/services/camera/libcameraservice/api1/CameraClient.h b/services/camera/libcameraservice/api1/CameraClient.h index 95616b2..17999a5 100644 --- a/services/camera/libcameraservice/api1/CameraClient.h +++ b/services/camera/libcameraservice/api1/CameraClient.h @@ -70,7 +70,9 @@ public: status_t initialize(CameraModule *module); - status_t dump(int fd, const Vector& args); + virtual status_t dump(int fd, const Vector& args); + + virtual status_t dumpClient(int fd, const Vector& args); private: diff --git a/services/camera/libcameraservice/api2/CameraDeviceClient.cpp b/services/camera/libcameraservice/api2/CameraDeviceClient.cpp index c717a56..84c0c3e 100644 --- a/services/camera/libcameraservice/api2/CameraDeviceClient.cpp +++ b/services/camera/libcameraservice/api2/CameraDeviceClient.cpp @@ -750,8 +750,11 @@ status_t CameraDeviceClient::tearDown(int streamId) { return res; } - status_t CameraDeviceClient::dump(int fd, const Vector& args) { + return BasicClient::dump(fd, args); +} + +status_t CameraDeviceClient::dumpClient(int fd, const Vector& args) { String8 result; result.appendFormat("CameraDeviceClient[%d] (%p) dump:\n", mCameraId, diff --git a/services/camera/libcameraservice/api2/CameraDeviceClient.h b/services/camera/libcameraservice/api2/CameraDeviceClient.h index 1f8b39d..486e68b 100644 --- a/services/camera/libcameraservice/api2/CameraDeviceClient.h +++ b/services/camera/libcameraservice/api2/CameraDeviceClient.h @@ -132,6 +132,8 @@ public: virtual status_t dump(int fd, const Vector& args); + virtual status_t dumpClient(int fd, const Vector& args); + /** * Device listener interface */ diff --git a/services/camera/libcameraservice/common/Camera2ClientBase.cpp b/services/camera/libcameraservice/common/Camera2ClientBase.cpp index ba0b264..fdb801e 100644 --- a/services/camera/libcameraservice/common/Camera2ClientBase.cpp +++ b/services/camera/libcameraservice/common/Camera2ClientBase.cpp @@ -123,7 +123,7 @@ Camera2ClientBase::~Camera2ClientBase() { } template -status_t Camera2ClientBase::dump(int fd, +status_t Camera2ClientBase::dumpClient(int fd, const Vector& args) { String8 result; result.appendFormat("Camera2ClientBase[%d] (%p) PID: %d, dump:\n", diff --git a/services/camera/libcameraservice/common/Camera2ClientBase.h b/services/camera/libcameraservice/common/Camera2ClientBase.h index f1cacdf..d66e11c 100644 --- a/services/camera/libcameraservice/common/Camera2ClientBase.h +++ b/services/camera/libcameraservice/common/Camera2ClientBase.h @@ -57,7 +57,7 @@ public: virtual ~Camera2ClientBase(); virtual status_t initialize(CameraModule *module); - virtual status_t dump(int fd, const Vector& args); + virtual status_t dumpClient(int fd, const Vector& args); /** * CameraDeviceBase::NotificationListener implementation -- cgit v1.1