Bug 1039901 - Part 1. MP3FrameParser sometimes gives wrong duration on B2G. Move the ProcessCacheData function from OmxDecoder to MediaOmxReader. r=edwin.

content/media/MP3FrameParser.h

@@ -132,7 +132,11 @@ public:
 
   // Returns true if the parser needs more data for duration estimation.
   bool NeedsData();
-
+  // Assign the total lenght of this mp3 stream
+  void SetLength(int64_t aLength) {
+    MutexAutoLock mon(mLock);
+    mLength = aLength;
+  }
 private:
 
   // Parses aBuffer, starting at offset 0. Returns the number of bytes

content/media/omx/MediaOmxReader.cpp

@@ -35,13 +35,106 @@ extern PRLogModuleInfo* gMediaDecoderLog;
 #define DECODER_LOG(type, msg)
 #endif
 
+class OmxReaderProcessCachedDataTask : public Task
+{
+public:
+  OmxReaderProcessCachedDataTask(MediaOmxReader* aOmxReader, int64_t aOffset)
+  : mOmxReader(aOmxReader),
+    mOffset(aOffset)
+  { }
+
+  void Run()
+  {
+    MOZ_ASSERT(!NS_IsMainThread());
+    MOZ_ASSERT(mOmxReader.get());
+    mOmxReader->ProcessCachedData(mOffset, false);
+  }
+
+private:
+  nsRefPtr<MediaOmxReader> mOmxReader;
+  int64_t                  mOffset;
+};
+
+// When loading an MP3 stream from a file, we need to parse the file's
+// content to find its duration. Reading files of 100 MiB or more can
+// delay the player app noticably, so the file is read and decoded in
+// smaller chunks.
+//
+// We first read on the decode thread, but parsing must be done on the
+// main thread. After we read the file's initial MiBs in the decode
+// thread, an instance of this class is scheduled to the main thread for
+// parsing the MP3 stream. The decode thread waits until it has finished.
+//
+// If there is more data available from the file, the runnable dispatches
+// a task to the IO thread for retrieving the next chunk of data, and
+// the IO task dispatches a runnable to the main thread for parsing the
+// data. This goes on until all of the MP3 file has been parsed.
+
+class OmxReaderNotifyDataArrivedRunnable : public nsRunnable
+{
+public:
+  OmxReaderNotifyDataArrivedRunnable(MediaOmxReader* aOmxReader,
+                                     const char* aBuffer, uint64_t aLength,
+                                     int64_t aOffset, uint64_t aFullLength)
+  : mOmxReader(aOmxReader),
+    mBuffer(aBuffer),
+    mLength(aLength),
+    mOffset(aOffset),
+    mFullLength(aFullLength)
+  {
+    MOZ_ASSERT(mOmxReader.get());
+    MOZ_ASSERT(mBuffer.get() || !mLength);
+  }
+
+  NS_IMETHOD Run()
+  {
+    NS_ASSERTION(NS_IsMainThread(), "Should be on main thread.");
+
+    NotifyDataArrived();
+
+    return NS_OK;
+  }
+
+private:
+  void NotifyDataArrived()
+  {
+    const char* buffer = mBuffer.get();
+
+    while (mLength) {
+      uint32_t length = std::min<uint64_t>(mLength, UINT32_MAX);
+      mOmxReader->NotifyDataArrived(buffer, length,
+                                    mOffset);
+      buffer  += length;
+      mLength -= length;
+      mOffset += length;
+    }
+
+    if (mOffset < mFullLength) {
+      // We cannot read data in the main thread because it
+      // might block for too long. Instead we post an IO task
+      // to the IO thread if there is more data available.
+      XRE_GetIOMessageLoop()->PostTask(FROM_HERE,
+          new OmxReaderProcessCachedDataTask(mOmxReader.get(), mOffset));
+    }
+  }
+
+  nsRefPtr<MediaOmxReader> mOmxReader;
+  nsAutoArrayPtr<const char>       mBuffer;
+  uint64_t                         mLength;
+  int64_t                          mOffset;
+  uint64_t                         mFullLength;
+};
+
 MediaOmxReader::MediaOmxReader(AbstractMediaDecoder *aDecoder)
   : MediaOmxCommonReader(aDecoder)
+  , mMP3FrameParser(-1)
   , mHasVideo(false)
   , mHasAudio(false)
   , mVideoSeekTimeUs(-1)
   , mAudioSeekTimeUs(-1)
   , mSkipCount(0)
