Closed
Bug 1350178
Opened 7 years ago
Closed 7 years ago
Get stagefright indices only when rust parser is off or TestMode is on.
Categories
(Core :: Audio/Video: Playback, enhancement)
Core
Audio/Video: Playback
Tracking
()
RESOLVED
FIXED
mozilla55
Tracking | Status | |
---|---|---|
firefox55 | --- | fixed |
People
(Reporter: ayang, Assigned: ayang)
Details
Attachments
(1 file)
Indice table could occupy significant memory, we don't need to keep both if TestMode is off.
Comment hidden (mozreview-request) |
Comment 2•7 years ago
|
||
mozreview-review |
Comment on attachment 8850797 [details] Bug 1350178 - Get stagefright indices only when rust parser is off or TestMode is on for saving memory usage. https://reviewboard.mozilla.org/r/123316/#review125702 ::: media/libstagefright/binding/MP4Metadata.cpp:426 (Diff revision 1) > mozilla::UniquePtr<IndiceWrapper> > MP4Metadata::GetTrackIndice(mozilla::TrackID aTrackID) > { > FallibleTArray<Index::Indice> indiceSF; > - if(!mStagefright->ReadTrackIndex(indiceSF, aTrackID)) { > + if ((mRustTestMode || !mPreferRust) && > + mStagefright->ReadTrackIndex(indiceSF, aTrackID)) { Originally this returned nullptr if ReadTrackIndex failed, now it returns nullptr if it succeeds (as long as `(mRustTestMode || !mPreferRust)` is true). Was that intentional?
Attachment #8850797 -
Flags: review?(kinetik) → review-
Comment hidden (mozreview-request) |
Comment 4•7 years ago
|
||
mozreview-review |
Comment on attachment 8850797 [details] Bug 1350178 - Get stagefright indices only when rust parser is off or TestMode is on for saving memory usage. https://reviewboard.mozilla.org/r/123316/#review125722 ::: media/libstagefright/binding/MP4Metadata.cpp:425 (Diff revision 2) > > mozilla::UniquePtr<IndiceWrapper> > MP4Metadata::GetTrackIndice(mozilla::TrackID aTrackID) > { > FallibleTArray<Index::Indice> indiceSF; > - if(!mStagefright->ReadTrackIndex(indiceSF, aTrackID)) { > + if ((mRustTestMode || !mPreferRust) && Can you make that `(!mPreferRust || mRustTestMode)` just for symmetry with the block below that fetches the Rust indices? It makes it slightly easier to spot the difference.
Attachment #8850797 -
Flags: review?(kinetik) → review+
Assignee: nobody → ayang
Comment hidden (mozreview-request) |
Assignee | ||
Comment 6•7 years ago
|
||
mozreview-review-reply |
Comment on attachment 8850797 [details] Bug 1350178 - Get stagefright indices only when rust parser is off or TestMode is on for saving memory usage. https://reviewboard.mozilla.org/r/123316/#review125722 > Can you make that `(!mPreferRust || mRustTestMode)` just for symmetry with the block below that fetches the Rust indices? It makes it slightly easier to spot the difference. Sure, I'll fixed it.
Assignee | ||
Comment 7•7 years ago
|
||
mozreview-review |
Comment on attachment 8850797 [details] Bug 1350178 - Get stagefright indices only when rust parser is off or TestMode is on for saving memory usage. https://reviewboard.mozilla.org/r/123316/#review125758 ::: media/libstagefright/binding/MP4Metadata.cpp:425 (Diff revision 2) > > mozilla::UniquePtr<IndiceWrapper> > MP4Metadata::GetTrackIndice(mozilla::TrackID aTrackID) > { > FallibleTArray<Index::Indice> indiceSF; > - if(!mStagefright->ReadTrackIndex(indiceSF, aTrackID)) { > + if ((mRustTestMode || !mPreferRust) && Sure. I'll fixed it.
Assignee | ||
Comment 8•7 years ago
|
||
mozreview-review-reply |
Comment on attachment 8850797 [details] Bug 1350178 - Get stagefright indices only when rust parser is off or TestMode is on for saving memory usage. https://reviewboard.mozilla.org/r/123316/#review125702 > Originally this returned nullptr if ReadTrackIndex failed, now it returns nullptr if it succeeds (as long as `(mRustTestMode || !mPreferRust)` is true). Was that intentional? Urg... it is a stupid mistake. Thanks for catch it!
Pushed by ayang@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/603c4b701a67 Get stagefright indices only when rust parser is off or TestMode is on for saving memory usage. r=kinetik
Comment 10•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/603c4b701a67
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox55:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
You need to log in
before you can comment on or make changes to this bug.
Description
•