From af3e91f01af4e6a4a55a5fb341209b0f592db316 Mon Sep 17 00:00:00 2001 From: Jean-Yves Avenard Date: Fri, 21 Aug 2015 14:45:30 +1000 Subject: [PATCH] Revert "Bug 1196398: [mp4] Do not allocate memory spanning across ftyp and moov atom. r=kentuckyfriedtakahe" This reverts commit 73156610be5f --- dom/media/fmp4/MP4Demuxer.cpp | 12 +++-- media/libstagefright/binding/MP4Metadata.cpp | 9 ++-- media/libstagefright/binding/MoofParser.cpp | 52 ++++--------------- .../binding/include/mp4_demuxer/MP4Metadata.h | 2 +- .../binding/include/mp4_demuxer/MoofParser.h | 3 -- 5 files changed, 25 insertions(+), 53 deletions(-) diff --git a/dom/media/fmp4/MP4Demuxer.cpp b/dom/media/fmp4/MP4Demuxer.cpp index 0a95ac56c86e..a21ff213941f 100644 --- a/dom/media/fmp4/MP4Demuxer.cpp +++ b/dom/media/fmp4/MP4Demuxer.cpp @@ -79,16 +79,22 @@ MP4Demuxer::Init() AutoPinned stream(mStream); // Check that we have enough data to read the metadata. - if (!mp4_demuxer::MP4Metadata::HasCompleteMetadata(stream)) { + MediaByteRange br = mp4_demuxer::MP4Metadata::MetadataRange(stream); + if (br.IsNull()) { return InitPromise::CreateAndReject(DemuxerFailureReason::WAITING_FOR_DATA, __func__); } - mInitData = mp4_demuxer::MP4Metadata::Metadata(stream); - if (!mInitData) { + if (!mInitData->SetLength(br.Length(), fallible)) { // OOM return InitPromise::CreateAndReject(DemuxerFailureReason::DEMUXER_ERROR, __func__); } + size_t size; + mStream->ReadAt(br.mStart, mInitData->Elements(), br.Length(), &size); + if (size != size_t(br.Length())) { + return InitPromise::CreateAndReject(DemuxerFailureReason::DEMUXER_ERROR, __func__); + } + nsRefPtr bufferstream = new mp4_demuxer::BufferStream(mInitData); diff --git a/media/libstagefright/binding/MP4Metadata.cpp b/media/libstagefright/binding/MP4Metadata.cpp index dfa488c3d0ac..5effe9f44998 100644 --- a/media/libstagefright/binding/MP4Metadata.cpp +++ b/media/libstagefright/binding/MP4Metadata.cpp @@ -287,14 +287,17 @@ MP4Metadata::HasCompleteMetadata(Stream* aSource) return parser->HasMetadata(); } -/*static*/ already_AddRefed -MP4Metadata::Metadata(Stream* aSource) +/*static*/ mozilla::MediaByteRange +MP4Metadata::MetadataRange(Stream* aSource) { // The MoofParser requires a monitor, but we don't need one here. mozilla::Monitor monitor("MP4Metadata::HasCompleteMetadata"); mozilla::MonitorAutoLock mon(monitor); auto parser = mozilla::MakeUnique(aSource, 0, false, &monitor); - return parser->Metadata(); + if (parser->HasMetadata()) { + return parser->mInitRange; + } + return mozilla::MediaByteRange(); } } // namespace mp4_demuxer diff --git a/media/libstagefright/binding/MoofParser.cpp b/media/libstagefright/binding/MoofParser.cpp index 61a1ec2c720e..7bb7d090de03 100644 --- a/media/libstagefright/binding/MoofParser.cpp +++ b/media/libstagefright/binding/MoofParser.cpp @@ -148,9 +148,8 @@ MoofParser::BlockingReadNextMoof() return false; } -void -MoofParser::ScanForMetadata(mozilla::MediaByteRange& aFtyp, - mozilla::MediaByteRange& aMoov) +bool +MoofParser::HasMetadata() { int64_t length = std::numeric_limits::max(); mSource->Length(&length); @@ -158,57 +157,24 @@ MoofParser::ScanForMetadata(mozilla::MediaByteRange& aFtyp, byteRanges.AppendElement(MediaByteRange(0, length)); nsRefPtr stream = new BlockingStream(mSource); + MediaByteRange ftyp; + MediaByteRange moov; BoxContext context(stream, byteRanges); for (Box box(&context, mOffset); box.IsAvailable(); box = box.Next()) { if (box.IsType("ftyp")) { - aFtyp = box.Range(); + ftyp = box.Range(); continue; } if (box.IsType("moov")) { - aMoov = box.Range(); + moov = box.Range(); break; } } - mInitRange = aFtyp.Extents(aMoov); -} - -bool -MoofParser::HasMetadata() -{ - MediaByteRange ftyp; - MediaByteRange moov; - ScanForMetadata(ftyp, moov); - return !!ftyp.Length() && !!moov.Length(); -} - -already_AddRefed -MoofParser::Metadata() -{ - MediaByteRange ftyp; - MediaByteRange moov; - ScanForMetadata(ftyp, moov); if (!ftyp.Length() || !moov.Length()) { - return nullptr; + return false; } - nsRefPtr metadata = new MediaByteBuffer(); - if (!metadata->SetLength(ftyp.Length() + moov.Length(), fallible)) { - // OOM - return nullptr; - } - - nsRefPtr stream = new BlockingStream(mSource); - size_t read; - bool rv = - stream->ReadAt(ftyp.mStart, metadata->Elements(), ftyp.Length(), &read); - if (!rv || read != ftyp.Length()) { - return nullptr; - } - rv = - stream->ReadAt(moov.mStart, metadata->Elements() + ftyp.Length(), moov.Length(), &read); - if (!rv || read != moov.Length()) { - return nullptr; - } - return metadata.forget(); + mInitRange = ftyp.Extents(moov); + return true; } Interval diff --git a/media/libstagefright/binding/include/mp4_demuxer/MP4Metadata.h b/media/libstagefright/binding/include/mp4_demuxer/MP4Metadata.h index 28a79bcec755..de9273c2725d 100644 --- a/media/libstagefright/binding/include/mp4_demuxer/MP4Metadata.h +++ b/media/libstagefright/binding/include/mp4_demuxer/MP4Metadata.h @@ -30,7 +30,7 @@ public: ~MP4Metadata(); static bool HasCompleteMetadata(Stream* aSource); - static already_AddRefed Metadata(Stream* aSource); + static mozilla::MediaByteRange MetadataRange(Stream* aSource); uint32_t GetNumberTracks(mozilla::TrackInfo::TrackType aType) const; mozilla::UniquePtr GetTrackInfo(mozilla::TrackInfo::TrackType aType, size_t aTrackNumber) const; diff --git a/media/libstagefright/binding/include/mp4_demuxer/MoofParser.h b/media/libstagefright/binding/include/mp4_demuxer/MoofParser.h index 742e1a72b11b..86daead28cd4 100644 --- a/media/libstagefright/binding/include/mp4_demuxer/MoofParser.h +++ b/media/libstagefright/binding/include/mp4_demuxer/MoofParser.h @@ -228,7 +228,6 @@ public: bool BlockingReadNextMoof(); bool HasMetadata(); - already_AddRefed Metadata(); MediaByteRange FirstCompleteMediaSegment(); MediaByteRange FirstCompleteMediaHeader(); @@ -245,8 +244,6 @@ public: Monitor* mMonitor; nsTArray& Moofs() { mMonitor->AssertCurrentThreadOwns(); return mMoofs; } private: - void ScanForMetadata(mozilla::MediaByteRange& aFtyp, - mozilla::MediaByteRange& aMoov); nsTArray mMoofs; nsTArray mMediaRanges; bool mIsAudio;