If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Get stagefright indices only when rust parser is off or TestMode is on.

RESOLVED FIXED in Firefox 55

Status

()

Core
Audio/Video: Playback
RESOLVED FIXED
6 months ago
6 months ago

People

(Reporter: alfredo, Assigned: alfredo)

Tracking

unspecified
mozilla55
Points:
---

Firefox Tracking Flags

(firefox55 fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

(Assignee)

Description

6 months ago
Indice table could occupy significant memory, we don't need to keep both if TestMode is off.
Comment hidden (mozreview-request)

Comment 2

6 months 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

6 months 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

6 months 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

6 months 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

6 months 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!

Comment 9

6 months ago
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

6 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/603c4b701a67
Status: NEW → RESOLVED
Last Resolved: 6 months 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.