Bug 1289683 - Prevent notifications from AudioOffloadPlayer after shutdown. r=sotaro
authorJW Wang <jwwang@mozilla.com>
Wed, 27 Jul 2016 15:04:07 +0800
changeset 347103 8e5c846a97744ed444c45db5c04a59f46fe8469c
parent 347102 97446e8fdc0c5941a1021ee05993546429d3759d
child 347104 ca06c0afd540b59fc90d81903c6c018bea1b4d9d
push id6389
push userraliiev@mozilla.com
push dateMon, 19 Sep 2016 13:38:22 +0000
treeherdermozilla-beta@01d67bfe6c81 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssotaro
bugs1289683
milestone50.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1289683 - Prevent notifications from AudioOffloadPlayer after shutdown. r=sotaro MozReview-Commit-ID: J4t4aYZyfVz
dom/media/omx/AudioOffloadPlayer.cpp
dom/media/omx/AudioOffloadPlayer.h
dom/media/omx/MediaOmxCommonDecoder.cpp
dom/media/omx/MediaOmxCommonDecoder.h
--- a/dom/media/omx/AudioOffloadPlayer.cpp
+++ b/dom/media/omx/AudioOffloadPlayer.cpp
@@ -15,16 +15,17 @@
  * 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.
  */
 
 #include "AudioOffloadPlayer.h"
 #include "nsComponentManagerUtils.h"
 #include "nsITimer.h"
+#include "MediaOmxCommonDecoder.h"
 #include "mozilla/dom/HTMLMediaElement.h"
 #include "VideoUtils.h"
 #include "mozilla/dom/power/PowerManagerService.h"
 #include "mozilla/dom/WakeLock.h"
 
 #include <binder/IPCThreadState.h>
 #include <stagefright/foundation/ADebug.h>
 #include <stagefright/foundation/ALooper.h>
@@ -53,41 +54,58 @@ static const uint64_t OFFLOAD_PAUSE_MAX_
 AudioOffloadPlayer::AudioOffloadPlayer(MediaOmxCommonDecoder* aObserver) :
   mStarted(false),
   mPlaying(false),
   mReachedEOS(false),
   mIsElementVisible(true),
   mSampleRate(0),
   mStartPosUs(0),
   mPositionTimeMediaUs(-1),
