Bug 1341483 - MP4Metadata::GetTrackIndice() now also returns a success/error code - r?kinetik draft
authorGerald Squelart <gsquelart@mozilla.com>
Wed, 15 Mar 2017 16:55:50 +1100
changeset 560830 572166bdf33bc1d1f3ac0b6081ab961d3f4d50c0
parent 560829 53766f81f74ef80a491427523039c2c8d9703147
child 560831 1ec54438befc112695e97dfd71bc95609246775f
push id53551
push usergsquelart@mozilla.com
push dateTue, 11 Apr 2017 23:47:44 +0000
reviewerskinetik
bugs1341483
milestone55.0a1
Bug 1341483 - MP4Metadata::GetTrackIndice() now also returns a success/error code - r?kinetik MozReview-Commit-ID: BIgvy5eKNJl
dom/media/fmp4/MP4Demuxer.cpp
media/libstagefright/binding/MP4Metadata.cpp
media/libstagefright/binding/include/mp4_demuxer/MP4Metadata.h
media/libstagefright/gtest/TestParser.cpp
--- a/dom/media/fmp4/MP4Demuxer.cpp
+++ b/dom/media/fmp4/MP4Demuxer.cpp
@@ -203,27 +203,26 @@ MP4Demuxer::Init()
           result = MediaResult(NS_ERROR_DOM_MEDIA_DEMUXER_ERR,
                                RESULT_DETAIL("Invalid MP4 audio track (%s)",
                                              info.Result().Description().get()));
         }
         continue;
       } else if (NS_FAILED(info.Result()) && result == NS_OK) {
         result = Move(info.Result());
       }
-      UniquePtr<mp4_demuxer::IndiceWrapper> indices =
+      mp4_demuxer::MP4Metadata::ResultAndIndice indices =
         metadata.GetTrackIndice(info.Ref()->mTrackId);
-      if (!indices) {
-        if (result == NS_OK) {
-          result =
-            MediaResult(NS_ERROR_DOM_MEDIA_DEMUXER_ERR,
-                        RESULT_DETAIL("Invalid MP4 audio track index list"));
+      if (!indices.Ref()) {
+        if (NS_FAILED(info.Result()) && result == NS_OK) {
+          result = Move(indices.Result());
         }
         continue;
       }
-      mAudioDemuxers[i] = new MP4TrackDemuxer(this, Move(info.Ref()), *indices.get());
+      mAudioDemuxers[i] =
+        new MP4TrackDemuxer(this, Move(info.Ref()), *indices.Ref().get());
     }
   }
 
   if (videoTrackCount.Ref() != 0) {
     mVideoDemuxers.SetLength(videoTrackCount.Ref());
     for (size_t i = 0; i < videoTrackCount.Ref(); i++) {
       mp4_demuxer::MP4Metadata::ResultAndTrackInfo info =
         metadata.GetTrackInfo(TrackInfo::kVideoTrack, i);
@@ -239,27 +238,26 @@ MP4Demuxer::Init()
           result = MediaResult(NS_ERROR_DOM_MEDIA_DEMUXER_ERR,
                                RESULT_DETAIL("Invalid MP4 video track (%s)",
                                              info.Result().Description().get()));
         }
         continue;
       } else if (NS_FAILED(info.Result()) && result == NS_OK) {
         result = Move(info.Result());
       }
-      UniquePtr<mp4_demuxer::IndiceWrapper> indices =
+      mp4_demuxer::MP4Metadata::ResultAndIndice indices =
         metadata.GetTrackIndice(info.Ref()->mTrackId);
-      if (!indices) {
-        if (result == NS_OK) {
-          result =
-            MediaResult(NS_ERROR_DOM_MEDIA_DEMUXER_ERR,
-                        RESULT_DETAIL("Invalid MP4 video track index list"));
+      if (!indices.Ref()) {
+        if (NS_FAILED(info.Result()) && result == NS_OK) {
+          result = Move(indices.Result());
         }
         continue;
       }
-      mVideoDemuxers[i] = new MP4TrackDemuxer(this, Move(info.Ref()), *indices.get());
+      mVideoDemuxers[i] =
+        new MP4TrackDemuxer(this, Move(info.Ref()), *indices.Ref().get());
     }
   }
 
   mp4_demuxer::MP4Metadata::ResultAndCryptoFile cryptoFile =
     metadata.Crypto();
   if (NS_FAILED(cryptoFile.Result()) && result == NS_OK) {
     result = Move(cryptoFile.Result());
   }
