Bug 1415809 - return error when parsing fails. r?kinetik draft
authorAlfredo.Yang <ayang@mozilla.com>
Thu, 09 Nov 2017 16:07:31 +0800
changeset 697574 7f2ee03327571b2420b7680af77b6cc618e18a29
parent 697501 d86385b5d8b9baf8ebadc88fd7c238745790c7d7
child 697575 d2c46b112937b49741ad0538058439216d777b62
push id89046
push userbmo:ayang@mozilla.com
push dateTue, 14 Nov 2017 08:24:44 +0000
reviewerskinetik
bugs1415809
milestone59.0a1
Bug 1415809 - return error when parsing fails. r?kinetik MozReview-Commit-ID: Dlzx9fsGEbE
dom/media/fmp4/MP4Demuxer.cpp
media/libstagefright/binding/MP4Metadata.cpp
media/libstagefright/binding/include/mp4_demuxer/MP4Metadata.h
--- a/dom/media/fmp4/MP4Demuxer.cpp
+++ b/dom/media/fmp4/MP4Demuxer.cpp
@@ -142,16 +142,21 @@ MP4Demuxer::Init()
   } else if (NS_FAILED(initData.Result()) && result == NS_OK) {
     result = Move(initData.Result());
   }
 
   RefPtr<mp4_demuxer::BufferStream> bufferstream =
     new mp4_demuxer::BufferStream(initData.Ref());
 
   mp4_demuxer::MP4Metadata metadata{bufferstream};
+  nsresult rv = metadata.Parse();
+  if (NS_FAILED(rv)) {
+    return InitPromise::CreateAndReject(
+      MediaResult(rv, RESULT_DETAIL("Parse MP4 metadata failed")), __func__);
+  }
 
   auto audioTrackCount = metadata.GetNumberTracks(TrackInfo::kAudioTrack);
   if (audioTrackCount.Ref() == mp4_demuxer::MP4Metadata::NumberTracksError()) {
     if (MediaPrefs::MediaWarningsAsErrors()) {
       return InitPromise::CreateAndReject(
         MediaResult(NS_ERROR_DOM_MEDIA_DEMUXER_ERR,
                     RESULT_DETAIL("Invalid audio track (%s)",
                                   audioTrackCount.Result().Description().get())),
--- a/media/libstagefright/binding/MP4Metadata.cpp
+++ b/media/libstagefright/binding/MP4Metadata.cpp
@@ -61,17 +61,17 @@ public:
   MP4Metadata::ResultAndTrackInfo GetTrackInfo(
     mozilla::TrackInfo::TrackType aType, size_t aTrackNumber) const;
   bool CanSeek() const;
 
   MP4Metadata::ResultAndCryptoFile Crypto() const;
 
   MediaResult ReadTrackIndice(mp4parse_byte_data* aIndices, mozilla::TrackID aTrackID);
 
-  bool Init();
+  nsresult Init();
 
 private:
   void UpdateCrypto();
   Maybe<uint32_t> TrackTypeToGlobalTrackIndex(mozilla::TrackInfo::TrackType aType, size_t aTrackNumber) const;
 
   CryptoFile mCrypto;
   RefPtr<Stream> mSource;
   RustStreamAdaptor mRustSource;
@@ -159,23 +159,28 @@ IndiceWrapperRust::GetIndice(size_t aInd
   return true;
 }
 
 MP4Metadata::MP4Metadata(Stream* aSource)
  : mRust(MakeUnique<MP4MetadataRust>(aSource))
  , mReportedAudioTrackTelemetry(false)
  , mReportedVideoTrackTelemetry(false)
 {
-  mRust->Init();
 }
 
 MP4Metadata::~MP4Metadata()
 {
 }
 
+nsresult
+MP4Metadata::Parse() const
+{
+  return mRust->Init();
+}
+
 /*static*/ MP4Metadata::ResultAndByteBuffer
 MP4Metadata::Metadata(Stream* aSource)
 {
   auto parser = mozilla::MakeUnique<MoofParser>(aSource, 0, false);
   RefPtr<mozilla::MediaByteBuffer> buffer = parser->Metadata();
   if (!buffer) {
     return {MediaResult(NS_ERROR_DOM_MEDIA_METADATA_ERR,
                         RESULT_DETAIL("Cannot parse metadata")),
@@ -279,47 +284,42 @@ read_source(uint8_t* buffer, uintptr_t s
   }
   return bytes_read;
 }
 
 MP4MetadataRust::MP4MetadataRust(Stream* aSource)
   : mSource(aSource)
   , mRustSource(aSource)
 {
-}
-
-MP4MetadataRust::~MP4MetadataRust()
-{
-}
-
-bool
-MP4MetadataRust::Init()
-{
   mp4parse_io io = { read_source, &mRustSource };
   mRustParser.reset(mp4parse_new(&io));
   MOZ_ASSERT(mRustParser);
 
   if (MOZ_LOG_TEST(gMP4MetadataLog, LogLevel::Debug)) {
     mp4parse_log(true);
   }
+}
 
-  mp4parse_status rv = mp4parse_read(mRustParser.get());
-  MOZ_LOG(gMP4MetadataLog, LogLevel::Debug, ("rust parser returned %d\n", rv));
-  Telemetry::Accumulate(Telemetry::MEDIA_RUST_MP4PARSE_SUCCESS,
-                        rv == mp4parse_status_OK);
-  if (rv != mp4parse_status_OK && rv != mp4parse_status_OOM) {
-    MOZ_LOG(gMP4MetadataLog, LogLevel::Info, ("Rust mp4 parser fails to parse this stream."));
-    MOZ_ASSERT(rv > 0);
-    Telemetry::Accumulate(Telemetry::MEDIA_RUST_MP4PARSE_ERROR_CODE, rv);
-    return false;
+MP4MetadataRust::~MP4MetadataRust()
+{
+}
+
+nsresult
+MP4MetadataRust::Init()
+{
+  mp4parse_status rv = mp4parse_read(mParser.get());
+  if (rv != mp4parse_status_OK) {
+    MOZ_LOG(gMP4MetadataLog, LogLevel::Debug, ("Parse failed, return code %d\n", rv));
+    return rv == mp4parse_status_OOM ? NS_ERROR_OUT_OF_MEMORY
+                                     : NS_ERROR_DOM_MEDIA_METADATA_ERR;
   }
 
   UpdateCrypto();
 
-  return true;
+  return NS_OK;
 }
 
 void
 MP4MetadataRust::UpdateCrypto()
 {
   mp4parse_pssh_info info = {};
   if (mp4parse_get_pssh_info(mRustParser.get(), &info) != mp4parse_status_OK) {
     return;
--- a/media/libstagefright/binding/include/mp4_demuxer/MP4Metadata.h
+++ b/media/libstagefright/binding/include/mp4_demuxer/MP4Metadata.h
@@ -69,16 +69,18 @@ public:
 
   using ResultAndTrackInfo =
     ResultAndType<mozilla::UniquePtr<mozilla::TrackInfo>>;
   ResultAndTrackInfo GetTrackInfo(mozilla::TrackInfo::TrackType aType,
                                   size_t aTrackNumber) const;
 
   bool CanSeek() const;
 
+  nsresult Parse() const;
+
   using ResultAndCryptoFile = ResultAndType<const CryptoFile*>;
   ResultAndCryptoFile Crypto() const;
 
   using ResultAndIndice = ResultAndType<mozilla::UniquePtr<IndiceWrapper>>;
   ResultAndIndice GetTrackIndice(mozilla::TrackID aTrackID);
 
 private:
   UniquePtr<MP4MetadataRust> mRust;