Bug 1343156 - Remove unnecessary MP4Metadata::HasCompleteMetadata - r=jya
authorGerald Squelart <gsquelart@mozilla.com>
Mon, 27 Feb 2017 10:06:45 +1100
changeset 374235 67617bd5c3e8cf3d44b5afdfa24257c2d2a131fe
parent 374234 93829484f746d0d2d6ad5eee55b0b1df000401c0
child 374236 13758ac39b56f55194fe7bee6befc9bfa8d10a2e
push id10863
push userjlorenzo@mozilla.com
push dateMon, 06 Mar 2017 23:02:23 +0000
treeherdermozilla-aurora@0931190cd725 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjya
bugs1343156
milestone54.0a1
Bug 1343156 - Remove unnecessary MP4Metadata::HasCompleteMetadata - r=jya MP4Metadata::Metadata() contains the same code at the beginning, so calling HasCompleteMetadata() is unnecessary, so we should just remove it completely. (Except to get a better error message, but this will be reinstated in an upcoming bug.) MozReview-Commit-ID: 2C3GI5fE0Ja
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
@@ -119,24 +119,16 @@ MP4Demuxer::MP4Demuxer(MediaResource* aR
 {
 }
 
 RefPtr<MP4Demuxer::InitPromise>
 MP4Demuxer::Init()
 {
   AutoPinned<mp4_demuxer::ResourceStream> stream(mStream);
 
-  // Check that we have enough data to read the metadata.
-  if (!mp4_demuxer::MP4Metadata::HasCompleteMetadata(stream)) {
-    return InitPromise::CreateAndReject(
-      MediaResult(NS_ERROR_DOM_MEDIA_DEMUXER_ERR,
-                  RESULT_DETAIL("Incomplete MP4 metadata")),
-      __func__);
-  }
-
   RefPtr<MediaByteBuffer> initData = mp4_demuxer::MP4Metadata::Metadata(stream);
   if (!initData) {
     return InitPromise::CreateAndReject(
       MediaResult(NS_ERROR_DOM_MEDIA_DEMUXER_ERR,
                   RESULT_DETAIL("Invalid MP4 metadata or OOM")),
       __func__);
   }
 
--- a/media/libstagefright/binding/MP4Metadata.cpp
+++ b/media/libstagefright/binding/MP4Metadata.cpp
@@ -76,17 +76,16 @@ private:
 };
 
 class MP4MetadataStagefright
 {
 public:
   explicit MP4MetadataStagefright(Stream* aSource);
   ~MP4MetadataStagefright();
 
-  static bool HasCompleteMetadata(Stream* aSource);
   static already_AddRefed<mozilla::MediaByteBuffer> 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;
 
@@ -126,17 +125,16 @@ private:
 };
 
 class MP4MetadataRust
 {
 public:
   explicit MP4MetadataRust(Stream* aSource);
   ~MP4MetadataRust();
 
-  static bool HasCompleteMetadata(Stream* aSource);
   static already_AddRefed<mozilla::MediaByteBuffer> 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;
 
@@ -166,22 +164,16 @@ MP4Metadata::MP4Metadata(Stream* aSource
 #endif
 {
 }
 
 MP4Metadata::~MP4Metadata()
 {
 }
 
-/*static*/ bool
-MP4Metadata::HasCompleteMetadata(Stream* aSource)
-{
-  return MP4MetadataStagefright::HasCompleteMetadata(aSource);
-}
-
 /*static*/ already_AddRefed<mozilla::MediaByteBuffer>
 MP4Metadata::Metadata(Stream* aSource)
 {
   return MP4MetadataStagefright::Metadata(aSource);
 }
 
 static const char *
 TrackTypeToString(mozilla::TrackInfo::TrackType aType)
@@ -608,23 +600,16 @@ MP4MetadataStagefright::GetTrackNumber(m
     int32_t value;
     if (metaData->findInt32(kKeyTrackID, &value) && value == aTrackID) {
       return i;
     }
   }
   return -1;
 }
 
-/*static*/ bool
-MP4MetadataStagefright::HasCompleteMetadata(Stream* aSource)
-{
-  auto parser = mozilla::MakeUnique<MoofParser>(aSource, 0, false);
-  return parser->HasMetadata();
-}
-
 /*static*/ already_AddRefed<mozilla::MediaByteBuffer>
 MP4MetadataStagefright::Metadata(Stream* aSource)
 {
   auto parser = mozilla::MakeUnique<MoofParser>(aSource, 0, false);
   return parser->Metadata();
 }
 
 #ifdef MOZ_RUST_MP4PARSE
@@ -882,23 +867,16 @@ MP4MetadataRust::ReadTrackIndice(mp4pars
   rv = mp4parse_get_indice_table(mRustParser.get(), aTrackID, aIndices);
   if (rv != MP4PARSE_OK) {
     return false;
   }
 
   return true;
 }
 
-/*static*/ bool
-MP4MetadataRust::HasCompleteMetadata(Stream* aSource)
-{
-  MOZ_ASSERT(false, "Not yet implemented");
-  return false;
-}
-
 /*static*/ already_AddRefed<mozilla::MediaByteBuffer>
 MP4MetadataRust::Metadata(Stream* aSource)
 {
   MOZ_ASSERT(false, "Not yet implemented");
   return nullptr;
 }
 #endif
 
--- a/media/libstagefright/binding/include/mp4_demuxer/MP4Metadata.h
+++ b/media/libstagefright/binding/include/mp4_demuxer/MP4Metadata.h
@@ -19,17 +19,16 @@ class MP4MetadataStagefright;
 class MP4MetadataRust;
 
 class MP4Metadata
 {
 public:
   explicit MP4Metadata(Stream* aSource);
   ~MP4Metadata();
 
-  static bool HasCompleteMetadata(Stream* aSource);
   static already_AddRefed<mozilla::MediaByteBuffer> 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;
 
--- a/media/libstagefright/gtest/TestParser.cpp
+++ b/media/libstagefright/gtest/TestParser.cpp
@@ -67,17 +67,16 @@ protected:
   const uint8_t* mBuffer;
   size_t mSize;
 };
 
 TEST(stagefright_MP4Metadata, EmptyStream)
 {
   RefPtr<Stream> stream = new TestStream(nullptr, 0);
 
-  EXPECT_FALSE(MP4Metadata::HasCompleteMetadata(stream));
   RefPtr<MediaByteBuffer> metadataBuffer = MP4Metadata::Metadata(stream);
   EXPECT_FALSE(metadataBuffer);
 
   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));
@@ -208,17 +207,16 @@ static const TestFileData testFiles[] = 
 
 TEST(stagefright_MPEG4Metadata, test_case_mp4)
 {
   for (size_t test = 0; test < ArrayLength(testFiles); ++test) {
     nsTArray<uint8_t> buffer = ReadTestFile(testFiles[test].mFilename);
     ASSERT_FALSE(buffer.IsEmpty());
     RefPtr<Stream> stream = new TestStream(buffer.Elements(), buffer.Length());
 
-    EXPECT_TRUE(MP4Metadata::HasCompleteMetadata(stream));
     RefPtr<MediaByteBuffer> metadataBuffer = MP4Metadata::Metadata(stream);
     EXPECT_TRUE(metadataBuffer);
 
     MP4Metadata metadata(stream);
     EXPECT_EQ(0u, metadata.GetNumberTracks(TrackInfo::kUndefinedTrack));
     EXPECT_EQ(testFiles[test].mNumberAudioTracks,
               metadata.GetNumberTracks(TrackInfo::kAudioTrack));
     EXPECT_EQ(testFiles[test].mNumberVideoTracks,
@@ -284,17 +282,16 @@ 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);
 
-        MP4Metadata::HasCompleteMetadata(stream);
         RefPtr<MediaByteBuffer> 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) {
@@ -460,17 +457,16 @@ 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);
 
-  EXPECT_TRUE(MP4Metadata::HasCompleteMetadata(stream));
   RefPtr<MediaByteBuffer> metadataBuffer = MP4Metadata::Metadata(stream);
   EXPECT_TRUE(metadataBuffer);
 
   MP4Metadata metadata(stream);
 
   EXPECT_EQ(1u, metadata.GetNumberTracks(TrackInfo::kVideoTrack));
   mozilla::UniquePtr<mozilla::TrackInfo> track =
     metadata.GetTrackInfo(TrackInfo::kVideoTrack, 0);