--- a/media/libstagefright/binding/MP4Metadata.cpp
+++ b/media/libstagefright/binding/MP4Metadata.cpp
@@ -7,16 +7,17 @@
 #include "media/stagefright/MediaDefs.h"
 #include "media/stagefright/MediaSource.h"
 #include "media/stagefright/MetaData.h"
 #include "mozilla/Assertions.h"
 #include "mozilla/CheckedInt.h"
 #include "mozilla/EndianUtils.h"
 #include "mozilla/Logging.h"
 #include "mozilla/RefPtr.h"
+#include "mozilla/SizePrintfMacros.h"
 #include "mozilla/Telemetry.h"
 #include "mozilla/UniquePtr.h"
 #include "VideoUtils.h"
 #include "mp4_demuxer/MoofParser.h"
 #include "mp4_demuxer/MP4Metadata.h"
 #include "mp4_demuxer/Stream.h"
 #include "MediaPrefs.h"
 #include "mp4parse.h"
@@ -84,17 +85,17 @@ public:
   MP4Metadata::ResultAndTrackCount
   GetNumberTracks(mozilla::TrackInfo::TrackType aType) const;
   MP4Metadata::ResultAndTrackInfo GetTrackInfo(
     mozilla::TrackInfo::TrackType aType, size_t aTrackNumber) const;
   bool CanSeek() const;
 
   MP4Metadata::ResultAndCryptoFile Crypto() const;
 
-  bool ReadTrackIndex(FallibleTArray<Index::Indice>& aDest, mozilla::TrackID aTrackID);
+  MediaResult ReadTrackIndex(FallibleTArray<Index::Indice>& aDest, mozilla::TrackID aTrackID);
 
 private:
   int32_t GetTrackNumber(mozilla::TrackID aTrackID);
   void UpdateCrypto(const stagefright::MetaData* aMetaData);
   mozilla::UniquePtr<mozilla::TrackInfo> CheckTrack(const char* aMimeType,
                                                     stagefright::MetaData* aMetaData,
                                                     int32_t aIndex) const;
   CryptoFile mCrypto;
@@ -133,17 +134,17 @@ public:
   MP4Metadata::ResultAndTrackCount
   GetNumberTracks(mozilla::TrackInfo::TrackType aType) const;
   MP4Metadata::ResultAndTrackInfo GetTrackInfo(
     mozilla::TrackInfo::TrackType aType, size_t aTrackNumber) const;
   bool CanSeek() const;
 
   MP4Metadata::ResultAndCryptoFile Crypto() const;
 
-  bool ReadTrackIndice(mp4parse_byte_data* aIndices, mozilla::TrackID aTrackID);
+  MediaResult ReadTrackIndice(mp4parse_byte_data* aIndices, mozilla::TrackID aTrackID);
 
 private:
   void UpdateCrypto();
   Maybe<uint32_t> TrackTypeToGlobalTrackIndex(mozilla::TrackInfo::TrackType aType, size_t aTrackNumber) const;
 
   CryptoFile mCrypto;
   RefPtr<Stream> mSource;
   RustStreamAdaptor mRustSource;
@@ -545,29 +546,33 @@ MP4Metadata::Crypto() const
 
   if (mPreferRust) {
     return rustCrypto;
   }
 
   return crypto;
 }
 
