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)

enhancement
Not set
normal

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 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 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 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.
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.
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
https://hg.mozilla.org/mozilla-central/rev/603c4b701a67
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: