From 5a17fde9ce50a4e5914863326dfa5055a8bcd25f Mon Sep 17 00:00:00 2001 From: James Dong Date: Fri, 20 Jul 2012 17:38:48 -0700 Subject: Removed a CHECK_EQ that is not needed. 'what' was just compared against Renderer::kWhatFlushComplete before entering the if block. Change-Id: I72c5c156f814621a24439d89e150c4e0d90edcbb --- media/libmediaplayerservice/nuplayer/NuPlayer.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/media/libmediaplayerservice/nuplayer/NuPlayer.cpp b/media/libmediaplayerservice/nuplayer/NuPlayer.cpp index 8f45491f..c66b2733 100644 --- a/media/libmediaplayerservice/nuplayer/NuPlayer.cpp +++ b/media/libmediaplayerservice/nuplayer/NuPlayer.cpp @@ -494,8 +494,6 @@ void NuPlayer::onMessageReceived(const sp &msg) { } } } else if (what == Renderer::kWhatFlushComplete) { - CHECK_EQ(what, (int32_t)Renderer::kWhatFlushComplete); - int32_t audio; CHECK(msg->findInt32("audio", &audio)); -- cgit v1.2.3 From fcbb8af593db05b2041c69ea9db8bcb6b9b899d5 Mon Sep 17 00:00:00 2001 From: James Dong Date: Sat, 4 Aug 2012 19:58:07 -0700 Subject: Fix a deadlock in commandSetVideoBufferCountL() o The lock to be acquired in recordingEnabled() has aleady been acquired in sendCommand() before the call to commandSetVideoBufferCountL(). Change-Id: I664d51ef449c9eb3576d5d56f73f29c98444ff3f --- services/camera/libcameraservice/Camera2Client.cpp | 9 ++++++++- services/camera/libcameraservice/Camera2Client.h | 2 ++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/services/camera/libcameraservice/Camera2Client.cpp b/services/camera/libcameraservice/Camera2Client.cpp index 0d45596a..8667bac3 100644 --- a/services/camera/libcameraservice/Camera2Client.cpp +++ b/services/camera/libcameraservice/Camera2Client.cpp @@ -793,8 +793,15 @@ void Camera2Client::stopRecording() { bool Camera2Client::recordingEnabled() { ATRACE_CALL(); Mutex::Autolock icl(mICameraLock); + if ( checkPid(__FUNCTION__) != OK) return false; + return recordingEnabledL(); +} + +bool Camera2Client::recordingEnabledL() { + ATRACE_CALL(); + return (mState == RECORD || mState == VIDEO_SNAPSHOT); } @@ -1633,7 +1640,7 @@ status_t Camera2Client::commandPingL() { } status_t Camera2Client::commandSetVideoBufferCountL(size_t count) { - if (recordingEnabled()) { + if (recordingEnabledL()) { ALOGE("%s: Camera %d: Error setting video buffer count after " "recording was started", __FUNCTION__, mCameraId); return INVALID_OPERATION; diff --git a/services/camera/libcameraservice/Camera2Client.h b/services/camera/libcameraservice/Camera2Client.h index 66515076..5bc91729 100644 --- a/services/camera/libcameraservice/Camera2Client.h +++ b/services/camera/libcameraservice/Camera2Client.h @@ -97,6 +97,8 @@ private: void stopPreviewL(); status_t startPreviewL(); + bool recordingEnabledL(); + // Individual commands for sendCommand() status_t commandStartSmoothZoomL(); status_t commandStopSmoothZoomL(); -- cgit v1.2.3 From 130c2556038d774f4728dd54f583df55c628fe85 Mon Sep 17 00:00:00 2001 From: Eino-Ville Talvala Date: Mon, 10 Sep 2012 09:53:09 -0700 Subject: Camera2: Sanity check inputs better. Bug: 7132141 Change-Id: I866a65dfe47464070a6ef4ac60be4801cb68327b --- services/camera/libcameraservice/camera2/Parameters.cpp | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/services/camera/libcameraservice/camera2/Parameters.cpp b/services/camera/libcameraservice/camera2/Parameters.cpp index e5942dca..71aa39b3 100644 --- a/services/camera/libcameraservice/camera2/Parameters.cpp +++ b/services/camera/libcameraservice/camera2/Parameters.cpp @@ -1553,6 +1553,8 @@ const char* Parameters::getStateName(State state) { int Parameters::formatStringToEnum(const char *format) { return + !format ? + HAL_PIXEL_FORMAT_YCrCb_420_SP : !strcmp(format, CameraParameters::PIXEL_FORMAT_YUV422SP) ? HAL_PIXEL_FORMAT_YCbCr_422_SP : // NV16 !strcmp(format, CameraParameters::PIXEL_FORMAT_YUV420SP) ? @@ -1606,6 +1608,8 @@ const char* Parameters::formatEnumToString(int format) { int Parameters::wbModeStringToEnum(const char *wbMode) { return + !wbMode ? + ANDROID_CONTROL_AWB_AUTO : !strcmp(wbMode, CameraParameters::WHITE_BALANCE_AUTO) ? ANDROID_CONTROL_AWB_AUTO : !strcmp(wbMode, CameraParameters::WHITE_BALANCE_INCANDESCENT) ? @@ -1627,6 +1631,8 @@ int Parameters::wbModeStringToEnum(const char *wbMode) { int Parameters::effectModeStringToEnum(const char *effectMode) { return + !effectMode ? + ANDROID_CONTROL_EFFECT_OFF : !strcmp(effectMode, CameraParameters::EFFECT_NONE) ? ANDROID_CONTROL_EFFECT_OFF : !strcmp(effectMode, CameraParameters::EFFECT_MONO) ? @@ -1650,6 +1656,8 @@ int Parameters::effectModeStringToEnum(const char *effectMode) { int Parameters::abModeStringToEnum(const char *abMode) { return + !abMode ? + ANDROID_CONTROL_AE_ANTIBANDING_AUTO : !strcmp(abMode, CameraParameters::ANTIBANDING_AUTO) ? ANDROID_CONTROL_AE_ANTIBANDING_AUTO : !strcmp(abMode, CameraParameters::ANTIBANDING_OFF) ? @@ -1663,6 +1671,8 @@ int Parameters::abModeStringToEnum(const char *abMode) { int Parameters::sceneModeStringToEnum(const char *sceneMode) { return + !sceneMode ? + ANDROID_CONTROL_SCENE_MODE_UNSUPPORTED : !strcmp(sceneMode, CameraParameters::SCENE_MODE_AUTO) ? ANDROID_CONTROL_SCENE_MODE_UNSUPPORTED : !strcmp(sceneMode, CameraParameters::SCENE_MODE_ACTION) ? @@ -1701,6 +1711,8 @@ int Parameters::sceneModeStringToEnum(const char *sceneMode) { Parameters::Parameters::flashMode_t Parameters::flashModeStringToEnum( const char *flashMode) { return + !flashMode ? + Parameters::FLASH_MODE_INVALID : !strcmp(flashMode, CameraParameters::FLASH_MODE_OFF) ? Parameters::FLASH_MODE_OFF : !strcmp(flashMode, CameraParameters::FLASH_MODE_AUTO) ? @@ -1717,6 +1729,8 @@ Parameters::Parameters::flashMode_t Parameters::flashModeStringToEnum( Parameters::Parameters::focusMode_t Parameters::focusModeStringToEnum( const char *focusMode) { return + !focusMode ? + Parameters::FOCUS_MODE_INVALID : !strcmp(focusMode, CameraParameters::FOCUS_MODE_AUTO) ? Parameters::FOCUS_MODE_AUTO : !strcmp(focusMode, CameraParameters::FOCUS_MODE_INFINITY) ? @@ -1737,6 +1751,8 @@ Parameters::Parameters::focusMode_t Parameters::focusModeStringToEnum( Parameters::Parameters::lightFxMode_t Parameters::lightFxStringToEnum( const char *lightFxMode) { return + !lightFxMode ? + Parameters::LIGHTFX_NONE : !strcmp(lightFxMode, CameraParameters::LIGHTFX_LOWLIGHT) ? Parameters::LIGHTFX_LOWLIGHT : !strcmp(lightFxMode, CameraParameters::LIGHTFX_HDR) ? -- cgit v1.2.3 From be29bbb22a9046be6838709ea12966fe4b6dfaeb Mon Sep 17 00:00:00 2001 From: Andreas Huber Date: Wed, 12 Sep 2012 12:08:55 -0700 Subject: Throttle SurfaceMediaSource. Change-Id: I214ce60f8d94df9c07041577e34ed1ad5e199fdb --- include/media/stagefright/SurfaceMediaSource.h | 6 ++++ media/libstagefright/SurfaceMediaSource.cpp | 45 +++++++++++++++++++------- 2 files changed, 39 insertions(+), 12 deletions(-) diff --git a/include/media/stagefright/SurfaceMediaSource.h b/include/media/stagefright/SurfaceMediaSource.h index 840b4aa7..9e07ea49 100644 --- a/include/media/stagefright/SurfaceMediaSource.h +++ b/include/media/stagefright/SurfaceMediaSource.h @@ -167,6 +167,8 @@ private: // this list in signalBufferReturned Vector > mCurrentBuffers; + size_t mNumPendingBuffers; + // mCurrentTimestamp is the timestamp for the current texture. It // gets set to mLastQueuedTimestamp each time updateTexImage is called. int64_t mCurrentTimestamp; @@ -202,10 +204,14 @@ private: // offset timestamps. int64_t mStartTimeNs; + size_t mMaxAcquiredBufferCount; + // mFrameAvailableCondition condition used to indicate whether there // is a frame available for dequeuing Condition mFrameAvailableCondition; + Condition mMediaBuffersAvailableCondition; + // Avoid copying and equating and default constructor DISALLOW_IMPLICIT_CONSTRUCTORS(SurfaceMediaSource); }; diff --git a/media/libstagefright/SurfaceMediaSource.cpp b/media/libstagefright/SurfaceMediaSource.cpp index 867f76d1..e2244379 100644 --- a/media/libstagefright/SurfaceMediaSource.cpp +++ b/media/libstagefright/SurfaceMediaSource.cpp @@ -18,8 +18,8 @@ #include #include -#include #include +#include #include #include @@ -39,12 +39,14 @@ SurfaceMediaSource::SurfaceMediaSource(uint32_t bufferWidth, uint32_t bufferHeig mWidth(bufferWidth), mHeight(bufferHeight), mCurrentSlot(BufferQueue::INVALID_BUFFER_SLOT), + mNumPendingBuffers(0), mCurrentTimestamp(0), mFrameRate(30), mStopped(false), mNumFramesReceived(0), mNumFramesEncoded(0), - mFirstFrameTimestamp(0) + mFirstFrameTimestamp(0), + mMaxAcquiredBufferCount(4) // XXX double-check the default { ALOGV("SurfaceMediaSource"); @@ -155,20 +157,32 @@ status_t SurfaceMediaSource::start(MetaData *params) ALOGE("bufferCount %d is too small", bufferCount); return BAD_VALUE; } + + mMaxAcquiredBufferCount = bufferCount; } - if (bufferCount != 0) { - status_t err = mBufferQueue->setMaxAcquiredBufferCount(bufferCount); - if (err != OK) { - return err; - } + CHECK_GT(mMaxAcquiredBufferCount, 1); + + status_t err = + mBufferQueue->setMaxAcquiredBufferCount(mMaxAcquiredBufferCount); + + if (err != OK) { + return err; } + mNumPendingBuffers = 0; + return OK; } status_t SurfaceMediaSource::setMaxAcquiredBufferCount(size_t count) { - return mBufferQueue->setMaxAcquiredBufferCount(count); + ALOGV("setMaxAcquiredBufferCount(%d)", count); + Mutex::Autolock lock(mMutex); + + CHECK_GT(count, 1); + mMaxAcquiredBufferCount = count; + + return OK; } @@ -216,9 +230,8 @@ sp SurfaceMediaSource::getFormat() // Note: Call only when you have the lock static void passMetadataBuffer(MediaBuffer **buffer, buffer_handle_t bufferHandle) { - // MediaBuffer allocates and owns this data - MediaBuffer *tempBuffer = new MediaBuffer(4 + sizeof(buffer_handle_t)); - char *data = (char *)tempBuffer->data(); + *buffer = new MediaBuffer(4 + sizeof(buffer_handle_t)); + char *data = (char *)(*buffer)->data(); if (data == NULL) { ALOGE("Cannot allocate memory for metadata buffer!"); return; @@ -226,7 +239,6 @@ static void passMetadataBuffer(MediaBuffer **buffer, OMX_U32 type = kMetadataBufferTypeGrallocSource; memcpy(data, &type, 4); memcpy(data + 4, &bufferHandle, sizeof(buffer_handle_t)); - *buffer = tempBuffer; ALOGV("handle = %p, , offset = %d, length = %d", bufferHandle, (*buffer)->range_length(), (*buffer)->range_offset()); @@ -240,6 +252,10 @@ status_t SurfaceMediaSource::read( MediaBuffer **buffer, *buffer = NULL; + while (!mStopped && mNumPendingBuffers == mMaxAcquiredBufferCount) { + mMediaBuffersAvailableCondition.wait(mMutex); + } + // Update the current buffer info // TODO: mCurrentSlot can be made a bufferstate since there // can be more than one "current" slots. @@ -306,6 +322,7 @@ status_t SurfaceMediaSource::read( MediaBuffer **buffer, mNumFramesEncoded++; // Pass the data to the MediaBuffer. Pass in only the metadata + passMetadataBuffer(buffer, mBufferSlot[mCurrentSlot]->handle); (*buffer)->setObserver(this); @@ -315,6 +332,7 @@ status_t SurfaceMediaSource::read( MediaBuffer **buffer, mNumFramesEncoded, mCurrentTimestamp / 1000, mCurrentTimestamp / 1000 - prevTimeStamp / 1000); + ++mNumPendingBuffers; return OK; } @@ -371,6 +389,9 @@ void SurfaceMediaSource::signalBufferReturned(MediaBuffer *buffer) { if (!foundBuffer) { CHECK(!"signalBufferReturned: bogus buffer"); } + + --mNumPendingBuffers; + mMediaBuffersAvailableCondition.broadcast(); } // Part of the BufferQueue::ConsumerListener -- cgit v1.2.3 From 5960ae0bc5269d6102e29f981ff3b3f598385083 Mon Sep 17 00:00:00 2001 From: Andreas Huber Date: Wed, 12 Sep 2012 12:15:18 -0700 Subject: Use proper number of video buffers to be acquired simultaneously from SurfaceMediaSource, this should match the number of encoder input buffers. Change-Id: Ibeb102337fd23698c5321c63dd3cb00b93e632b0 --- media/libstagefright/wifi-display/source/PlaybackSession.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/media/libstagefright/wifi-display/source/PlaybackSession.cpp b/media/libstagefright/wifi-display/source/PlaybackSession.cpp index 5d0ddf1f..a6bb8c6f 100644 --- a/media/libstagefright/wifi-display/source/PlaybackSession.cpp +++ b/media/libstagefright/wifi-display/source/PlaybackSession.cpp @@ -785,8 +785,7 @@ status_t WifiDisplaySource::PlaybackSession::addVideoSource() { } // Add one reference to account for the serializer. - // Add another two for unknown reasons. - err = source->setMaxAcquiredBufferCount(15); // XXX numInputBuffers + 2); + err = source->setMaxAcquiredBufferCount(numInputBuffers); CHECK_EQ(err, (status_t)OK); mBufferQueue = source->getBufferQueue(); -- cgit v1.2.3 From 4d41238ec1ad73ae4c8cb6d9dc87672639fc3854 Mon Sep 17 00:00:00 2001 From: Andreas Huber Date: Wed, 12 Sep 2012 14:06:17 -0700 Subject: Audio and video bitrate are now configurable through system properties adb shell setprop media.wfd.audio-bitrate 64000 adb shell setprop media.wfd.video-bitrate 10000000 are the defaults. Change-Id: Ib4d700748bdac2adffc6e7e31aff9c9f998e20f0 --- .../wifi-display/source/Converter.cpp | 25 ++++++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-) diff --git a/media/libstagefright/wifi-display/source/Converter.cpp b/media/libstagefright/wifi-display/source/Converter.cpp index 0b29df94..7c3b9084 100644 --- a/media/libstagefright/wifi-display/source/Converter.cpp +++ b/media/libstagefright/wifi-display/source/Converter.cpp @@ -20,6 +20,7 @@ #include "Converter.h" +#include #include #include #include @@ -62,6 +63,20 @@ sp Converter::getOutputFormat() const { return mOutputFormat; } +static int32_t getBitrate(const char *propName, int32_t defaultValue) { + char val[PROPERTY_VALUE_MAX]; + if (property_get(propName, val, NULL)) { + char *end; + unsigned long x = strtoul(val, &end, 10); + + if (*end == '\0' && end > val && x > 0) { + return x; + } + } + + return defaultValue; +} + status_t Converter::initEncoder() { AString inputMIME; CHECK(mInputFormat->findString("mime", &inputMIME)); @@ -87,10 +102,16 @@ status_t Converter::initEncoder() { mOutputFormat = mInputFormat->dup(); mOutputFormat->setString("mime", outputMIME.c_str()); + int32_t audioBitrate = getBitrate("media.wfd.audio-bitrate", 64000); + int32_t videoBitrate = getBitrate("media.wfd.video-bitrate", 10000000); + + ALOGI("using audio bitrate of %d bps, video bitrate of %d bps", + audioBitrate, videoBitrate); + if (isAudio) { - mOutputFormat->setInt32("bitrate", 64000); // 64 kBit/sec + mOutputFormat->setInt32("bitrate", audioBitrate); } else { - mOutputFormat->setInt32("bitrate", 10000000); // 10Mbit/sec + mOutputFormat->setInt32("bitrate", videoBitrate); mOutputFormat->setInt32("frame-rate", 60); mOutputFormat->setInt32("i-frame-interval", 3); // Iframes every 3 secs } -- cgit v1.2.3 From 999e8efefd954697202ab5ffaa4fa39bc251890f Mon Sep 17 00:00:00 2001 From: Andreas Huber Date: Wed, 12 Sep 2012 16:25:14 -0700 Subject: Various improvements to cleanly shutdown a wfd session. Change-Id: I86f0a27d7e8eb96200153bab847a862f21a19d13 --- .../wifi-display/source/Converter.cpp | 4 ++ .../wifi-display/source/MediaPuller.cpp | 25 ++++++- .../wifi-display/source/MediaPuller.h | 1 + .../wifi-display/source/PlaybackSession.cpp | 76 ++++++++++++---------- .../wifi-display/source/PlaybackSession.h | 2 + .../wifi-display/source/WifiDisplaySource.cpp | 27 ++++---- 6 files changed, 86 insertions(+), 49 deletions(-) diff --git a/media/libstagefright/wifi-display/source/Converter.cpp b/media/libstagefright/wifi-display/source/Converter.cpp index 7c3b9084..0e10b8d0 100644 --- a/media/libstagefright/wifi-display/source/Converter.cpp +++ b/media/libstagefright/wifi-display/source/Converter.cpp @@ -49,6 +49,10 @@ Converter::~Converter() { mEncoder->release(); mEncoder.clear(); } + + AString mime; + CHECK(mInputFormat->findString("mime", &mime)); + ALOGI("encoder (%s) shut down.", mime.c_str()); } status_t Converter::initCheck() const { diff --git a/media/libstagefright/wifi-display/source/MediaPuller.cpp b/media/libstagefright/wifi-display/source/MediaPuller.cpp index 786029a5..35ae5392 100644 --- a/media/libstagefright/wifi-display/source/MediaPuller.cpp +++ b/media/libstagefright/wifi-display/source/MediaPuller.cpp @@ -33,7 +33,13 @@ MediaPuller::MediaPuller( const sp &source, const sp ¬ify) : mSource(source), mNotify(notify), - mPullGeneration(0) { + mPullGeneration(0), + mIsAudio(false) { + sp meta = source->getFormat(); + const char *mime; + CHECK(meta->findCString(kKeyMIMEType, &mime)); + + mIsAudio = !strncasecmp(mime, "audio/", 6); } MediaPuller::~MediaPuller() { @@ -77,7 +83,14 @@ void MediaPuller::onMessageReceived(const sp &msg) { schedulePull(); } } else { + sp meta = mSource->getFormat(); + const char *tmp; + CHECK(meta->findCString(kKeyMIMEType, &tmp)); + AString mime = tmp; + + ALOGI("MediaPuller(%s) stopping.", mime.c_str()); err = mSource->stop(); + ALOGI("MediaPuller(%s) stopped.", mime.c_str()); ++mPullGeneration; } @@ -124,7 +137,15 @@ void MediaPuller::onMessageReceived(const sp &msg) { mbuf->range_length()); accessUnit->meta()->setInt64("timeUs", timeUs); - accessUnit->meta()->setPointer("mediaBuffer", mbuf); + + if (mIsAudio) { + mbuf->release(); + mbuf = NULL; + } else { + // video encoder will release MediaBuffer when done + // with underlying data. + accessUnit->meta()->setPointer("mediaBuffer", mbuf); + } sp notify = mNotify->dup(); diff --git a/media/libstagefright/wifi-display/source/MediaPuller.h b/media/libstagefright/wifi-display/source/MediaPuller.h index 52975018..134e1c08 100644 --- a/media/libstagefright/wifi-display/source/MediaPuller.h +++ b/media/libstagefright/wifi-display/source/MediaPuller.h @@ -49,6 +49,7 @@ private: sp mSource; sp mNotify; int32_t mPullGeneration; + bool mIsAudio; status_t postSynchronouslyAndReturnError(const sp &msg); void schedulePull(); diff --git a/media/libstagefright/wifi-display/source/PlaybackSession.cpp b/media/libstagefright/wifi-display/source/PlaybackSession.cpp index a6bb8c6f..41beba42 100644 --- a/media/libstagefright/wifi-display/source/PlaybackSession.cpp +++ b/media/libstagefright/wifi-display/source/PlaybackSession.cpp @@ -159,6 +159,8 @@ status_t WifiDisplaySource::PlaybackSession::Track::stop() { err = mMediaPuller->stop(); } + mConverter.clear(); + mStarted = false; return err; @@ -288,41 +290,6 @@ WifiDisplaySource::PlaybackSession::~PlaybackSession() { mLogFile = NULL; } #endif - - mTracks.clear(); - - mPacketizer.clear(); - - if (mSerializer != NULL) { - mSerializer->stop(); - - looper()->unregisterHandler(mSerializer->id()); - mSerializer.clear(); - } - - mTracks.clear(); - - if (mSerializerLooper != NULL) { - mSerializerLooper->stop(); - mSerializerLooper.clear(); - } - - if (mLegacyMode) { - sp sm = defaultServiceManager(); - sp binder = sm->getService(String16("SurfaceFlinger")); - sp service = interface_cast(binder); - CHECK(service != NULL); - - service->connectDisplay(NULL); - } - - if (mRTCPSessionID != 0) { - mNetSession->destroySession(mRTCPSessionID); - } - - if (mRTPSessionID != 0) { - mNetSession->destroySession(mRTPSessionID); - } } int32_t WifiDisplaySource::PlaybackSession::getRTPPort() const { @@ -369,6 +336,45 @@ status_t WifiDisplaySource::PlaybackSession::pause() { return OK; } +status_t WifiDisplaySource::PlaybackSession::destroy() { + mTracks.clear(); + + mPacketizer.clear(); + + if (mSerializer != NULL) { + mSerializer->stop(); + + looper()->unregisterHandler(mSerializer->id()); + mSerializer.clear(); + } + + mTracks.clear(); + + if (mSerializerLooper != NULL) { + mSerializerLooper->stop(); + mSerializerLooper.clear(); + } + + if (mLegacyMode) { + sp sm = defaultServiceManager(); + sp binder = sm->getService(String16("SurfaceFlinger")); + sp service = interface_cast(binder); + CHECK(service != NULL); + + service->connectDisplay(NULL); + } + + if (mRTCPSessionID != 0) { + mNetSession->destroySession(mRTCPSessionID); + } + + if (mRTPSessionID != 0) { + mNetSession->destroySession(mRTPSessionID); + } + + return OK; +} + void WifiDisplaySource::PlaybackSession::onMessageReceived( const sp &msg) { switch (msg->what()) { diff --git a/media/libstagefright/wifi-display/source/PlaybackSession.h b/media/libstagefright/wifi-display/source/PlaybackSession.h index 528a039f..cd0308e0 100644 --- a/media/libstagefright/wifi-display/source/PlaybackSession.h +++ b/media/libstagefright/wifi-display/source/PlaybackSession.h @@ -44,6 +44,8 @@ struct WifiDisplaySource::PlaybackSession : public AHandler { const char *clientIP, int32_t clientRtp, int32_t clientRtcp, bool useInterleavedTCP); + status_t destroy(); + int32_t getRTPPort() const; int64_t getLastLifesignUs() const; diff --git a/media/libstagefright/wifi-display/source/WifiDisplaySource.cpp b/media/libstagefright/wifi-display/source/WifiDisplaySource.cpp index 8e8f04a1..aeefcf3e 100644 --- a/media/libstagefright/wifi-display/source/WifiDisplaySource.cpp +++ b/media/libstagefright/wifi-display/source/WifiDisplaySource.cpp @@ -200,11 +200,14 @@ void WifiDisplaySource::onMessageReceived(const sp &msg) { CHECK(msg->senderAwaitsResponse(&replyID)); for (size_t i = mPlaybackSessions.size(); i-- > 0;) { - const sp &playbackSession = + sp playbackSession = mPlaybackSessions.valueAt(i); - looper()->unregisterHandler(playbackSession->id()); mPlaybackSessions.removeItemsAt(i); + + playbackSession->destroy(); + looper()->unregisterHandler(playbackSession->id()); + playbackSession.clear(); } if (mClient != NULL) { @@ -454,9 +457,7 @@ status_t WifiDisplaySource::sendM16(int32_t sessionID) { const ClientInfo &info = mClientInfos.valueFor(sessionID); request.append(StringPrintf("Session: %d\r\n", info.mPlaybackSessionID)); - - request.append("Content-Length: 0\r\n"); - request.append("\r\n"); + request.append("\r\n"); // Empty body status_t err = mNetSession->sendRequest(sessionID, request.c_str(), request.size()); @@ -761,7 +762,7 @@ void WifiDisplaySource::onSetupRequest( return; } #if 1 - // The LG dongle doesn't specify client_port=xxx apparently. + // The older LG dongles doesn't specify client_port=xxx apparently. } else if (transport == "RTP/AVP/UDP;unicast") { clientRtp = 19000; clientRtcp = clientRtp + 1; @@ -966,19 +967,21 @@ void WifiDisplaySource::onSetParameterRequest( int32_t cseq, const sp &data) { int32_t playbackSessionID; -#if 0 - // XXX the dongle does not include a "Session:" header in this request. sp playbackSession = findPlaybackSession(data, &playbackSessionID); +#if 1 + // XXX the older dongles do not include a "Session:" header in this request. + if (playbackSession == NULL) { + CHECK_EQ(mPlaybackSessions.size(), 1u); + playbackSessionID = mPlaybackSessions.keyAt(0); + playbackSession = mPlaybackSessions.valueAt(0); + } +#else if (playbackSession == NULL) { sendErrorResponse(sessionID, "454 Session Not Found", cseq); return; } -#else - CHECK_EQ(mPlaybackSessions.size(), 1u); - playbackSessionID = mPlaybackSessions.keyAt(0); - sp playbackSession = mPlaybackSessions.valueAt(0); #endif playbackSession->updateLiveness(); -- cgit v1.2.3 From ae254b02c5c325f7a9a8d0310c6949b0bc594588 Mon Sep 17 00:00:00 2001 From: Andreas Huber Date: Wed, 12 Sep 2012 16:48:23 -0700 Subject: Better video bandwidth utilization by not lying about the frame rate. log network bandwidth used (for data traffic). Change-Id: I043018624b3f02d94fa9c0cb9d15a6b2f2bd2eab --- media/libstagefright/wifi-display/source/Converter.cpp | 2 +- media/libstagefright/wifi-display/source/PlaybackSession.cpp | 11 ++++++++++- media/libstagefright/wifi-display/source/PlaybackSession.h | 2 ++ 3 files changed, 13 insertions(+), 2 deletions(-) diff --git a/media/libstagefright/wifi-display/source/Converter.cpp b/media/libstagefright/wifi-display/source/Converter.cpp index 0e10b8d0..c4845e3e 100644 --- a/media/libstagefright/wifi-display/source/Converter.cpp +++ b/media/libstagefright/wifi-display/source/Converter.cpp @@ -116,7 +116,7 @@ status_t Converter::initEncoder() { mOutputFormat->setInt32("bitrate", audioBitrate); } else { mOutputFormat->setInt32("bitrate", videoBitrate); - mOutputFormat->setInt32("frame-rate", 60); + mOutputFormat->setInt32("frame-rate", 30); mOutputFormat->setInt32("i-frame-interval", 3); // Iframes every 3 secs } diff --git a/media/libstagefright/wifi-display/source/PlaybackSession.cpp b/media/libstagefright/wifi-display/source/PlaybackSession.cpp index 41beba42..c38a3009 100644 --- a/media/libstagefright/wifi-display/source/PlaybackSession.cpp +++ b/media/libstagefright/wifi-display/source/PlaybackSession.cpp @@ -193,7 +193,8 @@ WifiDisplaySource::PlaybackSession::PlaybackSession( mNumSRsSent(0), mSendSRPending(false), mFirstPacketTimeUs(-1ll), - mHistoryLength(0) + mHistoryLength(0), + mTotalBytesSent(0ll) #if LOG_TRANSPORT_STREAM ,mLogFile(NULL) #endif @@ -1023,6 +1024,14 @@ ssize_t WifiDisplaySource::PlaybackSession::appendTSData( } else { mNetSession->sendRequest( mRTPSessionID, rtp, mTSQueue->size()); + + mTotalBytesSent += mTSQueue->size(); + int64_t delayUs = ALooper::GetNowUs() - mFirstPacketTimeUs; + + if (delayUs > 0ll) { + ALOGV("approx. net bandwidth used: %.2f Mbit/sec", + mTotalBytesSent * 8.0 / delayUs); + } } mTSQueue->setInt32Data(mRTPSeqNo - 1); diff --git a/media/libstagefright/wifi-display/source/PlaybackSession.h b/media/libstagefright/wifi-display/source/PlaybackSession.h index cd0308e0..88f6ea97 100644 --- a/media/libstagefright/wifi-display/source/PlaybackSession.h +++ b/media/libstagefright/wifi-display/source/PlaybackSession.h @@ -127,6 +127,8 @@ private: List > mHistory; size_t mHistoryLength; + uint64_t mTotalBytesSent; + #if LOG_TRANSPORT_STREAM FILE *mLogFile; #endif -- cgit v1.2.3 From 6cdbe6639b21d5e7bbcba295d167c33a6c376c9e Mon Sep 17 00:00:00 2001 From: Eino-Ville Talvala Date: Thu, 20 Sep 2012 14:44:20 -0700 Subject: Camera2: Clean up startup/shutdown sequences. - Close camera device on startup errors - Make sure all threads are shut down and the device is closed before returning from ICamera::disconnect. Bug: 7172680 Change-Id: I98611448ec5f2311e6604fa8ee5f9dde7bfdd988 --- services/camera/libcameraservice/Camera2Client.cpp | 35 ++++++--- services/camera/libcameraservice/Camera2Device.cpp | 85 +++++++++++++++------- services/camera/libcameraservice/Camera2Device.h | 2 +- 3 files changed, 85 insertions(+), 37 deletions(-) diff --git a/services/camera/libcameraservice/Camera2Client.cpp b/services/camera/libcameraservice/Camera2Client.cpp index 4fad83e7..495fedad 100644 --- a/services/camera/libcameraservice/Camera2Client.cpp +++ b/services/camera/libcameraservice/Camera2Client.cpp @@ -137,7 +137,6 @@ status_t Camera2Client::initialize(camera_module_t *module) Camera2Client::~Camera2Client() { ATRACE_CALL(); - ALOGV("Camera %d: Shutting down", mCameraId); mDestructionStarted = true; @@ -145,12 +144,6 @@ Camera2Client::~Camera2Client() { mClientPid = getCallingPid(); disconnect(); - mFrameProcessor->requestExit(); - mCaptureSequencer->requestExit(); - mJpegProcessor->requestExit(); - mZslProcessor->requestExit(); - mCallbackProcessor->requestExit(); - ALOGI("Camera %d: Closed", mCameraId); } @@ -365,15 +358,21 @@ status_t Camera2Client::dump(int fd, const Vector& args) { void Camera2Client::disconnect() { ATRACE_CALL(); - ALOGV("%s: E", __FUNCTION__); Mutex::Autolock icl(mICameraLock); status_t res; if ( (res = checkPid(__FUNCTION__) ) != OK) return; if (mDevice == 0) return; + ALOGV("Camera %d: Shutting down", mCameraId); + stopPreviewL(); + { + SharedParameters::Lock l(mParameters); + l.mParameters.state = Parameters::DISCONNECTED; + } + if (mPreviewStreamId != NO_STREAM) { mDevice->deleteStream(mPreviewStreamId); mPreviewStreamId = NO_STREAM; @@ -390,9 +389,25 @@ void Camera2Client::disconnect() { mZslProcessor->deleteStream(); + mFrameProcessor->requestExit(); + mCaptureSequencer->requestExit(); + mJpegProcessor->requestExit(); + mZslProcessor->requestExit(); + mCallbackProcessor->requestExit(); + + ALOGV("Camera %d: Waiting for threads", mCameraId); + + mFrameProcessor->join(); + mCaptureSequencer->join(); + mJpegProcessor->join(); + mZslProcessor->join(); + mCallbackProcessor->join(); + + ALOGV("Camera %d: Disconnecting device", mCameraId); + + mDevice->disconnect(); + mDevice.clear(); - SharedParameters::Lock l(mParameters); - l.mParameters.state = Parameters::DISCONNECTED; CameraService::Client::disconnect(); } diff --git a/services/camera/libcameraservice/Camera2Device.cpp b/services/camera/libcameraservice/Camera2Device.cpp index a171c46b..81c0496a 100644 --- a/services/camera/libcameraservice/Camera2Device.cpp +++ b/services/camera/libcameraservice/Camera2Device.cpp @@ -38,30 +38,25 @@ Camera2Device::Camera2Device(int id): Camera2Device::~Camera2Device() { - ALOGV("%s: Shutting down device for camera %d", __FUNCTION__, mId); - if (mDevice) { - status_t res; - res = mDevice->common.close(&mDevice->common); - if (res != OK) { - ALOGE("%s: Could not close camera %d: %s (%d)", - __FUNCTION__, - mId, strerror(-res), res); - } - mDevice = NULL; - } - ALOGV("%s: Shutdown complete", __FUNCTION__); + disconnect(); } status_t Camera2Device::initialize(camera_module_t *module) { ALOGV("%s: Initializing device for camera %d", __FUNCTION__, mId); + if (mDevice != NULL) { + ALOGE("%s: Already initialized!", __FUNCTION__); + return INVALID_OPERATION; + } status_t res; char name[10]; snprintf(name, sizeof(name), "%d", mId); + camera2_device_t *device; + res = module->common.methods->open(&module->common, name, - reinterpret_cast(&mDevice)); + reinterpret_cast(&device)); if (res != OK) { ALOGE("%s: Could not open camera %d: %s (%d)", __FUNCTION__, @@ -69,11 +64,12 @@ status_t Camera2Device::initialize(camera_module_t *module) return res; } - if (mDevice->common.version != CAMERA_DEVICE_API_VERSION_2_0) { + if (device->common.version != CAMERA_DEVICE_API_VERSION_2_0) { ALOGE("%s: Could not open camera %d: " "Camera device is not version %x, reports %x instead", __FUNCTION__, mId, CAMERA_DEVICE_API_VERSION_2_0, - mDevice->common.version); + device->common.version); + device->common.close(&device->common); return BAD_VALUE; } @@ -81,45 +77,81 @@ status_t Camera2Device::initialize(camera_module_t *module) res = module->get_camera_info(mId, &info); if (res != OK ) return res; - if (info.device_version != mDevice->common.version) { + if (info.device_version != device->common.version) { ALOGE("%s: HAL reporting mismatched camera_info version (%x)" " and device version (%x).", __FUNCTION__, - mDevice->common.version, info.device_version); + device->common.version, info.device_version); + device->common.close(&device->common); return BAD_VALUE; } - mDeviceInfo = info.static_camera_characteristics; - - res = mRequestQueue.setConsumerDevice(mDevice); + res = mRequestQueue.setConsumerDevice(device); if (res != OK) { ALOGE("%s: Camera %d: Unable to connect request queue to device: %s (%d)", __FUNCTION__, mId, strerror(-res), res); + device->common.close(&device->common); return res; } - res = mFrameQueue.setProducerDevice(mDevice); + res = mFrameQueue.setProducerDevice(device); if (res != OK) { ALOGE("%s: Camera %d: Unable to connect frame queue to device: %s (%d)", __FUNCTION__, mId, strerror(-res), res); + device->common.close(&device->common); return res; } - res = mDevice->ops->get_metadata_vendor_tag_ops(mDevice, &mVendorTagOps); + res = device->ops->get_metadata_vendor_tag_ops(device, &mVendorTagOps); if (res != OK ) { ALOGE("%s: Camera %d: Unable to retrieve tag ops from device: %s (%d)", __FUNCTION__, mId, strerror(-res), res); + device->common.close(&device->common); return res; } res = set_camera_metadata_vendor_tag_ops(mVendorTagOps); if (res != OK) { ALOGE("%s: Camera %d: Unable to set tag ops: %s (%d)", __FUNCTION__, mId, strerror(-res), res); + device->common.close(&device->common); + return res; + } + res = device->ops->set_notify_callback(device, notificationCallback, + NULL); + if (res != OK) { + ALOGE("%s: Camera %d: Unable to initialize notification callback!", + __FUNCTION__, mId); + device->common.close(&device->common); return res; } - setNotifyCallback(NULL); + + mDeviceInfo = info.static_camera_characteristics; + mDevice = device; return OK; } +status_t Camera2Device::disconnect() { + status_t res = OK; + if (mDevice) { + ALOGV("%s: Closing device for camera %d", __FUNCTION__, mId); + + int inProgressCount = mDevice->ops->get_in_progress_count(mDevice); + if (inProgressCount > 0) { + ALOGW("%s: Closing camera device %d with %d requests in flight!", + __FUNCTION__, mId, inProgressCount); + } + mStreams.clear(); + res = mDevice->common.close(&mDevice->common); + if (res != OK) { + ALOGE("%s: Could not close camera %d: %s (%d)", + __FUNCTION__, + mId, strerror(-res), res); + } + mDevice = NULL; + ALOGV("%s: Shutdown complete", __FUNCTION__); + } + return res; +} + status_t Camera2Device::dump(int fd, const Vector& args) { String8 result; @@ -354,7 +386,7 @@ status_t Camera2Device::createDefaultRequest(int templateId, status_t Camera2Device::waitUntilDrained() { static const uint32_t kSleepTime = 50000; // 50 ms static const uint32_t kMaxSleepTime = 10000000; // 10 s - ALOGV("%s: E", __FUNCTION__); + ALOGV("%s: Camera %d: Starting wait", __FUNCTION__, mId); if (mRequestQueue.getBufferCount() == CAMERA2_REQUEST_QUEUE_IS_BOTTOMLESS) return INVALID_OPERATION; @@ -364,11 +396,12 @@ status_t Camera2Device::waitUntilDrained() { usleep(kSleepTime); totalTime += kSleepTime; if (totalTime > kMaxSleepTime) { - ALOGE("%s: Waited %d us, requests still in flight", __FUNCTION__, - totalTime); + ALOGE("%s: Waited %d us, %d requests still in flight", __FUNCTION__, + mDevice->ops->get_in_progress_count(mDevice), totalTime); return TIMED_OUT; } } + ALOGV("%s: Camera %d: HAL is idle", __FUNCTION__, mId); return OK; } diff --git a/services/camera/libcameraservice/Camera2Device.h b/services/camera/libcameraservice/Camera2Device.h index a327d8da..38662e37 100644 --- a/services/camera/libcameraservice/Camera2Device.h +++ b/services/camera/libcameraservice/Camera2Device.h @@ -40,6 +40,7 @@ class Camera2Device : public virtual RefBase { ~Camera2Device(); status_t initialize(camera_module_t *module); + status_t disconnect(); status_t dump(int fd, const Vector& args); @@ -191,7 +192,6 @@ class Camera2Device : public virtual RefBase { buffer_handle_t *buffer, wp listener); private: - const int mId; camera2_device_t *mDevice; -- cgit v1.2.3 From 7352fca8d879f0d65807ed03221bf5c4fd4027b6 Mon Sep 17 00:00:00 2001 From: Andreas Huber Date: Mon, 1 Oct 2012 10:43:55 -0700 Subject: Make sure we still handle shutdown-related message even if we're in error-state. Change-Id: Ie12dd1a63306b4020b9de9eae007f6d768f02df6 related-to-bug: 7262673 --- .../wifi-display/source/PlaybackSession.cpp | 27 ++++++---------------- 1 file changed, 7 insertions(+), 20 deletions(-) diff --git a/media/libstagefright/wifi-display/source/PlaybackSession.cpp b/media/libstagefright/wifi-display/source/PlaybackSession.cpp index 829a16d3..c8b9afd8 100644 --- a/media/libstagefright/wifi-display/source/PlaybackSession.cpp +++ b/media/libstagefright/wifi-display/source/PlaybackSession.cpp @@ -526,10 +526,6 @@ void WifiDisplaySource::PlaybackSession::destroyAsync() { void WifiDisplaySource::PlaybackSession::onMessageReceived( const sp &msg) { - if (mWeAreDead) { - return; - } - switch (msg->what()) { case kWhatRTPNotify: case kWhatRTCPNotify: @@ -661,6 +657,13 @@ void WifiDisplaySource::PlaybackSession::onMessageReceived( case kWhatConverterNotify: { + if (mWeAreDead) { + ALOGV("dropping msg '%s' because we're dead", + msg->debugString().c_str()); + + break; + } + int32_t what; CHECK(msg->findInt32("what", &what)); @@ -1322,10 +1325,6 @@ bool WifiDisplaySource::PlaybackSession::allTracksHavePacketizerIndex() { return true; } -static inline size_t MIN(size_t a, size_t b) { - return (a < b) ? a : b; -} - status_t WifiDisplaySource::PlaybackSession::packetizeAccessUnit( size_t trackIndex, sp accessUnit) { const sp &track = mTracks.valueFor(trackIndex); @@ -1338,11 +1337,6 @@ status_t WifiDisplaySource::PlaybackSession::packetizeAccessUnit( if (mHDCP != NULL && !track->isAudio()) { isHDCPEncrypted = true; -#if 0 - ALOGI("in:"); - hexdump(accessUnit->data(), MIN(64, accessUnit->size())); -#endif - if (IsIDR(accessUnit)) { // XXX remove this once the encoder takes care of this. accessUnit = mPacketizer->prependCSD( @@ -1360,13 +1354,6 @@ status_t WifiDisplaySource::PlaybackSession::packetizeAccessUnit( err); return err; - } else { -#if 0 - ALOGI("out:"); - hexdump(accessUnit->data(), MIN(64, accessUnit->size())); - ALOGI("inputCTR: 0x%016llx", inputCTR); - ALOGI("streamCTR: 0x%08x", trackIndex); -#endif } HDCP_private_data[0] = 0x00; -- cgit v1.2.3 From 27f706e4db4fc53f63e60d80c19a435fb8bf78b9 Mon Sep 17 00:00:00 2001 From: Andreas Huber Date: Mon, 1 Oct 2012 11:22:05 -0700 Subject: Remove double negatives from SurfaceMediaSource ensure mStarted actually reflects the state of SurfaceMediaSource Change-Id: I92557896993ad8da23fe6940e997402ad63b8cbc related-to-bug: 7258622 --- include/media/stagefright/SurfaceMediaSource.h | 4 ++-- media/libstagefright/SurfaceMediaSource.cpp | 17 ++++++++++------- 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/include/media/stagefright/SurfaceMediaSource.h b/include/media/stagefright/SurfaceMediaSource.h index f60a5354..82e59653 100644 --- a/include/media/stagefright/SurfaceMediaSource.h +++ b/include/media/stagefright/SurfaceMediaSource.h @@ -193,8 +193,8 @@ private: // Set to a default of 30 fps if not specified by the client side int32_t mFrameRate; - // mStopped is a flag to check if the recording is going on - bool mStopped; + // mStarted is a flag to check if the recording is going on + bool mStarted; // mNumFramesReceived indicates the number of frames recieved from // the client side diff --git a/media/libstagefright/SurfaceMediaSource.cpp b/media/libstagefright/SurfaceMediaSource.cpp index 9d39d0eb..97f9c7cc 100644 --- a/media/libstagefright/SurfaceMediaSource.cpp +++ b/media/libstagefright/SurfaceMediaSource.cpp @@ -42,7 +42,7 @@ SurfaceMediaSource::SurfaceMediaSource(uint32_t bufferWidth, uint32_t bufferHeig mNumPendingBuffers(0), mCurrentTimestamp(0), mFrameRate(30), - mStopped(false), + mStarted(false), mNumFramesReceived(0), mNumFramesEncoded(0), mFirstFrameTimestamp(0), @@ -80,7 +80,7 @@ SurfaceMediaSource::SurfaceMediaSource(uint32_t bufferWidth, uint32_t bufferHeig SurfaceMediaSource::~SurfaceMediaSource() { ALOGV("~SurfaceMediaSource"); - CHECK(mStopped == true); + CHECK(!mStarted); } nsecs_t SurfaceMediaSource::getTimestamp() { @@ -140,6 +140,8 @@ status_t SurfaceMediaSource::start(MetaData *params) Mutex::Autolock lock(mMutex); + CHECK(!mStarted); + mStartTimeNs = 0; int64_t startTimeUs; int32_t bufferCount = 0; @@ -171,6 +173,7 @@ status_t SurfaceMediaSource::start(MetaData *params) } mNumPendingBuffers = 0; + mStarted = true; return OK; } @@ -191,7 +194,7 @@ status_t SurfaceMediaSource::stop() ALOGV("stop"); Mutex::Autolock lock(mMutex); - if (mStopped) { + if (!mStarted) { return OK; } @@ -208,7 +211,7 @@ status_t SurfaceMediaSource::stop() mMediaBuffersAvailableCondition.wait(mMutex); } - mStopped = true; + mStarted = false; mFrameAvailableCondition.signal(); mMediaBuffersAvailableCondition.signal(); @@ -270,7 +273,7 @@ status_t SurfaceMediaSource::read( MediaBuffer **buffer, *buffer = NULL; - while (!mStopped && mNumPendingBuffers == mMaxAcquiredBufferCount) { + while (mStarted && mNumPendingBuffers == mMaxAcquiredBufferCount) { mMediaBuffersAvailableCondition.wait(mMutex); } @@ -281,7 +284,7 @@ status_t SurfaceMediaSource::read( MediaBuffer **buffer, BufferQueue::BufferItem item; // If the recording has started and the queue is empty, then just // wait here till the frames come in from the client side - while (!mStopped) { + while (mStarted) { status_t err = mBufferQueue->acquireBuffer(&item); if (err == BufferQueue::NO_BUFFER_AVAILABLE) { @@ -322,7 +325,7 @@ status_t SurfaceMediaSource::read( MediaBuffer **buffer, // If the loop was exited as a result of stopping the recording, // it is OK - if (mStopped) { + if (!mStarted) { ALOGV("Read: SurfaceMediaSource is stopped. Returning ERROR_END_OF_STREAM."); return ERROR_END_OF_STREAM; } -- cgit v1.2.3 From 32ca2bfbe8b74a9d22d012c3df57983a661b8088 Mon Sep 17 00:00:00 2001 From: Andreas Huber Date: Mon, 1 Oct 2012 11:26:30 -0700 Subject: Perform an orderly shutdown if possible, force disconnect if necessary wait for up to 2 secs for the dongle to send us a "TEARDOWN", after that forcibly shutdown the connection. Change-Id: Ie049857cd468b7af6986d6305f725c54571b2276 related-to-bug: 7258622 --- .../wifi-display/source/PlaybackSession.cpp | 4 +- .../wifi-display/source/WifiDisplaySource.cpp | 54 ++++++++++++++++++++-- .../wifi-display/source/WifiDisplaySource.h | 20 ++++++++ 3 files changed, 71 insertions(+), 7 deletions(-) diff --git a/media/libstagefright/wifi-display/source/PlaybackSession.cpp b/media/libstagefright/wifi-display/source/PlaybackSession.cpp index c8b9afd8..9c065b28 100644 --- a/media/libstagefright/wifi-display/source/PlaybackSession.cpp +++ b/media/libstagefright/wifi-display/source/PlaybackSession.cpp @@ -173,13 +173,11 @@ status_t WifiDisplaySource::PlaybackSession::Track::start() { void WifiDisplaySource::PlaybackSession::Track::stopAsync() { ALOGV("Track::stopAsync isAudio=%d", mIsAudio); - CHECK(mStarted); - mConverter->shutdownAsync(); sp msg = new AMessage(kWhatMediaPullerStopped, id()); - if (mMediaPuller != NULL) { + if (mStarted && mMediaPuller != NULL) { mMediaPuller->stopAsync(msg); } else { msg->post(); diff --git a/media/libstagefright/wifi-display/source/WifiDisplaySource.cpp b/media/libstagefright/wifi-display/source/WifiDisplaySource.cpp index d5ffc650..adb15499 100644 --- a/media/libstagefright/wifi-display/source/WifiDisplaySource.cpp +++ b/media/libstagefright/wifi-display/source/WifiDisplaySource.cpp @@ -41,7 +41,8 @@ namespace android { WifiDisplaySource::WifiDisplaySource( const sp &netSession, const sp &client) - : mNetSession(netSession), + : mState(INITIALIZED), + mNetSession(netSession), mClient(client), mSessionID(0), mStopReplyID(0), @@ -61,6 +62,8 @@ WifiDisplaySource::~WifiDisplaySource() { } status_t WifiDisplaySource::start(const char *iface) { + CHECK_EQ(mState, INITIALIZED); + sp msg = new AMessage(kWhatStart, id()); msg->setString("iface", iface); @@ -137,6 +140,10 @@ void WifiDisplaySource::onMessageReceived(const sp &msg) { } } + if (err == OK) { + mState = AWAITING_CLIENT_CONNECTION; + } + sp response = new AMessage; response->setInt32("err", err); response->postReply(replyID); @@ -190,6 +197,8 @@ void WifiDisplaySource::onMessageReceived(const sp &msg) { break; } + CHECK_EQ(mState, AWAITING_CLIENT_CONNECTION); + CHECK(msg->findString("client-ip", &mClientInfo.mRemoteIP)); CHECK(msg->findString("server-ip", &mClientInfo.mLocalIP)); @@ -208,6 +217,8 @@ void WifiDisplaySource::onMessageReceived(const sp &msg) { ALOGI("We now have a client (%d) connected.", sessionID); + mState = AWAITING_CLIENT_SETUP; + status_t err = sendM1(sessionID); CHECK_EQ(err, (status_t)OK); break; @@ -234,14 +245,24 @@ void WifiDisplaySource::onMessageReceived(const sp &msg) { { CHECK(msg->senderAwaitsResponse(&mStopReplyID)); - if (mClientSessionID != 0 - && mClientInfo.mPlaybackSessionID != -1) { + CHECK_LT(mState, AWAITING_CLIENT_TEARDOWN); + + if (mState >= AWAITING_CLIENT_PLAY) { + // We have a session, i.e. a previous SETUP succeeded. + status_t err = sendM5( mClientSessionID, true /* requestShutdown */); if (err == OK) { + mState = AWAITING_CLIENT_TEARDOWN; + + (new AMessage(kWhatTeardownTriggerTimedOut, id()))->post( + kTeardownTriggerTimeouSecs * 1000000ll); + break; } + + // fall through. } finishStop(); @@ -293,6 +314,10 @@ void WifiDisplaySource::onMessageReceived(const sp &msg) { mClientInfo.mPlaybackSession->height(), 0 /* flags */); } + + if (mState == ABOUT_TO_PLAY) { + mState = PLAYING; + } } else if (what == PlaybackSession::kWhatSessionDestroyed) { disconnectClient2(); } else { @@ -339,6 +364,18 @@ void WifiDisplaySource::onMessageReceived(const sp &msg) { break; } + case kWhatTeardownTriggerTimedOut: + { + if (mState == AWAITING_CLIENT_TEARDOWN) { + ALOGI("TEARDOWN trigger timed out, forcing disconnection."); + + CHECK_NE(mStopReplyID, 0); + finishStop(); + break; + } + break; + } + #if REQUIRE_HDCP case kWhatHDCPNotify: { @@ -1020,6 +1057,8 @@ status_t WifiDisplaySource::onSetupRequest( return err; } + mState = AWAITING_CLIENT_PLAY; + scheduleReaper(); scheduleKeepAlive(sessionID); @@ -1057,6 +1096,9 @@ status_t WifiDisplaySource::onPlayRequest( playbackSession->finishPlay(); + CHECK_EQ(mState, AWAITING_CLIENT_PLAY); + mState = ABOUT_TO_PLAY; + return OK; } @@ -1107,7 +1149,8 @@ status_t WifiDisplaySource::onTeardownRequest( mNetSession->sendRequest(sessionID, response.c_str()); - if (mStopReplyID != 0) { + if (mState == AWAITING_CLIENT_TEARDOWN) { + CHECK_NE(mStopReplyID, 0); finishStop(); } else { mClient->onDisplayError(IRemoteDisplayClient::kDisplayErrorUnknown); @@ -1119,6 +1162,8 @@ status_t WifiDisplaySource::onTeardownRequest( void WifiDisplaySource::finishStop() { ALOGV("finishStop"); + mState = STOPPING; + disconnectClientAsync(); } @@ -1153,6 +1198,7 @@ void WifiDisplaySource::finishStop2() { } ALOGI("We're stopped."); + mState = STOPPED; status_t err = OK; diff --git a/media/libstagefright/wifi-display/source/WifiDisplaySource.h b/media/libstagefright/wifi-display/source/WifiDisplaySource.h index ade623aa..8c043cd0 100644 --- a/media/libstagefright/wifi-display/source/WifiDisplaySource.h +++ b/media/libstagefright/wifi-display/source/WifiDisplaySource.h @@ -55,6 +55,18 @@ private: struct HDCPObserver; #endif + enum State { + INITIALIZED, + AWAITING_CLIENT_CONNECTION, + AWAITING_CLIENT_SETUP, + AWAITING_CLIENT_PLAY, + ABOUT_TO_PLAY, + PLAYING, + AWAITING_CLIENT_TEARDOWN, + STOPPING, + STOPPED, + }; + enum { kWhatStart, kWhatRTSPNotify, @@ -64,6 +76,7 @@ private: kWhatKeepAlive, kWhatHDCPNotify, kWhatFinishStop2, + kWhatTeardownTriggerTimedOut, }; struct ResponseID { @@ -82,11 +95,18 @@ private: static const int64_t kReaperIntervalUs = 1000000ll; + // We request that the dongle send us a "TEARDOWN" in order to + // perform an orderly shutdown. We're willing to wait up to 2 secs + // for this message to arrive, after that we'll force a disconnect + // instead. + static const int64_t kTeardownTriggerTimeouSecs = 2; + static const int64_t kPlaybackSessionTimeoutSecs = 30; static const int64_t kPlaybackSessionTimeoutUs = kPlaybackSessionTimeoutSecs * 1000000ll; + State mState; sp mNetSession; sp mClient; struct in_addr mInterfaceAddr; -- cgit v1.2.3 From 0c1916460b8adbfe402778aa247e98573c5849e8 Mon Sep 17 00:00:00 2001 From: Andreas Huber Date: Wed, 3 Oct 2012 09:01:11 -0700 Subject: Increase polling frequency again temporarily to fix A/V issues Fixes a bug in the silence detection and increases the timeout after which we enter "silent mode" to 10 secs. Change-Id: I802b058f054becd5c377186664437f7b3970193f related-to-bug: 7248248 --- .../wifi-display/source/Converter.cpp | 29 +++++++++++----------- .../wifi-display/source/PlaybackSession.cpp | 9 +++++++ 2 files changed, 24 insertions(+), 14 deletions(-) diff --git a/media/libstagefright/wifi-display/source/Converter.cpp b/media/libstagefright/wifi-display/source/Converter.cpp index 6f336c7a..f044666b 100644 --- a/media/libstagefright/wifi-display/source/Converter.cpp +++ b/media/libstagefright/wifi-display/source/Converter.cpp @@ -242,16 +242,18 @@ void Converter::onMessageReceived(const sp &msg) { #if ENABLE_SILENCE_DETECTION if (!mIsVideo) { if (IsSilence(accessUnit)) { - if (!mInSilentMode) { - int64_t nowUs = ALooper::GetNowUs(); - - if (mFirstSilentFrameUs < 0ll) { - mFirstSilentFrameUs = nowUs; - } else if (nowUs >= mFirstSilentFrameUs + 1000000ll) { - mInSilentMode = true; - ALOGI("audio in silent mode now."); - break; - } + if (mInSilentMode) { + break; + } + + int64_t nowUs = ALooper::GetNowUs(); + + if (mFirstSilentFrameUs < 0ll) { + mFirstSilentFrameUs = nowUs; + } else if (nowUs >= mFirstSilentFrameUs + 10000000ll) { + mInSilentMode = true; + ALOGI("audio in silent mode now."); + break; } } else { if (mInSilentMode) { @@ -326,7 +328,7 @@ void Converter::scheduleDoMoreWork() { } mDoMoreWorkPending = true; - (new AMessage(kWhatDoMoreWork, id()))->post(mIsVideo ? 10000ll : 5000ll); + (new AMessage(kWhatDoMoreWork, id()))->post(1000ll); } status_t Converter::feedEncoderInputBuffers() { @@ -404,9 +406,8 @@ status_t Converter::doMoreWork() { sp buffer = new ABuffer(size); buffer->meta()->setInt64("timeUs", timeUs); - if (!mIsVideo) { - ALOGV("audio time %lld us (%.2f secs)", timeUs, timeUs / 1E6); - } + ALOGV("[%s] time %lld us (%.2f secs)", + mIsVideo ? "video" : "audio", timeUs, timeUs / 1E6); memcpy(buffer->data(), mEncoderOutputBuffers.itemAt(bufferIndex)->base() + offset, diff --git a/media/libstagefright/wifi-display/source/PlaybackSession.cpp b/media/libstagefright/wifi-display/source/PlaybackSession.cpp index c91b4c8f..ffdafed9 100644 --- a/media/libstagefright/wifi-display/source/PlaybackSession.cpp +++ b/media/libstagefright/wifi-display/source/PlaybackSession.cpp @@ -919,13 +919,22 @@ status_t WifiDisplaySource::PlaybackSession::addVideoSource() { source->setUseAbsoluteTimestamps(); +#if 1 sp videoSource = new RepeaterSource(source, 30.0 /* rateHz */); +#endif +#if 1 size_t numInputBuffers; status_t err = addSource( true /* isVideo */, videoSource, true /* isRepeaterSource */, &numInputBuffers); +#else + size_t numInputBuffers; + status_t err = addSource( + true /* isVideo */, source, false /* isRepeaterSource */, + &numInputBuffers); +#endif if (err != OK) { return err; -- cgit v1.2.3 From ab99f0450dc5743415a2817b539c9e757ede4836 Mon Sep 17 00:00:00 2001 From: Andreas Huber Date: Wed, 3 Oct 2012 10:16:58 -0700 Subject: Better power savings with wifi display code. No more polling the encoder for work to do, the encoder instead notifies if there's activity. Change-Id: Ia707211b4f5c5a6e6b70d750233d204a2d6bb778 related-to-bug: 7248248 --- include/media/stagefright/MediaCodec.h | 42 +++++++++++++-------- media/libstagefright/MediaCodec.cpp | 43 ++++++++++++++++++++++ .../wifi-display/source/Converter.cpp | 39 +++++++++++++++++--- .../libstagefright/wifi-display/source/Converter.h | 2 + 4 files changed, 104 insertions(+), 22 deletions(-) diff --git a/include/media/stagefright/MediaCodec.h b/include/media/stagefright/MediaCodec.h index 8c612d4a..cacfa54e 100644 --- a/include/media/stagefright/MediaCodec.h +++ b/include/media/stagefright/MediaCodec.h @@ -108,6 +108,11 @@ struct MediaCodec : public AHandler { status_t requestIDRFrame(); + // Notification will be posted once there "is something to do", i.e. + // an input/output buffer has become available, a format change is + // pending, an error is pending. + void requestActivityNotification(const sp ¬ify); + protected: virtual ~MediaCodec(); virtual void onMessageReceived(const sp &msg); @@ -132,22 +137,23 @@ private: }; enum { - kWhatInit = 'init', - kWhatConfigure = 'conf', - kWhatStart = 'strt', - kWhatStop = 'stop', - kWhatRelease = 'rele', - kWhatDequeueInputBuffer = 'deqI', - kWhatQueueInputBuffer = 'queI', - kWhatDequeueOutputBuffer = 'deqO', - kWhatReleaseOutputBuffer = 'relO', - kWhatGetBuffers = 'getB', - kWhatFlush = 'flus', - kWhatGetOutputFormat = 'getO', - kWhatDequeueInputTimedOut = 'dITO', - kWhatDequeueOutputTimedOut = 'dOTO', - kWhatCodecNotify = 'codc', - kWhatRequestIDRFrame = 'ridr', + kWhatInit = 'init', + kWhatConfigure = 'conf', + kWhatStart = 'strt', + kWhatStop = 'stop', + kWhatRelease = 'rele', + kWhatDequeueInputBuffer = 'deqI', + kWhatQueueInputBuffer = 'queI', + kWhatDequeueOutputBuffer = 'deqO', + kWhatReleaseOutputBuffer = 'relO', + kWhatGetBuffers = 'getB', + kWhatFlush = 'flus', + kWhatGetOutputFormat = 'getO', + kWhatDequeueInputTimedOut = 'dITO', + kWhatDequeueOutputTimedOut = 'dOTO', + kWhatCodecNotify = 'codc', + kWhatRequestIDRFrame = 'ridr', + kWhatRequestActivityNotification = 'racN', }; enum { @@ -191,6 +197,8 @@ private: List > mCSD; + sp mActivityNotify; + MediaCodec(const sp &looper); static status_t PostAndAwaitResponse( @@ -216,6 +224,8 @@ private: status_t setNativeWindow( const sp &surfaceTextureClient); + void postActivityNotificationIfPossible(); + DISALLOW_EVIL_CONSTRUCTORS(MediaCodec); }; diff --git a/media/libstagefright/MediaCodec.cpp b/media/libstagefright/MediaCodec.cpp index 7f974304..56e6df02 100644 --- a/media/libstagefright/MediaCodec.cpp +++ b/media/libstagefright/MediaCodec.cpp @@ -333,6 +333,12 @@ status_t MediaCodec::requestIDRFrame() { return OK; } +void MediaCodec::requestActivityNotification(const sp ¬ify) { + sp msg = new AMessage(kWhatRequestActivityNotification, id()); + msg->setMessage("notify", notify); + msg->post(); +} + //////////////////////////////////////////////////////////////////////////////// void MediaCodec::cancelPendingDequeueOperations() { @@ -498,6 +504,7 @@ void MediaCodec::onMessageReceived(const sp &msg) { sendErrorReponse = false; mFlags |= kFlagStickyError; + postActivityNotificationIfPossible(); cancelPendingDequeueOperations(); break; @@ -508,6 +515,7 @@ void MediaCodec::onMessageReceived(const sp &msg) { sendErrorReponse = false; mFlags |= kFlagStickyError; + postActivityNotificationIfPossible(); break; } } @@ -600,6 +608,7 @@ void MediaCodec::onMessageReceived(const sp &msg) { (new AMessage)->postReply(mReplyID); } else { mFlags |= kFlagOutputBuffersChanged; + postActivityNotificationIfPossible(); } } break; @@ -638,6 +647,7 @@ void MediaCodec::onMessageReceived(const sp &msg) { mOutputFormat = msg; mFlags |= kFlagOutputFormatChanged; + postActivityNotificationIfPossible(); break; } @@ -669,6 +679,8 @@ void MediaCodec::onMessageReceived(const sp &msg) { err); mFlags |= kFlagStickyError; + postActivityNotificationIfPossible(); + cancelPendingDequeueOperations(); } break; @@ -680,6 +692,8 @@ void MediaCodec::onMessageReceived(const sp &msg) { ++mDequeueInputTimeoutGeneration; mFlags &= ~kFlagDequeueInputPending; mDequeueInputReplyID = 0; + } else { + postActivityNotificationIfPossible(); } break; } @@ -709,7 +723,10 @@ void MediaCodec::onMessageReceived(const sp &msg) { ++mDequeueOutputTimeoutGeneration; mFlags &= ~kFlagDequeueOutputPending; mDequeueOutputReplyID = 0; + } else { + postActivityNotificationIfPossible(); } + break; } @@ -1145,6 +1162,15 @@ void MediaCodec::onMessageReceived(const sp &msg) { break; } + case kWhatRequestActivityNotification: + { + CHECK(mActivityNotify == NULL); + CHECK(msg->findMessage("notify", &mActivityNotify)); + + postActivityNotificationIfPossible(); + break; + } + default: TRESPASS(); } @@ -1210,6 +1236,8 @@ void MediaCodec::setState(State newState) { mFlags &= ~kFlagOutputFormatChanged; mFlags &= ~kFlagOutputBuffersChanged; mFlags &= ~kFlagStickyError; + + mActivityNotify.clear(); } mState = newState; @@ -1477,4 +1505,19 @@ status_t MediaCodec::setNativeWindow( return OK; } +void MediaCodec::postActivityNotificationIfPossible() { + if (mActivityNotify == NULL) { + return; + } + + if ((mFlags & (kFlagStickyError + | kFlagOutputBuffersChanged + | kFlagOutputFormatChanged)) + || !mAvailPortBuffers[kPortIndexInput].empty() + || !mAvailPortBuffers[kPortIndexOutput].empty()) { + mActivityNotify->post(); + mActivityNotify.clear(); + } +} + } // namespace android diff --git a/media/libstagefright/wifi-display/source/Converter.cpp b/media/libstagefright/wifi-display/source/Converter.cpp index f044666b..a9b4c238 100644 --- a/media/libstagefright/wifi-display/source/Converter.cpp +++ b/media/libstagefright/wifi-display/source/Converter.cpp @@ -274,8 +274,17 @@ void Converter::onMessageReceived(const sp &msg) { break; } - case kWhatDoMoreWork: + case kWhatEncoderActivity: { +#if 0 + int64_t whenUs; + if (msg->findInt64("whenUs", &whenUs)) { + int64_t nowUs = ALooper::GetNowUs(); + ALOGI("[%s] kWhatEncoderActivity after %lld us", + mIsVideo ? "video" : "audio", nowUs - whenUs); + } +#endif + mDoMoreWorkPending = false; if (mEncoder == NULL) { @@ -328,7 +337,17 @@ void Converter::scheduleDoMoreWork() { } mDoMoreWorkPending = true; - (new AMessage(kWhatDoMoreWork, id()))->post(1000ll); + +#if 1 + if (mEncoderActivityNotify == NULL) { + mEncoderActivityNotify = new AMessage(kWhatEncoderActivity, id()); + } + mEncoder->requestActivityNotification(mEncoderActivityNotify->dup()); +#else + sp notify = new AMessage(kWhatEncoderActivity, id()); + notify->setInt64("whenUs", ALooper::GetNowUs()); + mEncoder->requestActivityNotification(notify); +#endif } status_t Converter::feedEncoderInputBuffers() { @@ -375,15 +394,23 @@ status_t Converter::feedEncoderInputBuffers() { } status_t Converter::doMoreWork() { - size_t bufferIndex; - status_t err = mEncoder->dequeueInputBuffer(&bufferIndex); + status_t err; + + for (;;) { + size_t bufferIndex; + err = mEncoder->dequeueInputBuffer(&bufferIndex); + + if (err != OK) { + break; + } - if (err == OK) { mAvailEncoderInputIndices.push_back(bufferIndex); - feedEncoderInputBuffers(); } + feedEncoderInputBuffers(); + for (;;) { + size_t bufferIndex; size_t offset; size_t size; int64_t timeUs; diff --git a/media/libstagefright/wifi-display/source/Converter.h b/media/libstagefright/wifi-display/source/Converter.h index 93ff72fb..8d45395b 100644 --- a/media/libstagefright/wifi-display/source/Converter.h +++ b/media/libstagefright/wifi-display/source/Converter.h @@ -58,6 +58,7 @@ struct Converter : public AHandler { kWhatRequestIDRFrame, kWhatShutdown, kWhatMediaPullerNotify, + kWhatEncoderActivity, }; void shutdownAsync(); @@ -75,6 +76,7 @@ private: sp mOutputFormat; sp mEncoder; + sp mEncoderActivityNotify; Vector > mEncoderInputBuffers; Vector > mEncoderOutputBuffers; -- cgit v1.2.3 From 0c978a82784ce90c95e17040033f292d562680f4 Mon Sep 17 00:00:00 2001 From: Dave Burke Date: Tue, 2 Oct 2012 18:39:39 -0700 Subject: Drop video bitrate to 2.5mbps. We have too much corruption/loss at higher bitrates. Reviewed quality trade-off with jdong@. We can increase in the future if we have a better packet loss concealment solution. Bug: 7241844 Change-Id: I3b500a9c3a4429e834fb4c5ca04164d4e106fa9e --- media/libstagefright/wifi-display/source/Converter.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/media/libstagefright/wifi-display/source/Converter.cpp b/media/libstagefright/wifi-display/source/Converter.cpp index a9b4c238..0e8c9af1 100644 --- a/media/libstagefright/wifi-display/source/Converter.cpp +++ b/media/libstagefright/wifi-display/source/Converter.cpp @@ -128,7 +128,7 @@ status_t Converter::initEncoder() { mOutputFormat->setString("mime", outputMIME.c_str()); int32_t audioBitrate = getBitrate("media.wfd.audio-bitrate", 128000); - int32_t videoBitrate = getBitrate("media.wfd.video-bitrate", 5000000); + int32_t videoBitrate = getBitrate("media.wfd.video-bitrate", 2500000); ALOGI("using audio bitrate of %d bps, video bitrate of %d bps", audioBitrate, videoBitrate); -- cgit v1.2.3 From d6cc864079212a384692204a4c0183e8bf145121 Mon Sep 17 00:00:00 2001 From: Eino-Ville Talvala Date: Wed, 3 Oct 2012 14:59:29 -0700 Subject: Camera2: Properly update FPS range when FPS is set. Otherwise a getParameters followed by setParameters will trigger an incorrect reversion in FPS parameters. Bug: 7279267 Change-Id: I7426860f05497dbdf4245c34cef1b38f2f5a1832 --- services/camera/libcameraservice/camera2/Parameters.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/services/camera/libcameraservice/camera2/Parameters.cpp b/services/camera/libcameraservice/camera2/Parameters.cpp index fd44a3e3..5ba564d9 100644 --- a/services/camera/libcameraservice/camera2/Parameters.cpp +++ b/services/camera/libcameraservice/camera2/Parameters.cpp @@ -1039,6 +1039,10 @@ status_t Parameters::set(const String8& paramString) { validatedParams.previewFpsRange[1] = availableFrameRates.data.i32[i+1]; } + newParams.set(CameraParameters::KEY_PREVIEW_FPS_RANGE, + String8::format("%d,%d", + validatedParams.previewFpsRange[0] * kFpsToApiScale, + validatedParams.previewFpsRange[1] * kFpsToApiScale)); } // PICTURE_SIZE -- cgit v1.2.3 From c10ea778f589283d67ede3a0e1c10ab960962e77 Mon Sep 17 00:00:00 2001 From: Igor Murashkin Date: Wed, 3 Oct 2012 16:02:09 -0700 Subject: Camera2: Fix deadlock while zooming during record Acquired SharedParameters before mMutex in StreamingProcessor, this avoids any potential deadlocks since Camera2Client code would always acquire SharedParameters first before invoking StreamingProcessor. Bug: 7275259 Change-Id: Ia741162c455300378bee049f063590ece5328b95 --- .../camera2/StreamingProcessor.cpp | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/services/camera/libcameraservice/camera2/StreamingProcessor.cpp b/services/camera/libcameraservice/camera2/StreamingProcessor.cpp index 744b7ed2..8921172b 100644 --- a/services/camera/libcameraservice/camera2/StreamingProcessor.cpp +++ b/services/camera/libcameraservice/camera2/StreamingProcessor.cpp @@ -422,6 +422,9 @@ void StreamingProcessor::onFrameAvailable() { if (client == 0) return; { + /* acquire SharedParameters before mMutex so we don't dead lock + with Camera2Client code calling into StreamingProcessor */ + SharedParameters::Lock l(client->getParameters()); Mutex::Autolock m(mMutex); BufferItemConsumer::BufferItem imgBuffer; res = mRecordingConsumer->acquireBuffer(&imgBuffer); @@ -435,17 +438,14 @@ void StreamingProcessor::onFrameAvailable() { mRecordingFrameCount++; ALOGV("OnRecordingFrame: Frame %d", mRecordingFrameCount); - { - SharedParameters::Lock l(client->getParameters()); - // TODO: Signal errors here upstream - if (l.mParameters.state != Parameters::RECORD && - l.mParameters.state != Parameters::VIDEO_SNAPSHOT) { - ALOGV("%s: Camera %d: Discarding recording image buffers " - "received after recording done", __FUNCTION__, - client->getCameraId()); - mRecordingConsumer->releaseBuffer(imgBuffer); - return; - } + // TODO: Signal errors here upstream + if (l.mParameters.state != Parameters::RECORD && + l.mParameters.state != Parameters::VIDEO_SNAPSHOT) { + ALOGV("%s: Camera %d: Discarding recording image buffers " + "received after recording done", __FUNCTION__, + client->getCameraId()); + mRecordingConsumer->releaseBuffer(imgBuffer); + return; } if (mRecordingHeap == 0) { -- cgit v1.2.3 From ef33212df1b937e187cca4ffcb9269775e1a4dd8 Mon Sep 17 00:00:00 2001 From: Igor Murashkin Date: Fri, 5 Oct 2012 10:44:57 -0700 Subject: Camera2: Don't promote weak IBinder ptrs to strong ones The Binder driver does not support promoting weak pointers into strong pointers. Occassionally the system would lock up when trying to do this (when closing the camera app). Bug: 7289040 Change-Id: I8bc0b5c48616bf0b7f4eed1878ad4994ee635871 --- services/camera/libcameraservice/CameraService.cpp | 22 ++++++++++++---------- services/camera/libcameraservice/CameraService.h | 4 ++-- 2 files changed, 14 insertions(+), 12 deletions(-) diff --git a/services/camera/libcameraservice/CameraService.cpp b/services/camera/libcameraservice/CameraService.cpp index 4d48d8d7..6fbd6edb 100644 --- a/services/camera/libcameraservice/CameraService.cpp +++ b/services/camera/libcameraservice/CameraService.cpp @@ -227,7 +227,7 @@ void CameraService::removeClient(const sp& cameraClient) { Mutex::Autolock lock(mServiceLock); int outIndex; - sp client = findClientUnsafe(cameraClient, outIndex); + sp client = findClientUnsafe(cameraClient->asBinder(), outIndex); if (client != 0) { // Found our camera, clear and leave. @@ -241,7 +241,7 @@ void CameraService::removeClient(const sp& cameraClient) { } sp CameraService::findClientUnsafe( - const sp& cameraClient, int& outIndex) { + const wp& cameraClient, int& outIndex) { sp client; for (int i = 0; i < mNumberOfCameras; i++) { @@ -260,7 +260,7 @@ sp CameraService::findClientUnsafe( continue; } - if (cameraClient->asBinder() == client->getCameraClient()->asBinder()) { + if (cameraClient == client->getCameraClient()->asBinder()) { // Found our camera outIndex = i; return client; @@ -281,8 +281,8 @@ Mutex* CameraService::getClientLockById(int cameraId) { return &mClientLock[cameraId]; } -/*virtual*/sp CameraService::getClientByRemote( - const sp& cameraClient) { +sp CameraService::getClientByRemote( + const wp& cameraClient) { // Declare this before the lock to make absolutely sure the // destructor won't be called with the lock held. @@ -557,18 +557,20 @@ status_t CameraService::dump(int fd, const Vector& args) { /*virtual*/void CameraService::binderDied( const wp &who) { + /** + * While tempting to promote the wp into a sp, + * it's actually not supported by the binder driver + */ + ALOGV("java clients' binder died"); - sp whoStrong = who.promote(); + sp cameraClient = getClientByRemote(who); - if (whoStrong == 0) { + if (cameraClient == 0) { ALOGV("java clients' binder death already cleaned up (normal case)"); return; } - sp iCamClient = interface_cast(whoStrong); - - sp cameraClient = getClientByRemote(iCamClient); ALOGW("Disconnecting camera client %p since the binder for it " "died (this pid %d)", cameraClient.get(), getCallingPid()); diff --git a/services/camera/libcameraservice/CameraService.h b/services/camera/libcameraservice/CameraService.h index f1e7df64..4dab340d 100644 --- a/services/camera/libcameraservice/CameraService.h +++ b/services/camera/libcameraservice/CameraService.h @@ -55,7 +55,7 @@ public: virtual Client* getClientByIdUnsafe(int cameraId); virtual Mutex* getClientLockById(int cameraId); - virtual sp getClientByRemote(const sp& cameraClient); + virtual sp getClientByRemote(const wp& cameraClient); virtual status_t dump(int fd, const Vector& args); virtual status_t onTransact(uint32_t code, const Parcel& data, @@ -143,7 +143,7 @@ private: int mNumberOfCameras; // needs to be called with mServiceLock held - sp findClientUnsafe(const sp& cameraClient, int& outIndex); + sp findClientUnsafe(const wp& cameraClient, int& outIndex); // atomics to record whether the hardware is allocated to some client. volatile int32_t mBusy[MAX_CAMERAS]; -- cgit v1.2.3