Status
()
People
(Reporter: ayang, Assigned: ayang)
Tracking
Firefox Tracking Flags
(firefox59 fixed)
Details
Attachments
(7 attachments)
|
59 bytes,
text/x-review-board-request
|
kinetik
:
review+
|
Details |
|
59 bytes,
text/x-review-board-request
|
kinetik
:
review+
|
Details |
|
59 bytes,
text/x-review-board-request
|
kinetik
:
review+
|
Details |
|
59 bytes,
text/x-review-board-request
|
kinetik
:
review+
|
Details |
|
59 bytes,
text/x-review-board-request
|
kinetik
:
review+
|
Details |
|
59 bytes,
text/x-review-board-request
|
kinetik
:
review+
|
Details |
|
59 bytes,
text/x-review-board-request
|
kinetik
:
review+
|
Details |
After bug 1415809, we could move parser codes out of stagefright folder and then remove stagefright from gecko. Current idea: media/libstagefright/binding/mp4parse => media/mp4parse media/libstagefright/binding/*.cpp, media/libstagefright/binding/*.h => dom/media/fmp4 (maybe we should rename it to 'mp4'?) Any suggestion?
|
Assignee | |
Updated•2 years ago
|
Flags: needinfo?(kinetik)
Flags: needinfo?(gsquelart)
Flags: needinfo?(cpearce)
|
||
Comment 1•2 years ago
|
||
media/blah indicates external code, but it's really not. I believe the MoofParser , Index, Box, SinfParser and related mp4 code (but not rust metadata) be moved in dom/media/fmp4 H264 / SPS / ADTS parsing should be moved to dom/media/platforms/agnostic BufferReader, BitReader (that needs to be rewritten to no longer use stagefright bit reader) be moved to dom/media
|
||
Updated•2 years ago
|
Flags: needinfo?(gsquelart)
|
||
Updated•2 years ago
|
Flags: needinfo?(kinetik)
|
||
Comment 2•2 years ago
|
||
I agree with jya's suggestion. We should probably rename dom/media/fmp4 to dom/media/mp4, as it has not been solely about fragmented mp4 for a long time now.
Flags: needinfo?(cpearce)
| Comment hidden (mozreview-request) |
| Comment hidden (mozreview-request) |
| Comment hidden (mozreview-request) |
| Comment hidden (mozreview-request) |
| Comment hidden (mozreview-request) |
| Comment hidden (mozreview-request) |
| Comment hidden (mozreview-request) |
|
|
Assignee | |
Comment 10•2 years ago
|
||
Not sure what's wrong in 'OS X Cross Compiled opt'. https://treeherder.mozilla.org/#/jobs?repo=try&revision=1c79facbb191c27d858c1fa72b7ec9d0e3c4b63d
|
|
Assignee | |
Comment 11•2 years ago
|
||
| mozreview-review | ||
Comment on attachment 8928432 [details] Bug 1417011 - move MP4Metadata and MoofParser to ./dom/media/mp4. https://reviewboard.mozilla.org/r/199674/#review204766 ::: dom/media/mp4/moz.build:80 (Diff revision 1) > + CXXFLAGS += [ > + '-Wno-mismatched-tags', > + '-Wno-tautological-constant-out-of-range-compare', > + '-Wno-unreachable-code-return', > + '-Wno-implicit-fallthrough', > + ] I'll open a follow up bug to remove these things.
|
|
||
Comment 12•2 years ago
|
||
| mozreview-review | ||
Comment on attachment 8928431 [details] Bug 1417011 - move Adts.h, AnnexB.h and H264.h to agnostic/mp4_demuxer. https://reviewboard.mozilla.org/r/199672/#review204768 ::: commit-message-91920:1 (Diff revision 1) > +Bug 1417011 - move Adts.h, AnnexB.h and H264.h to agnostic/mp4_demuxer. r?kinetik this has little to do with mp4. it just happens that h264 or aac is used in a mp4 container. On second thought, this should go into dom/media/platforms/agnostic/bytestreams or dom/media/platforms/bytestreams
|
|
||
Comment 13•2 years ago
|
||
| mozreview-review | ||
Comment on attachment 8928430 [details] Bug 1417011 - move BufferReader, BitReader and ByteWriter to dom/media. https://reviewboard.mozilla.org/r/199670/#review204770 ::: commit-message-43822:1 (Diff revision 1) > +Bug 1417011 - move BufferReader, BitReader and ByteWriter to dom/media. r?kinetik can you have a following patch that remove the use of the mp4_demuxer namespace?
|
|
||
Comment 14•2 years ago
|
||
| mozreview-review | ||
Comment on attachment 8928434 [details] Bug 1417011 - remove stagefright folder. https://reviewboard.mozilla.org/r/199678/#review204772 ::: commit-message-d6b4a:1 (Diff revision 1) > +Bug 1417011 - remove stagefright folder. r?kinetik \o/
|
|
||
Comment 15•2 years ago
|
||
| mozreview-review | ||
Comment on attachment 8928428 [details] Bug 1417011 - rename fmp4 to mp4. https://reviewboard.mozilla.org/r/199666/#review205054
Attachment #8928428 -
Flags: review?(kinetik) → review+
|
|
||
Comment 16•2 years ago
|
||
| mozreview-review | ||
Comment on attachment 8928429 [details] Bug 1417011 - move rust mp4 parser to media/mp4parse-rust. https://reviewboard.mozilla.org/r/199668/#review205056 ::: media/mp4parse-rust/update-rust.sh:40 (Diff revision 1) > cp _upstream/mp4parse/mp4parse/tests/*.mp4 mp4parse/tests/ > rm -rf mp4parse_capi > mkdir -p mp4parse_capi/src > cp _upstream/mp4parse/mp4parse_capi/Cargo.toml mp4parse_capi/ > cp _upstream/mp4parse/mp4parse_capi/build.rs mp4parse_capi/ > -cp _upstream/mp4parse/mp4parse_capi/include/mp4parse.h include/ > +cp _upstream/mp4parse/mp4parse_capi/include/mp4parse.h ../../dom/media/mp4/ It's probably better to keep the mp4parse.h include within media/mp4parse-rust and export that include dir to dom/media, so that everything is in one place.
Attachment #8928429 -
Flags: review?(kinetik) → review+
|
|
||
Comment 17•2 years ago
|
||
| mozreview-review | ||
Comment on attachment 8928430 [details] Bug 1417011 - move BufferReader, BitReader and ByteWriter to dom/media. https://reviewboard.mozilla.org/r/199670/#review205058
Attachment #8928430 -
Flags: review?(kinetik) → review+
|
|
||
Comment 18•2 years ago
|
||
| mozreview-review | ||
Comment on attachment 8928431 [details] Bug 1417011 - move Adts.h, AnnexB.h and H264.h to agnostic/mp4_demuxer. https://reviewboard.mozilla.org/r/199672/#review205060 ::: dom/media/platforms/moz.build:42 (Diff revision 1) > ] > > DIRS += [ > 'agnostic/eme', > 'agnostic/gmp', > + 'agnostic/mp4_demuxer', r+ with the path jya suggested.
Attachment #8928431 -
Flags: review?(kinetik) → review+
|
|
||
Comment 19•2 years ago
|
||
| mozreview-review | ||
Comment on attachment 8928432 [details] Bug 1417011 - move MP4Metadata and MoofParser to ./dom/media/mp4. https://reviewboard.mozilla.org/r/199674/#review205062 ::: dom/media/mp4/moz.build:47 (Diff revision 1) > 'MP4Demuxer.cpp', > ] > > FINAL_LIBRARY = 'xul' > + > +# Depress warnnings for now. s/depress/suppress/
Attachment #8928432 -
Flags: review?(kinetik) → review+
|
|
||
Comment 20•2 years ago
|
||
| mozreview-review | ||
Comment on attachment 8928433 [details] Bug 1417011 - move mp4 gtest. https://reviewboard.mozilla.org/r/199676/#review205064 ::: commit-message-58cbb:3 (Diff revision 1) > +Bug 1417011 - move mp4 gtest. r?kinetik > + > +MozReview-Commit-ID: JvB0pEeHNZN Can also rename TestMP4Rust.cpp to remove "Rust"
Attachment #8928433 -
Flags: review?(kinetik) → review+
|
|
||
Comment 21•2 years ago
|
||
| mozreview-review | ||
Comment on attachment 8928434 [details] Bug 1417011 - remove stagefright folder. https://reviewboard.mozilla.org/r/199678/#review205066 ::: commit-message-d6b4a:1 (Diff revision 1) > +Bug 1417011 - remove stagefright folder. r?kinetik This is awesome, thanks for doing this!
Attachment #8928434 -
Flags: review?(kinetik) → review+
| Comment hidden (mozreview-request) |
| Comment hidden (mozreview-request) |
| Comment hidden (mozreview-request) |
| Comment hidden (mozreview-request) |
| Comment hidden (mozreview-request) |
| Comment hidden (mozreview-request) |
| Comment hidden (mozreview-request) |
|
|
Assignee | |
Comment 29•2 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=e1db3f0eeab297934fe2a572b30329fa6dc076a7
|
|
Assignee | |
Comment 30•2 years ago
|
||
(In reply to Jean-Yves Avenard [:jya] from comment #13) > Comment on attachment 8928430 [details] > Bug 1417011 - move BufferReader, BitReader and ByteWriter to dom/media. > > https://reviewboard.mozilla.org/r/199670/#review204770 > > ::: commit-message-43822:1 > (Diff revision 1) > > +Bug 1417011 - move BufferReader, BitReader and ByteWriter to dom/media. r?kinetik > > can you have a following patch that remove the use of the mp4_demuxer > namespace? Will do.
|
|
Assignee | |
Comment 31•2 years ago
|
||
(In reply to Jean-Yves Avenard [:jya] from comment #12) > Comment on attachment 8928431 [details] > Bug 1417011 - move Adts.h, AnnexB.h and H264.h to agnostic/mp4_demuxer. > > https://reviewboard.mozilla.org/r/199672/#review204768 > > ::: commit-message-91920:1 > (Diff revision 1) > > +Bug 1417011 - move Adts.h, AnnexB.h and H264.h to agnostic/mp4_demuxer. r?kinetik > > this has little to do with mp4. > > it just happens that h264 or aac is used in a mp4 container. > > On second thought, this should go into > dom/media/platforms/agnostic/bytestreams > or > dom/media/platforms/bytestreams Will do
|
||
Comment 32•2 years ago
|
||
Pushed by ayang@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/e289efd23095 rename fmp4 to mp4. r=kinetik https://hg.mozilla.org/integration/autoland/rev/441d3a12c9ad move rust mp4 parser to media/mp4parse-rust. r=kinetik https://hg.mozilla.org/integration/autoland/rev/d20919a219d3 move BufferReader, BitReader and ByteWriter to dom/media. r=kinetik https://hg.mozilla.org/integration/autoland/rev/583fc8854af0 move Adts.h, AnnexB.h and H264.h to agnostic/mp4_demuxer. r=kinetik https://hg.mozilla.org/integration/autoland/rev/874157f43b66 move MP4Metadata and MoofParser to ./dom/media/mp4. r=kinetik https://hg.mozilla.org/integration/autoland/rev/4434b4005377 move mp4 gtest. r=kinetik https://hg.mozilla.org/integration/autoland/rev/fac2ad4b4229 remove stagefright folder. r=kinetik
|
|
||
Comment 33•2 years ago
|
||
Unfortunate that this was pushed as is, when change of path was due to be done.
|
||
Comment 34•2 years ago
|
||
| bugherder | ||
https://hg.mozilla.org/mozilla-central/rev/e289efd23095 https://hg.mozilla.org/mozilla-central/rev/441d3a12c9ad https://hg.mozilla.org/mozilla-central/rev/d20919a219d3 https://hg.mozilla.org/mozilla-central/rev/583fc8854af0 https://hg.mozilla.org/mozilla-central/rev/874157f43b66 https://hg.mozilla.org/mozilla-central/rev/4434b4005377 https://hg.mozilla.org/mozilla-central/rev/fac2ad4b4229
Status: NEW → RESOLVED
Closed: 2 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
•