-  mInputBuffer(nullptr),
-  mObserver(aObserver)
+  mInputBuffer(nullptr)
 {
   MOZ_ASSERT(NS_IsMainThread());
 
   CHECK(aObserver);
 #if ANDROID_VERSION >= 21
   mSessionId = AudioSystem::newAudioUniqueId();
   AudioSystem::acquireAudioSessionId(mSessionId, -1);
 #else
   mSessionId = AudioSystem::newAudioSessionId();
   AudioSystem::acquireAudioSessionId(mSessionId);
 #endif
   mAudioSink = new AudioOutput(mSessionId,
       IPCThreadState::self()->getCallingUid());
+
+  nsCOMPtr<nsIThread> thread;
+  MOZ_ALWAYS_SUCCEEDS(NS_GetMainThread(getter_AddRefs(thread)));
+  mPositionChanged = mOnPositionChanged.Connect(
+    thread, aObserver, &MediaOmxCommonDecoder::NotifyOffloadPlayerPositionChanged);
+  mPlaybackEnded = mOnPlaybackEnded.Connect(
+    thread, aObserver, &MediaDecoder::PlaybackEnded);
+  mPlayerTearDown = mOnPlayerTearDown.Connect(
+    thread, aObserver, &MediaOmxCommonDecoder::AudioOffloadTearDown);
+  mSeekingStarted = mOnSeekingStarted.Connect(
+    thread, aObserver, &MediaDecoder::SeekingStarted);
 }
 
 AudioOffloadPlayer::~AudioOffloadPlayer()
 {
   Reset();
 #if ANDROID_VERSION >= 21
   AudioSystem::releaseAudioSessionId(mSessionId, -1);
 #else
   AudioSystem::releaseAudioSessionId(mSessionId);
 #endif
+
+  // Disconnect the listeners to prevent notifications from reaching
+  // the MediaOmxCommonDecoder object after shutdown.
+  mPositionChanged.Disconnect();
+  mPlaybackEnded.Disconnect();
+  mPlayerTearDown.Disconnect();
+  mSeekingStarted.Disconnect();
 }
 
 void AudioOffloadPlayer::SetSource(const sp<MediaSource> &aSource)
 {
   MOZ_ASSERT(NS_IsMainThread());
   CHECK(!mSource.get());
 
   mSource = aSource;
@@ -348,22 +366,17 @@ status_t AudioOffloadPlayer::DoSeek()
   AUDIO_OFFLOAD_LOG(LogLevel::Debug,
                     ("DoSeek ( %lld )", mSeekTarget.GetTime().ToMicroseconds()));
 
   mReachedEOS = false;
   mPositionTimeMediaUs = -1;
   mStartPosUs = mSeekTarget.GetTime().ToMicroseconds();
 
   if (!mSeekPromise.IsEmpty()) {
-    nsCOMPtr<nsIRunnable> nsEvent =
-      NewRunnableMethod<MediaDecoderEventVisibility>(
-        mObserver,
-        &MediaDecoder::SeekingStarted,
-        mSeekTarget.mEventVisibility);
-    NS_DispatchToCurrentThread(nsEvent);
+    mOnSeekingStarted.Notify(mSeekTarget.mEventVisibility);
   }
 
   if (mPlaying) {
     mAudioSink->Pause();
     mAudioSink->Flush();
     mAudioSink->Start();
 
   } else {
@@ -420,39 +433,36 @@ int64_t AudioOffloadPlayer::GetOutputPla
 void AudioOffloadPlayer::NotifyAudioEOS()
 {
   android::Mutex::Autolock autoLock(mLock);
   // We do not reset mSeekTarget here.
   if (!mSeekPromise.IsEmpty()) {
     MediaDecoder::SeekResolveValue val(mReachedEOS, mSeekTarget.mEventVisibility);
     mSeekPromise.Resolve(val, __func__);
   }
-  NS_DispatchToMainThread(NewRunnableMethod(mObserver,
-                                            &MediaDecoder::PlaybackEnded));
+  mOnPlaybackEnded.Notify();
 }
 
 void AudioOffloadPlayer::NotifyPositionChanged()
 {
-  NS_DispatchToMainThread(NewRunnableMethod(mObserver,
-                                            &MediaOmxCommonDecoder::NotifyOffloadPlayerPositionChanged));
+  mOnPositionChanged.Notify();
 }
 
 void AudioOffloadPlayer::NotifyAudioTearDown()
 {
   // Fallback to state machine.
   // state machine's seeks will be done with
   // MediaDecoderEventVisibility::Suppressed.
   android::Mutex::Autolock autoLock(mLock);
   // We do not reset mSeekTarget here.
   if (!mSeekPromise.IsEmpty()) {
     MediaDecoder::SeekResolveValue val(mReachedEOS, mSeekTarget.mEventVisibility);
     mSeekPromise.Resolve(val, __func__);
   }
-  NS_DispatchToMainThread(NewRunnableMethod(mObserver,
-                                            &MediaOmxCommonDecoder::AudioOffloadTearDown));
+  mOnPlayerTearDown.Notify();
 }
 
 // static
 size_t AudioOffloadPlayer::AudioSinkCallback(GonkAudioSink* aAudioSink,
                                              void* aBuffer,
                                              size_t aSize,
                                              void* aCookie,
                                              GonkAudioSink::cb_event_t aEvent)
--- a/dom/media/omx/AudioOffloadPlayer.h
+++ b/dom/media/omx/AudioOffloadPlayer.h
@@ -24,17 +24,17 @@
 #include <stagefright/MediaSource.h>
 #include <stagefright/TimeSource.h>
 #include <utils/threads.h>
 #include <utils/RefBase.h>
 
 #include "AudioOutput.h"
 #include "AudioOffloadPlayerBase.h"
 #include "MediaDecoderOwner.h"
-#include "MediaOmxCommonDecoder.h"
+#include "MediaEventSource.h"
 
 namespace mozilla {
 
 namespace dom {
 class WakeLock;
 }
 
 /**
@@ -53,32 +53,34 @@ class WakeLock;
  * MediaOmxCommonDecoder to GonkAudioSink as well as provide GonkAudioSink status
  * (position changed, playback ended, seek complete, audio tear down) back to
  * MediaOmxCommonDecoder
  *
  * It acts as a bridge between MediaOmxCommonDecoder and GonkAudioSink during
  * offload playback
  */
 
+class MediaOmxCommonDecoder;
+
 class AudioOffloadPlayer : public AudioOffloadPlayerBase
 {
   typedef android::Mutex Mutex;
   typedef android::MetaData MetaData;
   typedef android::status_t status_t;
   typedef android::AudioTrack AudioTrack;
   typedef android::MediaBuffer MediaBuffer;
   typedef android::MediaSource MediaSource;
 
 public:
   enum {
     REACHED_EOS,
     SEEK_COMPLETE
   };
 
-  AudioOffloadPlayer(MediaOmxCommonDecoder* aDecoder = nullptr);
+  AudioOffloadPlayer(MediaOmxCommonDecoder* aDecoder);
 
   ~AudioOffloadPlayer();
 
   // Caller retains ownership of "aSource".
   void SetSource(const android::sp<MediaSource> &aSource) override;
 
   // Start the source if it's not already started and open the GonkAudioSink to
   // create an offloaded audio track
@@ -170,33 +172,39 @@ private:
   // Audio sink wrapper to access offloaded audio tracks
   // Used in main thread and offload callback thread
   // Race conditions are protected in underlying Android::AudioTrack class
   android::sp<GonkAudioSink> mAudioSink;
 
   // Buffer used to get date from audio source. Used in offload callback thread
   MediaBuffer* mInputBuffer;
 
-  // MediaOmxCommonDecoder object used mainly to notify the audio sink status
-  MediaOmxCommonDecoder* mObserver;
-
   TimeStamp mLastFireUpdateTime;
 
   // Timer to trigger position changed events
   nsCOMPtr<nsITimer> mTimeUpdateTimer;
 
   // Timer to reset GonkAudioSink when audio is paused for OFFLOAD_PAUSE_MAX_USECS.
   // It is triggered in Pause() and canceled when there is a Play() within
   // OFFLOAD_PAUSE_MAX_USECS. Used only from main thread so no lock is needed.
   nsCOMPtr<nsITimer> mResetTimer;
 
   // To avoid device suspend when mResetTimer is going to be triggered.
   // Used only from main thread so no lock is needed.
   RefPtr<mozilla::dom::WakeLock> mWakeLock;
 
+  MediaEventProducer<void> mOnPositionChanged;
+  MediaEventProducer<void> mOnPlaybackEnded;
+  MediaEventProducer<void> mOnPlayerTearDown;
+  MediaEventProducer<MediaDecoderEventVisibility> mOnSeekingStarted;
+  MediaEventListener mPositionChanged;
+  MediaEventListener mPlaybackEnded;
+  MediaEventListener mPlayerTearDown;
+  MediaEventListener mSeekingStarted;
+
   // Provide the playback position in microseconds from total number of
   // frames played by audio track
   int64_t GetOutputPlayPositionUs_l() const;
 
   // Fill the buffer given by audio sink with data from compressed audio
   // source. Also handles the seek by seeking audio source and stop the sink in
   // case of error
   size_t FillBuffer(void *aData, size_t aSize);
--- a/dom/media/omx/MediaOmxCommonDecoder.cpp
+++ b/dom/media/omx/MediaOmxCommonDecoder.cpp
@@ -160,16 +160,17 @@ MediaOmxCommonDecoder::ResumeStateMachin
   GetStateMachine()->DispatchSetDormant(false);
   UpdateLogicalPosition();
 }
 
 void
 MediaOmxCommonDecoder::AudioOffloadTearDown()
 {
   MOZ_ASSERT(NS_IsMainThread());
+  MOZ_ASSERT(!IsShutdown());
   DECODER_LOG(LogLevel::Debug, ("%s", __PRETTY_FUNCTION__));
 
   // mAudioOffloadPlayer can be null here if ResumeStateMachine was called
   // just before because of some other error.
   if (mAudioOffloadPlayer) {
     ResumeStateMachine();
   }
 }
@@ -281,9 +282,16 @@ MediaOmxCommonDecoder::CreateStateMachin
 {
   mReader = CreateReader();
   if (mReader != nullptr) {
     mReader->SetAudioChannel(GetAudioChannel());
   }
   return CreateStateMachineFromReader(mReader);
 }
 
+void
+MediaOmxCommonDecoder::Shutdown()
+{
+  mAudioOffloadPlayer = nullptr;
+  MediaDecoder::Shutdown();
+}
+
 } // namespace mozilla
--- a/dom/media/omx/MediaOmxCommonDecoder.h
+++ b/dom/media/omx/MediaOmxCommonDecoder.h
@@ -41,16 +41,18 @@ public:
 
   MediaDecoderStateMachine* CreateStateMachine() override;
 
   virtual MediaOmxCommonReader* CreateReader() = 0;
   virtual MediaDecoderStateMachine* CreateStateMachineFromReader(MediaOmxCommonReader* aReader) = 0;
 
   void NotifyOffloadPlayerPositionChanged() { UpdateLogicalPosition(); }
 
+  void Shutdown() override;
+
 protected:
   virtual ~MediaOmxCommonDecoder();
   void PauseStateMachine();
   void ResumeStateMachine();
   bool CheckDecoderCanOffloadAudio();
   void DisableStateMachineAudioOffloading();
 
   MediaOmxCommonReader* mReader;