Call rust mp4parser from MP4Metadata

RESOLVED FIXED in Firefox 45



3 years ago
3 years ago


(Reporter: rillian, Assigned: rillian)


Dependency tree / graph

Firefox Tracking Flags

(firefox44 affected, firefox45 fixed)



(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:

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+
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

Comment 9

3 years ago
Last Resolved: 3 years ago
status-firefox45: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla45
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.