From dacc5afbf0f1bde7454c1eadf56edb669d0741a9 Mon Sep 17 00:00:00 2001 From: Monroe Linden Date: Tue, 27 Apr 2010 17:25:01 -0700 Subject: Architectural changes to LLPlugin message processing. LLPluginProcessParent can now optionally use a separate thread for reading messages from plugin sockets. If this is enabled, it will spawn a single thread and use apr_pollset_poll to wake up the thread when incoming data arrives instead of polling all the descriptors round-robin every frame. This should be somewhat more efficient, and should also allow blocking requests from plugins to be serviced much more quickly (once we start using them). This is currently disabled by default, until it's had a bit more focused testing on multiple platforms. Hooked up the switch to use the message read thread to the PluginUseReadThread debug setting and an item in the Advanced menu in the viewer, and to a checkbox in the UI in llmediaplugintest. Updated some debug logging in the plugin system to have appropriate tags and not log dire-looking warnings during normal operation. LLPluginProcessParent now once again explicitly kills plugin processes (instead of just closing their sockets and waiting for them to exit). The problem we were attempting to solve by not doing the kill (letting the webkit plugin write its cookie file on exit) has been solved another way. LLPluginProcessParent::sendMessage() now attempts to write the outgoing message to the socket immediately instead of waiting for the next frame. This should reduce the latency of sending plugin messages. Added a separate fast timer for LLViewerMedia::updateMedia(). --- indra/newview/llviewermedia.cpp | 11 +++++++++++ 1 file changed, 11 insertions(+) (limited to 'indra/newview/llviewermedia.cpp') diff --git a/indra/newview/llviewermedia.cpp b/indra/newview/llviewermedia.cpp index fd2bb0fdf9..8e210554fb 100644 --- a/indra/newview/llviewermedia.cpp +++ b/indra/newview/llviewermedia.cpp @@ -732,10 +732,21 @@ static bool proximity_comparitor(const LLViewerMediaImpl* i1, const LLViewerMedi } } +static LLFastTimer::DeclareTimer FTM_MEDIA_UPDATE("Update Media"); + ////////////////////////////////////////////////////////////////////////////////////////// // static void LLViewerMedia::updateMedia(void *dummy_arg) { + LLFastTimer t1(FTM_MEDIA_UPDATE); + + bool use_read_thread = gSavedSettings.getBOOL("PluginUseReadThread"); + if(LLPluginProcessParent::getUseReadThread() != use_read_thread) + { + // Enable/disable the plugin read thread + LLPluginProcessParent::setUseReadThread(use_read_thread); + } + sAnyMediaShowing = false; sUpdatedCookies = getCookieStore()->getChangedCookies(); if(!sUpdatedCookies.empty()) -- cgit v1.3 From b6ba0da7f5e8d830f776ec974287b2d451a4715f Mon Sep 17 00:00:00 2001 From: Leyla Farazha Date: Wed, 28 Apr 2010 17:48:05 -0700 Subject: Media sound attentuation first pass reviewed by Richard --- indra/newview/app_settings/settings.xml | 13 ++++++++++++- indra/newview/llviewermedia.cpp | 12 +++++++++++- 2 files changed, 23 insertions(+), 2 deletions(-) (limited to 'indra/newview/llviewermedia.cpp') diff --git a/indra/newview/app_settings/settings.xml b/indra/newview/app_settings/settings.xml index a6dbe00759..6f11a6d616 100644 --- a/indra/newview/app_settings/settings.xml +++ b/indra/newview/app_settings/settings.xml @@ -5838,7 +5838,18 @@ Value 1.0 - RecentItemsSortOrder + MediaRollOffFactor + + Comment + Multiplier to change rate of media attenuation + Persist + 1 + Type + F32 + Value + 10.0 + + RecentItemsSortOrder Comment Specifies sort key for recent inventory items (+0 = name, +1 = date, +2 = folders always by name, +4 = system folders to top) diff --git a/indra/newview/llviewermedia.cpp b/indra/newview/llviewermedia.cpp index 3c0345df90..fd2bb0fdf9 100644 --- a/indra/newview/llviewermedia.cpp +++ b/indra/newview/llviewermedia.cpp @@ -1914,7 +1914,15 @@ void LLViewerMediaImpl::updateVolume() { if(mMediaSource) { - mMediaSource->setVolume(mRequestedVolume * LLViewerMedia::getVolume()); + F32 attenuation_multiplier = 1.0; + + if (mProximityDistance > 0) + { + // the attenuation multiplier should never be more than one since that would increase volume + attenuation_multiplier = llmin(1.0, gSavedSettings.getF32("MediaRollOffFactor")/mProximityDistance); + } + + mMediaSource->setVolume(mRequestedVolume * LLViewerMedia::getVolume() * attenuation_multiplier); } } @@ -2427,6 +2435,8 @@ void LLViewerMediaImpl::update() } else { + updateVolume(); + // If we didn't just create the impl, it may need to get cookie updates. if(!sUpdatedCookies.empty()) { -- cgit v1.3 From 77b13dc2df679c46f648a4f99c8e4d8836983a48 Mon Sep 17 00:00:00 2001 From: Monroe Linden Date: Thu, 29 Apr 2010 17:33:37 -0700 Subject: Incorporate suggestions from Richard's review of the LLPlugin changes. Use LLMutexLock (stack-based locker/unlocker) for the straightforward cases instead of explicit lock()/unlock(). There are still a couple of cases (one overlapping lock lifetime and two loops that unlock the mutex to call another function inside the loop) where I'm leaving explicit lock/unlock calls. Rename LLPluginProcessParent::sPollThread to sReadThread, for consistency. Made the LLPluginProcessParent destructor hold mIncomingQueueMutex while removing the instance from the global list -- this should prevent a possible race condition in LLPluginProcessParent::poll(). Removed a redundant check when calling LLPluginProcessParent::setUseReadThread(). --- indra/llplugin/llpluginmessagepipe.cpp | 9 ++-- indra/llplugin/llpluginprocessparent.cpp | 78 +++++++++++++++++--------------- indra/llplugin/llpluginprocessparent.h | 2 +- indra/newview/llviewermedia.cpp | 8 +--- 4 files changed, 48 insertions(+), 49 deletions(-) (limited to 'indra/newview/llviewermedia.cpp') diff --git a/indra/llplugin/llpluginmessagepipe.cpp b/indra/llplugin/llpluginmessagepipe.cpp index a62534de95..89f8b44569 100644 --- a/indra/llplugin/llpluginmessagepipe.cpp +++ b/indra/llplugin/llpluginmessagepipe.cpp @@ -117,10 +117,9 @@ LLPluginMessagePipe::~LLPluginMessagePipe() bool LLPluginMessagePipe::addMessage(const std::string &message) { // queue the message for later output - mOutputMutex.lock(); + LLMutexLock lock(&mOutputMutex); mOutput += message; mOutput += MESSAGE_DELIMITER; // message separator - mOutputMutex.unlock(); return true; } @@ -173,7 +172,7 @@ bool LLPluginMessagePipe::pumpOutput() apr_status_t status; apr_size_t size; - mOutputMutex.lock(); + LLMutexLock lock(&mOutputMutex); if(!mOutput.empty()) { // write any outgoing messages @@ -225,7 +224,6 @@ bool LLPluginMessagePipe::pumpOutput() result = false; } } - mOutputMutex.unlock(); } return result; @@ -288,9 +286,8 @@ bool LLPluginMessagePipe::pumpInput(F64 timeout) if(size > 0) { - mInputMutex.lock(); + LLMutexLock lock(&mInputMutex); mInput.append(input_buf, size); - mInputMutex.unlock(); } if(status == APR_SUCCESS) diff --git a/indra/llplugin/llpluginprocessparent.cpp b/indra/llplugin/llpluginprocessparent.cpp index c61a903a2c..9d510e737d 100644 --- a/indra/llplugin/llpluginprocessparent.cpp +++ b/indra/llplugin/llpluginprocessparent.cpp @@ -50,7 +50,7 @@ apr_pollset_t *LLPluginProcessParent::sPollSet = NULL; bool LLPluginProcessParent::sPollsetNeedsRebuild = false; LLMutex *LLPluginProcessParent::sInstancesMutex; std::list LLPluginProcessParent::sInstances; -LLThread *LLPluginProcessParent::sPollThread = NULL; +LLThread *LLPluginProcessParent::sReadThread = NULL; class LLPluginProcessParentPollThread: public LLThread @@ -108,9 +108,10 @@ LLPluginProcessParent::LLPluginProcessParent(LLPluginProcessParentOwner *owner): mHeartbeat.stop(); // Don't add to the global list until fully constructed. - sInstancesMutex->lock(); - sInstances.push_back(this); - sInstancesMutex->unlock(); + { + LLMutexLock lock(sInstancesMutex); + sInstances.push_back(this); + } } LLPluginProcessParent::~LLPluginProcessParent() @@ -118,9 +119,14 @@ LLPluginProcessParent::~LLPluginProcessParent() LL_DEBUGS("Plugin") << "destructor" << LL_ENDL; // Remove from the global list before beginning destruction. - sInstancesMutex->lock(); - sInstances.remove(this); - sInstancesMutex->unlock(); + { + // Make sure to get the global mutex _first_ here, to avoid a possible deadlock against LLPluginProcessParent::poll() + LLMutexLock lock(sInstancesMutex); + { + LLMutexLock lock2(&mIncomingQueueMutex); + sInstances.remove(this); + } + } // Destroy any remaining shared memory regions sharedMemoryRegionsType::iterator iter; @@ -139,9 +145,11 @@ LLPluginProcessParent::~LLPluginProcessParent() void LLPluginProcessParent::killSockets(void) { - mIncomingQueueMutex.lock(); - killMessagePipe(); - mIncomingQueueMutex.unlock(); + { + LLMutexLock lock(&mIncomingQueueMutex); + killMessagePipe(); + } + mListenSocket.reset(); mSocket.reset(); } @@ -619,10 +627,10 @@ void LLPluginProcessParent::dirtyPollSet() { sPollsetNeedsRebuild = true; - if(sPollThread) + if(sReadThread) { - LL_DEBUGS("PluginPoll") << "unpausing polling thread " << LL_ENDL; - sPollThread->unpause(); + LL_DEBUGS("PluginPoll") << "unpausing read thread " << LL_ENDL; + sReadThread->unpause(); } } @@ -634,7 +642,7 @@ void LLPluginProcessParent::updatePollset() return; } - sInstancesMutex->lock(); + LLMutexLock lock(sInstancesMutex); if(sPollSet) { @@ -658,7 +666,7 @@ void LLPluginProcessParent::updatePollset() } } - if(sUseReadThread && sPollThread && !sPollThread->isQuitting()) + if(sUseReadThread && sReadThread && !sReadThread->isQuitting()) { if(!sPollSet && (count > 0)) { @@ -692,7 +700,6 @@ void LLPluginProcessParent::updatePollset() } } } - sInstancesMutex->unlock(); } void LLPluginProcessParent::setUseReadThread(bool use_read_thread) @@ -703,26 +710,26 @@ void LLPluginProcessParent::setUseReadThread(bool use_read_thread) if(sUseReadThread) { - if(!sPollThread) + if(!sReadThread) { // start up the read thread - LL_INFOS("PluginPoll") << "creating polling thread " << LL_ENDL; + LL_INFOS("PluginPoll") << "creating read thread " << LL_ENDL; // make sure the pollset gets rebuilt. sPollsetNeedsRebuild = true; - sPollThread = new LLPluginProcessParentPollThread; - sPollThread->start(); + sReadThread = new LLPluginProcessParentPollThread; + sReadThread->start(); } } else { - if(sPollThread) + if(sReadThread) { // shut down the read thread - LL_INFOS("PluginPoll") << "destroying polling thread " << LL_ENDL; - delete sPollThread; - sPollThread = NULL; + LL_INFOS("PluginPoll") << "destroying read thread " << LL_ENDL; + delete sReadThread; + sReadThread = NULL; } } @@ -756,21 +763,22 @@ void LLPluginProcessParent::poll(F64 timeout) if(self) { // Make sure this pointer is still in the instances list - sInstancesMutex->lock(); bool valid = false; - for(std::list::iterator iter = sInstances.begin(); iter != sInstances.end(); ++iter) { - if(*iter == self) + LLMutexLock lock(sInstancesMutex); + for(std::list::iterator iter = sInstances.begin(); iter != sInstances.end(); ++iter) { - // Lock the instance's mutex before unlocking the global mutex. - // This avoids a possible race condition where the instance gets deleted between this check and the servicePoll() call. - self->mIncomingQueueMutex.lock(); - valid = true; - break; + if(*iter == self) + { + // Lock the instance's mutex before unlocking the global mutex. + // This avoids a possible race condition where the instance gets deleted between this check and the servicePoll() call. + self->mIncomingQueueMutex.lock(); + valid = true; + break; + } } } - sInstancesMutex->unlock(); - + if(valid) { // The instance is still valid. @@ -872,9 +880,7 @@ void LLPluginProcessParent::receiveMessageEarly(const LLPluginMessage &message) if(!handled) { // any message that wasn't handled early needs to be queued. -// mIncomingQueueMutex.lock(); mIncomingQueue.push(message); -// mIncomingQueueMutex.unlock(); } } diff --git a/indra/llplugin/llpluginprocessparent.h b/indra/llplugin/llpluginprocessparent.h index 1ad0fbf059..4dff835b6a 100644 --- a/indra/llplugin/llpluginprocessparent.h +++ b/indra/llplugin/llpluginprocessparent.h @@ -188,7 +188,7 @@ private: static void dirtyPollSet(); static void updatePollset(); void servicePoll(); - static LLThread *sPollThread; + static LLThread *sReadThread; LLMutex mIncomingQueueMutex; std::queue mIncomingQueue; diff --git a/indra/newview/llviewermedia.cpp b/indra/newview/llviewermedia.cpp index 8e210554fb..a4d8dddfe4 100644 --- a/indra/newview/llviewermedia.cpp +++ b/indra/newview/llviewermedia.cpp @@ -740,12 +740,8 @@ void LLViewerMedia::updateMedia(void *dummy_arg) { LLFastTimer t1(FTM_MEDIA_UPDATE); - bool use_read_thread = gSavedSettings.getBOOL("PluginUseReadThread"); - if(LLPluginProcessParent::getUseReadThread() != use_read_thread) - { - // Enable/disable the plugin read thread - LLPluginProcessParent::setUseReadThread(use_read_thread); - } + // Enable/disable the plugin read thread + LLPluginProcessParent::setUseReadThread(gSavedSettings.getBOOL("PluginUseReadThread")); sAnyMediaShowing = false; sUpdatedCookies = getCookieStore()->getChangedCookies(); -- cgit v1.3 From 34e29fe71277609e6c18c967b8fd7a45116d4d82 Mon Sep 17 00:00:00 2001 From: Xiaohong Bao Date: Thu, 29 Apr 2010 12:24:46 -0600 Subject: add debug code for EXT-7011: crash at LLPluginClassMedia::idle [secondlife-bin llpluginclassmedia.cpp:158] --- indra/llplugin/llpluginclassmedia.cpp | 4 ++++ indra/llplugin/llpluginclassmedia.h | 8 ++++++++ indra/newview/llviewermedia.cpp | 3 ++- 3 files changed, 14 insertions(+), 1 deletion(-) (limited to 'indra/newview/llviewermedia.cpp') diff --git a/indra/llplugin/llpluginclassmedia.cpp b/indra/llplugin/llpluginclassmedia.cpp index 0c9b325b68..41ace62964 100644 --- a/indra/llplugin/llpluginclassmedia.cpp +++ b/indra/llplugin/llpluginclassmedia.cpp @@ -57,11 +57,15 @@ LLPluginClassMedia::LLPluginClassMedia(LLPluginClassMediaOwner *owner) mOwner = owner; mPlugin = NULL; reset(); + + //debug use + mDeleteOK = true ; } LLPluginClassMedia::~LLPluginClassMedia() { + llassert_always(mDeleteOK) ; reset(); } diff --git a/indra/llplugin/llpluginclassmedia.h b/indra/llplugin/llpluginclassmedia.h index 8c7b00f45b..66853c9940 100644 --- a/indra/llplugin/llpluginclassmedia.h +++ b/indra/llplugin/llpluginclassmedia.h @@ -373,6 +373,14 @@ protected: F64 mCurrentRate; F64 mLoadedDuration; +//-------------------------------------- + //debug use only + // +private: + bool mDeleteOK ; +public: + void setDeleteOK(bool flag) { mDeleteOK = flag ;} +//-------------------------------------- }; #endif // LL_LLPLUGINCLASSMEDIA_H diff --git a/indra/newview/llviewermedia.cpp b/indra/newview/llviewermedia.cpp index fd2bb0fdf9..480d4e9e69 100644 --- a/indra/newview/llviewermedia.cpp +++ b/indra/newview/llviewermedia.cpp @@ -1578,6 +1578,7 @@ void LLViewerMediaImpl::destroyMediaSource() if(mMediaSource) { + mMediaSource->setDeleteOK(true) ; delete mMediaSource; mMediaSource = NULL; } @@ -1729,7 +1730,7 @@ bool LLViewerMediaImpl::initializePlugin(const std::string& media_type) } mMediaSource = media_source; - + mMediaSource->setDeleteOK(false) ; updateVolume(); return true; -- cgit v1.3 From e2e31b1b6b049adbdf16d6078827239c135be49f Mon Sep 17 00:00:00 2001 From: Leyla Farazha Date: Fri, 30 Apr 2010 13:26:25 -0700 Subject: Added min and max for media sound attenuation reviewed by Richard --- indra/newview/llviewermedia.cpp | 28 +++++++++++++++++++++++----- indra/newview/llviewermedia.h | 1 + 2 files changed, 24 insertions(+), 5 deletions(-) (limited to 'indra/newview/llviewermedia.cpp') diff --git a/indra/newview/llviewermedia.cpp b/indra/newview/llviewermedia.cpp index fd2bb0fdf9..c8f3374f38 100644 --- a/indra/newview/llviewermedia.cpp +++ b/indra/newview/llviewermedia.cpp @@ -33,6 +33,7 @@ #include "llviewerprecompiledheaders.h" #include "llagent.h" +#include "llagentcamera.h" #include "llviewermedia.h" #include "llviewermediafocus.h" #include "llmimetypes.h" @@ -1914,15 +1915,29 @@ void LLViewerMediaImpl::updateVolume() { if(mMediaSource) { - F32 attenuation_multiplier = 1.0; + // always scale the volume by the global media volume + F32 volume = mRequestedVolume * LLViewerMedia::getVolume(); - if (mProximityDistance > 0) + // attenuate if this is not parcel media + if (!mIsParcelMedia) { - // the attenuation multiplier should never be more than one since that would increase volume - attenuation_multiplier = llmin(1.0, gSavedSettings.getF32("MediaRollOffFactor")/mProximityDistance); + if (mProximityCamera > gSavedSettings.getF32("MediaRollOffMaxDistance")) + { + volume = 0; + } + else + { + // attenuated volume = volume * roll_off rate / distance^2 + // adjust distance by saved setting so we can tune the distance at which attenuation begins + // the actual start distance is sqrt(MediaRollOffMaxDistance)+MediaRollOffStartAdjustment + F64 adjusted_distance = mProximityCamera - gSavedSettings.getF32("MediaRollOffStart"); + + // the attenuation multiplier should never be more than one since that would increase volume + volume = volume * llmin(1.0, gSavedSettings.getF32("MediaRollOffRate")/adjusted_distance*adjusted_distance); + } } - mMediaSource->setVolume(mRequestedVolume * LLViewerMedia::getVolume() * attenuation_multiplier); + mMediaSource->setVolume(volume); } } @@ -3009,6 +3024,9 @@ void LLViewerMediaImpl::calculateInterest() LLVector3d agent_global = gAgent.getPositionGlobal() ; LLVector3d global_delta = agent_global - obj_global ; mProximityDistance = global_delta.magVecSquared(); // use distance-squared because it's cheaper and sorts the same. + + LLVector3d camera_delta = gAgentCamera.getCameraPositionGlobal() - obj_global; + mProximityCamera = camera_delta.magVec(); } if(mNeedsMuteCheck) diff --git a/indra/newview/llviewermedia.h b/indra/newview/llviewermedia.h index e829d7a5b4..8626f4469e 100644 --- a/indra/newview/llviewermedia.h +++ b/indra/newview/llviewermedia.h @@ -431,6 +431,7 @@ private: bool mIsParcelMedia; S32 mProximity; F64 mProximityDistance; + F64 mProximityCamera; LLMimeDiscoveryResponder *mMimeTypeProbe; bool mMediaAutoPlay; std::string mMediaEntryURL; -- cgit v1.3 From 07c207346f63d464e45efe0be3aae64138982279 Mon Sep 17 00:00:00 2001 From: Leyla Farazha Date: Fri, 30 Apr 2010 15:59:59 -0700 Subject: updated media attenuation function reviewed by Richard --- indra/newview/app_settings/settings.xml | 26 ++++++++++++++++++++++++-- indra/newview/llviewermedia.cpp | 19 +++++++++---------- 2 files changed, 33 insertions(+), 12 deletions(-) (limited to 'indra/newview/llviewermedia.cpp') diff --git a/indra/newview/app_settings/settings.xml b/indra/newview/app_settings/settings.xml index 6f11a6d616..266ed2b5fd 100644 --- a/indra/newview/app_settings/settings.xml +++ b/indra/newview/app_settings/settings.xml @@ -5838,7 +5838,7 @@ Value 1.0 - MediaRollOffFactor + MediaRollOffRate Comment Multiplier to change rate of media attenuation @@ -5847,7 +5847,29 @@ Type F32 Value - 10.0 + 0.2 + + MediaRollOffMin + + Comment + Adjusts the distance at which media attentuation starts + Persist + 1 + Type + F32 + Value + 7.0 + + MediaRollOffMax + + Comment + Distance at which media volume is set to 0 + Persist + 1 + Type + F32 + Value + 30.0 RecentItemsSortOrder diff --git a/indra/newview/llviewermedia.cpp b/indra/newview/llviewermedia.cpp index c8f3374f38..076a9609e3 100644 --- a/indra/newview/llviewermedia.cpp +++ b/indra/newview/llviewermedia.cpp @@ -1918,22 +1918,21 @@ void LLViewerMediaImpl::updateVolume() // always scale the volume by the global media volume F32 volume = mRequestedVolume * LLViewerMedia::getVolume(); - // attenuate if this is not parcel media - if (!mIsParcelMedia) + if (mProximityCamera > 0) { - if (mProximityCamera > gSavedSettings.getF32("MediaRollOffMaxDistance")) + if (mProximityCamera > gSavedSettings.getF32("MediaRollOffMax")) { volume = 0; } - else + else if (mProximityCamera > gSavedSettings.getF32("MediaRollOffMin")) { - // attenuated volume = volume * roll_off rate / distance^2 - // adjust distance by saved setting so we can tune the distance at which attenuation begins - // the actual start distance is sqrt(MediaRollOffMaxDistance)+MediaRollOffStartAdjustment - F64 adjusted_distance = mProximityCamera - gSavedSettings.getF32("MediaRollOffStart"); - + // attenuated_volume = v / ( 1 + (roll_off_rate * (d - min))^2 + // the +1 is there so that for distance 0 the volume stays the same + F64 adjusted_distance = mProximityCamera - gSavedSettings.getF32("MediaRollOffMin"); + F64 attenuation = gSavedSettings.getF32("MediaRollOffRate") * adjusted_distance; + attenuation = attenuation * attenuation; // the attenuation multiplier should never be more than one since that would increase volume - volume = volume * llmin(1.0, gSavedSettings.getF32("MediaRollOffRate")/adjusted_distance*adjusted_distance); + volume = volume * llmin(1.0, 1 /(attenuation + 1)); } } -- cgit v1.3