Note: There are a few cases of duplicates in user autocompletion which are being worked on.

Call rust mp4parser from MP4Metadata

RESOLVED FIXED in Firefox 45

Status

()

Core
Audio/Video: Playback
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: rillian, Assigned: rillian)

Tracking

(Blocks: 1 bug)

Trunk
mozilla45
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox44 affected, firefox45 fixed)

Details

Attachments

(1 attachment, 2 obsolete attachments)

Anthony tells me sooner is better than later, so we should land my hacked-up buffer-copy patch calling the rust parser on init segments as soon as it's safe.
Created attachment 8679721 [details] [diff] [review]
Call rust mp4 parser v1

Here's my WIP hack. Needs conditionals, and seems to crash on quit.

Anthony, you can apply this and compile with `ac_add_options --enable-rust` in your mozconfig. Then:

./mach run bruce_vs_ironman_frag.mp4
[...]
rust parser found 2 tracks
Flags: needinfo?(ajones)
Depends on: 1219530
Pushed a version with MOZ_RUST conditionals (needs bug 1219530) to try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=9fb0897f1f98

I don't see the shutdown crash on Linux x64 debug with rustc 1.3.0... it'd be good to track that down.
The crash happened when I killed firefox (ctrl-C on the terminal) while a video was playing. Doesn't seem to happen if I quit from the gui.
Depends on: 1220754
Depends on: 1219983
Created attachment 8682104 [details] [diff] [review]
Call rust mp4 parser v2

Let's get something landed.
Attachment #8679721 - Attachment is obsolete: true
Attachment #8682104 - Flags: review?(kinetik)
Comment on attachment 8682104 [details] [diff] [review]
Call rust mp4 parser v2

Review of attachment 8682104 [details] [diff] [review]:
-----------------------------------------------------------------

r+ with assert converted to an error return.

::: media/libstagefright/binding/MP4Metadata.cpp
@@ +118,5 @@
> +    fprintf(stderr, "Couldn't get source length\n");
> +    return false;
> +  }
> +  fprintf(stderr, "Source length %d bytes\n", (long long int)length);
> +  MOZ_ASSERT(length > 0);

My earlier try run showed this failing; Length() can return -1 for unknown stream lengths I guess.
Attachment #8682104 - Flags: review?(kinetik) → review+
Created attachment 8682169 [details] [diff] [review]
Call rust mp4 parser v3

Convert assert to error return per review comment. Carrying forward r=kinetik.
Attachment #8682104 - Attachment is obsolete: true
Attachment #8682169 - Flags: review+

Comment 7

2 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/4a48d8551417
This patch will be functional once bug 1219530 lands. Until then you need to #define MOZ_RUST_MP4PARSE in media/libstagefright/binding/MP4Metadata.cpp.
Flags: needinfo?(ajones)
Blocks: 1220882
Blocks: 1220885
https://hg.mozilla.org/mozilla-central/rev/4a48d8551417
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox45: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla45
https://hg.mozilla.org/releases/mozilla-b2g44_v2_5/rev/4a48d8551417
status-b2g-v2.5: --- → fixed
removing the b2g 2.5 flag since this commit has been reverted due to an incorrect merge, sorry for the confusion
status-b2g-v2.5: fixed → ---
You need to log in before you can comment on or make changes to this bug.