Closed
Bug 1418868
Opened 3 years ago
Closed 3 years ago
Remove IndiceWrapper abstract class
Categories
(Core :: Audio/Video: Playback, enhancement, P3)
Core
Audio/Video: Playback
Tracking
()
RESOLVED
FIXED
mozilla59
Tracking | Status | |
---|---|---|
firefox59 | --- | fixed |
People
(Reporter: ayang, Assigned: ayang)
References
Details
Attachments
(1 file)
Since stagefright is removed, we don't need an abstract layer anymore.
Comment hidden (mozreview-request) |
Comment 2•3 years ago
|
||
mozreview-review |
Comment on attachment 8929957 [details] Bug 1418868 - remove IndiceWrapper abstract layer. https://reviewboard.mozilla.org/r/201116/#review206244 ::: dom/media/mp4/MP4Metadata.h:20 (Diff revision 1) > #include "ByteStream.h" > #include "mp4parse.h" > > namespace mozilla { > > -class IndiceWrapper { > +// the owner of mIndice is rust mp4 paser, so lifetime of this class the -> The paser -> parser ::: dom/media/mp4/MP4Metadata.h:21 (Diff revision 1) > #include "mp4parse.h" > > namespace mozilla { > > -class IndiceWrapper { > +// the owner of mIndice is rust mp4 paser, so lifetime of this class > +// SHOULD NOT longer than rust parser. This comment doesn't seem right. mUnique is a UniquePtr, which is owning, and it's allocated inside IndiceWrapper's ctor and the members of aRustIndice are copied over. So there's no connection between an IndiceWrapper and mp4 parser's lifetime, right? It seems like now we could make mIndice be a direct member rather than allocating it, too.
Attachment #8929957 -
Flags: review?(kinetik) → review-
Assignee | ||
Comment 3•3 years ago
|
||
(In reply to Matthew Gregan [:kinetik] from comment #2) > > -class IndiceWrapper { > > +// the owner of mIndice is rust mp4 paser, so lifetime of this class > > +// SHOULD NOT longer than rust parser. > > This comment doesn't seem right. mUnique is a UniquePtr, which is owning, > and it's allocated inside IndiceWrapper's ctor and the members of > aRustIndice are copied over. So there's no connection between an > IndiceWrapper and mp4 parser's lifetime, right? Yes, the memory owned by mp4 parser is the raw data from rust Vec. I'll update it. > > It seems like now we could make mIndice be a direct member rather than > allocating it, too. Will do.
Comment hidden (mozreview-request) |
Comment 5•3 years ago
|
||
mozreview-review |
Comment on attachment 8929957 [details] Bug 1418868 - remove IndiceWrapper abstract layer. https://reviewboard.mozilla.org/r/201116/#review206824
Attachment #8929957 -
Flags: review?(kinetik) → review+
Pushed by ayang@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/05e3e4ff97a5 remove IndiceWrapper abstract layer. r=kinetik
Comment 7•3 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/05e3e4ff97a5
Status: NEW → RESOLVED
Closed: 3 years ago
status-firefox59:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
You need to log in
before you can comment on or make changes to this bug.
Description
•