+  , mUseParserDuration(false)
+  , mLastParserDuration(-1)
 {
 #ifdef PR_LOGGING
   if (!gMediaDecoderLog) {
@@ -143,6 +236,15 @@ nsresult MediaOmxReader::ReadMetadata(MediaInfo* aInfo,
     return rv;
   }
 
+  bool isMP3 = mDecoder->GetResource()->GetContentType().EqualsASCII(AUDIO_MP3);
+  if (isMP3) {
+    // When read sdcard's file on b2g platform at constructor,
+    // the mDecoder->GetResource()->GetLength() would return -1.
+    // Delay set the total duration on this function.
+    mMP3FrameParser.SetLength(mDecoder->GetResource()->GetLength());
+    ProcessCachedData(0, true);
+  }
+
   if (!mOmxDecoder->TryLoad()) {
     return NS_ERROR_FAILURE;
   }
@@ -151,12 +253,24 @@ nsresult MediaOmxReader::ReadMetadata(MediaInfo* aInfo,
     return NS_OK;
   }
 
-  // Set the total duration (the max of the audio and video track).
+  if (isMP3 && mMP3FrameParser.IsMP3()) {
-  int64_t durationUs;
+    int64_t duration = mMP3FrameParser.GetDuration();
-  mOmxDecoder->GetDuration(&durationUs);
+    // The MP3FrameParser may reported a duration;
-  if (durationUs) {
+    // return -1 if no frame has been parsed.
-    ReentrantMonitorAutoEnter mon(mDecoder->GetReentrantMonitor());
+    if (duration >= 0) {
-    mDecoder->SetMediaDuration(durationUs);
+      ReentrantMonitorAutoEnter mon(mDecoder->GetReentrantMonitor());
+      mUseParserDuration = true;
+      mLastParserDuration = duration;
+      mDecoder->SetMediaDuration(mLastParserDuration);
+    }
+  } else {
+    // Set the total duration (the max of the audio and video track).
+    int64_t durationUs;
+    mOmxDecoder->GetDuration(&durationUs);
+    if (durationUs) {
+      ReentrantMonitorAutoEnter mon(mDecoder->GetReentrantMonitor());
+      mDecoder->SetMediaDuration(durationUs);
+    }
   }
 
   if (mOmxDecoder->HasVideo()) {
@@ -334,10 +448,22 @@ bool MediaOmxReader::DecodeVideoFrame(bool &aKeyframeSkip,
 
 void MediaOmxReader::NotifyDataArrived(const char* aBuffer, uint32_t aLength, int64_t aOffset)
 {
-  android::OmxDecoder *omxDecoder = mOmxDecoder.get();
+  MOZ_ASSERT(NS_IsMainThread());
 
-  if (omxDecoder) {
+  if (HasVideo()) {
-    omxDecoder->NotifyDataArrived(aBuffer, aLength, aOffset);
+    return;
+  }
+
+  if (!mMP3FrameParser.NeedsData()) {
+    return;
+  }
+
+  mMP3FrameParser.Parse(aBuffer, aLength, aOffset);
+  int64_t duration = mMP3FrameParser.GetDuration();
+  ReentrantMonitorAutoEnter mon(mDecoder->GetReentrantMonitor());
+  if (duration != mLastParserDuration && mUseParserDuration) {
+    mLastParserDuration = duration;
+    mDecoder->UpdateEstimatedMediaDuration(mLastParserDuration);
   }
 }
 
@@ -419,6 +545,48 @@ void MediaOmxReader::EnsureActive() {
   NS_ASSERTION(result == NS_OK, "OmxDecoder should be in play state to continue decoding");
 }
 
+int64_t MediaOmxReader::ProcessCachedData(int64_t aOffset, bool aWaitForCompletion)
+{
+  // We read data in chunks of 32 KiB. We can reduce this
+  // value if media, such as sdcards, is too slow.
+  // Because of SD card's slowness, need to keep sReadSize to small size.
+  // See Bug 914870.
+  static const int64_t sReadSize = 32 * 1024;
+
+  NS_ASSERTION(!NS_IsMainThread(), "Should not be on main thread.");
+
+  MOZ_ASSERT(mDecoder->GetResource());
+  int64_t resourceLength = mDecoder->GetResource()->GetCachedDataEnd(0);
+  NS_ENSURE_TRUE(resourceLength >= 0, -1);
+
+  if (aOffset >= resourceLength) {
+    return 0; // Cache is empty, nothing to do
+  }
+
+  int64_t bufferLength = std::min<int64_t>(resourceLength-aOffset, sReadSize);
+
+  nsAutoArrayPtr<char> buffer(new char[bufferLength]);
+
+  nsresult rv = mDecoder->GetResource()->ReadFromCache(buffer.get(),
+                                                       aOffset, bufferLength);
+  NS_ENSURE_SUCCESS(rv, -1);
+
+  nsRefPtr<OmxReaderNotifyDataArrivedRunnable> runnable(
+    new OmxReaderNotifyDataArrivedRunnable(this,
+                                           buffer.forget(),
+                                           bufferLength,
+                                           aOffset,
+                                           resourceLength));
+  if (aWaitForCompletion) {
+    rv = NS_DispatchToMainThread(runnable.get(), NS_DISPATCH_SYNC);
+  } else {
+    rv = NS_DispatchToMainThread(runnable.get());
+  }
+  NS_ENSURE_SUCCESS(rv, -1);
+
+  return resourceLength - aOffset - bufferLength;
+}
+
 android::sp<android::MediaSource> MediaOmxReader::GetAudioOffloadTrack()
 {
   if (!mOmxDecoder.get()) {

content/media/omx/MediaOmxReader.h

@@ -9,6 +9,8 @@
 #include "MediaOmxCommonReader.h"
 #include "MediaResource.h"
 #include "MediaDecoderReader.h"
+#include "nsMimeTypes.h"
+#include "MP3FrameParser.h"
 #include "nsRect.h"
 #include <ui/GraphicBuffer.h>
 #include <stagefright/MediaSource.h>
@@ -35,12 +37,13 @@ class MediaOmxReader : public MediaOmxCommonReader
   nsIntSize mInitialFrame;
   int64_t mVideoSeekTimeUs;
   int64_t mAudioSeekTimeUs;
+  int64_t mLastParserDuration;
   int32_t mSkipCount;
-
+  bool mUseParserDuration;
 protected:
   android::sp<android::OmxDecoder> mOmxDecoder;
   android::sp<android::MediaExtractor> mExtractor;
-
+  MP3FrameParser mMP3FrameParser;
   // Called by ReadMetadata() during MediaDecoderStateMachine::DecodeMetadata()
   // on decode thread. It create and initialize the OMX decoder including
   // setting up custom extractor. The extractor provide the essential
@@ -90,6 +93,8 @@ public:
 
   void ReleaseDecoder();
 
+  int64_t ProcessCachedData(int64_t aOffset, bool aWaitForCompletion);
+
   android::sp<android::MediaSource> GetAudioOffloadTrack();
 };
 

content/media/omx/OmxDecoder.cpp

@@ -44,156 +44,6 @@ using namespace MPAPI;
 using namespace mozilla;
 using namespace mozilla::gfx;
 using namespace mozilla::layers;
-
-namespace mozilla {
-
-class ReleaseOmxDecoderRunnable : public nsRunnable
-{
-public:
-  ReleaseOmxDecoderRunnable(const android::sp<android::OmxDecoder>& aOmxDecoder)
-  : mOmxDecoder(aOmxDecoder)
-  {
-  }
-
-  NS_METHOD Run() MOZ_OVERRIDE
-  {
-    MOZ_ASSERT(NS_IsMainThread());
-    mOmxDecoder = nullptr; // release OmxDecoder
-    return NS_OK;
-  }
-
-private:
-  android::sp<android::OmxDecoder> mOmxDecoder;
-};
-
-class OmxDecoderProcessCachedDataTask : public Task
-{
-public:
-  OmxDecoderProcessCachedDataTask(android::OmxDecoder* aOmxDecoder, int64_t aOffset)
-  : mOmxDecoder(aOmxDecoder),
-    mOffset(aOffset)
-  { }
-
-  void Run()
-  {
-    MOZ_ASSERT(!NS_IsMainThread());
-    MOZ_ASSERT(mOmxDecoder.get());
-    int64_t rem = mOmxDecoder->ProcessCachedData(mOffset, false);
-
-    if (rem <= 0) {
-      ReleaseOmxDecoderRunnable* r = new ReleaseOmxDecoderRunnable(mOmxDecoder);
-      mOmxDecoder.clear();
-      NS_DispatchToMainThread(r);
-    }
-  }
-
-private:
-  android::sp<android::OmxDecoder> mOmxDecoder;
-  int64_t                          mOffset;
-};
-
-// When loading an MP3 stream from a file, we need to parse the file's
-// content to find its duration. Reading files of 100 MiB or more can
-// delay the player app noticably, so the file is read and decoded in
-// smaller chunks.
-//
-// We first read on the decode thread, but parsing must be done on the
-// main thread. After we read the file's initial MiBs in the decode
-// thread, an instance of this class is scheduled to the main thread for
-// parsing the MP3 stream. The decode thread waits until it has finished.
-//
-// If there is more data available from the file, the runnable dispatches
-// a task to the IO thread for retrieving the next chunk of data, and
-// the IO task dispatches a runnable to the main thread for parsing the
-// data. This goes on until all of the MP3 file has been parsed.
-
-class OmxDecoderNotifyDataArrivedRunnable : public nsRunnable
-{
-public:
-  OmxDecoderNotifyDataArrivedRunnable(android::OmxDecoder* aOmxDecoder,
-                                      const char* aBuffer, uint64_t aLength,
-                                      int64_t aOffset, uint64_t aFullLength)
-  : mOmxDecoder(aOmxDecoder),
-    mBuffer(aBuffer),
-    mLength(aLength),
-    mOffset(aOffset),
-    mFullLength(aFullLength),
-    mCompletedMonitor("OmxDecoderNotifyDataArrived.mCompleted"),
-    mCompleted(false)
-  {
-    MOZ_ASSERT(mOmxDecoder.get());
-    MOZ_ASSERT(mBuffer.get() || !mLength);
-  }
-
-  NS_IMETHOD Run()
-  {
-    NS_ASSERTION(NS_IsMainThread(), "Should be on main thread.");
-
-    NotifyDataArrived();
-    Completed();
-
-    return NS_OK;
-  }
-
-  void WaitForCompletion()
-  {
-    MOZ_ASSERT(!NS_IsMainThread());
-
-    MonitorAutoLock mon(mCompletedMonitor);
-    if (!mCompleted) {
-      mCompletedMonitor.Wait();
-    }
-  }
-
-private:
-  void NotifyDataArrived()
-  {
-    const char* buffer = mBuffer.get();
-
-    while (mLength) {
-      uint32_t length = std::min<uint64_t>(mLength, UINT32_MAX);
-      bool success = mOmxDecoder->NotifyDataArrived(buffer, mLength,
-                                                    mOffset);
-      if (!success) {
-        return;
-      }
-
-      buffer  += length;
-      mLength -= length;
-      mOffset += length;
-    }
-
-    if (mOffset < mFullLength) {
-      // We cannot read data in the main thread because it
-      // might block for too long. Instead we post an IO task
-      // to the IO thread if there is more data available.
-      XRE_GetIOMessageLoop()->PostTask(FROM_HERE,
-          new OmxDecoderProcessCachedDataTask(mOmxDecoder.get(), mOffset));
-    }
-  }
-
-  // Call this function at the end of Run() to notify waiting
-  // threads.
-  void Completed()
-  {
-    MonitorAutoLock mon(mCompletedMonitor);
-    MOZ_ASSERT(!mCompleted);
-    mCompleted = true;
-    mCompletedMonitor.Notify();
-  }
-
-  android::sp<android::OmxDecoder> mOmxDecoder;
-  nsAutoArrayPtr<const char>       mBuffer;
-  uint64_t                         mLength;
-  int64_t                          mOffset;
-  uint64_t                         mFullLength;
-
-  Monitor mCompletedMonitor;
-  bool    mCompleted;
-};
-
-}
-
 using namespace android;
 
 OmxDecoder::OmxDecoder(MediaResource *aResource,
@@ -211,8 +61,6 @@ OmxDecoder::OmxDecoder(MediaResource *aResource,
   mAudioChannels(-1),
   mAudioSampleRate(-1),
   mDurationUs(-1),
-  mMP3FrameParser(aResource->GetLength()),
-  mIsMp3(false),
   mVideoBuffer(nullptr),
   mAudioBuffer(nullptr),
   mIsVideoSeeking(false),
@@ -268,9 +116,6 @@ bool OmxDecoder::Init(sp<MediaExtractor>& extractor) {
 
   const char* extractorMime;
   sp<MetaData> meta = extractor->getMetaData();
-  if (meta->findCString(kKeyMIMEType, &extractorMime) && !strcasecmp(extractorMime, AUDIO_MP3)) {
-    mIsMp3 = true;
-  }
 
   ssize_t audioTrackIndex = -1;
   ssize_t videoTrackIndex = -1;
@@ -342,17 +187,6 @@ bool OmxDecoder::TryLoad() {
     const char* audioMime;
     sp<MetaData> meta = mAudioTrack->getFormat();
 
-    if (mIsMp3) {
-      // Feed MP3 parser with cached data. Local files will be fully
-      // cached already, network streams will update with sucessive
-      // calls to NotifyDataArrived.
-      if (ProcessCachedData(0, true) >= 0) {
-        durationUs = mMP3FrameParser.GetDuration();
-        if (durationUs > totalDurationUs) {
-          totalDurationUs = durationUs;
-        }
-      }
-    }
     if ((durationUs == -1) && meta->findInt64(kKeyDuration, &durationUs)) {
       if (durationUs > totalDurationUs) {
         totalDurationUs = durationUs;
@@ -635,27 +469,6 @@ void OmxDecoder::ReleaseDecoder()
   mDecoder = nullptr;
 }
 
-bool OmxDecoder::NotifyDataArrived(const char* aBuffer, uint32_t aLength, int64_t aOffset)
-{
-  if (!mAudioTrack.get() || !mIsMp3 || !mMP3FrameParser.IsMP3() || !mDecoder) {
-    return false;
-  }
-
-  mMP3FrameParser.Parse(aBuffer, aLength, aOffset);
-
-  int64_t durationUs = mMP3FrameParser.GetDuration();
-
-  if (durationUs != mDurationUs) {
-    mDurationUs = durationUs;
-
-    MOZ_ASSERT(mDecoder);
-    ReentrantMonitorAutoEnter mon(mDecoder->GetReentrantMonitor());
-    mDecoder->UpdateEstimatedMediaDuration(mDurationUs);
-  }
-
-  return true;
-}
-
 void OmxDecoder::ReleaseVideoBuffer() {
   if (mVideoBuffer) {
     mVideoBuffer->release();
@@ -1085,46 +898,3 @@ OmxDecoder::RecycleCallback(TextureClient* aClient, void* aClosure)
   OmxDecoder* decoder = static_cast<OmxDecoder*>(aClosure);
   decoder->RecycleCallbackImp(aClient);
 }
-
-int64_t OmxDecoder::ProcessCachedData(int64_t aOffset, bool aWaitForCompletion)
-{
-  // We read data in chunks of 32 KiB. We can reduce this
-  // value if media, such as sdcards, is too slow.
-  // Because of SD card's slowness, need to keep sReadSize to small size.
-  // See Bug 914870.
-  static const int64_t sReadSize = 32 * 1024;
-
-  NS_ASSERTION(!NS_IsMainThread(), "Should not be on main thread.");
-
-  MOZ_ASSERT(mResource);
-
-  int64_t resourceLength = mResource->GetCachedDataEnd(0);
-  NS_ENSURE_TRUE(resourceLength >= 0, -1);
-
-  if (aOffset >= resourceLength) {
-    return 0; // Cache is empty, nothing to do
-  }
-
-  int64_t bufferLength = std::min<int64_t>(resourceLength-aOffset, sReadSize);
-
-  nsAutoArrayPtr<char> buffer(new char[bufferLength]);
-
-  nsresult rv = mResource->ReadFromCache(buffer.get(), aOffset, bufferLength);
-  NS_ENSURE_SUCCESS(rv, -1);
-
-  nsRefPtr<OmxDecoderNotifyDataArrivedRunnable> runnable(
-    new OmxDecoderNotifyDataArrivedRunnable(this,
-                                            buffer.forget(),
-                                            bufferLength,
-                                            aOffset,
-                                            resourceLength));
-
-  rv = NS_DispatchToMainThread(runnable.get());
-  NS_ENSURE_SUCCESS(rv, -1);
-
-  if (aWaitForCompletion) {
-    runnable->WaitForCompletion();
-  }
-
-  return resourceLength - aOffset - bufferLength;
-}

content/media/omx/OmxDecoder.h

@@ -162,8 +162,6 @@ public:
 
   void ReleaseDecoder();
 
-  bool NotifyDataArrived(const char* aBuffer, uint32_t aLength, int64_t aOffset);
-
   void GetDuration(int64_t *durationUs) {
     *durationUs = mDurationUs;
   }