Bug 1289683 - Prevent notifications from AudioOffloadPlayer after shutdown. r?sotaro draft
authorJW Wang <jwwang@mozilla.com>
Wed, 27 Jul 2016 15:04:07 +0800
changeset 393600 3e2f00ca7b730328145a291e45d8959df19302d9
parent 393599 4149a88923aebc94d4f7cd1fa21a3cbd25e6706b
child 393617 fb12fe919818dcd8c92efe60892e21e644afcb3e
push id24361
push userjwwang@mozilla.com
push dateThu, 28 Jul 2016 03:16:20 +0000
reviewerssotaro
bugs1289683
milestone50.0a1
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;