From bdd368796e7773f0fefb9e238dd16c9242180db5 Mon Sep 17 00:00:00 2001 From: Ruben Brunk Date: Mon, 3 Aug 2015 11:36:40 -0700 Subject: camera2: Add bad pixel opcode to img_utils. Bug: 22463079 Change-Id: Ied76bbdcf3f706cea6e249748c6bfd4092ff6d39 --- media/img_utils/include/img_utils/DngUtils.h | 31 ++++++++++ media/img_utils/src/DngUtils.cpp | 85 +++++++++++++++++++++++----- 2 files changed, 102 insertions(+), 14 deletions(-) (limited to 'media') diff --git a/media/img_utils/include/img_utils/DngUtils.h b/media/img_utils/include/img_utils/DngUtils.h index 3dcedc5..1d8df9c 100644 --- a/media/img_utils/include/img_utils/DngUtils.h +++ b/media/img_utils/include/img_utils/DngUtils.h @@ -138,6 +138,34 @@ class ANDROID_API OpcodeListBuilder : public LightRefBase { double opticalCenterY, const double* kCoeffs); + + /** + * Add FixBadPixelsList opcode for the given metadata parameters. + * + * Returns OK on success, or a negative error code. + */ + virtual status_t addBadPixelListForMetadata(const uint32_t* hotPixels, + uint32_t xyPairCount, + uint32_t colorFilterArrangement); + + /** + * Add FixBadPixelsList opcode. + * + * bayerPhase - 0=top-left of image is red, 1=top-left of image is green pixel in red row, + * 2=top-left of image is green pixel in blue row, 3=top-left of image is + * blue. + * badPointCount - number of (x,y) pairs of bad pixels are given in badPointRowColPairs. + * badRectCount - number of (top, left, bottom, right) tuples are given in + * badRectTopLeftBottomRightTuples + * + * Returns OK on success, or a negative error code. + */ + virtual status_t addBadPixelList(uint32_t bayerPhase, + uint32_t badPointCount, + uint32_t badRectCount, + const uint32_t* badPointRowColPairs, + const uint32_t* badRectTopLeftBottomRightTuples); + // TODO: Add other Opcode methods protected: static const uint32_t FLAG_OPTIONAL = 0x1u; @@ -146,6 +174,7 @@ class ANDROID_API OpcodeListBuilder : public LightRefBase { // Opcode IDs enum { WARP_RECTILINEAR_ID = 1, + FIX_BAD_PIXELS_LIST = 5, GAIN_MAP_ID = 9, }; @@ -161,6 +190,8 @@ class ANDROID_API OpcodeListBuilder : public LightRefBase { ByteArrayOutput mOpList; EndianOutput mEndianOut; + status_t addOpcodePreamble(uint32_t opcodeId); + }; } /*namespace img_utils*/ diff --git a/media/img_utils/src/DngUtils.cpp b/media/img_utils/src/DngUtils.cpp index b213403..9473dce 100644 --- a/media/img_utils/src/DngUtils.cpp +++ b/media/img_utils/src/DngUtils.cpp @@ -224,13 +224,7 @@ status_t OpcodeListBuilder::addGainMap(uint32_t top, uint32_t mapPlanes, const float* mapGains) { - uint32_t opcodeId = GAIN_MAP_ID; - - status_t err = mEndianOut.write(&opcodeId, 0, 1); - if (err != OK) return err; - - uint8_t version[] = {1, 3, 0, 0}; - err = mEndianOut.write(version, 0, NELEMS(version)); + status_t err = addOpcodePreamble(GAIN_MAP_ID); if (err != OK) return err; // Allow this opcode to be skipped if not supported @@ -334,13 +328,7 @@ status_t OpcodeListBuilder::addWarpRectilinear(uint32_t numPlanes, double opticalCenterY, const double* kCoeffs) { - uint32_t opcodeId = WARP_RECTILINEAR_ID; - - status_t err = mEndianOut.write(&opcodeId, 0, 1); - if (err != OK) return err; - - uint8_t version[] = {1, 3, 0, 0}; - err = mEndianOut.write(version, 0, NELEMS(version)); + status_t err = addOpcodePreamble(WARP_RECTILINEAR_ID); if (err != OK) return err; // Allow this opcode to be skipped if not supported @@ -373,5 +361,74 @@ status_t OpcodeListBuilder::addWarpRectilinear(uint32_t numPlanes, return OK; } +status_t OpcodeListBuilder::addBadPixelListForMetadata(const uint32_t* hotPixels, + uint32_t xyPairCount, + uint32_t colorFilterArrangement) { + if (colorFilterArrangement > 3) { + ALOGE("%s: Unknown color filter arrangement %" PRIu32, __FUNCTION__, + colorFilterArrangement); + return BAD_VALUE; + } + + return addBadPixelList(colorFilterArrangement, xyPairCount, 0, hotPixels, nullptr); +} + +status_t OpcodeListBuilder::addBadPixelList(uint32_t bayerPhase, + uint32_t badPointCount, + uint32_t badRectCount, + const uint32_t* badPointRowColPairs, + const uint32_t* badRectTopLeftBottomRightTuples) { + + status_t err = addOpcodePreamble(FIX_BAD_PIXELS_LIST); + if (err != OK) return err; + + // Allow this opcode to be skipped if not supported + uint32_t flags = FLAG_OPTIONAL; + + err = mEndianOut.write(&flags, 0, 1); + if (err != OK) return err; + + const uint32_t NUM_NON_VARLEN_FIELDS = 3; + const uint32_t SIZE_OF_POINT = 2; + const uint32_t SIZE_OF_RECT = 4; + + uint32_t totalSize = (NUM_NON_VARLEN_FIELDS + badPointCount * SIZE_OF_POINT + + badRectCount * SIZE_OF_RECT) * sizeof(uint32_t); + err = mEndianOut.write(&totalSize, 0, 1); + if (err != OK) return err; + + err = mEndianOut.write(&bayerPhase, 0, 1); + if (err != OK) return err; + + err = mEndianOut.write(&badPointCount, 0, 1); + if (err != OK) return err; + + err = mEndianOut.write(&badRectCount, 0, 1); + if (err != OK) return err; + + if (badPointCount > 0) { + err = mEndianOut.write(badPointRowColPairs, 0, SIZE_OF_POINT * badPointCount); + if (err != OK) return err; + } + + if (badRectCount > 0) { + err = mEndianOut.write(badRectTopLeftBottomRightTuples, 0, SIZE_OF_RECT * badRectCount); + if (err != OK) return err; + } + + mCount++; + return OK; +} + +status_t OpcodeListBuilder::addOpcodePreamble(uint32_t opcodeId) { + status_t err = mEndianOut.write(&opcodeId, 0, 1); + if (err != OK) return err; + + uint8_t version[] = {1, 3, 0, 0}; + err = mEndianOut.write(version, 0, NELEMS(version)); + if (err != OK) return err; + return OK; +} + } /*namespace img_utils*/ } /*namespace android*/ -- cgit v1.1 From fa4303dcbeac79452f35c078c8008f5c2e7622b7 Mon Sep 17 00:00:00 2001 From: Chong Zhang Date: Wed, 12 Aug 2015 16:32:00 -0700 Subject: adjust audio timestamp to account for AudioRecord latency bug: 22953017 Change-Id: Iccd1bb406ff68aa8bc3ccec35c8128625894f6ae --- media/libstagefright/AudioSource.cpp | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'media') diff --git a/media/libstagefright/AudioSource.cpp b/media/libstagefright/AudioSource.cpp index 3505844..6e4a1dd 100644 --- a/media/libstagefright/AudioSource.cpp +++ b/media/libstagefright/AudioSource.cpp @@ -290,6 +290,10 @@ void AudioSource::signalBufferReturned(MediaBuffer *buffer) { status_t AudioSource::dataCallback(const AudioRecord::Buffer& audioBuffer) { int64_t timeUs = systemTime() / 1000ll; + // Estimate the real sampling time of the 1st sample in this buffer + // from AudioRecord's latency. (Apply this adjustment first so that + // the start time logic is not affected.) + timeUs -= mRecord->latency() * 1000LL; ALOGV("dataCallbackTimestamp: %" PRId64 " us", timeUs); Mutex::Autolock autoLock(mLock); -- cgit v1.1 From 6d3cd2e22c73e5b554a2c4e34d51616f8737e571 Mon Sep 17 00:00:00 2001 From: Robert Shih Date: Thu, 13 Aug 2015 12:09:52 -0700 Subject: RTSPSource::pause mHandler NULL check Bug: 23151568 Change-Id: I2dba3e7388b1b84b8b762dbc82c8e5330a158d97 --- media/libmediaplayerservice/nuplayer/RTSPSource.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'media') diff --git a/media/libmediaplayerservice/nuplayer/RTSPSource.cpp b/media/libmediaplayerservice/nuplayer/RTSPSource.cpp index 58ff113..af0351e 100644 --- a/media/libmediaplayerservice/nuplayer/RTSPSource.cpp +++ b/media/libmediaplayerservice/nuplayer/RTSPSource.cpp @@ -134,7 +134,9 @@ void NuPlayer::RTSPSource::pause() { return; } } - mHandler->pause(); + if (mHandler != NULL) { + mHandler->pause(); + } } void NuPlayer::RTSPSource::resume() { -- cgit v1.1 From a5f50e98d1408addcaaac27e4d13981163d12a15 Mon Sep 17 00:00:00 2001 From: Wei Jia Date: Mon, 8 Jun 2015 14:01:42 -0700 Subject: DO NOT MERGE - SoftwareRenderer: sanity check buffer size before copying data. Bug: 21443020 Change-Id: I63cf86217b8201fb41809c23e4b752b845a93ee2 (cherry picked from commit 760f92f8b6da9c9cf128cb18fe3c09402fdde6cd) --- media/libstagefright/colorconversion/SoftwareRenderer.cpp | 7 +++++++ 1 file changed, 7 insertions(+) (limited to 'media') diff --git a/media/libstagefright/colorconversion/SoftwareRenderer.cpp b/media/libstagefright/colorconversion/SoftwareRenderer.cpp index 77f21b7..990d554 100644 --- a/media/libstagefright/colorconversion/SoftwareRenderer.cpp +++ b/media/libstagefright/colorconversion/SoftwareRenderer.cpp @@ -164,6 +164,9 @@ void SoftwareRenderer::render( buf->stride, buf->height, 0, 0, mCropWidth - 1, mCropHeight - 1); } else if (mColorFormat == OMX_COLOR_FormatYUV420Planar) { + if ((size_t)mWidth * mHeight * 3 / 2 > size) { + goto skip_copying; + } const uint8_t *src_y = (const uint8_t *)data; const uint8_t *src_u = (const uint8_t *)data + mWidth * mHeight; const uint8_t *src_v = src_u + (mWidth / 2 * mHeight / 2); @@ -193,6 +196,9 @@ void SoftwareRenderer::render( } } else { CHECK_EQ(mColorFormat, OMX_TI_COLOR_FormatYUV420PackedSemiPlanar); + if ((size_t)mWidth * mHeight * 3 / 2 > size) { + goto skip_copying; + } const uint8_t *src_y = (const uint8_t *)data; @@ -228,6 +234,7 @@ void SoftwareRenderer::render( } } +skip_copying: CHECK_EQ(0, mapper.unlock(buf->handle)); if ((err = mNativeWindow->queueBuffer(mNativeWindow.get(), buf, -- cgit v1.1 From dc5e47f013bfbb74c5c35ad976aa98d480cb351b Mon Sep 17 00:00:00 2001 From: Wei Jia Date: Mon, 8 Jun 2015 14:01:42 -0700 Subject: DO NOT MERGE - SoftwareRenderer: sanity check buffer size before copying data. Bug: 21443020 Change-Id: I63cf86217b8201fb41809c23e4b752b845a93ee2 (cherry picked from commit 760f92f8b6da9c9cf128cb18fe3c09402fdde6cd) --- media/libstagefright/colorconversion/SoftwareRenderer.cpp | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) (limited to 'media') diff --git a/media/libstagefright/colorconversion/SoftwareRenderer.cpp b/media/libstagefright/colorconversion/SoftwareRenderer.cpp index 1899b40..f906757 100644 --- a/media/libstagefright/colorconversion/SoftwareRenderer.cpp +++ b/media/libstagefright/colorconversion/SoftwareRenderer.cpp @@ -180,7 +180,7 @@ void SoftwareRenderer::resetFormatIfChanged(const sp &format) { } void SoftwareRenderer::render( - const void *data, size_t /*size*/, int64_t timestampNs, + const void *data, size_t size, int64_t timestampNs, void* /*platformPrivate*/, const sp& format) { resetFormatIfChanged(format); @@ -209,6 +209,9 @@ void SoftwareRenderer::render( buf->stride, buf->height, 0, 0, mCropWidth - 1, mCropHeight - 1); } else if (mColorFormat == OMX_COLOR_FormatYUV420Planar) { + if ((size_t)mWidth * mHeight * 3 / 2 > size) { + goto skip_copying; + } const uint8_t *src_y = (const uint8_t *)data; const uint8_t *src_u = (const uint8_t *)data + mWidth * mHeight; const uint8_t *src_v = src_u + (mWidth / 2 * mHeight / 2); @@ -238,6 +241,9 @@ void SoftwareRenderer::render( } } else { CHECK_EQ(mColorFormat, OMX_TI_COLOR_FormatYUV420PackedSemiPlanar); + if ((size_t)mWidth * mHeight * 3 / 2 > size) { + goto skip_copying; + } const uint8_t *src_y = (const uint8_t *)data; @@ -273,6 +279,7 @@ void SoftwareRenderer::render( } } +skip_copying: CHECK_EQ(0, mapper.unlock(buf->handle)); if ((err = native_window_set_buffers_timestamp(mNativeWindow.get(), -- cgit v1.1 From 0f3e2daa1d56c98196a719a6e641f3ed67b8e7bf Mon Sep 17 00:00:00 2001 From: Marco Nelissen Date: Tue, 16 Jun 2015 14:50:36 -0700 Subject: DO NOT MERGE - Fix software video decoder buffer size calculation Various software video decoders would specify the buffer size as if it were fully cropped, which then failed a sanity check in SoftwareRenderer. They now return the full buffer size. Bug: 21717327 Bug: 21443020 Change-Id: I19fcd091827ebd52a95a5509281a07ccc156e0e5 (cherry picked from commit 3ecc9db40b1fb9c7f807a5892e5c9625aac1fb06) --- media/libstagefright/codecs/on2/dec/SoftVPX.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'media') diff --git a/media/libstagefright/codecs/on2/dec/SoftVPX.cpp b/media/libstagefright/codecs/on2/dec/SoftVPX.cpp index 476e986..2bae2ad 100644 --- a/media/libstagefright/codecs/on2/dec/SoftVPX.cpp +++ b/media/libstagefright/codecs/on2/dec/SoftVPX.cpp @@ -151,7 +151,7 @@ void SoftVPX::onQueueFilled(OMX_U32 portIndex) { } outHeader->nOffset = 0; - outHeader->nFilledLen = (width * height * 3) / 2; + outHeader->nFilledLen = (mWidth * mHeight * 3) / 2; outHeader->nFlags = EOSseen ? OMX_BUFFERFLAG_EOS : 0; outHeader->nTimeStamp = inHeader->nTimeStamp; -- cgit v1.1 From 7aa86ecdf4a8b981aed2b6717e6a104a360d18c0 Mon Sep 17 00:00:00 2001 From: Marco Nelissen Date: Tue, 16 Jun 2015 14:50:36 -0700 Subject: DO NOT MERGE - Fix software video decoder buffer size calculation Various software video decoders would specify the buffer size as if it were fully cropped, which then failed a sanity check in SoftwareRenderer. They now return the full buffer size. Bug: 21717327 Bug: 21443020 Change-Id: I19fcd091827ebd52a95a5509281a07ccc156e0e5 (cherry picked from commit 3ecc9db40b1fb9c7f807a5892e5c9625aac1fb06) --- media/libstagefright/codecs/on2/dec/SoftVPX.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'media') diff --git a/media/libstagefright/codecs/on2/dec/SoftVPX.cpp b/media/libstagefright/codecs/on2/dec/SoftVPX.cpp index 828577a..09e8789 100644 --- a/media/libstagefright/codecs/on2/dec/SoftVPX.cpp +++ b/media/libstagefright/codecs/on2/dec/SoftVPX.cpp @@ -144,7 +144,7 @@ void SoftVPX::onQueueFilled(OMX_U32 /* portIndex */) { } outHeader->nOffset = 0; - outHeader->nFilledLen = (width * height * 3) / 2; + outHeader->nFilledLen = (outputBufferWidth() * outputBufferHeight() * 3) / 2; outHeader->nFlags = EOSseen ? OMX_BUFFERFLAG_EOS : 0; outHeader->nTimeStamp = inHeader->nTimeStamp; -- cgit v1.1 From 2e17eef829f1870d9bde963a356dfb11f120a6b5 Mon Sep 17 00:00:00 2001 From: Preetam Singh Ranawat Date: Wed, 12 Aug 2015 12:11:46 -0700 Subject: AudioSink: Fix for gapless offload playback Gapless mode is not working for offload playback due to mismatch in flags of current track and next track to be played. AUDIO_OUTPUT_FLAG_DIRECT is added in AudioTrack flags for current track which does not match exactly with requested flags for next track. Because of this mismatch, reuse of the AudioTrack is not allowed. To fix this, update audio sink flags with requested flags and use track flags only to setPlaybackRate if AUDIO_OUTPUT_FLAG_DIRECT flag is not set. Bug: 23221273 authored-by: Preetam Singh Ranawat Change-Id: I52761ccd854b66a7bc218e83c9b44598771c46f7 --- media/libmediaplayerservice/MediaPlayerService.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'media') diff --git a/media/libmediaplayerservice/MediaPlayerService.cpp b/media/libmediaplayerservice/MediaPlayerService.cpp index 56521a2..bcfd83a 100644 --- a/media/libmediaplayerservice/MediaPlayerService.cpp +++ b/media/libmediaplayerservice/MediaPlayerService.cpp @@ -1734,7 +1734,7 @@ status_t MediaPlayerService::AudioOutput::open( t->setVolume(mLeftVolume, mRightVolume); mSampleRateHz = sampleRate; - mFlags = t->getFlags(); // we suggest the flags above, but new AudioTrack() may not grant it. + mFlags = flags; mMsecsPerFrame = 1E3f / (mPlaybackRate.mSpeed * sampleRate); mFrameSize = t->frameSize(); uint32_t pos; @@ -1746,7 +1746,7 @@ status_t MediaPlayerService::AudioOutput::open( status_t res = NO_ERROR; // Note some output devices may give us a direct track even though we don't specify it. // Example: Line application b/17459982. - if ((mFlags & (AUDIO_OUTPUT_FLAG_COMPRESS_OFFLOAD | AUDIO_OUTPUT_FLAG_DIRECT)) == 0) { + if ((t->getFlags() & (AUDIO_OUTPUT_FLAG_COMPRESS_OFFLOAD | AUDIO_OUTPUT_FLAG_DIRECT)) == 0) { res = t->setPlaybackRate(mPlaybackRate); if (res == NO_ERROR) { t->setAuxEffectSendLevel(mSendLevel); -- cgit v1.1 From d0b5910f78b45ce98511d31ec327ccaafe127f3f Mon Sep 17 00:00:00 2001 From: Andy Hung Date: Wed, 26 Aug 2015 16:34:33 -0700 Subject: libmedia: clear reply data for IEffect command Bug: 23540907 Change-Id: Ib89afc6b273b0eb310bbc5a1bd92b1e3d407c249 --- media/libmedia/IEffect.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'media') diff --git a/media/libmedia/IEffect.cpp b/media/libmedia/IEffect.cpp index eb4b098..761b243 100644 --- a/media/libmedia/IEffect.cpp +++ b/media/libmedia/IEffect.cpp @@ -154,14 +154,14 @@ status_t BnEffect::onTransact( uint32_t cmdSize = data.readInt32(); char *cmd = NULL; if (cmdSize) { - cmd = (char *)malloc(cmdSize); + cmd = (char *)calloc(cmdSize, 1); data.read(cmd, cmdSize); } uint32_t replySize = data.readInt32(); uint32_t replySz = replySize; char *resp = NULL; if (replySize) { - resp = (char *)malloc(replySize); + resp = (char *)calloc(replySize, 1); } status_t status = command(cmdCode, cmdSize, cmd, &replySz, resp); reply->writeInt32(status); -- cgit v1.1 From 57bed83a539535bb64a33722fb67231119cb0618 Mon Sep 17 00:00:00 2001 From: Andy Hung Date: Wed, 26 Aug 2015 16:34:33 -0700 Subject: libmedia: clear reply data for IEffect command Bug: 23540907 Change-Id: Ib89afc6b273b0eb310bbc5a1bd92b1e3d407c249 --- media/libmedia/IEffect.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'media') diff --git a/media/libmedia/IEffect.cpp b/media/libmedia/IEffect.cpp index a303a8f..86c706a 100644 --- a/media/libmedia/IEffect.cpp +++ b/media/libmedia/IEffect.cpp @@ -151,14 +151,14 @@ status_t BnEffect::onTransact( uint32_t cmdSize = data.readInt32(); char *cmd = NULL; if (cmdSize) { - cmd = (char *)malloc(cmdSize); + cmd = (char *)calloc(cmdSize, 1); data.read(cmd, cmdSize); } uint32_t replySize = data.readInt32(); uint32_t replySz = replySize; char *resp = NULL; if (replySize) { - resp = (char *)malloc(replySize); + resp = (char *)calloc(replySize, 1); } status_t status = command(cmdCode, cmdSize, cmd, &replySz, resp); reply->writeInt32(status); -- cgit v1.1 From 566da808857c2c26e191ce18aba5abe97746fe1a Mon Sep 17 00:00:00 2001 From: Wei Jia Date: Thu, 27 Aug 2015 13:59:30 -0700 Subject: NuPlayer: do not create audio decoder if the player is still in shutdown process. Bug: 23350795 Change-Id: I46b02cf31d7d4447806910a9ecd8c3bda05f2f5b --- media/libmediaplayerservice/nuplayer/NuPlayer.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'media') diff --git a/media/libmediaplayerservice/nuplayer/NuPlayer.cpp b/media/libmediaplayerservice/nuplayer/NuPlayer.cpp index 77b9799..c0146d5 100644 --- a/media/libmediaplayerservice/nuplayer/NuPlayer.cpp +++ b/media/libmediaplayerservice/nuplayer/NuPlayer.cpp @@ -1488,7 +1488,9 @@ void NuPlayer::determineAudioModeChange() { } status_t NuPlayer::instantiateDecoder(bool audio, sp *decoder) { - if (*decoder != NULL) { + // The audio decoder could be cleared by tear down. If still in shut down + // process, no need to create a new audio decoder. + if (*decoder != NULL || (audio && mFlushingAudio == SHUT_DOWN)) { return OK; } -- cgit v1.1 From 0981df6e3db106bfb7a56a2b668c012fcc34dd2c Mon Sep 17 00:00:00 2001 From: Nick Kralevich Date: Thu, 20 Aug 2015 09:56:39 -0700 Subject: IMediaPlayer.cpp: make sure structures are initialized to 0 Credit https://code.google.com/p/android/issues/detail?id=183310 Bug: 23515142 Change-Id: Idbd66fb148bd0ac1dd78f8651d0164f2a41e2427 (cherry picked from commit b73b826cc16291b33649402497efbe0f946413bd) --- media/libmedia/IMediaPlayer.cpp | 2 ++ 1 file changed, 2 insertions(+) (limited to 'media') diff --git a/media/libmedia/IMediaPlayer.cpp b/media/libmedia/IMediaPlayer.cpp index e79bcd2..9b57902 100644 --- a/media/libmedia/IMediaPlayer.cpp +++ b/media/libmedia/IMediaPlayer.cpp @@ -510,6 +510,7 @@ status_t BnMediaPlayer::onTransact( CHECK_INTERFACE(IMediaPlayer, data, reply); struct sockaddr_in endpoint; + memset(&endpoint, 0, sizeof(endpoint)); int amt = data.readInt32(); if (amt == sizeof(endpoint)) { data.read(&endpoint, sizeof(struct sockaddr_in)); @@ -524,6 +525,7 @@ status_t BnMediaPlayer::onTransact( CHECK_INTERFACE(IMediaPlayer, data, reply); struct sockaddr_in endpoint; + memset(&endpoint, 0, sizeof(endpoint)); status_t res = getRetransmitEndpoint(&endpoint); reply->writeInt32(res); -- cgit v1.1 From c6fc6a3ca618b0e72ee565ded2e4960797f53fa6 Mon Sep 17 00:00:00 2001 From: Jeff Tinker Date: Wed, 26 Aug 2015 20:22:39 -0700 Subject: Fix for security vulnerability in media server bug: 23540426 Change-Id: Ifb12ac3350410a49ba7d81d1bde12822c3008cd5 --- media/libmedia/ICrypto.cpp | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) (limited to 'media') diff --git a/media/libmedia/ICrypto.cpp b/media/libmedia/ICrypto.cpp index 947294f..9703b0d 100644 --- a/media/libmedia/ICrypto.cpp +++ b/media/libmedia/ICrypto.cpp @@ -303,7 +303,25 @@ status_t BnCrypto::onTransact( AString errorDetailMsg; ssize_t result; - if (offset + totalSize > sharedBuffer->size()) { + size_t sumSubsampleSizes = 0; + bool overflow = false; + for (int32_t i = 0; i < numSubSamples; ++i) { + CryptoPlugin::SubSample &ss = subSamples[i]; + if (sumSubsampleSizes <= SIZE_MAX - ss.mNumBytesOfEncryptedData) { + sumSubsampleSizes += ss.mNumBytesOfEncryptedData; + } else { + overflow = true; + } + if (sumSubsampleSizes <= SIZE_MAX - ss.mNumBytesOfClearData) { + sumSubsampleSizes += ss.mNumBytesOfClearData; + } else { + overflow = true; + } + } + + if (overflow || sumSubsampleSizes != totalSize) { + result = -EINVAL; + } else if (offset + totalSize > sharedBuffer->size()) { result = -EINVAL; } else { result = decrypt( -- cgit v1.1 From d53aced041b7214a92b1f2fd5970d895bb9934e5 Mon Sep 17 00:00:00 2001 From: Marco Nelissen Date: Thu, 27 Aug 2015 13:49:32 -0700 Subject: Fix timedtext parsing Add bounds checking and fix other bugs. Bug: 23284974 Bug: 23541506 Bug: 23542351 Bug: 23542352 Change-Id: I53551efdf109ce1833e0c361efaf4cee7a851023 --- .../libstagefright/timedtext/TextDescriptions.cpp | 627 ++++++++++++--------- media/libstagefright/timedtext/TextDescriptions.h | 4 +- 2 files changed, 377 insertions(+), 254 deletions(-) (limited to 'media') diff --git a/media/libstagefright/timedtext/TextDescriptions.cpp b/media/libstagefright/timedtext/TextDescriptions.cpp index f9c1fe0..c762a74 100644 --- a/media/libstagefright/timedtext/TextDescriptions.cpp +++ b/media/libstagefright/timedtext/TextDescriptions.cpp @@ -30,9 +30,9 @@ status_t TextDescriptions::getParcelOfDescriptions( if (flags & IN_BAND_TEXT_3GPP) { if (flags & GLOBAL_DESCRIPTIONS) { - return extract3GPPGlobalDescriptions(data, size, parcel, 0); + return extract3GPPGlobalDescriptions(data, size, parcel); } else if (flags & LOCAL_DESCRIPTIONS) { - return extract3GPPLocalDescriptions(data, size, timeMs, parcel, 0); + return extract3GPPLocalDescriptions(data, size, timeMs, parcel); } } else if (flags & OUT_OF_BAND_TEXT_SRT) { if (flags & LOCAL_DESCRIPTIONS) { @@ -69,314 +69,437 @@ status_t TextDescriptions::extractSRTLocalDescriptions( // styles, and 'krok' box contains karaoke timing and positions. status_t TextDescriptions::extract3GPPLocalDescriptions( const uint8_t *data, ssize_t size, - int timeMs, Parcel *parcel, int depth) { - if (depth == 0) { - parcel->writeInt32(KEY_LOCAL_SETTING); - - // write start time to display this text sample - parcel->writeInt32(KEY_START_TIME); - parcel->writeInt32(timeMs); - - ssize_t textLen = (*data) << 8 | (*(data + 1)); - - // write text sample length and text sample itself - parcel->writeInt32(KEY_STRUCT_TEXT); - parcel->writeInt32(textLen); - parcel->writeInt32(textLen); - parcel->write(data + 2, textLen); - - if (size > textLen) { - data += (textLen + 2); - size -= (textLen + 2); - } else { - return OK; - } + int timeMs, Parcel *parcel) { + + parcel->writeInt32(KEY_LOCAL_SETTING); + + // write start time to display this text sample + parcel->writeInt32(KEY_START_TIME); + parcel->writeInt32(timeMs); + + if (size < 2) { + return OK; } + ssize_t textLen = (*data) << 8 | (*(data + 1)); - const uint8_t *tmpData = data; - ssize_t chunkSize = U32_AT(tmpData); - uint32_t chunkType = U32_AT(tmpData + 4); + if (size < textLen + 2) { + return OK; + } - if (chunkSize <= 0) { + // write text sample length and text sample itself + parcel->writeInt32(KEY_STRUCT_TEXT); + parcel->writeInt32(textLen); + parcel->writeInt32(textLen); + parcel->write(data + 2, textLen); + + if (size > textLen + 2) { + data += (textLen + 2); + size -= (textLen + 2); + } else { return OK; } - tmpData += 8; + while (size >= 8) { + const uint8_t *tmpData = data; + ssize_t chunkSize = U32_AT(tmpData); // size includes size and type + uint32_t chunkType = U32_AT(tmpData + 4); - switch(chunkType) { - // 'styl' box specifies the style of the text. - case FOURCC('s', 't', 'y', 'l'): - { - uint16_t count = U16_AT(tmpData); + if (chunkSize <= 8 || chunkSize > size) { + return OK; + } - tmpData += 2; + size_t remaining = chunkSize - 8; - for (int i = 0; i < count; i++) { - parcel->writeInt32(KEY_STRUCT_STYLE_LIST); - parcel->writeInt32(KEY_START_CHAR); - parcel->writeInt32(U16_AT(tmpData)); + tmpData += 8; - parcel->writeInt32(KEY_END_CHAR); - parcel->writeInt32(U16_AT(tmpData + 2)); + switch(chunkType) { + // 'styl' box specifies the style of the text. + case FOURCC('s', 't', 'y', 'l'): + { + if (remaining < 2) { + return OK; + } + size_t dataPos = parcel->dataPosition(); + uint16_t count = U16_AT(tmpData); - parcel->writeInt32(KEY_FONT_ID); - parcel->writeInt32(U16_AT(tmpData + 4)); + tmpData += 2; + remaining -= 2; - parcel->writeInt32(KEY_STYLE_FLAGS); - parcel->writeInt32(*(tmpData + 6)); + for (int i = 0; i < count; i++) { + if (remaining < 12) { + // roll back + parcel->setDataPosition(dataPos); + return OK; + } + parcel->writeInt32(KEY_STRUCT_STYLE_LIST); + parcel->writeInt32(KEY_START_CHAR); + parcel->writeInt32(U16_AT(tmpData)); - parcel->writeInt32(KEY_FONT_SIZE); - parcel->writeInt32(*(tmpData + 7)); + parcel->writeInt32(KEY_END_CHAR); + parcel->writeInt32(U16_AT(tmpData + 2)); - parcel->writeInt32(KEY_TEXT_COLOR_RGBA); - uint32_t rgba = *(tmpData + 8) << 24 | *(tmpData + 9) << 16 - | *(tmpData + 10) << 8 | *(tmpData + 11); - parcel->writeInt32(rgba); + parcel->writeInt32(KEY_FONT_ID); + parcel->writeInt32(U16_AT(tmpData + 4)); - tmpData += 12; + parcel->writeInt32(KEY_STYLE_FLAGS); + parcel->writeInt32(*(tmpData + 6)); + + parcel->writeInt32(KEY_FONT_SIZE); + parcel->writeInt32(*(tmpData + 7)); + + parcel->writeInt32(KEY_TEXT_COLOR_RGBA); + uint32_t rgba = *(tmpData + 8) << 24 | *(tmpData + 9) << 16 + | *(tmpData + 10) << 8 | *(tmpData + 11); + parcel->writeInt32(rgba); + + tmpData += 12; + remaining -= 12; + } + + break; + } + // 'krok' box. The number of highlight events is specified, and each + // event is specified by a starting and ending char offset and an end + // time for the event. + case FOURCC('k', 'r', 'o', 'k'): + { + if (remaining < 6) { + return OK; + } + size_t dataPos = parcel->dataPosition(); + + parcel->writeInt32(KEY_STRUCT_KARAOKE_LIST); + + int startTime = U32_AT(tmpData); + uint16_t count = U16_AT(tmpData + 4); + parcel->writeInt32(count); + + tmpData += 6; + remaining -= 6; + int lastEndTime = 0; + + for (int i = 0; i < count; i++) { + if (remaining < 8) { + // roll back + parcel->setDataPosition(dataPos); + return OK; + } + parcel->writeInt32(startTime + lastEndTime); + + lastEndTime = U32_AT(tmpData); + parcel->writeInt32(lastEndTime); + + parcel->writeInt32(U16_AT(tmpData + 4)); + parcel->writeInt32(U16_AT(tmpData + 6)); + + tmpData += 8; + remaining -= 8; + } + + break; } + // 'hlit' box specifies highlighted text + case FOURCC('h', 'l', 'i', 't'): + { + if (remaining < 4) { + return OK; + } - break; - } - // 'krok' box. The number of highlight events is specified, and each - // event is specified by a starting and ending char offset and an end - // time for the event. - case FOURCC('k', 'r', 'o', 'k'): - { + parcel->writeInt32(KEY_STRUCT_HIGHLIGHT_LIST); - parcel->writeInt32(KEY_STRUCT_KARAOKE_LIST); + // the start char offset to highlight + parcel->writeInt32(U16_AT(tmpData)); + // the last char offset to highlight + parcel->writeInt32(U16_AT(tmpData + 2)); - int startTime = U32_AT(tmpData); - uint16_t count = U16_AT(tmpData + 4); - parcel->writeInt32(count); + tmpData += 4; + remaining -= 4; + break; + } + // 'hclr' box specifies the RGBA color: 8 bits each of + // red, green, blue, and an alpha(transparency) value + case FOURCC('h', 'c', 'l', 'r'): + { + if (remaining < 4) { + return OK; + } + parcel->writeInt32(KEY_HIGHLIGHT_COLOR_RGBA); + + uint32_t rgba = *(tmpData) << 24 | *(tmpData + 1) << 16 + | *(tmpData + 2) << 8 | *(tmpData + 3); + parcel->writeInt32(rgba); + + tmpData += 4; + remaining -= 4; + break; + } + // 'dlay' box specifies a delay after a scroll in and/or + // before scroll out. + case FOURCC('d', 'l', 'a', 'y'): + { + if (remaining < 4) { + return OK; + } + parcel->writeInt32(KEY_SCROLL_DELAY); + + uint32_t delay = *(tmpData) << 24 | *(tmpData + 1) << 16 + | *(tmpData + 2) << 8 | *(tmpData + 3); + parcel->writeInt32(delay); + + tmpData += 4; + remaining -= 4; + break; + } + // 'href' box for hyper text link + case FOURCC('h', 'r', 'e', 'f'): + { + if (remaining < 5) { + return OK; + } - tmpData += 6; - int lastEndTime = 0; + size_t dataPos = parcel->dataPosition(); - for (int i = 0; i < count; i++) { - parcel->writeInt32(startTime + lastEndTime); + parcel->writeInt32(KEY_STRUCT_HYPER_TEXT_LIST); - lastEndTime = U32_AT(tmpData); - parcel->writeInt32(lastEndTime); + // the start offset of the text to be linked + parcel->writeInt32(U16_AT(tmpData)); + // the end offset of the text + parcel->writeInt32(U16_AT(tmpData + 2)); + // the number of bytes in the following URL + size_t len = *(tmpData + 4); + parcel->writeInt32(len); + + remaining -= 5; + + if (remaining < len) { + parcel->setDataPosition(dataPos); + return OK; + } + // the linked-to URL + parcel->writeInt32(len); + parcel->write(tmpData + 5, len); + + tmpData += (5 + len); + remaining -= len; + + if (remaining < 1) { + parcel->setDataPosition(dataPos); + return OK; + } + + // the number of bytes in the following "alt" string + len = *tmpData; + parcel->writeInt32(len); + + tmpData += 1; + remaining -= 1; + if (remaining < len) { + parcel->setDataPosition(dataPos); + return OK; + } + + // an "alt" string for user display + parcel->writeInt32(len); + parcel->write(tmpData, len); + + tmpData += 1; + remaining -= 1; + break; + } + // 'tbox' box to indicate the position of the text with values + // of top, left, bottom and right + case FOURCC('t', 'b', 'o', 'x'): + { + if (remaining < 8) { + return OK; + } + parcel->writeInt32(KEY_STRUCT_TEXT_POS); + parcel->writeInt32(U16_AT(tmpData)); + parcel->writeInt32(U16_AT(tmpData + 2)); parcel->writeInt32(U16_AT(tmpData + 4)); parcel->writeInt32(U16_AT(tmpData + 6)); tmpData += 8; + remaining -= 8; + break; } + // 'blnk' to specify the char range to be blinked + case FOURCC('b', 'l', 'n', 'k'): + { + if (remaining < 4) { + return OK; + } - break; - } - // 'hlit' box specifies highlighted text - case FOURCC('h', 'l', 'i', 't'): - { - parcel->writeInt32(KEY_STRUCT_HIGHLIGHT_LIST); + parcel->writeInt32(KEY_STRUCT_BLINKING_TEXT_LIST); - // the start char offset to highlight - parcel->writeInt32(U16_AT(tmpData)); - // the last char offset to highlight - parcel->writeInt32(U16_AT(tmpData + 2)); + // start char offset + parcel->writeInt32(U16_AT(tmpData)); + // end char offset + parcel->writeInt32(U16_AT(tmpData + 2)); - break; + tmpData += 4; + remaining -= 4; + break; + } + // 'twrp' box specifies text wrap behavior. If the value if 0x00, + // then no wrap. If it's 0x01, then automatic 'soft' wrap is enabled. + // 0x02-0xff are reserved. + case FOURCC('t', 'w', 'r', 'p'): + { + if (remaining < 1) { + return OK; + } + parcel->writeInt32(KEY_WRAP_TEXT); + parcel->writeInt32(*tmpData); + + tmpData += 1; + remaining -= 1; + break; + } + default: + { + break; + } } - // 'hclr' box specifies the RGBA color: 8 bits each of - // red, green, blue, and an alpha(transparency) value - case FOURCC('h', 'c', 'l', 'r'): - { - parcel->writeInt32(KEY_HIGHLIGHT_COLOR_RGBA); - uint32_t rgba = *(tmpData) << 24 | *(tmpData + 1) << 16 - | *(tmpData + 2) << 8 | *(tmpData + 3); - parcel->writeInt32(rgba); + data += chunkSize; + size -= chunkSize; + } + + return OK; +} - break; - } - // 'dlay' box specifies a delay after a scroll in and/or - // before scroll out. - case FOURCC('d', 'l', 'a', 'y'): - { - parcel->writeInt32(KEY_SCROLL_DELAY); +// To extract box 'tx3g' defined in 3GPP TS 26.245, and store it in a Parcel +status_t TextDescriptions::extract3GPPGlobalDescriptions( + const uint8_t *data, ssize_t size, Parcel *parcel) { + + parcel->writeInt32(KEY_GLOBAL_SETTING); - uint32_t delay = *(tmpData) << 24 | *(tmpData + 1) << 16 - | *(tmpData + 2) << 8 | *(tmpData + 3); - parcel->writeInt32(delay); + while (size >= 8) { + ssize_t chunkSize = U32_AT(data); + uint32_t chunkType = U32_AT(data + 4); + const uint8_t *tmpData = data; + tmpData += 8; + size_t remaining = size - 8; - break; + if (size < chunkSize) { + return OK; } - // 'href' box for hyper text link - case FOURCC('h', 'r', 'e', 'f'): - { - parcel->writeInt32(KEY_STRUCT_HYPER_TEXT_LIST); + switch(chunkType) { + case FOURCC('t', 'x', '3', 'g'): + { + if (remaining < 18) { // 8 just below, and another 10 a little further down + return OK; + } + tmpData += 8; // skip the first 8 bytes + remaining -=8; + parcel->writeInt32(KEY_DISPLAY_FLAGS); + parcel->writeInt32(U32_AT(tmpData)); + + parcel->writeInt32(KEY_STRUCT_JUSTIFICATION); + parcel->writeInt32(tmpData[4]); + parcel->writeInt32(tmpData[5]); + + parcel->writeInt32(KEY_BACKGROUND_COLOR_RGBA); + uint32_t rgba = *(tmpData + 6) << 24 | *(tmpData + 7) << 16 + | *(tmpData + 8) << 8 | *(tmpData + 9); + parcel->writeInt32(rgba); - // the start offset of the text to be linked - parcel->writeInt32(U16_AT(tmpData)); - // the end offset of the text - parcel->writeInt32(U16_AT(tmpData + 2)); + tmpData += 10; + remaining -= 10; - // the number of bytes in the following URL - int len = *(tmpData + 4); - parcel->writeInt32(len); + if (remaining < 8) { + return OK; + } + parcel->writeInt32(KEY_STRUCT_TEXT_POS); + parcel->writeInt32(U16_AT(tmpData)); + parcel->writeInt32(U16_AT(tmpData + 2)); + parcel->writeInt32(U16_AT(tmpData + 4)); + parcel->writeInt32(U16_AT(tmpData + 6)); - // the linked-to URL - parcel->writeInt32(len); - parcel->write(tmpData + 5, len); + tmpData += 8; + remaining -= 8; - tmpData += (5 + len); + if (remaining < 12) { + return OK; + } + parcel->writeInt32(KEY_STRUCT_STYLE_LIST); + parcel->writeInt32(KEY_START_CHAR); + parcel->writeInt32(U16_AT(tmpData)); - // the number of bytes in the following "alt" string - len = *tmpData; - parcel->writeInt32(len); + parcel->writeInt32(KEY_END_CHAR); + parcel->writeInt32(U16_AT(tmpData + 2)); - // an "alt" string for user display - parcel->writeInt32(len); - parcel->write(tmpData + 1, len); + parcel->writeInt32(KEY_FONT_ID); + parcel->writeInt32(U16_AT(tmpData + 4)); - break; - } - // 'tbox' box to indicate the position of the text with values - // of top, left, bottom and right - case FOURCC('t', 'b', 'o', 'x'): - { - parcel->writeInt32(KEY_STRUCT_TEXT_POS); - parcel->writeInt32(U16_AT(tmpData)); - parcel->writeInt32(U16_AT(tmpData + 2)); - parcel->writeInt32(U16_AT(tmpData + 4)); - parcel->writeInt32(U16_AT(tmpData + 6)); - - break; - } - // 'blnk' to specify the char range to be blinked - case FOURCC('b', 'l', 'n', 'k'): - { - parcel->writeInt32(KEY_STRUCT_BLINKING_TEXT_LIST); + parcel->writeInt32(KEY_STYLE_FLAGS); + parcel->writeInt32(*(tmpData + 6)); - // start char offset - parcel->writeInt32(U16_AT(tmpData)); - // end char offset - parcel->writeInt32(U16_AT(tmpData + 2)); + parcel->writeInt32(KEY_FONT_SIZE); + parcel->writeInt32(*(tmpData + 7)); - break; - } - // 'twrp' box specifies text wrap behavior. If the value if 0x00, - // then no wrap. If it's 0x01, then automatic 'soft' wrap is enabled. - // 0x02-0xff are reserved. - case FOURCC('t', 'w', 'r', 'p'): - { - parcel->writeInt32(KEY_WRAP_TEXT); - parcel->writeInt32(*tmpData); - - break; - } - default: - { - break; - } - } + parcel->writeInt32(KEY_TEXT_COLOR_RGBA); + rgba = *(tmpData + 8) << 24 | *(tmpData + 9) << 16 + | *(tmpData + 10) << 8 | *(tmpData + 11); + parcel->writeInt32(rgba); - if (size > chunkSize) { - data += chunkSize; - size -= chunkSize; - // continue to parse next box - return extract3GPPLocalDescriptions(data, size, 0, parcel, 1); - } + tmpData += 12; + remaining -= 12; - return OK; -} + if (remaining < 2) { + return OK; + } -// To extract box 'tx3g' defined in 3GPP TS 26.245, and store it in a Parcel -status_t TextDescriptions::extract3GPPGlobalDescriptions( - const uint8_t *data, ssize_t size, Parcel *parcel, int depth) { + size_t dataPos = parcel->dataPosition(); - ssize_t chunkSize = U32_AT(data); - uint32_t chunkType = U32_AT(data + 4); - const uint8_t *tmpData = data; - tmpData += 8; + parcel->writeInt32(KEY_STRUCT_FONT_LIST); + uint16_t count = U16_AT(tmpData); + parcel->writeInt32(count); - if (size < chunkSize) { - return OK; - } + tmpData += 2; + remaining -= 2; - if (depth == 0) { - parcel->writeInt32(KEY_GLOBAL_SETTING); - } - switch(chunkType) { - case FOURCC('t', 'x', '3', 'g'): - { - tmpData += 8; // skip the first 8 bytes - parcel->writeInt32(KEY_DISPLAY_FLAGS); - parcel->writeInt32(U32_AT(tmpData)); - - parcel->writeInt32(KEY_STRUCT_JUSTIFICATION); - parcel->writeInt32(tmpData[4]); - parcel->writeInt32(tmpData[5]); - - parcel->writeInt32(KEY_BACKGROUND_COLOR_RGBA); - uint32_t rgba = *(tmpData + 6) << 24 | *(tmpData + 7) << 16 - | *(tmpData + 8) << 8 | *(tmpData + 9); - parcel->writeInt32(rgba); - - tmpData += 10; - parcel->writeInt32(KEY_STRUCT_TEXT_POS); - parcel->writeInt32(U16_AT(tmpData)); - parcel->writeInt32(U16_AT(tmpData + 2)); - parcel->writeInt32(U16_AT(tmpData + 4)); - parcel->writeInt32(U16_AT(tmpData + 6)); - - tmpData += 8; - parcel->writeInt32(KEY_STRUCT_STYLE_LIST); - parcel->writeInt32(KEY_START_CHAR); - parcel->writeInt32(U16_AT(tmpData)); - - parcel->writeInt32(KEY_END_CHAR); - parcel->writeInt32(U16_AT(tmpData + 2)); - - parcel->writeInt32(KEY_FONT_ID); - parcel->writeInt32(U16_AT(tmpData + 4)); - - parcel->writeInt32(KEY_STYLE_FLAGS); - parcel->writeInt32(*(tmpData + 6)); - - parcel->writeInt32(KEY_FONT_SIZE); - parcel->writeInt32(*(tmpData + 7)); - - parcel->writeInt32(KEY_TEXT_COLOR_RGBA); - rgba = *(tmpData + 8) << 24 | *(tmpData + 9) << 16 - | *(tmpData + 10) << 8 | *(tmpData + 11); - parcel->writeInt32(rgba); - - tmpData += 12; - parcel->writeInt32(KEY_STRUCT_FONT_LIST); - uint16_t count = U16_AT(tmpData); - parcel->writeInt32(count); - - tmpData += 2; - for (int i = 0; i < count; i++) { - // font ID - parcel->writeInt32(U16_AT(tmpData)); + for (int i = 0; i < count; i++) { + if (remaining < 3) { + // roll back + parcel->setDataPosition(dataPos); + return OK; + } + // font ID + parcel->writeInt32(U16_AT(tmpData)); - // font name length - parcel->writeInt32(*(tmpData + 2)); + // font name length + parcel->writeInt32(*(tmpData + 2)); - int len = *(tmpData + 2); + size_t len = *(tmpData + 2); - parcel->write(tmpData + 3, len); - tmpData += 3 + len; - } + tmpData += 3; + remaining -= 3; - break; - } - default: - { - break; - } - } + if (remaining < len) { + // roll back + parcel->setDataPosition(dataPos); + return OK; + } - data += chunkSize; - size -= chunkSize; + parcel->write(tmpData, len); + tmpData += len; + remaining -= len; + } - if (size > 0) { - // continue to extract next 'tx3g' - return extract3GPPGlobalDescriptions(data, size, parcel, 1); + // there is a "DisparityBox" after this according to the spec, but we ignore it + break; + } + default: + { + break; + } + } + + data += chunkSize; + size -= chunkSize; } return OK; diff --git a/media/libstagefright/timedtext/TextDescriptions.h b/media/libstagefright/timedtext/TextDescriptions.h index 0144917..bf67f3f 100644 --- a/media/libstagefright/timedtext/TextDescriptions.h +++ b/media/libstagefright/timedtext/TextDescriptions.h @@ -72,10 +72,10 @@ private: int timeMs, Parcel *parcel); static status_t extract3GPPGlobalDescriptions( const uint8_t *data, ssize_t size, - Parcel *parcel, int depth); + Parcel *parcel); static status_t extract3GPPLocalDescriptions( const uint8_t *data, ssize_t size, - int timeMs, Parcel *parcel, int depth); + int timeMs, Parcel *parcel); DISALLOW_EVIL_CONSTRUCTORS(TextDescriptions); }; -- cgit v1.1 From cf75af8f76265fb2909028f5dc68c7029dbe5f49 Mon Sep 17 00:00:00 2001 From: Lajos Molnar Date: Mon, 31 Aug 2015 17:19:52 -0700 Subject: stagefright: MPEG4Extractor: allow 'hdlr' box before first track Bug: 21725583 Change-Id: I799c1967759c7e49fb50281a1708188450caac77 --- media/libstagefright/MPEG4Extractor.cpp | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) (limited to 'media') diff --git a/media/libstagefright/MPEG4Extractor.cpp b/media/libstagefright/MPEG4Extractor.cpp index a76334f..38ae6f3 100755 --- a/media/libstagefright/MPEG4Extractor.cpp +++ b/media/libstagefright/MPEG4Extractor.cpp @@ -1952,15 +1952,14 @@ status_t MPEG4Extractor::parseChunk(off64_t *offset, int depth) { return ERROR_IO; } - if (mLastTrack == NULL) - return ERROR_MALFORMED; - uint32_t type = ntohl(buffer); // For the 3GPP file format, the handler-type within the 'hdlr' box // shall be 'text'. We also want to support 'sbtl' handler type // for a practical reason as various MPEG4 containers use it. if (type == FOURCC('t', 'e', 'x', 't') || type == FOURCC('s', 'b', 't', 'l')) { - mLastTrack->meta->setCString(kKeyMIMEType, MEDIA_MIMETYPE_TEXT_3GPP); + if (mLastTrack != NULL) { + mLastTrack->meta->setCString(kKeyMIMEType, MEDIA_MIMETYPE_TEXT_3GPP); + } } break; -- cgit v1.1 From 7bb772e0c643ff3292599cf485b9dbf232bf39a4 Mon Sep 17 00:00:00 2001 From: Wei Jia Date: Tue, 1 Sep 2015 11:14:18 -0700 Subject: libstagefright: sanity check size before dereferencing pointer in Utils.cpp Also remove some CHECK's. Bug: 23680780 Change-Id: I62d0941e203e40209fa6fbe3f923f3efdc5a6c23 --- media/libstagefright/Utils.cpp | 36 ++++++++++++++++++++++++++++-------- 1 file changed, 28 insertions(+), 8 deletions(-) (limited to 'media') diff --git a/media/libstagefright/Utils.cpp b/media/libstagefright/Utils.cpp index f0a7277..7c8d441 100644 --- a/media/libstagefright/Utils.cpp +++ b/media/libstagefright/Utils.cpp @@ -211,8 +211,10 @@ status_t convertMetaDataToMessage( const uint8_t *ptr = (const uint8_t *)data; - CHECK(size >= 7); - CHECK_EQ((unsigned)ptr[0], 1u); // configurationVersion == 1 + if (size < 7 || ptr[0] != 1) { // configurationVersion == 1 + ALOGE("b/23680780"); + return BAD_VALUE; + } uint8_t profile __unused = ptr[1]; uint8_t level __unused = ptr[3]; @@ -238,7 +240,10 @@ status_t convertMetaDataToMessage( buffer->setRange(0, 0); for (size_t i = 0; i < numSeqParameterSets; ++i) { - CHECK(size >= 2); + if (size < 2) { + ALOGE("b/23680780"); + return BAD_VALUE; + } size_t length = U16_AT(ptr); ptr += 2; @@ -267,13 +272,19 @@ status_t convertMetaDataToMessage( } buffer->setRange(0, 0); - CHECK(size >= 1); + if (size < 1) { + ALOGE("b/23680780"); + return BAD_VALUE; + } size_t numPictureParameterSets = *ptr; ++ptr; --size; for (size_t i = 0; i < numPictureParameterSets; ++i) { - CHECK(size >= 2); + if (size < 2) { + ALOGE("b/23680780"); + return BAD_VALUE; + } size_t length = U16_AT(ptr); ptr += 2; @@ -297,8 +308,10 @@ status_t convertMetaDataToMessage( } else if (meta->findData(kKeyHVCC, &type, &data, &size)) { const uint8_t *ptr = (const uint8_t *)data; - CHECK(size >= 7); - CHECK_EQ((unsigned)ptr[0], 1u); // configurationVersion == 1 + if (size < 23 || ptr[0] != 1) { // configurationVersion == 1 + ALOGE("b/23680780"); + return BAD_VALUE; + } uint8_t profile __unused = ptr[1] & 31; uint8_t level __unused = ptr[12]; ptr += 22; @@ -317,6 +330,10 @@ status_t convertMetaDataToMessage( buffer->setRange(0, 0); for (i = 0; i < numofArrays; i++) { + if (size < 3) { + ALOGE("b/23680780"); + return BAD_VALUE; + } ptr += 1; size -= 1; @@ -327,7 +344,10 @@ status_t convertMetaDataToMessage( size -= 2; for (j = 0; j < numofNals; j++) { - CHECK(size >= 2); + if (size < 2) { + ALOGE("b/23680780"); + return BAD_VALUE; + } size_t length = U16_AT(ptr); ptr += 2; -- cgit v1.1 From 7d05308b16a688436331de2e94d89e46d05d8d1d Mon Sep 17 00:00:00 2001 From: Andy Hung Date: Thu, 27 Aug 2015 16:18:59 -0700 Subject: NuPlayerRenderer: Do not deliver audio too soon after stop For non-offloaded audio, do not deliver audio data too soon after stop when in paused mode. Otherwise the audio MixerThread will keep the track playing, instead of inactivating the track. Bug: 23167401 Change-Id: If376148c742fde2d20dc5d23bf0b894fe378e71a (cherry picked from commit b03dcb34cd44d77e5fe1559e72323e03c59931db) --- .../nuplayer/NuPlayerRenderer.cpp | 19 ++++++++++++++++++- .../libmediaplayerservice/nuplayer/NuPlayerRenderer.h | 1 + 2 files changed, 19 insertions(+), 1 deletion(-) (limited to 'media') diff --git a/media/libmediaplayerservice/nuplayer/NuPlayerRenderer.cpp b/media/libmediaplayerservice/nuplayer/NuPlayerRenderer.cpp index 04a46f4..de7f5e7 100644 --- a/media/libmediaplayerservice/nuplayer/NuPlayerRenderer.cpp +++ b/media/libmediaplayerservice/nuplayer/NuPlayerRenderer.cpp @@ -106,6 +106,7 @@ NuPlayer::Renderer::Renderer( mNotifyCompleteVideo(false), mSyncQueues(false), mPaused(false), + mPauseDrainAudioAllowedUs(0), mVideoSampleReceived(false), mVideoRenderingStarted(false), mVideoRenderingStartGeneration(0), @@ -630,6 +631,14 @@ void NuPlayer::Renderer::postDrainAudioQueue_l(int64_t delayUs) { return; } + // FIXME: if paused, wait until AudioTrack stop() is complete before delivering data. + if (mPaused) { + const int64_t diffUs = mPauseDrainAudioAllowedUs - ALooper::GetNowUs(); + if (diffUs > delayUs) { + delayUs = diffUs; + } + } + mDrainAudioQueuePending = true; sp msg = new AMessage(kWhatDrainAudioQueue, this); msg->setInt32("drainGeneration", mAudioDrainGeneration); @@ -1338,8 +1347,16 @@ void NuPlayer::Renderer::onFlush(const sp &msg) { mAudioSink->flush(); // Call stop() to signal to the AudioSink to completely fill the // internal buffer before resuming playback. + // FIXME: this is ignored after flush(). mAudioSink->stop(); - if (!mPaused) { + if (mPaused) { + // Race condition: if renderer is paused and audio sink is stopped, + // we need to make sure that the audio track buffer fully drains + // before delivering data. + // FIXME: remove this if we can detect if stop() is complete. + const int delayUs = 2 * 50 * 1000; // (2 full mixer thread cycles at 50ms) + mPauseDrainAudioAllowedUs = ALooper::GetNowUs() + delayUs; + } else { mAudioSink->start(); } mNumFramesWritten = 0; diff --git a/media/libmediaplayerservice/nuplayer/NuPlayerRenderer.h b/media/libmediaplayerservice/nuplayer/NuPlayerRenderer.h index 3e65649..87bcbf9 100644 --- a/media/libmediaplayerservice/nuplayer/NuPlayerRenderer.h +++ b/media/libmediaplayerservice/nuplayer/NuPlayerRenderer.h @@ -170,6 +170,7 @@ private: // modified on only renderer's thread. bool mPaused; + int64_t mPauseDrainAudioAllowedUs; // time when we can drain/deliver audio in pause mode. bool mVideoSampleReceived; bool mVideoRenderingStarted; -- cgit v1.1 From 25a634427dec455b79d73562131985ae85b98c43 Mon Sep 17 00:00:00 2001 From: Andy Hung Date: Tue, 1 Sep 2015 20:07:56 +0000 Subject: Make IEffect command more robust (second try) Bug: 23540907 Change-Id: If30cfa535ad51521053706fc40fc98d893db5bc7 (cherry picked from commit 10e6660cc5da65b027c90489ba7ac55d1504e012) --- media/libmedia/IEffect.cpp | 27 ++++++++++++++++++++------- 1 file changed, 20 insertions(+), 7 deletions(-) (limited to 'media') diff --git a/media/libmedia/IEffect.cpp b/media/libmedia/IEffect.cpp index 86c706a..1e54ca8 100644 --- a/media/libmedia/IEffect.cpp +++ b/media/libmedia/IEffect.cpp @@ -85,13 +85,15 @@ public: data.writeInt32(size); status_t status = remote()->transact(COMMAND, data, &reply); + if (status == NO_ERROR) { + status = reply.readInt32(); + } if (status != NO_ERROR) { if (pReplySize != NULL) *pReplySize = 0; return status; } - status = reply.readInt32(); size = reply.readInt32(); if (size != 0 && pReplyData != NULL && pReplySize != NULL) { reply.read(pReplyData, size); @@ -152,6 +154,10 @@ status_t BnEffect::onTransact( char *cmd = NULL; if (cmdSize) { cmd = (char *)calloc(cmdSize, 1); + if (cmd == NULL) { + reply->writeInt32(NO_MEMORY); + return NO_ERROR; + } data.read(cmd, cmdSize); } uint32_t replySize = data.readInt32(); @@ -159,15 +165,22 @@ status_t BnEffect::onTransact( char *resp = NULL; if (replySize) { resp = (char *)calloc(replySize, 1); + if (resp == NULL) { + free(cmd); + reply->writeInt32(NO_MEMORY); + return NO_ERROR; + } } status_t status = command(cmdCode, cmdSize, cmd, &replySz, resp); reply->writeInt32(status); - if (replySz < replySize) { - replySize = replySz; - } - reply->writeInt32(replySize); - if (replySize) { - reply->write(resp, replySize); + if (status == NO_ERROR) { + if (replySz < replySize) { + replySize = replySz; + } + reply->writeInt32(replySize); + if (replySize) { + reply->write(resp, replySize); + } } if (cmd) { free(cmd); -- cgit v1.1 From 7665f5886093e1aee07a5266b8c384e5d1186f34 Mon Sep 17 00:00:00 2001 From: Ronghua Wu Date: Wed, 2 Sep 2015 10:15:30 -0700 Subject: nuplayer: let non-offload AudioSink to handle the reconnect when there's video. Bug: 23707144 Change-Id: Ie0e2db8dc91a78f82fb935d165aa11abe73697a3 --- media/libmediaplayerservice/nuplayer/NuPlayerRenderer.cpp | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) (limited to 'media') diff --git a/media/libmediaplayerservice/nuplayer/NuPlayerRenderer.cpp b/media/libmediaplayerservice/nuplayer/NuPlayerRenderer.cpp index 04a46f4..4de08ff 100644 --- a/media/libmediaplayerservice/nuplayer/NuPlayerRenderer.cpp +++ b/media/libmediaplayerservice/nuplayer/NuPlayerRenderer.cpp @@ -877,6 +877,8 @@ bool NuPlayer::Renderer::onDrainAudioQueue() { ALOGV("AudioSink write would block when writing %zu bytes", copy); } else { ALOGE("AudioSink write error(%zd) when writing %zu bytes", written, copy); + // This can only happen when AudioSink was opened with doNotReconnect flag set to + // true, in which case the NuPlayer will handle the reconnect. notifyAudioTearDown(); } break; @@ -1764,6 +1766,12 @@ status_t NuPlayer::Renderer::onOpenAudioSink( const uint32_t frameCount = (unsigned long long)sampleRate * getAudioSinkPcmMsSetting() / 1000; + // The doNotReconnect means AudioSink will signal back and let NuPlayer to re-construct + // AudioSink. We don't want this when there's video because it will cause a video seek to + // the previous I frame. But we do want this when there's only audio because it will give + // NuPlayer a chance to switch from non-offload mode to offload mode. + // So we only set doNotReconnect when there's no video. + const bool doNotReconnect = !hasVideo; status_t err = mAudioSink->open( sampleRate, numChannels, @@ -1774,7 +1782,7 @@ status_t NuPlayer::Renderer::onOpenAudioSink( mUseAudioCallback ? this : NULL, (audio_output_flags_t)pcmFlags, NULL, - true /* doNotReconnect */, + doNotReconnect, frameCount); if (err == OK) { err = mAudioSink->setPlaybackRate(mPlaybackSettings); -- cgit v1.1 From 892354335d49f0b9fcd10e20e0c13e3cd0f1f1cb Mon Sep 17 00:00:00 2001 From: Robert Shih Date: Wed, 2 Sep 2015 16:46:59 -0700 Subject: Zero out return values in media binder calls More specifically when handling: * GET_STREAM_VOLUME in IAudioPolicyService, and * GET_CURRENT_POSITION and GET_DURATION in IMediaPlayer This prevents leaking uninitialized values across binder in error cases. Bug: 23756261 Change-Id: I0ffd900ab12b685b0611259ade4a3efb1ec5defe --- media/libmedia/IAudioPolicyService.cpp | 2 +- media/libmedia/IMediaPlayer.cpp | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) (limited to 'media') diff --git a/media/libmedia/IAudioPolicyService.cpp b/media/libmedia/IAudioPolicyService.cpp index 9ffc486..0dab7e3 100644 --- a/media/libmedia/IAudioPolicyService.cpp +++ b/media/libmedia/IAudioPolicyService.cpp @@ -567,7 +567,7 @@ status_t BnAudioPolicyService::onTransact( audio_stream_type_t stream = static_cast (data.readInt32()); audio_devices_t device = static_cast (data.readInt32()); - int index; + int index = 0; status_t status = getStreamVolumeIndex(stream, &index, device); reply->writeInt32(index); reply->writeInt32(static_cast (status)); diff --git a/media/libmedia/IMediaPlayer.cpp b/media/libmedia/IMediaPlayer.cpp index 9b57902..2a5c7a7 100644 --- a/media/libmedia/IMediaPlayer.cpp +++ b/media/libmedia/IMediaPlayer.cpp @@ -423,7 +423,7 @@ status_t BnMediaPlayer::onTransact( } break; case GET_CURRENT_POSITION: { CHECK_INTERFACE(IMediaPlayer, data, reply); - int msec; + int msec = 0; status_t ret = getCurrentPosition(&msec); reply->writeInt32(msec); reply->writeInt32(ret); @@ -431,7 +431,7 @@ status_t BnMediaPlayer::onTransact( } break; case GET_DURATION: { CHECK_INTERFACE(IMediaPlayer, data, reply); - int msec; + int msec = 0; status_t ret = getDuration(&msec); reply->writeInt32(msec); reply->writeInt32(ret); -- cgit v1.1 From a946d844a77906072f5eb7093d41db465d6514bb Mon Sep 17 00:00:00 2001 From: Robert Shih Date: Wed, 2 Sep 2015 16:46:59 -0700 Subject: Zero out return values in audio binder calls More specifically when handling GET_OUTPUT_FOR_ATTR in IAudioPolicyService. This prevents leaking a uninitialized `output` across binder if getOutputForAttr were to encounter errors. Bug: 23756261 Change-Id: Ibff8a1249a4e8a3c89a33a540dda428b10d6ca82 --- media/libmedia/IAudioPolicyService.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'media') diff --git a/media/libmedia/IAudioPolicyService.cpp b/media/libmedia/IAudioPolicyService.cpp index 12efa8a..d91b73b 100644 --- a/media/libmedia/IAudioPolicyService.cpp +++ b/media/libmedia/IAudioPolicyService.cpp @@ -816,7 +816,7 @@ status_t BnAudioPolicyService::onTransact( if (hasOffloadInfo) { data.read(&offloadInfo, sizeof(audio_offload_info_t)); } - audio_io_handle_t output; + audio_io_handle_t output = 0; status_t status = getOutputForAttr(hasAttributes ? &attr : NULL, &output, session, &stream, samplingRate, format, channelMask, -- cgit v1.1 From 4d7ac854c5a45d0e3af3d0af78b5a8c9807cbec6 Mon Sep 17 00:00:00 2001 From: Wei Jia Date: Mon, 31 Aug 2015 18:33:14 -0700 Subject: NuPlayerRenderer: avoid divison by zero when sample rate is 0. Also close AudioSink when failing to set playback rate. Bug: 23624664 Change-Id: I5bf8bcca4a21c26fb52821db597d61f7f1273d5c --- media/libmediaplayerservice/nuplayer/NuPlayerRenderer.cpp | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'media') diff --git a/media/libmediaplayerservice/nuplayer/NuPlayerRenderer.cpp b/media/libmediaplayerservice/nuplayer/NuPlayerRenderer.cpp index 4de08ff..0ac29a8 100644 --- a/media/libmediaplayerservice/nuplayer/NuPlayerRenderer.cpp +++ b/media/libmediaplayerservice/nuplayer/NuPlayerRenderer.cpp @@ -945,6 +945,10 @@ bool NuPlayer::Renderer::onDrainAudioQueue() { int64_t NuPlayer::Renderer::getDurationUsIfPlayedAtSampleRate(uint32_t numFrames) { int32_t sampleRate = offloadingAudio() ? mCurrentOffloadInfo.sample_rate : mCurrentPcmInfo.mSampleRate; + if (sampleRate == 0) { + ALOGE("sampleRate is 0 in %s mode", offloadingAudio() ? "offload" : "non-offload"); + return 0; + } // TODO: remove the (int32_t) casting below as it may overflow at 12.4 hours. return (int64_t)((int32_t)numFrames * 1000000LL / sampleRate); } @@ -1789,6 +1793,7 @@ status_t NuPlayer::Renderer::onOpenAudioSink( } if (err != OK) { ALOGW("openAudioSink: non offloaded open failed status: %d", err); + mAudioSink->close(); mCurrentPcmInfo = AUDIO_PCMINFO_INITIALIZER; return err; } -- cgit v1.1 From 76483691ea93aed0433dee050abfc9fa934c4f62 Mon Sep 17 00:00:00 2001 From: Wonsik Kim Date: Wed, 2 Sep 2015 16:02:19 +0900 Subject: Ogg: avoid size_t overflow in base64 decoding Bug: 23707088 Change-Id: I8d32841fee3213c721cdcc57788807ea64d19d74 --- media/libstagefright/OggExtractor.cpp | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) (limited to 'media') diff --git a/media/libstagefright/OggExtractor.cpp b/media/libstagefright/OggExtractor.cpp index 6fba8e1..d5c929e 100644 --- a/media/libstagefright/OggExtractor.cpp +++ b/media/libstagefright/OggExtractor.cpp @@ -1220,11 +1220,14 @@ static uint8_t *DecodeBase64(const char *s, size_t size, size_t *outSize) { } } - size_t outLen = 3 * size / 4 - padding; - - *outSize = outLen; + // We divide first to avoid overflow. It's OK to do this because we + // already made sure that size % 4 == 0. + size_t outLen = (size / 4) * 3 - padding; void *buffer = malloc(outLen); + if (buffer == NULL) { + return NULL; + } uint8_t *out = (uint8_t *)buffer; size_t j = 0; @@ -1243,10 +1246,10 @@ static uint8_t *DecodeBase64(const char *s, size_t size, size_t *outSize) { } else if (c == '/') { value = 63; } else if (c != '=') { - return NULL; + break; } else { if (i < n - padding) { - return NULL; + break; } value = 0; @@ -1264,6 +1267,13 @@ static uint8_t *DecodeBase64(const char *s, size_t size, size_t *outSize) { } } + // Check if we exited the loop early. + if (j < outLen) { + free(buffer); + return NULL; + } + + *outSize = outLen; return (uint8_t *)buffer; } -- cgit v1.1 From d26052738f7b095b7e318c8dde7f32db0a48450c Mon Sep 17 00:00:00 2001 From: Wonsik Kim Date: Wed, 2 Sep 2015 16:02:19 +0900 Subject: Ogg: avoid size_t overflow in base64 decoding Bug: 23707088 Change-Id: I8d32841fee3213c721cdcc57788807ea64d19d74 --- media/libstagefright/OggExtractor.cpp | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) (limited to 'media') diff --git a/media/libstagefright/OggExtractor.cpp b/media/libstagefright/OggExtractor.cpp index 073c53f..28c9c7c 100644 --- a/media/libstagefright/OggExtractor.cpp +++ b/media/libstagefright/OggExtractor.cpp @@ -844,11 +844,14 @@ static uint8_t *DecodeBase64(const char *s, size_t size, size_t *outSize) { } } - size_t outLen = 3 * size / 4 - padding; - - *outSize = outLen; + // We divide first to avoid overflow. It's OK to do this because we + // already made sure that size % 4 == 0. + size_t outLen = (size / 4) * 3 - padding; void *buffer = malloc(outLen); + if (buffer == NULL) { + return NULL; + } uint8_t *out = (uint8_t *)buffer; size_t j = 0; @@ -867,10 +870,10 @@ static uint8_t *DecodeBase64(const char *s, size_t size, size_t *outSize) { } else if (c == '/') { value = 63; } else if (c != '=') { - return NULL; + break; } else { if (i < n - padding) { - return NULL; + break; } value = 0; @@ -888,6 +891,13 @@ static uint8_t *DecodeBase64(const char *s, size_t size, size_t *outSize) { } } + // Check if we exited the loop early. + if (j < outLen) { + free(buffer); + return NULL; + } + + *outSize = outLen; return (uint8_t *)buffer; } -- cgit v1.1 From 9d916c771ca32cb2d0df27b85ce3e17bb6b48eaf Mon Sep 17 00:00:00 2001 From: Wei Jia Date: Tue, 1 Sep 2015 11:14:18 -0700 Subject: DO NOT MERGE - libstagefright: sanity check size before dereferencing pointer in Utils.cpp Also remove some CHECK's. Bug: 23680780 Change-Id: I62d0941e203e40209fa6fbe3f923f3efdc5a6c23 (cherry picked from commit 7bb772e0c643ff3292599cf485b9dbf232bf39a4) --- media/libstagefright/Utils.cpp | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) (limited to 'media') diff --git a/media/libstagefright/Utils.cpp b/media/libstagefright/Utils.cpp index 80d8cef..090c891 100644 --- a/media/libstagefright/Utils.cpp +++ b/media/libstagefright/Utils.cpp @@ -160,8 +160,10 @@ status_t convertMetaDataToMessage( const uint8_t *ptr = (const uint8_t *)data; - CHECK(size >= 7); - CHECK_EQ((unsigned)ptr[0], 1u); // configurationVersion == 1 + if (size < 7 || ptr[0] != 1) { // configurationVersion == 1 + ALOGE("b/23680780"); + return BAD_VALUE; + } uint8_t profile = ptr[1]; uint8_t level = ptr[3]; @@ -187,7 +189,10 @@ status_t convertMetaDataToMessage( buffer->setRange(0, 0); for (size_t i = 0; i < numSeqParameterSets; ++i) { - CHECK(size >= 2); + if (size < 2) { + ALOGE("b/23680780"); + return BAD_VALUE; + } size_t length = U16_AT(ptr); ptr += 2; @@ -216,13 +221,19 @@ status_t convertMetaDataToMessage( } buffer->setRange(0, 0); - CHECK(size >= 1); + if (size < 1) { + ALOGE("b/23680780"); + return BAD_VALUE; + } size_t numPictureParameterSets = *ptr; ++ptr; --size; for (size_t i = 0; i < numPictureParameterSets; ++i) { - CHECK(size >= 2); + if (size < 2) { + ALOGE("b/23680780"); + return BAD_VALUE; + } size_t length = U16_AT(ptr); ptr += 2; -- cgit v1.1 From c6a2815eadfce62702d58b3fa3887f24c49e1864 Mon Sep 17 00:00:00 2001 From: Wei Jia Date: Tue, 1 Sep 2015 11:14:18 -0700 Subject: DO NOT MERGE - libstagefright: sanity check size before dereferencing pointer in Utils.cpp Also remove some CHECK's. Bug: 23680780 Change-Id: I62d0941e203e40209fa6fbe3f923f3efdc5a6c23 (cherry picked from commit 7bb772e0c643ff3292599cf485b9dbf232bf39a4) --- media/libstagefright/Utils.cpp | 36 ++++++++++++++++++++++++++++-------- 1 file changed, 28 insertions(+), 8 deletions(-) (limited to 'media') diff --git a/media/libstagefright/Utils.cpp b/media/libstagefright/Utils.cpp index d86be6e..214e2fc 100644 --- a/media/libstagefright/Utils.cpp +++ b/media/libstagefright/Utils.cpp @@ -196,8 +196,10 @@ status_t convertMetaDataToMessage( const uint8_t *ptr = (const uint8_t *)data; - CHECK(size >= 7); - CHECK_EQ((unsigned)ptr[0], 1u); // configurationVersion == 1 + if (size < 7 || ptr[0] != 1) { // configurationVersion == 1 + ALOGE("b/23680780"); + return BAD_VALUE; + } uint8_t profile = ptr[1]; uint8_t level = ptr[3]; @@ -223,7 +225,10 @@ status_t convertMetaDataToMessage( buffer->setRange(0, 0); for (size_t i = 0; i < numSeqParameterSets; ++i) { - CHECK(size >= 2); + if (size < 2) { + ALOGE("b/23680780"); + return BAD_VALUE; + } size_t length = U16_AT(ptr); ptr += 2; @@ -252,13 +257,19 @@ status_t convertMetaDataToMessage( } buffer->setRange(0, 0); - CHECK(size >= 1); + if (size < 1) { + ALOGE("b/23680780"); + return BAD_VALUE; + } size_t numPictureParameterSets = *ptr; ++ptr; --size; for (size_t i = 0; i < numPictureParameterSets; ++i) { - CHECK(size >= 2); + if (size < 2) { + ALOGE("b/23680780"); + return BAD_VALUE; + } size_t length = U16_AT(ptr); ptr += 2; @@ -282,8 +293,10 @@ status_t convertMetaDataToMessage( } else if (meta->findData(kKeyHVCC, &type, &data, &size)) { const uint8_t *ptr = (const uint8_t *)data; - CHECK(size >= 7); - CHECK_EQ((unsigned)ptr[0], 1u); // configurationVersion == 1 + if (size < 23 || ptr[0] != 1) { // configurationVersion == 1 + ALOGE("b/23680780"); + return BAD_VALUE; + } uint8_t profile = ptr[1] & 31; uint8_t level = ptr[12]; ptr += 22; @@ -302,6 +315,10 @@ status_t convertMetaDataToMessage( buffer->setRange(0, 0); for (i = 0; i < numofArrays; i++) { + if (size < 3) { + ALOGE("b/23680780"); + return BAD_VALUE; + } ptr += 1; size -= 1; @@ -312,7 +329,10 @@ status_t convertMetaDataToMessage( size -= 2; for (j = 0; j < numofNals; j++) { - CHECK(size >= 2); + if (size < 2) { + ALOGE("b/23680780"); + return BAD_VALUE; + } size_t length = U16_AT(ptr); ptr += 2; -- cgit v1.1 From 715dcb9c90d86c1a02a0da056f3cee8875ad1230 Mon Sep 17 00:00:00 2001 From: Wei Jia Date: Fri, 4 Sep 2015 09:13:37 -0700 Subject: libstagefright: fix A_Refl to return immediately when there is an error. Bug: 23609206 Change-Id: I2ad25fb208df17f5a5b6d6b356eff2f400627f22 --- media/libstagefright/codecs/amrnb/dec/Android.mk | 2 +- media/libstagefright/codecs/amrnb/dec/src/a_refl.cpp | 5 ++++- 2 files changed, 5 insertions(+), 2 deletions(-) (limited to 'media') diff --git a/media/libstagefright/codecs/amrnb/dec/Android.mk b/media/libstagefright/codecs/amrnb/dec/Android.mk index 3750e2e..76a7f40 100644 --- a/media/libstagefright/codecs/amrnb/dec/Android.mk +++ b/media/libstagefright/codecs/amrnb/dec/Android.mk @@ -98,7 +98,7 @@ LOCAL_STATIC_LIBRARIES := \ libstagefright_amrnbdec libsndfile LOCAL_SHARED_LIBRARIES := \ - libstagefright_amrnb_common libaudioutils + libstagefright_amrnb_common libaudioutils liblog LOCAL_MODULE := libstagefright_amrnbdec_test LOCAL_MODULE_TAGS := optional diff --git a/media/libstagefright/codecs/amrnb/dec/src/a_refl.cpp b/media/libstagefright/codecs/amrnb/dec/src/a_refl.cpp index fb7cff3..696d2da 100644 --- a/media/libstagefright/codecs/amrnb/dec/src/a_refl.cpp +++ b/media/libstagefright/codecs/amrnb/dec/src/a_refl.cpp @@ -59,6 +59,8 @@ terms listed above has been obtained from the copyright holder. /*---------------------------------------------------------------------------- ; INCLUDES ----------------------------------------------------------------------------*/ +#include + #include "a_refl.h" #include "typedef.h" #include "cnst.h" @@ -291,7 +293,8 @@ void A_Refl( { refl[i] = 0; } - break; + ALOGE("b/23609206"); + return; } bState[j] = extract_l(L_temp); -- cgit v1.1 From 9c99c92af255ae64fe222245f15ad30b92a1fc8c Mon Sep 17 00:00:00 2001 From: Wonsik Kim Date: Mon, 7 Sep 2015 15:52:27 +0900 Subject: Avoid size_t overflow in base64 decoding once again Switch to foundation base64 function in OggExtractor and fix the issue there. Bug: 23707088 Change-Id: I999ae911177c88dc13f9ee9796ca93c5928b20b0 --- media/libstagefright/OggExtractor.cpp | 102 ++++------------------------- media/libstagefright/foundation/base64.cpp | 11 +++- 2 files changed, 20 insertions(+), 93 deletions(-) (limited to 'media') diff --git a/media/libstagefright/OggExtractor.cpp b/media/libstagefright/OggExtractor.cpp index d5c929e..578171f 100644 --- a/media/libstagefright/OggExtractor.cpp +++ b/media/libstagefright/OggExtractor.cpp @@ -23,6 +23,7 @@ #include #include #include +#include #include #include #include @@ -1202,93 +1203,18 @@ void parseVorbisComment( } -// The returned buffer should be free()d. -static uint8_t *DecodeBase64(const char *s, size_t size, size_t *outSize) { - *outSize = 0; - - if ((size % 4) != 0) { - return NULL; - } - - size_t n = size; - size_t padding = 0; - if (n >= 1 && s[n - 1] == '=') { - padding = 1; - - if (n >= 2 && s[n - 2] == '=') { - padding = 2; - } - } - - // We divide first to avoid overflow. It's OK to do this because we - // already made sure that size % 4 == 0. - size_t outLen = (size / 4) * 3 - padding; - - void *buffer = malloc(outLen); - if (buffer == NULL) { - return NULL; - } - - uint8_t *out = (uint8_t *)buffer; - size_t j = 0; - uint32_t accum = 0; - for (size_t i = 0; i < n; ++i) { - char c = s[i]; - unsigned value; - if (c >= 'A' && c <= 'Z') { - value = c - 'A'; - } else if (c >= 'a' && c <= 'z') { - value = 26 + c - 'a'; - } else if (c >= '0' && c <= '9') { - value = 52 + c - '0'; - } else if (c == '+') { - value = 62; - } else if (c == '/') { - value = 63; - } else if (c != '=') { - break; - } else { - if (i < n - padding) { - break; - } - - value = 0; - } - - accum = (accum << 6) | value; - - if (((i + 1) % 4) == 0) { - out[j++] = (accum >> 16); - - if (j < outLen) { out[j++] = (accum >> 8) & 0xff; } - if (j < outLen) { out[j++] = accum & 0xff; } - - accum = 0; - } - } - - // Check if we exited the loop early. - if (j < outLen) { - free(buffer); - return NULL; - } - - *outSize = outLen; - return (uint8_t *)buffer; -} - static void extractAlbumArt( const sp &fileMeta, const void *data, size_t size) { ALOGV("extractAlbumArt from '%s'", (const char *)data); - size_t flacSize; - uint8_t *flac = DecodeBase64((const char *)data, size, &flacSize); - - if (flac == NULL) { + sp flacBuffer = decodeBase64(AString((const char *)data, size)); + if (flacBuffer == NULL) { ALOGE("malformed base64 encoded data."); return; } + size_t flacSize = flacBuffer->size(); + uint8_t *flac = flacBuffer->data(); ALOGV("got flac of size %zu", flacSize); uint32_t picType; @@ -1298,24 +1224,24 @@ static void extractAlbumArt( char type[128]; if (flacSize < 8) { - goto exit; + return; } picType = U32_AT(flac); if (picType != 3) { // This is not a front cover. - goto exit; + return; } typeLen = U32_AT(&flac[4]); if (typeLen > sizeof(type) - 1) { - goto exit; + return; } // we've already checked above that flacSize >= 8 if (flacSize - 8 < typeLen) { - goto exit; + return; } memcpy(type, &flac[8], typeLen); @@ -1325,7 +1251,7 @@ static void extractAlbumArt( if (!strcmp(type, "-->")) { // This is not inline cover art, but an external url instead. - goto exit; + return; } descLen = U32_AT(&flac[8 + typeLen]); @@ -1333,7 +1259,7 @@ static void extractAlbumArt( if (flacSize < 32 || flacSize - 32 < typeLen || flacSize - 32 - typeLen < descLen) { - goto exit; + return; } dataLen = U32_AT(&flac[8 + typeLen + 4 + descLen + 16]); @@ -1341,7 +1267,7 @@ static void extractAlbumArt( // we've already checked above that (flacSize - 32 - typeLen - descLen) >= 0 if (flacSize - 32 - typeLen - descLen < dataLen) { - goto exit; + return; } ALOGV("got image data, %zu trailing bytes", @@ -1351,10 +1277,6 @@ static void extractAlbumArt( kKeyAlbumArt, 0, &flac[8 + typeLen + 4 + descLen + 20], dataLen); fileMeta->setCString(kKeyAlbumArtMIME, type); - -exit: - free(flac); - flac = NULL; } //////////////////////////////////////////////////////////////////////////////// diff --git a/media/libstagefright/foundation/base64.cpp b/media/libstagefright/foundation/base64.cpp index dcf5bef..7da7db9 100644 --- a/media/libstagefright/foundation/base64.cpp +++ b/media/libstagefright/foundation/base64.cpp @@ -22,11 +22,11 @@ namespace android { sp decodeBase64(const AString &s) { - if ((s.size() % 4) != 0) { + size_t n = s.size(); + if ((n % 4) != 0) { return NULL; } - size_t n = s.size(); size_t padding = 0; if (n >= 1 && s.c_str()[n - 1] == '=') { padding = 1; @@ -40,11 +40,16 @@ sp decodeBase64(const AString &s) { } } - size_t outLen = 3 * s.size() / 4 - padding; + // We divide first to avoid overflow. It's OK to do this because we + // already made sure that n % 4 == 0. + size_t outLen = (n / 4) * 3 - padding; sp buffer = new ABuffer(outLen); uint8_t *out = buffer->data(); + if (out == NULL || buffer->size() < outLen) { + return NULL; + } size_t j = 0; uint32_t accum = 0; for (size_t i = 0; i < n; ++i) { -- cgit v1.1 From 69ae6a87342d5260d8d8660accc8aa1b9367dbbe Mon Sep 17 00:00:00 2001 From: Wei Jia Date: Fri, 4 Sep 2015 09:13:37 -0700 Subject: libstagefright: fix A_Refl to return immediately when there is an error. Bug: 23609206 Change-Id: I2ad25fb208df17f5a5b6d6b356eff2f400627f22 (cherry picked from commit 715dcb9c90d86c1a02a0da056f3cee8875ad1230) --- media/libstagefright/codecs/amrnb/dec/src/a_refl.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) (limited to 'media') diff --git a/media/libstagefright/codecs/amrnb/dec/src/a_refl.cpp b/media/libstagefright/codecs/amrnb/dec/src/a_refl.cpp index fb7cff3..696d2da 100644 --- a/media/libstagefright/codecs/amrnb/dec/src/a_refl.cpp +++ b/media/libstagefright/codecs/amrnb/dec/src/a_refl.cpp @@ -59,6 +59,8 @@ terms listed above has been obtained from the copyright holder. /*---------------------------------------------------------------------------- ; INCLUDES ----------------------------------------------------------------------------*/ +#include + #include "a_refl.h" #include "typedef.h" #include "cnst.h" @@ -291,7 +293,8 @@ void A_Refl( { refl[i] = 0; } - break; + ALOGE("b/23609206"); + return; } bState[j] = extract_l(L_temp); -- cgit v1.1 From b946648cc63a4d328318b56215214ead575bc54a Mon Sep 17 00:00:00 2001 From: Robert Shih Date: Wed, 2 Sep 2015 14:02:47 -0700 Subject: Protect data source access with mutex during disconnect Bug: 23658148 Change-Id: Ic37cac7b5d166143e0b77e9919b0aaef486e4fdd --- .../nuplayer/GenericSource.cpp | 31 ++++++++++++++++------ .../libmediaplayerservice/nuplayer/GenericSource.h | 1 + 2 files changed, 24 insertions(+), 8 deletions(-) (limited to 'media') diff --git a/media/libmediaplayerservice/nuplayer/GenericSource.cpp b/media/libmediaplayerservice/nuplayer/GenericSource.cpp index 7dc9be7..b3eb5fd 100644 --- a/media/libmediaplayerservice/nuplayer/GenericSource.cpp +++ b/media/libmediaplayerservice/nuplayer/GenericSource.cpp @@ -467,9 +467,17 @@ void NuPlayer::GenericSource::finishPrepareAsync() { void NuPlayer::GenericSource::notifyPreparedAndCleanup(status_t err) { if (err != OK) { - mDataSource.clear(); - mCachedSource.clear(); - mHttpSource.clear(); + { + sp dataSource = mDataSource; + sp cachedSource = mCachedSource; + sp httpSource = mHttpSource; + { + Mutex::Autolock _l(mDisconnectLock); + mDataSource.clear(); + mCachedSource.clear(); + mHttpSource.clear(); + } + } mBitrate = -1; cancelPollBuffering(); @@ -522,13 +530,20 @@ void NuPlayer::GenericSource::resume() { } void NuPlayer::GenericSource::disconnect() { - if (mDataSource != NULL) { + sp dataSource, httpSource; + { + Mutex::Autolock _l(mDisconnectLock); + dataSource = mDataSource; + httpSource = mHttpSource; + } + + if (dataSource != NULL) { // disconnect data source - if (mDataSource->flags() & DataSource::kIsCachingDataSource) { - static_cast(mDataSource.get())->disconnect(); + if (dataSource->flags() & DataSource::kIsCachingDataSource) { + static_cast(dataSource.get())->disconnect(); } - } else if (mHttpSource != NULL) { - static_cast(mHttpSource.get())->disconnect(); + } else if (httpSource != NULL) { + static_cast(httpSource.get())->disconnect(); } } diff --git a/media/libmediaplayerservice/nuplayer/GenericSource.h b/media/libmediaplayerservice/nuplayer/GenericSource.h index dc85d2d..ac980ef 100644 --- a/media/libmediaplayerservice/nuplayer/GenericSource.h +++ b/media/libmediaplayerservice/nuplayer/GenericSource.h @@ -153,6 +153,7 @@ private: int32_t mPrevBufferPercentage; mutable Mutex mReadBufferLock; + mutable Mutex mDisconnectLock; sp mLooper; -- cgit v1.1 From f3eb82683a80341f5ac23057aab733a57963cab2 Mon Sep 17 00:00:00 2001 From: Robert Shih Date: Wed, 2 Sep 2015 14:02:47 -0700 Subject: DO NOT MERGE: Protect data source access with mutex during disconnect Bug: 23658148 Change-Id: Ic37cac7b5d166143e0b77e9919b0aaef486e4fdd --- .../nuplayer/GenericSource.cpp | 31 ++++++++++++++++------ .../libmediaplayerservice/nuplayer/GenericSource.h | 1 + 2 files changed, 24 insertions(+), 8 deletions(-) (limited to 'media') diff --git a/media/libmediaplayerservice/nuplayer/GenericSource.cpp b/media/libmediaplayerservice/nuplayer/GenericSource.cpp index 6859a1a..b6f3a9f 100644 --- a/media/libmediaplayerservice/nuplayer/GenericSource.cpp +++ b/media/libmediaplayerservice/nuplayer/GenericSource.cpp @@ -374,9 +374,17 @@ void NuPlayer::GenericSource::notifyPreparedAndCleanup(status_t err) { mMetaDataSize = -1ll; mContentType = ""; mSniffedMIME = ""; - mDataSource.clear(); - mCachedSource.clear(); - mHttpSource.clear(); + { + sp dataSource = mDataSource; + sp cachedSource = mCachedSource; + sp httpSource = mHttpSource; + { + Mutex::Autolock _l(mDisconnectLock); + mDataSource.clear(); + mCachedSource.clear(); + mHttpSource.clear(); + } + } cancelPollBuffering(); } @@ -498,13 +506,20 @@ void NuPlayer::GenericSource::resume() { } void NuPlayer::GenericSource::disconnect() { - if (mDataSource != NULL) { + sp dataSource, httpSource; + { + Mutex::Autolock _l(mDisconnectLock); + dataSource = mDataSource; + httpSource = mHttpSource; + } + + if (dataSource != NULL) { // disconnect data source - if (mDataSource->flags() & DataSource::kIsCachingDataSource) { - static_cast(mDataSource.get())->disconnect(); + if (dataSource->flags() & DataSource::kIsCachingDataSource) { + static_cast(dataSource.get())->disconnect(); } - } else if (mHttpSource != NULL) { - static_cast(mHttpSource.get())->disconnect(); + } else if (httpSource != NULL) { + static_cast(httpSource.get())->disconnect(); } } diff --git a/media/libmediaplayerservice/nuplayer/GenericSource.h b/media/libmediaplayerservice/nuplayer/GenericSource.h index f8601ea..1b68283 100644 --- a/media/libmediaplayerservice/nuplayer/GenericSource.h +++ b/media/libmediaplayerservice/nuplayer/GenericSource.h @@ -141,6 +141,7 @@ private: int32_t mPollBufferingGeneration; uint32_t mPendingReadBufferTypes; mutable Mutex mReadBufferLock; + mutable Mutex mDisconnectLock; sp mLooper; -- cgit v1.1 From 316c3d929ffb004b0150d515e82aede02208ce97 Mon Sep 17 00:00:00 2001 From: Wonsik Kim Date: Tue, 8 Sep 2015 17:32:28 +0900 Subject: NuCachedSource2: fix possible erroneous early free Because the constructor of NuCachedSource2 sent a message to AHandlerReflector object, AHandlerReflector::onMessageReceived could have executed just before the object gets wrapped in a strong pointer, resulting in erroneous early free. Fix the issue by using static Create function to ensure the message is sent after the object is wrapped in a sp. Bug: 23882800 Change-Id: I38a9d7a3083f184b4c81d0b00ba1661721278855 --- media/libstagefright/AwesomePlayer.cpp | 4 ++-- media/libstagefright/DataSource.cpp | 2 +- media/libstagefright/NuCachedSource2.cpp | 15 ++++++++++++--- media/libstagefright/include/NuCachedSource2.h | 7 ++++++- 4 files changed, 21 insertions(+), 7 deletions(-) (limited to 'media') diff --git a/media/libstagefright/AwesomePlayer.cpp b/media/libstagefright/AwesomePlayer.cpp index 4e6c2a6..3cd0b0e 100644 --- a/media/libstagefright/AwesomePlayer.cpp +++ b/media/libstagefright/AwesomePlayer.cpp @@ -2290,11 +2290,11 @@ status_t AwesomePlayer::finishSetDataSource_l() { // The widevine extractor does its own caching. #if 0 - mCachedSource = new NuCachedSource2( + mCachedSource = NuCachedSource2::Create( new ThrottledSource( mConnectingDataSource, 50 * 1024 /* bytes/sec */)); #else - mCachedSource = new NuCachedSource2( + mCachedSource = NuCachedSource2::Create( mConnectingDataSource, cacheConfig.isEmpty() ? NULL : cacheConfig.string(), disconnectAtHighwatermark); diff --git a/media/libstagefright/DataSource.cpp b/media/libstagefright/DataSource.cpp index 75ef288..5020c6c 100644 --- a/media/libstagefright/DataSource.cpp +++ b/media/libstagefright/DataSource.cpp @@ -246,7 +246,7 @@ sp DataSource::CreateFromURI( *contentType = httpSource->getMIMEType(); } - source = new NuCachedSource2( + source = NuCachedSource2::Create( httpSource, cacheConfig.isEmpty() ? NULL : cacheConfig.string(), disconnectAtHighwatermark); diff --git a/media/libstagefright/NuCachedSource2.cpp b/media/libstagefright/NuCachedSource2.cpp index f82636b..d6255d6 100644 --- a/media/libstagefright/NuCachedSource2.cpp +++ b/media/libstagefright/NuCachedSource2.cpp @@ -224,9 +224,6 @@ NuCachedSource2::NuCachedSource2( // So whenever we call DataSource::readAt it may end up in a call to // IMediaHTTPConnection::readAt and therefore call back into JAVA. mLooper->start(false /* runOnCallingThread */, true /* canCallJava */); - - Mutex::Autolock autoLock(mLock); - (new AMessage(kWhatFetchMore, mReflector))->post(); } NuCachedSource2::~NuCachedSource2() { @@ -237,6 +234,18 @@ NuCachedSource2::~NuCachedSource2() { mCache = NULL; } +// static +sp NuCachedSource2::Create( + const sp &source, + const char *cacheConfig, + bool disconnectAtHighwatermark) { + sp instance = new NuCachedSource2( + source, cacheConfig, disconnectAtHighwatermark); + Mutex::Autolock autoLock(instance->mLock); + (new AMessage(kWhatFetchMore, instance->mReflector))->post(); + return instance; +} + status_t NuCachedSource2::getEstimatedBandwidthKbps(int32_t *kbps) { if (mSource->flags() & kIsHTTPBasedSource) { HTTPBase* source = static_cast(mSource.get()); diff --git a/media/libstagefright/include/NuCachedSource2.h b/media/libstagefright/include/NuCachedSource2.h index 4252706..a29bdf9 100644 --- a/media/libstagefright/include/NuCachedSource2.h +++ b/media/libstagefright/include/NuCachedSource2.h @@ -28,7 +28,7 @@ struct ALooper; struct PageCache; struct NuCachedSource2 : public DataSource { - NuCachedSource2( + static sp Create( const sp &source, const char *cacheConfig = NULL, bool disconnectAtHighwatermark = false); @@ -72,6 +72,11 @@ protected: private: friend struct AHandlerReflector; + NuCachedSource2( + const sp &source, + const char *cacheConfig, + bool disconnectAtHighwatermark); + enum { kPageSize = 65536, kDefaultHighWaterThreshold = 20 * 1024 * 1024, -- cgit v1.1 From c9ac5dfdafed1c66beae090cafa97002764e0ca3 Mon Sep 17 00:00:00 2001 From: Wonsik Kim Date: Mon, 7 Sep 2015 15:52:27 +0900 Subject: Avoid size_t overflow in base64 decoding once again Switch to foundation base64 function in OggExtractor and fix the issue there. Bug: 23707088 Change-Id: I999ae911177c88dc13f9ee9796ca93c5928b20b0 --- media/libstagefright/OggExtractor.cpp | 102 ++++------------------------- media/libstagefright/foundation/base64.cpp | 11 +++- 2 files changed, 20 insertions(+), 93 deletions(-) (limited to 'media') diff --git a/media/libstagefright/OggExtractor.cpp b/media/libstagefright/OggExtractor.cpp index 130f5a5..10dce3b 100644 --- a/media/libstagefright/OggExtractor.cpp +++ b/media/libstagefright/OggExtractor.cpp @@ -22,6 +22,7 @@ #include #include +#include #include #include #include @@ -830,93 +831,18 @@ void parseVorbisComment( } -// The returned buffer should be free()d. -static uint8_t *DecodeBase64(const char *s, size_t size, size_t *outSize) { - *outSize = 0; - - if ((size % 4) != 0) { - return NULL; - } - - size_t n = size; - size_t padding = 0; - if (n >= 1 && s[n - 1] == '=') { - padding = 1; - - if (n >= 2 && s[n - 2] == '=') { - padding = 2; - } - } - - // We divide first to avoid overflow. It's OK to do this because we - // already made sure that size % 4 == 0. - size_t outLen = (size / 4) * 3 - padding; - - void *buffer = malloc(outLen); - if (buffer == NULL) { - return NULL; - } - - uint8_t *out = (uint8_t *)buffer; - size_t j = 0; - uint32_t accum = 0; - for (size_t i = 0; i < n; ++i) { - char c = s[i]; - unsigned value; - if (c >= 'A' && c <= 'Z') { - value = c - 'A'; - } else if (c >= 'a' && c <= 'z') { - value = 26 + c - 'a'; - } else if (c >= '0' && c <= '9') { - value = 52 + c - '0'; - } else if (c == '+') { - value = 62; - } else if (c == '/') { - value = 63; - } else if (c != '=') { - break; - } else { - if (i < n - padding) { - break; - } - - value = 0; - } - - accum = (accum << 6) | value; - - if (((i + 1) % 4) == 0) { - out[j++] = (accum >> 16); - - if (j < outLen) { out[j++] = (accum >> 8) & 0xff; } - if (j < outLen) { out[j++] = accum & 0xff; } - - accum = 0; - } - } - - // Check if we exited the loop early. - if (j < outLen) { - free(buffer); - return NULL; - } - - *outSize = outLen; - return (uint8_t *)buffer; -} - static void extractAlbumArt( const sp &fileMeta, const void *data, size_t size) { ALOGV("extractAlbumArt from '%s'", (const char *)data); - size_t flacSize; - uint8_t *flac = DecodeBase64((const char *)data, size, &flacSize); - - if (flac == NULL) { + sp flacBuffer = decodeBase64(AString((const char *)data, size)); + if (flacBuffer == NULL) { ALOGE("malformed base64 encoded data."); return; } + size_t flacSize = flacBuffer->size(); + uint8_t *flac = flacBuffer->data(); ALOGV("got flac of size %zu", flacSize); uint32_t picType; @@ -926,24 +852,24 @@ static void extractAlbumArt( char type[128]; if (flacSize < 8) { - goto exit; + return; } picType = U32_AT(flac); if (picType != 3) { // This is not a front cover. - goto exit; + return; } typeLen = U32_AT(&flac[4]); if (typeLen > sizeof(type) - 1) { - goto exit; + return; } // we've already checked above that flacSize >= 8 if (flacSize - 8 < typeLen) { - goto exit; + return; } memcpy(type, &flac[8], typeLen); @@ -953,7 +879,7 @@ static void extractAlbumArt( if (!strcmp(type, "-->")) { // This is not inline cover art, but an external url instead. - goto exit; + return; } descLen = U32_AT(&flac[8 + typeLen]); @@ -961,7 +887,7 @@ static void extractAlbumArt( if (flacSize < 32 || flacSize - 32 < typeLen || flacSize - 32 - typeLen < descLen) { - goto exit; + return; } dataLen = U32_AT(&flac[8 + typeLen + 4 + descLen + 16]); @@ -969,7 +895,7 @@ static void extractAlbumArt( // we've already checked above that (flacSize - 32 - typeLen - descLen) >= 0 if (flacSize - 32 - typeLen - descLen < dataLen) { - goto exit; + return; } ALOGV("got image data, %zu trailing bytes", @@ -979,10 +905,6 @@ static void extractAlbumArt( kKeyAlbumArt, 0, &flac[8 + typeLen + 4 + descLen + 20], dataLen); fileMeta->setCString(kKeyAlbumArtMIME, type); - -exit: - free(flac); - flac = NULL; } //////////////////////////////////////////////////////////////////////////////// diff --git a/media/libstagefright/foundation/base64.cpp b/media/libstagefright/foundation/base64.cpp index dcf5bef..7da7db9 100644 --- a/media/libstagefright/foundation/base64.cpp +++ b/media/libstagefright/foundation/base64.cpp @@ -22,11 +22,11 @@ namespace android { sp decodeBase64(const AString &s) { - if ((s.size() % 4) != 0) { + size_t n = s.size(); + if ((n % 4) != 0) { return NULL; } - size_t n = s.size(); size_t padding = 0; if (n >= 1 && s.c_str()[n - 1] == '=') { padding = 1; @@ -40,11 +40,16 @@ sp decodeBase64(const AString &s) { } } - size_t outLen = 3 * s.size() / 4 - padding; + // We divide first to avoid overflow. It's OK to do this because we + // already made sure that n % 4 == 0. + size_t outLen = (n / 4) * 3 - padding; sp buffer = new ABuffer(outLen); uint8_t *out = buffer->data(); + if (out == NULL || buffer->size() < outLen) { + return NULL; + } size_t j = 0; uint32_t accum = 0; for (size_t i = 0; i < n; ++i) { -- cgit v1.1 From 278ed118d5e6b0e216668551a5b8becdabd9aade Mon Sep 17 00:00:00 2001 From: Wonsik Kim Date: Mon, 7 Sep 2015 15:52:27 +0900 Subject: DO NOT MERGE Avoid size_t overflow in base64 decoding once again Switch to foundation base64 function in OggExtractor and fix the issue there. Bug: 23707088 Change-Id: I999ae911177c88dc13f9ee9796ca93c5928b20b0 --- media/libstagefright/OggExtractor.cpp | 104 ++++------------------------- media/libstagefright/foundation/base64.cpp | 11 ++- 2 files changed, 21 insertions(+), 94 deletions(-) (limited to 'media') diff --git a/media/libstagefright/OggExtractor.cpp b/media/libstagefright/OggExtractor.cpp index 28c9c7c..8a93d1c 100644 --- a/media/libstagefright/OggExtractor.cpp +++ b/media/libstagefright/OggExtractor.cpp @@ -22,6 +22,7 @@ #include #include +#include #include #include #include @@ -826,94 +827,19 @@ void parseVorbisComment( } -// The returned buffer should be free()d. -static uint8_t *DecodeBase64(const char *s, size_t size, size_t *outSize) { - *outSize = 0; - - if ((size % 4) != 0) { - return NULL; - } - - size_t n = size; - size_t padding = 0; - if (n >= 1 && s[n - 1] == '=') { - padding = 1; - - if (n >= 2 && s[n - 2] == '=') { - padding = 2; - } - } - - // We divide first to avoid overflow. It's OK to do this because we - // already made sure that size % 4 == 0. - size_t outLen = (size / 4) * 3 - padding; - - void *buffer = malloc(outLen); - if (buffer == NULL) { - return NULL; - } - - uint8_t *out = (uint8_t *)buffer; - size_t j = 0; - uint32_t accum = 0; - for (size_t i = 0; i < n; ++i) { - char c = s[i]; - unsigned value; - if (c >= 'A' && c <= 'Z') { - value = c - 'A'; - } else if (c >= 'a' && c <= 'z') { - value = 26 + c - 'a'; - } else if (c >= '0' && c <= '9') { - value = 52 + c - '0'; - } else if (c == '+') { - value = 62; - } else if (c == '/') { - value = 63; - } else if (c != '=') { - break; - } else { - if (i < n - padding) { - break; - } - - value = 0; - } - - accum = (accum << 6) | value; - - if (((i + 1) % 4) == 0) { - out[j++] = (accum >> 16); - - if (j < outLen) { out[j++] = (accum >> 8) & 0xff; } - if (j < outLen) { out[j++] = accum & 0xff; } - - accum = 0; - } - } - - // Check if we exited the loop early. - if (j < outLen) { - free(buffer); - return NULL; - } - - *outSize = outLen; - return (uint8_t *)buffer; -} - static void extractAlbumArt( const sp &fileMeta, const void *data, size_t size) { ALOGV("extractAlbumArt from '%s'", (const char *)data); - size_t flacSize; - uint8_t *flac = DecodeBase64((const char *)data, size, &flacSize); - - if (flac == NULL) { + sp flacBuffer = decodeBase64(AString((const char *)data, size)); + if (flacBuffer == NULL) { ALOGE("malformed base64 encoded data."); return; } - ALOGV("got flac of size %d", flacSize); + size_t flacSize = flacBuffer->size(); + uint8_t *flac = flacBuffer->data(); + ALOGV("got flac of size %zu", flacSize); uint32_t picType; uint32_t typeLen; @@ -922,24 +848,24 @@ static void extractAlbumArt( char type[128]; if (flacSize < 8) { - goto exit; + return; } picType = U32_AT(flac); if (picType != 3) { // This is not a front cover. - goto exit; + return; } typeLen = U32_AT(&flac[4]); if (typeLen > sizeof(type) - 1) { - goto exit; + return; } // we've already checked above that flacSize >= 8 if (flacSize - 8 < typeLen) { - goto exit; + return; } memcpy(type, &flac[8], typeLen); @@ -949,7 +875,7 @@ static void extractAlbumArt( if (!strcmp(type, "-->")) { // This is not inline cover art, but an external url instead. - goto exit; + return; } descLen = U32_AT(&flac[8 + typeLen]); @@ -957,7 +883,7 @@ static void extractAlbumArt( if (flacSize < 32 || flacSize - 32 < typeLen || flacSize - 32 - typeLen < descLen) { - goto exit; + return; } dataLen = U32_AT(&flac[8 + typeLen + 4 + descLen + 16]); @@ -965,7 +891,7 @@ static void extractAlbumArt( // we've already checked above that (flacSize - 32 - typeLen - descLen) >= 0 if (flacSize - 32 - typeLen - descLen < dataLen) { - goto exit; + return; } ALOGV("got image data, %d trailing bytes", @@ -975,10 +901,6 @@ static void extractAlbumArt( kKeyAlbumArt, 0, &flac[8 + typeLen + 4 + descLen + 20], dataLen); fileMeta->setCString(kKeyAlbumArtMIME, type); - -exit: - free(flac); - flac = NULL; } //////////////////////////////////////////////////////////////////////////////// diff --git a/media/libstagefright/foundation/base64.cpp b/media/libstagefright/foundation/base64.cpp index d5fb4e0..9581986 100644 --- a/media/libstagefright/foundation/base64.cpp +++ b/media/libstagefright/foundation/base64.cpp @@ -22,11 +22,11 @@ namespace android { sp decodeBase64(const AString &s) { - if ((s.size() % 4) != 0) { + size_t n = s.size(); + if ((n % 4) != 0) { return NULL; } - size_t n = s.size(); size_t padding = 0; if (n >= 1 && s.c_str()[n - 1] == '=') { padding = 1; @@ -36,11 +36,16 @@ sp decodeBase64(const AString &s) { } } - size_t outLen = 3 * s.size() / 4 - padding; + // We divide first to avoid overflow. It's OK to do this because we + // already made sure that n % 4 == 0. + size_t outLen = (n / 4) * 3 - padding; sp buffer = new ABuffer(outLen); uint8_t *out = buffer->data(); + if (out == NULL || buffer->size() < outLen) { + return NULL; + } size_t j = 0; uint32_t accum = 0; for (size_t i = 0; i < n; ++i) { -- cgit v1.1 From e995e477ad59b79145200c8f1e9e13c16c682d59 Mon Sep 17 00:00:00 2001 From: Wei Jia Date: Wed, 9 Sep 2015 09:48:34 -0700 Subject: IAudioFlinger: clear config before reading it from parcel. Bug: 23905951 Bug: 23912202 Change-Id: Id13a9d3cae2c09e7381b841e67ddfb188274d74c --- media/libmedia/IAudioFlinger.cpp | 30 ++++++++++++++++++++---------- media/libmedia/IAudioPolicyService.cpp | 14 +++++++++----- 2 files changed, 29 insertions(+), 15 deletions(-) (limited to 'media') diff --git a/media/libmedia/IAudioFlinger.cpp b/media/libmedia/IAudioFlinger.cpp index a3f014b..18a933f 100644 --- a/media/libmedia/IAudioFlinger.cpp +++ b/media/libmedia/IAudioFlinger.cpp @@ -1104,8 +1104,10 @@ status_t BnAudioFlinger::onTransact( case OPEN_OUTPUT: { CHECK_INTERFACE(IAudioFlinger, data, reply); audio_module_handle_t module = (audio_module_handle_t)data.readInt32(); - audio_config_t config; - data.read(&config, sizeof(audio_config_t)); + audio_config_t config = {}; + if (data.read(&config, sizeof(audio_config_t)) != NO_ERROR) { + ALOGE("b/23905951"); + } audio_devices_t devices = (audio_devices_t)data.readInt32(); String8 address(data.readString8()); audio_output_flags_t flags = (audio_output_flags_t) data.readInt32(); @@ -1149,8 +1151,10 @@ status_t BnAudioFlinger::onTransact( CHECK_INTERFACE(IAudioFlinger, data, reply); audio_module_handle_t module = (audio_module_handle_t)data.readInt32(); audio_io_handle_t input = (audio_io_handle_t)data.readInt32(); - audio_config_t config; - data.read(&config, sizeof(audio_config_t)); + audio_config_t config = {}; + if (data.read(&config, sizeof(audio_config_t)) != NO_ERROR) { + ALOGE("b/23905951"); + } audio_devices_t device = (audio_devices_t)data.readInt32(); String8 address(data.readString8()); audio_source_t source = (audio_source_t)data.readInt32(); @@ -1255,8 +1259,10 @@ status_t BnAudioFlinger::onTransact( } case CREATE_EFFECT: { CHECK_INTERFACE(IAudioFlinger, data, reply); - effect_descriptor_t desc; - data.read(&desc, sizeof(effect_descriptor_t)); + effect_descriptor_t desc = {}; + if (data.read(&desc, sizeof(effect_descriptor_t)) != NO_ERROR) { + ALOGE("b/23905951"); + } sp client = interface_cast(data.readStrongBinder()); int32_t priority = data.readInt32(); audio_io_handle_t output = (audio_io_handle_t) data.readInt32(); @@ -1333,8 +1339,10 @@ status_t BnAudioFlinger::onTransact( } break; case GET_AUDIO_PORT: { CHECK_INTERFACE(IAudioFlinger, data, reply); - struct audio_port port; - data.read(&port, sizeof(struct audio_port)); + struct audio_port port = {}; + if (data.read(&port, sizeof(struct audio_port)) != NO_ERROR) { + ALOGE("b/23905951"); + } status_t status = getAudioPort(&port); reply->writeInt32(status); if (status == NO_ERROR) { @@ -1346,8 +1354,10 @@ status_t BnAudioFlinger::onTransact( CHECK_INTERFACE(IAudioFlinger, data, reply); struct audio_patch patch; data.read(&patch, sizeof(struct audio_patch)); - audio_patch_handle_t handle; - data.read(&handle, sizeof(audio_patch_handle_t)); + audio_patch_handle_t handle = {}; + if (data.read(&handle, sizeof(audio_patch_handle_t)) != NO_ERROR) { + ALOGE("b/23905951"); + } status_t status = createAudioPatch(&patch, &handle); reply->writeInt32(status); if (status == NO_ERROR) { diff --git a/media/libmedia/IAudioPolicyService.cpp b/media/libmedia/IAudioPolicyService.cpp index 7e6b9fc..0cc954f 100644 --- a/media/libmedia/IAudioPolicyService.cpp +++ b/media/libmedia/IAudioPolicyService.cpp @@ -1148,8 +1148,10 @@ status_t BnAudioPolicyService::onTransact( case GET_AUDIO_PORT: { CHECK_INTERFACE(IAudioPolicyService, data, reply); - struct audio_port port; - data.read(&port, sizeof(struct audio_port)); + struct audio_port port = {}; + if (data.read(&port, sizeof(struct audio_port)) != NO_ERROR) { + ALOGE("b/23912202"); + } status_t status = getAudioPort(&port); reply->writeInt32(status); if (status == NO_ERROR) { @@ -1162,8 +1164,10 @@ status_t BnAudioPolicyService::onTransact( CHECK_INTERFACE(IAudioPolicyService, data, reply); struct audio_patch patch; data.read(&patch, sizeof(struct audio_patch)); - audio_patch_handle_t handle; - data.read(&handle, sizeof(audio_patch_handle_t)); + audio_patch_handle_t handle = {}; + if (data.read(&handle, sizeof(audio_patch_handle_t)) != NO_ERROR) { + ALOGE("b/23912202"); + } status_t status = createAudioPatch(&patch, &handle); reply->writeInt32(status); if (status == NO_ERROR) { @@ -1292,7 +1296,7 @@ status_t BnAudioPolicyService::onTransact( data.read(&source, sizeof(struct audio_port_config)); audio_attributes_t attributes; data.read(&attributes, sizeof(audio_attributes_t)); - audio_io_handle_t handle; + audio_io_handle_t handle = {}; status_t status = startAudioSource(&source, &attributes, &handle); reply->writeInt32(status); reply->writeInt32(handle); -- cgit v1.1 From 983dca391a76fb45df999fc40e8766b9ddb63511 Mon Sep 17 00:00:00 2001 From: Wei Jia Date: Thu, 10 Sep 2015 09:47:29 -0700 Subject: IAudioFlinger: always initialize variables to ensure no info leak when writing them to Parcel. Bug: 23953967 Change-Id: Ibbe841da149038675e9e8daea76c77558bc8564b --- media/libmedia/IAudioFlinger.cpp | 22 +++++++++++----------- media/libmedia/IAudioPolicyService.cpp | 8 ++++---- 2 files changed, 15 insertions(+), 15 deletions(-) (limited to 'media') diff --git a/media/libmedia/IAudioFlinger.cpp b/media/libmedia/IAudioFlinger.cpp index 18a933f..47ed359 100644 --- a/media/libmedia/IAudioFlinger.cpp +++ b/media/libmedia/IAudioFlinger.cpp @@ -937,7 +937,7 @@ status_t BnAudioFlinger::onTransact( pid_t tid = (pid_t) data.readInt32(); int sessionId = data.readInt32(); int clientUid = data.readInt32(); - status_t status; + status_t status = NO_ERROR; sp track; if ((haveSharedBuffer && (buffer == 0)) || ((buffer != 0) && (buffer->pointer() == NULL))) { @@ -972,7 +972,7 @@ status_t BnAudioFlinger::onTransact( size_t notificationFrames = data.readInt64(); sp cblk; sp buffers; - status_t status; + status_t status = NO_ERROR; sp record = openRecord(input, sampleRate, format, channelMask, opPackageName, &frameCount, &flags, tid, clientUid, &sessionId, ¬ificationFrames, cblk, buffers, &status); @@ -1111,7 +1111,7 @@ status_t BnAudioFlinger::onTransact( audio_devices_t devices = (audio_devices_t)data.readInt32(); String8 address(data.readString8()); audio_output_flags_t flags = (audio_output_flags_t) data.readInt32(); - uint32_t latencyMs; + uint32_t latencyMs = 0; audio_io_handle_t output; status_t status = openOutput(module, &output, &config, &devices, address, &latencyMs, flags); @@ -1190,8 +1190,8 @@ status_t BnAudioFlinger::onTransact( case GET_RENDER_POSITION: { CHECK_INTERFACE(IAudioFlinger, data, reply); audio_io_handle_t output = (audio_io_handle_t) data.readInt32(); - uint32_t halFrames; - uint32_t dspFrames; + uint32_t halFrames = 0; + uint32_t dspFrames = 0; status_t status = getRenderPosition(&halFrames, &dspFrames, output); reply->writeInt32(status); if (status == NO_ERROR) { @@ -1227,7 +1227,7 @@ status_t BnAudioFlinger::onTransact( } break; case QUERY_NUM_EFFECTS: { CHECK_INTERFACE(IAudioFlinger, data, reply); - uint32_t numEffects; + uint32_t numEffects = 0; status_t status = queryNumberEffects(&numEffects); reply->writeInt32(status); if (status == NO_ERROR) { @@ -1237,7 +1237,7 @@ status_t BnAudioFlinger::onTransact( } case QUERY_EFFECT: { CHECK_INTERFACE(IAudioFlinger, data, reply); - effect_descriptor_t desc; + effect_descriptor_t desc = {}; status_t status = queryEffect(data.readInt32(), &desc); reply->writeInt32(status); if (status == NO_ERROR) { @@ -1249,7 +1249,7 @@ status_t BnAudioFlinger::onTransact( CHECK_INTERFACE(IAudioFlinger, data, reply); effect_uuid_t uuid; data.read(&uuid, sizeof(effect_uuid_t)); - effect_descriptor_t desc; + effect_descriptor_t desc = {}; status_t status = getEffectDescriptor(&uuid, &desc); reply->writeInt32(status); if (status == NO_ERROR) { @@ -1268,9 +1268,9 @@ status_t BnAudioFlinger::onTransact( audio_io_handle_t output = (audio_io_handle_t) data.readInt32(); int sessionId = data.readInt32(); const String16 opPackageName = data.readString16(); - status_t status; - int id; - int enabled; + status_t status = NO_ERROR; + int id = 0; + int enabled = 0; sp effect = createEffect(&desc, client, priority, output, sessionId, opPackageName, &status, &id, &enabled); diff --git a/media/libmedia/IAudioPolicyService.cpp b/media/libmedia/IAudioPolicyService.cpp index 0cc954f..76b5924 100644 --- a/media/libmedia/IAudioPolicyService.cpp +++ b/media/libmedia/IAudioPolicyService.cpp @@ -932,7 +932,7 @@ status_t BnAudioPolicyService::onTransact( audio_channel_mask_t channelMask = data.readInt32(); audio_input_flags_t flags = (audio_input_flags_t) data.readInt32(); audio_port_handle_t selectedDeviceId = (audio_port_handle_t) data.readInt32(); - audio_io_handle_t input; + audio_io_handle_t input = {}; status_t status = getInputForAttr(&attr, &input, session, uid, samplingRate, format, channelMask, flags, selectedDeviceId); @@ -1242,9 +1242,9 @@ status_t BnAudioPolicyService::onTransact( CHECK_INTERFACE(IAudioPolicyService, data, reply); sp client = interface_cast( data.readStrongBinder()); - audio_session_t session; - audio_io_handle_t ioHandle; - audio_devices_t device; + audio_session_t session = {}; + audio_io_handle_t ioHandle = {}; + audio_devices_t device = {}; status_t status = acquireSoundTriggerSession(&session, &ioHandle, &device); reply->writeInt32(status); if (status == NO_ERROR) { -- cgit v1.1 From 5f2a76f02f6d88825f8c164b4d9f7f310583eee1 Mon Sep 17 00:00:00 2001 From: Wei Jia Date: Wed, 9 Sep 2015 09:48:34 -0700 Subject: DO NOT MERGE - IAudioFlinger: clear config before reading it from parcel. Bug: 23905951 Bug: 23912202 Change-Id: Id13a9d3cae2c09e7381b841e67ddfb188274d74c (cherry picked from commit e995e477ad59b79145200c8f1e9e13c16c682d59) --- media/libmedia/IAudioFlinger.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) (limited to 'media') diff --git a/media/libmedia/IAudioFlinger.cpp b/media/libmedia/IAudioFlinger.cpp index acfaea0..adc7186 100644 --- a/media/libmedia/IAudioFlinger.cpp +++ b/media/libmedia/IAudioFlinger.cpp @@ -1080,8 +1080,10 @@ status_t BnAudioFlinger::onTransact( } case CREATE_EFFECT: { CHECK_INTERFACE(IAudioFlinger, data, reply); - effect_descriptor_t desc; - data.read(&desc, sizeof(effect_descriptor_t)); + effect_descriptor_t desc = {}; + if (data.read(&desc, sizeof(effect_descriptor_t)) != NO_ERROR) { + ALOGE("b/23905951"); + } sp client = interface_cast(data.readStrongBinder()); int32_t priority = data.readInt32(); audio_io_handle_t output = (audio_io_handle_t) data.readInt32(); -- cgit v1.1 From 1c7719820359f4190cd4bfd1a24d521face7b4f8 Mon Sep 17 00:00:00 2001 From: Wei Jia Date: Wed, 9 Sep 2015 09:48:34 -0700 Subject: DO NOT MERGE - IAudioFlinger: clear config before reading it from parcel. Bug: 23905951 Bug: 23912202 Change-Id: Id13a9d3cae2c09e7381b841e67ddfb188274d74c (cherry picked from commit e995e477ad59b79145200c8f1e9e13c16c682d59) --- media/libmedia/IAudioFlinger.cpp | 30 ++++++++++++++++++++---------- media/libmedia/IAudioPolicyService.cpp | 12 ++++++++---- 2 files changed, 28 insertions(+), 14 deletions(-) (limited to 'media') diff --git a/media/libmedia/IAudioFlinger.cpp b/media/libmedia/IAudioFlinger.cpp index d7ca425..d31de58 100644 --- a/media/libmedia/IAudioFlinger.cpp +++ b/media/libmedia/IAudioFlinger.cpp @@ -1090,8 +1090,10 @@ status_t BnAudioFlinger::onTransact( case OPEN_OUTPUT: { CHECK_INTERFACE(IAudioFlinger, data, reply); audio_module_handle_t module = (audio_module_handle_t)data.readInt32(); - audio_config_t config; - data.read(&config, sizeof(audio_config_t)); + audio_config_t config = {}; + if (data.read(&config, sizeof(audio_config_t)) != NO_ERROR) { + ALOGE("b/23905951"); + } audio_devices_t devices = (audio_devices_t)data.readInt32(); String8 address(data.readString8()); audio_output_flags_t flags = (audio_output_flags_t) data.readInt32(); @@ -1135,8 +1137,10 @@ status_t BnAudioFlinger::onTransact( CHECK_INTERFACE(IAudioFlinger, data, reply); audio_module_handle_t module = (audio_module_handle_t)data.readInt32(); audio_io_handle_t input = (audio_io_handle_t)data.readInt32(); - audio_config_t config; - data.read(&config, sizeof(audio_config_t)); + audio_config_t config = {}; + if (data.read(&config, sizeof(audio_config_t)) != NO_ERROR) { + ALOGE("b/23905951"); + } audio_devices_t device = (audio_devices_t)data.readInt32(); String8 address(data.readString8()); audio_source_t source = (audio_source_t)data.readInt32(); @@ -1241,8 +1245,10 @@ status_t BnAudioFlinger::onTransact( } case CREATE_EFFECT: { CHECK_INTERFACE(IAudioFlinger, data, reply); - effect_descriptor_t desc; - data.read(&desc, sizeof(effect_descriptor_t)); + effect_descriptor_t desc = {}; + if (data.read(&desc, sizeof(effect_descriptor_t)) != NO_ERROR) { + ALOGE("b/23905951"); + } sp client = interface_cast(data.readStrongBinder()); int32_t priority = data.readInt32(); audio_io_handle_t output = (audio_io_handle_t) data.readInt32(); @@ -1318,8 +1324,10 @@ status_t BnAudioFlinger::onTransact( } break; case GET_AUDIO_PORT: { CHECK_INTERFACE(IAudioFlinger, data, reply); - struct audio_port port; - data.read(&port, sizeof(struct audio_port)); + struct audio_port port = {}; + if (data.read(&port, sizeof(struct audio_port)) != NO_ERROR) { + ALOGE("b/23905951"); + } status_t status = getAudioPort(&port); reply->writeInt32(status); if (status == NO_ERROR) { @@ -1331,8 +1339,10 @@ status_t BnAudioFlinger::onTransact( CHECK_INTERFACE(IAudioFlinger, data, reply); struct audio_patch patch; data.read(&patch, sizeof(struct audio_patch)); - audio_patch_handle_t handle; - data.read(&handle, sizeof(audio_patch_handle_t)); + audio_patch_handle_t handle = {}; + if (data.read(&handle, sizeof(audio_patch_handle_t)) != NO_ERROR) { + ALOGE("b/23905951"); + } status_t status = createAudioPatch(&patch, &handle); reply->writeInt32(status); if (status == NO_ERROR) { diff --git a/media/libmedia/IAudioPolicyService.cpp b/media/libmedia/IAudioPolicyService.cpp index 4e47c79..e6e01aa 100644 --- a/media/libmedia/IAudioPolicyService.cpp +++ b/media/libmedia/IAudioPolicyService.cpp @@ -983,8 +983,10 @@ status_t BnAudioPolicyService::onTransact( case GET_AUDIO_PORT: { CHECK_INTERFACE(IAudioPolicyService, data, reply); - struct audio_port port; - data.read(&port, sizeof(struct audio_port)); + struct audio_port port = {}; + if (data.read(&port, sizeof(struct audio_port)) != NO_ERROR) { + ALOGE("b/23912202"); + } status_t status = getAudioPort(&port); reply->writeInt32(status); if (status == NO_ERROR) { @@ -997,8 +999,10 @@ status_t BnAudioPolicyService::onTransact( CHECK_INTERFACE(IAudioPolicyService, data, reply); struct audio_patch patch; data.read(&patch, sizeof(struct audio_patch)); - audio_patch_handle_t handle; - data.read(&handle, sizeof(audio_patch_handle_t)); + audio_patch_handle_t handle = {}; + if (data.read(&handle, sizeof(audio_patch_handle_t)) != NO_ERROR) { + ALOGE("b/23912202"); + } status_t status = createAudioPatch(&patch, &handle); reply->writeInt32(status); if (status == NO_ERROR) { -- cgit v1.1 From 58828196edf2fc4debbd7913198a8149f039b4a9 Mon Sep 17 00:00:00 2001 From: Ronghua Wu Date: Thu, 10 Sep 2015 10:53:22 -0700 Subject: libstagefright: Do not add audio codecs to resource manager. They are too small anyhow. Bug: 23703241 Change-Id: I3c2882a1d6736bb8a4099289d8bab4974343586d --- media/libstagefright/MediaCodec.cpp | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) (limited to 'media') diff --git a/media/libstagefright/MediaCodec.cpp b/media/libstagefright/MediaCodec.cpp index cd59709..7019537 100644 --- a/media/libstagefright/MediaCodec.cpp +++ b/media/libstagefright/MediaCodec.cpp @@ -572,6 +572,7 @@ status_t MediaCodec::stop() { } status_t MediaCodec::reclaim() { + ALOGD("MediaCodec::reclaim(%p) %s", this, mInitName.c_str()); sp msg = new AMessage(kWhatRelease, this); msg->setInt32("reclaimed", 1); @@ -1154,8 +1155,10 @@ void MediaCodec::onMessageReceived(const sp &msg) { resourceType = String8(kResourceNonSecureCodec); } - const char *subtype = mIsVideo ? kResourceVideoCodec : kResourceAudioCodec; - addResource(resourceType, String8(subtype), 1); + if (mIsVideo) { + // audio codec is currently ignored. + addResource(resourceType, String8(kResourceVideoCodec), 1); + } (new AMessage)->postReply(mReplyID); break; -- cgit v1.1 From 28314aef9e8a666dbb75bbd555f6566a6c991f1c Mon Sep 17 00:00:00 2001 From: Wonsik Kim Date: Fri, 11 Sep 2015 06:51:10 +0000 Subject: Revert "Avoid size_t overflow in base64 decoding once again" This reverts commit c9ac5dfdafed1c66beae090cafa97002764e0ca3. Change-Id: Iae9707bbd8641a0bb00fcda39a20eb8b8f4f5232 --- media/libstagefright/OggExtractor.cpp | 102 +++++++++++++++++++++++++---- media/libstagefright/foundation/base64.cpp | 11 +--- 2 files changed, 93 insertions(+), 20 deletions(-) (limited to 'media') diff --git a/media/libstagefright/OggExtractor.cpp b/media/libstagefright/OggExtractor.cpp index 10dce3b..130f5a5 100644 --- a/media/libstagefright/OggExtractor.cpp +++ b/media/libstagefright/OggExtractor.cpp @@ -22,7 +22,6 @@ #include #include -#include #include #include #include @@ -831,18 +830,93 @@ void parseVorbisComment( } +// The returned buffer should be free()d. +static uint8_t *DecodeBase64(const char *s, size_t size, size_t *outSize) { + *outSize = 0; + + if ((size % 4) != 0) { + return NULL; + } + + size_t n = size; + size_t padding = 0; + if (n >= 1 && s[n - 1] == '=') { + padding = 1; + + if (n >= 2 && s[n - 2] == '=') { + padding = 2; + } + } + + // We divide first to avoid overflow. It's OK to do this because we + // already made sure that size % 4 == 0. + size_t outLen = (size / 4) * 3 - padding; + + void *buffer = malloc(outLen); + if (buffer == NULL) { + return NULL; + } + + uint8_t *out = (uint8_t *)buffer; + size_t j = 0; + uint32_t accum = 0; + for (size_t i = 0; i < n; ++i) { + char c = s[i]; + unsigned value; + if (c >= 'A' && c <= 'Z') { + value = c - 'A'; + } else if (c >= 'a' && c <= 'z') { + value = 26 + c - 'a'; + } else if (c >= '0' && c <= '9') { + value = 52 + c - '0'; + } else if (c == '+') { + value = 62; + } else if (c == '/') { + value = 63; + } else if (c != '=') { + break; + } else { + if (i < n - padding) { + break; + } + + value = 0; + } + + accum = (accum << 6) | value; + + if (((i + 1) % 4) == 0) { + out[j++] = (accum >> 16); + + if (j < outLen) { out[j++] = (accum >> 8) & 0xff; } + if (j < outLen) { out[j++] = accum & 0xff; } + + accum = 0; + } + } + + // Check if we exited the loop early. + if (j < outLen) { + free(buffer); + return NULL; + } + + *outSize = outLen; + return (uint8_t *)buffer; +} + static void extractAlbumArt( const sp &fileMeta, const void *data, size_t size) { ALOGV("extractAlbumArt from '%s'", (const char *)data); - sp flacBuffer = decodeBase64(AString((const char *)data, size)); - if (flacBuffer == NULL) { + size_t flacSize; + uint8_t *flac = DecodeBase64((const char *)data, size, &flacSize); + + if (flac == NULL) { ALOGE("malformed base64 encoded data."); return; } - size_t flacSize = flacBuffer->size(); - uint8_t *flac = flacBuffer->data(); ALOGV("got flac of size %zu", flacSize); uint32_t picType; @@ -852,24 +926,24 @@ static void extractAlbumArt( char type[128]; if (flacSize < 8) { - return; + goto exit; } picType = U32_AT(flac); if (picType != 3) { // This is not a front cover. - return; + goto exit; } typeLen = U32_AT(&flac[4]); if (typeLen > sizeof(type) - 1) { - return; + goto exit; } // we've already checked above that flacSize >= 8 if (flacSize - 8 < typeLen) { - return; + goto exit; } memcpy(type, &flac[8], typeLen); @@ -879,7 +953,7 @@ static void extractAlbumArt( if (!strcmp(type, "-->")) { // This is not inline cover art, but an external url instead. - return; + goto exit; } descLen = U32_AT(&flac[8 + typeLen]); @@ -887,7 +961,7 @@ static void extractAlbumArt( if (flacSize < 32 || flacSize - 32 < typeLen || flacSize - 32 - typeLen < descLen) { - return; + goto exit; } dataLen = U32_AT(&flac[8 + typeLen + 4 + descLen + 16]); @@ -895,7 +969,7 @@ static void extractAlbumArt( // we've already checked above that (flacSize - 32 - typeLen - descLen) >= 0 if (flacSize - 32 - typeLen - descLen < dataLen) { - return; + goto exit; } ALOGV("got image data, %zu trailing bytes", @@ -905,6 +979,10 @@ static void extractAlbumArt( kKeyAlbumArt, 0, &flac[8 + typeLen + 4 + descLen + 20], dataLen); fileMeta->setCString(kKeyAlbumArtMIME, type); + +exit: + free(flac); + flac = NULL; } //////////////////////////////////////////////////////////////////////////////// diff --git a/media/libstagefright/foundation/base64.cpp b/media/libstagefright/foundation/base64.cpp index 7da7db9..dcf5bef 100644 --- a/media/libstagefright/foundation/base64.cpp +++ b/media/libstagefright/foundation/base64.cpp @@ -22,11 +22,11 @@ namespace android { sp decodeBase64(const AString &s) { - size_t n = s.size(); - if ((n % 4) != 0) { + if ((s.size() % 4) != 0) { return NULL; } + size_t n = s.size(); size_t padding = 0; if (n >= 1 && s.c_str()[n - 1] == '=') { padding = 1; @@ -40,16 +40,11 @@ sp decodeBase64(const AString &s) { } } - // We divide first to avoid overflow. It's OK to do this because we - // already made sure that n % 4 == 0. - size_t outLen = (n / 4) * 3 - padding; + size_t outLen = 3 * s.size() / 4 - padding; sp buffer = new ABuffer(outLen); uint8_t *out = buffer->data(); - if (out == NULL || buffer->size() < outLen) { - return NULL; - } size_t j = 0; uint32_t accum = 0; for (size_t i = 0; i < n; ++i) { -- cgit v1.1 From 9104e704e43c9a6e691af52bb6f0aca71467979b Mon Sep 17 00:00:00 2001 From: Wonsik Kim Date: Fri, 11 Sep 2015 16:14:18 +0900 Subject: DO NOT MERGE fix build Bug: 23707088 Change-Id: Ib0d6cbc52710f33310d21b2eae1f243f0f8e8bca --- media/libstagefright/OggExtractor.cpp | 1 + 1 file changed, 1 insertion(+) (limited to 'media') diff --git a/media/libstagefright/OggExtractor.cpp b/media/libstagefright/OggExtractor.cpp index 8a93d1c..da94587 100644 --- a/media/libstagefright/OggExtractor.cpp +++ b/media/libstagefright/OggExtractor.cpp @@ -21,6 +21,7 @@ #include "include/OggExtractor.h" #include +#include #include #include #include -- cgit v1.1 From 3f5ff68327b1df21196f18a020aec474a0dd95fe Mon Sep 17 00:00:00 2001 From: Andy Hung Date: Fri, 11 Sep 2015 18:44:56 -0700 Subject: NuPlayerRenderer: Do not drain audio during teardown Bug: 23748678 Change-Id: I8b65786f25f4524e5e2e9476ecc2a4f4ab3aea9e --- media/libmediaplayerservice/nuplayer/NuPlayerRenderer.cpp | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'media') diff --git a/media/libmediaplayerservice/nuplayer/NuPlayerRenderer.cpp b/media/libmediaplayerservice/nuplayer/NuPlayerRenderer.cpp index 0ac29a8..93555f5 100644 --- a/media/libmediaplayerservice/nuplayer/NuPlayerRenderer.cpp +++ b/media/libmediaplayerservice/nuplayer/NuPlayerRenderer.cpp @@ -798,6 +798,10 @@ void NuPlayer::Renderer::drainAudioQueueUntilLastEOS() { } bool NuPlayer::Renderer::onDrainAudioQueue() { + // do not drain audio during teardown as queued buffers may be invalid. + if (mAudioTornDown) { + return false; + } // TODO: This call to getPosition checks if AudioTrack has been created // in AudioSink before draining audio. If AudioTrack doesn't exist, then // CHECKs on getPosition will fail. @@ -1477,6 +1481,7 @@ void NuPlayer::Renderer::onResume() { cancelAudioOffloadPauseTimeout(); status_t err = mAudioSink->start(); if (err != OK) { + ALOGE("cannot start AudioSink err %d", err); notifyAudioTearDown(); } } -- cgit v1.1 From 820c105f7a4dc0971ee563caea4c9b346854a2f7 Mon Sep 17 00:00:00 2001 From: Wonsik Kim Date: Fri, 11 Sep 2015 16:31:18 +0900 Subject: DO NOT MERGE Avoid size_t overflow in base64 decoding once again Switch to foundation base64 function in OggExtractor and fix the issue there. This reverts commit 28314aef9e8a666dbb75bbd555f6566a6c991f1c. Bug: 23707088 Change-Id: I268bd50431de5b5e579343bf1b425c42ada6daba --- media/libstagefright/OggExtractor.cpp | 103 ++++------------------------- media/libstagefright/foundation/base64.cpp | 11 ++- 2 files changed, 21 insertions(+), 93 deletions(-) (limited to 'media') diff --git a/media/libstagefright/OggExtractor.cpp b/media/libstagefright/OggExtractor.cpp index 130f5a5..900e2b0 100644 --- a/media/libstagefright/OggExtractor.cpp +++ b/media/libstagefright/OggExtractor.cpp @@ -21,7 +21,9 @@ #include "include/OggExtractor.h" #include +#include #include +#include #include #include #include @@ -830,93 +832,18 @@ void parseVorbisComment( } -// The returned buffer should be free()d. -static uint8_t *DecodeBase64(const char *s, size_t size, size_t *outSize) { - *outSize = 0; - - if ((size % 4) != 0) { - return NULL; - } - - size_t n = size; - size_t padding = 0; - if (n >= 1 && s[n - 1] == '=') { - padding = 1; - - if (n >= 2 && s[n - 2] == '=') { - padding = 2; - } - } - - // We divide first to avoid overflow. It's OK to do this because we - // already made sure that size % 4 == 0. - size_t outLen = (size / 4) * 3 - padding; - - void *buffer = malloc(outLen); - if (buffer == NULL) { - return NULL; - } - - uint8_t *out = (uint8_t *)buffer; - size_t j = 0; - uint32_t accum = 0; - for (size_t i = 0; i < n; ++i) { - char c = s[i]; - unsigned value; - if (c >= 'A' && c <= 'Z') { - value = c - 'A'; - } else if (c >= 'a' && c <= 'z') { - value = 26 + c - 'a'; - } else if (c >= '0' && c <= '9') { - value = 52 + c - '0'; - } else if (c == '+') { - value = 62; - } else if (c == '/') { - value = 63; - } else if (c != '=') { - break; - } else { - if (i < n - padding) { - break; - } - - value = 0; - } - - accum = (accum << 6) | value; - - if (((i + 1) % 4) == 0) { - out[j++] = (accum >> 16); - - if (j < outLen) { out[j++] = (accum >> 8) & 0xff; } - if (j < outLen) { out[j++] = accum & 0xff; } - - accum = 0; - } - } - - // Check if we exited the loop early. - if (j < outLen) { - free(buffer); - return NULL; - } - - *outSize = outLen; - return (uint8_t *)buffer; -} - static void extractAlbumArt( const sp &fileMeta, const void *data, size_t size) { ALOGV("extractAlbumArt from '%s'", (const char *)data); - size_t flacSize; - uint8_t *flac = DecodeBase64((const char *)data, size, &flacSize); - - if (flac == NULL) { + sp flacBuffer = decodeBase64(AString((const char *)data, size)); + if (flacBuffer == NULL) { ALOGE("malformed base64 encoded data."); return; } + size_t flacSize = flacBuffer->size(); + uint8_t *flac = flacBuffer->data(); ALOGV("got flac of size %zu", flacSize); uint32_t picType; @@ -926,24 +853,24 @@ static void extractAlbumArt( char type[128]; if (flacSize < 8) { - goto exit; + return; } picType = U32_AT(flac); if (picType != 3) { // This is not a front cover. - goto exit; + return; } typeLen = U32_AT(&flac[4]); if (typeLen > sizeof(type) - 1) { - goto exit; + return; } // we've already checked above that flacSize >= 8 if (flacSize - 8 < typeLen) { - goto exit; + return; } memcpy(type, &flac[8], typeLen); @@ -953,7 +880,7 @@ static void extractAlbumArt( if (!strcmp(type, "-->")) { // This is not inline cover art, but an external url instead. - goto exit; + return; } descLen = U32_AT(&flac[8 + typeLen]); @@ -961,7 +888,7 @@ static void extractAlbumArt( if (flacSize < 32 || flacSize - 32 < typeLen || flacSize - 32 - typeLen < descLen) { - goto exit; + return; } dataLen = U32_AT(&flac[8 + typeLen + 4 + descLen + 16]); @@ -969,7 +896,7 @@ static void extractAlbumArt( // we've already checked above that (flacSize - 32 - typeLen - descLen) >= 0 if (flacSize - 32 - typeLen - descLen < dataLen) { - goto exit; + return; } ALOGV("got image data, %zu trailing bytes", @@ -979,10 +906,6 @@ static void extractAlbumArt( kKeyAlbumArt, 0, &flac[8 + typeLen + 4 + descLen + 20], dataLen); fileMeta->setCString(kKeyAlbumArtMIME, type); - -exit: - free(flac); - flac = NULL; } //////////////////////////////////////////////////////////////////////////////// diff --git a/media/libstagefright/foundation/base64.cpp b/media/libstagefright/foundation/base64.cpp index dcf5bef..7da7db9 100644 --- a/media/libstagefright/foundation/base64.cpp +++ b/media/libstagefright/foundation/base64.cpp @@ -22,11 +22,11 @@ namespace android { sp decodeBase64(const AString &s) { - if ((s.size() % 4) != 0) { + size_t n = s.size(); + if ((n % 4) != 0) { return NULL; } - size_t n = s.size(); size_t padding = 0; if (n >= 1 && s.c_str()[n - 1] == '=') { padding = 1; @@ -40,11 +40,16 @@ sp decodeBase64(const AString &s) { } } - size_t outLen = 3 * s.size() / 4 - padding; + // We divide first to avoid overflow. It's OK to do this because we + // already made sure that n % 4 == 0. + size_t outLen = (n / 4) * 3 - padding; sp buffer = new ABuffer(outLen); uint8_t *out = buffer->data(); + if (out == NULL || buffer->size() < outLen) { + return NULL; + } size_t j = 0; uint32_t accum = 0; for (size_t i = 0; i < n; ++i) { -- cgit v1.1 From c894f81bdf106f648561e569e0dc97fc6046115b Mon Sep 17 00:00:00 2001 From: Wonsik Kim Date: Tue, 8 Sep 2015 17:32:28 +0900 Subject: DO NOT MERGE NuCachedSource2: fix possible erroneous early free Because the constructor of NuCachedSource2 sent a message to AHandlerReflector object, AHandlerReflector::onMessageReceived could have executed just before the object gets wrapped in a strong pointer, resulting in erroneous early free. Fix the issue by using static Create function to ensure the message is sent after the object is wrapped in a sp. Bug: 23882800 Change-Id: I38a9d7a3083f184b4c81d0b00ba1661721278855 --- media/libstagefright/AwesomePlayer.cpp | 4 ++-- media/libstagefright/DataSource.cpp | 2 +- media/libstagefright/NuCachedSource2.cpp | 15 ++++++++++++--- media/libstagefright/include/NuCachedSource2.h | 7 ++++++- 4 files changed, 21 insertions(+), 7 deletions(-) (limited to 'media') diff --git a/media/libstagefright/AwesomePlayer.cpp b/media/libstagefright/AwesomePlayer.cpp index 130207d..fe9f83f 100644 --- a/media/libstagefright/AwesomePlayer.cpp +++ b/media/libstagefright/AwesomePlayer.cpp @@ -2218,11 +2218,11 @@ status_t AwesomePlayer::finishSetDataSource_l() { // The widevine extractor does its own caching. #if 0 - mCachedSource = new NuCachedSource2( + mCachedSource = NuCachedSource2::Create( new ThrottledSource( mConnectingDataSource, 50 * 1024 /* bytes/sec */)); #else - mCachedSource = new NuCachedSource2( + mCachedSource = NuCachedSource2::Create( mConnectingDataSource, cacheConfig.isEmpty() ? NULL : cacheConfig.string(), disconnectAtHighwatermark); diff --git a/media/libstagefright/DataSource.cpp b/media/libstagefright/DataSource.cpp index 97987e2..5531ea74 100644 --- a/media/libstagefright/DataSource.cpp +++ b/media/libstagefright/DataSource.cpp @@ -212,7 +212,7 @@ sp DataSource::CreateFromURI( ©, &cacheConfig, &disconnectAtHighwatermark); } - source = new NuCachedSource2( + source = NuCachedSource2::Create( httpSource, cacheConfig.isEmpty() ? NULL : cacheConfig.string()); } else { diff --git a/media/libstagefright/NuCachedSource2.cpp b/media/libstagefright/NuCachedSource2.cpp index 05e599b..270d64a 100644 --- a/media/libstagefright/NuCachedSource2.cpp +++ b/media/libstagefright/NuCachedSource2.cpp @@ -214,9 +214,6 @@ NuCachedSource2::NuCachedSource2( mLooper->setName("NuCachedSource2"); mLooper->registerHandler(mReflector); mLooper->start(); - - Mutex::Autolock autoLock(mLock); - (new AMessage(kWhatFetchMore, mReflector->id()))->post(); } NuCachedSource2::~NuCachedSource2() { @@ -227,6 +224,18 @@ NuCachedSource2::~NuCachedSource2() { mCache = NULL; } +// static +sp NuCachedSource2::Create( + const sp &source, + const char *cacheConfig, + bool disconnectAtHighwatermark) { + sp instance = new NuCachedSource2( + source, cacheConfig, disconnectAtHighwatermark); + Mutex::Autolock autoLock(instance->mLock); + (new AMessage(kWhatFetchMore, instance->mReflector->id()))->post(); + return instance; +} + status_t NuCachedSource2::getEstimatedBandwidthKbps(int32_t *kbps) { if (mSource->flags() & kIsHTTPBasedSource) { HTTPBase* source = static_cast(mSource.get()); diff --git a/media/libstagefright/include/NuCachedSource2.h b/media/libstagefright/include/NuCachedSource2.h index 5db4b4b..949a49e 100644 --- a/media/libstagefright/include/NuCachedSource2.h +++ b/media/libstagefright/include/NuCachedSource2.h @@ -28,7 +28,7 @@ struct ALooper; struct PageCache; struct NuCachedSource2 : public DataSource { - NuCachedSource2( + static sp Create( const sp &source, const char *cacheConfig = NULL, bool disconnectAtHighwatermark = false); @@ -70,6 +70,11 @@ protected: private: friend struct AHandlerReflector; + NuCachedSource2( + const sp &source, + const char *cacheConfig, + bool disconnectAtHighwatermark); + enum { kPageSize = 65536, kDefaultHighWaterThreshold = 20 * 1024 * 1024, -- cgit v1.1 From 0d35dd2068d6422c3c77fb68f248cbabf3d0b10c Mon Sep 17 00:00:00 2001 From: Wonsik Kim Date: Tue, 8 Sep 2015 17:32:28 +0900 Subject: DO NOT MERGE NuCachedSource2: fix possible erroneous early free Because the constructor of NuCachedSource2 sent a message to AHandlerReflector object, AHandlerReflector::onMessageReceived could have executed just before the object gets wrapped in a strong pointer, resulting in erroneous early free. Fix the issue by using static Create function to ensure the message is sent after the object is wrapped in a sp. Bug: 23882800 Change-Id: I38a9d7a3083f184b4c81d0b00ba1661721278855 --- media/libstagefright/AwesomePlayer.cpp | 4 ++-- media/libstagefright/DataSource.cpp | 2 +- media/libstagefright/NuCachedSource2.cpp | 15 ++++++++++++--- media/libstagefright/include/NuCachedSource2.h | 7 ++++++- 4 files changed, 21 insertions(+), 7 deletions(-) (limited to 'media') diff --git a/media/libstagefright/AwesomePlayer.cpp b/media/libstagefright/AwesomePlayer.cpp index ab8ac79..493e598 100644 --- a/media/libstagefright/AwesomePlayer.cpp +++ b/media/libstagefright/AwesomePlayer.cpp @@ -2273,11 +2273,11 @@ status_t AwesomePlayer::finishSetDataSource_l() { // The widevine extractor does its own caching. #if 0 - mCachedSource = new NuCachedSource2( + mCachedSource = NuCachedSource2::Create( new ThrottledSource( mConnectingDataSource, 50 * 1024 /* bytes/sec */)); #else - mCachedSource = new NuCachedSource2( + mCachedSource = NuCachedSource2::Create( mConnectingDataSource, cacheConfig.isEmpty() ? NULL : cacheConfig.string(), disconnectAtHighwatermark); diff --git a/media/libstagefright/DataSource.cpp b/media/libstagefright/DataSource.cpp index c99db84..65ad0f1 100644 --- a/media/libstagefright/DataSource.cpp +++ b/media/libstagefright/DataSource.cpp @@ -243,7 +243,7 @@ sp DataSource::CreateFromURI( *contentType = httpSource->getMIMEType(); } - source = new NuCachedSource2( + source = NuCachedSource2::Create( httpSource, cacheConfig.isEmpty() ? NULL : cacheConfig.string(), disconnectAtHighwatermark); diff --git a/media/libstagefright/NuCachedSource2.cpp b/media/libstagefright/NuCachedSource2.cpp index bd0a41d..cbc1fe3 100644 --- a/media/libstagefright/NuCachedSource2.cpp +++ b/media/libstagefright/NuCachedSource2.cpp @@ -224,9 +224,6 @@ NuCachedSource2::NuCachedSource2( // So whenever we call DataSource::readAt it may end up in a call to // IMediaHTTPConnection::readAt and therefore call back into JAVA. mLooper->start(false /* runOnCallingThread */, true /* canCallJava */); - - Mutex::Autolock autoLock(mLock); - (new AMessage(kWhatFetchMore, mReflector->id()))->post(); } NuCachedSource2::~NuCachedSource2() { @@ -237,6 +234,18 @@ NuCachedSource2::~NuCachedSource2() { mCache = NULL; } +// static +sp NuCachedSource2::Create( + const sp &source, + const char *cacheConfig, + bool disconnectAtHighwatermark) { + sp instance = new NuCachedSource2( + source, cacheConfig, disconnectAtHighwatermark); + Mutex::Autolock autoLock(instance->mLock); + (new AMessage(kWhatFetchMore, instance->mReflector->id()))->post(); + return instance; +} + status_t NuCachedSource2::getEstimatedBandwidthKbps(int32_t *kbps) { if (mSource->flags() & kIsHTTPBasedSource) { HTTPBase* source = static_cast(mSource.get()); diff --git a/media/libstagefright/include/NuCachedSource2.h b/media/libstagefright/include/NuCachedSource2.h index 4252706..a29bdf9 100644 --- a/media/libstagefright/include/NuCachedSource2.h +++ b/media/libstagefright/include/NuCachedSource2.h @@ -28,7 +28,7 @@ struct ALooper; struct PageCache; struct NuCachedSource2 : public DataSource { - NuCachedSource2( + static sp Create( const sp &source, const char *cacheConfig = NULL, bool disconnectAtHighwatermark = false); @@ -72,6 +72,11 @@ protected: private: friend struct AHandlerReflector; + NuCachedSource2( + const sp &source, + const char *cacheConfig, + bool disconnectAtHighwatermark); + enum { kPageSize = 65536, kDefaultHighWaterThreshold = 20 * 1024 * 1024, -- cgit v1.1 From 9adc7283c84cea1be81d5bd55ce50aefa6328c6e Mon Sep 17 00:00:00 2001 From: Jeff Tinker Date: Mon, 14 Sep 2015 10:18:56 -0700 Subject: Fix for security vulnerability in media server DO NOT MERGE bug: 23540426 Change-Id: I5d602f99fd82e50d0136d47ce20cfa1ac9fd7ae2 --- media/libmedia/ICrypto.cpp | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) (limited to 'media') diff --git a/media/libmedia/ICrypto.cpp b/media/libmedia/ICrypto.cpp index bff4639..2053c45 100644 --- a/media/libmedia/ICrypto.cpp +++ b/media/libmedia/ICrypto.cpp @@ -255,7 +255,28 @@ status_t BnCrypto::onTransact( } AString errorDetailMsg; - ssize_t result = decrypt( + ssize_t result; + + size_t sumSubsampleSizes = 0; + bool overflow = false; + for (int32_t i = 0; i < numSubSamples; ++i) { + CryptoPlugin::SubSample &ss = subSamples[i]; + if (sumSubsampleSizes <= SIZE_MAX - ss.mNumBytesOfEncryptedData) { + sumSubsampleSizes += ss.mNumBytesOfEncryptedData; + } else { + overflow = true; + } + if (sumSubsampleSizes <= SIZE_MAX - ss.mNumBytesOfClearData) { + sumSubsampleSizes += ss.mNumBytesOfClearData; + } else { + overflow = true; + } + } + + if (overflow || sumSubsampleSizes != totalSize) { + result = -EINVAL; + } else { + result = decrypt( secure, key, iv, @@ -264,6 +285,7 @@ status_t BnCrypto::onTransact( subSamples, numSubSamples, secure ? secureBufferId : dstPtr, &errorDetailMsg); + } reply->writeInt32(result); -- cgit v1.1 From 4b219e9e5ab237eec9931497cf10db4d78982d84 Mon Sep 17 00:00:00 2001 From: Jeff Tinker Date: Mon, 14 Sep 2015 11:55:43 -0700 Subject: Fix for security vulnerability in media server DO NOT MERGE bug: 23540426 Change-Id: I7ca419e4008967a0387649e5293ac9d4be71d3c4 --- media/libmedia/ICrypto.cpp | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) (limited to 'media') diff --git a/media/libmedia/ICrypto.cpp b/media/libmedia/ICrypto.cpp index 7bd120e..0d68ee7 100644 --- a/media/libmedia/ICrypto.cpp +++ b/media/libmedia/ICrypto.cpp @@ -255,7 +255,28 @@ status_t BnCrypto::onTransact( } AString errorDetailMsg; - ssize_t result = decrypt( + ssize_t result; + + size_t sumSubsampleSizes = 0; + bool overflow = false; + for (int32_t i = 0; i < numSubSamples; ++i) { + CryptoPlugin::SubSample &ss = subSamples[i]; + if (sumSubsampleSizes <= SIZE_MAX - ss.mNumBytesOfEncryptedData) { + sumSubsampleSizes += ss.mNumBytesOfEncryptedData; + } else { + overflow = true; + } + if (sumSubsampleSizes <= SIZE_MAX - ss.mNumBytesOfClearData) { + sumSubsampleSizes += ss.mNumBytesOfClearData; + } else { + overflow = true; + } + } + + if (overflow || sumSubsampleSizes != totalSize) { + result = -EINVAL; + } else { + result = decrypt( secure, key, iv, @@ -264,6 +285,7 @@ status_t BnCrypto::onTransact( subSamples, numSubSamples, secure ? secureBufferId : dstPtr, &errorDetailMsg); + } reply->writeInt32(result); -- cgit v1.1 From 8336fc433eee213e201c8a65bf2a65b65bc67c44 Mon Sep 17 00:00:00 2001 From: Wei Jia Date: Thu, 10 Sep 2015 09:47:29 -0700 Subject: DO NOT MERGE - IAudioFlinger: always initialize variables to ensure no info leak when writing them to Parcel. Bug: 23953967 Change-Id: Ibbe841da149038675e9e8daea76c77558bc8564b (cherry picked from commit 983dca391a76fb45df999fc40e8766b9ddb63511) --- media/libmedia/IAudioFlinger.cpp | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) (limited to 'media') diff --git a/media/libmedia/IAudioFlinger.cpp b/media/libmedia/IAudioFlinger.cpp index adc7186..98d3dee 100644 --- a/media/libmedia/IAudioFlinger.cpp +++ b/media/libmedia/IAudioFlinger.cpp @@ -766,7 +766,7 @@ status_t BnAudioFlinger::onTransact( int sessionId = data.readInt32(); int clientUid = data.readInt32(); String8 name; - status_t status; + status_t status = NO_ERROR; sp track; if ((haveSharedBuffer && (buffer == 0)) || ((buffer != 0) && (buffer->pointer() == NULL))) { @@ -795,7 +795,7 @@ status_t BnAudioFlinger::onTransact( track_flags_t flags = (track_flags_t) data.readInt32(); pid_t tid = (pid_t) data.readInt32(); int sessionId = data.readInt32(); - status_t status; + status_t status = NO_ERROR; sp record = openRecord(input, sampleRate, format, channelMask, frameCount, &flags, tid, &sessionId, &status); LOG_ALWAYS_FATAL_IF((record != 0) != (status == NO_ERROR)); @@ -1013,8 +1013,8 @@ status_t BnAudioFlinger::onTransact( case GET_RENDER_POSITION: { CHECK_INTERFACE(IAudioFlinger, data, reply); audio_io_handle_t output = (audio_io_handle_t) data.readInt32(); - size_t halFrames; - size_t dspFrames; + size_t halFrames = 0; + size_t dspFrames = 0; status_t status = getRenderPosition(&halFrames, &dspFrames, output); reply->writeInt32(status); if (status == NO_ERROR) { @@ -1048,7 +1048,7 @@ status_t BnAudioFlinger::onTransact( } break; case QUERY_NUM_EFFECTS: { CHECK_INTERFACE(IAudioFlinger, data, reply); - uint32_t numEffects; + uint32_t numEffects = 0; status_t status = queryNumberEffects(&numEffects); reply->writeInt32(status); if (status == NO_ERROR) { @@ -1058,7 +1058,7 @@ status_t BnAudioFlinger::onTransact( } case QUERY_EFFECT: { CHECK_INTERFACE(IAudioFlinger, data, reply); - effect_descriptor_t desc; + effect_descriptor_t desc = {}; status_t status = queryEffect(data.readInt32(), &desc); reply->writeInt32(status); if (status == NO_ERROR) { @@ -1070,7 +1070,7 @@ status_t BnAudioFlinger::onTransact( CHECK_INTERFACE(IAudioFlinger, data, reply); effect_uuid_t uuid; data.read(&uuid, sizeof(effect_uuid_t)); - effect_descriptor_t desc; + effect_descriptor_t desc = {}; status_t status = getEffectDescriptor(&uuid, &desc); reply->writeInt32(status); if (status == NO_ERROR) { @@ -1088,9 +1088,9 @@ status_t BnAudioFlinger::onTransact( int32_t priority = data.readInt32(); audio_io_handle_t output = (audio_io_handle_t) data.readInt32(); int sessionId = data.readInt32(); - status_t status; - int id; - int enabled; + status_t status = NO_ERROR; + int id = 0; + int enabled = 0; sp effect = createEffect(&desc, client, priority, output, sessionId, &status, &id, &enabled); -- cgit v1.1 From 3b76870d146b1350db8a2f7797e06897c8c92dc2 Mon Sep 17 00:00:00 2001 From: Wei Jia Date: Thu, 10 Sep 2015 09:47:29 -0700 Subject: DO NOT MERGE - IAudioFlinger: always initialize variables to ensure no info leak when writing them to Parcel. Bug: 23953967 Change-Id: Ibbe841da149038675e9e8daea76c77558bc8564b (cherry picked from commit 983dca391a76fb45df999fc40e8766b9ddb63511) --- media/libmedia/IAudioFlinger.cpp | 22 +++++++++++----------- media/libmedia/IAudioPolicyService.cpp | 6 +++--- 2 files changed, 14 insertions(+), 14 deletions(-) (limited to 'media') diff --git a/media/libmedia/IAudioFlinger.cpp b/media/libmedia/IAudioFlinger.cpp index d31de58..a6ed199 100644 --- a/media/libmedia/IAudioFlinger.cpp +++ b/media/libmedia/IAudioFlinger.cpp @@ -924,7 +924,7 @@ status_t BnAudioFlinger::onTransact( pid_t tid = (pid_t) data.readInt32(); int sessionId = data.readInt32(); int clientUid = data.readInt32(); - status_t status; + status_t status = NO_ERROR; sp track; if ((haveSharedBuffer && (buffer == 0)) || ((buffer != 0) && (buffer->pointer() == NULL))) { @@ -957,7 +957,7 @@ status_t BnAudioFlinger::onTransact( size_t notificationFrames = data.readInt64(); sp cblk; sp buffers; - status_t status; + status_t status = NO_ERROR; sp record = openRecord(input, sampleRate, format, channelMask, &frameCount, &flags, tid, &sessionId, ¬ificationFrames, @@ -1097,7 +1097,7 @@ status_t BnAudioFlinger::onTransact( audio_devices_t devices = (audio_devices_t)data.readInt32(); String8 address(data.readString8()); audio_output_flags_t flags = (audio_output_flags_t) data.readInt32(); - uint32_t latencyMs; + uint32_t latencyMs = 0; audio_io_handle_t output; status_t status = openOutput(module, &output, &config, &devices, address, &latencyMs, flags); @@ -1176,8 +1176,8 @@ status_t BnAudioFlinger::onTransact( case GET_RENDER_POSITION: { CHECK_INTERFACE(IAudioFlinger, data, reply); audio_io_handle_t output = (audio_io_handle_t) data.readInt32(); - uint32_t halFrames; - uint32_t dspFrames; + uint32_t halFrames = 0; + uint32_t dspFrames = 0; status_t status = getRenderPosition(&halFrames, &dspFrames, output); reply->writeInt32(status); if (status == NO_ERROR) { @@ -1213,7 +1213,7 @@ status_t BnAudioFlinger::onTransact( } break; case QUERY_NUM_EFFECTS: { CHECK_INTERFACE(IAudioFlinger, data, reply); - uint32_t numEffects; + uint32_t numEffects = 0; status_t status = queryNumberEffects(&numEffects); reply->writeInt32(status); if (status == NO_ERROR) { @@ -1223,7 +1223,7 @@ status_t BnAudioFlinger::onTransact( } case QUERY_EFFECT: { CHECK_INTERFACE(IAudioFlinger, data, reply); - effect_descriptor_t desc; + effect_descriptor_t desc = {}; status_t status = queryEffect(data.readInt32(), &desc); reply->writeInt32(status); if (status == NO_ERROR) { @@ -1235,7 +1235,7 @@ status_t BnAudioFlinger::onTransact( CHECK_INTERFACE(IAudioFlinger, data, reply); effect_uuid_t uuid; data.read(&uuid, sizeof(effect_uuid_t)); - effect_descriptor_t desc; + effect_descriptor_t desc = {}; status_t status = getEffectDescriptor(&uuid, &desc); reply->writeInt32(status); if (status == NO_ERROR) { @@ -1253,9 +1253,9 @@ status_t BnAudioFlinger::onTransact( int32_t priority = data.readInt32(); audio_io_handle_t output = (audio_io_handle_t) data.readInt32(); int sessionId = data.readInt32(); - status_t status; - int id; - int enabled; + status_t status = NO_ERROR; + int id = 0; + int enabled = 0; sp effect = createEffect(&desc, client, priority, output, sessionId, &status, &id, &enabled); diff --git a/media/libmedia/IAudioPolicyService.cpp b/media/libmedia/IAudioPolicyService.cpp index e6e01aa..e840956 100644 --- a/media/libmedia/IAudioPolicyService.cpp +++ b/media/libmedia/IAudioPolicyService.cpp @@ -1071,9 +1071,9 @@ status_t BnAudioPolicyService::onTransact( CHECK_INTERFACE(IAudioPolicyService, data, reply); sp client = interface_cast( data.readStrongBinder()); - audio_session_t session; - audio_io_handle_t ioHandle; - audio_devices_t device; + audio_session_t session = {}; + audio_io_handle_t ioHandle = {}; + audio_devices_t device = {}; status_t status = acquireSoundTriggerSession(&session, &ioHandle, &device); reply->writeInt32(status); if (status == NO_ERROR) { -- cgit v1.1 From 636539eb1a0d407d7f82b7c9a6d9833f7715e287 Mon Sep 17 00:00:00 2001 From: Jeff Tinker Date: Wed, 16 Sep 2015 10:23:12 -0700 Subject: DO NOT MERGE Fix vulnerability in mediaserver ICrypto.cpp: ASLR bypass using DECRYPT IPC bug: 24074485 Change-Id: I40dd0e92083c7093030393b16dbab59323306a4e --- media/libmedia/ICrypto.cpp | 2 ++ 1 file changed, 2 insertions(+) (limited to 'media') diff --git a/media/libmedia/ICrypto.cpp b/media/libmedia/ICrypto.cpp index bff4639..a88b393 100644 --- a/media/libmedia/ICrypto.cpp +++ b/media/libmedia/ICrypto.cpp @@ -236,6 +236,7 @@ status_t BnCrypto::onTransact( size_t totalSize = data.readInt32(); void *srcData = malloc(totalSize); + memset(srcData, 0, totalSize); data.read(srcData, totalSize); int32_t numSubSamples = data.readInt32(); @@ -252,6 +253,7 @@ status_t BnCrypto::onTransact( secureBufferId = (void *)data.readIntPtr(); } else { dstPtr = malloc(totalSize); + memset(dstPtr, 0, totalSize); } AString errorDetailMsg; -- cgit v1.1 From 5e7e87a383fdb1fece977097a7e3cc51b296f3a0 Mon Sep 17 00:00:00 2001 From: Jeff Tinker Date: Wed, 16 Sep 2015 10:26:28 -0700 Subject: DO NOT MERGE Fix vulnerability in mediaserver ICrypto.cpp: ASLR bypass using DECRYPT IPC bug: 24074485 Change-Id: Ia12942d6b86adde28745908d36a728ab5d69a037 --- media/libmedia/ICrypto.cpp | 2 ++ 1 file changed, 2 insertions(+) (limited to 'media') diff --git a/media/libmedia/ICrypto.cpp b/media/libmedia/ICrypto.cpp index 7bd120e..505d782 100644 --- a/media/libmedia/ICrypto.cpp +++ b/media/libmedia/ICrypto.cpp @@ -236,6 +236,7 @@ status_t BnCrypto::onTransact( size_t totalSize = data.readInt32(); void *srcData = malloc(totalSize); + memset(srcData, 0, totalSize); data.read(srcData, totalSize); int32_t numSubSamples = data.readInt32(); @@ -252,6 +253,7 @@ status_t BnCrypto::onTransact( secureBufferId = reinterpret_cast(static_cast(data.readInt64())); } else { dstPtr = malloc(totalSize); + memset(dstPtr, 0, totalSize); } AString errorDetailMsg; -- cgit v1.1 From 4cac44b53cc9f965cc2c9706b1d7ee2cd79f4066 Mon Sep 17 00:00:00 2001 From: Wei Jia Date: Wed, 16 Sep 2015 15:01:16 -0700 Subject: IAudioFlinger: fix the missing initialization of variable to ensure no info leak when writing them to Parcel. Bug: 23953967 Change-Id: I3a1d0144ba3832649e322c197ff0f03305ee7829 --- media/libmedia/IAudioFlinger.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'media') diff --git a/media/libmedia/IAudioFlinger.cpp b/media/libmedia/IAudioFlinger.cpp index 47ed359..0bf503a 100644 --- a/media/libmedia/IAudioFlinger.cpp +++ b/media/libmedia/IAudioFlinger.cpp @@ -1112,7 +1112,7 @@ status_t BnAudioFlinger::onTransact( String8 address(data.readString8()); audio_output_flags_t flags = (audio_output_flags_t) data.readInt32(); uint32_t latencyMs = 0; - audio_io_handle_t output; + audio_io_handle_t output = AUDIO_IO_HANDLE_NONE; status_t status = openOutput(module, &output, &config, &devices, address, &latencyMs, flags); ALOGV("OPEN_OUTPUT output, %d", output); -- cgit v1.1 From 3fd87605288d6d8e5abebadbddfa6071387fecdd Mon Sep 17 00:00:00 2001 From: Jeff Tinker Date: Wed, 16 Sep 2015 13:21:51 -0700 Subject: DO NOT MERGE Fix vulnerability in mediaserver ICrypto.cpp: ASLR bypass using DECRYPT IPC bug: 24074485 Change-Id: I61cd77f0894140547f666a80526ebfe1ec3d2db6 --- media/libmedia/ICrypto.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'media') diff --git a/media/libmedia/ICrypto.cpp b/media/libmedia/ICrypto.cpp index 9703b0d..a398ff7 100644 --- a/media/libmedia/ICrypto.cpp +++ b/media/libmedia/ICrypto.cpp @@ -297,7 +297,7 @@ status_t BnCrypto::onTransact( if (secure) { secureBufferId = reinterpret_cast(static_cast(data.readInt64())); } else { - dstPtr = malloc(totalSize); + dstPtr = calloc(1, totalSize); } AString errorDetailMsg; -- cgit v1.1 From 40715a2ee896edd2df4023d9f6f586977887d34c Mon Sep 17 00:00:00 2001 From: Wei Jia Date: Wed, 16 Sep 2015 15:01:16 -0700 Subject: IAudioFlinger: fix the missing initialization of variable to ensure no info leak when writing them to Parcel. Bug: 23953967 Change-Id: I3a1d0144ba3832649e322c197ff0f03305ee7829 (cherry picked from commit 4cac44b53cc9f965cc2c9706b1d7ee2cd79f4066) --- media/libmedia/IAudioFlinger.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'media') diff --git a/media/libmedia/IAudioFlinger.cpp b/media/libmedia/IAudioFlinger.cpp index a6ed199..ac2d1c9 100644 --- a/media/libmedia/IAudioFlinger.cpp +++ b/media/libmedia/IAudioFlinger.cpp @@ -1098,7 +1098,7 @@ status_t BnAudioFlinger::onTransact( String8 address(data.readString8()); audio_output_flags_t flags = (audio_output_flags_t) data.readInt32(); uint32_t latencyMs = 0; - audio_io_handle_t output; + audio_io_handle_t output = AUDIO_IO_HANDLE_NONE; status_t status = openOutput(module, &output, &config, &devices, address, &latencyMs, flags); ALOGV("OPEN_OUTPUT output, %d", output); -- cgit v1.1 From 2a886d196ae717adc353a9fb4371b6a5abbd89a5 Mon Sep 17 00:00:00 2001 From: Lajos Molnar Date: Thu, 17 Sep 2015 17:01:52 -0700 Subject: stagefright: only pass valid framerates in msg and meta Bug: 21573897 Change-Id: Ide83419fa0c92726a33bad2570321e0df2558429 --- media/libstagefright/Utils.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'media') diff --git a/media/libstagefright/Utils.cpp b/media/libstagefright/Utils.cpp index 7c8d441..17f0201 100644 --- a/media/libstagefright/Utils.cpp +++ b/media/libstagefright/Utils.cpp @@ -199,7 +199,7 @@ status_t convertMetaDataToMessage( } int32_t fps; - if (meta->findInt32(kKeyFrameRate, &fps)) { + if (meta->findInt32(kKeyFrameRate, &fps) && fps > 0) { msg->setInt32("frame-rate", fps); } @@ -664,7 +664,7 @@ void convertMessageToMetaData(const sp &msg, sp &meta) { } int32_t fps; - if (msg->findInt32("frame-rate", &fps)) { + if (msg->findInt32("frame-rate", &fps) && fps > 0) { meta->setInt32(kKeyFrameRate, fps); } -- cgit v1.1 From e1c9766e23a1e5084d9fdbcc0b2005406627f1d4 Mon Sep 17 00:00:00 2001 From: Sungmin Choi Date: Sun, 13 Sep 2015 18:48:13 -0700 Subject: stagefright: Move google mpeg2 codec into media_codecs_google_tv.xml Only TV devices need to support MPEG2 decoder. Bug: 23885483 Change-Id: Ic9dd03f9433c41bda1b5b6be3285499ebfd1d0e6 --- .../libstagefright/data/media_codecs_google_tv.xml | 29 ++++++++++++++++++++++ .../data/media_codecs_google_video.xml | 9 ------- 2 files changed, 29 insertions(+), 9 deletions(-) create mode 100644 media/libstagefright/data/media_codecs_google_tv.xml mode change 100755 => 100644 media/libstagefright/data/media_codecs_google_video.xml (limited to 'media') diff --git a/media/libstagefright/data/media_codecs_google_tv.xml b/media/libstagefright/data/media_codecs_google_tv.xml new file mode 100644 index 0000000..9b4a1ac --- /dev/null +++ b/media/libstagefright/data/media_codecs_google_tv.xml @@ -0,0 +1,29 @@ + + + + + + + + + + + + + + + + diff --git a/media/libstagefright/data/media_codecs_google_video.xml b/media/libstagefright/data/media_codecs_google_video.xml old mode 100755 new mode 100644 index 740f96b..81a6d00 --- a/media/libstagefright/data/media_codecs_google_video.xml +++ b/media/libstagefright/data/media_codecs_google_video.xml @@ -16,15 +16,6 @@ - - - - - - - - - -- cgit v1.1 From 12d4a6a2636f41d1ee1bc10a23df13ce09efbff6 Mon Sep 17 00:00:00 2001 From: Wonsik Kim Date: Mon, 7 Sep 2015 15:52:27 +0900 Subject: DO NOT MERGE Avoid size_t overflow in base64 decoding once again Switch to foundation base64 function in OggExtractor and fix the issue there. Bug: 23707088 Change-Id: If8ba3347c213fe7a36668c943ed264f2871ad468 --- media/libstagefright/OggExtractor.cpp | 102 ++++------------------------- media/libstagefright/foundation/base64.cpp | 11 +++- 2 files changed, 20 insertions(+), 93 deletions(-) (limited to 'media') diff --git a/media/libstagefright/OggExtractor.cpp b/media/libstagefright/OggExtractor.cpp index d5c929e..578171f 100644 --- a/media/libstagefright/OggExtractor.cpp +++ b/media/libstagefright/OggExtractor.cpp @@ -23,6 +23,7 @@ #include #include #include +#include #include #include #include @@ -1202,93 +1203,18 @@ void parseVorbisComment( } -// The returned buffer should be free()d. -static uint8_t *DecodeBase64(const char *s, size_t size, size_t *outSize) { - *outSize = 0; - - if ((size % 4) != 0) { - return NULL; - } - - size_t n = size; - size_t padding = 0; - if (n >= 1 && s[n - 1] == '=') { - padding = 1; - - if (n >= 2 && s[n - 2] == '=') { - padding = 2; - } - } - - // We divide first to avoid overflow. It's OK to do this because we - // already made sure that size % 4 == 0. - size_t outLen = (size / 4) * 3 - padding; - - void *buffer = malloc(outLen); - if (buffer == NULL) { - return NULL; - } - - uint8_t *out = (uint8_t *)buffer; - size_t j = 0; - uint32_t accum = 0; - for (size_t i = 0; i < n; ++i) { - char c = s[i]; - unsigned value; - if (c >= 'A' && c <= 'Z') { - value = c - 'A'; - } else if (c >= 'a' && c <= 'z') { - value = 26 + c - 'a'; - } else if (c >= '0' && c <= '9') { - value = 52 + c - '0'; - } else if (c == '+') { - value = 62; - } else if (c == '/') { - value = 63; - } else if (c != '=') { - break; - } else { - if (i < n - padding) { - break; - } - - value = 0; - } - - accum = (accum << 6) | value; - - if (((i + 1) % 4) == 0) { - out[j++] = (accum >> 16); - - if (j < outLen) { out[j++] = (accum >> 8) & 0xff; } - if (j < outLen) { out[j++] = accum & 0xff; } - - accum = 0; - } - } - - // Check if we exited the loop early. - if (j < outLen) { - free(buffer); - return NULL; - } - - *outSize = outLen; - return (uint8_t *)buffer; -} - static void extractAlbumArt( const sp &fileMeta, const void *data, size_t size) { ALOGV("extractAlbumArt from '%s'", (const char *)data); - size_t flacSize; - uint8_t *flac = DecodeBase64((const char *)data, size, &flacSize); - - if (flac == NULL) { + sp flacBuffer = decodeBase64(AString((const char *)data, size)); + if (flacBuffer == NULL) { ALOGE("malformed base64 encoded data."); return; } + size_t flacSize = flacBuffer->size(); + uint8_t *flac = flacBuffer->data(); ALOGV("got flac of size %zu", flacSize); uint32_t picType; @@ -1298,24 +1224,24 @@ static void extractAlbumArt( char type[128]; if (flacSize < 8) { - goto exit; + return; } picType = U32_AT(flac); if (picType != 3) { // This is not a front cover. - goto exit; + return; } typeLen = U32_AT(&flac[4]); if (typeLen > sizeof(type) - 1) { - goto exit; + return; } // we've already checked above that flacSize >= 8 if (flacSize - 8 < typeLen) { - goto exit; + return; } memcpy(type, &flac[8], typeLen); @@ -1325,7 +1251,7 @@ static void extractAlbumArt( if (!strcmp(type, "-->")) { // This is not inline cover art, but an external url instead. - goto exit; + return; } descLen = U32_AT(&flac[8 + typeLen]); @@ -1333,7 +1259,7 @@ static void extractAlbumArt( if (flacSize < 32 || flacSize - 32 < typeLen || flacSize - 32 - typeLen < descLen) { - goto exit; + return; } dataLen = U32_AT(&flac[8 + typeLen + 4 + descLen + 16]); @@ -1341,7 +1267,7 @@ static void extractAlbumArt( // we've already checked above that (flacSize - 32 - typeLen - descLen) >= 0 if (flacSize - 32 - typeLen - descLen < dataLen) { - goto exit; + return; } ALOGV("got image data, %zu trailing bytes", @@ -1351,10 +1277,6 @@ static void extractAlbumArt( kKeyAlbumArt, 0, &flac[8 + typeLen + 4 + descLen + 20], dataLen); fileMeta->setCString(kKeyAlbumArtMIME, type); - -exit: - free(flac); - flac = NULL; } //////////////////////////////////////////////////////////////////////////////// diff --git a/media/libstagefright/foundation/base64.cpp b/media/libstagefright/foundation/base64.cpp index dcf5bef..7da7db9 100644 --- a/media/libstagefright/foundation/base64.cpp +++ b/media/libstagefright/foundation/base64.cpp @@ -22,11 +22,11 @@ namespace android { sp decodeBase64(const AString &s) { - if ((s.size() % 4) != 0) { + size_t n = s.size(); + if ((n % 4) != 0) { return NULL; } - size_t n = s.size(); size_t padding = 0; if (n >= 1 && s.c_str()[n - 1] == '=') { padding = 1; @@ -40,11 +40,16 @@ sp decodeBase64(const AString &s) { } } - size_t outLen = 3 * s.size() / 4 - padding; + // We divide first to avoid overflow. It's OK to do this because we + // already made sure that n % 4 == 0. + size_t outLen = (n / 4) * 3 - padding; sp buffer = new ABuffer(outLen); uint8_t *out = buffer->data(); + if (out == NULL || buffer->size() < outLen) { + return NULL; + } size_t j = 0; uint32_t accum = 0; for (size_t i = 0; i < n; ++i) { -- cgit v1.1 From ed78e2fdf53b72be4647be88a02a120869415015 Mon Sep 17 00:00:00 2001 From: Wei Jia Date: Fri, 18 Sep 2015 10:04:09 -0700 Subject: StagefrightMetadataRetriever: handle error returned from convertMetaDataToMessage(). Bug: 23680780 Change-Id: I09dbbf95b2c874b9760938646e48a7ed543f1577 --- media/libstagefright/StagefrightMetadataRetriever.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) (limited to 'media') diff --git a/media/libstagefright/StagefrightMetadataRetriever.cpp b/media/libstagefright/StagefrightMetadataRetriever.cpp index 7c554db..e37e909 100644 --- a/media/libstagefright/StagefrightMetadataRetriever.cpp +++ b/media/libstagefright/StagefrightMetadataRetriever.cpp @@ -146,7 +146,10 @@ static VideoFrame *extractVideoFrame( sp format = source->getFormat(); sp videoFormat; - convertMetaDataToMessage(trackMeta, &videoFormat); + if (convertMetaDataToMessage(trackMeta, &videoFormat) != OK) { + ALOGW("Failed to convert meta data to message"); + return NULL; + } // TODO: Use Flexible color instead videoFormat->setInt32("color-format", OMX_COLOR_FormatYUV420Planar); -- cgit v1.1 From 6f7fb4012b51b8dbebb53e88f8d8d1ee4f717f29 Mon Sep 17 00:00:00 2001 From: Ian Pedowitz Date: Fri, 18 Sep 2015 22:48:19 -0700 Subject: Fix Build Decoders/Encoders tags were mismatched when ag/770962 was submitted Change-Id: I58654b5df172d28ac4ccebde1a615c485479a2db --- media/libstagefright/data/media_codecs_google_tv.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'media') diff --git a/media/libstagefright/data/media_codecs_google_tv.xml b/media/libstagefright/data/media_codecs_google_tv.xml index 9b4a1ac..330c6fb 100644 --- a/media/libstagefright/data/media_codecs_google_tv.xml +++ b/media/libstagefright/data/media_codecs_google_tv.xml @@ -25,5 +25,5 @@ - + -- cgit v1.1 From e22f5f490cb69e30e8a2630868e58db41838bcb3 Mon Sep 17 00:00:00 2001 From: Andy Hung Date: Tue, 22 Sep 2015 12:01:23 -0700 Subject: DO NOT MERGE - Fix build for commit 69ae6a87 test app on lmp-mr1-ub-dev but not on lmp-mr1-dev now needs liblog. Change-Id: Ia995d9101e85f6a68c85d3156fd6b78fa96c87e0 --- media/libstagefright/codecs/amrnb/dec/Android.mk | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'media') diff --git a/media/libstagefright/codecs/amrnb/dec/Android.mk b/media/libstagefright/codecs/amrnb/dec/Android.mk index 3750e2e..76a7f40 100644 --- a/media/libstagefright/codecs/amrnb/dec/Android.mk +++ b/media/libstagefright/codecs/amrnb/dec/Android.mk @@ -98,7 +98,7 @@ LOCAL_STATIC_LIBRARIES := \ libstagefright_amrnbdec libsndfile LOCAL_SHARED_LIBRARIES := \ - libstagefright_amrnb_common libaudioutils + libstagefright_amrnb_common libaudioutils liblog LOCAL_MODULE := libstagefright_amrnbdec_test LOCAL_MODULE_TAGS := optional -- cgit v1.1 From 6eda0b5770eeb215d0a37ef1478c5541bd8067fd Mon Sep 17 00:00:00 2001 From: Harish Mahendrakar Date: Tue, 4 Aug 2015 10:12:58 +0530 Subject: SoftAVCDec: Reduced memory requirements Bug: 24221026 Change-Id: I2aaaf88e7dc9a30156764f83a81fc0cad00142f9 --- media/libstagefright/codecs/avcdec/SoftAVCDec.cpp | 364 +++++----------------- media/libstagefright/codecs/avcdec/SoftAVCDec.h | 16 +- 2 files changed, 84 insertions(+), 296 deletions(-) (limited to 'media') diff --git a/media/libstagefright/codecs/avcdec/SoftAVCDec.cpp b/media/libstagefright/codecs/avcdec/SoftAVCDec.cpp index e083315..7c5093e 100644 --- a/media/libstagefright/codecs/avcdec/SoftAVCDec.cpp +++ b/media/libstagefright/codecs/avcdec/SoftAVCDec.cpp @@ -38,10 +38,10 @@ namespace android { /** Function and structure definitions to keep code similar for each codec */ #define ivdec_api_function ih264d_api_function -#define ivdext_init_ip_t ih264d_init_ip_t -#define ivdext_init_op_t ih264d_init_op_t -#define ivdext_fill_mem_rec_ip_t ih264d_fill_mem_rec_ip_t -#define ivdext_fill_mem_rec_op_t ih264d_fill_mem_rec_op_t +#define ivdext_create_ip_t ih264d_create_ip_t +#define ivdext_create_op_t ih264d_create_op_t +#define ivdext_delete_ip_t ih264d_delete_ip_t +#define ivdext_delete_op_t ih264d_delete_op_t #define ivdext_ctl_set_num_cores_ip_t ih264d_ctl_set_num_cores_ip_t #define ivdext_ctl_set_num_cores_op_t ih264d_ctl_set_num_cores_op_t @@ -115,15 +115,12 @@ SoftAVC::SoftAVC( 320 /* width */, 240 /* height */, callbacks, appData, component), mCodecCtx(NULL), - mMemRecords(NULL), mFlushOutBuffer(NULL), mOmxColorFormat(OMX_COLOR_FormatYUV420Planar), mIvColorFormat(IV_YUV_420P), - mNewWidth(mWidth), - mNewHeight(mHeight), - mNewLevel(0), mChangingResolution(false), - mSignalledError(false) { + mSignalledError(false), + mStride(mWidth){ initPorts( kNumBuffers, INPUT_BUF_SIZE, kNumBuffers, CODEC_MIME_TYPE); @@ -132,14 +129,23 @@ SoftAVC::SoftAVC( // If input dump is enabled, then open create an empty file GENERATE_FILE_NAMES(); CREATE_DUMP_FILE(mInFile); - - CHECK_EQ(initDecoder(mWidth, mHeight), (status_t)OK); } SoftAVC::~SoftAVC() { CHECK_EQ(deInitDecoder(), (status_t)OK); } +static void *ivd_aligned_malloc(void *ctxt, WORD32 alignment, WORD32 size) { + UNUSED(ctxt); + return memalign(alignment, size); +} + +static void ivd_aligned_free(void *ctxt, void *buf) { + UNUSED(ctxt); + free(buf); + return; +} + static size_t GetCPUCoreCount() { long cpuCoreCount = 1; #if defined(_SC_NPROCESSORS_ONLN) @@ -149,7 +155,7 @@ static size_t GetCPUCoreCount() { cpuCoreCount = sysconf(_SC_NPROC_ONLN); #endif CHECK(cpuCoreCount >= 1); - ALOGD("Number of CPU cores: %ld", cpuCoreCount); + ALOGV("Number of CPU cores: %ld", cpuCoreCount); return (size_t)cpuCoreCount; } @@ -235,12 +241,10 @@ status_t SoftAVC::resetDecoder() { } mSignalledError = false; - /* Set the run-time (dynamic) parameters */ - setParams(outputBufferWidth()); - /* Set number of cores/threads to be used by the codec */ setNumCores(); + mStride = 0; return OK; } @@ -287,160 +291,41 @@ status_t SoftAVC::setFlushMode() { return OK; } -status_t SoftAVC::initDecoder(uint32_t width, uint32_t height) { +status_t SoftAVC::initDecoder() { IV_API_CALL_STATUS_T status; - UWORD32 u4_num_reorder_frames; - UWORD32 u4_num_ref_frames; - UWORD32 u4_share_disp_buf; - WORD32 i4_level; - mNumCores = GetCPUCoreCount(); mCodecCtx = NULL; - /* Initialize number of ref and reorder modes (for H264) */ - u4_num_reorder_frames = 16; - u4_num_ref_frames = 16; - u4_share_disp_buf = 0; - - uint32_t displayStride = mIsAdaptive ? mAdaptiveMaxWidth : width; - uint32_t displayHeight = mIsAdaptive ? mAdaptiveMaxHeight : height; - uint32_t displaySizeY = displayStride * displayHeight; - - if(mNewLevel == 0){ - if (displaySizeY > (1920 * 1088)) { - i4_level = 50; - } else if (displaySizeY > (1280 * 720)) { - i4_level = 40; - } else if (displaySizeY > (720 * 576)) { - i4_level = 31; - } else if (displaySizeY > (624 * 320)) { - i4_level = 30; - } else if (displaySizeY > (352 * 288)) { - i4_level = 21; - } else { - i4_level = 20; - } - } else { - i4_level = mNewLevel; - } - - { - iv_num_mem_rec_ip_t s_num_mem_rec_ip; - iv_num_mem_rec_op_t s_num_mem_rec_op; - - s_num_mem_rec_ip.u4_size = sizeof(s_num_mem_rec_ip); - s_num_mem_rec_op.u4_size = sizeof(s_num_mem_rec_op); - s_num_mem_rec_ip.e_cmd = IV_CMD_GET_NUM_MEM_REC; - - ALOGV("Get number of mem records"); - status = ivdec_api_function( - mCodecCtx, (void *)&s_num_mem_rec_ip, (void *)&s_num_mem_rec_op); - if (IV_SUCCESS != status) { - ALOGE("Error in getting mem records: 0x%x", - s_num_mem_rec_op.u4_error_code); - return UNKNOWN_ERROR; - } - - mNumMemRecords = s_num_mem_rec_op.u4_num_mem_rec; - } - - mMemRecords = (iv_mem_rec_t *)ivd_aligned_malloc( - 128, mNumMemRecords * sizeof(iv_mem_rec_t)); - if (mMemRecords == NULL) { - ALOGE("Allocation failure"); - return NO_MEMORY; - } - - memset(mMemRecords, 0, mNumMemRecords * sizeof(iv_mem_rec_t)); - - { - size_t i; - ivdext_fill_mem_rec_ip_t s_fill_mem_ip; - ivdext_fill_mem_rec_op_t s_fill_mem_op; - iv_mem_rec_t *ps_mem_rec; - - s_fill_mem_ip.s_ivd_fill_mem_rec_ip_t.u4_size = - sizeof(ivdext_fill_mem_rec_ip_t); - s_fill_mem_ip.i4_level = i4_level; - s_fill_mem_ip.u4_num_reorder_frames = u4_num_reorder_frames; - s_fill_mem_ip.u4_num_ref_frames = u4_num_ref_frames; - s_fill_mem_ip.u4_share_disp_buf = u4_share_disp_buf; - s_fill_mem_ip.u4_num_extra_disp_buf = 0; - s_fill_mem_ip.e_output_format = mIvColorFormat; - - s_fill_mem_ip.s_ivd_fill_mem_rec_ip_t.e_cmd = IV_CMD_FILL_NUM_MEM_REC; - s_fill_mem_ip.s_ivd_fill_mem_rec_ip_t.pv_mem_rec_location = mMemRecords; - s_fill_mem_ip.s_ivd_fill_mem_rec_ip_t.u4_max_frm_wd = displayStride; - s_fill_mem_ip.s_ivd_fill_mem_rec_ip_t.u4_max_frm_ht = displayHeight; - s_fill_mem_op.s_ivd_fill_mem_rec_op_t.u4_size = - sizeof(ivdext_fill_mem_rec_op_t); - - ps_mem_rec = mMemRecords; - for (i = 0; i < mNumMemRecords; i++) { - ps_mem_rec[i].u4_size = sizeof(iv_mem_rec_t); - } - - status = ivdec_api_function( - mCodecCtx, (void *)&s_fill_mem_ip, (void *)&s_fill_mem_op); - - if (IV_SUCCESS != status) { - ALOGE("Error in filling mem records: 0x%x", - s_fill_mem_op.s_ivd_fill_mem_rec_op_t.u4_error_code); - return UNKNOWN_ERROR; - } - mNumMemRecords = - s_fill_mem_op.s_ivd_fill_mem_rec_op_t.u4_num_mem_rec_filled; - - ps_mem_rec = mMemRecords; - - for (i = 0; i < mNumMemRecords; i++) { - ps_mem_rec->pv_base = ivd_aligned_malloc( - ps_mem_rec->u4_mem_alignment, ps_mem_rec->u4_mem_size); - if (ps_mem_rec->pv_base == NULL) { - ALOGE("Allocation failure for memory record #%zu of size %u", - i, ps_mem_rec->u4_mem_size); - status = IV_FAIL; - return NO_MEMORY; - } - - ps_mem_rec++; - } - } + mStride = outputBufferWidth(); /* Initialize the decoder */ { - ivdext_init_ip_t s_init_ip; - ivdext_init_op_t s_init_op; + ivdext_create_ip_t s_create_ip; + ivdext_create_op_t s_create_op; void *dec_fxns = (void *)ivdec_api_function; - s_init_ip.s_ivd_init_ip_t.u4_size = sizeof(ivdext_init_ip_t); - s_init_ip.s_ivd_init_ip_t.e_cmd = (IVD_API_COMMAND_TYPE_T)IV_CMD_INIT; - s_init_ip.s_ivd_init_ip_t.pv_mem_rec_location = mMemRecords; - s_init_ip.s_ivd_init_ip_t.u4_frm_max_wd = displayStride; - s_init_ip.s_ivd_init_ip_t.u4_frm_max_ht = displayHeight; + s_create_ip.s_ivd_create_ip_t.u4_size = sizeof(ivdext_create_ip_t); + s_create_ip.s_ivd_create_ip_t.e_cmd = IVD_CMD_CREATE; + s_create_ip.s_ivd_create_ip_t.u4_share_disp_buf = 0; + s_create_op.s_ivd_create_op_t.u4_size = sizeof(ivdext_create_op_t); + s_create_ip.s_ivd_create_ip_t.e_output_format = mIvColorFormat; + s_create_ip.s_ivd_create_ip_t.pf_aligned_alloc = ivd_aligned_malloc; + s_create_ip.s_ivd_create_ip_t.pf_aligned_free = ivd_aligned_free; + s_create_ip.s_ivd_create_ip_t.pv_mem_ctxt = NULL; - s_init_ip.i4_level = i4_level; - s_init_ip.u4_num_reorder_frames = u4_num_reorder_frames; - s_init_ip.u4_num_ref_frames = u4_num_ref_frames; - s_init_ip.u4_share_disp_buf = u4_share_disp_buf; - s_init_ip.u4_num_extra_disp_buf = 0; + status = ivdec_api_function(mCodecCtx, (void *)&s_create_ip, (void *)&s_create_op); - s_init_op.s_ivd_init_op_t.u4_size = sizeof(s_init_op); - - s_init_ip.s_ivd_init_ip_t.u4_num_mem_rec = mNumMemRecords; - s_init_ip.s_ivd_init_ip_t.e_output_format = mIvColorFormat; - - mCodecCtx = (iv_obj_t *)mMemRecords[0].pv_base; + mCodecCtx = (iv_obj_t*)s_create_op.s_ivd_create_op_t.pv_handle; mCodecCtx->pv_fxns = dec_fxns; mCodecCtx->u4_size = sizeof(iv_obj_t); - status = ivdec_api_function(mCodecCtx, (void *)&s_init_ip, (void *)&s_init_op); if (status != IV_SUCCESS) { + ALOGE("Error in create: 0x%x", + s_create_op.s_ivd_create_op_t.u4_error_code); + deInitDecoder(); mCodecCtx = NULL; - ALOGE("Error in init: 0x%x", - s_init_op.s_ivd_init_op_t.u4_error_code); return UNKNOWN_ERROR; } } @@ -449,7 +334,7 @@ status_t SoftAVC::initDecoder(uint32_t width, uint32_t height) { resetPlugin(); /* Set the run time (dynamic) parameters */ - setParams(displayStride); + setParams(mStride); /* Set number of cores/threads to be used by the codec */ setNumCores(); @@ -457,61 +342,37 @@ status_t SoftAVC::initDecoder(uint32_t width, uint32_t height) { /* Get codec version */ logVersion(); - /* Allocate internal picture buffer */ - uint32_t bufferSize = displaySizeY * 3 / 2; - mFlushOutBuffer = (uint8_t *)ivd_aligned_malloc(128, bufferSize); - if (NULL == mFlushOutBuffer) { - ALOGE("Could not allocate flushOutputBuffer of size %u", bufferSize); - return NO_MEMORY; - } - - mInitNeeded = false; mFlushNeeded = false; return OK; } status_t SoftAVC::deInitDecoder() { size_t i; + IV_API_CALL_STATUS_T status; - if (mMemRecords) { - iv_mem_rec_t *ps_mem_rec; + if (mCodecCtx) { + ivdext_delete_ip_t s_delete_ip; + ivdext_delete_op_t s_delete_op; - ps_mem_rec = mMemRecords; - for (i = 0; i < mNumMemRecords; i++) { - if (ps_mem_rec->pv_base) { - ivd_aligned_free(ps_mem_rec->pv_base); - } - ps_mem_rec++; + s_delete_ip.s_ivd_delete_ip_t.u4_size = sizeof(ivdext_delete_ip_t); + s_delete_ip.s_ivd_delete_ip_t.e_cmd = IVD_CMD_DELETE; + + s_delete_op.s_ivd_delete_op_t.u4_size = sizeof(ivdext_delete_op_t); + + status = ivdec_api_function(mCodecCtx, (void *)&s_delete_ip, (void *)&s_delete_op); + if (status != IV_SUCCESS) { + ALOGE("Error in delete: 0x%x", + s_delete_op.s_ivd_delete_op_t.u4_error_code); + return UNKNOWN_ERROR; } - ivd_aligned_free(mMemRecords); - mMemRecords = NULL; } - if (mFlushOutBuffer) { - ivd_aligned_free(mFlushOutBuffer); - mFlushOutBuffer = NULL; - } - mInitNeeded = true; mChangingResolution = false; return OK; } -status_t SoftAVC::reInitDecoder(uint32_t width, uint32_t height) { - status_t ret; - - deInitDecoder(); - - ret = initDecoder(width, height); - if (OK != ret) { - ALOGE("Create failure"); - deInitDecoder(); - return NO_MEMORY; - } - return OK; -} - void SoftAVC::onReset() { SoftVideoDecoderOMXComponent::onReset(); @@ -520,23 +381,6 @@ void SoftAVC::onReset() { resetPlugin(); } -OMX_ERRORTYPE SoftAVC::internalSetParameter(OMX_INDEXTYPE index, const OMX_PTR params) { - const uint32_t oldWidth = mWidth; - const uint32_t oldHeight = mHeight; - OMX_ERRORTYPE ret = SoftVideoDecoderOMXComponent::internalSetParameter(index, params); - if (mWidth != oldWidth || mHeight != oldHeight) { - mNewWidth = mWidth; - mNewHeight = mHeight; - status_t err = reInitDecoder(mNewWidth, mNewHeight); - if (err != OK) { - notify(OMX_EventError, OMX_ErrorUnsupportedSetting, err, NULL); - mSignalledError = true; - return OMX_ErrorUnsupportedSetting; - } - } - return ret; -} - void SoftAVC::setDecodeArgs( ivd_video_decode_ip_t *ps_dec_ip, ivd_video_decode_op_t *ps_dec_op, @@ -587,6 +431,17 @@ void SoftAVC::onPortFlushCompleted(OMX_U32 portIndex) { if (kOutputPortIndex == portIndex) { setFlushMode(); + /* Allocate a picture buffer to flushed data */ + uint32_t displayStride = outputBufferWidth(); + uint32_t displayHeight = outputBufferHeight(); + + uint32_t bufferSize = displayStride * displayHeight * 3 / 2; + mFlushOutBuffer = (uint8_t *)memalign(128, bufferSize); + if (NULL == mFlushOutBuffer) { + ALOGE("Could not allocate flushOutputBuffer of size %zu", bufferSize); + return; + } + while (true) { ivd_video_decode_ip_t s_dec_ip; ivd_video_decode_op_t s_dec_op; @@ -601,6 +456,12 @@ void SoftAVC::onPortFlushCompleted(OMX_U32 portIndex) { break; } } + + if (mFlushOutBuffer) { + free(mFlushOutBuffer); + mFlushOutBuffer = NULL; + } + } } @@ -614,6 +475,17 @@ void SoftAVC::onQueueFilled(OMX_U32 portIndex) { return; } + if (NULL == mCodecCtx) { + if (OK != initDecoder()) { + return; + } + } + if (outputBufferWidth() != mStride) { + /* Set the run-time (dynamic) parameters */ + mStride = outputBufferWidth(); + setParams(mStride); + } + List &inQueue = getPortQueue(kInputPortIndex); List &outQueue = getPortQueue(kOutputPortIndex); @@ -676,22 +548,6 @@ void SoftAVC::onQueueFilled(OMX_U32 portIndex) { } } - // When there is an init required and the decoder is not in flush mode, - // update output port's definition and reinitialize decoder. - if (mInitNeeded && !mIsInFlush) { - bool portWillReset = false; - - status_t err = reInitDecoder(mNewWidth, mNewHeight); - if (err != OK) { - notify(OMX_EventError, OMX_ErrorUnsupportedSetting, err, NULL); - mSignalledError = true; - return; - } - - handlePortSettingsChange(&portWillReset, mNewWidth, mNewHeight); - return; - } - /* Get a free slot in timestamp array to hold input timestamp */ { size_t i; @@ -726,10 +582,7 @@ void SoftAVC::onQueueFilled(OMX_U32 portIndex) { IV_API_CALL_STATUS_T status; status = ivdec_api_function(mCodecCtx, (void *)&s_dec_ip, (void *)&s_dec_op); - bool unsupportedDimensions = - (IVD_STREAM_WIDTH_HEIGHT_NOT_SUPPORTED == (s_dec_op.u4_error_code & 0xFF)); bool resChanged = (IVD_RES_CHANGED == (s_dec_op.u4_error_code & 0xFF)); - bool unsupportedLevel = (IH264D_UNSUPPORTED_LEVEL == (s_dec_op.u4_error_code & 0xFF)); GETTIME(&mTimeEnd, NULL); /* Compute time taken for decode() */ @@ -747,46 +600,6 @@ void SoftAVC::onQueueFilled(OMX_U32 portIndex) { mTimeStampsValid[timeStampIx] = false; } - - // This is needed to handle CTS DecoderTest testCodecResetsH264WithoutSurface, - // which is not sending SPS/PPS after port reconfiguration and flush to the codec. - if (unsupportedDimensions && !mFlushNeeded) { - bool portWillReset = false; - mNewWidth = s_dec_op.u4_pic_wd; - mNewHeight = s_dec_op.u4_pic_ht; - - status_t err = reInitDecoder(mNewWidth, mNewHeight); - if (err != OK) { - notify(OMX_EventError, OMX_ErrorUnsupportedSetting, err, NULL); - mSignalledError = true; - return; - } - - handlePortSettingsChange(&portWillReset, mNewWidth, mNewHeight); - - setDecodeArgs(&s_dec_ip, &s_dec_op, inHeader, outHeader, timeStampIx); - - ivdec_api_function(mCodecCtx, (void *)&s_dec_ip, (void *)&s_dec_op); - return; - } - - if (unsupportedLevel && !mFlushNeeded) { - - mNewLevel = 51; - - status_t err = reInitDecoder(mNewWidth, mNewHeight); - if (err != OK) { - notify(OMX_EventError, OMX_ErrorUnsupportedSetting, err, NULL); - mSignalledError = true; - return; - } - - setDecodeArgs(&s_dec_ip, &s_dec_op, inHeader, outHeader, timeStampIx); - - ivdec_api_function(mCodecCtx, (void *)&s_dec_ip, (void *)&s_dec_op); - return; - } - // If the decoder is in the changing resolution mode and there is no output present, // that means the switching is done and it's ready to reset the decoder and the plugin. if (mChangingResolution && !s_dec_op.u4_output_present) { @@ -796,28 +609,11 @@ void SoftAVC::onQueueFilled(OMX_U32 portIndex) { continue; } - if (unsupportedDimensions || resChanged) { + if (resChanged) { mChangingResolution = true; if (mFlushNeeded) { setFlushMode(); } - - if (unsupportedDimensions) { - mNewWidth = s_dec_op.u4_pic_wd; - mNewHeight = s_dec_op.u4_pic_ht; - mInitNeeded = true; - } - continue; - } - - if (unsupportedLevel) { - - if (mFlushNeeded) { - setFlushMode(); - } - - mNewLevel = 51; - mInitNeeded = true; continue; } diff --git a/media/libstagefright/codecs/avcdec/SoftAVCDec.h b/media/libstagefright/codecs/avcdec/SoftAVCDec.h index 1ec8991..9dcabb4 100644 --- a/media/libstagefright/codecs/avcdec/SoftAVCDec.h +++ b/media/libstagefright/codecs/avcdec/SoftAVCDec.h @@ -23,9 +23,6 @@ namespace android { -#define ivd_aligned_malloc(alignment, size) memalign(alignment, size) -#define ivd_aligned_free(buf) free(buf) - /** Number of entries in the time-stamp array */ #define MAX_TIME_STAMPS 64 @@ -62,7 +59,6 @@ protected: virtual void onQueueFilled(OMX_U32 portIndex); virtual void onPortFlushCompleted(OMX_U32 portIndex); virtual void onReset(); - virtual OMX_ERRORTYPE internalSetParameter(OMX_INDEXTYPE index, const OMX_PTR params); private: // Number of input and output buffers enum { @@ -70,8 +66,6 @@ private: }; iv_obj_t *mCodecCtx; // Codec context - iv_mem_rec_t *mMemRecords; // Memory records requested by the codec - size_t mNumMemRecords; // Number of memory records requested by the codec size_t mNumCores; // Number of cores to be uesd by the codec @@ -97,17 +91,15 @@ private: bool mIsInFlush; // codec is flush mode bool mReceivedEOS; // EOS is receieved on input port - bool mInitNeeded; - uint32_t mNewWidth; - uint32_t mNewHeight; - uint32_t mNewLevel; + // The input stream has changed to a different resolution, which is still supported by the // codec. So the codec is switching to decode the new resolution. bool mChangingResolution; bool mFlushNeeded; bool mSignalledError; + size_t mStride; - status_t initDecoder(uint32_t width, uint32_t height); + status_t initDecoder(); status_t deInitDecoder(); status_t setFlushMode(); status_t setParams(size_t stride); @@ -115,7 +107,7 @@ private: status_t setNumCores(); status_t resetDecoder(); status_t resetPlugin(); - status_t reInitDecoder(uint32_t width, uint32_t height); + void setDecodeArgs( ivd_video_decode_ip_t *ps_dec_ip, -- cgit v1.1 From ddd346c7d54519e056b5b8b6d58b647770b3bb01 Mon Sep 17 00:00:00 2001 From: Flanker Date: Fri, 11 Sep 2015 19:05:47 +0800 Subject: stagefright: fix AMessage::FromParcel Add check for incoming mNumItems. Also add check readCString return value. Fix style & add log. Bug: 24123723 Change-Id: If41a5312c27d868f481893eef56019b6807c39b7 --- media/libstagefright/foundation/AMessage.cpp | 28 +++++++++++++++++++++++++--- 1 file changed, 25 insertions(+), 3 deletions(-) (limited to 'media') diff --git a/media/libstagefright/foundation/AMessage.cpp b/media/libstagefright/foundation/AMessage.cpp index e549ff6..725a574 100644 --- a/media/libstagefright/foundation/AMessage.cpp +++ b/media/libstagefright/foundation/AMessage.cpp @@ -601,13 +601,24 @@ sp AMessage::FromParcel(const Parcel &parcel) { msg->setWhat(what); msg->mNumItems = static_cast(parcel.readInt32()); + if (msg->mNumItems > kMaxNumItems) { + ALOGE("Too large number of items clipped."); + msg->mNumItems = kMaxNumItems; + } + for (size_t i = 0; i < msg->mNumItems; ++i) { Item *item = &msg->mItems[i]; const char *name = parcel.readCString(); - item->setName(name, strlen(name)); - item->mType = static_cast(parcel.readInt32()); + if (name == NULL) { + ALOGE("Failed reading name for an item. Parsing aborted."); + msg->mNumItems = i; + break; + } + item->mType = static_cast(parcel.readInt32()); + // setName() happens below so that we don't leak memory when parsing + // is aborted in the middle. switch (item->mType) { case kTypeInt32: { @@ -641,7 +652,16 @@ sp AMessage::FromParcel(const Parcel &parcel) { case kTypeString: { - item->u.stringValue = new AString(parcel.readCString()); + const char *stringValue = parcel.readCString(); + if (stringValue == NULL) { + ALOGE("Failed reading string value from a parcel. " + "Parsing aborted."); + msg->mNumItems = i; + continue; + // The loop will terminate subsequently. + } else { + item->u.stringValue = new AString(stringValue); + } break; } @@ -660,6 +680,8 @@ sp AMessage::FromParcel(const Parcel &parcel) { TRESPASS(); } } + + item->setName(name, strlen(name)); } return msg; -- cgit v1.1 From 2b8cd9cbb3e72ffd048ffdd1609fac74f61a22ac Mon Sep 17 00:00:00 2001 From: Flanker Date: Fri, 11 Sep 2015 19:05:47 +0800 Subject: stagefright: fix AMessage::FromParcel Add check for incoming mNumItems. Also add check readCString return value. Fix style & add log. Bug: 24123723 Change-Id: If41a5312c27d868f481893eef56019b6807c39b7 --- media/libstagefright/foundation/AMessage.cpp | 28 +++++++++++++++++++++++++--- 1 file changed, 25 insertions(+), 3 deletions(-) (limited to 'media') diff --git a/media/libstagefright/foundation/AMessage.cpp b/media/libstagefright/foundation/AMessage.cpp index 795e8a6..26bea2c 100644 --- a/media/libstagefright/foundation/AMessage.cpp +++ b/media/libstagefright/foundation/AMessage.cpp @@ -535,13 +535,24 @@ sp AMessage::FromParcel(const Parcel &parcel) { sp msg = new AMessage(what); msg->mNumItems = static_cast(parcel.readInt32()); + if (msg->mNumItems > kMaxNumItems) { + ALOGE("Too large number of items clipped."); + msg->mNumItems = kMaxNumItems; + } + for (size_t i = 0; i < msg->mNumItems; ++i) { Item *item = &msg->mItems[i]; const char *name = parcel.readCString(); - item->setName(name, strlen(name)); - item->mType = static_cast(parcel.readInt32()); + if (name == NULL) { + ALOGE("Failed reading name for an item. Parsing aborted."); + msg->mNumItems = i; + break; + } + item->mType = static_cast(parcel.readInt32()); + // setName() happens below so that we don't leak memory when parsing + // is aborted in the middle. switch (item->mType) { case kTypeInt32: { @@ -575,7 +586,16 @@ sp AMessage::FromParcel(const Parcel &parcel) { case kTypeString: { - item->u.stringValue = new AString(parcel.readCString()); + const char *stringValue = parcel.readCString(); + if (stringValue == NULL) { + ALOGE("Failed reading string value from a parcel. " + "Parsing aborted."); + msg->mNumItems = i; + continue; + // The loop will terminate subsequently. + } else { + item->u.stringValue = new AString(stringValue); + } break; } @@ -594,6 +614,8 @@ sp AMessage::FromParcel(const Parcel &parcel) { TRESPASS(); } } + + item->setName(name, strlen(name)); } return msg; -- cgit v1.1 From 3737a3fa121796131ea5b782230e65dad9ccf90f Mon Sep 17 00:00:00 2001 From: Flanker Date: Fri, 11 Sep 2015 19:05:47 +0800 Subject: DO NOT MERGE stagefright: fix AMessage::FromParcel Add check for incoming mNumItems. Also add check readCString return value. Fix style & add log. Bug: 24123723 Change-Id: If41a5312c27d868f481893eef56019b6807c39b7 --- media/libstagefright/foundation/AMessage.cpp | 25 ++++++++++++++++++++++--- 1 file changed, 22 insertions(+), 3 deletions(-) (limited to 'media') diff --git a/media/libstagefright/foundation/AMessage.cpp b/media/libstagefright/foundation/AMessage.cpp index dc42f91..1300f16 100644 --- a/media/libstagefright/foundation/AMessage.cpp +++ b/media/libstagefright/foundation/AMessage.cpp @@ -453,13 +453,23 @@ sp AMessage::FromParcel(const Parcel &parcel) { sp msg = new AMessage(what); msg->mNumItems = static_cast(parcel.readInt32()); + if (msg->mNumItems > kMaxNumItems) { + ALOGE("Too large number of items clipped."); + msg->mNumItems = kMaxNumItems; + } for (size_t i = 0; i < msg->mNumItems; ++i) { Item *item = &msg->mItems[i]; - item->mName = AAtomizer::Atomize(parcel.readCString()); - item->mType = static_cast(parcel.readInt32()); + const char *name = parcel.readCString(); + if (name == NULL) { + ALOGE("Failed reading name for an item. Parsing aborted."); + msg->mNumItems = i; + break; + } + item->mName = AAtomizer::Atomize(name); + item->mType = static_cast(parcel.readInt32()); switch (item->mType) { case kTypeInt32: { @@ -493,7 +503,16 @@ sp AMessage::FromParcel(const Parcel &parcel) { case kTypeString: { - item->u.stringValue = new AString(parcel.readCString()); + const char *stringValue = parcel.readCString(); + if (stringValue == NULL) { + ALOGE("Failed reading string value from a parcel. " + "Parsing aborted."); + msg->mNumItems = i; + continue; + // The loop will terminate subsequently. + } else { + item->u.stringValue = new AString(stringValue); + } break; } -- cgit v1.1 From f99498ee4de7123e2fd71778c6877be44fbd1506 Mon Sep 17 00:00:00 2001 From: Eino-Ville Talvala Date: Fri, 25 Sep 2015 16:52:55 -0700 Subject: CameraService: Use SCHED_FIFO for request queue thread in HFR - Move SchedulingPolicyService from audioservice to mediautils - When starting up a high speed stream config, set request queue thread to SCHED_FIFO using SchedulingPolicyService Bug: 24227252 Change-Id: I224b59142bd111caf563779f55cddd62385b9bac --- media/utils/Android.mk | 2 + media/utils/ISchedulingPolicyService.cpp | 80 ++++++++++++++++++++++ media/utils/ISchedulingPolicyService.h | 45 ++++++++++++ media/utils/SchedulingPolicyService.cpp | 62 +++++++++++++++++ .../include/mediautils/SchedulingPolicyService.h | 31 +++++++++ 5 files changed, 220 insertions(+) create mode 100644 media/utils/ISchedulingPolicyService.cpp create mode 100644 media/utils/ISchedulingPolicyService.h create mode 100644 media/utils/SchedulingPolicyService.cpp create mode 100644 media/utils/include/mediautils/SchedulingPolicyService.h (limited to 'media') diff --git a/media/utils/Android.mk b/media/utils/Android.mk index dfadbc8..54d22b1 100644 --- a/media/utils/Android.mk +++ b/media/utils/Android.mk @@ -18,6 +18,8 @@ include $(CLEAR_VARS) LOCAL_SRC_FILES := \ BatteryNotifier.cpp \ + ISchedulingPolicyService.cpp \ + SchedulingPolicyService.cpp LOCAL_SHARED_LIBRARIES := \ libbinder \ diff --git a/media/utils/ISchedulingPolicyService.cpp b/media/utils/ISchedulingPolicyService.cpp new file mode 100644 index 0000000..f55bc02 --- /dev/null +++ b/media/utils/ISchedulingPolicyService.cpp @@ -0,0 +1,80 @@ +/* + * Copyright (C) 2012 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#define LOG_TAG "ISchedulingPolicyService" +//#define LOG_NDEBUG 0 + +#include +#include "ISchedulingPolicyService.h" + +namespace android { + +// Keep in sync with frameworks/base/core/java/android/os/ISchedulingPolicyService.aidl +enum { + REQUEST_PRIORITY_TRANSACTION = IBinder::FIRST_CALL_TRANSACTION, +}; + +// ---------------------------------------------------------------------- + +class BpSchedulingPolicyService : public BpInterface +{ +public: + BpSchedulingPolicyService(const sp& impl) + : BpInterface(impl) + { + } + + virtual int requestPriority(int32_t pid, int32_t tid, int32_t prio, bool asynchronous) + { + Parcel data, reply; + data.writeInterfaceToken(ISchedulingPolicyService::getInterfaceDescriptor()); + data.writeInt32(pid); + data.writeInt32(tid); + data.writeInt32(prio); + uint32_t flags = asynchronous ? IBinder::FLAG_ONEWAY : 0; + status_t status = remote()->transact(REQUEST_PRIORITY_TRANSACTION, data, &reply, flags); + if (status != NO_ERROR) { + return status; + } + if (asynchronous) { + return NO_ERROR; + } + // fail on exception: force binder reconnection + if (reply.readExceptionCode() != 0) { + return DEAD_OBJECT; + } + return reply.readInt32(); + } +}; + +IMPLEMENT_META_INTERFACE(SchedulingPolicyService, "android.os.ISchedulingPolicyService"); + +// ---------------------------------------------------------------------- + +status_t BnSchedulingPolicyService::onTransact( + uint32_t code, const Parcel& data, Parcel* reply, uint32_t flags) +{ + switch (code) { + case REQUEST_PRIORITY_TRANSACTION: + // Not reached + return NO_ERROR; + break; + default: + return BBinder::onTransact(code, data, reply, flags); + } +} + +} // namespace android diff --git a/media/utils/ISchedulingPolicyService.h b/media/utils/ISchedulingPolicyService.h new file mode 100644 index 0000000..b94b191 --- /dev/null +++ b/media/utils/ISchedulingPolicyService.h @@ -0,0 +1,45 @@ +/* + * Copyright (C) 2012 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#ifndef ANDROID_ISCHEDULING_POLICY_SERVICE_H +#define ANDROID_ISCHEDULING_POLICY_SERVICE_H + +#include + +namespace android { + +class ISchedulingPolicyService : public IInterface +{ +public: + DECLARE_META_INTERFACE(SchedulingPolicyService); + + virtual int requestPriority(/*pid_t*/int32_t pid, /*pid_t*/int32_t tid, + int32_t prio, bool asynchronous) = 0; + +}; + +class BnSchedulingPolicyService : public BnInterface +{ +public: + virtual status_t onTransact( uint32_t code, + const Parcel& data, + Parcel* reply, + uint32_t flags = 0); +}; + +} // namespace android + +#endif // ANDROID_ISCHEDULING_POLICY_SERVICE_H diff --git a/media/utils/SchedulingPolicyService.cpp b/media/utils/SchedulingPolicyService.cpp new file mode 100644 index 0000000..17ee9bc --- /dev/null +++ b/media/utils/SchedulingPolicyService.cpp @@ -0,0 +1,62 @@ +/* + * Copyright (C) 2012 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#define LOG_TAG "SchedulingPolicyService" +//#define LOG_NDEBUG 0 + +#include +#include +#include "ISchedulingPolicyService.h" +#include "mediautils/SchedulingPolicyService.h" + +namespace android { + +static sp sSchedulingPolicyService; +static const String16 _scheduling_policy("scheduling_policy"); +static Mutex sMutex; + +int requestPriority(pid_t pid, pid_t tid, int32_t prio, bool asynchronous) +{ + // FIXME merge duplicated code related to service lookup, caching, and error recovery + int ret; + for (;;) { + sMutex.lock(); + sp sps = sSchedulingPolicyService; + sMutex.unlock(); + if (sps == 0) { + sp binder = defaultServiceManager()->checkService(_scheduling_policy); + if (binder == 0) { + sleep(1); + continue; + } + sps = interface_cast(binder); + sMutex.lock(); + sSchedulingPolicyService = sps; + sMutex.unlock(); + } + ret = sps->requestPriority(pid, tid, prio, asynchronous); + if (ret != DEAD_OBJECT) { + break; + } + ALOGW("SchedulingPolicyService died"); + sMutex.lock(); + sSchedulingPolicyService.clear(); + sMutex.unlock(); + } + return ret; +} + +} // namespace android diff --git a/media/utils/include/mediautils/SchedulingPolicyService.h b/media/utils/include/mediautils/SchedulingPolicyService.h new file mode 100644 index 0000000..a9870d4 --- /dev/null +++ b/media/utils/include/mediautils/SchedulingPolicyService.h @@ -0,0 +1,31 @@ +/* + * Copyright (C) 2012 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#ifndef _ANDROID_SCHEDULING_POLICY_SERVICE_H +#define _ANDROID_SCHEDULING_POLICY_SERVICE_H + +namespace android { + +// Request elevated priority for thread tid, whose thread group leader must be pid. +// The priority parameter is currently restricted to either 1 or 2. +// The asynchronous parameter should be 'true' to return immediately, +// after the request is enqueued but not necessarily executed. +// The default value 'false' means to return after request has been enqueued and executed. +int requestPriority(pid_t pid, pid_t tid, int32_t prio, bool asynchronous = false); + +} // namespace android + +#endif // _ANDROID_SCHEDULING_POLICY_SERVICE_H -- cgit v1.1 From 5cae16bdce77b0a3ba590b55637f7d55a2f35402 Mon Sep 17 00:00:00 2001 From: Wei Jia Date: Mon, 28 Sep 2015 14:50:47 -0700 Subject: MPEG4Extractor: ensure buffer size is not less than 8 for LastCommentData. Bug: 24346430 Change-Id: I897a724e968841d9160f819d06c0ce22f6d743c4 --- media/libstagefright/MPEG4Extractor.cpp | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'media') diff --git a/media/libstagefright/MPEG4Extractor.cpp b/media/libstagefright/MPEG4Extractor.cpp index 38ae6f3..4e12c07 100755 --- a/media/libstagefright/MPEG4Extractor.cpp +++ b/media/libstagefright/MPEG4Extractor.cpp @@ -2564,6 +2564,12 @@ status_t MPEG4Extractor::parseITunesMetaData(off64_t offset, size_t size) { mLastCommentName.setTo((const char *)buffer + 4); break; case FOURCC('d', 'a', 't', 'a'): + if (size < 8) { + delete[] buffer; + buffer = NULL; + ALOGE("b/24346430"); + return ERROR_MALFORMED; + } mLastCommentData.setTo((const char *)buffer + 8); break; } -- cgit v1.1 From 8dde7269a5356503d2b283234b6cb46d0c3f214e Mon Sep 17 00:00:00 2001 From: Wei Jia Date: Mon, 28 Sep 2015 11:32:23 -0700 Subject: OMX: allow only secure codec to remotely call allocateBuffer. Bug: 24310423 Change-Id: Iebcfc58b447f925ec2134898060af2ef227266a3 --- media/libmedia/IOMX.cpp | 6 ++++++ media/libstagefright/include/OMX.h | 2 ++ media/libstagefright/include/OMXNodeInstance.h | 5 +++++ media/libstagefright/omx/OMX.cpp | 5 +++++ media/libstagefright/omx/OMXNodeInstance.cpp | 1 + 5 files changed, 19 insertions(+) (limited to 'media') diff --git a/media/libmedia/IOMX.cpp b/media/libmedia/IOMX.cpp index 16da65e..5423c2a 100644 --- a/media/libmedia/IOMX.cpp +++ b/media/libmedia/IOMX.cpp @@ -963,6 +963,12 @@ status_t BnOMX::onTransact( node_id node = (node_id)data.readInt32(); OMX_U32 port_index = data.readInt32(); + if (!isSecure(node) || port_index != 0 /* kPortIndexInput */) { + ALOGE("b/24310423"); + reply->writeInt32(INVALID_OPERATION); + return NO_ERROR; + } + size_t size = data.readInt64(); buffer_id buffer; diff --git a/media/libstagefright/include/OMX.h b/media/libstagefright/include/OMX.h index d468dfc..e7c4f6d 100644 --- a/media/libstagefright/include/OMX.h +++ b/media/libstagefright/include/OMX.h @@ -140,6 +140,8 @@ public: virtual void binderDied(const wp &the_late_who); + virtual bool isSecure(IOMX::node_id node); + OMX_ERRORTYPE OnEvent( node_id node, OMX_IN OMX_EVENTTYPE eEvent, diff --git a/media/libstagefright/include/OMXNodeInstance.h b/media/libstagefright/include/OMXNodeInstance.h index f68e0a9..e5fb45b 100644 --- a/media/libstagefright/include/OMXNodeInstance.h +++ b/media/libstagefright/include/OMXNodeInstance.h @@ -125,6 +125,10 @@ struct OMXNodeInstance { const void *data, size_t size); + bool isSecure() const { + return mIsSecure; + } + // handles messages and removes them from the list void onMessages(std::list &messages); void onMessage(const omx_message &msg); @@ -142,6 +146,7 @@ private: OMX_HANDLETYPE mHandle; sp mObserver; bool mDying; + bool mIsSecure; // Lock only covers mGraphicBufferSource. We can't always use mLock // because of rare instances where we'd end up locking it recursively. diff --git a/media/libstagefright/omx/OMX.cpp b/media/libstagefright/omx/OMX.cpp index cb7ab5e..7f357c9 100644 --- a/media/libstagefright/omx/OMX.cpp +++ b/media/libstagefright/omx/OMX.cpp @@ -194,6 +194,11 @@ void OMX::binderDied(const wp &the_late_who) { instance->onObserverDied(mMaster); } +bool OMX::isSecure(node_id node) { + OMXNodeInstance *instance = findInstance(node); + return (instance == NULL ? false : instance->isSecure()); +} + bool OMX::livesLocally(node_id /* node */, pid_t pid) { return pid == getpid(); } diff --git a/media/libstagefright/omx/OMXNodeInstance.cpp b/media/libstagefright/omx/OMXNodeInstance.cpp index 9f1c5d8..94a213a 100644 --- a/media/libstagefright/omx/OMXNodeInstance.cpp +++ b/media/libstagefright/omx/OMXNodeInstance.cpp @@ -203,6 +203,7 @@ OMXNodeInstance::OMXNodeInstance( mDebugLevelBumpPendingBuffers[1] = 0; mMetadataType[0] = kMetadataBufferTypeInvalid; mMetadataType[1] = kMetadataBufferTypeInvalid; + mIsSecure = AString(name).endsWith(".secure"); } OMXNodeInstance::~OMXNodeInstance() { -- cgit v1.1 From 4802c0c507681634aee38518581a080bfa443ae2 Mon Sep 17 00:00:00 2001 From: Praveen Chavan Date: Tue, 29 Sep 2015 02:25:47 -0700 Subject: AudioSystem: Fix race condition in accessing ioDescriptors The vector mIoDescriptors can be simultaneouly modified and accessed by 2 threads. Acquire a lock while wrapping the ioDescriptor in a sp<> Bug: 24576810 Author: Haynes Mathew George Change-Id: I73c79ef8eca092b500a7ead3a5ebd0bcf75f9920 --- media/libmedia/AudioSystem.cpp | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) (limited to 'media') diff --git a/media/libmedia/AudioSystem.cpp b/media/libmedia/AudioSystem.cpp index 3bfb09a..9d645f0 100644 --- a/media/libmedia/AudioSystem.cpp +++ b/media/libmedia/AudioSystem.cpp @@ -476,7 +476,7 @@ void AudioSystem::AudioFlingerClient::ioConfigChanged(audio_io_config_event even switch (event) { case AUDIO_OUTPUT_OPENED: case AUDIO_INPUT_OPENED: { - sp oldDesc = getIoDescriptor(ioDesc->mIoHandle); + sp oldDesc = getIoDescriptor_l(ioDesc->mIoHandle); if (oldDesc == 0) { mIoDescriptors.add(ioDesc->mIoHandle, ioDesc); } else { @@ -498,7 +498,7 @@ void AudioSystem::AudioFlingerClient::ioConfigChanged(audio_io_config_event even } break; case AUDIO_OUTPUT_CLOSED: case AUDIO_INPUT_CLOSED: { - if (getIoDescriptor(ioDesc->mIoHandle) == 0) { + if (getIoDescriptor_l(ioDesc->mIoHandle) == 0) { ALOGW("ioConfigChanged() closing unknown %s %d", event == AUDIO_OUTPUT_CLOSED ? "output" : "input", ioDesc->mIoHandle); break; @@ -512,7 +512,7 @@ void AudioSystem::AudioFlingerClient::ioConfigChanged(audio_io_config_event even case AUDIO_OUTPUT_CONFIG_CHANGED: case AUDIO_INPUT_CONFIG_CHANGED: { - sp oldDesc = getIoDescriptor(ioDesc->mIoHandle); + sp oldDesc = getIoDescriptor_l(ioDesc->mIoHandle); if (oldDesc == 0) { ALOGW("ioConfigChanged() modifying unknown output! %d", ioDesc->mIoHandle); break; @@ -575,7 +575,7 @@ status_t AudioSystem::AudioFlingerClient::getInputBufferSize( return NO_ERROR; } -sp AudioSystem::AudioFlingerClient::getIoDescriptor(audio_io_handle_t ioHandle) +sp AudioSystem::AudioFlingerClient::getIoDescriptor_l(audio_io_handle_t ioHandle) { sp desc; ssize_t index = mIoDescriptors.indexOfKey(ioHandle); @@ -585,6 +585,12 @@ sp AudioSystem::AudioFlingerClient::getIoDescriptor(audio_io_ return desc; } +sp AudioSystem::AudioFlingerClient::getIoDescriptor(audio_io_handle_t ioHandle) +{ + Mutex::Autolock _l(mLock); + return getIoDescriptor_l(ioHandle); +} + status_t AudioSystem::AudioFlingerClient::addAudioDeviceCallback( const sp& callback, audio_io_handle_t audioIo) { -- cgit v1.1 From 257b3bc581bbc65318a4cc2d3c22a07a4429dc1d Mon Sep 17 00:00:00 2001 From: Marco Nelissen Date: Mon, 5 Oct 2015 10:46:11 -0700 Subject: Don't crash when there's no conceal frame Bug: 24630158 Change-Id: If042aebebb58c218eb7bbf01dcddbcbd05dca1d6 --- media/libstagefright/codecs/m4v_h263/dec/src/conceal.cpp | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'media') diff --git a/media/libstagefright/codecs/m4v_h263/dec/src/conceal.cpp b/media/libstagefright/codecs/m4v_h263/dec/src/conceal.cpp index e9ead01..03e4119 100644 --- a/media/libstagefright/codecs/m4v_h263/dec/src/conceal.cpp +++ b/media/libstagefright/codecs/m4v_h263/dec/src/conceal.cpp @@ -19,6 +19,7 @@ #include "vlc_decode.h" #include "bitstream.h" #include "scaling.h" +#include "log/log.h" /* ====================================================================== / Function : ConcealTexture_I() @@ -137,6 +138,10 @@ Modified: 6/04/2001 rewrote the function ****************************************************************************/ void CopyVopMB(Vop *curr, uint8 *prevFrame, int mbnum, int width_Y, int height) { + if (curr == NULL || prevFrame == NULL) { + ALOGE("b/24630158"); + return; + } int width_C = width_Y >> 1; int row = MB_SIZE; uint8 *y1, *y2, *u1, *u2, *v1, *v2; -- cgit v1.1 From b3694ff5a5bcecd4b6cedca156f6effb55bbf4ca Mon Sep 17 00:00:00 2001 From: Wei Jia Date: Mon, 5 Oct 2015 10:44:23 -0700 Subject: ID3: check possible integer overflow for extendedHeaderSize and paddingSize. Bug: 24623447 Change-Id: Ifbc74454d6e28ad7136efe35ab638a07e46398b1 --- media/libstagefright/id3/ID3.cpp | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) (limited to 'media') diff --git a/media/libstagefright/id3/ID3.cpp b/media/libstagefright/id3/ID3.cpp index 00f87aa..4410579 100644 --- a/media/libstagefright/id3/ID3.cpp +++ b/media/libstagefright/id3/ID3.cpp @@ -194,6 +194,13 @@ struct id3_header { if (header.version_major == 4) { void *copy = malloc(size); + if (copy == NULL) { + free(mData); + mData = NULL; + ALOGE("b/24623447, no more memory"); + return false; + } + memcpy(copy, mData, size); bool success = removeUnsynchronizationV2_4(false /* iTunesHack */); @@ -234,7 +241,14 @@ struct id3_header { return false; } - size_t extendedHeaderSize = U32_AT(&mData[0]) + 4; + size_t extendedHeaderSize = U32_AT(&mData[0]); + if (extendedHeaderSize > SIZE_MAX - 4) { + free(mData); + mData = NULL; + ALOGE("b/24623447, extendedHeaderSize is too large"); + return false; + } + extendedHeaderSize += 4; if (extendedHeaderSize > mSize) { free(mData); @@ -252,7 +266,10 @@ struct id3_header { if (extendedHeaderSize >= 10) { size_t paddingSize = U32_AT(&mData[6]); - if (mFirstFrameOffset + paddingSize > mSize) { + if (paddingSize > SIZE_MAX - mFirstFrameOffset) { + ALOGE("b/24623447, paddingSize is too large"); + } + if (paddingSize > mSize - mFirstFrameOffset) { free(mData); mData = NULL; -- cgit v1.1 From e6d904fe5f6e7c7fc1d5fca2798dd3512468b118 Mon Sep 17 00:00:00 2001 From: Wei Jia Date: Mon, 28 Sep 2015 14:50:47 -0700 Subject: MPEG4Extractor: ensure buffer size is not less than 8 for LastCommentData. Bug: 24346430 Change-Id: I897a724e968841d9160f819d06c0ce22f6d743c4 (cherry picked from commit 5cae16bdce77b0a3ba590b55637f7d55a2f35402) --- media/libstagefright/MPEG4Extractor.cpp | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'media') diff --git a/media/libstagefright/MPEG4Extractor.cpp b/media/libstagefright/MPEG4Extractor.cpp index 92065d1..990aa54 100644 --- a/media/libstagefright/MPEG4Extractor.cpp +++ b/media/libstagefright/MPEG4Extractor.cpp @@ -2153,6 +2153,12 @@ status_t MPEG4Extractor::parseMetaData(off64_t offset, size_t size) { mLastCommentName.setTo((const char *)buffer + 4); break; case FOURCC('d', 'a', 't', 'a'): + if (size < 8) { + delete[] buffer; + buffer = NULL; + ALOGE("b/24346430"); + return ERROR_MALFORMED; + } mLastCommentData.setTo((const char *)buffer + 8); break; } -- cgit v1.1 From 5d101298d8b0a78a1dc5bd26dbdada411f4ecd4d Mon Sep 17 00:00:00 2001 From: Marco Nelissen Date: Fri, 2 Oct 2015 15:12:00 -0700 Subject: Check NAL size before use Bug: 24441553 Bug: 24445122 Change-Id: Ib7f025769adbafd5a2cb64fae5562a0a565945c2 --- media/libstagefright/MPEG4Extractor.cpp | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) (limited to 'media') diff --git a/media/libstagefright/MPEG4Extractor.cpp b/media/libstagefright/MPEG4Extractor.cpp index 92065d1..af6b8bd 100644 --- a/media/libstagefright/MPEG4Extractor.cpp +++ b/media/libstagefright/MPEG4Extractor.cpp @@ -3293,7 +3293,10 @@ status_t MPEG4Source::read( (const uint8_t *)mBuffer->data() + mBuffer->range_offset(); size_t nal_size = parseNALSize(src); - if (mBuffer->range_length() < mNALLengthSize + nal_size) { + if (mNALLengthSize > SIZE_MAX - nal_size) { + ALOGE("b/24441553, b/24445122"); + } + if (mBuffer->range_length() - mNALLengthSize < nal_size) { ALOGE("incomplete NAL unit."); mBuffer->release(); @@ -3566,7 +3569,11 @@ status_t MPEG4Source::fragmentedRead( (const uint8_t *)mBuffer->data() + mBuffer->range_offset(); size_t nal_size = parseNALSize(src); - if (mBuffer->range_length() < mNALLengthSize + nal_size) { + if (mNALLengthSize > SIZE_MAX - nal_size) { + ALOGE("b/24441553, b/24445122"); + } + + if (mBuffer->range_length() - mNALLengthSize < nal_size) { ALOGE("incomplete NAL unit."); mBuffer->release(); -- cgit v1.1 From 61be84167c1a11c8a030e00a2b79b2bc4fdf617f Mon Sep 17 00:00:00 2001 From: Andy Hung Date: Tue, 6 Oct 2015 10:51:09 -0700 Subject: AudioTrack: Prevent stop() from reissuing last marker event Avoid a duplicate marker event race condition (1 in 30 or less) by clearing marker reached in start() not stop(). Bug: 24497521 Change-Id: I9520d063c7d173d2e642174bf60a2bfe75edf085 --- media/libmedia/AudioTrack.cpp | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) (limited to 'media') diff --git a/media/libmedia/AudioTrack.cpp b/media/libmedia/AudioTrack.cpp index 444f4d8..ab3d66a 100644 --- a/media/libmedia/AudioTrack.cpp +++ b/media/libmedia/AudioTrack.cpp @@ -523,6 +523,15 @@ status_t AudioTrack::start() mTimestampStartupGlitchReported = false; mRetrogradeMotionReported = false; + // If previousState == STATE_STOPPED, we reactivate markers (mMarkerPosition != 0) + // as the position is reset to 0. This is legacy behavior. This is not done + // in stop() to avoid a race condition where the last marker event is issued twice. + // Note: the if is technically unnecessary because previousState == STATE_FLUSHED + // is only for streaming tracks, and mMarkerReached is already set to false. + if (previousState == STATE_STOPPED) { + mMarkerReached = false; + } + // For offloaded tracks, we don't know if the hardware counters are really zero here, // since the flush is asynchronous and stop may not fully drain. // We save the time when the track is started to later verify whether @@ -592,9 +601,9 @@ void AudioTrack::stop() mProxy->interrupt(); mAudioTrack->stop(); - // the playback head position will reset to 0, so if a marker is set, we need - // to activate it again - mMarkerReached = false; + + // Note: legacy handling - stop does not clear playback marker + // and periodic update counter, but flush does for streaming tracks. if (mSharedBuffer != 0) { // clear buffer position and loop count. -- cgit v1.1