-mozilla::UniquePtr<IndiceWrapper>
+MP4Metadata::ResultAndIndice
 MP4Metadata::GetTrackIndice(mozilla::TrackID aTrackID)
 {
   FallibleTArray<Index::Indice> indiceSF;
-  if ((!mPreferRust || mRustTestMode) &&
-       !mStagefright->ReadTrackIndex(indiceSF, aTrackID)) {
-    return nullptr;
+  if (!mPreferRust || mRustTestMode) {
+    MediaResult rv = mStagefright->ReadTrackIndex(indiceSF, aTrackID);
+    if (NS_FAILED(rv)) {
+      return {Move(rv), nullptr};
+    }
   }
 
   mp4parse_byte_data indiceRust = {};
-  if ((mPreferRust || mRustTestMode) &&
-      !mRust->ReadTrackIndice(&indiceRust, aTrackID)) {
-    return nullptr;
+  if (mPreferRust || mRustTestMode) {
+    MediaResult rvRust = mRust->ReadTrackIndice(&indiceRust, aTrackID);
+    if (NS_FAILED(rvRust)) {
+      return {Move(rvRust), nullptr};
+    }
   }
 
 #ifndef RELEASE_OR_BETA
   if (mRustTestMode) {
     MOZ_DIAGNOSTIC_ASSERT(indiceRust.length == indiceSF.Length());
     for (uint32_t i = 0; i < indiceRust.length; i++) {
       MOZ_DIAGNOSTIC_ASSERT(indiceRust.indices[i].start_offset == indiceSF[i].start_offset);
       MOZ_DIAGNOSTIC_ASSERT(indiceRust.indices[i].end_offset == indiceSF[i].end_offset);
@@ -581,43 +586,45 @@ MP4Metadata::GetTrackIndice(mozilla::Tra
 
   UniquePtr<IndiceWrapper> indice;
   if (mPreferRust) {
     indice = mozilla::MakeUnique<IndiceWrapperRust>(indiceRust);
   } else {
     indice = mozilla::MakeUnique<IndiceWrapperStagefright>(indiceSF);
   }
 
-  return indice;
+  return {NS_OK, Move(indice)};
 }
 
-static inline bool
+static inline MediaResult
 ConvertIndex(FallibleTArray<Index::Indice>& aDest,
              const nsTArray<stagefright::MediaSource::Indice>& aIndex,
              int64_t aMediaTime)
 {
   if (!aDest.SetCapacity(aIndex.Length(), mozilla::fallible)) {
-    return false;
+    return MediaResult{NS_ERROR_OUT_OF_MEMORY,
+                       RESULT_DETAIL("Could not resize to %" PRIuSIZE " indices",
+                                     aIndex.Length())};
   }
   for (size_t i = 0; i < aIndex.Length(); i++) {
     Index::Indice indice;
     const stagefright::MediaSource::Indice& s_indice = aIndex[i];
     indice.start_offset = s_indice.start_offset;
     indice.end_offset = s_indice.end_offset;
     indice.start_composition = s_indice.start_composition - aMediaTime;
     indice.end_composition = s_indice.end_composition - aMediaTime;
     indice.start_decode = s_indice.start_decode;
     indice.sync = s_indice.sync;
     // FIXME: Make this infallible after bug 968520 is done.
     MOZ_ALWAYS_TRUE(aDest.AppendElement(indice, mozilla::fallible));
     MOZ_LOG(sLog, LogLevel::Debug, ("s_o: %" PRIu64 ", e_o: %" PRIu64 ", s_c: %" PRIu64 ", e_c: %" PRIu64 ", s_d: %" PRIu64 ", sync: %d\n",
                                     indice.start_offset, indice.end_offset, indice.start_composition, indice.end_composition,
                                     indice.start_decode, indice.sync));
   }
-  return true;
+  return NS_OK;
 }
 
 MP4MetadataStagefright::MP4MetadataStagefright(Stream* aSource)
   : mSource(aSource)
   , mMetadataExtractor(new MPEG4Extractor(new DataSourceAdapter(mSource)))
   , mCanSeek(mMetadataExtractor->flags() & MediaExtractor::CAN_SEEK)
 {
   sp<MetaData> metaData = mMetadataExtractor->getMetaData();
@@ -780,36 +787,39 @@ MP4MetadataStagefright::UpdateCrypto(con
   // There's no point in checking that the type matches anything because it
   // isn't set consistently in the MPEG4Extractor.
   if (!aMetaData->findData(kKeyPssh, &type, &data, &size)) {
     return;
   }
   mCrypto.Update(reinterpret_cast<const uint8_t*>(data), size);
 }
 
-bool
+MediaResult
 MP4MetadataStagefright::ReadTrackIndex(FallibleTArray<Index::Indice>& aDest, mozilla::TrackID aTrackID)
 {
   size_t numTracks = mMetadataExtractor->countTracks();
   int32_t trackNumber = GetTrackNumber(aTrackID);
   if (trackNumber < 0) {
-    return false;
+    return MediaResult(NS_ERROR_DOM_MEDIA_METADATA_ERR,
+                       RESULT_DETAIL("Cannot find track id %d",
+                                     int(aTrackID)));
   }
   sp<MediaSource> track = mMetadataExtractor->getTrack(trackNumber);
   if (!track.get()) {
-    return false;
+    return MediaResult(NS_ERROR_DOM_MEDIA_METADATA_ERR,
+                       RESULT_DETAIL("Cannot access track id %d",
+                                     int(aTrackID)));
   }
   sp<MetaData> metadata = mMetadataExtractor->getTrackMetaData(trackNumber);
   int64_t mediaTime;
   if (!metadata->findInt64(kKeyMediaTime, &mediaTime)) {
     mediaTime = 0;
   }
-  bool rv = ConvertIndex(aDest, track->exportIndex(), mediaTime);
 
-  return rv;
+  return ConvertIndex(aDest, track->exportIndex(), mediaTime);
 }
 
 int32_t
 MP4MetadataStagefright::GetTrackNumber(mozilla::TrackID aTrackID)
 {
   size_t numTracks = mMetadataExtractor->countTracks();
   for (size_t i = 0; i < numTracks; i++) {
     sp<MetaData> metaData = mMetadataExtractor->getTrackMetaData(i);
@@ -1094,35 +1104,41 @@ MP4MetadataRust::CanSeek() const
 }
 
 MP4Metadata::ResultAndCryptoFile
 MP4MetadataRust::Crypto() const
 {
   return {NS_OK, &mCrypto};
 }
 
-bool
+MediaResult
 MP4MetadataRust::ReadTrackIndice(mp4parse_byte_data* aIndices, mozilla::TrackID aTrackID)
 {
   uint8_t fragmented = false;
   auto rv = mp4parse_is_fragmented(mRustParser.get(), aTrackID, &fragmented);
   if (rv != mp4parse_status_OK) {
-    return false;
+    return MediaResult(NS_ERROR_DOM_MEDIA_METADATA_ERR,
+                       RESULT_DETAIL("Cannot parse whether track id %d is "
+                                     "fragmented, mp4parse_error=%d",
+                                     int(aTrackID), int(rv)));
   }
 
   if (fragmented) {
-    return true;
+    return NS_OK;
   }
 
   rv = mp4parse_get_indice_table(mRustParser.get(), aTrackID, aIndices);
   if (rv != mp4parse_status_OK) {
-    return false;
+    return MediaResult(NS_ERROR_DOM_MEDIA_METADATA_ERR,
+                       RESULT_DETAIL("Cannot parse index table in track id %d, "
+                                     "mp4parse_error=%d",
+                                     int(aTrackID), int(rv)));
   }
 
-  return true;
+  return NS_OK;
 }
 
 /*static*/ MP4Metadata::ResultAndByteBuffer
 MP4MetadataRust::Metadata(Stream* aSource)
 {
   MOZ_ASSERT(false, "Not yet implemented");
   return {NS_ERROR_NOT_IMPLEMENTED, nullptr};
 }
--- a/media/libstagefright/binding/include/mp4_demuxer/MP4Metadata.h
+++ b/media/libstagefright/binding/include/mp4_demuxer/MP4Metadata.h
@@ -72,17 +72,18 @@ public:
   ResultAndTrackInfo GetTrackInfo(mozilla::TrackInfo::TrackType aType,
                                   size_t aTrackNumber) const;
 
   bool CanSeek() const;
 
   using ResultAndCryptoFile = ResultAndType<const CryptoFile*>;
   ResultAndCryptoFile Crypto() const;
 
-  mozilla::UniquePtr<IndiceWrapper> GetTrackIndice(mozilla::TrackID aTrackID);
+  using ResultAndIndice = ResultAndType<mozilla::UniquePtr<IndiceWrapper>>;
+  ResultAndIndice GetTrackIndice(mozilla::TrackID aTrackID);
 
 private:
   UniquePtr<MP4MetadataStagefright> mStagefright;
   UniquePtr<MP4MetadataRust> mRust;
   mutable bool mPreferRust;
   mutable bool mReportedAudioTrackTelemetry;
   mutable bool mReportedVideoTrackTelemetry;
 #ifndef RELEASE_OR_BETA
--- a/media/libstagefright/gtest/TestParser.cpp
+++ b/media/libstagefright/gtest/TestParser.cpp
@@ -314,21 +314,22 @@ TEST(stagefright_MPEG4Metadata, test_cas
         const VideoInfo* videoInfo = trackInfo.Ref()->GetAsVideoInfo();
         ASSERT_TRUE(!!videoInfo);
         EXPECT_TRUE(videoInfo->IsValid());
         EXPECT_TRUE(videoInfo->IsVideo());
         EXPECT_EQ(tests[test].mVideoDuration, videoInfo->mDuration);
         EXPECT_EQ(tests[test].mWidth, videoInfo->mDisplay.width);
         EXPECT_EQ(tests[test].mHeight, videoInfo->mDisplay.height);
 
-        UniquePtr<IndiceWrapper> indices = metadata.GetTrackIndice(videoInfo->mTrackId);
-        EXPECT_TRUE(!!indices);
-        for (size_t i = 0; i < indices->Length(); i++) {
+        MP4Metadata::ResultAndIndice indices =
+          metadata.GetTrackIndice(videoInfo->mTrackId);
+        EXPECT_TRUE(!!indices.Ref());
+        for (size_t i = 0; i < indices.Ref()->Length(); i++) {
           Index::Indice data;
-          EXPECT_TRUE(indices->GetIndice(i, data));
+          EXPECT_TRUE(indices.Ref()->GetIndice(i, data));
           EXPECT_TRUE(data.start_offset <= data.end_offset);
           EXPECT_TRUE(data.start_composition <= data.end_composition);
         }
       }
       trackInfo = metadata.GetTrackInfo(TrackInfo::kAudioTrack, 0);
       if (tests[test].mNumberAudioTracks == 0 ||
           tests[test].mNumberAudioTracks == E) {
         EXPECT_TRUE(!trackInfo.Ref());
@@ -339,21 +340,22 @@ TEST(stagefright_MPEG4Metadata, test_cas
         EXPECT_TRUE(audioInfo->IsValid());
         EXPECT_TRUE(audioInfo->IsAudio());
         EXPECT_EQ(tests[test].mAudioDuration, audioInfo->mDuration);
         EXPECT_EQ(tests[test].mAudioProfile, audioInfo->mProfile);
         if (tests[test].mAudioDuration != audioInfo->mDuration) {
           MOZ_RELEASE_ASSERT(false);
         }
 
-        UniquePtr<IndiceWrapper> indices = metadata.GetTrackIndice(audioInfo->mTrackId);
-        EXPECT_TRUE(!!indices);
-        for (size_t i = 0; i < indices->Length(); i++) {
+        MP4Metadata::ResultAndIndice indices =
+          metadata.GetTrackIndice(audioInfo->mTrackId);
+        EXPECT_TRUE(!!indices.Ref());
+        for (size_t i = 0; i < indices.Ref()->Length(); i++) {
           Index::Indice data;
-          EXPECT_TRUE(indices->GetIndice(i, data));
+          EXPECT_TRUE(indices.Ref()->GetIndice(i, data));
           EXPECT_TRUE(data.start_offset <= data.end_offset);
           EXPECT_TRUE(int64_t(data.start_composition) <= int64_t(data.end_composition));
         }
       }
       EXPECT_FALSE(metadata.GetTrackInfo(TrackInfo::kTextTrack, 0).Ref());
       EXPECT_FALSE(metadata.GetTrackInfo(static_cast<TrackInfo::TrackType>(-1), 0).Ref());
       // We can see anywhere in any MPEG4.
       EXPECT_TRUE(metadata.CanSeek());