Bug 993753 - Don't set media decoders idle while seeking. r=kinetik
authorChris Pearce <cpearce@mozilla.com>
Tue, 15 Apr 2014 15:01:34 +1200
changeset 190469 6cc06a35f253203d7986d07f3188415eef4aa5bc
parent 190468 1515ebdea6768197428fd998cbbaf458b0187209
child 190470 81ce5041d5d944ca9c0464cd8703d2e80df64476
push idunknown
push userunknown
push dateunknown
reviewerskinetik
bugs993753
milestone31.0a1
Bug 993753 - Don't set media decoders idle while seeking. r=kinetik
content/media/MediaDecoderStateMachine.cpp
content/media/omx/MediaOmxReader.cpp
content/media/omx/MediaOmxReader.h
--- a/content/media/MediaDecoderStateMachine.cpp
+++ b/content/media/MediaDecoderStateMachine.cpp
@@ -1551,16 +1551,17 @@ MediaDecoderStateMachine::DispatchDecode
   const bool needToDecodeAudio = NeedToDecodeAudio();
   const bool needToDecodeVideo = NeedToDecodeVideo();
 
   // If we're in completed state, we should not need to decode anything else.
   MOZ_ASSERT(mState != DECODER_STATE_COMPLETED ||
              (!needToDecodeAudio && !needToDecodeVideo));
 
   bool needIdle = !mDecoder->IsLogicallyPlaying() &&
