Bug 1341483 - MP4Metadata::Metadata() now also returns a success/error code - r?kinetik draft
authorGerald Squelart <gsquelart@mozilla.com>
Mon, 27 Feb 2017 13:01:11 +1100
changeset 560826 0f0824d4def213e0f1e311106a0546cbf056bae7
parent 560825 520d1afa8740579915b4db629c409654f93cfa46
child 560827 5fdb247997247e282232b05ba95c0300ebcc0582
push id53551
push usergsquelart@mozilla.com
push dateTue, 11 Apr 2017 23:47:44 +0000
reviewerskinetik
bugs1341483
milestone55.0a1
Bug 1341483 - MP4Metadata::Metadata() now also returns a success/error code - r?kinetik The returned MediaResult is used as error or warning in MP4Demuxer::Init(). MozReview-Commit-ID: Bnv4xG8bCJ4
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
@@ -122,26 +122,31 @@ MP4Demuxer::MP4Demuxer(MediaResource* aR
 RefPtr<MP4Demuxer::InitPromise>
 MP4Demuxer::Init()
 {
   AutoPinned<mp4_demuxer::ResourceStream> stream(mStream);
 
   // 'result' will capture the first warning, if any.
   MediaResult result{NS_OK};
 
-  RefPtr<MediaByteBuffer> initData = mp4_demuxer::MP4Metadata::Metadata(stream);
-  if (!initData) {
+  mp4_demuxer::MP4Metadata::ResultAndByteBuffer initData =
+    mp4_demuxer::MP4Metadata::Metadata(stream);
+  if (!initData.Ref()) {
     return InitPromise::CreateAndReject(
-      MediaResult(NS_ERROR_DOM_MEDIA_DEMUXER_ERR,
-                  RESULT_DETAIL("Invalid MP4 metadata or OOM")),
+      NS_FAILED(initData.Result())
+      ? Move(initData.Result())
+      : MediaResult(NS_ERROR_DOM_MEDIA_DEMUXER_ERR,
+                    RESULT_DETAIL("Invalid MP4 metadata or OOM")),
       __func__);
+  } else if (NS_FAILED(initData.Result()) && result == NS_OK) {
+    result = Move(initData.Result());
   }
 
   RefPtr<mp4_demuxer::BufferStream> bufferstream =
-    new mp4_demuxer::BufferStream(initData);
+    new mp4_demuxer::BufferStream(initData.Ref());
 
   mp4_demuxer::MP4Metadata metadata{bufferstream};
 
   auto audioTrackCount = metadata.GetNumberTracks(TrackInfo::kAudioTrack);
   auto videoTrackCount = metadata.GetNumberTracks(TrackInfo::kVideoTrack);
   if (audioTrackCount == 0 && videoTrackCount == 0) {
     return InitPromise::CreateAndReject(
       MediaResult(NS_ERROR_DOM_MEDIA_DEMUXER_ERR,
--- a/media/libstagefright/binding/MP4Metadata.cpp
+++ b/media/libstagefright/binding/MP4Metadata.cpp
@@ -74,17 +74,18 @@ private:
 };
 
 class MP4MetadataStagefright
 {
 public:
   explicit MP4MetadataStagefright(Stream* aSource);
   ~MP4MetadataStagefright();
 
-  static already_AddRefed<mozilla::MediaByteBuffer> Metadata(Stream* aSource);
+  static MP4Metadata::ResultAndByteBuffer Metadata(Stream* aSource);
+
   uint32_t GetNumberTracks(mozilla::TrackInfo::TrackType aType) const;
   mozilla::UniquePtr<mozilla::TrackInfo> GetTrackInfo(mozilla::TrackInfo::TrackType aType,
                                                       size_t aTrackNumber) const;
   bool CanSeek() const;
 
   const CryptoFile& Crypto() const;
 
   bool ReadTrackIndex(FallibleTArray<Index::Indice>& aDest, mozilla::TrackID aTrackID);
@@ -121,17 +122,18 @@ private:
 };
 
 class MP4MetadataRust
 {
 public:
   explicit MP4MetadataRust(Stream* aSource);
   ~MP4MetadataRust();
 
-  static already_AddRefed<mozilla::MediaByteBuffer> Metadata(Stream* aSource);
+  static MP4Metadata::ResultAndByteBuffer Metadata(Stream* aSource);
+
   uint32_t GetNumberTracks(mozilla::TrackInfo::TrackType aType) const;
   mozilla::UniquePtr<mozilla::TrackInfo> GetTrackInfo(mozilla::TrackInfo::TrackType aType,
                                                       size_t aTrackNumber) const;
   bool CanSeek() const;
 
   const CryptoFile& Crypto() const;
 
   bool ReadTrackIndice(mp4parse_byte_data* aIndices, mozilla::TrackID aTrackID);
@@ -238,17 +240,17 @@ MP4Metadata::MP4Metadata(Stream* aSource
 #endif
 {
 }
 
 MP4Metadata::~MP4Metadata()
 {
 }
 
-/*static*/ already_AddRefed<mozilla::MediaByteBuffer>
+/*static*/ MP4Metadata::ResultAndByteBuffer
 MP4Metadata::Metadata(Stream* aSource)
 {
   return MP4MetadataStagefright::Metadata(aSource);
 }
 
 static const char *
 TrackTypeToString(mozilla::TrackInfo::TrackType aType)
 {
@@ -680,21 +682,27 @@ MP4MetadataStagefright::GetTrackNumber(m
     int32_t value;
     if (metaData->findInt32(kKeyTrackID, &value) && value == aTrackID) {
       return i;
     }
   }
   return -1;
 }
 
-/*static*/ already_AddRefed<mozilla::MediaByteBuffer>
+/*static*/ MP4Metadata::ResultAndByteBuffer
 MP4MetadataStagefright::Metadata(Stream* aSource)
 {
   auto parser = mozilla::MakeUnique<MoofParser>(aSource, 0, false);
-  return parser->Metadata();
+  RefPtr<mozilla::MediaByteBuffer> buffer = parser->Metadata();
+  if (!buffer) {
+    return {MediaResult(NS_ERROR_DOM_MEDIA_METADATA_ERR,
+                        RESULT_DETAIL("Cannot parse metadata")),
+            nullptr};
+  }
+  return {NS_OK, Move(buffer)};
 }
 
 bool
 RustStreamAdaptor::Read(uint8_t* buffer, uintptr_t size, size_t* bytes_read)
 {
   if (!mOffset.isValid()) {
     MOZ_LOG(sLog, LogLevel::Error, ("Overflow in source stream offset"));
     return false;
@@ -946,16 +954,16 @@ MP4MetadataRust::ReadTrackIndice(mp4pars
   rv = mp4parse_get_indice_table(mRustParser.get(), aTrackID, aIndices);
   if (rv != mp4parse_status_OK) {
     return false;
   }
 
   return true;
 }
 
-/*static*/ already_AddRefed<mozilla::MediaByteBuffer>
+/*static*/ MP4Metadata::ResultAndByteBuffer
 MP4MetadataRust::Metadata(Stream* aSource)
 {
   MOZ_ASSERT(false, "Not yet implemented");
-  return nullptr;
+  return {NS_ERROR_NOT_IMPLEMENTED, nullptr};
 }
 
 } // namespace mp4_demuxer
--- a/media/libstagefright/binding/include/mp4_demuxer/MP4Metadata.h
+++ b/media/libstagefright/binding/include/mp4_demuxer/MP4Metadata.h
@@ -1,25 +1,26 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #ifndef MP4METADATA_H_
 #define MP4METADATA_H_
 
+#include "mozilla/TypeTraits.h"
 #include "mozilla/UniquePtr.h"
 #include "mp4_demuxer/DecoderData.h"
 #include "mp4_demuxer/Index.h"
 #include "MediaData.h"
 #include "MediaInfo.h"
+#include "MediaResult.h"
 #include "Stream.h"
 #include "mp4parse.h"
 
-namespace mp4_demuxer
-{
+namespace mp4_demuxer {
 
 class MP4MetadataStagefright;
 class MP4MetadataRust;
 
 class IndiceWrapper {
 public:
   virtual size_t Length() const = 0;
 
@@ -31,17 +32,42 @@ public:
 };
 
 class MP4Metadata
 {
 public:
   explicit MP4Metadata(Stream* aSource);
   ~MP4Metadata();
 
-  static already_AddRefed<mozilla::MediaByteBuffer> Metadata(Stream* aSource);
+  // Simple template class containing a MediaResult and another type.
+  template <typename T>
+  class ResultAndType
+  {
+  public:
+    template <typename M2, typename T2>
+    ResultAndType(M2&& aM, T2&& aT)
+      : mResult(Forward<M2>(aM)), mT(Forward<T2>(aT))
+    {
+    }
+    ResultAndType(const ResultAndType&) = default;
+    ResultAndType& operator=(const ResultAndType&) = default;
+    ResultAndType(ResultAndType&&) = default;
+    ResultAndType& operator=(ResultAndType&&) = default;
+
+    mozilla::MediaResult& Result() { return mResult; }
+    T& Ref() { return mT; }
+
+  private:
+    mozilla::MediaResult mResult;
+    typename mozilla::Decay<T>::Type mT;
+  };
+
+  using ResultAndByteBuffer = ResultAndType<RefPtr<mozilla::MediaByteBuffer>>;
+  static ResultAndByteBuffer Metadata(Stream* aSource);
+
   uint32_t GetNumberTracks(mozilla::TrackInfo::TrackType aType) const;
   mozilla::UniquePtr<mozilla::TrackInfo> GetTrackInfo(mozilla::TrackInfo::TrackType aType,
                                                       size_t aTrackNumber) const;
   bool CanSeek() const;
 
   const CryptoFile& Crypto() const;
 
   mozilla::UniquePtr<IndiceWrapper> GetTrackIndice(mozilla::TrackID aTrackID);
--- a/media/libstagefright/gtest/TestParser.cpp
+++ b/media/libstagefright/gtest/TestParser.cpp
@@ -69,18 +69,20 @@ protected:
   const uint8_t* mBuffer;
   size_t mSize;
 };
 
 TEST(stagefright_MP4Metadata, EmptyStream)
 {
   RefPtr<Stream> stream = new TestStream(nullptr, 0);
 
-  RefPtr<MediaByteBuffer> metadataBuffer = MP4Metadata::Metadata(stream);
-  EXPECT_FALSE(metadataBuffer);
+  MP4Metadata::ResultAndByteBuffer metadataBuffer =
+    MP4Metadata::Metadata(stream);
+  EXPECT_TRUE(NS_OK != metadataBuffer.Result());
+  EXPECT_FALSE(static_cast<bool>(metadataBuffer.Ref()));
 
   MP4Metadata metadata(stream);
   EXPECT_EQ(0u, metadata.GetNumberTracks(TrackInfo::kUndefinedTrack));
   EXPECT_EQ(0u, metadata.GetNumberTracks(TrackInfo::kAudioTrack));
   EXPECT_EQ(0u, metadata.GetNumberTracks(TrackInfo::kVideoTrack));
   EXPECT_EQ(0u, metadata.GetNumberTracks(TrackInfo::kTextTrack));
   EXPECT_EQ(0u, metadata.GetNumberTracks(static_cast<TrackInfo::TrackType>(-1)));
   EXPECT_FALSE(metadata.GetTrackInfo(TrackInfo::kUndefinedTrack, 0));
@@ -270,18 +272,20 @@ TEST(stagefright_MPEG4Metadata, test_cas
       length = ArrayLength(testFiles);
     }
 
     for (size_t test = 0; test < length; ++test) {
       nsTArray<uint8_t> buffer = ReadTestFile(tests[test].mFilename);
       ASSERT_FALSE(buffer.IsEmpty());
       RefPtr<Stream> stream = new TestStream(buffer.Elements(), buffer.Length());
 
-      RefPtr<MediaByteBuffer> metadataBuffer = MP4Metadata::Metadata(stream);
-      EXPECT_TRUE(metadataBuffer);
+      MP4Metadata::ResultAndByteBuffer metadataBuffer =
+        MP4Metadata::Metadata(stream);
+      EXPECT_EQ(NS_OK, metadataBuffer.Result());
+      EXPECT_TRUE(metadataBuffer.Ref());
 
       MP4Metadata metadata(stream);
       EXPECT_EQ(0u, metadata.GetNumberTracks(TrackInfo::kUndefinedTrack));
       EXPECT_EQ(tests[test].mNumberAudioTracks,
                 metadata.GetNumberTracks(TrackInfo::kAudioTrack));
       EXPECT_EQ(tests[test].mNumberVideoTracks,
                 metadata.GetNumberTracks(TrackInfo::kVideoTrack));
       EXPECT_EQ(0u, metadata.GetNumberTracks(TrackInfo::kTextTrack));
@@ -355,17 +359,18 @@ TEST(stagefright_MPEG4Metadata, test_cas
     // making sure it doesn't crash.
     // No checks because results would differ for each position.
     for (size_t offset = 0; offset < buffer.Length() - step; offset += step) {
       size_t size = buffer.Length() - offset;
       while (size > 0) {
         RefPtr<TestStream> stream =
           new TestStream(buffer.Elements() + offset, size);
 
-        RefPtr<MediaByteBuffer> metadataBuffer = MP4Metadata::Metadata(stream);
+        MP4Metadata::ResultAndByteBuffer metadataBuffer =
+          MP4Metadata::Metadata(stream);
         MP4Metadata metadata(stream);
 
         if (stream->mHighestSuccessfulEndOffset <= 0) {
           // No successful reads -> Cutting down the size won't change anything.
           break;
         }
         if (stream->mHighestSuccessfulEndOffset < size) {
           // Read up to a point before the end -> Resize down to that point.
@@ -547,18 +552,20 @@ uint8_t media_libstagefright_gtest_video
 const uint32_t media_libstagefright_gtest_video_init_mp4_len = 745;
 
 TEST(stagefright_MP4Metadata, EmptyCTTS)
 {
   RefPtr<MediaByteBuffer> buffer = new MediaByteBuffer(media_libstagefright_gtest_video_init_mp4_len);
   buffer->AppendElements(media_libstagefright_gtest_video_init_mp4, media_libstagefright_gtest_video_init_mp4_len);
   RefPtr<BufferStream> stream = new BufferStream(buffer);
 
-  RefPtr<MediaByteBuffer> metadataBuffer = MP4Metadata::Metadata(stream);
-  EXPECT_TRUE(metadataBuffer);
+  MP4Metadata::ResultAndByteBuffer metadataBuffer =
+    MP4Metadata::Metadata(stream);
+  EXPECT_EQ(NS_OK, metadataBuffer.Result());
+  EXPECT_TRUE(metadataBuffer.Ref());
 
   MP4Metadata metadata(stream);
 
   EXPECT_EQ(1u, metadata.GetNumberTracks(TrackInfo::kVideoTrack));
   mozilla::UniquePtr<mozilla::TrackInfo> track =
     metadata.GetTrackInfo(TrackInfo::kVideoTrack, 0);
   EXPECT_TRUE(track != nullptr);
   // We can seek anywhere in any MPEG4.