+                  mState != DECODER_STATE_SEEKING &&
                   !needToDecodeAudio &&
                   !needToDecodeVideo &&
                   !IsPlaying();
 
   if (needToDecodeAudio) {
     EnsureAudioDecodeTaskQueued();
   }
   if (needToDecodeVideo) {
--- a/content/media/omx/MediaOmxReader.cpp
+++ b/content/media/omx/MediaOmxReader.cpp
@@ -35,23 +35,26 @@ namespace mozilla {
 
 #ifdef PR_LOGGING
 extern PRLogModuleInfo* gMediaDecoderLog;
 #define DECODER_LOG(type, msg) PR_LOG(gMediaDecoderLog, type, msg)
 #else
 #define DECODER_LOG(type, msg)
 #endif
 
-MediaOmxReader::MediaOmxReader(AbstractMediaDecoder *aDecoder) :
-  MediaDecoderReader(aDecoder),
-  mHasVideo(false),
-  mHasAudio(false),
-  mVideoSeekTimeUs(-1),
-  mAudioSeekTimeUs(-1),
-  mSkipCount(0)
+MediaOmxReader::MediaOmxReader(AbstractMediaDecoder *aDecoder)
+  : MediaDecoderReader(aDecoder)
+  , mHasVideo(false)
+  , mHasAudio(false)
+  , mVideoSeekTimeUs(-1)
+  , mAudioSeekTimeUs(-1)
+  , mSkipCount(0)
+#ifdef DEBUG
+  , mIsActive(true)
+#endif
 {
 #ifdef PR_LOGGING
   if (!gMediaDecoderLog) {
     gMediaDecoderLog = PR_NewLogModule("MediaDecoder");
   }
 #endif
 
   mAudioChannel = dom::AudioChannelService::GetDefaultAudioChannel();
@@ -127,16 +130,17 @@ nsresult MediaOmxReader::InitOmxDecoder(
   }
   return NS_OK;
 }
 
 nsresult MediaOmxReader::ReadMetadata(MediaInfo* aInfo,
                                       MetadataTags** aTags)
 {
   NS_ASSERTION(mDecoder->OnDecodeThread(), "Should be on decode thread.");
+  MOZ_ASSERT(mIsActive);
 
   *aTags = nullptr;
 
   // Initialize the internal OMX Decoder.
   nsresult rv = InitOmxDecoder();
   if (NS_FAILED(rv)) {
     return rv;
   }
@@ -202,16 +206,18 @@ nsresult MediaOmxReader::ReadMetadata(Me
  *aInfo = mInfo;
 
   return NS_OK;
 }
 
 bool MediaOmxReader::DecodeVideoFrame(bool &aKeyframeSkip,
                                       int64_t aTimeThreshold)
 {
+  MOZ_ASSERT(mIsActive);
+
   // Record number of frames decoded and parsed. Automatically update the
   // stats counters using the AutoNotifyDecoded stack-based class.
   uint32_t parsed = 0, decoded = 0;
   AbstractMediaDecoder::AutoNotifyDecoded autoNotify(mDecoder, parsed, decoded);
 
   bool doSeek = mVideoSeekTimeUs != -1;
   if (doSeek) {
     aTimeThreshold = mVideoSeekTimeUs;
@@ -330,16 +336,17 @@ void MediaOmxReader::NotifyDataArrived(c
   if (omxDecoder) {
     omxDecoder->NotifyDataArrived(aBuffer, aLength, aOffset);
   }
 }
 
 bool MediaOmxReader::DecodeAudioData()
 {
   NS_ASSERTION(mDecoder->OnDecodeThread(), "Should be on decode thread.");
+  MOZ_ASSERT(mIsActive);
 
   // This is the approximate byte position in the stream.
   int64_t pos = mDecoder->GetResource()->Tell();
 
   // Read next frame
   MPAPI::AudioFrame source;
   if (!mOmxDecoder->ReadAudio(&source, mAudioSeekTimeUs)) {
     return false;
@@ -363,16 +370,17 @@ bool MediaOmxReader::DecodeAudioData()
                               OmxCopy(static_cast<uint8_t *>(source.mData),
                                       source.mSize,
                                       source.mAudioChannels));
 }
 
 nsresult MediaOmxReader::Seek(int64_t aTarget, int64_t aStartTime, int64_t aEndTime, int64_t aCurrentTime)
 {
   NS_ASSERTION(mDecoder->OnDecodeThread(), "Should be on decode thread.");
+  MOZ_ASSERT(mIsActive);
 
   ResetDecode();
   VideoFrameContainer* container = mDecoder->GetVideoFrameContainer();
   if (container && container->GetImageContainer()) {
     container->GetImageContainer()->ClearAllImagesExceptFront();
   }
 
   if (mHasAudio && mHasVideo) {
@@ -397,23 +405,29 @@ nsresult MediaOmxReader::Seek(int64_t aT
 static uint64_t BytesToTime(int64_t offset, uint64_t length, uint64_t durationUs) {
   double perc = double(offset) / double(length);
   if (perc > 1.0)
     perc = 1.0;
   return uint64_t(double(durationUs) * perc);
 }
 
 void MediaOmxReader::SetIdle() {
+#ifdef DEBUG
+  mIsActive = false;
+#endif
   if (!mOmxDecoder.get()) {
     return;
   }
   mOmxDecoder->Pause();
 }
 
 void MediaOmxReader::SetActive() {
+#ifdef DEBUG
+  mIsActive = true;
+#endif
   if (!mOmxDecoder.get()) {
     return;
   }
   DebugOnly<nsresult> result = mOmxDecoder->Play();
   NS_ASSERTION(result == NS_OK, "OmxDecoder should be in play state to continue decoding");
 }
 
 #ifdef MOZ_AUDIO_OFFLOAD
--- a/content/media/omx/MediaOmxReader.h
+++ b/content/media/omx/MediaOmxReader.h
@@ -95,13 +95,18 @@ public:
 
 #ifdef MOZ_AUDIO_OFFLOAD
   // Check whether it is possible to offload current audio track. This access
   // canOffloadStream() from libStageFright Utils.cpp, which is not there in
   // ANDROID_VERSION < 19
   void CheckAudioOffload();
 #endif
 
+private:
+  // This flag is true when SetActive() has been called without a matching
+  // SetIdle(). This is used to sanity check the SetIdle/SetActive calls, to
+  // ensure SetActive has been called before a decode call.
+  DebugOnly<bool> mIsActive;
 };
 
 } // namespace mozilla
 
 #endif